Fix BindableLayout's incorrect layout.children.add -> use insert (#5582)
authorChris van de Steeg <chris@chrisvandesteeg.nl>
Thu, 21 Mar 2019 13:24:31 +0000 (14:24 +0100)
committerStephane Delcroix <stephane@delcroix.org>
Thu, 21 Mar 2019 13:24:31 +0000 (14:24 +0100)
* Fix incorrect layout.children.add -> use insert

* Modify unit test to match the problem with this issue

- fixes #5579

Xamarin.Forms.Core.UnitTests/BindableLayoutTests.cs
Xamarin.Forms.Core/BindableLayout.cs

index e2a6ddc..9a4154e 100644 (file)
@@ -49,10 +49,10 @@ namespace Xamarin.Forms.Core.UnitTests
                                IsPlatformEnabled = true,
                        };
 
-                       var itemsSource = new ObservableCollection<int>();
+                       var itemsSource = new ObservableCollection<int>() { 0, 1, 2, 3, 4 };
                        BindableLayout.SetItemsSource(layout, itemsSource);
 
-                       itemsSource.Insert(0, 1);
+                       itemsSource.Insert(2, 5);
                        Assert.IsTrue(IsLayoutWithItemsSource(itemsSource, layout));
                }
 
@@ -413,8 +413,8 @@ namespace Xamarin.Forms.Core.UnitTests
                                OnPropertyChanged(new PropertyChangedEventArgs("Item[]"));
                                OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, changedItems, 0));
                        }
-               }     
-      
+               }
+
                class MyDataTemplateSelectorTest : DataTemplateSelector
                {
                        readonly Func<object, BindableObject, DataTemplate> _func;
index 228071e..6bfe6cf 100644 (file)
@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Specialized;
+using Xamarin.Forms.Internals;
 
 namespace Xamarin.Forms
 {
@@ -175,16 +176,16 @@ namespace Xamarin.Forms
                        int i = 0;
                        foreach (object item in _itemsSource)
                        {
-                               layout.Children.Add(CreateItemView(item, i++, layout));
+                               layout.Children.Add(CreateItemView(item, layout));
                        }
                }
 
-               View CreateItemView(object item, int index, Layout<View> layout)
+               View CreateItemView(object item, Layout<View> layout)
                {
-                       return CreateItemView(item, index, _itemTemplate ?? _itemTemplateSelector?.SelectTemplate(item, layout));
+                       return CreateItemView(item, _itemTemplate ?? _itemTemplateSelector?.SelectTemplate(item, layout));
                }
 
-               View CreateItemView(object item, int index, DataTemplate dataTemplate)
+               View CreateItemView(object item, DataTemplate dataTemplate)
                {
                        if (dataTemplate != null)
                        {
@@ -206,63 +207,10 @@ namespace Xamarin.Forms
                                return;
                        }
 
-                       switch (e.Action)
-                       {
-                               case NotifyCollectionChangedAction.Add:
-                                       {
-                                               if (e.NewStartingIndex == -1)
-                                                       goto case NotifyCollectionChangedAction.Reset;
-                                               int i = e.NewStartingIndex;
-                                               foreach (object item in e.NewItems)
-                                               {
-                                                       layout.Children.Add(CreateItemView(item, i++, layout));
-                                               }
-                                       }
-                                       break;
-                               case NotifyCollectionChangedAction.Remove:
-                                       {
-                                               if (e.OldStartingIndex == -1)
-                                                       goto case NotifyCollectionChangedAction.Reset;
-                                               for (int i = 0; i < e.OldItems.Count; i++)
-                                               {
-                                                       layout.Children.RemoveAt(e.OldStartingIndex);
-                                               }
-                                       }
-                                       break;
-                               case NotifyCollectionChangedAction.Replace:
-                                       {
-                                               if (e.OldStartingIndex == -1)
-                                                       goto case NotifyCollectionChangedAction.Reset;
-                                               int i = e.NewStartingIndex;
-                                               foreach (object item in e.NewItems)
-                                               {
-                                                       layout.Children[i] = CreateItemView(item, i, layout);
-                                                       ++i;
-                                               }
-                                       }
-                                       break;
-                               case NotifyCollectionChangedAction.Move:
-                                       {
-                                               if (e.OldStartingIndex == -1 || e.NewStartingIndex == -1)
-                                                       goto case NotifyCollectionChangedAction.Reset;
-                                               for (int i = 0; i < e.NewItems.Count; ++i)
-                                               {
-                                                       int iFrom = e.OldStartingIndex + i;
-                                                       int iTo = e.NewStartingIndex + i;
-                                                       View fromView = layout.Children[iFrom];
-                                                       View toView = layout.Children[iTo];
-                                                       layout.Children.Remove(fromView);
-                                                       layout.Children.Remove(toView);
-                                                       layout.Children.Insert(iFrom, toView);
-                                                       layout.Children.Insert(iTo, fromView);
-                                               }
-                                       }
-                                       break;
-
-                               case NotifyCollectionChangedAction.Reset:
-                                       layout.Children.Clear();
-                                       break;
-                       }
+                       e.Apply(
+                               insert: (item, index, _) => layout.Children.Insert(index, CreateItemView(item, layout)),
+                               removeAt: (item, index) => layout.Children.RemoveAt(index),
+                               reset: CreateChildren);
                }
        }
-}
+}
\ No newline at end of file