ComboBox ItemsSource changed => SelectedItem is ruined

Question

Ok, this has been bugging me for a while now. And I wonder how others handle the following case:

<ComboBox ItemsSource="{Binding MyItems}" SelectedItem="{Binding SelectedItem}"/>

The DataContext object's code:

public ObservableCollection<MyItem> MyItems { get; set; }
public MyItem SelectedItem { get; set; }

public void RefreshMyItems()
{
    MyItems.Clear();
    foreach(var myItem in LoadItems()) MyItems.Add(myItem);
}

public class MyItem
{
    public int Id { get; set; }
    public override bool Equals(object obj)
    {
        return this.Id == ((MyItem)obj).Id;
    }
}

Obviously when the RefreshMyItems() method is called the combo box receives the Collection Changed events, updates its items and does not find the SelectedItem in the refreshed collection => sets the SelectedItem to null. But I would need the combo box to use Equals method to select the correct item in the new collection.

In other words - the ItemsSource collection still contains the correct MyItem, but it is a new object. And I want the combo box to use something like Equals to select it automatically (this is made even harder because first the source collection calls Clear() which resets the collection and already at that point the SelectedItem is set to null).

UPDATE 2 Before copy-pasting the code below please note that it is far from perfection! And note that it does not bind two ways by default.

UPDATE Just in case someone has the same problem (an attached property as proposed by Pavlo Glazkov in his answer):

public static class CBSelectedItem
{
    public static object GetSelectedItem(DependencyObject obj)
    {
        return (object)obj.GetValue(SelectedItemProperty);
    }

    public static void SetSelectedItem(DependencyObject obj, object value)
    {
        obj.SetValue(SelectedItemProperty, value);
    }

    // Using a DependencyProperty as the backing store for SelectedIte.  This enables animation, styling, binding, etc...
    public static readonly DependencyProperty SelectedItemProperty =
        DependencyProperty.RegisterAttached("SelectedItem", typeof(object), typeof(CBSelectedItem), new UIPropertyMetadata(null, SelectedItemChanged));


    private static List<WeakReference> ComboBoxes = new List<WeakReference>();
    private static void SelectedItemChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
    {
        ComboBox cb = (ComboBox) d;

        // Set the selected item of the ComboBox since the value changed
        if (cb.SelectedItem != e.NewValue) cb.SelectedItem = e.NewValue;

        // If we already handled this ComboBox - return
        if(ComboBoxes.SingleOrDefault(o => o.Target == cb) != null) return;

        // Check if the ItemsSource supports notifications
        if(cb.ItemsSource is INotifyCollectionChanged)
        {
            // Add ComboBox to the list of handled combo boxes so we do not handle it again in the future
            ComboBoxes.Add(new WeakReference(cb));

            // When the ItemsSource collection changes we set the SelectedItem to correct value (using Equals)
            ((INotifyCollectionChanged) cb.ItemsSource).CollectionChanged +=
                delegate(object sender, NotifyCollectionChangedEventArgs e2)
                    {
                        var collection = (IEnumerable<object>) sender;
                        cb.SelectedItem = collection.SingleOrDefault(o => o.Equals(GetSelectedItem(cb)));
                    };

            // If the user has selected some new value in the combo box - update the attached property too
            cb.SelectionChanged += delegate(object sender, SelectionChangedEventArgs e3)
                                       {
                                           // We only want to handle cases that actually change the selection
                                           if(e3.AddedItems.Count == 1)
                                           {
                                               SetSelectedItem((DependencyObject)sender, e3.AddedItems[0]);
                                           }
                                       };
        }

    }
}

Solution 1

The standard ComboBox doesn't have that logic. And as you mentioned SelectedItem becomes null already after you call Clear, so the ComboBox has no idea about you intention to add the same item later and therefore it does nothing to select it. That being said, you will have to memorize the previously selected item manually and after you've updated you collection restore the selection also manually. Usually it is done something like this:

public void RefreshMyItems()
{
    var previouslySelectedItem = SelectedItem;

    MyItems.Clear();
    foreach(var myItem in LoadItems()) MyItems.Add(myItem);

    SelectedItem = MyItems.SingleOrDefault(i => i.Id == previouslySelectedItem.Id);

}

