Merge pull request #2699 from cwensley/curtis/mac-fix-gridview-updati…

…ng-items Mac: Fix issue updating items in a GridView
picoe · Nov 9, 2024 · 3dcde01 · 3dcde01
2 parents c98adbd + 1bc2004
commit 3dcde01
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 62 deletions.
diff --git a/src/Eto.Mac/Forms/Controls/GridViewHandler.cs b/src/Eto.Mac/Forms/Controls/GridViewHandler.cs
@@ -533,84 +533,47 @@ class CollectionHandler : EnumerableChangedHandler<object>
 		{
 			public GridViewHandler Handler { get; set; }
 
-			public override void AddRange(IEnumerable<object> items)
+			protected override void BeginUpdates()
 			{
-				Handler.Control.ReloadData();
+				base.BeginUpdates();
+				Handler.Control.BeginUpdates();
+			}
+
+			protected override void EndUpdates()
+			{
+				base.EndUpdates();
+				Handler.Control.EndUpdates();
 				Handler.AutoSizeColumns(true);
 			}
 
-			static Selector selInsertRowsWithAnimation = new Selector("insertRowsAtIndexes:withAnimation:");
-			static Selector selRemoveRowsWithAnimation = new Selector("removeRowsAtIndexes:withAnimation:");
+			public override void AddRange(IEnumerable<object> items)
+			{
+				Handler.Control.ReloadData();
+			}
 
 			public override void AddItem(object item)
 			{
-				if (Handler.Control.RespondsToSelector(selInsertRowsWithAnimation))
-				{
-					Handler.Control.BeginUpdates();
-					Handler.Control.InsertRows(new NSIndexSet(Count), NSTableViewAnimation.SlideDown);
-					Handler.Control.EndUpdates();
-				}
-				else
-					Handler.Control.ReloadData();
-
-				Handler.AutoSizeColumns(true);
+				Handler.Control.InsertRows(new NSIndexSet(Count), NSTableViewAnimation.SlideDown);
 			}
 
+			public override void ReplaceItem(int index, object newItem)
+			{
+				Handler.Control.ReloadData(NSIndexSet.FromIndex((nint)index), NSIndexSet.FromNSRange(new NSRange(0, Handler.Control.TableColumns().Length)));
+			}
+
 			public override void InsertItem(int index, object item)
 			{
-				if (Handler.Control.RespondsToSelector(selInsertRowsWithAnimation))
-				{
-					Handler.Control.BeginUpdates();
-					Handler.Control.InsertRows(new NSIndexSet(index), NSTableViewAnimation.SlideDown);
-					Handler.Control.EndUpdates();
-				}
-				else
-				{
-					var rows = Handler.SelectedRows.Select(r => r >= index ? r + 1 : r).ToArray();
-					Handler.SuppressSelectionChanged++;
-					Handler.Control.ReloadData();
-					Handler.SelectedRows = rows;
-					Handler.SuppressSelectionChanged--;
-				}
-
-				Handler.AutoSizeColumns(true);
+				Handler.Control.InsertRows(new NSIndexSet(index), NSTableViewAnimation.SlideDown);
 			}
 
 			public override void RemoveItem(int index)
 			{
-				if (Handler.Control.RespondsToSelector(selRemoveRowsWithAnimation))
-				{
-					Handler.Control.BeginUpdates();
-					Handler.Control.RemoveRows(new NSIndexSet(index), NSTableViewAnimation.SlideUp);
-					Handler.Control.EndUpdates();
-				}
-				else
-				{
-					// need to adjust selected rows to shift them up
-					bool isSelected = false;
-					var rows = Handler.SelectedRows.Where(r =>
-					{
-						if (r != index)
-							return true;
-						isSelected = true;
-						return false;
-					}).Select(r => r > index ? r - 1 : r).ToArray();
-					Handler.SuppressSelectionChanged++;
-					Handler.Control.ReloadData();
-					Handler.SelectedRows = rows;
-					Handler.SuppressSelectionChanged--;
-					// item being removed was selected, so trigger change
-					if (isSelected)
-						Handler.Callback.OnSelectionChanged(Handler.Widget, EventArgs.Empty);
-				}
-
-				Handler.AutoSizeColumns(true);
+				Handler.Control.RemoveRows(new NSIndexSet(index), NSTableViewAnimation.SlideUp);
 			}
 
 			public override void RemoveAllItems()
 			{
 				Handler.Control.ReloadData();
-				Handler.AutoSizeColumns(true);
 			}
 		}
 

diff --git a/src/Eto/CollectionChangedHandler.cs b/src/Eto/CollectionChangedHandler.cs
@@ -250,9 +250,33 @@ public virtual void Reset()
 		if (items != null)
 			AddRange(items);
 	}
