Bug 58451 - ListView SelectedItem Binding Issue
Summary: ListView SelectedItem Binding Issue
Status: IN_PROGRESS
Alias: None
Product: Forms
Classification: Xamarin
Component: Forms ()
Version: unspecified
Hardware: PC Windows
: Normal normal
Target Milestone: ---
Assignee: Bugzilla
URL: https://stackoverflow.com/questions/4...
Depends on:
Blocks:
 
Reported: 2017-07-28 03:56 UTC by Melbourne Developer
Modified: 2017-12-14 19:04 UTC (History)
6 users (show)

Tags: ac fr
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 58451 on Developer Community or GitHub if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
IN_PROGRESS

Description Melbourne Developer 2017-07-28 03:56:51 UTC
I can consistently show that the SelectedItem behaviour on ListView (UWP + Android) is inconsistent and that it is not using the Equals method in most cases to check for equality of items. There is an initial discussion here, but reproduction steps are below.

https://stackoverflow.com/questions/45364080/xamarin-forms-listview-selecteditem-binding-issue

Please follow steps on UWP:

#Issue 1
-Please clone this repo: https://ChristianFindlay@bitbucket.org/ChristianFindlay/xamarin-forms-scratch.git
-Run the sample on UWP, or Android
-Click the Async ListView button and wait for items to appear
-Notice that the SelectedItem is not displayed.

#Issue 2
-Click 'New Model'
-Notice that the SelectedItem is not displayed.
-Select First
-Click 'New Model'
-Notice that SelectedItem is displayed. I.e. the behaviour from null -> 2 is different from 1 -> 2

#Issue 3
-Close and reopen sample
-Wait for items to appear
-Click 'Set To 2'
-Notice that the SelectedItem is not displayed.
-Click 'Set To New 2'
-Notice that the SelectedItem is not displayed.
-Select First
-Click 'Set To 2'
-Notice that the SelectedItem is displayed.
-Select First
-Click 'Set To New 2'
-Notice that the SelectedItem is not displayed.

Throughout these repro steps, notice that the Equals method is being called regularly because you can see the debug messages in the output window. (E.g. An ItemModel was tested for equality. Equal: True), but it seems that the control only pays attention to the Equals method in corner cases.

The SelectedItem property sometimes works, but it is inconsistent and is comparing by reference in many cases instead of using the Equals method. This is completely out of line with standard Microsoft binding protocol and makes the ListView incompatible with other platforms like WPF, UWP, and Silverlight.
Comment 1 Laura 2017-07-28 07:21:19 UTC
Can reproduce on Android with a HTC U11.
Comment 2 Yuri 2017-07-31 14:58:39 UTC
One of the problems that ListView allows to set SelectedItem to an item which is not in the SourceItems
Comment 3 Paul DiPietro [MSFT] 2017-08-01 02:47:49 UTC
For the time being I will set this to needinfo, but I think it's because the core issue might have to do with the first bulletpoint in particular, although it should be alleviated by explicitly setting the SelectedItem in the Items_ItemsLoaded method in the provided repro.

I've noted Yuri's point about the ListView setting a SelectedItem to something not in the ItemsSource. There might be room for improvement there, but I cannot say for certain at this very moment. With regards to the CreateNewModel method, doing something like this seems to allow for more expected behavior, if I'm understanding correctly:

(In this case, ItemModel two is renamed to selectedItemModel and a counter value is added above as a private value):

        private void CreateNewModel()
        {
            var alvm = new AsyncListViewModel { ItemModel = GetNewTwo(count) };
            if (items != null)
            {
                items.Add(alvm.ItemModel);
                selectedItemModel = alvm.ItemModel;
                count++;
            }
            BindingContext = alvm;
        }

Updating the BindingContext on subsequent clicks of "New Model" updates the selection to that new item model because it is actually in the items list which the ItemsSource binds to. When a 1/2/3 item is added, the first one with that value in the list is selected (and does not change if it is already selected).

What would be helpful here is an elaboration of intent and expectation taking this into account. I apologize if I haven't been entirely helpful just yet, but I'd like to fully grasp what aspect of the ListView behavior is incorrect and more concretely define the report rather than break it into several parts (which sometimes makes resolving these more difficult).
Comment 5 Melbourne Developer 2017-08-01 05:01:42 UTC
I have now included in the solution a WPF sample with code shared with Xamarin Forms. The ListView in WPF behaves in almost exactly the same way.

I had thought that we should be able to set SelectedItem to a new object that does not exist in the list by reference, but this seems to be the same behaviour on WPF - i.e. the SelectedItem needs to be found in the list by object reference (as opposed to using the Equals method). 

