Merge pull request #297 from xamarin/ermau-fix-scollectionview

Fix filtering issue with SimpleCollectionView

2 files changed, 10 insertions, 28 deletions

diff --git a/Xamarin.PropertyEditing/ViewModels/ResourceSelectorViewModel.cs b/Xamarin.PropertyEditing/ViewModels/ResourceSelectorViewModel.cs
index 8bd6f2f..2c8d57e 100644
--- a/Xamarin.PropertyEditing/ViewModels/ResourceSelectorViewModel.cs
+++ b/Xamarin.PropertyEditing/ViewModels/ResourceSelectorViewModel.cs
@@ -154,10 +154,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 				return;
 			}
 
-			if (this.resourcesView.Options.Filter != ResourceFilter)
-				this.resourcesView.Options.Filter = ResourceFilter;
-			else
-				this.resourcesView.UpdateFilter();
+			this.resourcesView.Options.Filter = ResourceFilter;
 		}
 
 		private bool ResourceFilter (object item)
diff --git a/Xamarin.PropertyEditing/ViewModels/SimpleCollectionView.cs b/Xamarin.PropertyEditing/ViewModels/SimpleCollectionView.cs
index c0b2245..6d95003 100644
--- a/Xamarin.PropertyEditing/ViewModels/SimpleCollectionView.cs
+++ b/Xamarin.PropertyEditing/ViewModels/SimpleCollectionView.cs
@@ -71,9 +71,8 @@ namespace Xamarin.PropertyEditing.ViewModels
 			get { return this.filter; }
 			set
 			{
-				if (this.filter == value)
-					return;
-
+				// Purposefully doesn't check for existing equality since a common case is setting to the same delegate but
+				// variables it checks only have changed.
 				this.filter = value;
 				OnPropertyChanged();
 			}
@@ -112,19 +111,6 @@ namespace Xamarin.PropertyEditing.ViewModels
 
 		public bool IsFiltering => Options?.Filter != null;
 
-		/// <summary>
-		/// Forces an update of the contens as if the filter has changed.
-		/// </summary>
-		/// <remarks>
-		/// Some filters may not change their delegate, but that delegate may act different based on outside forces.
-		/// In that scenario, call this to trigger the update.
-		/// </remarks>
-		public void UpdateFilter()
-		{
-			FilterCore (isPureSubset: this.wasFilterNull);
-			this.wasFilterNull = Options.Filter == null;
-		}
-
 		public IEnumerator GetEnumerator ()
 		{
 			bool haveChildren = (Options.ChildOptions != null);
@@ -148,9 +134,9 @@ namespace Xamarin.PropertyEditing.ViewModels
 		private readonly SimpleCollectionView parent;
 		private readonly string key;
 		private readonly object item;
+		private readonly IEnumerable source;
 		private SimpleCollectionViewOptions options;
 		private bool wasFilterNull;
-		private IEnumerable source;
 
 		private IComparer<string> Comparer => Options?.DisplayComparer ?? Comparer<string>.Default;
 
@@ -215,7 +201,8 @@ namespace Xamarin.PropertyEditing.ViewModels
 		private void OnOptionsPropertyChanged (object sender, PropertyChangedEventArgs e)
 		{
 			if (e.PropertyName == nameof(SimpleCollectionViewOptions.Filter)) {
-				UpdateFilter();
+				FilterCore (isPureSubset: this.wasFilterNull);
+				this.wasFilterNull = Options.Filter == null;
 			} else
 				Reset();
 		}
@@ -251,6 +238,8 @@ namespace Xamarin.PropertyEditing.ViewModels
 				}
 			}
 
+			RemoveFiltered (toRemove, notify);
+
 			if (!isPureSubset) {
 				var toAdd = new List<string> (filteredOut.Count);
 				foreach (string key in filteredOut) {
@@ -263,8 +252,6 @@ namespace Xamarin.PropertyEditing.ViewModels
 				AddFiltered (toAdd, notify);
 			}
 
-			RemoveFiltered (toRemove, notify);
-
 			if (hadChildren != HasChildElements)
 				OnPropertyChanged (nameof(HasChildElements));
 		}
@@ -288,7 +275,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 					
 					if (Comparer.Compare (currentKey, key) > 0) {
 						index = i;
-						this.filtered.Insert (i, key, this.arranged[i]);
+						this.filtered.Insert (i, key, this.arranged[key]);
 						added = true;
 						break;
 					}
@@ -388,15 +375,13 @@ namespace Xamarin.PropertyEditing.ViewModels
 
 				this.arranged.Add (sourceItem.Key, e);
 
-				SimpleCollectionView childView = null;
 				IEnumerable children = (Options.ChildrenSelector != null) ? this.options.ChildrenSelector (sourceItem.Item) : sourceItem.Item as IEnumerable;
 				if (Options.ChildOptions != null) {
 					if (children == null) {
 						throw new InvalidOperationException ("ChildOptions specified, but element not enumerable or no selector");
 					}
 
-					childView = new SimpleCollectionView (children, Options.ChildOptions, this, sourceItem.Key, sourceItem.Item);
-					e.ChildrenView = childView;
+					e.ChildrenView = new SimpleCollectionView (children, Options.ChildOptions, this, sourceItem.Key, sourceItem.Item);
 				}
 
 				if (!filtering || this.options.Filter (e.Item))