+
+	/// <summary>
+	/// Called before any methods to change the collection have been called.
+	/// </summary>
+	/// <remarks>
+	/// There may be multiple methods called when the collection changes, such as RemoveRange then AddRange or
+	/// RemoveItem then InsertItem.  When this happens, some platforms may need to know all of the changes before
+	/// refreshing the view.  After all of the update methods, <see cref="EndUpdates"/> is called to signal that they are done.
+	/// </remarks>
+	/// <seealso cref="EndUpdates"/>
+	protected virtual void BeginUpdates()
+	{
+	}
+
+	/// <summary>
+	/// Called after any methods to change the collection have been called.
+	/// </summary>
+	/// <remarks>
+	/// See <see cref="BeginUpdates"/> for details.
+	/// </remarks>
+	protected virtual void EndUpdates()
+	{
+	}
 
 	private protected virtual void CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 	{
+		BeginUpdates();
 		switch (e.Action)
 		{
 			case NotifyCollectionChangedAction.Add:
@@ -287,23 +311,60 @@ private protected virtual void CollectionChanged(object sender, NotifyCollection
 			case NotifyCollectionChangedAction.Replace:
 				if (e.OldStartingIndex != -1)
 				{
-					RemoveRange(e.OldStartingIndex, e.OldItems.Count);
-					InsertRange(e.OldStartingIndex, e.NewItems.Cast<TItem>());
+					if (e.OldItems.Count == e.NewItems.Count && e.OldStartingIndex == e.NewStartingIndex)
+					{
+						ReplaceRange(e.OldStartingIndex, e.NewItems.Cast<TItem>());
+					}
+					else
+					{
+						RemoveRange(e.OldStartingIndex, e.OldItems.Count);
+						InsertRange(e.NewStartingIndex, e.NewItems.Cast<TItem>());
+					}
 				}
 				else
 				{
 					for (int i = 0; i < e.OldItems.Count; i++)
 					{
 						var index = IndexOf((TItem)e.OldItems[i]);
-						RemoveItem(index);
-						InsertItem(index, (TItem)e.NewItems[i]);
+						ReplaceItem(index, (TItem)e.NewItems[i]);
 					}
 				}
 				break;
 			case NotifyCollectionChangedAction.Reset:
 				Reset();
 				break;
 		}
+		EndUpdates();
+	}
+
+	/// <summary>
+	/// Replaces the item at the specified index
+	/// </summary>
+	/// <remarks>
+	/// This by default will remove then insert the item at the specified index.
+	/// </remarks>
+	/// <param name="index">Index of the item to replace</param>
+	/// <param name="newItem">New item</param>
+	public virtual void ReplaceItem(int index, TItem newItem)
+	{
+		RemoveItem(index);
+		InsertItem(index, newItem);
+	}
+
+	/// <summary>
+	/// Replaces a range of items starting at the specified index.
+	/// </summary>
+	/// <remarks>
+	/// By default this calls <see cref="ReplaceItem"/> for each item.
+	/// </remarks>
+	/// <param name="index">Index of the start of the range</param>
+	/// <param name="newItems">New items to replace the existing items at the same indexes</param>
+	public virtual void ReplaceRange(int index, IEnumerable<TItem> newItems)
+	{
+		foreach (var item in newItems)
+		{
+			ReplaceItem(index++, item);
+		}
 	}
 }
 

diff --git a/test/Eto.Test/UnitTests/Forms/Controls/GridViewTests.cs b/test/Eto.Test/UnitTests/Forms/Controls/GridViewTests.cs
@@ -10,6 +10,42 @@ protected override void SetDataStore(GridView grid, IEnumerable<object> dataStor
 			grid.DataStore = dataStore;
 		}
 
+		[Test, ManualTest]
+		public void MultipleChangesShouldWork() => ManualForm(
+			"Scroll while the collection is updated,\nand ensure all items are correct and not duplicated.",
+			form =>
+			{
+
+				var collection = new ObservableCollection<GridItem>();
+				for (int i = 0; i < 20; i++)
+				{
+					collection.Add(new GridItem(true, $"Item {i}"));
+				}
+
+				var gv = new GridView
+				{
+					DataStore = collection,
+					Size = new Size(200, 200),
+					Columns = {
+						new GridColumn { HeaderText = "Check", DataCell = new CheckBoxCell(0) },
+						new GridColumn { HeaderText = "Text", DataCell = new TextBoxCell(1) }
+					},
+				};
+
+				Application.Instance.AsyncInvoke(async () =>
+				{
+					for (int i = 0; i < collection.Count; i++)
+					{
+						if (!form.Loaded)
+							return;
+						// gv.SelectedRow = i;
+						await Task.Delay(1000);
+						collection[i] = new GridItem(true, $"Changed {i}");
+					}
+				});
+				return gv;
+			});
+
 		[Test, ManualTest]
 		public void CellClickShouldHaveMouseInformation()
 		{