UWP ListView with observable collection always seems to refresh the entire list

[nbevans]

Description

The UWP ListView seems to not respect the observable collection rules. So things like delete/add etc will actually cause the entire list to refresh from scratch - rather than just updating the items to which the operation concerns.

The side affect of this is that any changes to the observable collection (e.g. ObservableCollection) result in the list scrolling right to the top.

This definitely "used to work" - because I recall seeing the UWP animations that occur when its behaving properly - I think on some version before 3.1. I am now on 3.2 pre3 and the issue is there.

Steps to Reproduce

ListViewReplaceUWP.zip

Expected Behavior

UWP ListView to respect the rules of a observable collection.

Actual Behavior

Refreshes the entire listing no matter what type of observable collection operation was performed.

Basic Information

• Version with issue:
• Last known good version: possibly a build of XamForms before 3.1 ?
• IDE:
• Platform Target Frameworks:
  • iOS:
  • Android:
  • UWP: 1709
• Android Support Library Version:
• Nuget Packages:
• Affected Devices:

[nbevans]

I can actually see the issue now in the source code of ListViewRenderer.

https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.UAP/ListViewRenderer.cs

The NotifyCollectionChangedAction.Replace has been wired up to ReloadData, and actually performs the same function as NotifyCollectionChangedAction.Reset. This is incorrect. NotifyCollectionChangedAction.Replace is a bonafide observable operation and should be properly implemented. Only NotifyCollectionChangedAction.Reset should cause a full reload of the data. Android's implementation does this properly for years with our app.

[nbevans]

                        items.RemoveAt(index)
                        items.Insert(index, replacement)
                        //items.[index] <- replacement

I changed our code to this (ya that's F# sorry) instead and it seems to improve the issue - at least the entire listing is not refreshing now and it's not jumping to the top anymore - but it does mean that UWP renders two transition effects, one for the removal and one for the insertion. Which is not ideal. A proper implementation of Replace operation would be better.

Edit: Unfortunately this isn't suitable as a short term solution because after a few invocations it throws an AVE: {System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.}

[nbevans]

Thank you that was fast. In case anyone is desperate like me, I've back ported this to v3.2-pre3 as a hacky renderer:-

I only tested this on 3.2pre3. It shouldn't be used long term. When 3.3 is out this issue will go away.

    public sealed class FixupObservableReplaceListViewRenderer : ListViewRenderer {
        private void InvokePrivateMethod(string methodName) {
            var m1 = typeof(ListViewRenderer).GetMethod(methodName, BindingFlags.NonPublic | BindingFlags.Instance);
            m1.Invoke(this, new object[] { });
        }

        private static FieldInfo GetEventField(Type type, string eventName) {
            FieldInfo field = null;
            while (type != null) {
                /* Find events defined as field */
                field = type.GetField(eventName, BindingFlags.Static | BindingFlags.Instance | BindingFlags.NonPublic);
                if (field != null && (field.FieldType == typeof(MulticastDelegate) || field.FieldType.IsSubclassOf(typeof(MulticastDelegate))))
                    break;

                /* Find events defined as property { add; remove; } */
                field = type.GetField("EVENT_" + eventName.ToUpper(), BindingFlags.Static | BindingFlags.Instance | BindingFlags.NonPublic);
                if (field != null)
                    break;
                type = type.BaseType;
            }
            return field;
        }

        protected override void OnElementChanged(ElementChangedEventArgs<ListView> e) {
            base.OnElementChanged(e);

            if (e.OldElement != null) {
                ((ITemplatedItemsView<Cell>)e.OldElement).TemplatedItems.CollectionChanged -= TemplatedItems_CollectionChanged;
            }

            if (e.NewElement != null) {
                var ti = ((ITemplatedItemsView<Cell>)e.NewElement).TemplatedItems;
                var fi = GetEventField(ti.GetType(), "CollectionChanged");
                if (fi == null) return;
                fi.SetValue(ti, null);

                ((ITemplatedItemsView<Cell>)e.NewElement).TemplatedItems.CollectionChanged += TemplatedItems_CollectionChanged;
            }
        }

        private void TemplatedItems_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) {
            var p1 = typeof(ListViewRenderer).GetProperty("SourceItems", BindingFlags.NonPublic | BindingFlags.Instance);
            var sourceItems = (ObservableCollection<object>)p1.GetValue(this);

            switch (e.Action) {
                case NotifyCollectionChangedAction.Add:
                    if (e.NewStartingIndex < 0)
                        goto case NotifyCollectionChangedAction.Reset;

                    // if a NewStartingIndex that's too high is passed in just add the items.
                    // I realize this is enforcing bad behavior but prior to this synchronization
                    // code being added it wouldn't cause the app to crash whereas now it does
                    // so this code accounts for that in order to ensure smooth sailing for the user
                    if (e.NewStartingIndex >= sourceItems.Count) {
                        for (int i = 0; i < e.NewItems.Count; i++)
                            sourceItems.Add((e.NewItems[i] as BindableObject).BindingContext);
                    } else {
                        for (int i = e.NewItems.Count - 1; i >= 0; i--)
                            sourceItems.Insert(e.NewStartingIndex, (e.NewItems[i] as BindableObject).BindingContext);
                    }

                    break;
                case NotifyCollectionChangedAction.Remove:
                    for (int i = e.OldItems.Count - 1; i >= 0; i--)
                        sourceItems.RemoveAt(e.OldStartingIndex);
                    break;
                case NotifyCollectionChangedAction.Move: {
                        for (var i = 0; i < e.OldItems.Count; i++) {
                            var oldi = e.OldStartingIndex;
                            var newi = e.NewStartingIndex;

                            if (e.NewStartingIndex < e.OldStartingIndex) {
                                oldi += i;
                                newi += i;
                            }

                            sourceItems.Move(oldi, newi);
                        }
                    }
                    break;
                case NotifyCollectionChangedAction.Replace: {
                        var newi = e.NewStartingIndex;
                        for (var i = 0; i < e.NewItems.Count; i++) {
                            newi += i;
                            sourceItems[newi] = (e.NewItems[i] as BindableObject).BindingContext;
                        }
                    }
                    break;
                case NotifyCollectionChangedAction.Reset:
                default:
                    InvokePrivateMethod("ClearSizeEstimate");
                    InvokePrivateMethod("ReloadData");
                    break;
            }

            Device.BeginInvokeOnMainThread(() => List?.UpdateLayout());
        }
    }