From 27e11150b78f0b62071de74f48bf3248d167e120 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 30 Jan 2020 11:52:13 +0100 Subject: [PATCH 01/10] Make comparing IndexPath with null do something useful. --- src/Avalonia.Controls/IndexPath.cs | 7 +++++++ .../Avalonia.Controls.UnitTests/IndexPathTests.cs | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/Avalonia.Controls/IndexPath.cs b/src/Avalonia.Controls/IndexPath.cs index 32d8c2b051..6c5aaf7ad1 100644 --- a/src/Avalonia.Controls/IndexPath.cs +++ b/src/Avalonia.Controls/IndexPath.cs @@ -58,6 +58,11 @@ namespace Avalonia.Controls public int GetAt(int index) { + if (index >= GetSize()) + { + throw new IndexOutOfRangeException(); + } + return _path?[index] ?? (_index - 1); } @@ -169,5 +174,7 @@ namespace Avalonia.Controls public static bool operator >=(IndexPath x, IndexPath y) { return x.CompareTo(y) >= 0; } public static bool operator ==(IndexPath x, IndexPath y) { return x.CompareTo(y) == 0; } public static bool operator !=(IndexPath x, IndexPath y) { return x.CompareTo(y) != 0; } + public static bool operator ==(IndexPath? x, IndexPath? y) { return (x ?? default).CompareTo(y ?? default) == 0; } + public static bool operator !=(IndexPath? x, IndexPath? y) { return (x ?? default).CompareTo(y ?? default) != 0; } } } diff --git a/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs b/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs index 190e92ed5e..1e4aa0a2b8 100644 --- a/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs +++ b/tests/Avalonia.Controls.UnitTests/IndexPathTests.cs @@ -77,5 +77,19 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(0, a.CompareTo(b)); Assert.Equal(a.GetHashCode(), b.GetHashCode()); } + + [Fact] + public void Null_Equality() + { + var a = new IndexPath(null); + var b = new IndexPath(1); + + // Implementing operator == on a struct automatically implements an operator which + // accepts null, so make sure this does something useful. + Assert.True(a == null); + Assert.False(a != null); + Assert.False(b == null); + Assert.True(b != null); + } } } From fcaa250c72cbbb691911456a1be0691663e41c63 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 3 Feb 2020 13:52:43 +0100 Subject: [PATCH 02/10] Ported fix and test from WinUI. https://github.com/microsoft/microsoft-ui-xaml/pull/1922 --- .../Utils/SelectionTreeHelper.cs | 18 +++++++---- .../SelectionModelTests.cs | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs b/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs index 93102a7b5b..430ecabbb8 100644 --- a/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs +++ b/src/Avalonia.Controls/Utils/SelectionTreeHelper.cs @@ -132,15 +132,21 @@ namespace Avalonia.Controls.Utils private static bool IsSubSet(IndexPath path, IndexPath subset) { - bool isSubset = true; - for (int i = 0; i < subset.GetSize(); i++) + var subsetSize = subset.GetSize(); + if (path.GetSize() < subsetSize) { - isSubset = path.GetAt(i) == subset.GetAt(i); - if (!isSubset) - break; + return false; + } + + for (int i = 0; i < subsetSize; i++) + { + if (path.GetAt(i) != subset.GetAt(i)) + { + return false; + } } - return isSubset; + return true; } private static IndexPath StartPath(IndexPath path, int length) diff --git a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs index 6c3137c636..208d85d8fd 100644 --- a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs @@ -898,6 +898,37 @@ namespace Avalonia.Controls.UnitTests }); } + [Fact] + public void SelectRangeRegressionTest() + { + RunOnUIThread.Execute(() => + { + var selectionModel = new SelectionModel() + { + Source = CreateNestedData(1, 2, 3) + }; + + // length of start smaller than end used to cause an out of range error. + selectionModel.SelectRange(IndexPath.CreateFrom(0), IndexPath.CreateFrom(1, 1)); + + ValidateSelection(selectionModel, + new List() + { + Path(0, 0), + Path(0, 1), + Path(0, 2), + Path(0), + Path(1, 0), + Path(1, 1) + }, + new List() + { + Path(), + Path(1) + }, + 1 /* selectedInnerNodes */); + }); + } [Fact] public void Disposing_Unhooks_CollectionChanged_Handlers() From bc4eefcf1b0e90d7d93cb9ffd09607a3e5d78fbe Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 26 Jan 2020 09:59:48 +0100 Subject: [PATCH 03/10] Add `IndexRange` list add/remove methods. Add or remove index ranges from a list of index ranges, merging and splitting ranges as required. --- src/Avalonia.Controls/IndexRange.cs | 173 +++++++++- .../IndexRangeTests.cs | 307 ++++++++++++++++++ 2 files changed, 474 insertions(+), 6 deletions(-) create mode 100644 tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs diff --git a/src/Avalonia.Controls/IndexRange.cs b/src/Avalonia.Controls/IndexRange.cs index b1a112ab39..124f1e0500 100644 --- a/src/Avalonia.Controls/IndexRange.cs +++ b/src/Avalonia.Controls/IndexRange.cs @@ -3,12 +3,17 @@ // // Licensed to The Avalonia Project under MIT License, courtesy of The .NET Foundation. +using System; +using System.Collections.Generic; + #nullable enable namespace Avalonia.Controls { - internal readonly struct IndexRange + internal readonly struct IndexRange : IEquatable { + private static readonly IndexRange s_invalid = new IndexRange(int.MinValue, int.MinValue); + public IndexRange(int begin, int end) { // Accept out of order begin/end pairs, just swap them. @@ -25,11 +30,9 @@ namespace Avalonia.Controls public int Begin { get; } public int End { get; } + public int Count => (End - Begin) + 1; - public bool Contains(int index) - { - return index >= Begin && index <= End; - } + public bool Contains(int index) => index >= Begin && index <= End; public bool Split(int splitIndex, out IndexRange before, out IndexRange after) { @@ -54,6 +57,164 @@ namespace Avalonia.Controls public bool Intersects(IndexRange other) { return (Begin <= other.End) && (End >= other.Begin); - } + } + + public bool Adjacent(IndexRange other) + { + return Begin == other.End + 1 || End == other.Begin - 1; + } + + public override bool Equals(object? obj) + { + return obj is IndexRange range && Equals(range); + } + + public bool Equals(IndexRange other) + { + return Begin == other.Begin && End == other.End; + } + + public override int GetHashCode() + { + var hashCode = 1903003160; + hashCode = hashCode * -1521134295 + Begin.GetHashCode(); + hashCode = hashCode * -1521134295 + End.GetHashCode(); + return hashCode; + } + + public override string ToString() => $"[{Begin}..{End}]"; + + public static bool operator ==(IndexRange left, IndexRange right) => left.Equals(right); + public static bool operator !=(IndexRange left, IndexRange right) => !(left == right); + + public static int Add( + IList ranges, + IndexRange range, + IList? added = null) + { + var result = 0; + + for (var i = 0; i < ranges.Count && range != s_invalid; ++i) + { + var existing = ranges[i]; + + if (range.Intersects(existing) || range.Adjacent(existing)) + { + if (range.Begin < existing.Begin) + { + var add = new IndexRange(range.Begin, existing.Begin - 1); + ranges[i] = new IndexRange(range.Begin, existing.End); + added?.Add(add); + result += add.Count; + } + + range = range.End <= existing.End ? + s_invalid : + new IndexRange(existing.End + 1, range.End); + } + else if (range.End < existing.Begin) + { + ranges.Insert(i, range); + added?.Add(range); + result += range.Count; + range = s_invalid; + } + } + + if (range != s_invalid) + { + ranges.Add(range); + added?.Add(range); + result += range.Count; + } + + MergeRanges(ranges); + return result; + } + + public static int Remove( + IList ranges, + IndexRange range, + IList? removed = null) + { + var result = 0; + + for (var i = 0; i < ranges.Count; ++i) + { + var existing = ranges[i]; + + if (range.Intersects(existing)) + { + if (range.Begin <= existing.Begin && range.End >= existing.End) + { + ranges.RemoveAt(i--); + removed?.Add(existing); + result += existing.Count; + } + else if (range.Begin > existing.Begin && range.End >= existing.End) + { + ranges[i] = new IndexRange(existing.Begin, range.Begin - 1); + removed?.Add(new IndexRange(range.Begin, existing.End)); + result += existing.End - (range.Begin - 1); + } + else if (range.Begin > existing.Begin && range.End < existing.End) + { + ranges[i] = new IndexRange(existing.Begin, range.Begin - 1); + ranges.Insert(++i, new IndexRange(range.End + 1, existing.End)); + removed?.Add(range); + result += range.Count; + } + else if (range.End <= existing.End) + { + var remove = new IndexRange(existing.Begin, range.End); + ranges[i] = new IndexRange(range.End + 1, existing.End); + removed?.Add(remove); + result += remove.Count; + } + } + } + + return result; + } + + public static IEnumerable Subtract( + IndexRange lhs, + IEnumerable rhs) + { + var result = new List { lhs }; + + foreach (var range in rhs) + { + Remove(result, range); + } + + return result; + } + + public static IEnumerable EnumerateIndices(IEnumerable ranges) + { + foreach (var range in ranges) + { + for (var i = range.Begin; i <= range.End; ++i) + { + yield return i; + } + } + } + + private static void MergeRanges(IList ranges) + { + for (var i = ranges.Count - 2; i >= 0; --i) + { + var r = ranges[i]; + var r1 = ranges[i + 1]; + + if (r.Intersects(r1) || r.End == r1.Begin - 1) + { + ranges[i] = new IndexRange(r.Begin, r1.End); + ranges.RemoveAt(i + 1); + } + } + } } } diff --git a/tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs b/tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs new file mode 100644 index 0000000000..e0f46d9fa9 --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs @@ -0,0 +1,307 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Xunit; + +namespace Avalonia.Controls.UnitTests +{ + public class IndexRangeTests + { + [Fact] + public void Add_Should_Add_Range_To_Empty_List() + { + var ranges = new List(); + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(0, 4), selected); + + Assert.Equal(5, result); + Assert.Equal(new[] { new IndexRange(0, 4) }, ranges); + Assert.Equal(new[] { new IndexRange(0, 4) }, selected); + } + + [Fact] + public void Add_Should_Add_Non_Intersecting_Range_At_End() + { + var ranges = new List { new IndexRange(0, 4) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(8, 10), selected); + + Assert.Equal(3, result); + Assert.Equal(new[] { new IndexRange(0, 4), new IndexRange(8, 10) }, ranges); + Assert.Equal(new[] { new IndexRange(8, 10) }, selected); + } + + [Fact] + public void Add_Should_Add_Non_Intersecting_Range_At_Beginning() + { + var ranges = new List { new IndexRange(8, 10) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(0, 4), selected); + + Assert.Equal(5, result); + Assert.Equal(new[] { new IndexRange(0, 4), new IndexRange(8, 10) }, ranges); + Assert.Equal(new[] { new IndexRange(0, 4) }, selected); + } + + [Fact] + public void Add_Should_Add_Non_Intersecting_Range_In_Middle() + { + var ranges = new List { new IndexRange(0, 4), new IndexRange(14, 16) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(8, 10), selected); + + Assert.Equal(3, result); + Assert.Equal(new[] { new IndexRange(0, 4), new IndexRange(8, 10), new IndexRange(14, 16) }, ranges); + Assert.Equal(new[] { new IndexRange(8, 10) }, selected); + } + + [Fact] + public void Add_Should_Add_Intersecting_Range_Start() + { + var ranges = new List { new IndexRange(8, 10) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(6, 9), selected); + + Assert.Equal(2, result); + Assert.Equal(new[] { new IndexRange(6, 10) }, ranges); + Assert.Equal(new[] { new IndexRange(6, 7) }, selected); + } + + [Fact] + public void Add_Should_Add_Intersecting_Range_End() + { + var ranges = new List { new IndexRange(8, 10) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(9, 12), selected); + + Assert.Equal(2, result); + Assert.Equal(new[] { new IndexRange(8, 12) }, ranges); + Assert.Equal(new[] { new IndexRange(11, 12) }, selected); + } + + [Fact] + public void Add_Should_Add_Intersecting_Range_Both() + { + var ranges = new List { new IndexRange(8, 10) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(6, 12), selected); + + Assert.Equal(4, result); + Assert.Equal(new[] { new IndexRange(6, 12) }, ranges); + Assert.Equal(new[] { new IndexRange(6, 7), new IndexRange(11, 12) }, selected); + } + + [Fact] + public void Add_Should_Join_Two_Intersecting_Ranges() + { + var ranges = new List { new IndexRange(8, 10), new IndexRange(12, 14) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(8, 14), selected); + + Assert.Equal(1, result); + Assert.Equal(new[] { new IndexRange(8, 14) }, ranges); + Assert.Equal(new[] { new IndexRange(11, 11) }, selected); + } + + [Fact] + public void Add_Should_Join_Two_Intersecting_Ranges_And_Add_Ranges() + { + var ranges = new List { new IndexRange(8, 10), new IndexRange(12, 14) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(6, 18), selected); + + Assert.Equal(7, result); + Assert.Equal(new[] { new IndexRange(6, 18) }, ranges); + Assert.Equal(new[] { new IndexRange(6, 7), new IndexRange(11, 11), new IndexRange(15, 18) }, selected); + } + + [Fact] + public void Add_Should_Not_Add_Already_Selected_Range() + { + var ranges = new List { new IndexRange(8, 10) }; + var selected = new List(); + var result = IndexRange.Add(ranges, new IndexRange(9, 10), selected); + + Assert.Equal(0, result); + Assert.Equal(new[] { new IndexRange(8, 10) }, ranges); + Assert.Empty(selected); + } + + [Fact] + public void Remove_Should_Remove_Entire_Range() + { + var ranges = new List { new IndexRange(8, 10) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(8, 10), deselected); + + Assert.Equal(3, result); + Assert.Empty(ranges); + Assert.Equal(new[] { new IndexRange(8, 10) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_Start_Of_Range() + { + var ranges = new List { new IndexRange(8, 12) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(8, 10), deselected); + + Assert.Equal(3, result); + Assert.Equal(new[] { new IndexRange(11, 12) }, ranges); + Assert.Equal(new[] { new IndexRange(8, 10) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_End_Of_Range() + { + var ranges = new List { new IndexRange(8, 12) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(10, 12), deselected); + + Assert.Equal(3, result); + Assert.Equal(new[] { new IndexRange(8, 9) }, ranges); + Assert.Equal(new[] { new IndexRange(10, 12) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_Overlapping_End_Of_Range() + { + var ranges = new List { new IndexRange(8, 12) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(10, 14), deselected); + + Assert.Equal(3, result); + Assert.Equal(new[] { new IndexRange(8, 9) }, ranges); + Assert.Equal(new[] { new IndexRange(10, 12) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_Middle_Of_Range() + { + var ranges = new List { new IndexRange(10, 20) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(12, 16), deselected); + + Assert.Equal(5, result); + Assert.Equal(new[] { new IndexRange(10, 11), new IndexRange(17, 20) }, ranges); + Assert.Equal(new[] { new IndexRange(12, 16) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_Multiple_Ranges() + { + var ranges = new List { new IndexRange(8, 10), new IndexRange(12, 14), new IndexRange(16, 18) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(6, 15), deselected); + + Assert.Equal(6, result); + Assert.Equal(new[] { new IndexRange(16, 18) }, ranges); + Assert.Equal(new[] { new IndexRange(8, 10), new IndexRange(12, 14) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_Multiple_And_Partial_Ranges_1() + { + var ranges = new List { new IndexRange(8, 10), new IndexRange(12, 14), new IndexRange(16, 18) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(9, 15), deselected); + + Assert.Equal(5, result); + Assert.Equal(new[] { new IndexRange(8, 8), new IndexRange(16, 18) }, ranges); + Assert.Equal(new[] { new IndexRange(9, 10), new IndexRange(12, 14) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_Multiple_And_Partial_Ranges_2() + { + var ranges = new List { new IndexRange(8, 10), new IndexRange(12, 14), new IndexRange(16, 18) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(8, 13), deselected); + + Assert.Equal(5, result); + Assert.Equal(new[] { new IndexRange(14, 14), new IndexRange(16, 18) }, ranges); + Assert.Equal(new[] { new IndexRange(8, 10), new IndexRange(12, 13) }, deselected); + } + + [Fact] + public void Remove_Should_Remove_Multiple_And_Partial_Ranges_3() + { + var ranges = new List { new IndexRange(8, 10), new IndexRange(12, 14), new IndexRange(16, 18) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(9, 13), deselected); + + Assert.Equal(4, result); + Assert.Equal(new[] { new IndexRange(8, 8), new IndexRange(14, 14), new IndexRange(16, 18) }, ranges); + Assert.Equal(new[] { new IndexRange(9, 10), new IndexRange(12, 13) }, deselected); + } + + [Fact] + public void Remove_Should_Do_Nothing_For_Unselected_Range() + { + var ranges = new List { new IndexRange(8, 10) }; + var deselected = new List(); + var result = IndexRange.Remove(ranges, new IndexRange(2, 4), deselected); + + Assert.Equal(0, result); + Assert.Equal(new[] { new IndexRange(8, 10) }, ranges); + Assert.Empty(deselected); + } + + [Fact] + public void Stress_Test() + { + const int iterations = 100; + var random = new Random(0); + var selection = new List(); + var expected = new List(); + + IndexRange Generate() + { + var start = random.Next(100); + return new IndexRange(start, start + random.Next(20)); + } + + for (var i = 0; i < iterations; ++i) + { + var toAdd = random.Next(5); + + for (var j = 0; j < toAdd; ++j) + { + var range = Generate(); + IndexRange.Add(selection, range); + + for (var k = range.Begin; k <= range.End; ++k) + { + if (!expected.Contains(k)) + { + expected.Add(k); + } + } + + var actual = IndexRange.EnumerateIndices(selection).ToList(); + expected.Sort(); + Assert.Equal(expected, actual); + } + + var toRemove = random.Next(5); + + for (var j = 0; j < toRemove; ++j) + { + var range = Generate(); + IndexRange.Remove(selection, range); + + for (var k = range.Begin; k <= range.End; ++k) + { + expected.Remove(k); + } + + var actual = IndexRange.EnumerateIndices(selection).ToList(); + Assert.Equal(expected, actual); + } + + selection.Clear(); + expected.Clear(); + } + } + } +} From 7548dc9c2edaf687ebe1d1bfe9371386ef1d5df9 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 26 Jan 2020 11:47:51 +0100 Subject: [PATCH 04/10] Added SelectionModel changed args. `SelectionModel` as ported from WinUI has no information about what changed in a `SelectionChanged` event. This adds that information along with unit tests. --- src/Avalonia.Controls/SelectionModel.cs | 191 +++++--- .../SelectionModelChangeSet.cs | 144 ++++++ ...SelectionModelSelectionChangedEventArgs.cs | 45 ++ src/Avalonia.Controls/SelectionNode.cs | 150 +++---- .../SelectionModelTests.cs | 417 ++++++++++++++++++ 5 files changed, 804 insertions(+), 143 deletions(-) create mode 100644 src/Avalonia.Controls/SelectionModelChangeSet.cs diff --git a/src/Avalonia.Controls/SelectionModel.cs b/src/Avalonia.Controls/SelectionModel.cs index 34d5f78434..5e2fb32243 100644 --- a/src/Avalonia.Controls/SelectionModel.cs +++ b/src/Avalonia.Controls/SelectionModel.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; using System.ComponentModel; +using System.Linq; using Avalonia.Controls.Utils; #nullable enable @@ -19,7 +20,6 @@ namespace Avalonia.Controls private IReadOnlyList? _selectedIndicesCached; private IReadOnlyList? _selectedItemsCached; private SelectionModelChildrenRequestedEventArgs? _childrenRequestedEventArgs; - private SelectionModelSelectionChangedEventArgs? _selectionChangedEventArgs; public event EventHandler? ChildrenRequested; public event PropertyChangedEventHandler? PropertyChanged; @@ -36,9 +36,12 @@ namespace Avalonia.Controls get => _rootNode?.Source; set { - ClearSelection(resetAnchor: true, raiseSelectionChanged: false); + using (var operation = new Operation(this)) + { + ClearSelection(resetAnchor: true); + } + _rootNode.Source = value; - OnSelectionChanged(); RaisePropertyChanged("Source"); } } @@ -55,12 +58,13 @@ namespace Avalonia.Controls if (value && selectedIndices != null && selectedIndices.Count > 0) { + using var operation = new Operation(this); + // We want to be single select, so make sure there is only // one selected item. var firstSelectionIndexPath = selectedIndices[0]; - ClearSelection(resetAnchor: true, raiseSelectionChanged: false); - SelectWithPathImpl(firstSelectionIndexPath, select: true, raiseSelectionChanged: false); - // Setting SelectedIndex will raise SelectionChanged event. + ClearSelection(resetAnchor: true); + SelectWithPathImpl(firstSelectionIndexPath, select: true); SelectedIndex = firstSelectionIndexPath; } @@ -131,9 +135,9 @@ namespace Avalonia.Controls if (!isSelected.HasValue || !isSelected.Value) { - ClearSelection(resetAnchor: true, raiseSelectionChanged: false); - SelectWithPathImpl(value, select: true, raiseSelectionChanged: false); - OnSelectionChanged(); + using var operation = new Operation(this); + ClearSelection(resetAnchor: true); + SelectWithPathImpl(value, select: true); } } } @@ -289,7 +293,7 @@ namespace Avalonia.Controls public void Dispose() { - ClearSelection(resetAnchor: false, raiseSelectionChanged: false); + ClearSelection(resetAnchor: false); _rootNode?.Dispose(); _selectedIndicesCached = null; _selectedItemsCached = null; @@ -299,17 +303,41 @@ namespace Avalonia.Controls public void SetAnchorIndex(int groupIndex, int index) => AnchorIndex = new IndexPath(groupIndex, index); - public void Select(int index) => SelectImpl(index, select: true); + public void Select(int index) + { + using var operation = new Operation(this); + SelectImpl(index, select: true); + } - public void Select(int groupIndex, int itemIndex) => SelectWithGroupImpl(groupIndex, itemIndex, select: true); + public void Select(int groupIndex, int itemIndex) + { + using var operation = new Operation(this); + SelectWithGroupImpl(groupIndex, itemIndex, select: true); + } - public void SelectAt(IndexPath index) => SelectWithPathImpl(index, select: true, raiseSelectionChanged: true); + public void SelectAt(IndexPath index) + { + using var operation = new Operation(this); + SelectWithPathImpl(index, select: true); + } - public void Deselect(int index) => SelectImpl(index, select: false); + public void Deselect(int index) + { + using var operation = new Operation(this); + SelectImpl(index, select: false); + } - public void Deselect(int groupIndex, int itemIndex) => SelectWithGroupImpl(groupIndex, itemIndex, select: false); + public void Deselect(int groupIndex, int itemIndex) + { + using var operation = new Operation(this); + SelectWithGroupImpl(groupIndex, itemIndex, select: false); + } - public void DeselectAt(IndexPath index) => SelectWithPathImpl(index, select: false, raiseSelectionChanged: true); + public void DeselectAt(IndexPath index) + { + using var operation = new Operation(this); + SelectWithPathImpl(index, select: false); + } public bool? IsSelected(int index) { @@ -383,46 +411,56 @@ namespace Avalonia.Controls public void SelectRangeFromAnchor(int index) { + using var operation = new Operation(this); SelectRangeFromAnchorImpl(index, select: true); } public void SelectRangeFromAnchor(int endGroupIndex, int endItemIndex) { + using var operation = new Operation(this); SelectRangeFromAnchorWithGroupImpl(endGroupIndex, endItemIndex, select: true); } public void SelectRangeFromAnchorTo(IndexPath index) { + using var operation = new Operation(this); SelectRangeImpl(AnchorIndex, index, select: true); } public void DeselectRangeFromAnchor(int index) { + using var operation = new Operation(this); SelectRangeFromAnchorImpl(index, select: false); } public void DeselectRangeFromAnchor(int endGroupIndex, int endItemIndex) { + using var operation = new Operation(this); SelectRangeFromAnchorWithGroupImpl(endGroupIndex, endItemIndex, false /* select */); } public void DeselectRangeFromAnchorTo(IndexPath index) { + using var operation = new Operation(this); SelectRangeImpl(AnchorIndex, index, select: false); } public void SelectRange(IndexPath start, IndexPath end) { + using var operation = new Operation(this); SelectRangeImpl(start, end, select: true); } public void DeselectRange(IndexPath start, IndexPath end) { + using var operation = new Operation(this); SelectRangeImpl(start, end, select: false); } public void SelectAll() { + using var operation = new Operation(this); + SelectionTreeHelper.Traverse( _rootNode, realizeChildren: true, @@ -433,13 +471,12 @@ namespace Avalonia.Controls info.Node.SelectAll(); } }); - - OnSelectionChanged(); } public void ClearSelection() { - ClearSelection(resetAnchor: true, raiseSelectionChanged: true); + using var operation = new Operation(this); + ClearSelection(resetAnchor: true); } protected void OnPropertyChanged(string propertyName) @@ -452,9 +489,15 @@ namespace Avalonia.Controls PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); } - public void OnSelectionInvalidatedDueToCollectionChange() + public void OnSelectionInvalidatedDueToCollectionChange( + IEnumerable? removedItems) { - OnSelectionChanged(); + var e = new SelectionModelSelectionChangedEventArgs( + Enumerable.Empty(), + Enumerable.Empty(), + removedItems ?? Enumerable.Empty(), + Enumerable.Empty()); + OnSelectionChanged(e); } internal object? ResolvePath(object data, SelectionNode sourceNode) @@ -496,7 +539,7 @@ namespace Avalonia.Controls return resolved; } - private void ClearSelection(bool resetAnchor, bool raiseSelectionChanged) + private void ClearSelection(bool resetAnchor) { SelectionTreeHelper.Traverse( _rootNode, @@ -507,27 +550,17 @@ namespace Avalonia.Controls { AnchorIndex = default; } - - if (raiseSelectionChanged) - { - OnSelectionChanged(); - } } - private void OnSelectionChanged() + private void OnSelectionChanged(SelectionModelSelectionChangedEventArgs? e = null) { _selectedIndicesCached = null; _selectedItemsCached = null; // Raise SelectionChanged event - if (SelectionChanged != null) + if (e != null) { - if (_selectionChangedEventArgs == null) - { - _selectionChangedEventArgs = new SelectionModelSelectionChangedEventArgs(); - } - - SelectionChanged(this, _selectionChangedEventArgs); + SelectionChanged?.Invoke(this, e); } RaisePropertyChanged(nameof(SelectedIndex)); @@ -544,7 +577,7 @@ namespace Avalonia.Controls { if (_singleSelect) { - ClearSelection(resetAnchor: true, raiseSelectionChanged: false); + ClearSelection(resetAnchor: true); } var selected = _rootNode.Select(index, select); @@ -553,15 +586,13 @@ namespace Avalonia.Controls { AnchorIndex = new IndexPath(index); } - - OnSelectionChanged(); } private void SelectWithGroupImpl(int groupIndex, int itemIndex, bool select) { if (_singleSelect) { - ClearSelection(resetAnchor: true, raiseSelectionChanged: false); + ClearSelection(resetAnchor: true); } var childNode = _rootNode.GetAt(groupIndex, realizeChild: true); @@ -571,17 +602,15 @@ namespace Avalonia.Controls { AnchorIndex = new IndexPath(groupIndex, itemIndex); } - - OnSelectionChanged(); } - private void SelectWithPathImpl(IndexPath index, bool select, bool raiseSelectionChanged) + private void SelectWithPathImpl(IndexPath index, bool select) { bool selected = false; if (_singleSelect) { - ClearSelection(resetAnchor: true, raiseSelectionChanged: false); + ClearSelection(resetAnchor: true); } SelectionTreeHelper.TraverseIndexPath( @@ -601,11 +630,6 @@ namespace Avalonia.Controls { AnchorIndex = index; } - - if (raiseSelectionChanged) - { - OnSelectionChanged(); - } } private void SelectRangeFromAnchorImpl(int index, bool select) @@ -618,12 +642,7 @@ namespace Avalonia.Controls anchorIndex = anchor.GetAt(0); } - bool selected = _rootNode.SelectRange(new IndexRange(anchorIndex, index), select); - - if (selected) - { - OnSelectionChanged(); - } + _rootNode.SelectRange(new IndexRange(anchorIndex, index), select); } private void SelectRangeFromAnchorWithGroupImpl(int endGroupIndex, int endItemIndex, bool select) @@ -650,18 +669,12 @@ namespace Avalonia.Controls endItemIndex = temp; } - var selected = false; for (int groupIdx = startGroupIndex; groupIdx <= endGroupIndex; groupIdx++) { var groupNode = _rootNode.GetAt(groupIdx, realizeChild: true)!; int startIndex = groupIdx == startGroupIndex ? startItemIndex : 0; int endIndex = groupIdx == endGroupIndex ? endItemIndex : groupNode.DataCount - 1; - selected |= groupNode.SelectRange(new IndexRange(startIndex, endIndex), select); - } - - if (selected) - { - OnSelectionChanged(); + groupNode.SelectRange(new IndexRange(startIndex, endIndex), select); } } @@ -691,8 +704,55 @@ namespace Avalonia.Controls info.ParentNode!.Select(info.Path.GetAt(info.Path.GetSize() - 1), select); } }); + } + + private void BeginOperation() + { + if (SelectionChanged != null) + { + _rootNode.BeginOperation(); + } + } + + private void EndOperation() + { + static IEnumerable? Concat(IEnumerable? a, IEnumerable b) + { + return a == null ? b : a.Concat(b); + } - OnSelectionChanged(); + SelectionModelSelectionChangedEventArgs? e = null; + + if (SelectionChanged != null) + { + IEnumerable? selectedIndices = null; + IEnumerable? deselectedIndices = null; + IEnumerable? selectedItems = null; + IEnumerable? deselectedItems = null; + + foreach (var changes in _rootNode.EndOperation()) + { + if (changes.HasChanges) + { + selectedIndices = Concat(selectedIndices, changes.SelectedIndices); + deselectedIndices = Concat(deselectedIndices, changes.DeselectedIndices); + selectedItems = Concat(selectedItems, changes.SelectedItems); + deselectedItems = Concat(deselectedItems, changes.DeselectedItems); + } + } + + if (selectedIndices != null || deselectedIndices != null || + selectedItems != null || deselectedItems != null) + { + e = new SelectionModelSelectionChangedEventArgs( + deselectedIndices ?? Enumerable.Empty(), + selectedIndices ?? Enumerable.Empty(), + deselectedItems ?? Enumerable.Empty(), + selectedItems ?? Enumerable.Empty()); + } + } + + OnSelectionChanged(e); } internal class SelectedItemInfo @@ -706,5 +766,12 @@ namespace Avalonia.Controls public SelectionNode Node { get; } public IndexPath Path { get; } } + + private struct Operation : IDisposable + { + private readonly SelectionModel _manager; + public Operation(SelectionModel manager) => (_manager = manager).BeginOperation(); + public void Dispose() => _manager.EndOperation(); + } } } diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs new file mode 100644 index 0000000000..989136ac8d --- /dev/null +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -0,0 +1,144 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +#nullable enable + +namespace Avalonia.Controls +{ + internal class SelectionModelChangeSet + { + private SelectionNode _owner; + private List? _selected; + private List? _deselected; + + public SelectionModelChangeSet(SelectionNode owner) => _owner = owner; + + public bool IsTracking { get; private set; } + public bool HasChanges => _selected?.Count > 0 || _deselected?.Count > 0; + public IEnumerable SelectedIndices => EnumerateIndices(_selected); + public IEnumerable DeselectedIndices => EnumerateIndices(_deselected); + public IEnumerable SelectedItems => EnumerateItems(_selected); + public IEnumerable DeselectedItems => EnumerateItems(_deselected); + + public void BeginOperation() + { + if (IsTracking) + { + throw new AvaloniaInternalException("SelectionModel change operation already in progress."); + } + + IsTracking = true; + _selected?.Clear(); + _deselected?.Clear(); + } + + public void EndOperation() => IsTracking = false; + + public void Selected(IndexRange range) + { + if (!IsTracking) + { + return; + } + + Add(range, ref _selected, _deselected); + } + + public void Selected(IEnumerable ranges) + { + if (!IsTracking) + { + return; + } + + foreach (var range in ranges) + { + Selected(range); + } + } + + public void Deselected(IndexRange range) + { + if (!IsTracking) + { + return; + } + + Add(range, ref _deselected, _selected); + } + + public void Deselected(IEnumerable ranges) + { + if (!IsTracking) + { + return; + } + + foreach (var range in ranges) + { + Deselected(range); + } + } + + private static void Add( + IndexRange range, + ref List? add, + List? remove) + { + if (remove != null) + { + var removed = new List(); + IndexRange.Remove(remove, range, removed); + var selected = IndexRange.Subtract(range, removed); + + if (selected.Any()) + { + add ??= new List(); + + foreach (var r in selected) + { + IndexRange.Add(add, r); + } + } + } + else + { + add ??= new List(); + IndexRange.Add(add, range); + } + } + + private IEnumerable EnumerateIndices(IEnumerable? ranges) + { + var path = _owner.IndexPath; + + if (ranges != null) + { + foreach (var range in ranges) + { + for (var i = range.Begin; i <= range.End; ++i) + { + yield return path.CloneWithChildIndex(i); + } + } + } + } + + private IEnumerable EnumerateItems(IEnumerable? ranges) + { + var items = _owner.ItemsSourceView; + + if (ranges != null && items != null) + { + foreach (var range in ranges) + { + for (var i = range.Begin; i <= range.End; ++i) + { + yield return items.GetAt(i); + } + } + } + } + } +} diff --git a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs index c8edc1f8ae..4976bf1827 100644 --- a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs +++ b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs @@ -4,6 +4,7 @@ // Licensed to The Avalonia Project under MIT License, courtesy of The .NET Foundation. using System; +using System.Collections.Generic; #nullable enable @@ -11,5 +12,49 @@ namespace Avalonia.Controls { public class SelectionModelSelectionChangedEventArgs : EventArgs { + private readonly IEnumerable _selectedIndicesSource; + private readonly IEnumerable _deselectedIndicesSource; + private readonly IEnumerable _selectedItemsSource; + private readonly IEnumerable _deselectedItemsSource; + private List? _selectedIndices; + private List? _deselectedIndices; + private List? _selectedItems; + private List? _deselectedItems; + + public SelectionModelSelectionChangedEventArgs( + IEnumerable deselectedIndices, + IEnumerable selectedIndices, + IEnumerable deselectedItems, + IEnumerable selectedItems) + { + _selectedIndicesSource = selectedIndices; + _deselectedIndicesSource = deselectedIndices; + _selectedItemsSource = selectedItems; + _deselectedItemsSource = deselectedItems; + } + + /// + /// Gets the indices of the items that were added to the selection. + /// + public IReadOnlyList SelectedIndices => + _selectedIndices ?? (_selectedIndices = new List(_selectedIndicesSource)); + + /// + /// Gets the indices of the items that were removed from the selection. + /// + public IReadOnlyList DeselectedIndices => + _deselectedIndices ?? (_deselectedIndices = new List(_deselectedIndicesSource)); + + /// + /// Gets the items that were added to the selection. + /// + public IReadOnlyList SelectedItems => + _selectedItems ?? (_selectedItems = new List(_selectedItemsSource)); + + /// + /// Gets the items that were removed from the selection. + /// + public IReadOnlyList DeselectedItems => + _deselectedItems ?? (_deselectedItems = new List(_deselectedItemsSource)); } } diff --git a/src/Avalonia.Controls/SelectionNode.cs b/src/Avalonia.Controls/SelectionNode.cs index 363eb35b94..d462a51228 100644 --- a/src/Avalonia.Controls/SelectionNode.cs +++ b/src/Avalonia.Controls/SelectionNode.cs @@ -7,6 +7,7 @@ using System; using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; +using System.Linq; #nullable enable @@ -28,6 +29,7 @@ namespace Avalonia.Controls private readonly SelectionNode? _parent; private readonly List _selected = new List(); private readonly List _selectedIndicesCached = new List(); + private SelectionModelChangeSet? _changes; private object? _source; private bool _selectedIndicesCacheIsValid; @@ -134,6 +136,11 @@ namespace Avalonia.Controls { child = new SelectionNode(_manager, parent: this); child.Source = resolvedChild; + + if (_changes?.IsTracking == true) + { + child.BeginOperation(); + } } else { @@ -276,12 +283,50 @@ namespace Avalonia.Controls } } + public IEnumerable SelectedItems + { + get => SelectedIndices.Select(x => ItemsSourceView!.GetAt(x)); + } + public void Dispose() { ItemsSourceView?.Dispose(); UnhookCollectionChangedHandler(); } + public void BeginOperation() + { + _changes ??= new SelectionModelChangeSet(this); + _changes.BeginOperation(); + + for (var i = 0; i < _childrenNodes.Count; ++i) + { + _childrenNodes[i]?.BeginOperation(); + } + } + + public IEnumerable EndOperation() + { + if (_changes != null) + { + _changes.EndOperation(); + yield return _changes; + + for (var i = 0; i < _childrenNodes.Count; ++i) + { + var child = _childrenNodes[i]; + + if (child != null) + { + foreach (var changes in child.EndOperation()) + { + yield return changes; + } + } + } + } + } + public bool Select(int index, bool select) { return Select(index, select, raiseOnSelectionChanged: true); @@ -349,21 +394,13 @@ namespace Avalonia.Controls private void AddRange(IndexRange addRange, bool raiseOnSelectionChanged) { - // TODO: Check for duplicates (Task 14107720) - // TODO: Optimize by merging adjacent ranges (Task 14107720) - var oldCount = SelectedCount; + var selected = new List(); - for (int i = addRange.Begin; i <= addRange.End; i++) - { - if (!IsSelected(i)) - { - SelectedCount++; - } - } + SelectedCount += IndexRange.Add(_selected, addRange, selected); - if (oldCount != SelectedCount) + if (selected.Count > 0) { - _selected.Add(addRange); + _changes?.Selected(selected); if (raiseOnSelectionChanged) { @@ -374,71 +411,17 @@ namespace Avalonia.Controls private void RemoveRange(IndexRange removeRange, bool raiseOnSelectionChanged) { - int oldCount = SelectedCount; + var removed = new List(); - // TODO: Prevent overlap of Ranges in _selected (Task 14107720) - for (int i = removeRange.Begin; i <= removeRange.End; i++) - { - if (IsSelected(i)) - { - SelectedCount--; - } - } + SelectedCount -= IndexRange.Remove(_selected, removeRange, removed); - if (oldCount != SelectedCount) + if (removed.Count > 0) { - // Build up a both a list of Ranges to remove and ranges to add - var toRemove = new List(); - var toAdd = new List(); - - foreach (var range in _selected) - { - // If this range intersects the remove range, we have to do something - if (removeRange.Intersects(range)) - { - // Intersection with the beginning of the range - // Anything to the left of the point (exclusive) stays - // Anything to the right of the point (inclusive) gets clipped - if (range.Contains(removeRange.Begin - 1)) - { - range.Split(removeRange.Begin - 1, out var before, out _); - toAdd.Add(before); - } + _changes?.Deselected(removed); - // Intersection with the end of the range - // Anything to the left of the point (inclusive) gets clipped - // Anything to the right of the point (exclusive) stays - if (range.Contains(removeRange.End)) - { - if (range.Split(removeRange.End, out _, out var after)) - { - toAdd.Add(after); - } - } - - // Remove this Range from the collection - // New ranges will be added for any remaining subsections - toRemove.Add(range); - } - } - - bool change = ((toRemove.Count > 0) || (toAdd.Count > 0)); - - if (change) + if (raiseOnSelectionChanged) { - // Remove tagged ranges - foreach (var remove in toRemove) - { - _selected.Remove(remove); - } - - // Add new ranges - _selected.AddRange(toAdd); - - if (raiseOnSelectionChanged) - { - OnSelectionChanged(); - } + OnSelectionChanged(); } } } @@ -448,6 +431,7 @@ namespace Avalonia.Controls // Deselect all items if (_selected.Count > 0) { + _changes?.Deselected(_selected); _selected.Clear(); OnSelectionChanged(); } @@ -496,6 +480,7 @@ namespace Avalonia.Controls private void OnSourceListChanged(object dataSource, NotifyCollectionChangedEventArgs args) { bool selectionInvalidated = false; + IList? removed = null; switch (args.Action) { @@ -507,7 +492,7 @@ namespace Avalonia.Controls case NotifyCollectionChangedAction.Remove: { - selectionInvalidated = OnItemsRemoved(args.OldStartingIndex, args.OldItems.Count); + (selectionInvalidated, removed) = OnItemsRemoved(args.OldStartingIndex, args.OldItems); break; } @@ -520,7 +505,7 @@ namespace Avalonia.Controls case NotifyCollectionChangedAction.Replace: { - selectionInvalidated = OnItemsRemoved(args.OldStartingIndex, args.OldItems.Count); + (selectionInvalidated, removed) = OnItemsRemoved(args.OldStartingIndex, args.OldItems); selectionInvalidated |= OnItemsAdded(args.NewStartingIndex, args.NewItems.Count); break; } @@ -529,7 +514,7 @@ namespace Avalonia.Controls if (selectionInvalidated) { OnSelectionChanged(); - _manager.OnSelectionInvalidatedDueToCollectionChange(); + _manager.OnSelectionInvalidatedDueToCollectionChange(removed); } } @@ -609,21 +594,23 @@ namespace Avalonia.Controls return selectionInvalidated; } - private bool OnItemsRemoved(int index, int count) + private (bool, IList) OnItemsRemoved(int index, IList items) { - bool selectionInvalidated = false; + var selectionInvalidated = false; + var removed = new List(); + var count = items.Count; // Remove the items from the selection for leaf if (ItemsSourceView!.Count > 0) { bool isSelected = false; - for (int i = index; i <= index + count - 1; i++) + for (int i = 0; i <= count - 1; i++) { - if (IsSelected(i)) + if (IsSelected(index + i)) { isSelected = true; - break; + removed.Add(items[i]); } } @@ -654,6 +641,7 @@ namespace Avalonia.Controls { if (_childrenNodes[index] != null) { + removed.AddRange(_childrenNodes[index]!.SelectedItems); RealizedChildrenNodeCount--; } _childrenNodes.RemoveAt(index); @@ -696,7 +684,7 @@ namespace Avalonia.Controls } } - return selectionInvalidated; + return (selectionInvalidated, removed); } private void OnSelectionChanged() diff --git a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs index 208d85d8fd..1cca809c1d 100644 --- a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs @@ -930,6 +930,423 @@ namespace Avalonia.Controls.UnitTests }); } + [Fact] + public void Selecting_Item_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(4) }, e.SelectedIndices); + Assert.Equal(new object[] { 4 }, e.SelectedItems); + ++raised; + }; + + target.Select(4); + + Assert.Equal(1, raised); + } + + [Fact] + public void Selecting_Already_Selected_Item_Doesnt_Raise_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.Select(4); + target.SelectionChanged += (s, e) => ++raised; + target.Select(4); + + Assert.Equal(0, raised); + } + + [Fact] + public void SingleSelecting_Item_Raises_SelectionChanged() + { + var target = new SelectionModel { SingleSelect = true }; + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.Select(3); + + target.SelectionChanged += (s, e) => + { + Assert.Equal(new[] { new IndexPath(3) }, e.DeselectedIndices); + Assert.Equal(new object[] { 3 }, e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(4) }, e.SelectedIndices); + Assert.Equal(new object[] { 4 }, e.SelectedItems); + ++raised; + }; + + target.Select(4); + + Assert.Equal(1, raised); + } + + [Fact] + public void SingleSelecting_Already_Selected_Item_Doesnt_Raise_SelectionChanged() + { + var target = new SelectionModel { SingleSelect = true }; + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.Select(4); + target.SelectionChanged += (s, e) => ++raised; + target.Select(4); + + Assert.Equal(0, raised); + } + + [Fact] + public void Selecting_Item_With_Group_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = CreateNestedData(1, 2, 3); + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(1, 1) }, e.SelectedIndices); + Assert.Equal(new object[] { 4 }, e.SelectedItems); + ++raised; + }; + + target.Select(1, 1); + + Assert.Equal(1, raised); + } + + [Fact] + public void SelectAt_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = CreateNestedData(1, 2, 3); + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(1, 1) }, e.SelectedIndices); + Assert.Equal(new object[] { 4 }, e.SelectedItems); + ++raised; + }; + + target.SelectAt(new IndexPath(1, 1)); + + Assert.Equal(1, raised); + } + + [Fact] + public void SelectAll_Raises_SelectionChanged() + { + var target = new SelectionModel { SingleSelect = true }; + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.SelectionChanged += (s, e) => + { + var expected = Enumerable.Range(0, 10); + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(expected.Select(x => new IndexPath(x)), e.SelectedIndices); + Assert.Equal(expected, e.SelectedItems.Cast()); + ++raised; + }; + + target.SelectAll(); + + Assert.Equal(1, raised); + } + + [Fact] + public void SelectAll_With_Already_Selected_Items_Raises_SelectionChanged() + { + var target = new SelectionModel { SingleSelect = true }; + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.Select(4); + + target.SelectionChanged += (s, e) => + { + var expected = Enumerable.Range(0, 10).Except(new[] { 4 }); + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(expected.Select(x => new IndexPath(x)), e.SelectedIndices); + Assert.Equal(expected, e.SelectedItems.Cast()); + ++raised; + }; + + target.SelectAll(); + + Assert.Equal(1, raised); + } + + [Fact] + public void SelectRangeFromAnchor_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.SelectionChanged += (s, e) => + { + var expected = Enumerable.Range(4, 3); + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(expected.Select(x => new IndexPath(x)), e.SelectedIndices); + Assert.Equal(expected, e.SelectedItems.Cast()); + ++raised; + }; + + target.AnchorIndex = new IndexPath(4); + target.SelectRangeFromAnchor(6); + + Assert.Equal(1, raised); + } + + [Fact] + public void SelectRangeFromAnchor_With_Group_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = CreateNestedData(1, 2, 10); + target.SelectionChanged += (s, e) => + { + var expected = Enumerable.Range(11, 6); + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(expected.Select(x => new IndexPath(x / 10, x % 10)), e.SelectedIndices); + Assert.Equal(expected, e.SelectedItems.Cast()); + ++raised; + }; + + target.AnchorIndex = new IndexPath(1, 1); + target.SelectRangeFromAnchor(1, 6); + + Assert.Equal(1, raised); + } + + [Fact] + public void SelectRangeFromAnchorTo_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = CreateNestedData(1, 2, 10); + target.SelectionChanged += (s, e) => + { + var expected = Enumerable.Range(11, 6); + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(expected.Select(x => new IndexPath(x / 10, x % 10)), e.SelectedIndices); + Assert.Equal(expected, e.SelectedItems.Cast()); + ++raised; + }; + + target.AnchorIndex = new IndexPath(1, 1); + target.SelectRangeFromAnchorTo(new IndexPath(1, 6)); + + Assert.Equal(1, raised); + } + + [Fact] + public void ClearSelection_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.Select(4); + target.Select(5); + + target.SelectionChanged += (s, e) => + { + var expected = Enumerable.Range(4, 2); + Assert.Equal(expected.Select(x => new IndexPath(x)), e.DeselectedIndices); + Assert.Equal(expected, e.DeselectedItems.Cast()); + Assert.Empty(e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + target.ClearSelection(); + + Assert.Equal(1, raised); + } + + [Fact] + public void Changing_Source_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.Select(4); + target.Select(5); + + target.SelectionChanged += (s, e) => + { + var expected = Enumerable.Range(4, 2); + Assert.Equal(expected.Select(x => new IndexPath(x)), e.DeselectedIndices); + Assert.Equal(expected, e.DeselectedItems.Cast()); + Assert.Empty(e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + target.Source = Enumerable.Range(20, 10).ToList(); + + Assert.Equal(1, raised); + } + + [Fact] + public void Setting_SelectedIndex_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var raised = 0; + + target.Source = Enumerable.Range(0, 10).ToList(); + target.Select(4); + target.Select(5); + + target.SelectionChanged += (s, e) => + { + Assert.Equal(new[] { new IndexPath(4), new IndexPath(5) }, e.DeselectedIndices); + Assert.Equal(new object[] { 4, 5 }, e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(6) }, e.SelectedIndices); + Assert.Equal(new object[] { 6 }, e.SelectedItems); + ++raised; + }; + + target.SelectedIndex = new IndexPath(6); + + Assert.Equal(1, raised); + } + + [Fact] + public void Removing_Selected_Item_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var data = new ObservableCollection(Enumerable.Range(0, 10)); + var raised = 0; + + target.Source = data; + target.Select(4); + target.Select(5); + + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Equal(new object[] { 4 }, e.DeselectedItems); + Assert.Empty(e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + data.Remove(4); + + Assert.Equal(1, raised); + } + + [Fact] + public void Removing_Selected_Child_Item_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var data = CreateNestedData(1, 2, 3); + var raised = 0; + + target.Source = data; + target.SelectRange(new IndexPath(0), new IndexPath(1, 1)); + + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Equal(new object[] { 1}, e.DeselectedItems); + Assert.Empty(e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + ((AvaloniaList)data[0]).RemoveAt(1); + + Assert.Equal(1, raised); + } + + [Fact] + public void Removing_Selected_Item_With_Children_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var data = CreateNestedData(1, 2, 3); + var raised = 0; + + target.Source = data; + target.SelectRange(new IndexPath(0), new IndexPath(1, 1)); + + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Equal(new object[] { 0, 1, 2 }, e.DeselectedItems); + Assert.Empty(e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + data.RemoveAt(0); + + Assert.Equal(1, raised); + } + + [Fact] + public void Removing_Unselected_Item_Before_Selected_Item_Raises_SelectionChanged() + { + var target = new SelectionModel(); + var data = new ObservableCollection(Enumerable.Range(0, 10)); + var raised = 0; + + target.Source = data; + target.Select(8); + + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Empty(e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + data.Remove(6); + + Assert.Equal(1, raised); + } + + [Fact] + public void Removing_Unselected_Item_After_Selected_Item_Doesnt_Raise_SelectionChanged() + { + var target = new SelectionModel(); + var data = new ObservableCollection(Enumerable.Range(0, 10)); + var raised = 0; + + target.Source = data; + target.Select(4); + + target.SelectionChanged += (s, e) => ++raised; + + data.Remove(6); + + Assert.Equal(0, raised); + } + [Fact] public void Disposing_Unhooks_CollectionChanged_Handlers() { From e2132fedf93ef1bdf446f766853b130aaeb9d21c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 29 Jan 2020 21:48:14 +0100 Subject: [PATCH 05/10] Added some failing tests. That demonstrate some problems with the `SelectionModel` change notifications found so far. --- .../SelectionModelTests.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs index 1cca809c1d..de9fa4f11f 100644 --- a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs @@ -1392,6 +1392,38 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(3, target.SelectedItems.Count); } + [Fact] + public void Not_Enumerating_Changes_Does_Not_Prevent_Further_Operations() + { + var data = new[] { "foo", "bar", "baz" }; + var target = new SelectionModel { Source = data }; + + target.SelectionChanged += (s, e) => { }; + + target.SelectAll(); + target.ClearSelection(); + } + + [Fact] + public void Can_Change_Selection_From_SelectionChanged() + { + var data = new[] { "foo", "bar", "baz" }; + var target = new SelectionModel { Source = data }; + var raised = 0; + + target.SelectionChanged += (s, e) => + { + if (raised++ == 0) + { + target.ClearSelection(); + } + }; + + target.SelectAll(); + + Assert.Equal(2, raised); + } + private int GetSubscriberCount(AvaloniaList list) { return ((INotifyCollectionChangedDebug)list).GetCollectionChangedSubscribers()?.Length ?? 0; From 859aba1043855105e66ff9f89f6960513744e6a0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 29 Jan 2020 21:48:43 +0100 Subject: [PATCH 06/10] Refactor of SelectionModel change notifications. To address issues found. --- src/Avalonia.Controls/SelectionModel.cs | 53 ++----- .../SelectionModelChangeSet.cs | 140 +++++------------- ...SelectionModelSelectionChangedEventArgs.cs | 80 ++++++---- src/Avalonia.Controls/SelectionNode.cs | 60 +++++--- .../SelectionNodeOperation.cs | 80 ++++++++++ 5 files changed, 215 insertions(+), 198 deletions(-) create mode 100644 src/Avalonia.Controls/SelectionNodeOperation.cs diff --git a/src/Avalonia.Controls/SelectionModel.cs b/src/Avalonia.Controls/SelectionModel.cs index 5e2fb32243..49f2c8d2f6 100644 --- a/src/Avalonia.Controls/SelectionModel.cs +++ b/src/Avalonia.Controls/SelectionModel.cs @@ -490,13 +490,9 @@ namespace Avalonia.Controls } public void OnSelectionInvalidatedDueToCollectionChange( - IEnumerable? removedItems) + IReadOnlyList? removedItems) { - var e = new SelectionModelSelectionChangedEventArgs( - Enumerable.Empty(), - Enumerable.Empty(), - removedItems ?? Enumerable.Empty(), - Enumerable.Empty()); + var e = new SelectionModelSelectionChangedEventArgs(null, null, removedItems, null); OnSelectionChanged(e); } @@ -706,50 +702,19 @@ namespace Avalonia.Controls }); } - private void BeginOperation() - { - if (SelectionChanged != null) - { - _rootNode.BeginOperation(); - } - } + private void BeginOperation() => _rootNode.BeginOperation(); private void EndOperation() { - static IEnumerable? Concat(IEnumerable? a, IEnumerable b) - { - return a == null ? b : a.Concat(b); - } + var changes = new List(); + _rootNode.EndOperation(changes); SelectionModelSelectionChangedEventArgs? e = null; - - if (SelectionChanged != null) + + if (changes.Count > 0) { - IEnumerable? selectedIndices = null; - IEnumerable? deselectedIndices = null; - IEnumerable? selectedItems = null; - IEnumerable? deselectedItems = null; - - foreach (var changes in _rootNode.EndOperation()) - { - if (changes.HasChanges) - { - selectedIndices = Concat(selectedIndices, changes.SelectedIndices); - deselectedIndices = Concat(deselectedIndices, changes.DeselectedIndices); - selectedItems = Concat(selectedItems, changes.SelectedItems); - deselectedItems = Concat(deselectedItems, changes.DeselectedItems); - } - } - - if (selectedIndices != null || deselectedIndices != null || - selectedItems != null || deselectedItems != null) - { - e = new SelectionModelSelectionChangedEventArgs( - deselectedIndices ?? Enumerable.Empty(), - selectedIndices ?? Enumerable.Empty(), - deselectedItems ?? Enumerable.Empty(), - selectedItems ?? Enumerable.Empty()); - } + var changeSet = new SelectionModelChangeSet(changes); + e = changeSet.CreateEventArgs(); } OnSelectionChanged(e); diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs index 989136ac8d..b195117ae6 100644 --- a/src/Avalonia.Controls/SelectionModelChangeSet.cs +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -1,144 +1,80 @@ using System; using System.Collections.Generic; -using System.Linq; - -#nullable enable namespace Avalonia.Controls { internal class SelectionModelChangeSet { - private SelectionNode _owner; - private List? _selected; - private List? _deselected; - - public SelectionModelChangeSet(SelectionNode owner) => _owner = owner; - - public bool IsTracking { get; private set; } - public bool HasChanges => _selected?.Count > 0 || _deselected?.Count > 0; - public IEnumerable SelectedIndices => EnumerateIndices(_selected); - public IEnumerable DeselectedIndices => EnumerateIndices(_deselected); - public IEnumerable SelectedItems => EnumerateItems(_selected); - public IEnumerable DeselectedItems => EnumerateItems(_deselected); + private List _changes; - public void BeginOperation() + public SelectionModelChangeSet(List changes) { - if (IsTracking) - { - throw new AvaloniaInternalException("SelectionModel change operation already in progress."); - } - - IsTracking = true; - _selected?.Clear(); - _deselected?.Clear(); + _changes = changes; } - public void EndOperation() => IsTracking = false; - - public void Selected(IndexRange range) + public SelectionModelSelectionChangedEventArgs CreateEventArgs() { - if (!IsTracking) - { - return; - } - - Add(range, ref _selected, _deselected); + return new SelectionModelSelectionChangedEventArgs( + CreateIndices(x => x.DeselectedRanges), + CreateIndices(x => x.SelectedRanges), + CreateItems(x => x.DeselectedRanges), + CreateItems(x => x.SelectedRanges)); } - public void Selected(IEnumerable ranges) + private IReadOnlyList CreateIndices(Func?> selector) { - if (!IsTracking) + if (_changes == null) { - return; + return Array.Empty(); } - foreach (var range in ranges) - { - Selected(range); - } - } - - public void Deselected(IndexRange range) - { - if (!IsTracking) - { - return; - } - - Add(range, ref _deselected, _selected); - } + var result = new List(); - public void Deselected(IEnumerable ranges) - { - if (!IsTracking) + foreach (var i in _changes) { - return; - } + var ranges = selector(i); - foreach (var range in ranges) - { - Deselected(range); - } - } - - private static void Add( - IndexRange range, - ref List? add, - List? remove) - { - if (remove != null) - { - var removed = new List(); - IndexRange.Remove(remove, range, removed); - var selected = IndexRange.Subtract(range, removed); - - if (selected.Any()) + if (ranges != null) { - add ??= new List(); - - foreach (var r in selected) + foreach (var j in ranges) { - IndexRange.Add(add, r); + for (var k = j.Begin; k <= j.End; ++k) + { + result.Add(i.Path.CloneWithChildIndex(k)); + } } } } - else - { - add ??= new List(); - IndexRange.Add(add, range); - } + + return result; } - private IEnumerable EnumerateIndices(IEnumerable? ranges) + private IReadOnlyList CreateItems(Func?> selector) { - var path = _owner.IndexPath; - - if (ranges != null) + if (_changes == null) { - foreach (var range in ranges) - { - for (var i = range.Begin; i <= range.End; ++i) - { - yield return path.CloneWithChildIndex(i); - } - } + return Array.Empty(); } - } - private IEnumerable EnumerateItems(IEnumerable? ranges) - { - var items = _owner.ItemsSourceView; + var result = new List(); - if (ranges != null && items != null) + foreach (var i in _changes) { - foreach (var range in ranges) + var ranges = selector(i); + + if (ranges != null && i.Items != null) { - for (var i = range.Begin; i <= range.End; ++i) + foreach (var j in ranges) { - yield return items.GetAt(i); + for (var k = j.Begin; k <= j.End; ++k) + { + result.Add(i.Items.GetAt(k)); + } } } } + + return result; } } } diff --git a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs index 4976bf1827..ae98f6a1ce 100644 --- a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs +++ b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs @@ -12,49 +12,73 @@ namespace Avalonia.Controls { public class SelectionModelSelectionChangedEventArgs : EventArgs { - private readonly IEnumerable _selectedIndicesSource; - private readonly IEnumerable _deselectedIndicesSource; - private readonly IEnumerable _selectedItemsSource; - private readonly IEnumerable _deselectedItemsSource; - private List? _selectedIndices; - private List? _deselectedIndices; - private List? _selectedItems; - private List? _deselectedItems; + private readonly IEnumerable? _deselectedIndicesSource; + private readonly IEnumerable? _selectedIndicesSource; + private readonly IEnumerable? _deselectedItemsSource; + private readonly IEnumerable? _selectedItemsSource; + private IReadOnlyList? _deselectedIndices; + private IReadOnlyList? _selectedIndices; + private IReadOnlyList? _deselectedItems; + private IReadOnlyList? _selectedItems; public SelectionModelSelectionChangedEventArgs( - IEnumerable deselectedIndices, - IEnumerable selectedIndices, - IEnumerable deselectedItems, - IEnumerable selectedItems) + IReadOnlyList? deselectedIndices, + IReadOnlyList? selectedIndices, + IReadOnlyList? deselectedItems, + IReadOnlyList? selectedItems) { - _selectedIndicesSource = selectedIndices; - _deselectedIndicesSource = deselectedIndices; - _selectedItemsSource = selectedItems; - _deselectedItemsSource = deselectedItems; + _deselectedIndices = deselectedIndices ?? Array.Empty(); + _selectedIndices = selectedIndices ?? Array.Empty(); + _deselectedItems = deselectedItems ?? Array.Empty(); + _selectedItems= selectedItems ?? Array.Empty(); } - /// - /// Gets the indices of the items that were added to the selection. - /// - public IReadOnlyList SelectedIndices => - _selectedIndices ?? (_selectedIndices = new List(_selectedIndicesSource)); + public SelectionModelSelectionChangedEventArgs( + IEnumerable? deselectedIndices, + IEnumerable? selectedIndices, + IEnumerable? deselectedItems, + IEnumerable? selectedItems) + { + static void Set(IEnumerable? source, ref IEnumerable? sourceField, ref IReadOnlyList? field) + { + if (source != null) + { + sourceField = source; + } + else + { + field = Array.Empty(); + } + } + + Set(deselectedIndices, ref _deselectedIndicesSource, ref _deselectedIndices); + Set(selectedIndices, ref _selectedIndicesSource, ref _selectedIndices); + Set(deselectedItems, ref _deselectedItemsSource, ref _deselectedItems); + Set(selectedItems, ref _selectedItemsSource, ref _selectedItems); + } /// /// Gets the indices of the items that were removed from the selection. /// - public IReadOnlyList DeselectedIndices => - _deselectedIndices ?? (_deselectedIndices = new List(_deselectedIndicesSource)); + public IReadOnlyList DeselectedIndices + => _deselectedIndices ??= new List(_deselectedIndicesSource); /// - /// Gets the items that were added to the selection. + /// Gets the indices of the items that were added to the selection. /// - public IReadOnlyList SelectedItems => - _selectedItems ?? (_selectedItems = new List(_selectedItemsSource)); + public IReadOnlyList SelectedIndices + => _selectedIndices ??= new List(_selectedIndicesSource); /// /// Gets the items that were removed from the selection. /// - public IReadOnlyList DeselectedItems => - _deselectedItems ?? (_deselectedItems = new List(_deselectedItemsSource)); + public IReadOnlyList DeselectedItems + => _deselectedItems ??= new List(_deselectedItemsSource); + + /// + /// Gets the items that were added to the selection. + /// + public IReadOnlyList SelectedItems + => _selectedItems ??= new List(_selectedItemsSource); } } diff --git a/src/Avalonia.Controls/SelectionNode.cs b/src/Avalonia.Controls/SelectionNode.cs index d462a51228..85db801100 100644 --- a/src/Avalonia.Controls/SelectionNode.cs +++ b/src/Avalonia.Controls/SelectionNode.cs @@ -29,7 +29,7 @@ namespace Avalonia.Controls private readonly SelectionNode? _parent; private readonly List _selected = new List(); private readonly List _selectedIndicesCached = new List(); - private SelectionModelChangeSet? _changes; + private SelectionNodeOperation? _operation; private object? _source; private bool _selectedIndicesCacheIsValid; @@ -137,7 +137,7 @@ namespace Avalonia.Controls child = new SelectionNode(_manager, parent: this); child.Source = resolvedChild; - if (_changes?.IsTracking == true) + if (_operation != null) { child.BeginOperation(); } @@ -296,33 +296,45 @@ namespace Avalonia.Controls public void BeginOperation() { - _changes ??= new SelectionModelChangeSet(this); - _changes.BeginOperation(); + if (_operation != null) + { + throw new AvaloniaInternalException("Selection operation already in progress."); + } + + _operation = new SelectionNodeOperation(this); for (var i = 0; i < _childrenNodes.Count; ++i) { - _childrenNodes[i]?.BeginOperation(); + var child = _childrenNodes[i]; + + if (child != null && child != _manager.SharedLeafNode) + { + child.BeginOperation(); + } } } - public IEnumerable EndOperation() + public void EndOperation(List changes) { - if (_changes != null) + if (_operation == null) { - _changes.EndOperation(); - yield return _changes; + throw new AvaloniaInternalException("No selection operation in progress."); + } - for (var i = 0; i < _childrenNodes.Count; ++i) - { - var child = _childrenNodes[i]; + if (_operation.HasChanges) + { + changes.Add(_operation); + } - if (child != null) - { - foreach (var changes in child.EndOperation()) - { - yield return changes; - } - } + _operation = null; + + for (var i = 0; i < _childrenNodes.Count; ++i) + { + var child = _childrenNodes[i]; + + if (child != null && child != _manager.SharedLeafNode) + { + child.EndOperation(changes); } } } @@ -400,7 +412,7 @@ namespace Avalonia.Controls if (selected.Count > 0) { - _changes?.Selected(selected); + _operation?.Selected(selected); if (raiseOnSelectionChanged) { @@ -417,7 +429,7 @@ namespace Avalonia.Controls if (removed.Count > 0) { - _changes?.Deselected(removed); + _operation?.Deselected(removed); if (raiseOnSelectionChanged) { @@ -431,7 +443,7 @@ namespace Avalonia.Controls // Deselect all items if (_selected.Count > 0) { - _changes?.Deselected(_selected); + _operation?.Deselected(_selected); _selected.Clear(); OnSelectionChanged(); } @@ -480,7 +492,7 @@ namespace Avalonia.Controls private void OnSourceListChanged(object dataSource, NotifyCollectionChangedEventArgs args) { bool selectionInvalidated = false; - IList? removed = null; + List? removed = null; switch (args.Action) { @@ -594,7 +606,7 @@ namespace Avalonia.Controls return selectionInvalidated; } - private (bool, IList) OnItemsRemoved(int index, IList items) + private (bool, List) OnItemsRemoved(int index, IList items) { var selectionInvalidated = false; var removed = new List(); diff --git a/src/Avalonia.Controls/SelectionNodeOperation.cs b/src/Avalonia.Controls/SelectionNodeOperation.cs new file mode 100644 index 0000000000..04b8554f7c --- /dev/null +++ b/src/Avalonia.Controls/SelectionNodeOperation.cs @@ -0,0 +1,80 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +#nullable enable + +namespace Avalonia.Controls +{ + internal class SelectionNodeOperation + { + private readonly SelectionNode _owner; + private List? _selected; + private List? _deselected; + + public SelectionNodeOperation(SelectionNode owner) + { + _owner = owner; + } + + public bool HasChanges => _selected?.Count > 0 || _deselected?.Count > 0; + public List? SelectedRanges => _selected; + public List? DeselectedRanges => _deselected; + public IndexPath Path => _owner.IndexPath; + public ItemsSourceView? Items => _owner.ItemsSourceView; + + public void Selected(IndexRange range) + { + Add(range, ref _selected, _deselected); + } + + public void Selected(IEnumerable ranges) + { + foreach (var range in ranges) + { + Selected(range); + } + } + + public void Deselected(IndexRange range) + { + Add(range, ref _deselected, _selected); + } + + public void Deselected(IEnumerable ranges) + { + foreach (var range in ranges) + { + Deselected(range); + } + } + + private static void Add( + IndexRange range, + ref List? add, + List? remove) + { + if (remove != null) + { + var removed = new List(); + IndexRange.Remove(remove, range, removed); + var selected = IndexRange.Subtract(range, removed); + + if (selected.Any()) + { + add ??= new List(); + + foreach (var r in selected) + { + IndexRange.Add(add, r); + } + } + } + else + { + add ??= new List(); + IndexRange.Add(add, range); + } + } + } +} From b120e5282e2c70c4f9100704cd9c883c5388437f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 31 Jan 2020 10:17:56 +0100 Subject: [PATCH 07/10] Lazily create the selected/deselected lists. --- .../SelectionModelChangeSet.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs index b195117ae6..e927afb9b9 100644 --- a/src/Avalonia.Controls/SelectionModelChangeSet.cs +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -21,15 +21,13 @@ namespace Avalonia.Controls CreateItems(x => x.SelectedRanges)); } - private IReadOnlyList CreateIndices(Func?> selector) + private IEnumerable CreateIndices(Func?> selector) { if (_changes == null) { - return Array.Empty(); + yield break; } - var result = new List(); - foreach (var i in _changes) { var ranges = selector(i); @@ -40,20 +38,18 @@ namespace Avalonia.Controls { for (var k = j.Begin; k <= j.End; ++k) { - result.Add(i.Path.CloneWithChildIndex(k)); + yield return i.Path.CloneWithChildIndex(k); } } } } - - return result; } - private IReadOnlyList CreateItems(Func?> selector) + private IEnumerable CreateItems(Func?> selector) { if (_changes == null) { - return Array.Empty(); + yield break; } var result = new List(); @@ -68,13 +64,11 @@ namespace Avalonia.Controls { for (var k = j.Begin; k <= j.End; ++k) { - result.Add(i.Items.GetAt(k)); + yield return i.Items.GetAt(k); } } } } - - return result; } } } From 9073234f725cdbef28e713353fd52e430030bb92 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 1 Feb 2020 00:48:57 +0100 Subject: [PATCH 08/10] Use SelectedItems for change event args. --- src/Avalonia.Controls/IndexRange.cs | 12 ++ src/Avalonia.Controls/SelectedItems.cs | 37 ++--- src/Avalonia.Controls/SelectionModel.cs | 14 +- .../SelectionModelChangeSet.cs | 149 ++++++++++++++---- ...SelectionModelSelectionChangedEventArgs.cs | 53 +------ .../SelectionNodeOperation.cs | 32 +++- 6 files changed, 193 insertions(+), 104 deletions(-) diff --git a/src/Avalonia.Controls/IndexRange.cs b/src/Avalonia.Controls/IndexRange.cs index 124f1e0500..1dc161c699 100644 --- a/src/Avalonia.Controls/IndexRange.cs +++ b/src/Avalonia.Controls/IndexRange.cs @@ -202,6 +202,18 @@ namespace Avalonia.Controls } } + public static int GetCount(IEnumerable ranges) + { + var result = 0; + + foreach (var range in ranges) + { + result += (range.End - range.Begin) + 1; + } + + return result; + } + private static void MergeRanges(IList ranges) { for (var i = ranges.Count - 2; i >= 0; --i) diff --git a/src/Avalonia.Controls/SelectedItems.cs b/src/Avalonia.Controls/SelectedItems.cs index af43742670..a3acb48765 100644 --- a/src/Avalonia.Controls/SelectedItems.cs +++ b/src/Avalonia.Controls/SelectedItems.cs @@ -6,44 +6,37 @@ using System; using System.Collections; using System.Collections.Generic; -using SelectedItemInfo = Avalonia.Controls.SelectionModel.SelectedItemInfo; #nullable enable namespace Avalonia.Controls { - internal class SelectedItems : IReadOnlyList + public interface ISelectedItemInfo { - private readonly List _infos; - private readonly Func, int, T> _getAtImpl; + public IndexPath Path { get; } + } + + internal class SelectedItems : IReadOnlyList + where Tinfo : ISelectedItemInfo + { + private readonly List _infos; + private readonly Func, int, TValue> _getAtImpl; public SelectedItems( - List infos, - Func, int, T> getAtImpl) + List infos, + int count, + Func, int, TValue> getAtImpl) { _infos = infos; _getAtImpl = getAtImpl; - - foreach (var info in infos) - { - var node = info.Node; - - if (node != null) - { - Count += node.SelectedCount; - } - else - { - throw new InvalidOperationException("Selection changed after the SelectedIndices/Items property was read."); - } - } + Count = count; } - public T this[int index] => _getAtImpl(_infos, index); + public TValue this[int index] => _getAtImpl(_infos, index); public int Count { get; } - public IEnumerator GetEnumerator() + public IEnumerator GetEnumerator() { for (var i = 0; i < Count; ++i) { diff --git a/src/Avalonia.Controls/SelectionModel.cs b/src/Avalonia.Controls/SelectionModel.cs index 49f2c8d2f6..c8d2c5cc9e 100644 --- a/src/Avalonia.Controls/SelectionModel.cs +++ b/src/Avalonia.Controls/SelectionModel.cs @@ -165,6 +165,7 @@ namespace Avalonia.Controls if (_selectedItemsCached == null) { var selectedInfos = new List(); + var count = 0; if (_rootNode.Source != null) { @@ -176,6 +177,7 @@ namespace Avalonia.Controls if (currentInfo.Node.SelectedCount > 0) { selectedInfos.Add(new SelectedItemInfo(currentInfo.Node, currentInfo.Path)); + count += currentInfo.Node.SelectedCount; } }); } @@ -185,8 +187,9 @@ namespace Avalonia.Controls // the selected item at a particular index. This avoid having to create the storage and copying // needed in a dumb vector. This also allows us to expose a tree of selected nodes into an // easier to consume flat vector view of objects. - var selectedItems = new SelectedItems ( + var selectedItems = new SelectedItems ( selectedInfos, + count, (infos, index) => { var currentIndex = 0; @@ -233,6 +236,8 @@ namespace Avalonia.Controls if (_selectedIndicesCached == null) { var selectedInfos = new List(); + var count = 0; + SelectionTreeHelper.Traverse( _rootNode, false, @@ -241,6 +246,7 @@ namespace Avalonia.Controls if (currentInfo.Node.SelectedCount > 0) { selectedInfos.Add(new SelectedItemInfo(currentInfo.Node, currentInfo.Path)); + count += currentInfo.Node.SelectedCount; } }); @@ -249,8 +255,9 @@ namespace Avalonia.Controls // the IndexPath at a particular index. This avoid having to create the storage and copying // needed in a dumb vector. This also allows us to expose a tree of selected nodes into an // easier to consume flat vector view of IndexPaths. - var indices = new SelectedItems( + var indices = new SelectedItems( selectedInfos, + count, (infos, index) => // callback for GetAt(index) { var currentIndex = 0; @@ -720,7 +727,7 @@ namespace Avalonia.Controls OnSelectionChanged(e); } - internal class SelectedItemInfo + internal class SelectedItemInfo : ISelectedItemInfo { public SelectedItemInfo(SelectionNode node, IndexPath path) { @@ -730,6 +737,7 @@ namespace Avalonia.Controls public SelectionNode Node { get; } public IndexPath Path { get; } + public int Count => Node.SelectedCount; } private struct Operation : IDisposable diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs index e927afb9b9..c0228a1cbc 100644 --- a/src/Avalonia.Controls/SelectionModelChangeSet.cs +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -14,61 +14,144 @@ namespace Avalonia.Controls public SelectionModelSelectionChangedEventArgs CreateEventArgs() { + var deselectedCount = 0; + var selectedCount = 0; + + foreach (var change in _changes) + { + deselectedCount += change.DeselectedCount; + selectedCount += change.SelectedCount; + } + + var deselectedIndices = new SelectedItems( + _changes, + deselectedCount, + GetDeselectedIndexAt); + var selectedIndices = new SelectedItems( + _changes, + selectedCount, + GetSelectedIndexAt); + var deselectedItems = new SelectedItems( + _changes, + deselectedCount, + GetDeselectedItemAt); + var selectedItems = new SelectedItems( + _changes, + selectedCount, + GetSelectedItemAt); + return new SelectionModelSelectionChangedEventArgs( - CreateIndices(x => x.DeselectedRanges), - CreateIndices(x => x.SelectedRanges), - CreateItems(x => x.DeselectedRanges), - CreateItems(x => x.SelectedRanges)); + deselectedIndices, + selectedIndices, + deselectedItems, + selectedItems); } - private IEnumerable CreateIndices(Func?> selector) + private IndexPath GetDeselectedIndexAt( + List infos, + int index) { - if (_changes == null) - { - yield break; - } + static int GetCount(SelectionNodeOperation info) => info.DeselectedCount; + static List GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; + return GetIndexAt(infos, index, GetCount, GetRanges); + } - foreach (var i in _changes) + private IndexPath GetSelectedIndexAt( + List infos, + int index) + { + static int GetCount(SelectionNodeOperation info) => info.SelectedCount; + static List GetRanges(SelectionNodeOperation info) => info.SelectedRanges; + return GetIndexAt(infos, index, GetCount, GetRanges); + } + + private object GetDeselectedItemAt( + List infos, + int index) + { + static int GetCount(SelectionNodeOperation info) => info.DeselectedCount; + static List GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; + return GetItemAt(infos, index, GetCount, GetRanges); + } + + private object GetSelectedItemAt( + List infos, + int index) + { + static int GetCount(SelectionNodeOperation info) => info.SelectedCount; + static List GetRanges(SelectionNodeOperation info) => info.SelectedRanges; + return GetItemAt(infos, index, GetCount, GetRanges); + } + + private IndexPath GetIndexAt( + List infos, + int index, + Func getCount, + Func> getRanges) + { + var currentIndex = 0; + IndexPath path = default; + + foreach (var info in infos) { - var ranges = selector(i); + var currentCount = getCount(info); - if (ranges != null) + if (index >= currentIndex && index < currentIndex + currentCount) { - foreach (var j in ranges) - { - for (var k = j.Begin; k <= j.End; ++k) - { - yield return i.Path.CloneWithChildIndex(k); - } - } + int targetIndex = GetIndexAt(getRanges(info), index - currentIndex); + path = info.Path.CloneWithChildIndex(targetIndex); + break; } + + currentIndex += currentCount; } + + return path; } - private IEnumerable CreateItems(Func?> selector) + private object GetItemAt( + List infos, + int index, + Func getCount, + Func> getRanges) { - if (_changes == null) + var currentIndex = 0; + object item = null; + + foreach (var info in infos) { - yield break; + var currentCount = getCount(info); + + if (index >= currentIndex && index < currentIndex + currentCount) + { + int targetIndex = GetIndexAt(getRanges(info), index - currentIndex); + item = info.Items.GetAt(targetIndex); + break; + } + + currentIndex += currentCount; } - var result = new List(); + return item; + } - foreach (var i in _changes) + private int GetIndexAt(List ranges, int index) + { + var currentIndex = 0; + + foreach (var range in ranges) { - var ranges = selector(i); + var currentCount = (range.End - range.Begin) + 1; - if (ranges != null && i.Items != null) + if (index >= currentIndex && index < currentIndex + currentCount) { - foreach (var j in ranges) - { - for (var k = j.Begin; k <= j.End; ++k) - { - yield return i.Items.GetAt(k); - } - } + return range.Begin + (index - currentIndex); } + + currentIndex += currentCount; } + + throw new IndexOutOfRangeException(); } } } diff --git a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs index ae98f6a1ce..4e64ee6e6f 100644 --- a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs +++ b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs @@ -12,73 +12,36 @@ namespace Avalonia.Controls { public class SelectionModelSelectionChangedEventArgs : EventArgs { - private readonly IEnumerable? _deselectedIndicesSource; - private readonly IEnumerable? _selectedIndicesSource; - private readonly IEnumerable? _deselectedItemsSource; - private readonly IEnumerable? _selectedItemsSource; - private IReadOnlyList? _deselectedIndices; - private IReadOnlyList? _selectedIndices; - private IReadOnlyList? _deselectedItems; - private IReadOnlyList? _selectedItems; - public SelectionModelSelectionChangedEventArgs( IReadOnlyList? deselectedIndices, IReadOnlyList? selectedIndices, IReadOnlyList? deselectedItems, IReadOnlyList? selectedItems) { - _deselectedIndices = deselectedIndices ?? Array.Empty(); - _selectedIndices = selectedIndices ?? Array.Empty(); - _deselectedItems = deselectedItems ?? Array.Empty(); - _selectedItems= selectedItems ?? Array.Empty(); - } - - public SelectionModelSelectionChangedEventArgs( - IEnumerable? deselectedIndices, - IEnumerable? selectedIndices, - IEnumerable? deselectedItems, - IEnumerable? selectedItems) - { - static void Set(IEnumerable? source, ref IEnumerable? sourceField, ref IReadOnlyList? field) - { - if (source != null) - { - sourceField = source; - } - else - { - field = Array.Empty(); - } - } - - Set(deselectedIndices, ref _deselectedIndicesSource, ref _deselectedIndices); - Set(selectedIndices, ref _selectedIndicesSource, ref _selectedIndices); - Set(deselectedItems, ref _deselectedItemsSource, ref _deselectedItems); - Set(selectedItems, ref _selectedItemsSource, ref _selectedItems); + DeselectedIndices = deselectedIndices ?? Array.Empty(); + SelectedIndices = selectedIndices ?? Array.Empty(); + DeselectedItems = deselectedItems ?? Array.Empty(); + SelectedItems= selectedItems ?? Array.Empty(); } /// /// Gets the indices of the items that were removed from the selection. /// - public IReadOnlyList DeselectedIndices - => _deselectedIndices ??= new List(_deselectedIndicesSource); + public IReadOnlyList DeselectedIndices { get; } /// /// Gets the indices of the items that were added to the selection. /// - public IReadOnlyList SelectedIndices - => _selectedIndices ??= new List(_selectedIndicesSource); + public IReadOnlyList SelectedIndices { get; } /// /// Gets the items that were removed from the selection. /// - public IReadOnlyList DeselectedItems - => _deselectedItems ??= new List(_deselectedItemsSource); + public IReadOnlyList DeselectedItems { get; } /// /// Gets the items that were added to the selection. /// - public IReadOnlyList SelectedItems - => _selectedItems ??= new List(_selectedItemsSource); + public IReadOnlyList SelectedItems { get; } } } diff --git a/src/Avalonia.Controls/SelectionNodeOperation.cs b/src/Avalonia.Controls/SelectionNodeOperation.cs index 04b8554f7c..9622a52f00 100644 --- a/src/Avalonia.Controls/SelectionNodeOperation.cs +++ b/src/Avalonia.Controls/SelectionNodeOperation.cs @@ -6,11 +6,13 @@ using System.Linq; namespace Avalonia.Controls { - internal class SelectionNodeOperation + internal class SelectionNodeOperation : ISelectedItemInfo { private readonly SelectionNode _owner; private List? _selected; private List? _deselected; + private int _selectedCount = -1; + private int _deselectedCount = -1; public SelectionNodeOperation(SelectionNode owner) { @@ -23,9 +25,36 @@ namespace Avalonia.Controls public IndexPath Path => _owner.IndexPath; public ItemsSourceView? Items => _owner.ItemsSourceView; + public int SelectedCount + { + get + { + if (_selectedCount == -1) + { + _selectedCount = (_selected != null) ? IndexRange.GetCount(_selected) : 0; + } + + return _selectedCount; + } + } + + public int DeselectedCount + { + get + { + if (_deselectedCount == -1) + { + _deselectedCount = (_deselected != null) ? IndexRange.GetCount(_deselected) : 0; + } + + return _deselectedCount; + } + } + public void Selected(IndexRange range) { Add(range, ref _selected, _deselected); + _selectedCount = -1; } public void Selected(IEnumerable ranges) @@ -39,6 +68,7 @@ namespace Avalonia.Controls public void Deselected(IndexRange range) { Add(range, ref _deselected, _selected); + _deselectedCount = -1; } public void Deselected(IEnumerable ranges) From 01a194520122b0f8bf688f21925f31f953ff3b8a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 3 Feb 2020 14:11:39 +0100 Subject: [PATCH 09/10] Add nullable annotations. --- .../SelectionModelChangeSet.cs | 47 ++++++++++--------- ...SelectionModelSelectionChangedEventArgs.cs | 12 ++--- 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs index c0228a1cbc..57bf369585 100644 --- a/src/Avalonia.Controls/SelectionModelChangeSet.cs +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +#nullable enable + namespace Avalonia.Controls { internal class SelectionModelChangeSet @@ -31,11 +33,11 @@ namespace Avalonia.Controls _changes, selectedCount, GetSelectedIndexAt); - var deselectedItems = new SelectedItems( + var deselectedItems = new SelectedItems( _changes, deselectedCount, GetDeselectedItemAt); - var selectedItems = new SelectedItems( + var selectedItems = new SelectedItems( _changes, selectedCount, GetSelectedItemAt); @@ -52,7 +54,7 @@ namespace Avalonia.Controls int index) { static int GetCount(SelectionNodeOperation info) => info.DeselectedCount; - static List GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; + static List? GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; return GetIndexAt(infos, index, GetCount, GetRanges); } @@ -61,25 +63,25 @@ namespace Avalonia.Controls int index) { static int GetCount(SelectionNodeOperation info) => info.SelectedCount; - static List GetRanges(SelectionNodeOperation info) => info.SelectedRanges; + static List? GetRanges(SelectionNodeOperation info) => info.SelectedRanges; return GetIndexAt(infos, index, GetCount, GetRanges); } - private object GetDeselectedItemAt( + private object? GetDeselectedItemAt( List infos, int index) { static int GetCount(SelectionNodeOperation info) => info.DeselectedCount; - static List GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; + static List? GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; return GetItemAt(infos, index, GetCount, GetRanges); } - private object GetSelectedItemAt( + private object? GetSelectedItemAt( List infos, int index) { static int GetCount(SelectionNodeOperation info) => info.SelectedCount; - static List GetRanges(SelectionNodeOperation info) => info.SelectedRanges; + static List? GetRanges(SelectionNodeOperation info) => info.SelectedRanges; return GetItemAt(infos, index, GetCount, GetRanges); } @@ -87,7 +89,7 @@ namespace Avalonia.Controls List infos, int index, Func getCount, - Func> getRanges) + Func?> getRanges) { var currentIndex = 0; IndexPath path = default; @@ -109,14 +111,14 @@ namespace Avalonia.Controls return path; } - private object GetItemAt( + private object? GetItemAt( List infos, int index, Func getCount, - Func> getRanges) + Func?> getRanges) { var currentIndex = 0; - object item = null; + object? item = null; foreach (var info in infos) { @@ -125,7 +127,7 @@ namespace Avalonia.Controls if (index >= currentIndex && index < currentIndex + currentCount) { int targetIndex = GetIndexAt(getRanges(info), index - currentIndex); - item = info.Items.GetAt(targetIndex); + item = info.Items?.GetAt(targetIndex); break; } @@ -135,20 +137,23 @@ namespace Avalonia.Controls return item; } - private int GetIndexAt(List ranges, int index) + private int GetIndexAt(List? ranges, int index) { var currentIndex = 0; - foreach (var range in ranges) + if (ranges != null) { - var currentCount = (range.End - range.Begin) + 1; - - if (index >= currentIndex && index < currentIndex + currentCount) + foreach (var range in ranges) { - return range.Begin + (index - currentIndex); - } + var currentCount = (range.End - range.Begin) + 1; - currentIndex += currentCount; + if (index >= currentIndex && index < currentIndex + currentCount) + { + return range.Begin + (index - currentIndex); + } + + currentIndex += currentCount; + } } throw new IndexOutOfRangeException(); diff --git a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs index 4e64ee6e6f..5e2efdf331 100644 --- a/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs +++ b/src/Avalonia.Controls/SelectionModelSelectionChangedEventArgs.cs @@ -15,13 +15,13 @@ namespace Avalonia.Controls public SelectionModelSelectionChangedEventArgs( IReadOnlyList? deselectedIndices, IReadOnlyList? selectedIndices, - IReadOnlyList? deselectedItems, - IReadOnlyList? selectedItems) + IReadOnlyList? deselectedItems, + IReadOnlyList? selectedItems) { DeselectedIndices = deselectedIndices ?? Array.Empty(); SelectedIndices = selectedIndices ?? Array.Empty(); - DeselectedItems = deselectedItems ?? Array.Empty(); - SelectedItems= selectedItems ?? Array.Empty(); + DeselectedItems = deselectedItems ?? Array.Empty(); + SelectedItems= selectedItems ?? Array.Empty(); } /// @@ -37,11 +37,11 @@ namespace Avalonia.Controls /// /// Gets the items that were removed from the selection. /// - public IReadOnlyList DeselectedItems { get; } + public IReadOnlyList DeselectedItems { get; } /// /// Gets the items that were added to the selection. /// - public IReadOnlyList SelectedItems { get; } + public IReadOnlyList SelectedItems { get; } } } From 04f8516c32e4e2c70b8a0f4af5f0543d58bb56a6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 3 Feb 2020 14:36:24 +0100 Subject: [PATCH 10/10] Handle null SelectionModel.Source. --- src/Avalonia.Controls/SelectionModel.cs | 20 ++++++++- .../SelectionModelChangeSet.cs | 28 ++++++++----- src/Avalonia.Controls/SelectionNode.cs | 7 +++- .../SelectionModelTests.cs | 42 +++++++++++++++++++ 4 files changed, 83 insertions(+), 14 deletions(-) diff --git a/src/Avalonia.Controls/SelectionModel.cs b/src/Avalonia.Controls/SelectionModel.cs index c8d2c5cc9e..796d108f60 100644 --- a/src/Avalonia.Controls/SelectionModel.cs +++ b/src/Avalonia.Controls/SelectionModel.cs @@ -36,13 +36,29 @@ namespace Avalonia.Controls get => _rootNode?.Source; set { - using (var operation = new Operation(this)) + var wasNull = _rootNode.Source == null; + + if (_rootNode.Source != null) { - ClearSelection(resetAnchor: true); + using (var operation = new Operation(this)) + { + ClearSelection(resetAnchor: true); + } } _rootNode.Source = value; + RaisePropertyChanged("Source"); + + if (wasNull) + { + var e = new SelectionModelSelectionChangedEventArgs( + null, + SelectedIndices, + null, + SelectedItems); + OnSelectionChanged(e); + } } } diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs index 57bf369585..bff84eca92 100644 --- a/src/Avalonia.Controls/SelectionModelChangeSet.cs +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -16,30 +16,38 @@ namespace Avalonia.Controls public SelectionModelSelectionChangedEventArgs CreateEventArgs() { - var deselectedCount = 0; - var selectedCount = 0; + var deselectedIndexCount = 0; + var selectedIndexCount = 0; + var deselectedItemCount = 0; + var selectedItemCount = 0; foreach (var change in _changes) { - deselectedCount += change.DeselectedCount; - selectedCount += change.SelectedCount; + deselectedIndexCount += change.DeselectedCount; + selectedIndexCount += change.SelectedCount; + + if (change.Items != null) + { + deselectedItemCount += change.DeselectedCount; + selectedItemCount += change.SelectedCount; + } } var deselectedIndices = new SelectedItems( _changes, - deselectedCount, + deselectedIndexCount, GetDeselectedIndexAt); var selectedIndices = new SelectedItems( _changes, - selectedCount, + selectedIndexCount, GetSelectedIndexAt); var deselectedItems = new SelectedItems( _changes, - deselectedCount, + deselectedItemCount, GetDeselectedItemAt); var selectedItems = new SelectedItems( _changes, - selectedCount, + selectedItemCount, GetSelectedItemAt); return new SelectionModelSelectionChangedEventArgs( @@ -71,7 +79,7 @@ namespace Avalonia.Controls List infos, int index) { - static int GetCount(SelectionNodeOperation info) => info.DeselectedCount; + static int GetCount(SelectionNodeOperation info) => info.Items != null ? info.DeselectedCount : 0; static List? GetRanges(SelectionNodeOperation info) => info.DeselectedRanges; return GetItemAt(infos, index, GetCount, GetRanges); } @@ -80,7 +88,7 @@ namespace Avalonia.Controls List infos, int index) { - static int GetCount(SelectionNodeOperation info) => info.SelectedCount; + static int GetCount(SelectionNodeOperation info) => info.Items != null ? info.SelectedCount : 0; static List? GetRanges(SelectionNodeOperation info) => info.SelectedRanges; return GetItemAt(infos, index, GetCount, GetRanges); } diff --git a/src/Avalonia.Controls/SelectionNode.cs b/src/Avalonia.Controls/SelectionNode.cs index 85db801100..04144e1ed0 100644 --- a/src/Avalonia.Controls/SelectionNode.cs +++ b/src/Avalonia.Controls/SelectionNode.cs @@ -48,8 +48,11 @@ namespace Avalonia.Controls { if (_source != value) { - ClearSelection(); - UnhookCollectionChangedHandler(); + if (_source != null) + { + ClearSelection(); + UnhookCollectionChangedHandler(); + } _source = value; diff --git a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs index de9fa4f11f..1950da2818 100644 --- a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs @@ -1424,6 +1424,48 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(2, raised); } + [Fact] + public void Raises_SelectionChanged_With_No_Source() + { + var target = new SelectionModel(); + var raised = 0; + + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(1) }, e.SelectedIndices); + Assert.Empty(e.SelectedItems); + ++raised; + }; + + target.Select(1); + + Assert.Equal(new[] { new IndexPath(1) }, target.SelectedIndices); + Assert.Empty(target.SelectedItems); + } + + [Fact] + public void Raises_SelectionChanged_With_Items_After_Source_Is_Set() + { + var target = new SelectionModel(); + var raised = 0; + + target.Select(1); + target.SelectionChanged += (s, e) => + { + Assert.Empty(e.DeselectedIndices); + Assert.Empty(e.DeselectedItems); + Assert.Equal(new[] { new IndexPath(1) }, e.SelectedIndices); + Assert.Equal(new[] { "bar" }, e.SelectedItems); + ++raised; + }; + + target.Source = new[] { "foo", "bar", "baz" }; + + Assert.Equal(1, raised); + } + private int GetSubscriberCount(AvaloniaList list) { return ((INotifyCollectionChangedDebug)list).GetCollectionChangedSubscribers()?.Length ?? 0;