WPF two way binding doesn't work until control is modified

Question

I'm trying to synchronize selection in a DataGrid using a collection in my data. I have this mostly working, with one little quirk.

When I change selection in the DataGrid changes are written to my data collection, so far so good. Then, if the data collection changes selection in my DataGrid is updated, as expected. However, if I modify my data before modifying the DataGrid then the DataGrid selection does not update.

An example of the first, working case

An example of the second, non-working case

Code

using System.Collections;
using System.Collections.ObjectModel;
using System.Windows;
using System.Windows.Controls;

namespace Testbed
{
    public class Widget
    {
        public string Name { get; set; }
    }

    public class Data
    {
        public static Data Instance { get; } = new Data();

        public ObservableCollection<Widget> Widgets         { get; set; } = new ObservableCollection<Widget>();
        public IList                        SelectedWidgets { get; set; } = new ObservableCollection<Widget>();

        Data()
        {
            Widgets.Add(new Widget() { Name = "Widget 1" });
            Widgets.Add(new Widget() { Name = "Widget 2" });
            Widgets.Add(new Widget() { Name = "Widget 3" });
        }
    };

    public class BindableDataGrid : DataGrid
    {
        public static readonly DependencyProperty SelectedItemsProperty = DependencyProperty.Register(
            "SelectedItems",
            typeof(IList),
            typeof(BindableDataGrid),
            new PropertyMetadata(default(IList)));

        public new IList SelectedItems
        {
            get { return (IList) GetValue(SelectedItemsProperty); }
            set { SetValue(SelectedItemsProperty, value); }
        }

        protected override void OnSelectionChanged(SelectionChangedEventArgs e)
        {
            base.OnSelectionChanged(e);
            SetCurrentValue(SelectedItemsProperty, base.SelectedItems);
        }
    }

    public partial class MainWindow : Window
    {
        public MainWindow ()
        {
            InitializeComponent();
        }

        private void Button1_Click(object sender, RoutedEventArgs e) { Button_Clicked(0); }
        private void Button2_Click(object sender, RoutedEventArgs e) { Button_Clicked(1); }
        private void Button3_Click(object sender, RoutedEventArgs e) { Button_Clicked(2); }

        private void Button_Clicked(int index)
        {
            Data data = Data.Instance;
            Widget widget = data.Widgets[index];

            if (data.SelectedWidgets.Contains(widget))
            {
                data.SelectedWidgets.Remove(widget);
            }
            else
            {
                data.SelectedWidgets.Add(widget);
            }
        }
    }
}

And markup

<Window
    x:Class="Testbed.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:test="clr-namespace:Testbed"
    Title="MainWindow"
    Height="480" Width="640"
    DataContext="{Binding Source={x:Static test:Data.Instance}}">

    <Grid>
        <Grid.ColumnDefinitions>
            <ColumnDefinition MinWidth="210" />
            <ColumnDefinition Width="5" />
            <ColumnDefinition MinWidth="210" />
            <ColumnDefinition Width="5" />
            <ColumnDefinition MinWidth="210" />
        </Grid.ColumnDefinitions>

        <!-- Change selection through data -->
        <StackPanel Grid.Column="0">
            <Button Content="Select Widget 1" Click="Button1_Click"/>
            <Button Content="Select Widget 2" Click="Button2_Click"/>
            <Button Content="Select Widget 3" Click="Button3_Click"/>
        </StackPanel>

        <!-- Current selection in data -->
        <DataGrid Grid.Column="2"
            ItemsSource="{Binding SelectedWidgets}"
            IsReadOnly="true">
        </DataGrid>

        <!-- Change selection through UI -->
        <test:BindableDataGrid Grid.Column="4"
            SelectionMode="Extended"
            ColumnWidth="*"
            ItemsSource="{Binding Widgets}"
            SelectedItems="{Binding SelectedWidgets, Mode=TwoWay}"
            IsReadOnly="true">
        <DataGrid.RowStyle>
            <Style TargetType="{x:Type DataGridRow}">
                <Style.Resources>
                <SolidColorBrush x:Key="{x:Static SystemColors.InactiveSelectionHighlightBrushKey}" Color="CornflowerBlue"/>
                </Style.Resources>
            </Style>
        </DataGrid.RowStyle>
        </test:BindableDataGrid>
    </Grid>

</Window>

Answer 1

@Drreamer's answer pointed me in the right direction. However, it boiled down to embracing the fact that the source data collection was being replaced by the DataGrid.SelectedItems collection. It ends up bypassing OnPropertyChanged after the first modification because both ends of the binding are actually the same object.

I didn't want the source collection to be replaced so I found another solution that synchronizes the contents of the collections. It has the benefit of being more direct as well.