If you want to apply the same behavior to all ComboBoxes (or perhaps all Selector controls), you can consider creating a Behavior(an attached property or blend behavior). This behavior will subscribe to the SelectionChanged and CollectionChanged events and will save/restore the selected item when appropriate.

Solution 2

This is the top google result for "wpf itemssource equals" right now, so to anyone trying the same approach as in the question, it does work as long as you fully implement equality functions. Here is a complete MyItem implementation:

public class MyItem : IEquatable<MyItem>
{
    public int Id { get; set; }

    public bool Equals(MyItem other)
    {
        if (Object.ReferenceEquals(other, null)) return false;
        if (Object.ReferenceEquals(other, this)) return true;
        return this.Id == other.Id;
    }

    public sealed override bool Equals(object obj)
    {
        var otherMyItem = obj as MyItem;
        if (Object.ReferenceEquals(otherMyItem, null)) return false;
        return otherMyItem.Equals(this);
    }

    public override int GetHashCode()
    {
        return this.Id.GetHashCode();
    }

    public static bool operator ==(MyItem myItem1, MyItem myItem2)
    {
        return Object.Equals(myItem1, myItem2);
    }

    public static bool operator !=(MyItem myItem1, MyItem myItem2)
    {
        return !(myItem1 == myItem2);
    }
}

I successfully tested this with a multiple selection ListBox, where listbox.SelectedItems.Add(item) was failing to select the matching item, but worked after I implemented the above on item.

Solution 3

Unfortunately when setting ItemsSource on a Selector object it immediately sets SelectedValue or SelectedItem to null even if corresponding item is in new ItemsSource.

No matter if you implement Equals.. functions or you use a implicitly comparable type for your SelectedValue.

Well, you can save SelectedItem/Value prior to setting ItemsSource and than restore. But what if there's a binding on SelectedItem/Value which will be called twice: set to null restore original.

That's additional overhead and even it can cause some undesired behavior.

Here's a solution which I made. Will work for any Selector object. Just clear SelectedValue binding prior to setting ItemsSource.

UPD: Added try/finally to protect from exceptions in handlers, also added null check for binding.

public static class ComboBoxItemsSourceDecorator
{
    public static readonly DependencyProperty ItemsSourceProperty = DependencyProperty.RegisterAttached(
        "ItemsSource", typeof(IEnumerable), typeof(ComboBoxItemsSourceDecorator), new PropertyMetadata(null, ItemsSourcePropertyChanged)
    );

    public static void SetItemsSource(UIElement element, IEnumerable value)
    {
        element.SetValue(ItemsSourceProperty, value);
    }

    public static IEnumerable GetItemsSource(UIElement element)
    {
        return (IEnumerable)element.GetValue(ItemsSourceProperty);
    }

    static void ItemsSourcePropertyChanged(DependencyObject element, 
                    DependencyPropertyChangedEventArgs e)
    {
        var target = element as Selector;
        if (element == null)
            return;

        // Save original binding 
        var originalBinding = BindingOperations.GetBinding(target, Selector.SelectedValueProperty);

        BindingOperations.ClearBinding(target, Selector.SelectedValueProperty);
        try
        {
            target.ItemsSource = e.NewValue as IEnumerable;
        }
        finally
        {
            if (originalBinding != null)
                BindingOperations.SetBinding(target, Selector.SelectedValueProperty, originalBinding);
        }
    }
}

Here's a XAML example:

                <telerik:RadComboBox Grid.Column="1" x:Name="cmbDevCamera" DataContext="{Binding Settings}" SelectedValue="{Binding SelectedCaptureDevice}" 
                                     SelectedValuePath="guid" e:ComboBoxItemsSourceDecorator.ItemsSource="{Binding CaptureDeviceList}" >
                </telerik:RadComboBox>

Unit Test

Here is a unit test case proving that it works. Just comment out the #define USE_DECORATOR to see the test fail when using the standard bindings.

#define USE_DECORATOR

