Skip to content

Commit 880f63e

Browse files
michael-hawkermichael-hawker
authored
Merge pull request #1627 from Shayon/fix_refilter_bug_and_ordering_bug
Fix some ACV bugs
2 parents ae3771b + 9a32086 commit 880f63e

File tree

2 files changed

+236
-25
lines changed

2 files changed

+236
-25
lines changed

‎Microsoft.Toolkit.Uwp.UI/AdvancedCollectionView/AdvancedCollectionView.cs‎

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,7 @@ public void Insert(int index, object item)
208208
throw new NotSupportedException("Collection is read-only.");
209209
}
210210

211-
if (_sortDescriptions.Count > 0 || _filter != null)
212-
{
213-
// no sense in inserting w/ filters or sorts, just add it
214-
_sourceList.Add(item);
215-
}
216-
else
217-
{
218-
_sourceList.Insert(index, item);
219-
}
211+
_sourceList.Insert(index, item);
220212
}
221213

222214
/// <summary>
@@ -555,15 +547,20 @@ private void HandleFilterChanged()
555547
}
556548

557549
var viewHash = new HashSet<object>(_view);
550+
var viewIndex = 0;
558551
for (var index = 0; index < _sourceList.Count; index++)
559552
{
560553
var item = _sourceList[index];
561554
if (viewHash.Contains(item))
562555
{
556+
viewIndex++;
563557
continue;
564558
}
565559

566-
HandleItemAdded(index, item);
560+
if (HandleItemAdded(index, item, viewIndex))
561+
{
562+
viewIndex++;
563+
}
567564
}
568565
}
569566

@@ -642,50 +639,71 @@ private void SourceNcc_CollectionChanged(object sender, NotifyCollectionChangedE
642639
}
643640
}
644641

645-
private void HandleItemAdded(int newStartingIndex, object newItem)
642+
private bool HandleItemAdded(int newStartingIndex, object newItem, int? viewIndex = null)
646643
{
647644
if (_filter != null && !_filter(newItem))
648645
{
649-
return;
646+
return false;
650647
}
651648

649+
var newViewIndex = _view.Count;
650+
652651
if (_sortDescriptions.Any())
653652
{
654653
_sortProperties.Clear();
655-
newStartingIndex = _view.BinarySearch(newItem, this);
656-
if (newStartingIndex < 0)
654+
newViewIndex = _view.BinarySearch(newItem, this);
655+
if (newViewIndex < 0)
657656
{
658-
newStartingIndex = ~newStartingIndex;
657+
newViewIndex = ~newViewIndex;
659658
}
660659
}
661660
else if (_filter != null)
662661
{
663662
if (_sourceList == null)
664663
{
665664
HandleSourceChanged();
666-
return;
665+
return false;
667666
}
668667

669-
var visibleBelowIndex = 0;
670-
for (var i = newStartingIndex; i < _sourceList.Count; i++)
668+
if (newStartingIndex == 0 || _view.Count == 0)
669+
{
670+
newViewIndex = 0;
671+
}
672+
else if (newStartingIndex == _sourceList.Count - 1)
673+
{
674+
newViewIndex = _view.Count - 1;
675+
}
676+
else if (viewIndex.HasValue)
677+
{
678+
newViewIndex = viewIndex.Value;
679+
}
680+
else
671681
{
672-
if (!_filter(_sourceList[i]))
682+
for (int i = 0, j = 0; i < _sourceList.Count; i++)
673683
{
674-
visibleBelowIndex++;
684+
if (i == newStartingIndex)
685+
{
686+
newViewIndex = j;
687+
break;
688+
}
689+
690+
if (_view[j] == _sourceList[i])
691+
{
692+
j++;
693+
}
675694
}
676695
}
677-
678-
newStartingIndex = _view.Count - visibleBelowIndex;
679696
}
680697

681-
_view.Insert(newStartingIndex, newItem);
682-
if (newStartingIndex <= _index)
698+
_view.Insert(newViewIndex, newItem);
699+
if (newViewIndex <= _index)
683700
{
684701
_index++;
685702
}
686703

687-
var e = new VectorChangedEventArgs(CollectionChange.ItemInserted, newStartingIndex, newItem);
704+
var e = new VectorChangedEventArgs(CollectionChange.ItemInserted, newViewIndex, newItem);
688705
OnVectorChanged(e);
706+
return true;
689707
}
690708

691709
private void HandleItemRemoved(int oldStartingIndex, object oldItem)

‎UnitTests/UI/Test_AdvancedCollectionView.cs‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,132 @@ public void Test_AdvancedCollectionView_Filter()
6868
Assert.AreEqual(2, a.Count);
6969
}
7070

