From cd37520b7141c8d0140be887e8ee0fcc85471c6a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 25 May 2020 16:43:22 +0200 Subject: [PATCH] Trim invalid selection when Source first assigned. `SelectionModel` can have a selection set before its `Source` is initialized. In this case, be sure to trim invalid selections from the model before continuing. Fixes #3919 --- src/Avalonia.Controls/IndexRange.cs | 47 +++++++++++ src/Avalonia.Controls/SelectionModel.cs | 7 +- .../SelectionModelChangeSet.cs | 2 +- src/Avalonia.Controls/SelectionNode.cs | 21 +++++ .../IndexRangeTests.cs | 82 +++++++++++++++++++ .../SelectionModelTests.cs | 44 +++++++--- 6 files changed, 187 insertions(+), 16 deletions(-) diff --git a/src/Avalonia.Controls/IndexRange.cs b/src/Avalonia.Controls/IndexRange.cs index 1dc161c699..e45d013af4 100644 --- a/src/Avalonia.Controls/IndexRange.cs +++ b/src/Avalonia.Controls/IndexRange.cs @@ -132,6 +132,53 @@ namespace Avalonia.Controls return result; } + public static int Intersect( + IList ranges, + IndexRange range, + IList? removed = null) + { + var result = 0; + + for (var i = 0; i < ranges.Count && range != s_invalid; ++i) + { + var existing = ranges[i]; + + if (existing.End < range.Begin || existing.Begin > range.End) + { + removed?.Add(existing); + ranges.RemoveAt(i--); + result += existing.Count; + } + else + { + if (existing.Begin < range.Begin) + { + var except = new IndexRange(existing.Begin, range.Begin - 1); + removed?.Add(except); + ranges[i] = existing = new IndexRange(range.Begin, existing.End); + result += except.Count; + } + + if (existing.End > range.End) + { + var except = new IndexRange(range.End + 1, existing.End); + removed?.Add(except); + ranges[i] = new IndexRange(existing.Begin, range.End); + result += except.Count; + } + } + } + + MergeRanges(ranges); + + if (removed is object) + { + MergeRanges(removed); + } + + return result; + } + public static int Remove( IList ranges, IndexRange range, diff --git a/src/Avalonia.Controls/SelectionModel.cs b/src/Avalonia.Controls/SelectionModel.cs index dd4934f9e5..7a64421326 100644 --- a/src/Avalonia.Controls/SelectionModel.cs +++ b/src/Avalonia.Controls/SelectionModel.cs @@ -45,12 +45,9 @@ namespace Avalonia.Controls if (_rootNode.Source != null) { - if (_rootNode.Source != null) + using (var operation = new Operation(this)) { - using (var operation = new Operation(this)) - { - ClearSelection(resetAnchor: true); - } + ClearSelection(resetAnchor: true); } } diff --git a/src/Avalonia.Controls/SelectionModelChangeSet.cs b/src/Avalonia.Controls/SelectionModelChangeSet.cs index 6e77dc5755..d1df38656a 100644 --- a/src/Avalonia.Controls/SelectionModelChangeSet.cs +++ b/src/Avalonia.Controls/SelectionModelChangeSet.cs @@ -135,7 +135,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?.Count > targetIndex ? info.Items?.GetAt(targetIndex) : null; break; } diff --git a/src/Avalonia.Controls/SelectionNode.cs b/src/Avalonia.Controls/SelectionNode.cs index 0b00db88c3..1a3bde1765 100644 --- a/src/Avalonia.Controls/SelectionNode.cs +++ b/src/Avalonia.Controls/SelectionNode.cs @@ -101,6 +101,7 @@ namespace Avalonia.Controls ItemsSourceView = newDataSource; + TrimInvalidSelections(); PopulateSelectedItemsFromSelectedIndices(); HookupCollectionChangedHandler(); OnSelectionChanged(); @@ -108,6 +109,26 @@ namespace Avalonia.Controls } } + private void TrimInvalidSelections() + { + if (_selected == null || ItemsSourceView == null) + { + return; + } + + var validRange = ItemsSourceView.Count > 0 ? new IndexRange(0, ItemsSourceView.Count - 1) : new IndexRange(-1, -1); + var removed = new List(); + var removedCount = IndexRange.Intersect(_selected, validRange, removed); + + if (removedCount > 0) + { + using var operation = _manager.Update(); + SelectedCount -= removedCount; + OnSelectionChanged(); + _operation!.Deselected(removed); + } + } + public ItemsSourceView? ItemsSourceView { get; private set; } public int DataCount => ItemsSourceView?.Count ?? 0; public int ChildrenNodeCount => _childrenNodes.Count; diff --git a/tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs b/tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs index e0f46d9fa9..e01c752658 100644 --- a/tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs +++ b/tests/Avalonia.Controls.UnitTests/IndexRangeTests.cs @@ -127,6 +127,88 @@ namespace Avalonia.Controls.UnitTests Assert.Empty(selected); } + [Fact] + public void Intersect_Should_Remove_Items_From_Beginning() + { + var ranges = new List { new IndexRange(0, 10) }; + var removed = new List(); + var result = IndexRange.Intersect(ranges, new IndexRange(2, 12), removed); + + Assert.Equal(2, result); + Assert.Equal(new[] { new IndexRange(2, 10) }, ranges); + Assert.Equal(new[] { new IndexRange(0, 1) }, removed); + } + + [Fact] + public void Intersect_Should_Remove_Items_From_End() + { + var ranges = new List { new IndexRange(0, 10) }; + var removed = new List(); + var result = IndexRange.Intersect(ranges, new IndexRange(0, 8), removed); + + Assert.Equal(2, result); + Assert.Equal(new[] { new IndexRange(0, 8) }, ranges); + Assert.Equal(new[] { new IndexRange(9, 10) }, removed); + } + + [Fact] + public void Intersect_Should_Remove_Entire_Range_Start() + { + var ranges = new List { new IndexRange(0, 5), new IndexRange(6, 10) }; + var removed = new List(); + var result = IndexRange.Intersect(ranges, new IndexRange(6, 10), removed); + + Assert.Equal(6, result); + Assert.Equal(new[] { new IndexRange(6, 10) }, ranges); + Assert.Equal(new[] { new IndexRange(0, 5) }, removed); + } + + [Fact] + public void Intersect_Should_Remove_Entire_Range_End() + { + var ranges = new List { new IndexRange(0, 5), new IndexRange(6, 10) }; + var removed = new List(); + var result = IndexRange.Intersect(ranges, new IndexRange(0, 4), removed); + + Assert.Equal(6, result); + Assert.Equal(new[] { new IndexRange(0, 4) }, ranges); + Assert.Equal(new[] { new IndexRange(5, 10) }, removed); + } + + [Fact] + public void Intersect_Should_Remove_Entire_Range_Start_End() + { + var ranges = new List + { + new IndexRange(0, 2), + new IndexRange(3, 7), + new IndexRange(8, 10) + }; + var removed = new List(); + var result = IndexRange.Intersect(ranges, new IndexRange(3, 7), removed); + + Assert.Equal(6, result); + Assert.Equal(new[] { new IndexRange(3, 7) }, ranges); + Assert.Equal(new[] { new IndexRange(0, 2), new IndexRange(8, 10) }, removed); + } + + [Fact] + public void Intersect_Should_Remove_Entire_And_Partial_Range_Start_End() + { + var ranges = new List + { + new IndexRange(0, 2), + new IndexRange(3, 7), + new IndexRange(8, 10) + }; + var removed = new List(); + var result = IndexRange.Intersect(ranges, new IndexRange(4, 6), removed); + + Assert.Equal(8, result); + Assert.Equal(new[] { new IndexRange(4, 6) }, ranges); + Assert.Equal(new[] { new IndexRange(0, 3), new IndexRange(7, 10) }, removed); + } + [Fact] public void Remove_Should_Remove_Entire_Range() { diff --git a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs index 211fc44caf..1dc5fead26 100644 --- a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs @@ -1775,7 +1775,17 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void Assigning_Source_With_Less_Items_Than_Selection_Trims_Selection() + public void Initializing_Source_With_Less_Items_Than_Selection_Trims_Selection() + { + var data = new[] { "foo", "bar", "baz" }; + var target = new SelectionModel(); + target.SelectedIndex = new IndexPath(4); + target.Source = data; + Assert.Empty(target.SelectedIndices); + } + + [Fact] + public void Initializing_Source_With_Less_Items_Than_Selection_Trims_Selection_RetainSelection() { var data = new[] { "foo", "bar", "baz" }; var target = new SelectionModel { RetainSelectionOnReset = true }; @@ -1785,7 +1795,7 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void Assigning_Source_With_Less_Items_Than_Multiple_Selection_Trims_Selection() + public void Initializing_Source_With_Less_Items_Than_Multiple_Selection_Trims_Selection() { var data = new[] { "foo", "bar", "baz" }; var target = new SelectionModel { RetainSelectionOnReset = true }; @@ -1796,17 +1806,31 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(new IndexPath(2), target.SelectedIndices.First()); } - // test that going from non-null source to non-null source clears selection [Fact] - public void Changing_Source_With_Valid_IndexPath_Retains_Selection() + public void Initializing_Source_With_Less_Items_Than_Selection_Raises_SelectionChanged() { - var data = new[] { "foo", "bar", "baz", "boo", "hoo" }; - var smallerData = new[] { "foo", "bar", "baz" }; - var target = new SelectionModel { RetainSelectionOnReset = true }; + var data = new[] { "foo", "bar", "baz" }; + var target = new SelectionModel(); + var raised = 0; + + target.SelectedIndex = new IndexPath(4); + + target.SelectionChanged += (s, e) => + { + if (raised == 0) + { + Assert.Equal(new[] { Path(4) }, e.DeselectedIndices); + Assert.Equal(new object[] { null }, e.DeselectedItems); + Assert.Empty(e.SelectedIndices); + Assert.Empty(e.SelectedItems); + } + + ++raised; + }; + target.Source = data; - target.SelectedIndex = new IndexPath(2); - target.Source = smallerData; - Assert.Equal(1, target.SelectedIndices.Count); + + Assert.Equal(2, raised); } private int GetSubscriberCount(AvaloniaList list)