From 8c3458e9306cbfb17305b3bc3ae6362434a8a0d8 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 17 Sep 2020 12:00:36 +0200 Subject: [PATCH 1/4] Remove sandbox from ncrunch. --- .ncrunch/Sandbox.v3.ncrunchproject | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .ncrunch/Sandbox.v3.ncrunchproject diff --git a/.ncrunch/Sandbox.v3.ncrunchproject b/.ncrunch/Sandbox.v3.ncrunchproject new file mode 100644 index 0000000000..319cd523ce --- /dev/null +++ b/.ncrunch/Sandbox.v3.ncrunchproject @@ -0,0 +1,5 @@ + + + True + + \ No newline at end of file From 73ffcf4648ee3a6afbe123c752e2913ef76d304d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 17 Sep 2020 12:21:22 +0200 Subject: [PATCH 2/4] Added failing tests simulating tabstrip/carousel pair. --- .../Primitives/SelectingItemsControlTests.cs | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index 192d6e0286..514d3b5475 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -1813,6 +1813,88 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(1, raised); } + [Fact] + public void Handles_Removing_Last_Item_In_Two_Controls_With_Bound_SelectedIndex() + { + var items = new ObservableCollection { "foo" }; + + // Simulates problem with TabStrip and Carousel with bound SelectedIndex. + var tabStrip = new TestSelector + { + Items = items, + SelectionMode = SelectionMode.AlwaysSelected, + }; + + var carousel = new TestSelector + { + Items = items, + [!Carousel.SelectedIndexProperty] = tabStrip[!TabStrip.SelectedIndexProperty], + }; + + var tabStripRaised = 0; + var carouselRaised = 0; + + tabStrip.SelectionChanged += (s, e) => + { + Assert.Equal(new[] { "foo" }, e.RemovedItems); + Assert.Empty(e.AddedItems); + ++tabStripRaised; + }; + + carousel.SelectionChanged += (s, e) => + { + Assert.Equal(new[] { "foo" }, e.RemovedItems); + Assert.Empty(e.AddedItems); + ++carouselRaised; + }; + + items.RemoveAt(0); + + Assert.Equal(1, tabStripRaised); + Assert.Equal(1, carouselRaised); + } + + [Fact] + public void Handles_Removing_Last_Item_In_Controls_With_Bound_SelectedItem() + { + var items = new ObservableCollection { "foo" }; + + // Simulates problem with TabStrip and Carousel with bound SelectedItem. + var tabStrip = new TestSelector + { + Items = items, + SelectionMode = SelectionMode.AlwaysSelected, + }; + + var carousel = new TestSelector + { + Items = items, + [!Carousel.SelectedItemProperty] = tabStrip[!TabStrip.SelectedItemProperty], + }; + + var tabStripRaised = 0; + var carouselRaised = 0; + + tabStrip.SelectionChanged += (s, e) => + { + Assert.Equal(new[] { "foo" }, e.RemovedItems); + Assert.Empty(e.AddedItems); + ++tabStripRaised; + }; + + carousel.SelectionChanged += (s, e) => + { + Assert.Equal(new[] { "foo" }, e.RemovedItems); + Assert.Empty(e.AddedItems); + ++carouselRaised; + }; + + items.RemoveAt(0); + + Assert.Equal(1, tabStripRaised); + Assert.Equal(1, carouselRaised); + } + private static void Prepare(SelectingItemsControl target) { var root = new TestRoot From b5f81e52909e2edd1ce12e346af031c460cd8490 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 17 Sep 2020 12:25:34 +0200 Subject: [PATCH 3/4] Don't coerce when deselecting a range. If the deselection happens because of a `Clear` during a source collection changed event handler, then the selected indexes may not be in the valid range of the post-change items. In this case, we still want the items to be cleared, and not coercing the range has no downside. --- src/Avalonia.Controls/Selection/SelectionModel.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Avalonia.Controls/Selection/SelectionModel.cs b/src/Avalonia.Controls/Selection/SelectionModel.cs index fd27cb340a..2492129780 100644 --- a/src/Avalonia.Controls/Selection/SelectionModel.cs +++ b/src/Avalonia.Controls/Selection/SelectionModel.cs @@ -242,12 +242,7 @@ namespace Avalonia.Controls.Selection { using var update = BatchUpdate(); var o = update.Operation; - var range = CoerceRange(start, end); - - if (range.Begin == -1) - { - return; - } + var range = new IndexRange(start, end); if (RangesEnabled) { From f79f910c4df91ba54c483eaf02fdcaef81841a00 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 17 Sep 2020 12:35:36 +0200 Subject: [PATCH 4/4] Coerce lower bound for deselecting items. We can't deselect -1. --- src/Avalonia.Controls/Selection/SelectionModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Selection/SelectionModel.cs b/src/Avalonia.Controls/Selection/SelectionModel.cs index 2492129780..3b5d57a7b8 100644 --- a/src/Avalonia.Controls/Selection/SelectionModel.cs +++ b/src/Avalonia.Controls/Selection/SelectionModel.cs @@ -242,7 +242,7 @@ namespace Avalonia.Controls.Selection { using var update = BatchUpdate(); var o = update.Operation; - var range = new IndexRange(start, end); + var range = new IndexRange(Math.Max(0, start), end); if (RangesEnabled) {