using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Security.Permissions;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Controls.Primitives;
using System.Windows.Data;
using System.Windows.Threading;
using FluentAssertions;
using ReactiveUI;
using ReactiveUI.Ext;
using ReactiveUI.Fody.Helpers;
using Xunit;

namespace Weingartner.Controls.Spec
{
    public class ComboxBoxItemsSourceDecoratorSpec
    {
        [WpfFact]
        public async Task ControlSpec ()
        {
            var comboBox = new ComboBox();
            try
            {

                var numbers1 = new[] {new {Number = 10, i = 0}, new {Number = 20, i = 1}, new {Number = 30, i = 2}};
                var numbers2 = new[] {new {Number = 11, i = 3}, new {Number = 20, i = 4}, new {Number = 31, i = 5}};
                var numbers3 = new[] {new {Number = 12, i = 6}, new {Number = 20, i = 7}, new {Number = 32, i = 8}};

                comboBox.SelectedValuePath = "Number";
                comboBox.DisplayMemberPath = "Number";


                var binding = new Binding("Numbers");
                binding.Mode = BindingMode.OneWay;
                binding.UpdateSourceTrigger=UpdateSourceTrigger.PropertyChanged;
                binding.ValidatesOnDataErrors = true;

#if USE_DECORATOR
                BindingOperations.SetBinding(comboBox, ComboBoxItemsSourceDecorator.ItemsSourceProperty, binding );
#else
                BindingOperations.SetBinding(comboBox, ItemsControl.ItemsSourceProperty, binding );
#endif

                DoEvents();

                var selectedValueBinding = new Binding("SelectedValue");
                BindingOperations.SetBinding(comboBox, Selector.SelectedValueProperty, selectedValueBinding);

                var viewModel = ViewModel.Create(numbers1, 20);
                comboBox.DataContext = viewModel;

                // Check the values after the data context is initially set
                comboBox.SelectedIndex.Should().Be(1);
                comboBox.SelectedItem.Should().BeSameAs(numbers1[1]);
                viewModel.SelectedValue.Should().Be(20);

                // Change the list of of numbers and check the values
                viewModel.Numbers = numbers2;
                DoEvents();

                comboBox.SelectedIndex.Should().Be(1);
                comboBox.SelectedItem.Should().BeSameAs(numbers2[1]);
                viewModel.SelectedValue.Should().Be(20);

                // Set the list of numbers to null and verify that SelectedValue is preserved
                viewModel.Numbers = null;
                DoEvents();

                comboBox.SelectedIndex.Should().Be(-1);
                comboBox.SelectedValue.Should().Be(20); // Notice that we have preserved the SelectedValue
                viewModel.SelectedValue.Should().Be(20);


                // Set the list of numbers again after being set to null and see that
                // SelectedItem is now correctly mapped to what SelectedValue was.
                viewModel.Numbers = numbers3;
                DoEvents();

                comboBox.SelectedIndex.Should().Be(1);
                comboBox.SelectedItem.Should().BeSameAs(numbers3[1]);
                viewModel.SelectedValue.Should().Be(20);


            }
            finally
            {
                Dispatcher.CurrentDispatcher.InvokeShutdown();
            }
        }

        public class ViewModel<T> : ReactiveObject
        {
            [Reactive] public int SelectedValue { get; set;}
            [Reactive] public IList<T> Numbers { get; set; }

            public ViewModel(IList<T> numbers, int selectedValue)
            {
                Numbers = numbers;
                SelectedValue = selectedValue;
            }
        }

        public static class ViewModel
        {
            public static ViewModel<T> Create<T>(IList<T> numbers, int selectedValue)=>new ViewModel<T>(numbers, selectedValue);
        }

        /// <summary>
        /// From http://stackoverflow.com/a/23823256/158285
        /// </summary>
        public static class ComboBoxItemsSourceDecorator
        {
            private static ConcurrentDictionary<DependencyObject, Binding> _Cache = new ConcurrentDictionary<DependencyObject, Binding>();

            public static readonly DependencyProperty ItemsSourceProperty = DependencyProperty.RegisterAttached(
                "ItemsSource", typeof(IEnumerable), typeof(ComboBoxItemsSourceDecorator), new PropertyMetadata(null, ItemsSourcePropertyChanged)
            );

