fix(dragdrop): Don't use the Move on ObservableCollection in reorder …

…(avoid reflection and acts like UWP) + Other mix PR review
unoplatform · Dec 2, 2020 · 95b3438 · 95b3438
1 parent 186c978
commit 95b3438
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 62 deletions.
diff --git a/src/Uno.Foundation/Collections/IObservableVector.cs b/src/Uno.Foundation/Collections/IObservableVector.cs
@@ -7,16 +7,16 @@ internal interface IObservableVector
 		/// </summary>
 		event VectorChangedEventHandler UntypedVectorChanged;
 
-		public object this[int index] { get; }
+		object this[int index] { get; }
 
-		public int Count { get; }
+		int Count { get; }
 
-		public int IndexOf(object item);
+		int IndexOf(object item);
 
-		public void Add(object item);
+		void Add(object item);
 
-		public void Insert(int index, object item);
+		void Insert(int index, object item);
 
-		public void RemoveAt(int index);
+		void RemoveAt(int index);
 	}
 }
diff --git a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/DragItemsCompletedEventArgs.cs b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/DragItemsCompletedEventArgs.cs
@@ -1,8 +1,6 @@
 using System.Collections.Generic;
 using Windows.ApplicationModel.DataTransfer;
 
-#nullable enable
-
 namespace Windows.UI.Xaml.Controls
 {
 	public partial class DragItemsCompletedEventArgs

diff --git a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/DragItemsStartingEventArgs.cs b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/DragItemsStartingEventArgs.cs
@@ -5,7 +5,7 @@
 
 namespace Windows.UI.Xaml.Controls
 {
-	public  partial class DragItemsStartingEventArgs 
+	public partial class DragItemsStartingEventArgs 
 	{
 		private readonly DragStartingEventArgs? _inner;
 

diff --git a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.DragDrop.cs b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.DragDrop.cs
@@ -209,53 +209,23 @@ private static void OnReorderCompleted(object sender, _DragEventArgs dragEventAr
 					ProcessMove(
 						items.Count,
 						items.IndexOf,
-						(oldIndex, newIndex) =>
-						{
-							var item = items[oldIndex];
-							items.RemoveAt(oldIndex);
-							if (newIndex >= items.Count)
-							{
-								items.Add(item);
-							}
-							else
-							{
-								items.Insert(newIndex, item);
-							}
-						});
+						(oldIndex, newIndex) => DoMove(items, oldIndex, newIndex));
 					break;
 
 				case IObservableVector vector:
 					ProcessMove(
 						vector.Count,
 						vector.IndexOf,
-						(oldIndex, newIndex) =>
-						{
-							var item = vector[oldIndex];
-							vector.RemoveAt(oldIndex);
-							if (newIndex >= vector.Count)
-							{
-								vector.Add(item);
-							}
-							else
-							{
-								vector.Insert(newIndex, item);
-							}
-						});
+						(oldIndex, newIndex) => DoMove(vector, oldIndex, newIndex));
 					break;
 
-				default:
+				case IList list when IsObservableCollection(list):
 					// The UWP ListView seems to automatically push back changes only if the ItemsSource inherits from ObservableCollection
-					var srcType = unwrappedSource.GetType();
-					if (srcType.IsGenericType
-						&& srcType.GetGenericTypeDefinition() == typeof(ObservableCollection<>)
-						&& srcType.GetMethod(nameof(ObservableCollection<object>.Move), new[] { typeof(int), typeof(int) }) is { } mv)
-					{
-						ProcessMove(
-							((ICollection)unwrappedSource).Count,
-							((ICollection)unwrappedSource).IndexOf,
-							(oldIndex, newIndex) => mv.Invoke(unwrappedSource, new object[] { oldIndex, newIndex }));
-						return;
-					}
+					// Note: UWP does not use the Move method on ObservableCollection!
+					ProcessMove(
+						list.Count,
+						list.IndexOf,
+						(oldIndex, newIndex) => DoMove(list, oldIndex, newIndex));
 					break;
 			}
 
@@ -320,5 +290,55 @@ void ProcessMove(
 		partial void UpdateReordering(Point location, FrameworkElement draggedContainer, object draggedItem);
 
 		partial void CompleteReordering(FrameworkElement draggedContainer, object draggedItem, ref Uno.UI.IndexPath? updatedIndex);
+
+		#region Helpers
+		private static bool IsObservableCollection(object src)
+		{
+			var srcType = src.GetType();
+			return srcType.IsGenericType && srcType.GetGenericTypeDefinition() == typeof(ObservableCollection<>);
+		}
+
+		private static void DoMove(ItemCollection items, int oldIndex, int newIndex)
+		{
+			var item = items[oldIndex];
+			items.RemoveAt(oldIndex);
+			if (newIndex >= items.Count)
+			{
+				items.Add(item);
+			}
+			else
+			{
+				items.Insert(newIndex, item);
+			}
+		}
+
+		private static void DoMove(IObservableVector vector, int oldIndex, int newIndex)
+		{
+			var item = vector[oldIndex];
+			vector.RemoveAt(oldIndex);
+			if (newIndex >= vector.Count)
+			{
+				vector.Add(item);
+			}
+			else
+			{
+				vector.Insert(newIndex, item);
+			}
+		}
+
+		private static void DoMove(IList list, int oldIndex, int newIndex)
+		{
+			var item = list[oldIndex];
+			list.RemoveAt(oldIndex);
+			if (newIndex >= list.Count)
+			{
+				list.Add(item);
+			}
+			else
+			{
+				list.Insert(newIndex, item);
+			}
+		}
+		#endregion
 	}
 }
diff --git a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.cs b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.cs
@@ -19,7 +19,6 @@
 using Uno.Extensions.Specialized;
 using System.Collections;
 using System.Linq;
-using System.Reflection;
 using Windows.UI.Xaml.Controls.Primitives;
 using Uno.Logging;
 using Uno.Disposables;

diff --git a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.managed.cs b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.managed.cs
@@ -65,7 +65,7 @@ private protected VirtualizingPanelGenerator Generator
 		/// <summary>
 		/// The current average line height based on materialized lines. Used to estimate scroll extent of unmaterialized items.
 		/// </summary>
-		protected double _averageLineHeight;
+		private double _averageLineHeight;
 
 		/// <summary>
 		/// The previous item to the old first visible item, used when a lightweight layout rebuild is called.
@@ -868,24 +868,12 @@ private void SetBounds(FrameworkElement view, Rect bounds)
 
 		private double? GetItemsStart()
 		{
-			var firstView = GetFirstMaterializedLine()?.FirstView; // FUCK ICI IL FAUT QUE JE REPLACE MON ITEM AU BON ENDROIT ... le plus simple serait de déterminer le target "IndexPath"
-			//if (firstView == _pendingReorder?.element)
-			//{
-
-			//}
-
+			var firstView = GetFirstMaterializedLine()?.FirstView;
 			if (firstView != null)
 			{
 				return GetMeasuredStart(firstView);
 			}
 
-
-			//var halfLine = _averageLineHeight / 2;
-			//if (reorder.offset > extentOffset - halfLine && reorder.offset < extentOffset + halfLine)
-			//{
-			//	extentOffset += reorder.breadth;
-			//}
-
 			return null;
 		}