71+
[TestCategory("AdvancedCollectionView")]
72+
[UITestMethod]
73+
public void Test_AdvancedCollectionView_Filter_Preserves_Order()
74+
{
75+
var l = new ObservableCollection<string>
76+
{
77+
"lorem",
78+
"ipsum",
79+
"dolor",
80+
"sit",
81+
"amet"
82+
};
83+
84+
var a = new AdvancedCollectionView(l)
85+
{
86+
Filter = (x) => x.ToString().Length < 5
87+
};
88+
89+
Assert.AreEqual(2, a.Count);
90+
Assert.AreEqual("sit", a[0]);
91+
Assert.AreEqual("amet", a[1]);
92+
93+
a.Insert(4, "how");
94+
95+
Assert.AreEqual(3, a.Count);
96+
Assert.AreEqual("sit", a[0]);
97+
Assert.AreEqual("how", a[1]);
98+
Assert.AreEqual("amet", a[2]);
99+
100+
}
101+
102+
[TestCategory("AdvancedCollectionView")]
103+
[UITestMethod]
104+
public void Test_AdvancedCollectionView_Filter_Preserves_Order_When_Inserting_Duplicate()
105+
{
106+
var l = new ObservableCollection<string>
107+
{
108+
"lorem",
109+
"ipsum",
110+
"dolor",
111+
"sit",
112+
"amet"
113+
};
114+
115+
var a = new AdvancedCollectionView(l)
116+
{
117+
Filter = (x) => x.ToString().Length >= 5
118+
};
119+
120+
Assert.AreEqual(3, a.Count);
121+
Assert.AreEqual("lorem", a[0]);
122+
Assert.AreEqual("ipsum", a[1]);
123+
Assert.AreEqual("dolor", a[2]);
124+
125+
a.Insert(3, "ipsum");
126+
127+
Assert.AreEqual(4, a.Count);
128+
Assert.AreEqual("lorem", a[0]);
129+
Assert.AreEqual("ipsum", a[1]);
130+
Assert.AreEqual("dolor", a[2]);
131+
Assert.AreEqual("ipsum", a[3]);
132+
133+
}
134+
135+
[TestCategory("AdvancedCollectionView")]
136+
[UITestMethod]
137+
public void Test_AdvancedCollectionView_Updating_Filter_Preserves_Order_With_Duplicate()
138+
{
139+
var l = new ObservableCollection<string>
140+
{
141+
"lorem",
142+
"ipsum",
143+
"dolor",
144+
"sit",
145+
"ipsum",
146+
"amet"
147+
};
148+
149+
var a = new AdvancedCollectionView(l)
150+
{
151+
Filter = (x) => x.ToString().Length < 5
152+
};
153+
154+
Assert.AreEqual(2, a.Count);
155+
Assert.AreEqual("sit", a[0]);
156+
Assert.AreEqual("amet", a[1]);
157+
158+
a.Filter = (x) => x.ToString().Length >= 5;
159+
160+
Assert.AreEqual(4, a.Count);
161+
Assert.AreEqual("lorem", a[0]);
162+
Assert.AreEqual("ipsum", a[1]);
163+
Assert.AreEqual("dolor", a[2]);
164+
Assert.AreEqual("ipsum", a[3]);
165+
166+
}
167+
168+
[TestCategory("AdvancedCollectionView")]
169+
[UITestMethod]
170+
public void Test_AdvancedCollectionView_Filter_Preserves_Order_When_Inserting_After_Items_In_View()
171+
{
172+
var l = new ObservableCollection<string>
173+
{
174+
"lorem",
175+
"ipsum",
176+
"dolor",
177+
"sitter",
178+
"amet"
179+
};
180+
181+
var a = new AdvancedCollectionView(l)
182+
{
183+
Filter = (x) => x.ToString().Length < 5
184+
};
185+
186+
Assert.AreEqual(1, a.Count);
187+
Assert.AreEqual(a[0], "amet");
188+
189+
a.Insert(0, "how");
190+
191+
Assert.AreEqual(2, a.Count);
192+
Assert.AreEqual(a[0], "how");
193+
Assert.AreEqual(a[1], "amet");
194+
195+
}
196+
71197
[TestCategory("AdvancedCollectionView")]
72198
[UITestMethod]
73199
public void Test_AdvancedCollectionView_Updating()
@@ -700,6 +826,73 @@ public void Test_AdvancedCollectionView_Combined_Using_Shaping()
700826
Assert.AreEqual(2, a.Count);
701827
}
702828

829+
[TestCategory("AdvancedCollectionView")]
830+
[UITestMethod]
831+
public void Test_AdvancedCollectionView_Combined_Using_Shaping_Filter_Back_In()
832+
{
833+
var l = new ObservableCollection<Person>
834+
{
835+
new Person()
836+
{
837+
Name = "lorem",
838+
Age = 4
839+
},
840+
new Person()
841+
{
842+
Name = "imsum",
843+
Age = 8
844+
},
845+
new Person()
846+
{
847+
Name = "dolor",
848+
Age = 15
849+
},
850+
new Person()
851+
{
852+
Name = "sit",
853+
Age = 16
854+
},
855+
new Person()
856+
{
857+
Name = "amet",
858+
Age = 23
859+
},
860+
new Person()
861+
{
862+
Name = "consectetur",
863+
Age = 42
864+
},
865+
};
866+
867+
var a = new AdvancedCollectionView(l, true);
868+
869+
a.Filter = (x) => ((Person)x).Name.Length > 5;
870+
a.RefreshFilter();
871+
872+
a.Filter = (x) => ((Person)x).Name.Length > 4;
873+
a.RefreshFilter();
874+
875+
a.SortDescriptions.Add(new SortDescription(nameof(Person.Age), SortDirection.Descending));
876+
877+
Assert.AreEqual(42, ((Person)a.First()).Age);
878+
Assert.AreEqual(4, a.Count);
879+
880+
l.Add(new Person
881+
{
882+
Name = "foo",
883+
Age = 50
884+
});
885+
886+
l.Add(new Person
887+
{
888+
Name = "Person McPersonface",
889+
Age = 10
890+
});
891+
892+
Assert.AreEqual(42, ((Person)a.First()).Age);
893+
Assert.AreEqual(5, a.Count);
894+
}
895+
703896
[TestCategory("AdvancedCollectionView")]
704897
[UITestMethod]
705898
public void Test_AdvancedCollectionView_Sorting_OnSelf_With_Shaping()

0 commit comments

Comments
 (0)