*************************************************
The main difference is that if you run the WPF sample, you can see that the second item is selected in the ListView whereas, in Xamarin Forms it not selected despite the fact that the SelectedItem is set to a value. ***This is the crux of the issue***
*************************************************
Comment 6 Yuri 2017-08-01 15:15:14 UTC
Technically speaking there are not the same objects, so using Equals may be not right thing to do. I always considered selected item as item in the list because that is what you see and what you select
Comment 7 Yuri 2017-08-01 15:34:14 UTC
Actually Equals will not work either. Here is the sample

 public class TestObject
    {
        public string Name { get; set; }
    }

            var t1 = new TestObject { Name = "1" };
            var t2 = new TestObject { Name = "2" };
            var comp = (t1 == t2);
            var eq = t1.Equals(t2);


Both "comp" and "eq" are false in regular C# console app
Comment 8 Yuri 2017-08-01 15:36:02 UTC
Sorry, set the same name to "1", the same result

            var t1 = new TestObject { Name = "1" };
            var t2 = new TestObject { Name = "1" };
            var comp = (t1 == t2);
            var eq = t1.Equals(t2);
Comment 9 Melbourne Developer 2017-08-01 23:14:53 UTC
The code above will not work because the Equals method has not been overridden.  

The equality check is one of the questionable issues with the control but it is not the main issue. 

I don't want to take away the focus from the main bug which is:
*************************************************
The main difference is that if you run the WPF sample, you can see that the second item is selected in the ListView whereas, in Xamarin Forms it not selected despite the fact that the SelectedItem is set to a value. ***This is the crux of the issue***
*************************************************

But, here is something to think about. Each of the platforms uses different code to check for equality of items. I think the ListView on UWP uses this code to check for equality and this explains some of the inconsistencies:

		public int GetGlobalIndexOfItem(object group, object item)
		{
			if (!IsGroupingEnabled)
				return ListProxy.IndexOf(item);

			var count = 0;
			if (_groupedItems != null)
			{
				foreach (KeyValuePair<object, TemplatedItemsList<TView, TItem>> kvp in _groupedItems)
				{
					count++;

					if (ReferenceEquals(group, kvp.Key))
					{
						int index = kvp.Value.GetGlobalIndexOfItem(item);
						if (index != -1)
							return count + index;
					}

					count += kvp.Value.GetDescendantCount();
				}
			}

			return -1;
		}

