[UWP] Fixes CollectionChanged events in ListView

Xamarin.Forms.ControlGallery.WindowsUniversal/CustomRenderers.cs

@@ -11,6 +11,7 @@
 [assembly: ExportRenderer(typeof(Xamarin.Forms.Controls.Bugzilla42602.TextBoxView), typeof(Xamarin.Forms.ControlGallery.WindowsUniversal.TextBoxViewRenderer))]
 [assembly: ExportRenderer(typeof(Issue1683.EntryKeyboardFlags), typeof(EntryRendererKeyboardFlags))]
 [assembly: ExportRenderer(typeof(Issue1683.EditorKeyboardFlags), typeof(EditorRendererKeyboardFlags))]
+[assembly: ExportRenderer(typeof(Issue3273.SortableListView), typeof(SortableListViewRenderer))]
 namespace Xamarin.Forms.ControlGallery.WindowsUniversal
 {
 	public class EntryRendererKeyboardFlags : EntryRenderer
@@ -36,6 +37,24 @@ protected override void OnElementPropertyChanged(object sender, PropertyChangedE
 		}
 	}
 
+	public class SortableListViewRenderer : ListViewRenderer
+	{
+		protected override void OnElementChanged(ElementChangedEventArgs<ListView> e)
+		{
+			base.OnElementChanged(e);
+
+			if (e.NewElement != null)
+			{
+				var control = Control as Windows.UI.Xaml.Controls.ListView;
+
+				control.AllowDrop = true;
+				control.CanDragItems = true;
+				control.CanReorderItems = true;
+				control.ReorderMode = ListViewReorderMode.Enabled;
+			}
+		}
+	}
+
 	public static class KeyboardFlagExtensions
 	{
 		public static void TestKeyboardFlags(this FormsTextBox Control, KeyboardFlags? flags)

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla56771.cs

@@ -39,6 +39,11 @@ private void InitializeData()
 
 		protected override void Init()
 		{
+			data.CollectionChanged += (_, e) =>
+			{
+				var log = $"<{DateTime.Now.ToLongTimeString()}> {e.Action} action fired.";
+				System.Diagnostics.Debug.WriteLine(log);
+			};
 			var label = new Label { Text = "Click the Add 2 button." };
 			var button = new Button
 			{

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3273.cs

@@ -0,0 +1,82 @@
+using System;
+using System.Collections.ObjectModel;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+using Xamarin.Forms.Core.UITests;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if UITEST
+	[Category(UITestCategories.ListView)]
+#endif
+	[Preserve (AllMembers = true)]
+	[Issue (IssueTracker.Github, 3273, "Drag and drop reordering not firing CollectionChanged", PlatformAffected.UWP)]
+	public class Issue3273 : TestContentPage
+	{
+		[Preserve(AllMembers = true)]
+		public class SortableListView : ListView
+		{
+		}
+
+		protected override void Init ()
+		{
+			var statusLabel = new Label();
+			var actionLabel = new Label();
+			var Items = new ObservableCollection<string>
+			{
+				"drag",
+				"and",
+				"drop",
+				"me",
+				"please"
+			};
+			BindingContext = Items;
+
+			Items.CollectionChanged += (_, e) =>
+			{
+				statusLabel.Text = "Success";
+				var log = $"<{DateTime.Now.ToLongTimeString()}> {e.Action} action fired.";
+				actionLabel.Text += $"{log}{Environment.NewLine}";
+				System.Diagnostics.Debug.WriteLine(log);
+			};
+			Items.RemoveAt(4);
+			Items.Move(0, 1);
+
+			var listView = new SortableListView();
+			listView.SetBinding(ListView.ItemsSourceProperty, ".");
+
+			Content = new StackLayout
+			{
+				Children = {
+					statusLabel,
+					new Button {
+						Text = "Move items",
+						Command = new Command(() =>
+						{
+							actionLabel.Text = string.Empty;
+							statusLabel.Text = "Failed";
+							Items.Move(0, 1);
+						})
+					},
+					listView,
+					new ListView(),
+					actionLabel
+				}
+			};
+		}
+
+#if UITEST
+		[Test]
+		public void Issue3273Test()
+		{
+			RunningApp.WaitForElement("Move items");
+			RunningApp.Tap("Move items");
+			RunningApp.WaitForElement("Success");
+		}
+#endif
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -258,6 +258,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue3271.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3390.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3000.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue3273.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3053.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue2617.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3087.cs" />

Xamarin.Forms.Platform.UAP/ListViewRenderer.cs

@@ -25,12 +25,13 @@ namespace Xamarin.Forms.Platform.UWP
 	public class ListViewRenderer : ViewRenderer<ListView, FrameworkElement>
 	{
 		ITemplatedItemsView<Cell> TemplatedItemsView => Element;
-		ObservableCollection<object> SourceItems => _context?.Source as ObservableCollection<object>;
-		CollectionViewSource _context;
+		bool _collectionIsWrapped;
+		IList _collection = null;
 		bool _itemWasClicked;
 		bool _subscribedToItemClick;
 		bool _subscribedToTapped;
 		bool _disposed;
+		CollectionViewSource _collectionViewSource;
 
 		protected WListView List { get; private set; }
 
@@ -63,9 +64,7 @@ protected override void OnElementChanged(ElementChangedEventArgs<ListView> e)
 						GroupStyleSelector = (GroupStyleSelector)WApp.Current.Resources["ListViewGroupSelector"]
 					};
 
-					List.SelectionChanged += OnControlSelectionChanged;
-
-					List.SetBinding(ItemsControl.ItemsSourceProperty, "");
+					List.SelectionChanged += OnControlSelectionChanged;	
 				}
 
 				ReloadData();
@@ -82,76 +81,104 @@ protected override void OnElementChanged(ElementChangedEventArgs<ListView> e)
 			}
 		}
 
-		void ReloadData()
+		bool IsObservableCollection(object source)
 		{
-			if (Element?.ItemsSource == null && _context != null)
-				_context.Source = null;
-
-			var allSourceItems = new ObservableCollection<object>();
+			var type = source.GetType();
+			return type.IsGenericType &&
+				   type.GetGenericTypeDefinition() == typeof(ObservableCollection<>);
+		}
 
-			if (Element?.ItemsSource != null)
+		void ReloadData()
+		{
+			if (Element?.ItemsSource == null)
 			{
-				foreach (var item in Element.ItemsSource)
-					allSourceItems.Add(item);
+				_collection = null;
+			}
+			else
+			{
+				_collectionIsWrapped = !IsObservableCollection(Element.ItemsSource);
+				if (_collectionIsWrapped)
+				{
+					_collection = new ObservableCollection<object>();
+					foreach (var item in Element.ItemsSource)
+						_collection.Add(item);
+				}
+				else
+				{
+					_collection = (IList)Element.ItemsSource;
+				}
 			}
 
-			// WinRT throws an exception if you set ItemsSource directly to a CVS, so bind it.
-			List.DataContext = _context = new CollectionViewSource
+			if (_collectionViewSource != null)
+				_collectionViewSource.Source = null;
+
+			_collectionViewSource = new CollectionViewSource
 			{
-				Source = allSourceItems,
+				Source = _collection,
 				IsSourceGrouped = Element.IsGroupingEnabled
 			};
+
+			List.ItemsSource = _collectionViewSource.View;
 		}
 
 		void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 		{
-			switch (e.Action)
+			if (_collectionIsWrapped && _collection != null)
 			{
-				case NotifyCollectionChangedAction.Add:
-					if (e.NewStartingIndex < 0)
-						goto case NotifyCollectionChangedAction.Reset;
+				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 >= _collection.Count)
+						{
+							for (int i = 0; i < e.NewItems.Count; i++)
+								_collection.Add((e.NewItems[i] as BindableObject).BindingContext);
+						}
+						else
+						{
+							for (int i = e.NewItems.Count - 1; i >= 0; i--)
+								_collection.Insert(e.NewStartingIndex, (e.NewItems[i] as BindableObject).BindingContext);
+						}
 
-					// 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--)
+							_collection.RemoveAt(e.OldStartingIndex);
+						break;
+					case NotifyCollectionChangedAction.Move:
+						for (var i = 0; i < e.OldItems.Count; i++)
+						{
+							var oldi = e.OldStartingIndex;
+							var newi = e.NewStartingIndex;
 
-					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;
+							}
 
-						if (e.NewStartingIndex < e.OldStartingIndex)
-						{
-							oldi += i;
-							newi += i;
+							// we know that wrapped collection is an ObservableCollection<object>
+							((ObservableCollection<object>)_collection).Move(oldi, newi);
 						}
-
-						SourceItems.Move(oldi, newi);
-					}
-					break;
-				case NotifyCollectionChangedAction.Replace:
-				case NotifyCollectionChangedAction.Reset:
-				default:
-					ClearSizeEstimate();
-					ReloadData();
-					break;
+						break;
+					case NotifyCollectionChangedAction.Replace:
+					case NotifyCollectionChangedAction.Reset:
+					default:
+						ClearSizeEstimate();
+						ReloadData();
+						break;
+				}
+			}
+			else if (e.Action == NotifyCollectionChangedAction.Reset)
+			{
+				ClearSizeEstimate();
+				ReloadData();
 			}
 
 			Device.BeginInvokeOnMainThread(() => List?.UpdateLayout());
@@ -223,8 +250,17 @@ protected override void Dispose(bool disposing)
 						_subscribedToItemClick = false;
 						List.ItemClick -= OnListItemClicked;
 					}
+
 					List.SelectionChanged -= OnControlSelectionChanged;
+					if (_collectionViewSource != null)
+						_collectionViewSource.Source = null;
+
 					List.DataContext = null;
+
+					// Leaving this here as a warning because setting this to null causes
+					// an AccessViolationException if you run Issue1975
+					// List.ItemsSource = null;
+
 					List = null;
 				}
 
@@ -297,8 +333,8 @@ void UpdateGrouping()
 		{
 			bool grouping = Element.IsGroupingEnabled;
 
-			if (_context != null)
-				_context.IsSourceGrouped = grouping;
+			if (_collectionViewSource != null)
+				_collectionViewSource.IsSourceGrouped = grouping;
 
 			var templatedItems = TemplatedItemsView.TemplatedItems;
 			if (grouping && templatedItems.ShortNames != null)