            public static void SetItemsSource(UIElement element, IEnumerable value)
            {
                element.SetValue(ItemsSourceProperty, value);
            }

            public static IEnumerable GetItemsSource(UIElement element)
            {
                return (IEnumerable)element.GetValue(ItemsSourceProperty);
            }

            static void ItemsSourcePropertyChanged(DependencyObject element,
                            DependencyPropertyChangedEventArgs e)
            {
                var target = element as Selector;
                if (target == null)
                    return;

                // Save original binding 
                var originalBinding = BindingOperations.GetBinding(target, Selector.SelectedValueProperty);
                BindingOperations.ClearBinding(target, Selector.SelectedValueProperty);
                try
                {
                    target.ItemsSource = e.NewValue as IEnumerable;
                }
                finally
                {
                    if (originalBinding != null )
                        BindingOperations.SetBinding(target, Selector.SelectedValueProperty, originalBinding);
                }
            }
        }

        [SecurityPermission(SecurityAction.Demand, Flags = SecurityPermissionFlag.UnmanagedCode)]
        public static void DoEvents()
        {
            DispatcherFrame frame = new DispatcherFrame();
            Dispatcher.CurrentDispatcher.BeginInvoke(DispatcherPriority.Background, new DispatcherOperationCallback(ExitFrame), frame);
            Dispatcher.PushFrame(frame);
        }

        private static object ExitFrame(object frame)
        {
            ((DispatcherFrame)frame).Continue = false;
            return null;
        }


    }
}

Comments

[eran otzap]

iv'e came across this issue and solved it in the following manner stackoverflow.com/questions/12337442/…

[ket]

For those having issues with this approach and who, like me, didn't realize it at first: this solution may work better when used together with nmclean's answer.

[Jefim]

It is a solution, of course. Though then comes a question - is there a way to apply this to a whole set of combo boxes. I also don't think that there is a good way to access the binding target from within a value converter (and I will need access to the ItemsSource to select the correct one).

[biju]

You can use a multivalue converter and pass in your collection as binding

[Pavlo Glazkov]

Value converter will not help with this because after Clear is called there is nothing to select to. You will need to store the previously selected item somewhere anyhow.

[Jefim]

Yep, exactly my thoughts :) I though I would write an attached property for this case. Thanks for the hint! (I updated the post with the attached property code)

[cplotts]

Thanks for your answer! I had forgotten to implement operator== and operator!= ... and this was causing me headaches!

[Andrew Barber]

It's extremely difficult to see how this is an answer to the question posed. Without some explanation here, it seems like you just posted some random code.

[Alexandru Dicu]

This solution works also for the default combo box. However, you must check if originalBinding is not null before executing the last line of code, because on the first load of the item source the original binding will be null.

[Ran Sagy]

This was very useful to a sudden similar issue we encountered since running under the 4.5.1 runtime. Thanks!

[Christian Findlay]

This is a common problem with ComboBoxes and other Selector controls in WPF, Silverlight, and UWP. Here is a proposed solution to fix the problem across all Selector controls and platforms without having to write code behind code every time. stackoverflow.com/questions/36003805/…

[tomosius]

@MelbourneDeveloper the link is down - returns page not found.

[bokibeg]

This should be upvoted more as it is the only valid acceptable answer as it eliminates the issue altogether and works in multi threaded app.

[gts13]

I can't get it work. Do I need to save the previous value and after the I have updated the collection to restore it?

[Istvan Heckl]

For me ItemsSourcePropertyChanged was executed only when the app started. Later, the ObservableCollection corresponding to the e:ComboBoxItemsSourceDecorator.ItemsSource was refreshed in the ViewModel. At this point ItemsSourcePropertyChanged was not executed, so after the refresh the SelectedItem is null.

[Kim Homann]

It works, thank you! But it seems not 100% correct what you say. My first try was to update the ItemsSource first and afterwards call GetBinding, ClearBinding and SetBinding. It might be a coincidence, but this approach works as well. So my suspicion is that the SelectedItem property is updated to the same instance in the new ItemsSource, and only the display is not updated. But your approach feels safer, so I did it like you propose.