You can see that if grouping is enabled, it returns the index with IndexOf which will end up checking with the overridden Equals method (https://msdn.microsoft.com/en-us/library/e4w08k17(v=vs.110).aspx). If not, it will fall back on checking by reference ReferenceEquals(group, kvp.Key) . So, my guess is that there are a few inconsistencies around this. But, again, this is not the real issue - it's not even inconsistent with WPF. So, I don't want to draw attention away from the actual problem which is getting the selected item to show up in the control.
Comment 10 Melbourne Developer 2017-08-01 23:15:39 UTC
What info is needed in order to look in to this problem?
Comment 11 Melbourne Developer 2017-08-07 04:21:44 UTC
Hello...
Comment 12 Melbourne Developer 2017-09-20 23:28:38 UTC
Just for your information, this is a ListView that was built by our team that supports async behaviour, doesn't require a custom renderer, and allows for multi select:

https://github.com/MelbourneDeveloper/Adapt.Presentation/blob/master/Adapt.Presentation.Standard/Adapt/Presentation/Controls/AdaptListView.cs

using System;
using System.Collections;
using System.Collections.Specialized;
using System.Linq;
using Xamarin.Forms;

namespace Adapt.Presentation.Controls
{
    public class AdaptListView : ScrollView
    {
        #region Enums
        public enum ItemSelectorSelectionMode
        {
            Single,
            Multi
        }
        #endregion

        #region Events
        public event EventHandler SelectionChanged;
        #endregion

        #region Private Fields
        private readonly StackLayout _StackList = new StackLayout { Spacing = 0 };
        #endregion Private Fields

        #region Constructor
        public AdaptListView()
        {
            Content = _StackList;
        }
        #endregion Constructor

        #region Dependency Properties

        #region SelectedBackgroundColorProperty
        public static readonly BindableProperty SelectedBackgroundColorProperty = BindableProperty.Create(nameof(SelectedBackgroundColor), typeof(Color), typeof(AdaptListView), Color.Gray, BindingMode.OneWayToSource);
        #endregion

        #region SelectedItemProperty
        public static readonly BindableProperty SelectedItemProperty = BindableProperty.Create(nameof(SelectedItem), typeof(object), typeof(AdaptListView), null, BindingMode.OneWayToSource, propertyChanged: OnSelectedItemChanged);

        private static void OnSelectedItemChanged(BindableObject bindable, object oldValue, object newValue)
        {
            var control = (AdaptListView)bindable;
            control.RefreshSelection();
        }
        #endregion

        #region SelectedItemsProperty		
        public static readonly BindableProperty SelectedItemsProperty = BindableProperty.Create(nameof(SelectedItems), typeof(IList), typeof(AdaptListView), null, BindingMode.OneWayToSource, propertyChanged: OnSelectedItemsChanged);

        private static void OnSelectedItemsChanged(BindableObject bindable, object oldValue, object newValue)
        {
            var control = (AdaptListView)bindable;
            control.RefreshSelection();

            if (newValue is INotifyCollectionChanged notifyCollectionChanged)
            {
                notifyCollectionChanged.CollectionChanged += control.NotifyCollectionChanged_CollectionChanged;
            }
        }
        #endregion

        #region ItemsSourceProperty
        public static readonly BindableProperty ItemsSourceProperty = BindableProperty.Create(nameof(ItemsSource), typeof(IEnumerable), typeof(AdaptListView), null, propertyChanged: OnItemsSourceChanged);

        private static void OnItemsSourceChanged(BindableObject bindable, object oldValue, object newValue)
        {
            //TODO: We need to detach the old collection changed handler here...

            var control = (AdaptListView)bindable;
            if (newValue is INotifyCollectionChanged itemsSource)
            {
                itemsSource.CollectionChanged += (s, e) => control.RefreshItems();
            }
            control.RefreshItems();
        }
        #endregion

        #endregion Dependency Properties

        #region Public Properties

        public ItemSelectorSelectionMode SelectionMode { get; set; }

        public IEnumerable ItemsSource
        {
            get => (IEnumerable)GetValue(ItemsSourceProperty);
            set => SetValue(ItemsSourceProperty, value);
        }

        public object SelectedItem
        {
            get => GetValue(SelectedItemProperty);
            set => SetValue(SelectedItemProperty, value);
        }

        public IList SelectedItems
        {
            get => (IList)GetValue(SelectedItemsProperty);
            set
            {
                //TODO: We need to detach the previous collection's event here

                SetValue(SelectedItemsProperty, value);
                SelectionChanged?.Invoke(this, new EventArgs());
            }
        }

        public Color SelectedBackgroundColor
        {
            get => (Color)GetValue(SelectedBackgroundColorProperty);
            set => SetValue(SelectedBackgroundColorProperty, value);
        }

        public DataTemplate ItemTemplate { get; set; }

        #endregion Public Properties

        #region Event Handlers
        private void NotifyCollectionChanged_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
        {
            RefreshSelection();
            SelectionChanged?.Invoke(this, new EventArgs());
        }
        #endregion

        #region Private Methods
        private void RefreshSelection()
        {
            //TODO: Support for duplicate records
            var dataItems = _StackList.Children.ToDictionary(x => x, y => y.BindingContext);

            foreach (var item in dataItems)
            {
                var view = item.Key;

                switch (SelectionMode)
                {
                    case ItemSelectorSelectionMode.Single:

                        if (GetIsEqual(item.Value, SelectedItem))
                        {
                            SetToBackgroundColor(view);
                        }
                        else
                        {
                            SetToTransparent(view);
                        }

                        break;

                    case ItemSelectorSelectionMode.Multi:

                        SetToTransparent(view);

                        if (SelectedItems != null)
                        {
                            foreach (var selectedItem in SelectedItems)
                            {
                                if (GetIsEqual(item.Value, selectedItem))
                                {
                                    SetToBackgroundColor(view);
                                }
                            }
                        }

                        break;
                }
            }
        }

        private static bool GetIsEqual(object item, object selectedItem)
        {
            return item != null && item.Equals(selectedItem);
        }

        private void SetToBackgroundColor(View view)
        {
            view.BackgroundColor = SelectedBackgroundColor;
        }

        private static void SetToTransparent(View view)
        {
            view.BackgroundColor = Color.Transparent;
        }

        private void RefreshItems()
        {
            if (ItemsSource == null)
            {
                return;
            }

            _StackList.Children.Clear();
            foreach (var item in ItemsSource)
            {
                var view = (View)ItemTemplate.CreateContent();
                view.BindingContext = item;
#pragma warning disable CS0618 // Type or member is obsolete
                view.GestureRecognizers.Add(new TapGestureRecognizer(TemplateTapped));
#pragma warning restore CS0618 // Type or member is obsolete
                _StackList.Children.Add(view);
            }

            RefreshSelection();
        }

        private void TemplateTapped(View obj)
        {
            if (obj == null)
            {
                return;
            }

            var bindingContext = obj.BindingContext;

            switch (SelectionMode)
            {
                case ItemSelectorSelectionMode.Single:
                    SelectedItem = bindingContext;
                    break;
                case ItemSelectorSelectionMode.Multi:
                    if (SelectedItems != null)
                    {
                        if (!SelectedItems.Contains(bindingContext))
                        {
                            SelectedItems.Add(bindingContext);
                        }
                        else
                        {
                            SelectedItems.Remove(bindingContext);
                        }
                    }
                    break;
            }
        }

        #endregion Private Methods
    }
}
Comment 13 Stephane Delcroix 2017-09-21 10:16:59 UTC
https://github.com/xamarin/Xamarin.Forms/pull/1152