fix(itemscontrol): Fixed problem with internal assignment of indexes …

…to elements in ItemsControl fixes #8203
unoplatform · Mar 3, 2022 · 4b63312 · 4b63312
1 parent fc15fff
commit 4b63312
Show file tree

Hide file tree

Showing 2 changed files with 130 additions and 5 deletions.
diff --git a/src/Uno.UI.Tests/Windows_UI_XAML_Controls/ItemsControlTests/Given_ItemsControl.cs b/src/Uno.UI.Tests/Windows_UI_XAML_Controls/ItemsControlTests/Given_ItemsControl.cs
@@ -11,6 +11,8 @@
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Controls;
 using Windows.UI.Xaml.Data;
+using FluentAssertions;
+using FluentAssertions.Execution;
 
 namespace Uno.UI.Tests.ItemsControlTests
 {
@@ -135,6 +137,112 @@ public void When_OnItemsSourceChanged()
 			Assert.AreEqual(1, count);
 		}
 
+		[TestMethod]
+		public void When_OnItemsSourceChanged_AfterRemove_ThenIndexesAreRecalculated()
+		{
+			void Operation(ObservableCollection<string> list)
+			{
+				list.RemoveAt(1);
+			}
+
+			CheckItemsSourceChanged(Operation);
+		}
+
+		[TestMethod]
+		public void When_OnItemsSourceChanged_AfterInsert_ThenIndexesAreRecalculated()
+		{
+			void Operation(ObservableCollection<string> list)
+			{
+				list.Insert(2, "new item");
+			}
+
+			CheckItemsSourceChanged(Operation);
+		}
+
+		[TestMethod]
+		public void When_OnItemsSourceChanged_AfterAdd_ThenIndexesAreRecalculated()
+		{
+			void Operation(ObservableCollection<string> list)
+			{
+				list.Add("new item at the end");
+			}
+
+			CheckItemsSourceChanged(Operation);
+		}
+
+		[TestMethod]
+		public void When_OnItemsSourceChanged_AfterClear_ThenIndexesAreRecalculated()
+		{
+			void Operation(ObservableCollection<string> list)
+			{
+				list.Clear();
+			}
+
+			CheckItemsSourceChanged(Operation);
+		}
+
+		[TestMethod]
+		public void When_OnItemsSourceChanged_AfterMove_ThenIndexesAreRecalculated()
+		{
+			void Operation(ObservableCollection<string> list)
+			{
+				list.Move(1, 3);
+			}
+
+			CheckItemsSourceChanged(Operation);
+		}
+
+		[TestMethod]
+		public void When_OnItemsSourceChanged_AfterReplace_ThenIndexesAreRecalculated()
+		{
+			void Operation(ObservableCollection<string> list)
+			{
+				list[1] = "new index 1";
+			}
+
+			CheckItemsSourceChanged(Operation);
+		}
+
+		private static void CheckItemsSourceChanged(Action<ObservableCollection<string>> operation)
+		{
+			var source = new ObservableCollection<string>();
+
+			source.Add("item 0");
+			source.Add("item 1");
+			source.Add("item 2");
+			source.Add("item 3");
+			source.Add("item 4");
+
+			var panel = new StackPanel();
+			var sut = new ItemsControl
+			{
+				ItemsPanelRoot = panel,
+				InternalItemsPanelRoot = panel,
+				ItemTemplate = new DataTemplate(() => { return new Border(); }),
+				ItemsSource = source
+			};
+
+			using var _ = new AssertionScope();
+
+			CheckIndexes();
+
+			operation(source);
+
+			CheckIndexes();
+
+			void CheckIndexes()
+			{
+				panel.Children.Should().HaveCount(source.Count);
+
+				for (var i = 0; i < panel.Children.Count; i++)
+				{
+					var child = panel.Children[i];
+					var index = child.GetValue(ItemsControl.IndexForItemContainerProperty);
+					index.Should().Be(i);
+				}
+			}
+		}
+
 		[TestMethod]
 		public void When_CollectionViewSource()
 		{

diff --git a/src/Uno.UI/UI/Xaml/Controls/ItemsControl/ItemsControl.cs b/src/Uno.UI/UI/Xaml/Controls/ItemsControl/ItemsControl.cs
@@ -995,6 +995,18 @@ void LocalCleanupContainer(object container)
 				if (container is DependencyObject doContainer)
 				{
 					CleanUpContainer(doContainer);
+					doContainer.ClearValue(IndexForItemContainerProperty);
+				}
+			}
+
+			void ReassignIndexes(int startingIndex)
+			{
+				var children = ItemsPanelRoot.Children;
+				var count = children.Count;
+				for (var i = startingIndex; i<count; i++)
+				{
+					var container = children[i];
+					container.SetValue(IndexForItemContainerProperty, i);
 				}
 			}
 
@@ -1014,28 +1026,33 @@ void LocalCleanupContainer(object container)
 				else if (args.Action == NotifyCollectionChangedAction.Remove
 					&& args.OldItems.Count == 1)
 				{
-					var container = ItemsPanelRoot.Children[args.OldStartingIndex];
+					var index = args.OldStartingIndex;
+					var container = ItemsPanelRoot.Children[index];
 
-					ItemsPanelRoot.Children.RemoveAt(args.OldStartingIndex);
+					ItemsPanelRoot.Children.RemoveAt(index);
 
 					LocalCleanupContainer(container);
+					ReassignIndexes(index);
 					RequestLayoutPartial();
 					return;
 				}
 				else if (args.Action == NotifyCollectionChangedAction.Add
 					&& args.NewItems.Count == 1)
 				{
-					ItemsPanelRoot.Children.Insert(args.NewStartingIndex, (UIElement)LocalCreateContainer(args.NewStartingIndex));
+					var index = args.NewStartingIndex;
+					ItemsPanelRoot.Children.Insert(index, (UIElement)LocalCreateContainer(index));
+					ReassignIndexes(index+1);
 					RequestLayoutPartial();
 					return;
 				}
 				else if (args.Action == NotifyCollectionChangedAction.Replace
 					&& args.NewItems.Count == 1)
 				{
-					var container = ItemsPanelRoot.Children[args.NewStartingIndex];
+					var index = args.NewStartingIndex;
+					var container = ItemsPanelRoot.Children[index];
 					LocalCleanupContainer(container);
 
-					ItemsPanelRoot.Children[args.NewStartingIndex] = (UIElement)LocalCreateContainer(args.NewStartingIndex);
+					ItemsPanelRoot.Children[index] = (UIElement)LocalCreateContainer(index);
 					RequestLayoutPartial();
 					return;
 				}