When SelectedItems is initialized by the DependencyProperty I stash a reference to the source and target collections. I also register for CollectionChanged on the source and override OnSelectionChanged on the target. Whenever one collection changes I clear the other collection and copy the contents over. As another bonus I no longer have to expose my source collection as IList to allow the DependencyProperty to work since I'm not using it after caching off the source.

    public class BindableDataGrid : DataGrid
    {
        public static readonly DependencyProperty SelectedItemsProperty = DependencyProperty.Register(
            "SelectedItems",
            typeof(IList),
            typeof(BindableDataGrid),
            new PropertyMetadata(OnPropertyChanged));

        private static void OnPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
        {
            BindableDataGrid bdg = (BindableDataGrid) d;
            if (bdg.initialized) return;
            bdg.initialized = true;

            bdg.source = (IList) e.NewValue;
            bdg.target = ((DataGrid) bdg).SelectedItems;
            ((INotifyCollectionChanged) e.NewValue).CollectionChanged += bdg.OnCollectionChanged;
        }

        public new IList SelectedItems
        {
            get { return (IList) GetValue(SelectedItemsProperty); }
            set { SetValue(SelectedItemsProperty, value); }
        }

        IList source;
        IList target;
        bool synchronizing;
        bool initialized;

        private void OnSourceChanged()
        {
            if (synchronizing) return;
            synchronizing = true;
            target.Clear();
            foreach (var item in source)
                target.Add(item);
            synchronizing = false;
        }

        private void OnTargetChanged()
        {
            if (synchronizing) return;
            synchronizing = true;
            source.Clear();
            foreach (var item in target)
                source.Add(item);
            synchronizing = false;
        }

        private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
        {
            OnSourceChanged();
        }

        protected override void OnSelectionChanged(SelectionChangedEventArgs e)
        {
            base.OnSelectionChanged(e);
            OnTargetChanged();
        }
    }

I'm sure there's is a much more elegant way to solve this, but this is the best I've got right now.

Answer 2

The problem occurs because you do not handle notifications of the BindableDataGrid.SelectedItems collection. In the first case you do not need to handle them manually because you actually get the SelectedItems collection from the base DataGrid class and pass it to the view model from the OnSelectionChanged method call. The base DataGrid handle notifications of this collection itself.

However, if you click the button first, the SelectedItems property get a new collection and the base DataGrid knows nothing about it. I think that you need to handle the propertyChangedCallback, and handle notifications of provided collections to update selection in the grid manually. Refer to the following code demonstrating the concept. Note that I have renamed the property for simplicity but still have not debugged it.

public static readonly DependencyProperty SelectedItemsNewProperty = DependencyProperty.Register(
      "SelectedItemsNew",
      typeof(IList),
      typeof(BindableDataGrid), new PropertyMetadata(OnPropertyChanged));
        private static void OnPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) {
     BindableDataGrid bdg = (BindableDataGrid)d;
     if (e.OldValue as INotifyCollectionChanged != null)
        (e.NewValue as INotifyCollectionChanged).CollectionChanged -= bdg.BindableDataGrid_CollectionChanged;
     if (Object.ReferenceEquals(e.NewValue, bdg.SelectedItems))
        return;
     if( e.NewValue as INotifyCollectionChanged != null )
        (e.NewValue as INotifyCollectionChanged).CollectionChanged += bdg.BindableDataGrid_CollectionChanged;
     bdg.SynchronizeSelection(e.NewValue as IList);
  }
  private void BindableDataGrid_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) {
     SynchronizeSelection((IList)sender);
  }
  private void SynchronizeSelection( IList collection) {         
     SelectedItems.Clear();
     if (collection != null) 
        foreach (var item in collection)
           SelectedItems.Add(item);         
  }

Answer 3

This happens because your new SelectedItems property never updates the base SelectedItems when it is set. The problem with that is, of course, that MultiSelector.SelectedItems is readonly. It was designed specifically not to be set-able - but it was also designed to be updatable.

The reason your code works at all is because when you change the selection via the BindableDataGrid, SelectedWidgets gets replaced with the DataGrid's internal SelectedItemsCollection. After that point, you are adding and removing from that collection, so it updates the DataGrid.

Of course, this doesn't work if you haven't changed the selection yet, because OnSelectionChanged doesn't run until then, so SetCurrentValue is never called, so the binding never updated SelectedWidgets. But that's fine, all you have to do is called SetCurrentValue as part of BindableDataGrid's initialization.

Add this to BindableDataGrid:

protected override void OnInitialized(EventArgs e)
{
    base.OnInitialized(e);
    SetCurrentValue(SelectedItemsProperty, base.SelectedItems);
}

Be careful, though, because this will still break if you try to set SelectedItems sometime after initialization. It would be nice if you could make it readonly, but that prevents it from being used in data binding. So make sure that your binding uses OneWayToSource not TwoWay:

<test:BindableDataGrid Grid.Column="4"
    SelectionMode="Extended"
    ColumnWidth="*"
    ItemsSource="{Binding Widgets}"
    SelectedItems="{Binding SelectedWidgets, Mode=OneWayToSource}"
    IsReadOnly="true">
    <DataGrid.RowStyle>
        <Style TargetType="{x:Type DataGridRow}">
            <Style.Resources>
                <SolidColorBrush x:Key="{x:Static SystemColors.InactiveSelectionHighlightBrushKey}" Color="CornflowerBlue"/>
            </Style.Resources>
        </Style>
    </DataGrid.RowStyle>
</test:BindableDataGrid>

If you want to insure this never breaks, you can add a CoerceValueCallback to make sure the new SelectedItems is never set to something other than base.SelectedItems:

public static readonly DependencyProperty SelectedItemsProperty = DependencyProperty.Register(
    "SelectedItems",
    typeof(IList),
    typeof(BindableDataGrid),
    new PropertyMetadata(default(IList), null, (o, v) => ((BindableDataGrid)o).CoerceBindableSelectedItems(v)));

protected object CoerceBindableSelectedItems(object baseValue)
{
    return base.SelectedItems;
}