From 5b252620b8d87343c20471ea19902c013945f9bf Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 15:36:32 +0200 Subject: [PATCH 01/46] Update ncrunch config. --- Avalonia.v3.ncrunchsolution | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Avalonia.v3.ncrunchsolution b/Avalonia.v3.ncrunchsolution index bef7e45524..afce1018ec 100644 --- a/Avalonia.v3.ncrunchsolution +++ b/Avalonia.v3.ncrunchsolution @@ -6,6 +6,9 @@ src\Avalonia.Build.Tasks\bin\Debug\netstandard2.0\Mono.Cecil.dll True + + RunApiCompat = false + .ncrunch True From a0dfa32ba5bb4d12d24f262f74d866099adf2f70 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 15:49:31 +0200 Subject: [PATCH 02/46] Added failing test for #4654. --- .../TabControlTests.cs | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs index fd52aeb9af..b4378938de 100644 --- a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs @@ -122,11 +122,56 @@ namespace Avalonia.Controls.UnitTests Items = collection, }; - target.ApplyTemplate(); + Prepare(target); target.SelectedItem = collection[1]; + + Assert.Same(collection[1], target.SelectedItem); + Assert.Equal(collection[1].Content, target.SelectedContent); + collection.RemoveAt(1); Assert.Same(collection[0], target.SelectedItem); + Assert.Equal(collection[0].Content, target.SelectedContent); + } + + [Fact] + public void Removal_Should_Set_New_Item0_When_Item0_Selected() + { + var collection = new ObservableCollection() + { + new TabItem + { + Name = "first", + Content = "foo", + }, + new TabItem + { + Name = "second", + Content = "bar", + }, + new TabItem + { + Name = "3rd", + Content = "barf", + }, + }; + + var target = new TabControl + { + Template = TabControlTemplate(), + Items = collection, + }; + + Prepare(target); + target.SelectedItem = collection[0]; + + Assert.Same(collection[0], target.SelectedItem); + Assert.Equal(collection[0].Content, target.SelectedContent); + + collection.RemoveAt(0); + + Assert.Same(collection[0], target.SelectedItem); + Assert.Equal(collection[0].Content, target.SelectedContent); } [Fact] @@ -349,6 +394,8 @@ namespace Avalonia.Controls.UnitTests } } + + private IControlTemplate TabControlTemplate() { return new FuncControlTemplate((parent, scope) => @@ -383,6 +430,13 @@ namespace Avalonia.Controls.UnitTests }.RegisterInNameScope(scope)); } + private void Prepare(TabControl target) + { + ApplyTemplate(target); + target.Measure(Size.Infinity); + target.Arrange(new Rect(target.DesiredSize)); + } + private void ApplyTemplate(TabControl target) { target.ApplyTemplate(); From 10b3c880927e9effeedc2a546b66a987ce3293be Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 17:03:31 +0200 Subject: [PATCH 03/46] Added failing SelectedItem/Index property changed tests. --- .../Primitives/SelectingItemsControlTests.cs | 218 ++++++++++++++++++ 1 file changed, 218 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index 00d148093a..c6615f04f9 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -554,6 +554,44 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.False(items.Single().IsSelected); } + [Fact] + public void Removing_Selected_Item_Should_Update_Selection_With_AlwaysSelected() + { + var item0 = new Item(); + var item1 = new Item(); + var items = new AvaloniaList + { + item0, + item1, + }; + + var target = new TestSelector + { + Items = items, + Template = Template(), + SelectionMode = SelectionMode.AlwaysSelected, + }; + + Prepare(target); + target.SelectedIndex = 1; + + Assert.Equal(items[1], target.SelectedItem); + Assert.Equal(1, target.SelectedIndex); + + SelectionChangedEventArgs receivedArgs = null; + + target.SelectionChanged += (_, args) => receivedArgs = args; + + items.RemoveAt(1); + + Assert.Same(item0, target.SelectedItem); + Assert.Equal(0, target.SelectedIndex); + Assert.NotNull(receivedArgs); + Assert.Equal(new[] { item0 }, receivedArgs.AddedItems); + Assert.Equal(new[] { item1 }, receivedArgs.RemovedItems); + Assert.True(items.Single().IsSelected); + } + [Fact] public void Removing_Selected_Item_Should_Clear_Selection_With_BeginInit() { @@ -771,6 +809,186 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.True(called); } + [Fact] + public void Setting_SelectedIndex_Should_Raise_PropertyChanged_Events() + { + var items = new ObservableCollection { "foo", "bar", "baz" }; + + var target = new TestSelector + { + Items = items, + Template = Template(), + }; + + var selectedIndexRaised = 0; + var selectedItemRaised = 0; + + target.PropertyChanged += (s, e) => + { + if (e.Property == SelectingItemsControl.SelectedIndexProperty) + { + Assert.Equal(-1, e.OldValue); + Assert.Equal(1, e.NewValue); + ++selectedIndexRaised; + } + else if (e.Property == SelectingItemsControl.SelectedItemProperty) + { + Assert.Null(e.OldValue); + Assert.Equal("bar", e.NewValue); + ++selectedItemRaised; + } + }; + + target.SelectedIndex = 1; + + Assert.Equal(1, selectedIndexRaised); + Assert.Equal(1, selectedItemRaised); + } + + [Fact] + public void Removing_Selected_Item_Should_Raise_PropertyChanged_Events() + { + var items = new ObservableCollection { "foo", "bar", "baz" }; + + var target = new TestSelector + { + Items = items, + Template = Template(), + }; + + var selectedIndexRaised = 0; + var selectedItemRaised = 0; + target.SelectedIndex = 1; + + target.PropertyChanged += (s, e) => + { + if (e.Property == SelectingItemsControl.SelectedIndexProperty) + { + Assert.Equal(1, e.OldValue); + Assert.Equal(-1, e.NewValue); + ++selectedIndexRaised; + } + else if (e.Property == SelectingItemsControl.SelectedItemProperty) + { + Assert.Equal("bar", e.OldValue); + Assert.Null(e.NewValue); + } + }; + + items.RemoveAt(1); + + Assert.Equal(1, selectedIndexRaised); + Assert.Equal(0, selectedItemRaised); + } + + [Fact] + public void Removing_Selected_Item0_Should_Raise_PropertyChanged_Events_With_AlwaysSelected() + { + var items = new ObservableCollection { "foo", "bar", "baz" }; + + var target = new TestSelector + { + Items = items, + Template = Template(), + SelectionMode = SelectionMode.AlwaysSelected, + }; + + var selectedIndexRaised = 0; + var selectedItemRaised = 0; + target.SelectedIndex = 0; + + target.PropertyChanged += (s, e) => + { + if (e.Property == SelectingItemsControl.SelectedIndexProperty) + { + ++selectedIndexRaised; + } + else if (e.Property == SelectingItemsControl.SelectedItemProperty) + { + Assert.Equal("foo", e.OldValue); + Assert.Equal("bar", e.NewValue); + ++selectedItemRaised; + } + }; + + items.RemoveAt(0); + + Assert.Equal(0, selectedIndexRaised); + Assert.Equal(1, selectedItemRaised); + } + + [Fact] + public void Removing_Selected_Item1_Should_Raise_PropertyChanged_Events_With_AlwaysSelected() + { + var items = new ObservableCollection { "foo", "bar", "baz" }; + + var target = new TestSelector + { + Items = items, + Template = Template(), + SelectionMode = SelectionMode.AlwaysSelected, + }; + + var selectedIndexRaised = 0; + var selectedItemRaised = 0; + target.SelectedIndex = 1; + + target.PropertyChanged += (s, e) => + { + if (e.Property == SelectingItemsControl.SelectedIndexProperty) + { + Assert.Equal(1, e.OldValue); + Assert.Equal(0, e.NewValue); + ++selectedIndexRaised; + } + else if (e.Property == SelectingItemsControl.SelectedItemProperty) + { + Assert.Equal("bar", e.OldValue); + Assert.Equal("foo", e.NewValue); + } + }; + + items.RemoveAt(1); + + Assert.Equal(1, selectedIndexRaised); + Assert.Equal(0, selectedItemRaised); + } + + [Fact] + public void Removing_Item_Before_Selection_Should_Raise_PropertyChanged_Events() + { + var items = new ObservableCollection { "foo", "bar", "baz" }; + + var target = new SelectingItemsControl + { + Items = items, + Template = Template(), + }; + + var selectedIndexRaised = 0; + var selectedItemRaised = 0; + target.SelectedIndex = 1; + + target.PropertyChanged += (s, e) => + { + if (e.Property == SelectingItemsControl.SelectedIndexProperty) + { + Assert.Equal(1, e.OldValue); + Assert.Equal(0, e.NewValue); + ++selectedIndexRaised; + } + else if (e.Property == SelectingItemsControl.SelectedItemProperty) + { + ++selectedItemRaised; + } + }; + + items.RemoveAt(0); + + Assert.Equal(1, selectedIndexRaised); + Assert.Equal(0, selectedItemRaised); + } + [Fact] public void Order_Of_Setting_Items_And_SelectedIndex_During_Initialization_Should_Not_Matter() { From 5ac25a26f4ee09fd37fd2b3fc386134e884a30a7 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 17:08:22 +0200 Subject: [PATCH 04/46] Tweak raising SelectedIndex/Item property changed. --- .../Primitives/SelectingItemsControl.cs | 2 +- .../Selection/SelectionModel.cs | 42 ++++++++++++------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 5f8c5da2f8..fcef3b1d08 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -613,7 +613,7 @@ namespace Avalonia.Controls.Primitives RaisePropertyChanged(SelectedIndexProperty, _oldSelectedIndex, SelectedIndex); _oldSelectedIndex = SelectedIndex; } - else if (e.PropertyName == nameof(ISelectionModel.SelectedItem)) + else if (e.PropertyName == nameof(ISelectionModel.SelectedItem) && _oldSelectedItem != SelectedItem) { RaisePropertyChanged(SelectedItemProperty, _oldSelectedItem, SelectedItem); _oldSelectedItem = SelectedItem; diff --git a/src/Avalonia.Controls/Selection/SelectionModel.cs b/src/Avalonia.Controls/Selection/SelectionModel.cs index 2556bd4c4c..2181b322b4 100644 --- a/src/Avalonia.Controls/Selection/SelectionModel.cs +++ b/src/Avalonia.Controls/Selection/SelectionModel.cs @@ -345,7 +345,9 @@ namespace Avalonia.Controls.Selection LostSelection(this, EventArgs.Empty); } - CommitOperation(update.Operation); + // Don't raise PropertyChanged events here as the OnSourceCollectionChanged event that + // let to this method being called will raise them if necessary. + CommitOperation(update.Operation, raisePropertyChanged: false); } private protected override CollectionChangeState OnItemsAdded(int index, IList items) @@ -430,6 +432,11 @@ namespace Avalonia.Controls.Selection RaisePropertyChanged(nameof(SelectedIndex)); } + if (e.Action == NotifyCollectionChangedAction.Remove && e.OldStartingIndex <= oldSelectedIndex) + { + RaisePropertyChanged(nameof(SelectedItem)); + } + if (oldAnchorIndex != _anchorIndex) { RaisePropertyChanged(nameof(AnchorIndex)); @@ -611,7 +618,7 @@ namespace Avalonia.Controls.Selection } } - private void CommitOperation(Operation operation) + private void CommitOperation(Operation operation, bool raisePropertyChanged = true) { try { @@ -679,23 +686,26 @@ namespace Avalonia.Controls.Selection } } - if (oldSelectedIndex != _selectedIndex) + if (raisePropertyChanged) { - indexesChanged = true; - RaisePropertyChanged(nameof(SelectedIndex)); - RaisePropertyChanged(nameof(SelectedItem)); - } + if (oldSelectedIndex != _selectedIndex) + { + indexesChanged = true; + RaisePropertyChanged(nameof(SelectedIndex)); + RaisePropertyChanged(nameof(SelectedItem)); + } - if (oldAnchorIndex != _anchorIndex) - { - indexesChanged = true; - RaisePropertyChanged(nameof(AnchorIndex)); - } + if (oldAnchorIndex != _anchorIndex) + { + indexesChanged = true; + RaisePropertyChanged(nameof(AnchorIndex)); + } - if (indexesChanged) - { - RaisePropertyChanged(nameof(SelectedIndexes)); - RaisePropertyChanged(nameof(SelectedItems)); + if (indexesChanged) + { + RaisePropertyChanged(nameof(SelectedIndexes)); + RaisePropertyChanged(nameof(SelectedItems)); + } } } finally From 90dc7ea95238ab696316b89fd00066d13f4a0541 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 17:33:15 +0200 Subject: [PATCH 05/46] Fix failing test but actual issue not fixed. Fixed the failing test for #4654 but actual issue still remains. Needs more test. --- src/Avalonia.Controls/TabControl.cs | 55 ++++++----------------------- 1 file changed, 11 insertions(+), 44 deletions(-) diff --git a/src/Avalonia.Controls/TabControl.cs b/src/Avalonia.Controls/TabControl.cs index f81e355a7d..f7e7126324 100644 --- a/src/Avalonia.Controls/TabControl.cs +++ b/src/Avalonia.Controls/TabControl.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.Linq; using Avalonia.Collections; using Avalonia.Controls.Generators; @@ -66,7 +67,7 @@ namespace Avalonia.Controls SelectionModeProperty.OverrideDefaultValue(SelectionMode.AlwaysSelected); ItemsPanelProperty.OverrideDefaultValue(DefaultPanel); AffectsMeasure(TabStripPlacementProperty); - SelectedIndexProperty.Changed.AddClassHandler((x, e) => x.UpdateSelectedContent(e)); + SelectedItemProperty.Changed.AddClassHandler((x, e) => x.UpdateSelectedContent()); } /// @@ -145,55 +146,21 @@ namespace Avalonia.Controls protected override void OnContainersMaterialized(ItemContainerEventArgs e) { base.OnContainersMaterialized(e); - - if (SelectedContent != null || SelectedIndex == -1) - { - return; - } - - var container = (TabItem)ItemContainerGenerator.ContainerFromIndex(SelectedIndex); - - if (container == null) - { - return; - } - - UpdateSelectedContent(container); - } - - private void UpdateSelectedContent(AvaloniaPropertyChangedEventArgs e) - { - var index = (int)e.NewValue; - - if (index == -1) - { - SelectedContentTemplate = null; - - SelectedContent = null; - - return; - } - - var container = (TabItem)ItemContainerGenerator.ContainerFromIndex(index); - - if (container == null) - { - return; - } - - UpdateSelectedContent(container); + UpdateSelectedContent(); } - private void UpdateSelectedContent(IContentControl item) + private void UpdateSelectedContent() { - if (SelectedContentTemplate != item.ContentTemplate) + if (SelectedIndex == -1) { - SelectedContentTemplate = item.ContentTemplate; + SelectedContent = SelectedContentTemplate = null; } - - if (SelectedContent != item.Content) + else { - SelectedContent = item.Content; + var container = SelectedItem as IContentControl ?? + ItemContainerGenerator.ContainerFromIndex(SelectedIndex) as IContentControl; + SelectedContentTemplate = container?.ContentTemplate; + SelectedContent = container?.Content; } } From a38bdcf146a8f9f4647d20634b3e7f8689e781d6 Mon Sep 17 00:00:00 2001 From: "Artyom V. Gorchakov" Date: Fri, 11 Sep 2020 18:49:48 +0300 Subject: [PATCH 06/46] Correct ReactiveUI.Events Package Id --- .../Avalonia.ReactiveUI.Events.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.ReactiveUI.Events/Avalonia.ReactiveUI.Events.csproj b/src/Avalonia.ReactiveUI.Events/Avalonia.ReactiveUI.Events.csproj index b95c8946e2..75eeb92f42 100644 --- a/src/Avalonia.ReactiveUI.Events/Avalonia.ReactiveUI.Events.csproj +++ b/src/Avalonia.ReactiveUI.Events/Avalonia.ReactiveUI.Events.csproj @@ -1,7 +1,7 @@  netstandard2.0 - Avalonia.ReactiveUI + Avalonia.ReactiveUI.Events From f4017162cd5ca4c65cd92a2a7f7a90c638505640 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 20:34:42 +0200 Subject: [PATCH 07/46] Add additional failing test for #4654. --- .../TabControlTests.cs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs index b4378938de..e6f7ac601f 100644 --- a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs @@ -174,6 +174,36 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(collection[0].Content, target.SelectedContent); } + [Fact] + public void Removal_Should_Set_New_Item0_When_Item0_Selected_With_DataTemplate() + { + using var app = UnitTestApplication.Start(TestServices.StyledWindow); + + var collection = new ObservableCollection() + { + new Item("first"), + new Item("second"), + new Item("3rd"), + }; + + var target = new TabControl + { + Template = TabControlTemplate(), + Items = collection, + }; + + Prepare(target); + target.SelectedItem = collection[0]; + + Assert.Same(collection[0], target.SelectedItem); + Assert.Equal(collection[0], target.SelectedContent); + + collection.RemoveAt(0); + + Assert.Same(collection[0], target.SelectedItem); + Assert.Equal(collection[0], target.SelectedContent); + } + [Fact] public void TabItem_Templates_Should_Be_Set_Before_TabItem_ApplyTemplate() { @@ -394,8 +424,6 @@ namespace Avalonia.Controls.UnitTests } } - - private IControlTemplate TabControlTemplate() { return new FuncControlTemplate((parent, scope) => From 82273680f7bcb29c098c0f240e082bbf1fb43928 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 20:35:11 +0200 Subject: [PATCH 08/46] Update selected content when containers recycled. Hopefully fixes #4654 once and for all. --- src/Avalonia.Controls/TabControl.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Avalonia.Controls/TabControl.cs b/src/Avalonia.Controls/TabControl.cs index f7e7126324..306a9d3e6a 100644 --- a/src/Avalonia.Controls/TabControl.cs +++ b/src/Avalonia.Controls/TabControl.cs @@ -149,6 +149,12 @@ namespace Avalonia.Controls UpdateSelectedContent(); } + protected override void OnContainersRecycled(ItemContainerEventArgs e) + { + base.OnContainersRecycled(e); + UpdateSelectedContent(); + } + private void UpdateSelectedContent() { if (SelectedIndex == -1) From 6275a1ca27078db13cf5a553b43d4b085a7395f1 Mon Sep 17 00:00:00 2001 From: artyom Date: Sat, 12 Sep 2020 00:29:14 +0300 Subject: [PATCH 09/46] Don't test code that is generated at nukebuild time --- Avalonia.sln | 27 ------------ nukebuild/Build.cs | 1 - ...valonia.ReactiveUI.Events.UnitTests.csproj | 15 ------- .../BasicControlEventsTest.cs | 44 ------------------- 4 files changed, 87 deletions(-) delete mode 100644 tests/Avalonia.ReactiveUI.Events.UnitTests/Avalonia.ReactiveUI.Events.UnitTests.csproj delete mode 100644 tests/Avalonia.ReactiveUI.Events.UnitTests/BasicControlEventsTest.cs diff --git a/Avalonia.sln b/Avalonia.sln index ddcd61408d..aeae8f2f1f 100644 --- a/Avalonia.sln +++ b/Avalonia.sln @@ -224,8 +224,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.Markup.Xaml.Loader EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.ReactiveUI.Events", "src\Avalonia.ReactiveUI.Events\Avalonia.ReactiveUI.Events.csproj", "{28F18757-C3E6-4BBE-A37D-11BA2AB9177C}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.ReactiveUI.Events.UnitTests", "tests\Avalonia.ReactiveUI.Events.UnitTests\Avalonia.ReactiveUI.Events.UnitTests.csproj", "{780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}" -EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution src\Shared\RenderHelpers\RenderHelpers.projitems*{3c4c0cb4-0c0f-4450-a37b-148c84ff905f}*SharedItemsImports = 13 @@ -2040,30 +2038,6 @@ Global {28F18757-C3E6-4BBE-A37D-11BA2AB9177C}.Release|iPhone.Build.0 = Release|Any CPU {28F18757-C3E6-4BBE-A37D-11BA2AB9177C}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU {28F18757-C3E6-4BBE-A37D-11BA2AB9177C}.Release|iPhoneSimulator.Build.0 = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|Any CPU.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|Any CPU.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhone.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhone.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhoneSimulator.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhoneSimulator.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|Any CPU.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|Any CPU.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhone.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhone.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhoneSimulator.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhoneSimulator.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|Any CPU.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhone.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhone.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|Any CPU.ActiveCfg = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|Any CPU.Build.0 = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhone.ActiveCfg = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhone.Build.0 = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhoneSimulator.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -2122,7 +2096,6 @@ Global {351337F5-D66F-461B-A957-4EF60BDB4BA6} = {C5A00AC3-B34C-4564-9BDD-2DA473EF4D8B} {3C84E04B-36CF-4D0D-B965-C26DD649D1F3} = {A0CC0258-D18C-4AB3-854F-7101680FC3F9} {909A8CBD-7D0E-42FD-B841-022AD8925820} = {8B6A8209-894F-4BA1-B880-965FD453982C} - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7} = {C5A00AC3-B34C-4564-9BDD-2DA473EF4D8B} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {87366D66-1391-4D90-8999-95A620AD786A} diff --git a/nukebuild/Build.cs b/nukebuild/Build.cs index 24398accbb..7e2bbc13bc 100644 --- a/nukebuild/Build.cs +++ b/nukebuild/Build.cs @@ -241,7 +241,6 @@ partial class Build : NukeBuild RunCoreTest("Avalonia.Visuals.UnitTests"); RunCoreTest("Avalonia.Skia.UnitTests"); RunCoreTest("Avalonia.ReactiveUI.UnitTests"); - RunCoreTest("Avalonia.ReactiveUI.Events.UnitTests"); }); Target RunRenderTests => _ => _ diff --git a/tests/Avalonia.ReactiveUI.Events.UnitTests/Avalonia.ReactiveUI.Events.UnitTests.csproj b/tests/Avalonia.ReactiveUI.Events.UnitTests/Avalonia.ReactiveUI.Events.UnitTests.csproj deleted file mode 100644 index 19a6fd138e..0000000000 --- a/tests/Avalonia.ReactiveUI.Events.UnitTests/Avalonia.ReactiveUI.Events.UnitTests.csproj +++ /dev/null @@ -1,15 +0,0 @@ - - - netcoreapp3.1 - - - - - - - - - - - - diff --git a/tests/Avalonia.ReactiveUI.Events.UnitTests/BasicControlEventsTest.cs b/tests/Avalonia.ReactiveUI.Events.UnitTests/BasicControlEventsTest.cs deleted file mode 100644 index 1092c98246..0000000000 --- a/tests/Avalonia.ReactiveUI.Events.UnitTests/BasicControlEventsTest.cs +++ /dev/null @@ -1,44 +0,0 @@ -using System; -using System.Reactive.Linq; -using Avalonia.Controls; -using Avalonia.UnitTests; -using Xunit; - -namespace Avalonia.ReactiveUI.Events.UnitTests -{ - public class BasicControlEventsTest - { - public class EventsControl : UserControl - { - public bool IsAttached { get; private set; } - - public EventsControl() - { - var attached = this - .Events() - .AttachedToVisualTree - .Select(args => true); - - this.Events() - .DetachedFromVisualTree - .Select(args => false) - .Merge(attached) - .Subscribe(marker => IsAttached = marker); - } - } - - [Fact] - public void Should_Generate_Events_Wrappers() - { - var root = new TestRoot(); - var control = new EventsControl(); - Assert.False(control.IsAttached); - - root.Child = control; - Assert.True(control.IsAttached); - - root.Child = null; - Assert.False(control.IsAttached); - } - } -} From b30120770c6fce6b581b9f47424df011930db005 Mon Sep 17 00:00:00 2001 From: artyom Date: Sat, 12 Sep 2020 00:52:23 +0300 Subject: [PATCH 10/46] Ignore Events_Avalonia.cs which is autogenerated --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b5a46e16f4..7d672c7755 100644 --- a/.gitignore +++ b/.gitignore @@ -117,6 +117,7 @@ ClientBin/ *.[Pp]ublish.xml *.pfx *.publishsettings +Events_Avalonia.cs # RIA/Silverlight projects Generated_Code/ From d84a98f65ac04f03f40bbc4b33c9201fc660e1cc Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 12 Sep 2020 00:17:15 +0200 Subject: [PATCH 11/46] Added failing test for #4272. --- .../Primitives/SelectingItemsControlTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index c6615f04f9..b6619aaa73 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -1594,6 +1594,31 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(new[] { "foo" }, target.SelectedItems); } + [Fact] + public void Preserves_Initial_SelectedItems_When_Bound() + { + // Issue #4272 (there are two issues there, this addresses the second one). + var vm = new SelectionViewModel + { + Items = { "foo", "bar", "baz" }, + SelectedItems = { "bar" }, + }; + + var target = new ListBox + { + [!ListBox.ItemsProperty] = new Binding("Items"), + [!ListBox.SelectedItemsProperty] = new Binding("SelectedItems"), + DataContext = vm, + }; + + Prepare(target); + + Assert.Equal(1, target.SelectedIndex); + Assert.Equal(new[] { 1 }, target.Selection.SelectedIndexes); + Assert.Equal("bar", target.SelectedItem); + Assert.Equal(new[] { "bar" }, target.SelectedItems); + } + private static void Prepare(SelectingItemsControl target) { var root = new TestRoot @@ -1663,6 +1688,7 @@ namespace Avalonia.Controls.UnitTests.Primitives public SelectionViewModel() { Items = new ObservableCollection(); + SelectedItems = new ObservableCollection(); } public int SelectedIndex @@ -1676,6 +1702,7 @@ namespace Avalonia.Controls.UnitTests.Primitives } public ObservableCollection Items { get; } + public ObservableCollection SelectedItems { get; } } private class RootWithItems : TestRoot From 4f161701bd1713fe5f2b88a420fb04eb1b7591f8 Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Sun, 13 Sep 2020 05:01:33 -0400 Subject: [PATCH 12/46] Do not override BindingMode in the DataGridBoundColumn --- src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs b/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs index 8e82bf1a38..440a277a10 100644 --- a/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs +++ b/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs @@ -49,8 +49,13 @@ namespace Avalonia.Controls { if(_binding is Avalonia.Data.Binding binding) { + if (binding.Mode == BindingMode.OneWayToSource) + { + throw new InvalidOperationException("DataGridColumn doesn't support BindingMode.OneWayToSource"); + } + // Force the TwoWay binding mode if there is a Path present. TwoWay binding requires a Path. - if (!String.IsNullOrEmpty(binding.Path)) + if (!String.IsNullOrEmpty(binding.Path) && binding.Mode == BindingMode.Default) { binding.Mode = BindingMode.TwoWay; } From f8e75bf424b7697ac94bb6bebb440ce6c7b2ab26 Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Sun, 13 Sep 2020 05:19:01 -0400 Subject: [PATCH 13/46] Remove obsolete comments --- src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs b/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs index 440a277a10..1e72a07760 100644 --- a/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs +++ b/src/Avalonia.Controls.DataGrid/DataGridBoundColumn.cs @@ -51,10 +51,9 @@ namespace Avalonia.Controls { if (binding.Mode == BindingMode.OneWayToSource) { - throw new InvalidOperationException("DataGridColumn doesn't support BindingMode.OneWayToSource"); + throw new InvalidOperationException("DataGridColumn doesn't support BindingMode.OneWayToSource. Use BindingMode.TwoWay instead."); } - // Force the TwoWay binding mode if there is a Path present. TwoWay binding requires a Path. if (!String.IsNullOrEmpty(binding.Path) && binding.Mode == BindingMode.Default) { binding.Mode = BindingMode.TwoWay; From 22580652fa6f0893c90f1db544dd69eafb980f13 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 10:11:30 +0200 Subject: [PATCH 14/46] More failing SelectionModel tests. --- .../Selection/SelectionModelTests_Multiple.cs | 56 +++++++++++++++++++ .../Selection/SelectionModelTests_Single.cs | 56 ++++++++++++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs index 3eddd35465..5d0c6d31e1 100644 --- a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs +++ b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs @@ -121,6 +121,34 @@ namespace Avalonia.Controls.UnitTests.Selection Assert.Equal(0, raised); } + [Fact] + public void Initializing_Source_Raises_SelectedItems_PropertyChanged() + { + var target = CreateTarget(false); + var selectedItemRaised = 0; + var selectedItemsRaised = 0; + + target.Select(1); + target.Select(2); + + target.PropertyChanged += (s, e) => + { + if (e.PropertyName == nameof(target.SelectedItem)) + { + ++selectedItemRaised; + } + else if (e.PropertyName == nameof(target.SelectedItems)) + { + ++selectedItemsRaised; + } + }; + + target.Source = new[] { "foo", "bar", "baz" }; + + Assert.Equal(1, selectedItemRaised); + Assert.Equal(1, selectedItemsRaised); + } + [Fact] public void Initializing_Source_Respects_Range_SourceItem_Order() { @@ -152,6 +180,34 @@ namespace Avalonia.Controls.UnitTests.Selection Assert.Equal("bar", target.SelectedItem); Assert.Equal(new[] { "bar" }, target.SelectedItems); } + + [Fact] + public void Changing_Source_To_Null_Raises_SelectedItems_PropertyChanged() + { + var target = CreateTarget(); + var selectedItemRaised = 0; + var selectedItemsRaised = 0; + + target.Select(1); + target.Select(2); + + target.PropertyChanged += (s, e) => + { + if (e.PropertyName == nameof(target.SelectedItem)) + { + ++selectedItemRaised; + } + else if (e.PropertyName == nameof(target.SelectedItems)) + { + ++selectedItemsRaised; + } + }; + + target.Source = null; + + Assert.Equal(1, selectedItemRaised); + Assert.Equal(1, selectedItemsRaised); + } } public class SelectedIndex diff --git a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs index 1b37730797..66a2cef921 100644 --- a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs +++ b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs @@ -174,6 +174,33 @@ namespace Avalonia.Controls.UnitTests.Selection Assert.Equal(new[] { "bar" }, target.SelectedItems); } + [Fact] + public void Initializing_Source_Raises_SelectedItems_PropertyChanged() + { + var target = CreateTarget(false); + var selectedItemRaised = 0; + var selectedItemsRaised = 0; + + target.Select(1); + + target.PropertyChanged += (s, e) => + { + if (e.PropertyName == nameof(target.SelectedItem)) + { + ++selectedItemRaised; + } + else if (e.PropertyName == nameof(target.SelectedItems)) + { + ++selectedItemsRaised; + } + }; + + target.Source = new[] { "foo", "bar", "baz" }; + + Assert.Equal(1, selectedItemRaised); + Assert.Equal(1, selectedItemsRaised); + } + [Fact] public void Changing_Source_To_Null_Doesnt_Clear_Selection() { @@ -194,7 +221,7 @@ namespace Avalonia.Controls.UnitTests.Selection } [Fact] - public void Changing_Source_To_NonNUll_First_Clears_Old_Selection() + public void Changing_Source_To_NonNull_First_Clears_Old_Selection() { var target = CreateTarget(); var raised = 0; @@ -219,6 +246,33 @@ namespace Avalonia.Controls.UnitTests.Selection Assert.Equal(1, raised); } + [Fact] + public void Changing_Source_To_Null_Raises_SelectedItems_PropertyChanged() + { + var target = CreateTarget(); + var selectedItemRaised = 0; + var selectedItemsRaised = 0; + + target.Select(1); + + target.PropertyChanged += (s, e) => + { + if (e.PropertyName == nameof(target.SelectedItem)) + { + ++selectedItemRaised; + } + else if (e.PropertyName == nameof(target.SelectedItems)) + { + ++selectedItemsRaised; + } + }; + + target.Source = null; + + Assert.Equal(1, selectedItemRaised); + Assert.Equal(1, selectedItemsRaised); + } + [Fact] public void Raises_PropertyChanged() { From aeaaccb7e0fe239f9994cc01d753b99211e057c7 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 10:17:01 +0200 Subject: [PATCH 15/46] Raise item(s) property changed on Source changing. --- src/Avalonia.Controls/Selection/SelectionModel.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Avalonia.Controls/Selection/SelectionModel.cs b/src/Avalonia.Controls/Selection/SelectionModel.cs index 2181b322b4..326bd10655 100644 --- a/src/Avalonia.Controls/Selection/SelectionModel.cs +++ b/src/Avalonia.Controls/Selection/SelectionModel.cs @@ -692,6 +692,10 @@ namespace Avalonia.Controls.Selection { indexesChanged = true; RaisePropertyChanged(nameof(SelectedIndex)); + } + + if (oldSelectedIndex != _selectedIndex || operation.IsSourceUpdate) + { RaisePropertyChanged(nameof(SelectedItem)); } @@ -704,6 +708,10 @@ namespace Avalonia.Controls.Selection if (indexesChanged) { RaisePropertyChanged(nameof(SelectedIndexes)); + } + + if (indexesChanged || operation.IsSourceUpdate) + { RaisePropertyChanged(nameof(SelectedItems)); } } From b73ba990777c9727cad4efcd64b968c33b86f336 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 15:16:34 +0200 Subject: [PATCH 16/46] Reafactor SelectingItemsControl selection. - Remove `SelectedItemsSync` and store `SelectedItems` in a new `InternalSelectionModel` - Store transient `SelectingItemsControl` state in an `UpdateState` object Fixes #4272 --- .../Primitives/SelectingItemsControl.cs | 225 +++++++++++--- .../Selection/InternalSelectionModel.cs | 251 ++++++++++++++++ .../Selection/SelectionModel.cs | 56 +++- .../Utils/SelectedItemsSync.cs | 283 ------------------ .../Selection/InternalSelectionModelTests.cs | 243 +++++++++++++++ .../Utils/SelectedItemsSyncTests.cs | 278 ----------------- 6 files changed, 712 insertions(+), 624 deletions(-) create mode 100644 src/Avalonia.Controls/Selection/InternalSelectionModel.cs delete mode 100644 src/Avalonia.Controls/Utils/SelectedItemsSync.cs create mode 100644 tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs delete mode 100644 tests/Avalonia.Controls.UnitTests/Utils/SelectedItemsSyncTests.cs diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index fcef3b1d08..3e106afdc0 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -6,7 +6,6 @@ using System.ComponentModel; using System.Linq; using Avalonia.Controls.Generators; using Avalonia.Controls.Selection; -using Avalonia.Controls.Utils; using Avalonia.Data; using Avalonia.Input; using Avalonia.Input.Platform; @@ -70,8 +69,8 @@ namespace Avalonia.Controls.Primitives /// /// Defines the property. /// - protected static readonly DirectProperty SelectedItemsProperty = - AvaloniaProperty.RegisterDirect( + protected static readonly DirectProperty SelectedItemsProperty = + AvaloniaProperty.RegisterDirect( nameof(SelectedItems), o => o.SelectedItems, (o, v) => o.SelectedItems = v); @@ -111,12 +110,11 @@ namespace Avalonia.Controls.Primitives RoutingStrategies.Bubble); private static readonly IList Empty = Array.Empty(); - private SelectedItemsSync? _selectedItemsSync; private ISelectionModel? _selection; private int _oldSelectedIndex; private object? _oldSelectedItem; - private int _initializing; private bool _ignoreContainerSelectionChanged; + private UpdateState? _updateState; /// /// Initializes static members of the class. @@ -149,8 +147,23 @@ namespace Avalonia.Controls.Primitives /// public int SelectedIndex { - get => Selection.SelectedIndex; - set => Selection.SelectedIndex = value; + get + { + return _updateState?.SelectedIndex.HasValue == true ? + _updateState.SelectedIndex.Value : + Selection.SelectedIndex; + } + set + { + if (_updateState is object) + { + _updateState.SelectedIndex = value; + } + else + { + Selection.SelectedIndex = value; + } + } } /// @@ -158,17 +171,56 @@ namespace Avalonia.Controls.Primitives /// public object? SelectedItem { - get => Selection.SelectedItem; - set => Selection.SelectedItem = value; + get + { + return _updateState?.SelectedItem.HasValue == true ? + _updateState.SelectedItem.Value : + Selection.SelectedItem; + } + set + { + if (_updateState is object) + { + _updateState.SelectedItem = value; + } + else + { + Selection.SelectedItem = value; + } + } } /// /// Gets or sets the selected items. /// - protected IList SelectedItems + /// + /// By default returns a collection that can be modified in order to manipulate the control + /// selection, however this property will return null if is + /// re-assigned; you should only use _either_ Selection or SelectedItems. + /// + protected IList? SelectedItems { - get => SelectedItemsSync.SelectedItems; - set => SelectedItemsSync.SelectedItems = value; + get + { + return _updateState?.SelectedItems.HasValue == true ? + _updateState.SelectedItems.Value : + (Selection as InternalSelectionModel)?.SelectedItems; + } + set + { + if (_updateState is object) + { + _updateState.SelectedItems = new Optional(value); + } + else if (Selection is InternalSelectionModel i) + { + i.SelectedItems = value; + } + else + { + throw new InvalidOperationException("Cannot set both Selection and SelectedItems."); + } + } } /// @@ -178,19 +230,30 @@ namespace Avalonia.Controls.Primitives { get { - if (_selection is null) + if (_updateState?.Selection.HasValue == true) { - _selection = CreateDefaultSelectionModel(); - InitializeSelectionModel(_selection); + return _updateState.Selection.Value; } + else + { + if (_selection is null) + { + _selection = CreateDefaultSelectionModel(); + InitializeSelectionModel(_selection); + } - return _selection; + return _selection; + } } set { value ??= CreateDefaultSelectionModel(); - if (_selection != value) + if (_updateState is object) + { + _updateState.Selection = new Optional(value); + } + else if (_selection != value) { if (value.Source != null && value.Source != Items) { @@ -234,20 +297,18 @@ namespace Avalonia.Controls.Primitives /// protected bool AlwaysSelected => (SelectionMode & SelectionMode.AlwaysSelected) != 0; - private SelectedItemsSync SelectedItemsSync => _selectedItemsSync ??= new SelectedItemsSync(Selection); - /// public override void BeginInit() { base.BeginInit(); - ++_initializing; + BeginUpdating(); } /// public override void EndInit() { base.EndInit(); - --_initializing; + EndUpdating(); } /// @@ -351,30 +412,14 @@ namespace Avalonia.Controls.Primitives protected override void OnDataContextBeginUpdate() { base.OnDataContextBeginUpdate(); - ++_initializing; - - if (_selection is object) - { - _selection.Source = null; - } + BeginUpdating(); } /// protected override void OnDataContextEndUpdate() { base.OnDataContextEndUpdate(); - --_initializing; - - if (_selection is object && _initializing == 0) - { - _selection.Source = Items; - - if (Items is null) - { - _selection.Clear(); - _selectedItemsSync?.SelectedItems?.Clear(); - } - } + EndUpdating(); } protected override void OnInitialized() @@ -411,9 +456,7 @@ namespace Avalonia.Controls.Primitives { base.OnPropertyChanged(change); - if (change.Property == ItemsProperty && - _initializing == 0 && - _selection is object) + if (change.Property == ItemsProperty && _updateState is null && _selection is object) { var newValue = change.NewValue.GetValueOrDefault(); _selection.Source = newValue; @@ -789,7 +832,7 @@ namespace Avalonia.Controls.Primitives private ISelectionModel CreateDefaultSelectionModel() { - return new SelectionModel + return new InternalSelectionModel { SingleSelect = !SelectionMode.HasFlagCustom(SelectionMode.Multiple), }; @@ -797,7 +840,7 @@ namespace Avalonia.Controls.Primitives private void InitializeSelectionModel(ISelectionModel model) { - if (_initializing == 0) + if (_updateState is null) { model.Source = Items; } @@ -825,9 +868,6 @@ namespace Avalonia.Controls.Primitives UpdateContainerSelection(); - _selectedItemsSync ??= new SelectedItemsSync(model); - _selectedItemsSync.SelectionModel = model; - if (SelectedIndex != -1) { RaiseEvent(new SelectionChangedEventArgs( @@ -845,5 +885,96 @@ namespace Avalonia.Controls.Primitives model.SelectionChanged -= OnSelectionModelSelectionChanged; } } + + private void BeginUpdating() + { + _updateState ??= new UpdateState(); + _updateState.UpdateCount++; + } + + private void EndUpdating() + { + if (_updateState is object && --_updateState.UpdateCount == 0) + { + var state = _updateState; + _updateState = null; + + if (state.Selection.HasValue) + { + Selection = state.Selection.Value; + } + + if (state.SelectedItems.HasValue) + { + SelectedItems = state.SelectedItems.Value; + } + + Selection.Source = Items; + + if (Items is null) + { + Selection.Clear(); + } + + if (state.SelectedIndex.HasValue) + { + SelectedIndex = state.SelectedIndex.Value; + } + else if (state.SelectedItem.HasValue) + { + SelectedItem = state.SelectedItem.Value; + } + } + } + + // When in a BeginInit..EndInit block, or when the DataContext is updating, we need to + // defer changes to the selection model because we have no idea in which order properties + // will be set. Consider: + // + // - Both Items and SelectedItem are bound + // - The DataContext changes + // - The binding for SelectedItem updates first, producing an item + // - Items is searched to find the index of the new selected item + // - However Items isn't yet updated; the item is not found + // - SelectedIndex is incorrectly set to -1 + // + // This logic cannot be encapsulated in SelectionModel because the selection model can also + // be bound, consider: + // + // - Both Items and Selection are bound + // - The DataContext changes + // - The binding for Items updates first + // - The new items are assigned to Selection.Source + // - The binding for Selection updates, producing a new SelectionModel + // - Both the old and new SelectionModels have the incorrect Source + private class UpdateState + { + private Optional _selectedIndex; + private Optional _selectedItem; + + public int UpdateCount { get; set; } + public Optional Selection { get; set; } + public Optional SelectedItems { get; set; } + + public Optional SelectedIndex + { + get => _selectedIndex; + set + { + _selectedIndex = value; + _selectedItem = default; + } + } + + public Optional SelectedItem + { + get => _selectedItem; + set + { + _selectedItem = value; + _selectedIndex = default; + } + } + } } } diff --git a/src/Avalonia.Controls/Selection/InternalSelectionModel.cs b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs new file mode 100644 index 0000000000..dd7b7d25cd --- /dev/null +++ b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs @@ -0,0 +1,251 @@ +using System; +using System.Collections; +using System.Collections.Specialized; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using Avalonia.Collections; + +#nullable enable + +namespace Avalonia.Controls.Selection +{ + internal class InternalSelectionModel : SelectionModel + { + private IList? _selectedItems; + private bool _ignoreModelChanges; + private bool _ignoreSelectedItemsChanges; + + public InternalSelectionModel() + { + SelectionChanged += OnSelectionChanged; + SourceReset += OnSourceReset; + } + + [AllowNull] + public new IList SelectedItems + { + get + { + if (_selectedItems is null) + { + _selectedItems = new AvaloniaList(); + SubscribeToSelectedItems(); + } + + return _selectedItems; + } + set + { + value ??= new AvaloniaList(); + + if (value.IsFixedSize) + { + throw new NotSupportedException("Cannot assign fixed size selection to SelectedItems."); + } + + if (_selectedItems != value) + { + UnsubscribeFromSelectedItems(); + _selectedItems = value; + SyncFromSelectedItems(); + SubscribeToSelectedItems(); + + if (ItemsView is null) + { + SetInitSelectedItems(value); + } + } + } + } + + private protected override void SetSource(IEnumerable? value) + { + try + { + _ignoreSelectedItemsChanges = true; + base.SetSource(value); + } + finally + { + _ignoreSelectedItemsChanges = false; + } + + SyncToSelectedItems(); + } + + private void SyncToSelectedItems() + { + if (_selectedItems is object) + { + try + { + _ignoreSelectedItemsChanges = true; + _selectedItems.Clear(); + + foreach (var i in base.SelectedItems) + { + _selectedItems.Add(i); + } + } + finally + { + _ignoreSelectedItemsChanges = false; + } + } + } + + private void SyncFromSelectedItems() + { + if (Source is null || _selectedItems is null) + { + return; + } + + try + { + _ignoreModelChanges = true; + + using (BatchUpdate()) + { + Clear(); + Add(_selectedItems); + } + } + finally + { + _ignoreModelChanges = false; + } + } + + private void SubscribeToSelectedItems() + { + if (_selectedItems is INotifyCollectionChanged incc) + { + incc.CollectionChanged += OnSelectedItemsCollectionChanged; + } + } + + private void UnsubscribeFromSelectedItems() + { + if (_selectedItems is INotifyCollectionChanged incc) + { + incc.CollectionChanged += OnSelectedItemsCollectionChanged; + } + } + + private void OnSelectionChanged(object sender, SelectionModelSelectionChangedEventArgs e) + { + if (_ignoreModelChanges) + { + return; + } + + try + { + var items = SelectedItems; + var deselected = e.DeselectedItems.ToList(); + var selected = e.SelectedItems.ToList(); + + _ignoreSelectedItemsChanges = true; + + foreach (var i in deselected) + { + items.Remove(i); + } + + foreach (var i in selected) + { + items.Add(i); + } + } + finally + { + _ignoreSelectedItemsChanges = false; + } + } + + private void OnSourceReset(object sender, EventArgs e) => SyncFromSelectedItems(); + + private void OnSelectedItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) + { + if (_ignoreSelectedItemsChanges) + { + return; + } + + if (_selectedItems == null) + { + throw new AvaloniaInternalException("CollectionChanged raised but we don't have items."); + } + + void Remove() + { + foreach (var i in e.OldItems) + { + var index = IndexOf(Source, i); + + if (index != -1) + { + Deselect(index); + } + } + } + + try + { + using var operation = BatchUpdate(); + + _ignoreModelChanges = true; + + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + Add(e.NewItems); + break; + case NotifyCollectionChangedAction.Remove: + Remove(); + break; + case NotifyCollectionChangedAction.Replace: + Remove(); + Add(e.NewItems); + break; + case NotifyCollectionChangedAction.Reset: + Clear(); + Add(_selectedItems); + break; + } + } + finally + { + _ignoreModelChanges = false; + } + } + + private void Add(IList newItems) + { + foreach (var i in newItems) + { + var index = IndexOf(Source, i); + + if (index != -1) + { + Select(index); + } + } + } + + private static int IndexOf(object? source, object? item) + { + if (source is IList l) + { + return l.IndexOf(item); + } + else if (source is ItemsSourceView v) + { + return v.IndexOf(item); + } + + return -1; + } + } +} diff --git a/src/Avalonia.Controls/Selection/SelectionModel.cs b/src/Avalonia.Controls/Selection/SelectionModel.cs index 326bd10655..fd27cb340a 100644 --- a/src/Avalonia.Controls/Selection/SelectionModel.cs +++ b/src/Avalonia.Controls/Selection/SelectionModel.cs @@ -20,8 +20,7 @@ namespace Avalonia.Controls.Selection private SelectedItems? _selectedItems; private SelectedItems.Untyped? _selectedItemsUntyped; private EventHandler? _untypedSelectionChanged; - [AllowNull] private T _initSelectedItem = default; - private bool _hasInitSelectedItem; + private IList? _initSelectedItems; public SelectionModel() { @@ -82,7 +81,19 @@ namespace Avalonia.Controls.Selection [MaybeNull, AllowNull] public T SelectedItem { - get => ItemsView is object ? GetItemAt(_selectedIndex) : _initSelectedItem; + get + { + if (ItemsView is object) + { + return GetItemAt(_selectedIndex); + } + else if (_initSelectedItems is object && _initSelectedItems.Count > 0) + { + return (T)_initSelectedItems[0]; + } + + return default; + } set { if (ItemsView is object) @@ -92,8 +103,9 @@ namespace Avalonia.Controls.Selection else { Clear(); - _initSelectedItem = value; - _hasInitSelectedItem = true; +#pragma warning disable CS8601 + SetInitSelectedItems(new T[] { value }); +#pragma warning restore CS8601 } } } @@ -102,9 +114,10 @@ namespace Avalonia.Controls.Selection { get { - if (ItemsView is null && _hasInitSelectedItem) + if (ItemsView is null && _initSelectedItems is object) { - return new[] { _initSelectedItem }; + return _initSelectedItems is IReadOnlyList i ? + i : _initSelectedItems.Cast().ToList(); } return _selectedItems ??= new SelectedItems(this); @@ -258,8 +271,7 @@ namespace Avalonia.Controls.Selection o.SelectedIndex = -1; } - _initSelectedItem = default; - _hasInitSelectedItem = false; + _initSelectedItems = null; } public void SelectAll() => SelectRange(0, int.MaxValue); @@ -270,7 +282,7 @@ namespace Avalonia.Controls.Selection PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); } - private void SetSource(IEnumerable? value) + private protected virtual void SetSource(IEnumerable? value) { if (base.Source != value) { @@ -292,11 +304,14 @@ namespace Avalonia.Controls.Selection { update.Operation.IsSourceUpdate = true; - if (_hasInitSelectedItem) + if (_initSelectedItems is object && ItemsView is object) { - SelectedItem = _initSelectedItem; - _initSelectedItem = default; - _hasInitSelectedItem = false; + foreach (T i in _initSelectedItems) + { + Select(ItemsView.IndexOf(i)); + } + + _initSelectedItems = null; } else { @@ -466,6 +481,16 @@ namespace Avalonia.Controls.Selection return true; } + private protected void SetInitSelectedItems(IList items) + { + if (Source is object) + { + throw new InvalidOperationException("Cannot set init selected items when Source is set."); + } + + _initSelectedItems = items; + } + protected override void OnSourceCollectionChangeFinished() { if (_operation is object) @@ -539,8 +564,7 @@ namespace Avalonia.Controls.Selection o.SelectedIndex = o.AnchorIndex = start; } - _initSelectedItem = default; - _hasInitSelectedItem = false; + _initSelectedItems = null; } [return: MaybeNull] diff --git a/src/Avalonia.Controls/Utils/SelectedItemsSync.cs b/src/Avalonia.Controls/Utils/SelectedItemsSync.cs deleted file mode 100644 index 83b62c7b6e..0000000000 --- a/src/Avalonia.Controls/Utils/SelectedItemsSync.cs +++ /dev/null @@ -1,283 +0,0 @@ -using System; -using System.Collections; -using System.Collections.Specialized; -using System.ComponentModel; -using System.Linq; -using Avalonia.Collections; -using Avalonia.Controls.Selection; - -#nullable enable - -namespace Avalonia.Controls.Utils -{ - /// - /// Synchronizes an with a list of SelectedItems. - /// - internal class SelectedItemsSync : IDisposable - { - private ISelectionModel _selectionModel; - private IList _selectedItems; - private bool _updatingItems; - private bool _updatingModel; - - public SelectedItemsSync(ISelectionModel model) - { - _selectionModel = model ?? throw new ArgumentNullException(nameof(model)); - _selectedItems = new AvaloniaList(); - SyncSelectedItemsWithSelectionModel(); - SubscribeToSelectedItems(_selectedItems); - SubscribeToSelectionModel(model); - } - - public ISelectionModel SelectionModel - { - get => _selectionModel; - set - { - if (_selectionModel != value) - { - value = value ?? throw new ArgumentNullException(nameof(value)); - UnsubscribeFromSelectionModel(_selectionModel); - _selectionModel = value; - SubscribeToSelectionModel(_selectionModel); - SyncSelectedItemsWithSelectionModel(); - } - } - } - - public IList SelectedItems - { - get => _selectedItems; - set - { - value ??= new AvaloniaList(); - - if (_selectedItems != value) - { - if (value.IsFixedSize) - { - throw new NotSupportedException( - "Cannot assign fixed size selection to SelectedItems."); - } - - UnsubscribeFromSelectedItems(_selectedItems); - _selectedItems = value; - SubscribeToSelectedItems(_selectedItems); - SyncSelectionModelWithSelectedItems(); - } - } - } - - public void Dispose() - { - UnsubscribeFromSelectedItems(_selectedItems); - UnsubscribeFromSelectionModel(_selectionModel); - } - - private void SyncSelectedItemsWithSelectionModel() - { - _updatingItems = true; - - try - { - _selectedItems.Clear(); - - if (_selectionModel.Source is object) - { - foreach (var i in _selectionModel.SelectedItems) - { - _selectedItems.Add(i); - } - } - } - finally - { - _updatingItems = false; - } - } - - private void SyncSelectionModelWithSelectedItems() - { - _updatingModel = true; - - try - { - if (_selectionModel.Source is object) - { - using (_selectionModel.BatchUpdate()) - { - SelectionModel.Clear(); - Add(_selectedItems); - } - } - } - finally - { - _updatingModel = false; - } - } - - private void SelectedItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) - { - if (_updatingItems) - { - return; - } - - if (_selectedItems == null) - { - throw new AvaloniaInternalException("CollectionChanged raised but we don't have items."); - } - - void Remove() - { - foreach (var i in e.OldItems) - { - var index = IndexOf(SelectionModel.Source, i); - - if (index != -1) - { - SelectionModel.Deselect(index); - } - } - } - - try - { - using var operation = SelectionModel.BatchUpdate(); - - _updatingModel = true; - - switch (e.Action) - { - case NotifyCollectionChangedAction.Add: - Add(e.NewItems); - break; - case NotifyCollectionChangedAction.Remove: - Remove(); - break; - case NotifyCollectionChangedAction.Replace: - Remove(); - Add(e.NewItems); - break; - case NotifyCollectionChangedAction.Reset: - SelectionModel.Clear(); - Add(_selectedItems); - break; - } - } - finally - { - _updatingModel = false; - } - } - - private void Add(IList newItems) - { - foreach (var i in newItems) - { - var index = IndexOf(SelectionModel.Source, i); - - if (index != -1) - { - SelectionModel.Select(index); - } - } - } - - private void SelectionModelPropertyChanged(object sender, PropertyChangedEventArgs e) - { - if (e.PropertyName == nameof(ISelectionModel.Source)) - { - if (_selectedItems.Count > 0) - { - SyncSelectionModelWithSelectedItems(); - } - else - { - SyncSelectedItemsWithSelectionModel(); - } - } - } - - private void SelectionModelSelectionChanged(object sender, SelectionModelSelectionChangedEventArgs e) - { - if (_updatingModel || _selectionModel.Source is null) - { - return; - } - - try - { - var deselected = e.DeselectedItems.ToList(); - var selected = e.SelectedItems.ToList(); - - _updatingItems = true; - - foreach (var i in deselected) - { - _selectedItems.Remove(i); - } - - foreach (var i in selected) - { - _selectedItems.Add(i); - } - } - finally - { - _updatingItems = false; - } - } - - private void SelectionModelSourceReset(object sender, EventArgs e) - { - SyncSelectionModelWithSelectedItems(); - } - - - private void SubscribeToSelectedItems(IList selectedItems) - { - if (selectedItems is INotifyCollectionChanged incc) - { - incc.CollectionChanged += SelectedItemsCollectionChanged; - } - } - - private void SubscribeToSelectionModel(ISelectionModel model) - { - model.PropertyChanged += SelectionModelPropertyChanged; - model.SelectionChanged += SelectionModelSelectionChanged; - model.SourceReset += SelectionModelSourceReset; - } - - private void UnsubscribeFromSelectedItems(IList selectedItems) - { - if (selectedItems is INotifyCollectionChanged incc) - { - incc.CollectionChanged -= SelectedItemsCollectionChanged; - } - } - - private void UnsubscribeFromSelectionModel(ISelectionModel model) - { - model.PropertyChanged -= SelectionModelPropertyChanged; - model.SelectionChanged -= SelectionModelSelectionChanged; - model.SourceReset -= SelectionModelSourceReset; - } - - private static int IndexOf(object? source, object? item) - { - if (source is IList l) - { - return l.IndexOf(item); - } - else if (source is ItemsSourceView v) - { - return v.IndexOf(item); - } - - return -1; - } - } -} diff --git a/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs new file mode 100644 index 0000000000..0f3ecef969 --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs @@ -0,0 +1,243 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.Specialized; +using Avalonia.Collections; +using Avalonia.Controls.Selection; +using Xunit; + +namespace Avalonia.Controls.UnitTests.Selection +{ + public class InternalSelectionModelTests + { + [Fact] + public void Selecting_Item_Adds_To_SelectedItems() + { + var target = CreateTarget(); + + target.Select(0); + + Assert.Equal(new[] { "foo" }, target.SelectedItems); + } + + [Fact] + public void Selecting_Duplicate_On_Model_Adds_To_SelectedItems() + { + var target = CreateTarget(source: new[] { "foo", "bar", "baz", "foo", "bar", "baz" }); + + target.SelectRange(1, 4); + + Assert.Equal(new[] { "bar", "baz", "foo", "bar" }, target.SelectedItems); + } + + [Fact] + public void Deselecting_On_Model_Removes_SelectedItem() + { + var target = CreateTarget(); + + target.SelectRange(1, 2); + target.Deselect(1); + + Assert.Equal(new[] { "baz" }, target.SelectedItems); + } + + [Fact] + public void Deselecting_Duplicate_On_Model_Removes_SelectedItem() + { + var target = CreateTarget(source: new[] { "foo", "bar", "baz", "foo", "bar", "baz" }); + + target.SelectRange(1, 2); + target.Select(4); + target.Deselect(4); + + Assert.Equal(new[] { "baz", "bar" }, target.SelectedItems); + } + + [Fact] + public void Adding_To_SelectedItems_Selects_On_Model() + { + var target = CreateTarget(); + + target.SelectRange(1, 2); + target.SelectedItems.Add("foo"); + + Assert.Equal(new[] { 0, 1, 2 }, target.SelectedIndexes); + Assert.Equal(new[] { "bar", "baz", "foo" }, target.SelectedItems); + } + + [Fact] + public void Removing_From_SelectedItems_Deselects_On_Model() + { + var target = CreateTarget(); + + target.SelectRange(1, 2); + target.SelectedItems.Remove("baz"); + + Assert.Equal(new[] { 1 }, target.SelectedIndexes); + Assert.Equal(new[] { "bar" }, target.SelectedItems); + } + + [Fact] + public void Replacing_SelectedItem_Updates_Model() + { + var target = CreateTarget(); + + target.SelectRange(1, 2); + target.SelectedItems[0] = "foo"; + + Assert.Equal(new[] { 0, 2 }, target.SelectedIndexes); + Assert.Equal(new[] { "foo", "baz" }, target.SelectedItems); + } + + [Fact] + public void Clearing_SelectedItems_Updates_Model() + { + var target = CreateTarget(); + + target.SelectedItems.Clear(); + + Assert.Empty(target.SelectedIndexes); + } + + [Fact] + public void Setting_SelectedItems_Updates_Model() + { + var target = CreateTarget(); + var oldItems = target.SelectedItems; + + var newItems = new AvaloniaList { "foo", "baz" }; + target.SelectedItems = newItems; + + Assert.Equal(new[] { 0, 2 }, target.SelectedIndexes); + Assert.Same(newItems, target.SelectedItems); + Assert.NotSame(oldItems, target.SelectedItems); + Assert.Equal(new[] { "foo", "baz" }, newItems); + } + + [Fact] + public void Setting_Items_To_Null_Clears_Selection() + { + var target = CreateTarget(); + + target.SelectRange(1, 2); + target.SelectedItems = null; + + Assert.Empty(target.SelectedIndexes); + } + + [Fact] + public void Setting_Items_To_Null_Creates_Empty_Items() + { + var target = CreateTarget(); + var oldItems = target.SelectedItems; + + target.SelectedItems = null; + + Assert.NotNull(target.SelectedItems); + Assert.NotSame(oldItems, target.SelectedItems); + Assert.IsType>(target.SelectedItems); + } + + [Fact] + public void Adds_Null_SelectedItems_When_Source_Is_Null() + { + var target = CreateTarget(nullSource: true); + + target.SelectRange(1, 2); + Assert.Equal(new object[] { null, null }, target.SelectedItems); + } + + [Fact] + public void Updates_SelectedItems_When_Source_Changes_From_Null() + { + var target = CreateTarget(nullSource: true); + + target.SelectRange(1, 2); + Assert.Equal(new object[] { null, null }, target.SelectedItems); + + target.Source = new[] { "foo", "bar", "baz" }; + Assert.Equal(new[] { "bar", "baz" }, target.SelectedItems); + } + + [Fact] + public void Updates_SelectedItems_When_Source_Changes_To_Null() + { + var target = CreateTarget(); + + target.SelectRange(1, 2); + Assert.Equal(new[] { "bar", "baz" }, target.SelectedItems); + + target.Source = null; + Assert.Equal(new object[] { null, null }, target.SelectedItems); + } + + [Fact] + public void SelectedItems_Can_Be_Set_Before_Source() + { + var target = CreateTarget(nullSource: true); + var items = new AvaloniaList { "foo", "bar", "baz" }; + var selectedItems = new AvaloniaList { "bar" }; + + target.SelectedItems = selectedItems; + target.Source = items; + + Assert.Equal(1, target.SelectedIndex); + } + + [Fact] + public void Does_Not_Accept_Fixed_Size_Items() + { + var target = CreateTarget(); + + Assert.Throws(() => + target.SelectedItems = new[] { "foo", "bar", "baz" }); + } + + [Fact] + public void Restores_Selection_On_Items_Reset() + { + var items = new ResettingCollection(new[] { "foo", "bar", "baz" }); + var target = CreateTarget(source: items); + + target.SelectedIndex = 1; + items.Reset(new[] { "baz", "foo", "bar" }); + + Assert.Equal(2, target.SelectedIndex); + } + + private static InternalSelectionModel CreateTarget( + bool singleSelect = false, + IList source = null, + bool nullSource = false) + { + source ??= !nullSource ? new[] { "foo", "bar", "baz" } : null; + + var result = new InternalSelectionModel + { + SingleSelect = singleSelect, + }; + + ((ISelectionModel)result).Source = source; + return result; + } + + private class ResettingCollection : List, INotifyCollectionChanged + { + public ResettingCollection(IEnumerable items) + { + AddRange(items); + } + + public void Reset(IEnumerable items) + { + Clear(); + AddRange(items); + CollectionChanged?.Invoke( + this, + new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + } + + public event NotifyCollectionChangedEventHandler CollectionChanged; + } + } +} diff --git a/tests/Avalonia.Controls.UnitTests/Utils/SelectedItemsSyncTests.cs b/tests/Avalonia.Controls.UnitTests/Utils/SelectedItemsSyncTests.cs deleted file mode 100644 index 3899d9dfbf..0000000000 --- a/tests/Avalonia.Controls.UnitTests/Utils/SelectedItemsSyncTests.cs +++ /dev/null @@ -1,278 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Collections.Specialized; -using Avalonia.Collections; -using Avalonia.Controls.Selection; -using Avalonia.Controls.Utils; -using Xunit; - -namespace Avalonia.Controls.UnitTests.Utils -{ - public class SelectedItemsSyncTests - { - [Fact] - public void Initial_Items_Are_From_Model() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - Assert.Equal(new[] { "bar", "baz" }, items); - } - - [Fact] - public void Selecting_On_Model_Adds_Item() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - target.SelectionModel.Select(0); - - Assert.Equal(new[] { "bar", "baz", "foo" }, items); - } - - [Fact] - public void Selecting_Duplicate_On_Model_Adds_Item() - { - var target = CreateTarget(new[] { "foo", "bar", "baz", "foo", "bar", "baz" }); - var items = target.SelectedItems; - - target.SelectionModel.Select(4); - - Assert.Equal(new[] { "bar", "baz", "bar" }, items); - } - - [Fact] - public void Deselecting_On_Model_Removes_Item() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - target.SelectionModel.Deselect(1); - - Assert.Equal(new[] { "baz" }, items); - } - - [Fact] - public void Deselecting_Duplicate_On_Model_Removes_Item() - { - var target = CreateTarget(new[] { "foo", "bar", "baz", "foo", "bar", "baz" }); - var items = target.SelectedItems; - - target.SelectionModel.Select(4); - target.SelectionModel.Deselect(4); - - Assert.Equal(new[] { "baz", "bar" }, items); - } - - [Fact] - public void Reassigning_Model_Resets_Items() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - var newModel = new SelectionModel - { - Source = (string[])target.SelectionModel.Source, - SingleSelect = false - }; - - newModel.Select(0); - newModel.Select(1); - - target.SelectionModel = newModel; - - Assert.Equal(new[] { "foo", "bar" }, items); - } - - [Fact] - public void Reassigning_Model_Tracks_New_Model() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - var newModel = new SelectionModel - { - Source = (string[])target.SelectionModel.Source, - SingleSelect = false - }; - - target.SelectionModel = newModel; - - newModel.Select(0); - newModel.Select(1); - - Assert.Equal(new[] { "foo", "bar" }, items); - } - - [Fact] - public void Adding_To_Items_Selects_On_Model() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - items.Add("foo"); - - Assert.Equal(new[] { 0, 1, 2 }, target.SelectionModel.SelectedIndexes); - Assert.Equal(new[] { "bar", "baz", "foo" }, items); - } - - [Fact] - public void Removing_From_Items_Deselects_On_Model() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - items.Remove("baz"); - - Assert.Equal(new[] { 1 }, target.SelectionModel.SelectedIndexes); - Assert.Equal(new[] { "bar" }, items); - } - - [Fact] - public void Replacing_Item_Updates_Model() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - items[0] = "foo"; - - Assert.Equal(new[] { 0, 2 }, target.SelectionModel.SelectedIndexes); - Assert.Equal(new[] { "foo", "baz" }, items); - } - - [Fact] - public void Clearing_Items_Updates_Model() - { - var target = CreateTarget(); - var items = target.SelectedItems; - - items.Clear(); - - Assert.Empty(target.SelectionModel.SelectedIndexes); - } - - [Fact] - public void Setting_Items_Updates_Model() - { - var target = CreateTarget(); - var oldItems = target.SelectedItems; - - var newItems = new AvaloniaList { "foo", "baz" }; - target.SelectedItems = newItems; - - Assert.Equal(new[] { 0, 2 }, target.SelectionModel.SelectedIndexes); - Assert.Same(newItems, target.SelectedItems); - Assert.NotSame(oldItems, target.SelectedItems); - Assert.Equal(new[] { "foo", "baz" }, newItems); - } - - [Fact] - public void Setting_Items_Subscribes_To_Model() - { - var target = CreateTarget(); - var items = new AvaloniaList { "foo", "baz" }; - - target.SelectedItems = items; - target.SelectionModel.Select(1); - - Assert.Equal(new[] { "foo", "baz", "bar" }, items); - } - - [Fact] - public void Setting_Items_To_Null_Creates_Empty_Items() - { - var target = CreateTarget(); - var oldItems = target.SelectedItems; - - target.SelectedItems = null; - - var newItems = Assert.IsType>(target.SelectedItems); - - Assert.NotSame(oldItems, newItems); - } - - [Fact] - public void Handles_Null_Model_Source() - { - var model = new SelectionModel { SingleSelect = false }; - model.Select(1); - - var target = new SelectedItemsSync(model); - var items = target.SelectedItems; - - Assert.Empty(items); - - model.Select(2); - model.Source = new[] { "foo", "bar", "baz" }; - - Assert.Equal(new[] { "bar", "baz" }, items); - } - - [Fact] - public void Does_Not_Accept_Fixed_Size_Items() - { - var target = CreateTarget(); - - Assert.Throws(() => - target.SelectedItems = new[] { "foo", "bar", "baz" }); - } - - [Fact] - public void Selected_Items_Can_Be_Set_Before_SelectionModel_Source() - { - var model = new SelectionModel(); - var target = new SelectedItemsSync(model); - var items = new AvaloniaList { "foo", "bar", "baz" }; - var selectedItems = new AvaloniaList { "bar" }; - - target.SelectedItems = selectedItems; - model.Source = items; - - Assert.Equal(1, model.SelectedIndex); - } - - [Fact] - public void Restores_Selection_On_Items_Reset() - { - var items = new ResettingCollection(new[] { "foo", "bar", "baz" }); - var model = new SelectionModel { Source = items }; - var target = new SelectedItemsSync(model); - - model.SelectedIndex = 1; - items.Reset(new[] { "baz", "foo", "bar" }); - - Assert.Equal(2, model.SelectedIndex); - } - - private static SelectedItemsSync CreateTarget( - IEnumerable items = null) - { - items ??= new[] { "foo", "bar", "baz" }; - - var model = new SelectionModel { Source = items, SingleSelect = false }; - model.SelectRange(1, 2); - - var target = new SelectedItemsSync(model); - return target; - } - - private class ResettingCollection : List, INotifyCollectionChanged - { - public ResettingCollection(IEnumerable items) - { - AddRange(items); - } - - public void Reset(IEnumerable items) - { - Clear(); - AddRange(items); - CollectionChanged?.Invoke( - this, - new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); - } - - public event NotifyCollectionChangedEventHandler CollectionChanged; - } - } -} From faaa8f28e97ed7a4f01b0e8f39a1e6539c4d2865 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 16:31:48 +0200 Subject: [PATCH 17/46] Added failing tests for #4048. --- .../Primitives/SelectingItemsControlTests.cs | 21 +++++++++++++++++++ .../Selection/InternalSelectionModelTests.cs | 11 ++++++++++ 2 files changed, 32 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index b6619aaa73..f3d0a8f94b 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -1619,6 +1619,27 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(new[] { "bar" }, target.SelectedItems); } + [Fact] + public void Preserves_SelectedItem_When_Items_Changed() + { + // Issue #4048 + var target = new SelectingItemsControl + { + Items = new[] { "foo", "bar", "baz"}, + SelectedItem = "bar", + }; + + Prepare(target); + + Assert.Equal(1, target.SelectedIndex); + Assert.Equal("bar", target.SelectedItem); + + target.Items = new[] { "qux", "foo", "bar" }; + + Assert.Equal(2, target.SelectedIndex); + Assert.Equal("bar", target.SelectedItem); + } + private static void Prepare(SelectingItemsControl target) { var root = new TestRoot diff --git a/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs index 0f3ecef969..9626bd18ef 100644 --- a/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs @@ -205,6 +205,17 @@ namespace Avalonia.Controls.UnitTests.Selection Assert.Equal(2, target.SelectedIndex); } + [Fact] + public void Preserves_Selection_On_Source_Changed() + { + var target = CreateTarget(); + + target.SelectedIndex = 1; + target.Source = new[] { "baz", "foo", "bar" }; + + Assert.Equal(2, target.SelectedIndex); + } + private static InternalSelectionModel CreateTarget( bool singleSelect = false, IList source = null, From 290f28675c54653cadfb8422e398149b19254bcf Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 16:32:03 +0200 Subject: [PATCH 18/46] Preserve selection when source changes. Fixes #4048. --- .../Selection/InternalSelectionModel.cs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Selection/InternalSelectionModel.cs b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs index dd7b7d25cd..7c2e0b912b 100644 --- a/src/Avalonia.Controls/Selection/InternalSelectionModel.cs +++ b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Collections.Specialized; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -60,6 +61,14 @@ namespace Avalonia.Controls.Selection private protected override void SetSource(IEnumerable? value) { + object?[]? oldSelection = null; + + if (Source is object && value is object) + { + oldSelection = new object?[SelectedItems.Count]; + SelectedItems.CopyTo(oldSelection, 0); + } + try { _ignoreSelectedItemsChanges = true; @@ -70,7 +79,18 @@ namespace Avalonia.Controls.Selection _ignoreSelectedItemsChanges = false; } - SyncToSelectedItems(); + if (oldSelection is null) + { + SyncToSelectedItems(); + } + else + { + foreach (var i in oldSelection) + { + var index = ItemsView!.IndexOf(i); + Select(index); + } + } } private void SyncToSelectedItems() From 517a52f988fd6f0262e19f30b20b42b4671e2c13 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 19:50:24 +0200 Subject: [PATCH 19/46] Failing tests for SelectedItems property changed events. --- .../Primitives/SelectingItemsControlTests.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index f3d0a8f94b..618f18b3ee 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -1640,6 +1640,62 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal("bar", target.SelectedItem); } + [Fact] + public void Setting_SelectedItems_Raises_PropertyChanged() + { + var target = new TestSelector + { + Items = new[] { "foo", "bar", "baz" }, + }; + + var raised = 0; + var newValue = new AvaloniaList(); + + Prepare(target); + + target.PropertyChanged += (s, e) => + { + if (e.Property == ListBox.SelectedItemsProperty) + { + Assert.Null(e.OldValue); + Assert.Same(newValue, e.NewValue); + ++raised; + } + }; + + target.SelectedItems = newValue; + + Assert.Equal(1, raised); + } + + [Fact] + public void Setting_Selection_Raises_SelectedItems_PropertyChanged() + { + var target = new TestSelector + { + Items = new[] { "foo", "bar", "baz" }, + }; + + var raised = 0; + var oldValue = target.SelectedItems; + + Prepare(target); + + target.PropertyChanged += (s, e) => + { + if (e.Property == ListBox.SelectedItemsProperty) + { + Assert.Same(oldValue, e.OldValue); + Assert.Null(e.NewValue); + ++raised; + } + }; + + target.Selection = new SelectionModel(); + + Assert.Equal(1, raised); + } + private static void Prepare(SelectingItemsControl target) { var root = new TestRoot @@ -1750,6 +1806,12 @@ namespace Avalonia.Controls.UnitTests.Primitives set => base.Selection = value; } + public new IList SelectedItems + { + get => base.SelectedItems; + set => base.SelectedItems = value; + } + public new SelectionMode SelectionMode { get => base.SelectionMode; From 9e0c92c6307961e7c6dca23bbcb343527fea0e82 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 10:22:32 +0200 Subject: [PATCH 20/46] Raise SelectedItems property changed events. --- .../Primitives/SelectingItemsControl.cs | 44 +++++++++-- .../Selection/InternalSelectionModel.cs | 40 +++++----- .../Selection/InternalSelectionModelTests.cs | 78 +++++++++---------- 3 files changed, 99 insertions(+), 63 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 3e106afdc0..1df3224fd4 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -113,6 +113,7 @@ namespace Avalonia.Controls.Primitives private ISelectionModel? _selection; private int _oldSelectedIndex; private object? _oldSelectedItem; + private IList? _oldSelectedItems; private bool _ignoreContainerSelectionChanged; private UpdateState? _updateState; @@ -149,6 +150,10 @@ namespace Avalonia.Controls.Primitives { get { + // When a Begin/EndInit/DataContext update is in place we return the value to be + // updated here, even though it's not yet active and the property changed notification + // has not yet been raised. If we don't do this then the old value will be written back + // to the source when two-way bound, and the update value will be lost. return _updateState?.SelectedIndex.HasValue == true ? _updateState.SelectedIndex.Value : Selection.SelectedIndex; @@ -173,6 +178,7 @@ namespace Avalonia.Controls.Primitives { get { + // See SelectedIndex setter for more information. return _updateState?.SelectedItem.HasValue == true ? _updateState.SelectedItem.Value : Selection.SelectedItem; @@ -202,9 +208,19 @@ namespace Avalonia.Controls.Primitives { get { - return _updateState?.SelectedItems.HasValue == true ? - _updateState.SelectedItems.Value : - (Selection as InternalSelectionModel)?.SelectedItems; + // See SelectedIndex setter for more information. + if (_updateState?.SelectedItems.HasValue == true) + { + return _updateState.SelectedItems.Value; + } + else if (Selection is InternalSelectionModel ism) + { + var result = ism.WritableSelectedItems; + _oldSelectedItems = result; + return result; + } + + return null; } set { @@ -214,7 +230,7 @@ namespace Avalonia.Controls.Primitives } else if (Selection is InternalSelectionModel i) { - i.SelectedItems = value; + i.WritableSelectedItems = value; } else { @@ -275,6 +291,15 @@ namespace Avalonia.Controls.Primitives } InitializeSelectionModel(_selection); + + if (_oldSelectedItems != SelectedItems) + { + RaisePropertyChanged( + SelectedItemsProperty, + new Optional(_oldSelectedItems), + new BindingValue(SelectedItems)); + _oldSelectedItems = SelectedItems; + } } } } @@ -651,7 +676,7 @@ namespace Avalonia.Controls.Primitives ScrollIntoView(Selection.AnchorIndex); } } - else if (e.PropertyName == nameof(ISelectionModel.SelectedIndex)) + else if (e.PropertyName == nameof(ISelectionModel.SelectedIndex) && _oldSelectedIndex != SelectedIndex) { RaisePropertyChanged(SelectedIndexProperty, _oldSelectedIndex, SelectedIndex); _oldSelectedIndex = SelectedIndex; @@ -661,6 +686,15 @@ namespace Avalonia.Controls.Primitives RaisePropertyChanged(SelectedItemProperty, _oldSelectedItem, SelectedItem); _oldSelectedItem = SelectedItem; } + else if (e.PropertyName == nameof(InternalSelectionModel.WritableSelectedItems) && + _oldSelectedItems != (Selection as InternalSelectionModel)?.SelectedItems) + { + RaisePropertyChanged( + SelectedItemsProperty, + new Optional(_oldSelectedItems), + new BindingValue(SelectedItems)); + _oldSelectedItems = SelectedItems; + } } /// diff --git a/src/Avalonia.Controls/Selection/InternalSelectionModel.cs b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs index 7c2e0b912b..a28e4b2785 100644 --- a/src/Avalonia.Controls/Selection/InternalSelectionModel.cs +++ b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs @@ -12,7 +12,7 @@ namespace Avalonia.Controls.Selection { internal class InternalSelectionModel : SelectionModel { - private IList? _selectedItems; + private IList? _writableSelectedItems; private bool _ignoreModelChanges; private bool _ignoreSelectedItemsChanges; @@ -23,17 +23,17 @@ namespace Avalonia.Controls.Selection } [AllowNull] - public new IList SelectedItems + public IList WritableSelectedItems { get { - if (_selectedItems is null) + if (_writableSelectedItems is null) { - _selectedItems = new AvaloniaList(); + _writableSelectedItems = new AvaloniaList(); SubscribeToSelectedItems(); } - return _selectedItems; + return _writableSelectedItems; } set { @@ -44,10 +44,10 @@ namespace Avalonia.Controls.Selection throw new NotSupportedException("Cannot assign fixed size selection to SelectedItems."); } - if (_selectedItems != value) + if (_writableSelectedItems != value) { UnsubscribeFromSelectedItems(); - _selectedItems = value; + _writableSelectedItems = value; SyncFromSelectedItems(); SubscribeToSelectedItems(); @@ -55,6 +55,8 @@ namespace Avalonia.Controls.Selection { SetInitSelectedItems(value); } + + RaisePropertyChanged(nameof(WritableSelectedItems)); } } } @@ -65,8 +67,8 @@ namespace Avalonia.Controls.Selection if (Source is object && value is object) { - oldSelection = new object?[SelectedItems.Count]; - SelectedItems.CopyTo(oldSelection, 0); + oldSelection = new object?[WritableSelectedItems.Count]; + WritableSelectedItems.CopyTo(oldSelection, 0); } try @@ -95,16 +97,16 @@ namespace Avalonia.Controls.Selection private void SyncToSelectedItems() { - if (_selectedItems is object) + if (_writableSelectedItems is object) { try { _ignoreSelectedItemsChanges = true; - _selectedItems.Clear(); + _writableSelectedItems.Clear(); foreach (var i in base.SelectedItems) { - _selectedItems.Add(i); + _writableSelectedItems.Add(i); } } finally @@ -116,7 +118,7 @@ namespace Avalonia.Controls.Selection private void SyncFromSelectedItems() { - if (Source is null || _selectedItems is null) + if (Source is null || _writableSelectedItems is null) { return; } @@ -128,7 +130,7 @@ namespace Avalonia.Controls.Selection using (BatchUpdate()) { Clear(); - Add(_selectedItems); + Add(_writableSelectedItems); } } finally @@ -139,7 +141,7 @@ namespace Avalonia.Controls.Selection private void SubscribeToSelectedItems() { - if (_selectedItems is INotifyCollectionChanged incc) + if (_writableSelectedItems is INotifyCollectionChanged incc) { incc.CollectionChanged += OnSelectedItemsCollectionChanged; } @@ -147,7 +149,7 @@ namespace Avalonia.Controls.Selection private void UnsubscribeFromSelectedItems() { - if (_selectedItems is INotifyCollectionChanged incc) + if (_writableSelectedItems is INotifyCollectionChanged incc) { incc.CollectionChanged += OnSelectedItemsCollectionChanged; } @@ -162,7 +164,7 @@ namespace Avalonia.Controls.Selection try { - var items = SelectedItems; + var items = WritableSelectedItems; var deselected = e.DeselectedItems.ToList(); var selected = e.SelectedItems.ToList(); @@ -193,7 +195,7 @@ namespace Avalonia.Controls.Selection return; } - if (_selectedItems == null) + if (_writableSelectedItems == null) { throw new AvaloniaInternalException("CollectionChanged raised but we don't have items."); } @@ -231,7 +233,7 @@ namespace Avalonia.Controls.Selection break; case NotifyCollectionChangedAction.Reset: Clear(); - Add(_selectedItems); + Add(_writableSelectedItems); break; } } diff --git a/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs index 9626bd18ef..b64812e290 100644 --- a/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Selection/InternalSelectionModelTests.cs @@ -11,23 +11,23 @@ namespace Avalonia.Controls.UnitTests.Selection public class InternalSelectionModelTests { [Fact] - public void Selecting_Item_Adds_To_SelectedItems() + public void Selecting_Item_Adds_To_WritableSelectedItems() { var target = CreateTarget(); target.Select(0); - Assert.Equal(new[] { "foo" }, target.SelectedItems); + Assert.Equal(new[] { "foo" }, target.WritableSelectedItems); } [Fact] - public void Selecting_Duplicate_On_Model_Adds_To_SelectedItems() + public void Selecting_Duplicate_On_Model_Adds_To_WritableSelectedItems() { var target = CreateTarget(source: new[] { "foo", "bar", "baz", "foo", "bar", "baz" }); target.SelectRange(1, 4); - Assert.Equal(new[] { "bar", "baz", "foo", "bar" }, target.SelectedItems); + Assert.Equal(new[] { "bar", "baz", "foo", "bar" }, target.WritableSelectedItems); } [Fact] @@ -38,7 +38,7 @@ namespace Avalonia.Controls.UnitTests.Selection target.SelectRange(1, 2); target.Deselect(1); - Assert.Equal(new[] { "baz" }, target.SelectedItems); + Assert.Equal(new[] { "baz" }, target.WritableSelectedItems); } [Fact] @@ -50,31 +50,31 @@ namespace Avalonia.Controls.UnitTests.Selection target.Select(4); target.Deselect(4); - Assert.Equal(new[] { "baz", "bar" }, target.SelectedItems); + Assert.Equal(new[] { "baz", "bar" }, target.WritableSelectedItems); } [Fact] - public void Adding_To_SelectedItems_Selects_On_Model() + public void Adding_To_WritableSelectedItems_Selects_On_Model() { var target = CreateTarget(); target.SelectRange(1, 2); - target.SelectedItems.Add("foo"); + target.WritableSelectedItems.Add("foo"); Assert.Equal(new[] { 0, 1, 2 }, target.SelectedIndexes); - Assert.Equal(new[] { "bar", "baz", "foo" }, target.SelectedItems); + Assert.Equal(new[] { "bar", "baz", "foo" }, target.WritableSelectedItems); } [Fact] - public void Removing_From_SelectedItems_Deselects_On_Model() + public void Removing_From_WritableSelectedItems_Deselects_On_Model() { var target = CreateTarget(); target.SelectRange(1, 2); - target.SelectedItems.Remove("baz"); + target.WritableSelectedItems.Remove("baz"); Assert.Equal(new[] { 1 }, target.SelectedIndexes); - Assert.Equal(new[] { "bar" }, target.SelectedItems); + Assert.Equal(new[] { "bar" }, target.WritableSelectedItems); } [Fact] @@ -83,34 +83,34 @@ namespace Avalonia.Controls.UnitTests.Selection var target = CreateTarget(); target.SelectRange(1, 2); - target.SelectedItems[0] = "foo"; + target.WritableSelectedItems[0] = "foo"; Assert.Equal(new[] { 0, 2 }, target.SelectedIndexes); - Assert.Equal(new[] { "foo", "baz" }, target.SelectedItems); + Assert.Equal(new[] { "foo", "baz" }, target.WritableSelectedItems); } [Fact] - public void Clearing_SelectedItems_Updates_Model() + public void Clearing_WritableSelectedItems_Updates_Model() { var target = CreateTarget(); - target.SelectedItems.Clear(); + target.WritableSelectedItems.Clear(); Assert.Empty(target.SelectedIndexes); } [Fact] - public void Setting_SelectedItems_Updates_Model() + public void Setting_WritableSelectedItems_Updates_Model() { var target = CreateTarget(); - var oldItems = target.SelectedItems; + var oldItems = target.WritableSelectedItems; var newItems = new AvaloniaList { "foo", "baz" }; - target.SelectedItems = newItems; + target.WritableSelectedItems = newItems; Assert.Equal(new[] { 0, 2 }, target.SelectedIndexes); - Assert.Same(newItems, target.SelectedItems); - Assert.NotSame(oldItems, target.SelectedItems); + Assert.Same(newItems, target.WritableSelectedItems); + Assert.NotSame(oldItems, target.WritableSelectedItems); Assert.Equal(new[] { "foo", "baz" }, newItems); } @@ -120,7 +120,7 @@ namespace Avalonia.Controls.UnitTests.Selection var target = CreateTarget(); target.SelectRange(1, 2); - target.SelectedItems = null; + target.WritableSelectedItems = null; Assert.Empty(target.SelectedIndexes); } @@ -129,56 +129,56 @@ namespace Avalonia.Controls.UnitTests.Selection public void Setting_Items_To_Null_Creates_Empty_Items() { var target = CreateTarget(); - var oldItems = target.SelectedItems; + var oldItems = target.WritableSelectedItems; - target.SelectedItems = null; + target.WritableSelectedItems = null; - Assert.NotNull(target.SelectedItems); - Assert.NotSame(oldItems, target.SelectedItems); - Assert.IsType>(target.SelectedItems); + Assert.NotNull(target.WritableSelectedItems); + Assert.NotSame(oldItems, target.WritableSelectedItems); + Assert.IsType>(target.WritableSelectedItems); } [Fact] - public void Adds_Null_SelectedItems_When_Source_Is_Null() + public void Adds_Null_WritableSelectedItems_When_Source_Is_Null() { var target = CreateTarget(nullSource: true); target.SelectRange(1, 2); - Assert.Equal(new object[] { null, null }, target.SelectedItems); + Assert.Equal(new object[] { null, null }, target.WritableSelectedItems); } [Fact] - public void Updates_SelectedItems_When_Source_Changes_From_Null() + public void Updates_WritableSelectedItems_When_Source_Changes_From_Null() { var target = CreateTarget(nullSource: true); target.SelectRange(1, 2); - Assert.Equal(new object[] { null, null }, target.SelectedItems); + Assert.Equal(new object[] { null, null }, target.WritableSelectedItems); target.Source = new[] { "foo", "bar", "baz" }; - Assert.Equal(new[] { "bar", "baz" }, target.SelectedItems); + Assert.Equal(new[] { "bar", "baz" }, target.WritableSelectedItems); } [Fact] - public void Updates_SelectedItems_When_Source_Changes_To_Null() + public void Updates_WritableSelectedItems_When_Source_Changes_To_Null() { var target = CreateTarget(); target.SelectRange(1, 2); - Assert.Equal(new[] { "bar", "baz" }, target.SelectedItems); + Assert.Equal(new[] { "bar", "baz" }, target.WritableSelectedItems); target.Source = null; - Assert.Equal(new object[] { null, null }, target.SelectedItems); + Assert.Equal(new object[] { null, null }, target.WritableSelectedItems); } [Fact] - public void SelectedItems_Can_Be_Set_Before_Source() + public void WritableSelectedItems_Can_Be_Set_Before_Source() { var target = CreateTarget(nullSource: true); var items = new AvaloniaList { "foo", "bar", "baz" }; - var selectedItems = new AvaloniaList { "bar" }; + var WritableSelectedItems = new AvaloniaList { "bar" }; - target.SelectedItems = selectedItems; + target.WritableSelectedItems = WritableSelectedItems; target.Source = items; Assert.Equal(1, target.SelectedIndex); @@ -190,7 +190,7 @@ namespace Avalonia.Controls.UnitTests.Selection var target = CreateTarget(); Assert.Throws(() => - target.SelectedItems = new[] { "foo", "bar", "baz" }); + target.WritableSelectedItems = new[] { "foo", "bar", "baz" }); } [Fact] From e240b0d3228050870aa1f65ebb8e553bb3d7227e Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 11:26:40 +0200 Subject: [PATCH 21/46] Added sandbox app. This is a blank app for use in repros, experiments and perf testing. Please don't commit changes here. --- Avalonia.sln | 27 +++++++++++++++++++++++++++ samples/Sandbox/App.axaml | 8 ++++++++ samples/Sandbox/App.axaml.cs | 22 ++++++++++++++++++++++ samples/Sandbox/MainWindow.axaml | 4 ++++ samples/Sandbox/MainWindow.axaml.cs | 20 ++++++++++++++++++++ samples/Sandbox/Program.cs | 17 +++++++++++++++++ samples/Sandbox/Sandbox.csproj | 18 ++++++++++++++++++ 7 files changed, 116 insertions(+) create mode 100644 samples/Sandbox/App.axaml create mode 100644 samples/Sandbox/App.axaml.cs create mode 100644 samples/Sandbox/MainWindow.axaml create mode 100644 samples/Sandbox/MainWindow.axaml.cs create mode 100644 samples/Sandbox/Program.cs create mode 100644 samples/Sandbox/Sandbox.csproj diff --git a/Avalonia.sln b/Avalonia.sln index ddcd61408d..ffcf266b0c 100644 --- a/Avalonia.sln +++ b/Avalonia.sln @@ -226,6 +226,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.ReactiveUI.Events" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.ReactiveUI.Events.UnitTests", "tests\Avalonia.ReactiveUI.Events.UnitTests\Avalonia.ReactiveUI.Events.UnitTests.csproj", "{780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Sandbox", "samples\Sandbox\Sandbox.csproj", "{11BE52AF-E2DD-4CF0-B19A-05285ACAF571}" +EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution src\Shared\RenderHelpers\RenderHelpers.projitems*{3c4c0cb4-0c0f-4450-a37b-148c84ff905f}*SharedItemsImports = 13 @@ -2064,6 +2066,30 @@ Global {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhone.Build.0 = Release|Any CPU {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhoneSimulator.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|Any CPU.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|Any CPU.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|iPhone.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|iPhone.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|iPhoneSimulator.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|iPhoneSimulator.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.AppStore|Any CPU.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.AppStore|Any CPU.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.AppStore|iPhone.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.AppStore|iPhone.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.AppStore|iPhoneSimulator.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.AppStore|iPhoneSimulator.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Debug|Any CPU.Build.0 = Debug|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Debug|iPhone.ActiveCfg = Debug|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Debug|iPhone.Build.0 = Debug|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Release|Any CPU.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Release|Any CPU.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Release|iPhone.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Release|iPhone.Build.0 = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Release|iPhoneSimulator.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -2123,6 +2149,7 @@ Global {3C84E04B-36CF-4D0D-B965-C26DD649D1F3} = {A0CC0258-D18C-4AB3-854F-7101680FC3F9} {909A8CBD-7D0E-42FD-B841-022AD8925820} = {8B6A8209-894F-4BA1-B880-965FD453982C} {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7} = {C5A00AC3-B34C-4564-9BDD-2DA473EF4D8B} + {11BE52AF-E2DD-4CF0-B19A-05285ACAF571} = {9B9E3891-2366-4253-A952-D08BCEB71098} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {87366D66-1391-4D90-8999-95A620AD786A} diff --git a/samples/Sandbox/App.axaml b/samples/Sandbox/App.axaml new file mode 100644 index 0000000000..699781eb94 --- /dev/null +++ b/samples/Sandbox/App.axaml @@ -0,0 +1,8 @@ + + + + + diff --git a/samples/Sandbox/App.axaml.cs b/samples/Sandbox/App.axaml.cs new file mode 100644 index 0000000000..7eb8345784 --- /dev/null +++ b/samples/Sandbox/App.axaml.cs @@ -0,0 +1,22 @@ +using Avalonia; +using Avalonia.Controls.ApplicationLifetimes; +using Avalonia.Markup.Xaml; + +namespace Sandbox +{ + public class App : Application + { + public override void Initialize() + { + AvaloniaXamlLoader.Load(this); + } + + public override void OnFrameworkInitializationCompleted() + { + if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktopLifetime) + { + desktopLifetime.MainWindow = new MainWindow(); + } + } + } +} diff --git a/samples/Sandbox/MainWindow.axaml b/samples/Sandbox/MainWindow.axaml new file mode 100644 index 0000000000..6929f192c7 --- /dev/null +++ b/samples/Sandbox/MainWindow.axaml @@ -0,0 +1,4 @@ + + diff --git a/samples/Sandbox/MainWindow.axaml.cs b/samples/Sandbox/MainWindow.axaml.cs new file mode 100644 index 0000000000..b7222e043d --- /dev/null +++ b/samples/Sandbox/MainWindow.axaml.cs @@ -0,0 +1,20 @@ +using Avalonia; +using Avalonia.Controls; +using Avalonia.Markup.Xaml; + +namespace Sandbox +{ + public class MainWindow : Window + { + public MainWindow() + { + this.InitializeComponent(); + this.AttachDevTools(); + } + + private void InitializeComponent() + { + AvaloniaXamlLoader.Load(this); + } + } +} diff --git a/samples/Sandbox/Program.cs b/samples/Sandbox/Program.cs new file mode 100644 index 0000000000..4d7eda8d9f --- /dev/null +++ b/samples/Sandbox/Program.cs @@ -0,0 +1,17 @@ +using Avalonia; +using Avalonia.ReactiveUI; + +namespace Sandbox +{ + public class Program + { + static void Main(string[] args) + { + AppBuilder.Configure() + .UsePlatformDetect() + .UseReactiveUI() + .LogToDebug() + .StartWithClassicDesktopLifetime(args); + } + } +} diff --git a/samples/Sandbox/Sandbox.csproj b/samples/Sandbox/Sandbox.csproj new file mode 100644 index 0000000000..1a0a8a7ce5 --- /dev/null +++ b/samples/Sandbox/Sandbox.csproj @@ -0,0 +1,18 @@ + + + + WinExe + netcoreapp3.1 + true + + + + + + + + + + + + From 14f314341ed61f2bf66fb79f9be57b3b51c8766b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 12:21:13 +0200 Subject: [PATCH 22/46] Fix scrolling to selection. A few `AutoScrollToSelectedItem` improvements: - Scroll to current selected item when it's set to true - Scroll to current selected item when list first displayed - Scroll to current selected item when attached to visual tree if the selection was changed while it wasn't attached Fixes #4100 --- src/Avalonia.Controls/ListBox.cs | 1 + .../Primitives/SelectingItemsControl.cs | 48 ++++++- .../Primitives/SelectingItemsControlTests.cs | 119 +++++++++++++++++- 3 files changed, 162 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Controls/ListBox.cs b/src/Avalonia.Controls/ListBox.cs index f7e86d697a..d1b8038581 100644 --- a/src/Avalonia.Controls/ListBox.cs +++ b/src/Avalonia.Controls/ListBox.cs @@ -163,6 +163,7 @@ namespace Avalonia.Controls protected override void OnApplyTemplate(TemplateAppliedEventArgs e) { + base.OnApplyTemplate(e); Scroll = e.NameScope.Find("PART_ScrollViewer"); } } diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 1df3224fd4..b07bd13850 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -116,6 +116,7 @@ namespace Avalonia.Controls.Primitives private IList? _oldSelectedItems; private bool _ignoreContainerSelectionChanged; private UpdateState? _updateState; + private bool _hasScrolledToSelectedItem; /// /// Initializes static members of the class. @@ -381,6 +382,28 @@ namespace Avalonia.Controls.Primitives } } + protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e) + { + base.OnAttachedToVisualTree(e); + AutoScrollToSelectedItemIfNecessary(); + } + + protected override void OnApplyTemplate(TemplateAppliedEventArgs e) + { + base.OnApplyTemplate(e); + + void ExecuteScrollWhenLayoutUpdated(object sender, EventArgs e) + { + LayoutUpdated -= ExecuteScrollWhenLayoutUpdated; + AutoScrollToSelectedItemIfNecessary(); + } + + if (AutoScrollToSelectedItem) + { + LayoutUpdated += ExecuteScrollWhenLayoutUpdated; + } + } + /// protected override void OnContainersMaterialized(ItemContainerEventArgs e) { @@ -481,6 +504,10 @@ namespace Avalonia.Controls.Primitives { base.OnPropertyChanged(change); + if (change.Property == AutoScrollToSelectedItemProperty) + { + AutoScrollToSelectedItemIfNecessary(); + } if (change.Property == ItemsProperty && _updateState is null && _selection is object) { var newValue = change.NewValue.GetValueOrDefault(); @@ -669,12 +696,10 @@ namespace Avalonia.Controls.Primitives /// The event args. private void OnSelectionModelPropertyChanged(object sender, PropertyChangedEventArgs e) { - if (e.PropertyName == nameof(ISelectionModel.AnchorIndex) && AutoScrollToSelectedItem) + if (e.PropertyName == nameof(ISelectionModel.AnchorIndex)) { - if (Selection.AnchorIndex > 0) - { - ScrollIntoView(Selection.AnchorIndex); - } + _hasScrolledToSelectedItem = false; + AutoScrollToSelectedItemIfNecessary(); } else if (e.PropertyName == nameof(ISelectionModel.SelectedIndex) && _oldSelectedIndex != SelectedIndex) { @@ -751,6 +776,19 @@ namespace Avalonia.Controls.Primitives } } + private void AutoScrollToSelectedItemIfNecessary() + { + if (AutoScrollToSelectedItem && + !_hasScrolledToSelectedItem && + Presenter is object && + Selection.AnchorIndex > 0 && + ((IVisual)this).IsAttachedToVisualTree) + { + ScrollIntoView(Selection.AnchorIndex); + _hasScrolledToSelectedItem = true; + } + } + /// /// Called when a container raises the . /// diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index 618f18b3ee..192d6e0286 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -1402,12 +1402,36 @@ namespace Avalonia.Controls.UnitTests.Primitives Items = items, }; + var raised = false; + Prepare(target); + target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => raised = true); + target.SelectedIndex = 2; + + Assert.True(raised); + } + + [Fact] + public void AutoScrollToSelectedItem_Causes_Scroll_To_Initial_SelectedItem() + { + var items = new ObservableCollection + { + "Foo", + "Bar", + "Baz" + }; + + var target = new ListBox + { + Template = Template(), + Items = items, + }; var raised = false; - target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => raised = true); + target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => raised = true); target.SelectedIndex = 2; + Prepare(target); Assert.True(raised); } @@ -1451,6 +1475,99 @@ namespace Avalonia.Controls.UnitTests.Primitives } } + [Fact] + public void AutoScrollToSelectedItem_Scrolls_When_Reattached_To_Visual_Tree_If_Selection_Changed_While_Detached_From_Visual_Tree() + { + var items = new ObservableCollection + { + "Foo", + "Bar", + "Baz" + }; + + var target = new ListBox + { + Template = Template(), + Items = items, + SelectedIndex = 2, + }; + + var raised = false; + + Prepare(target); + + var root = (TestRoot)target.Parent; + + target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => raised = true); + + root.Child = null; + target.SelectedIndex = 1; + root.Child = target; + + Assert.True(raised); + } + + [Fact] + public void AutoScrollToSelectedItem_Doesnt_Scroll_If_Reattached_To_Visual_Tree_With_No_Selection_Change() + { + var items = new ObservableCollection + { + "Foo", + "Bar", + "Baz" + }; + + var target = new ListBox + { + Template = Template(), + Items = items, + SelectedIndex = 2, + }; + + var raised = false; + + Prepare(target); + + var root = (TestRoot)target.Parent; + + target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => raised = true); + + root.Child = null; + root.Child = target; + + Assert.False(raised); + } + + [Fact] + public void AutoScrollToSelectedItem_Causes_Scroll_When_Turned_On() + { + var items = new ObservableCollection + { + "Foo", + "Bar", + "Baz" + }; + + var target = new ListBox + { + Template = Template(), + Items = items, + AutoScrollToSelectedItem = false, + }; + + Prepare(target); + + var raised = false; + target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => raised = true); + target.SelectedIndex = 2; + + Assert.False(raised); + + target.AutoScrollToSelectedItem = true; + + Assert.True(raised); + } + [Fact] public void Can_Set_Both_SelectedItem_And_SelectedItems_During_Initialization() { From aaeda72aec0bb8a0e84eb0e03310f93571299233 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 12:21:46 +0200 Subject: [PATCH 23/46] Removed unused methods. --- .../Primitives/SelectingItemsControl.cs | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index b07bd13850..3b089921ef 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -849,14 +849,6 @@ namespace Avalonia.Controls.Primitives } } - private void MarkContainersUnselected() - { - foreach (var container in ItemContainerGenerator.Containers) - { - MarkContainerSelected(container.ContainerControl, false); - } - } - /// /// Sets an item container's 'selected' class or . /// @@ -872,23 +864,6 @@ namespace Avalonia.Controls.Primitives } } - /// - /// Sets an item container's 'selected' class or . - /// - /// The item. - /// Whether the item should be selected or deselected. - private int MarkItemSelected(object item, bool selected) - { - var index = IndexOf(Items, item); - - if (index != -1) - { - MarkItemSelected(index, selected); - } - - return index; - } - private void UpdateContainerSelection() { if (Presenter?.Panel is IPanel panel) From 1b4fe4f5632b3ddb47bec68d43dcaa4e97675cbc Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 13:55:59 +0200 Subject: [PATCH 24/46] Refactor ListBoxPage. - Make the list box fill available space - `SelectionMode` is a `[Flags]` enum so a combo box didn't make sense - Add check for `AutoScrollToSelectedItem` --- samples/ControlCatalog/MainView.xaml | 5 +- samples/ControlCatalog/Pages/ListBoxPage.xaml | 50 ++++++++---------- .../ViewModels/ListBoxPageViewModel.cs | 51 ++++++++++++++----- 3 files changed, 63 insertions(+), 43 deletions(-) diff --git a/samples/ControlCatalog/MainView.xaml b/samples/ControlCatalog/MainView.xaml index efc90357ed..790813fda0 100644 --- a/samples/ControlCatalog/MainView.xaml +++ b/samples/ControlCatalog/MainView.xaml @@ -45,7 +45,10 @@ - + + + diff --git a/samples/ControlCatalog/Pages/ListBoxPage.xaml b/samples/ControlCatalog/Pages/ListBoxPage.xaml index edf3d41bf5..3521ad71a9 100644 --- a/samples/ControlCatalog/Pages/ListBoxPage.xaml +++ b/samples/ControlCatalog/Pages/ListBoxPage.xaml @@ -1,35 +1,25 @@ - - ListBox - Hosts a collection of ListBoxItem. - - - - - - - - - - - - - Single - Multiple - Toggle - AlwaysSelected - - + + + ListBox + Hosts a collection of ListBoxItem. - + + Multiple + Toggle + AlwaysSelected + AutoScrollToSelectedItem + + + + + + + + diff --git a/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs b/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs index d088576998..a963e8b6eb 100644 --- a/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs +++ b/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs @@ -10,15 +10,30 @@ namespace ControlCatalog.ViewModels { public class ListBoxPageViewModel : ReactiveObject { + private bool _multiple; + private bool _toggle; + private bool _alwaysSelected; + private bool _autoScrollToSelectedItem = true; private int _counter; - private SelectionMode _selectionMode; + private ObservableAsPropertyHelper _selectionMode; public ListBoxPageViewModel() { Items = new ObservableCollection(Enumerable.Range(1, 10000).Select(i => GenerateItem())); + Selection = new SelectionModel(); Selection.Select(1); + _selectionMode = this.WhenAnyValue( + x => x.Multiple, + x => x.Toggle, + x => x.AlwaysSelected, + (m, t, a) => + (m ? SelectionMode.Multiple : 0) | + (t ? SelectionMode.Toggle : 0) | + (a ? SelectionMode.AlwaysSelected : 0)) + .ToProperty(this, x => x.SelectionMode); + AddItemCommand = ReactiveCommand.Create(() => Items.Add(GenerateItem())); RemoveItemCommand = ReactiveCommand.Create(() => @@ -42,25 +57,37 @@ namespace ControlCatalog.ViewModels } public ObservableCollection Items { get; } - public SelectionModel Selection { get; } + public SelectionMode SelectionMode => _selectionMode.Value; - public ReactiveCommand AddItemCommand { get; } + public bool Multiple + { + get => _multiple; + set => this.RaiseAndSetIfChanged(ref _multiple, value); + } - public ReactiveCommand RemoveItemCommand { get; } + public bool Toggle + { + get => _toggle; + set => this.RaiseAndSetIfChanged(ref _toggle, value); + } - public ReactiveCommand SelectRandomItemCommand { get; } + public bool AlwaysSelected + { + get => _alwaysSelected; + set => this.RaiseAndSetIfChanged(ref _alwaysSelected, value); + } - public SelectionMode SelectionMode + public bool AutoScrollToSelectedItem { - get => _selectionMode; - set - { - Selection.Clear(); - this.RaiseAndSetIfChanged(ref _selectionMode, value); - } + get => _autoScrollToSelectedItem; + set => this.RaiseAndSetIfChanged(ref _autoScrollToSelectedItem, value); } + public ReactiveCommand AddItemCommand { get; } + public ReactiveCommand RemoveItemCommand { get; } + public ReactiveCommand SelectRandomItemCommand { get; } + private string GenerateItem() => $"Item {_counter++.ToString()}"; } } From 172feab259256061170894ab709a112a100bbbcb Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 14:01:18 +0200 Subject: [PATCH 25/46] Fix removing selected items in ListBoxPage. Previously, would remove all items if `AlwaysSelected` enabled. --- samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs b/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs index a963e8b6eb..f75bc32105 100644 --- a/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs +++ b/samples/ControlCatalog/ViewModels/ListBoxPageViewModel.cs @@ -38,9 +38,11 @@ namespace ControlCatalog.ViewModels RemoveItemCommand = ReactiveCommand.Create(() => { - while (Selection.Count > 0) + var items = Selection.SelectedItems.ToList(); + + foreach (var item in items) { - Items.Remove(Selection.SelectedItems.First()); + Items.Remove(item); } }); From 293564e7049bbd07a9ba5194415921d52f739a01 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 14:44:59 +0200 Subject: [PATCH 26/46] Off-by-one error. --- src/Avalonia.Controls/Primitives/SelectingItemsControl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 3b089921ef..c954f9fd4a 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -781,7 +781,7 @@ namespace Avalonia.Controls.Primitives if (AutoScrollToSelectedItem && !_hasScrolledToSelectedItem && Presenter is object && - Selection.AnchorIndex > 0 && + Selection.AnchorIndex >= 0 && ((IVisual)this).IsAttachedToVisualTree) { ScrollIntoView(Selection.AnchorIndex); From ffcc0c1156a5e368b9262c9430438e89faf06bdf Mon Sep 17 00:00:00 2001 From: Giuseppe Lippolis Date: Mon, 14 Sep 2020 12:56:52 +0200 Subject: [PATCH 27/46] Allow ScrollViewer to scroll the content up, down, left and right by one page. --- src/Avalonia.Controls/ScrollViewer.cs | 36 +++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/ScrollViewer.cs b/src/Avalonia.Controls/ScrollViewer.cs index 4600301410..6b75149d62 100644 --- a/src/Avalonia.Controls/ScrollViewer.cs +++ b/src/Avalonia.Controls/ScrollViewer.cs @@ -448,6 +448,38 @@ namespace Avalonia.Controls Offset += new Vector(_smallChange.Width, 0); } + /// + /// Scrolls the content upward by one page. + /// + public void PageUp() + { + VerticalScrollBarValue = Math.Max(_offset.Y - _viewport.Height, 0); + } + + /// + /// Scrolls the content downward by one page. + /// + public void PageDown() + { + VerticalScrollBarValue = Math.Min(_offset.Y + _viewport.Height, VerticalScrollBarMaximum); + } + + /// + /// Scrolls the content left by one page. + /// + public void PageLeft() + { + HorizontalScrollBarValue = Math.Max(_offset.X - _viewport.Width, 0); + } + + /// + /// Scrolls the content tight by one page. + /// + public void PageRight() + { + HorizontalScrollBarValue = Math.Min(_offset.X + _viewport.Width, HorizontalScrollBarMaximum); + } + /// /// Scrolls to the top-left corner of the content. /// @@ -623,12 +655,12 @@ namespace Avalonia.Controls { if (e.Key == Key.PageUp) { - VerticalScrollBarValue = Math.Max(_offset.Y - _viewport.Height, 0); + PageUp(); e.Handled = true; } else if (e.Key == Key.PageDown) { - VerticalScrollBarValue = Math.Min(_offset.Y + _viewport.Height, VerticalScrollBarMaximum); + PageDown(); e.Handled = true; } } From 6992bd72ceabe81dc58f0af82186e53c1575dd2d Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Thu, 17 Sep 2020 00:37:56 -0400 Subject: [PATCH 28/46] Add Sorting event and use ListSortDirection instead of bool property --- .../Collections/DataGridSortDescription.cs | 42 +++++--- src/Avalonia.Controls.DataGrid/DataGrid.cs | 5 + .../DataGridColumn.cs | 2 +- .../DataGridColumnHeader.cs | 97 ++++++++++--------- .../DataGridColumns.cs | 5 + src/Avalonia.Controls.DataGrid/EventArgs.cs | 4 +- 6 files changed, 91 insertions(+), 64 deletions(-) diff --git a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs index f741d40571..ffb36d187d 100644 --- a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs +++ b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs @@ -1,19 +1,21 @@ using System; using System.Collections; using System.Collections.Generic; +using System.ComponentModel; using System.Globalization; using System.Linq; -using System.Text; -using Avalonia.Controls; using Avalonia.Controls.Utils; -using Avalonia.Utilities; namespace Avalonia.Collections { public abstract class DataGridSortDescription { public virtual string PropertyPath => null; - public virtual bool Descending => false; + + [Obsolete("Use Direction property to read or override sorting direction.")] + public virtual bool Descending => Direction == ListSortDirection.Descending; + + public virtual ListSortDirection Direction => ListSortDirection.Ascending; public bool HasPropertyPath => !String.IsNullOrEmpty(PropertyPath); public abstract IComparer Comparer { get; } @@ -105,7 +107,7 @@ namespace Avalonia.Collections private class DataGridPathSortDescription : DataGridSortDescription { - private readonly bool _descending; + private readonly ListSortDirection _direction; private readonly string _propertyPath; private readonly Lazy _cultureSensitiveComparer; private readonly Lazy> _comparer; @@ -130,19 +132,19 @@ namespace Avalonia.Collections public override string PropertyPath => _propertyPath; public override IComparer Comparer => _comparer.Value; - public override bool Descending => _descending; + public override ListSortDirection Direction => _direction; - public DataGridPathSortDescription(string propertyPath, bool descending, CultureInfo culture) + public DataGridPathSortDescription(string propertyPath, ListSortDirection direction, CultureInfo culture) { _propertyPath = propertyPath; - _descending = descending; + _direction = direction; _cultureSensitiveComparer = new Lazy(() => new CultureSensitiveComparer(culture ?? CultureInfo.CurrentCulture)); _comparer = new Lazy>(() => Comparer.Create((x, y) => Compare(x, y))); } - private DataGridPathSortDescription(DataGridPathSortDescription inner, bool descending) + private DataGridPathSortDescription(DataGridPathSortDescription inner, ListSortDirection direction) { _propertyPath = inner._propertyPath; - _descending = descending; + _direction = direction; _propertyType = inner._propertyType; _cultureSensitiveComparer = inner._cultureSensitiveComparer; _internalComparer = inner._internalComparer; @@ -201,7 +203,7 @@ namespace Avalonia.Collections result = _internalComparer?.Compare(v1, v2) ?? 0; - if (_descending) + if (Direction == ListSortDirection.Descending) return -result; else return result; @@ -218,7 +220,7 @@ namespace Avalonia.Collections } public override IOrderedEnumerable OrderBy(IEnumerable seq) { - if(_descending) + if (Direction == ListSortDirection.Descending) { return seq.OrderByDescending(o => GetValue(o), InternalComparer); } @@ -229,7 +231,7 @@ namespace Avalonia.Collections } public override IOrderedEnumerable ThenBy(IOrderedEnumerable seq) { - if (_descending) + if (Direction == ListSortDirection.Descending) { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } @@ -241,13 +243,21 @@ namespace Avalonia.Collections internal override DataGridSortDescription SwitchSortDirection() { - return new DataGridPathSortDescription(this, !_descending); + var newDirection = _direction == ListSortDirection.Ascending ? ListSortDirection.Descending : ListSortDirection.Ascending; + return new DataGridPathSortDescription(this, newDirection); } } - public static DataGridSortDescription FromPath(string propertyPath, bool descending = false, CultureInfo culture = null) + public static DataGridSortDescription FromPath(string propertyPath, ListSortDirection direction = ListSortDirection.Ascending, CultureInfo culture = null) + { + return new DataGridPathSortDescription(propertyPath, direction, culture); + } + + + [Obsolete("Use overload taking a ListSortDirection.")] + public static DataGridSortDescription FromPath(string propertyPath, bool descending, CultureInfo culture = null) { - return new DataGridPathSortDescription(propertyPath, descending, culture); + return new DataGridPathSortDescription(propertyPath, descending ? ListSortDirection.Descending : ListSortDirection.Ascending, culture); } } diff --git a/src/Avalonia.Controls.DataGrid/DataGrid.cs b/src/Avalonia.Controls.DataGrid/DataGrid.cs index f7903086ab..9ca0b91523 100644 --- a/src/Avalonia.Controls.DataGrid/DataGrid.cs +++ b/src/Avalonia.Controls.DataGrid/DataGrid.cs @@ -1229,6 +1229,11 @@ namespace Avalonia.Controls remove { AddHandler(SelectionChangedEvent, value); } } + /// + /// Occurs when the sorting request is triggered. + /// + public event EventHandler Sorting; + /// /// Occurs when a /// object becomes available for reuse. diff --git a/src/Avalonia.Controls.DataGrid/DataGridColumn.cs b/src/Avalonia.Controls.DataGrid/DataGridColumn.cs index 128fbde0c1..23c4acdf6c 100644 --- a/src/Avalonia.Controls.DataGrid/DataGridColumn.cs +++ b/src/Avalonia.Controls.DataGrid/DataGridColumn.cs @@ -1047,4 +1047,4 @@ namespace Avalonia.Controls } -} \ No newline at end of file +} diff --git a/src/Avalonia.Controls.DataGrid/DataGridColumnHeader.cs b/src/Avalonia.Controls.DataGrid/DataGridColumnHeader.cs index 017718bc92..25aae99942 100644 --- a/src/Avalonia.Controls.DataGrid/DataGridColumnHeader.cs +++ b/src/Avalonia.Controls.DataGrid/DataGridColumnHeader.cs @@ -161,13 +161,14 @@ namespace Avalonia.Controls var sort = OwningColumn.GetSortDescription(); if (sort != null) { - CurrentSortingState = sort.Descending ? ListSortDirection.Descending : ListSortDirection.Ascending; + CurrentSortingState = sort.Direction; } } + PseudoClasses.Set(":sortascending", - CurrentSortingState.HasValue && CurrentSortingState.Value == ListSortDirection.Ascending); + CurrentSortingState == ListSortDirection.Ascending); PseudoClasses.Set(":sortdescending", - CurrentSortingState.HasValue && CurrentSortingState.Value == ListSortDirection.Descending); + CurrentSortingState == ListSortDirection.Descending); } internal void UpdateSeparatorVisibility(DataGridColumn lastVisibleColumn) @@ -215,70 +216,76 @@ namespace Avalonia.Controls internal void ProcessSort(KeyModifiers keyModifiers) { // if we can sort: - // - DataConnection.AllowSort is true, and // - AllowUserToSortColumns and CanSort are true, and - // - OwningColumn is bound, and - // - SortDescriptionsCollection exists, and - // - the column's data type is comparable + // - OwningColumn is bound // then try to sort if (OwningColumn != null && OwningGrid != null && OwningGrid.EditingRow == null && OwningColumn != OwningGrid.ColumnsInternal.FillerColumn - && OwningGrid.DataConnection.AllowSort && OwningGrid.CanUserSortColumns - && OwningColumn.CanUserSort - && OwningGrid.DataConnection.SortDescriptions != null) + && OwningColumn.CanUserSort) { - DataGrid owningGrid = OwningGrid; + var ea = new DataGridColumnEventArgs(OwningColumn); + OwningGrid.OnColumnSorting(ea); - DataGridSortDescription newSort; + if (!ea.Handled && OwningGrid.DataConnection.AllowSort && OwningGrid.DataConnection.SortDescriptions != null) + { + // - DataConnection.AllowSort is true, and + // - SortDescriptionsCollection exists, and + // - the column's data type is comparable - KeyboardHelper.GetMetaKeyState(keyModifiers, out bool ctrl, out bool shift); + DataGrid owningGrid = OwningGrid; + DataGridSortDescription newSort; - DataGridSortDescription sort = OwningColumn.GetSortDescription(); - IDataGridCollectionView collectionView = owningGrid.DataConnection.CollectionView; - Debug.Assert(collectionView != null); - using (collectionView.DeferRefresh()) - { - // if shift is held down, we multi-sort, therefore if it isn't, we'll clear the sorts beforehand - if (!shift || owningGrid.DataConnection.SortDescriptions.Count == 0) - { - owningGrid.DataConnection.SortDescriptions.Clear(); - } + KeyboardHelper.GetMetaKeyState(keyModifiers, out bool ctrl, out bool shift); + + DataGridSortDescription sort = OwningColumn.GetSortDescription(); + IDataGridCollectionView collectionView = owningGrid.DataConnection.CollectionView; + Debug.Assert(collectionView != null); - // if ctrl is held down, we only clear the sort directions - if (!ctrl) + using (collectionView.DeferRefresh()) { - if (sort != null) + // if shift is held down, we multi-sort, therefore if it isn't, we'll clear the sorts beforehand + if (!shift || owningGrid.DataConnection.SortDescriptions.Count == 0) { - newSort = sort.SwitchSortDirection(); + owningGrid.DataConnection.SortDescriptions.Clear(); + } - // changing direction should not affect sort order, so we replace this column's - // sort description instead of just adding it to the end of the collection - int oldIndex = owningGrid.DataConnection.SortDescriptions.IndexOf(sort); - if (oldIndex >= 0) + // if ctrl is held down, we only clear the sort directions + if (!ctrl) + { + if (sort != null) { - owningGrid.DataConnection.SortDescriptions.Remove(sort); - owningGrid.DataConnection.SortDescriptions.Insert(oldIndex, newSort); + newSort = sort.SwitchSortDirection(); + + // changing direction should not affect sort order, so we replace this column's + // sort description instead of just adding it to the end of the collection + int oldIndex = owningGrid.DataConnection.SortDescriptions.IndexOf(sort); + if (oldIndex >= 0) + { + owningGrid.DataConnection.SortDescriptions.Remove(sort); + owningGrid.DataConnection.SortDescriptions.Insert(oldIndex, newSort); + } + else + { + owningGrid.DataConnection.SortDescriptions.Add(newSort); + } } else { + string propertyName = OwningColumn.GetSortPropertyName(); + // no-opt if we couldn't find a property to sort on + if (string.IsNullOrEmpty(propertyName)) + { + return; + } + + newSort = DataGridSortDescription.FromPath(propertyName, culture: collectionView.Culture); + owningGrid.DataConnection.SortDescriptions.Add(newSort); } } - else - { - string propertyName = OwningColumn.GetSortPropertyName(); - // no-opt if we couldn't find a property to sort on - if (string.IsNullOrEmpty(propertyName)) - { - return; - } - - newSort = DataGridSortDescription.FromPath(propertyName, culture: collectionView.Culture); - owningGrid.DataConnection.SortDescriptions.Add(newSort); - } } } } diff --git a/src/Avalonia.Controls.DataGrid/DataGridColumns.cs b/src/Avalonia.Controls.DataGrid/DataGridColumns.cs index 5b75bc73f9..46bcd0d347 100644 --- a/src/Avalonia.Controls.DataGrid/DataGridColumns.cs +++ b/src/Avalonia.Controls.DataGrid/DataGridColumns.cs @@ -33,6 +33,11 @@ namespace Avalonia.Controls ColumnReordering?.Invoke(this, e); } + protected internal virtual void OnColumnSorting(DataGridColumnEventArgs e) + { + Sorting?.Invoke(this, e); + } + /// /// Adjusts the widths of all columns with DisplayIndex >= displayIndex such that the total /// width is adjusted by the given amount, if possible. If the total desired adjustment amount diff --git a/src/Avalonia.Controls.DataGrid/EventArgs.cs b/src/Avalonia.Controls.DataGrid/EventArgs.cs index 10e2be795e..7590a8ed61 100644 --- a/src/Avalonia.Controls.DataGrid/EventArgs.cs +++ b/src/Avalonia.Controls.DataGrid/EventArgs.cs @@ -289,7 +289,7 @@ namespace Avalonia.Controls /// /// Provides data for column-related events. /// - public class DataGridColumnEventArgs : EventArgs + public class DataGridColumnEventArgs : HandledEventArgs { /// /// Initializes a new instance of the class. @@ -566,4 +566,4 @@ namespace Avalonia.Controls private set; } } -} \ No newline at end of file +} From 8dd60eefe1ae90713ce994eb2c638b7704933d13 Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Thu, 17 Sep 2020 01:25:49 -0400 Subject: [PATCH 29/46] Allow to set custom IComparer in DataGridSortDescription.FromPath --- .../Collections/DataGridSortDescription.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs index ffb36d187d..46d9cd039f 100644 --- a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs +++ b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs @@ -120,7 +120,7 @@ namespace Avalonia.Collections { if (_internalComparerTyped == null && _internalComparer != null) { - if (_internalComparerTyped is IComparer c) + if (_internalComparer is IComparer c) _internalComparerTyped = c; else _internalComparerTyped = Comparer.Create((x, y) => _internalComparer.Compare(x, y)); @@ -134,11 +134,12 @@ namespace Avalonia.Collections public override IComparer Comparer => _comparer.Value; public override ListSortDirection Direction => _direction; - public DataGridPathSortDescription(string propertyPath, ListSortDirection direction, CultureInfo culture) + public DataGridPathSortDescription(string propertyPath, ListSortDirection direction, IComparer internalComparer, CultureInfo culture) { _propertyPath = propertyPath; _direction = direction; _cultureSensitiveComparer = new Lazy(() => new CultureSensitiveComparer(culture ?? CultureInfo.CurrentCulture)); + _internalComparer = internalComparer; _comparer = new Lazy>(() => Comparer.Create((x, y) => Compare(x, y))); } private DataGridPathSortDescription(DataGridPathSortDescription inner, ListSortDirection direction) @@ -250,14 +251,19 @@ namespace Avalonia.Collections public static DataGridSortDescription FromPath(string propertyPath, ListSortDirection direction = ListSortDirection.Ascending, CultureInfo culture = null) { - return new DataGridPathSortDescription(propertyPath, direction, culture); + return new DataGridPathSortDescription(propertyPath, direction, null, culture); } [Obsolete("Use overload taking a ListSortDirection.")] public static DataGridSortDescription FromPath(string propertyPath, bool descending, CultureInfo culture = null) { - return new DataGridPathSortDescription(propertyPath, descending ? ListSortDirection.Descending : ListSortDirection.Ascending, culture); + return new DataGridPathSortDescription(propertyPath, descending ? ListSortDirection.Descending : ListSortDirection.Ascending, null, culture); + } + + public static DataGridSortDescription FromPath(string propertyPath, ListSortDirection direction = ListSortDirection.Ascending, IComparer internalComparer = null) + { + return new DataGridPathSortDescription(propertyPath, direction, internalComparer, null); } } From b94748795adfafb5ebbea7d116f4d388378a9979 Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Thu, 17 Sep 2020 01:28:21 -0400 Subject: [PATCH 30/46] Add custom IComparer sorting to the DataGrid page --- .../ControlCatalog/Pages/DataGridPage.xaml.cs | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/samples/ControlCatalog/Pages/DataGridPage.xaml.cs b/samples/ControlCatalog/Pages/DataGridPage.xaml.cs index 0b7fb12aff..1893c4e5e7 100644 --- a/samples/ControlCatalog/Pages/DataGridPage.xaml.cs +++ b/samples/ControlCatalog/Pages/DataGridPage.xaml.cs @@ -1,8 +1,12 @@ +using System.Collections; using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; using Avalonia.Controls; using Avalonia.Markup.Xaml; using ControlCatalog.Models; using Avalonia.Collections; +using Avalonia.Data; namespace ControlCatalog.Pages { @@ -11,13 +15,23 @@ namespace ControlCatalog.Pages public DataGridPage() { this.InitializeComponent(); + + var dataGridSortDescription = DataGridSortDescription.FromPath(nameof(Country.Region), ListSortDirection.Ascending, new ReversedStringComparer()); + var colelctionView1 = new DataGridCollectionView(Countries.All); + colelctionView1.SortDescriptions.Add(dataGridSortDescription); var dg1 = this.FindControl("dataGrid1"); dg1.IsReadOnly = true; dg1.LoadingRow += Dg1_LoadingRow; - var collectionView1 = new DataGridCollectionView(Countries.All); - //collectionView.GroupDescriptions.Add(new PathGroupDescription("Region")); - - dg1.Items = collectionView1; + dg1.Sorting += (s, a) => + { + var property = ((a.Column as DataGridBoundColumn)?.Binding as Binding).Path; + if (property == dataGridSortDescription.PropertyPath + && !colelctionView1.SortDescriptions.Contains(dataGridSortDescription)) + { + colelctionView1.SortDescriptions.Add(dataGridSortDescription); + } + }; + dg1.Items = colelctionView1; var dg2 = this.FindControl("dataGridGrouping"); dg2.IsReadOnly = true; @@ -53,5 +67,20 @@ namespace ControlCatalog.Pages { AvaloniaXamlLoader.Load(this); } + + private class ReversedStringComparer : IComparer, IComparer + { + public int Compare(object x, object y) + { + if (x is string left && y is string right) + { + var reversedLeft = new string(left.Reverse().ToArray()); + var reversedRight = new string(right.Reverse().ToArray()); + return reversedLeft.CompareTo(reversedRight); + } + + return Comparer.Default.Compare(x, y); + } + } } } From 92f1c06a9691d2541ee8e785a7c65a0d2ff89a0b Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Thu, 17 Sep 2020 01:36:41 -0400 Subject: [PATCH 31/46] Update DataGrid tests --- .../Collections/DataGridSortDescription.cs | 4 ++-- .../Collections/DataGridSortDescriptionTests.cs | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs index 46d9cd039f..fe115f5012 100644 --- a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs +++ b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs @@ -261,9 +261,9 @@ namespace Avalonia.Collections return new DataGridPathSortDescription(propertyPath, descending ? ListSortDirection.Descending : ListSortDirection.Ascending, null, culture); } - public static DataGridSortDescription FromPath(string propertyPath, ListSortDirection direction = ListSortDirection.Ascending, IComparer internalComparer = null) + public static DataGridSortDescription FromPath(string propertyPath, ListSortDirection direction, IComparer comparer) { - return new DataGridPathSortDescription(propertyPath, direction, internalComparer, null); + return new DataGridPathSortDescription(propertyPath, direction, comparer, null); } } diff --git a/tests/Avalonia.Controls.DataGrid.UnitTests/Collections/DataGridSortDescriptionTests.cs b/tests/Avalonia.Controls.DataGrid.UnitTests/Collections/DataGridSortDescriptionTests.cs index a1a734f650..04d7ce3fc7 100644 --- a/tests/Avalonia.Controls.DataGrid.UnitTests/Collections/DataGridSortDescriptionTests.cs +++ b/tests/Avalonia.Controls.DataGrid.UnitTests/Collections/DataGridSortDescriptionTests.cs @@ -1,4 +1,5 @@ using System; +using System.ComponentModel; using System.Linq; using Avalonia.Collections; using Xunit; @@ -18,7 +19,7 @@ namespace Avalonia.Controls.DataGrid.UnitTests.Collections new Item("c", "c"), }; var expectedResult = items.OrderBy(i => i.Prop1).ToList(); - var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop1), @descending: false); + var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop1), ListSortDirection.Ascending); sortDescription.Initialize(typeof(Item)); var result = sortDescription.OrderBy(items).ToList(); @@ -36,7 +37,7 @@ namespace Avalonia.Controls.DataGrid.UnitTests.Collections new Item("c", "c"), }; var expectedResult = items.OrderByDescending(i => i.Prop1).ToList(); - var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop1), @descending: true); + var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop1), ListSortDirection.Descending); sortDescription.Initialize(typeof(Item)); var result = sortDescription.OrderBy(items).ToList(); @@ -61,7 +62,7 @@ namespace Avalonia.Controls.DataGrid.UnitTests.Collections new Item("a", "b"), new Item("a", "c"), }; - var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop2), @descending: false); + var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop2), ListSortDirection.Ascending); sortDescription.Initialize(typeof(Item)); var result = sortDescription.ThenBy(items).ToList(); @@ -86,7 +87,7 @@ namespace Avalonia.Controls.DataGrid.UnitTests.Collections new Item("a", "b"), new Item("a", "a"), }; - var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop2), @descending: true); + var sortDescription = DataGridSortDescription.FromPath(nameof(Item.Prop2), ListSortDirection.Descending); sortDescription.Initialize(typeof(Item)); var result = sortDescription.ThenBy(items).ToList(); From 8c3458e9306cbfb17305b3bc3ae6362434a8a0d8 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 17 Sep 2020 12:00:36 +0200 Subject: [PATCH 32/46] 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 33/46] 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 34/46] 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 35/46] 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) { From 91d18be95465844b287cab8f575c87ba752a1f68 Mon Sep 17 00:00:00 2001 From: artyom Date: Thu, 17 Sep 2020 16:34:32 +0300 Subject: [PATCH 36/46] Remove Avalonia.ReactiveUI.Events.UnitTests from solution file --- Avalonia.sln | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/Avalonia.sln b/Avalonia.sln index ffcf266b0c..34ad19b41d 100644 --- a/Avalonia.sln +++ b/Avalonia.sln @@ -224,8 +224,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.Markup.Xaml.Loader EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.ReactiveUI.Events", "src\Avalonia.ReactiveUI.Events\Avalonia.ReactiveUI.Events.csproj", "{28F18757-C3E6-4BBE-A37D-11BA2AB9177C}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Avalonia.ReactiveUI.Events.UnitTests", "tests\Avalonia.ReactiveUI.Events.UnitTests\Avalonia.ReactiveUI.Events.UnitTests.csproj", "{780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}" -EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Sandbox", "samples\Sandbox\Sandbox.csproj", "{11BE52AF-E2DD-4CF0-B19A-05285ACAF571}" EndProject Global @@ -2042,30 +2040,6 @@ Global {28F18757-C3E6-4BBE-A37D-11BA2AB9177C}.Release|iPhone.Build.0 = Release|Any CPU {28F18757-C3E6-4BBE-A37D-11BA2AB9177C}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU {28F18757-C3E6-4BBE-A37D-11BA2AB9177C}.Release|iPhoneSimulator.Build.0 = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|Any CPU.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|Any CPU.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhone.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhone.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhoneSimulator.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Ad-Hoc|iPhoneSimulator.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|Any CPU.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|Any CPU.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhone.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhone.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhoneSimulator.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.AppStore|iPhoneSimulator.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|Any CPU.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhone.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhone.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|Any CPU.ActiveCfg = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|Any CPU.Build.0 = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhone.ActiveCfg = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhone.Build.0 = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7}.Release|iPhoneSimulator.Build.0 = Release|Any CPU {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|Any CPU.ActiveCfg = Release|Any CPU {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|Any CPU.Build.0 = Release|Any CPU {11BE52AF-E2DD-4CF0-B19A-05285ACAF571}.Ad-Hoc|iPhone.ActiveCfg = Release|Any CPU @@ -2148,7 +2122,6 @@ Global {351337F5-D66F-461B-A957-4EF60BDB4BA6} = {C5A00AC3-B34C-4564-9BDD-2DA473EF4D8B} {3C84E04B-36CF-4D0D-B965-C26DD649D1F3} = {A0CC0258-D18C-4AB3-854F-7101680FC3F9} {909A8CBD-7D0E-42FD-B841-022AD8925820} = {8B6A8209-894F-4BA1-B880-965FD453982C} - {780AC0FE-8DD2-419F-A1DC-AC7E3EB393F7} = {C5A00AC3-B34C-4564-9BDD-2DA473EF4D8B} {11BE52AF-E2DD-4CF0-B19A-05285ACAF571} = {9B9E3891-2366-4253-A952-D08BCEB71098} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution From 3608deeeb1c48dd0e91da8f52482069dfdf2fd54 Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Wed, 16 Sep 2020 18:13:07 +0300 Subject: [PATCH 37/46] Fixed SleepLoopRenderTimer --- src/Avalonia.Visuals/Rendering/SleepLoopRenderTimer.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/SleepLoopRenderTimer.cs b/src/Avalonia.Visuals/Rendering/SleepLoopRenderTimer.cs index 79e029ccd2..9cc94ffac3 100644 --- a/src/Avalonia.Visuals/Rendering/SleepLoopRenderTimer.cs +++ b/src/Avalonia.Visuals/Rendering/SleepLoopRenderTimer.cs @@ -45,14 +45,13 @@ namespace Avalonia.Rendering void LoopProc() { - var now = _st.Elapsed; - var lastTick = now; - + var lastTick = _st.Elapsed; while (true) { + var now = _st.Elapsed; var timeTillNextTick = lastTick + _timeBetweenTicks - now; if (timeTillNextTick.TotalMilliseconds > 1) Thread.Sleep(timeTillNextTick); - + lastTick = now; lock (_lock) { if (_count == 0) @@ -63,7 +62,7 @@ namespace Avalonia.Rendering } _tick?.Invoke(now); - now = _st.Elapsed; + } } From 6ca33a86c57fcf9aaa412b1df454c276662ca3a9 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Thu, 17 Sep 2020 11:16:16 -0400 Subject: [PATCH 38/46] Update samples/ControlCatalog/Pages/DataGridPage.xaml.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dariusz Komosiński --- samples/ControlCatalog/Pages/DataGridPage.xaml.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/ControlCatalog/Pages/DataGridPage.xaml.cs b/samples/ControlCatalog/Pages/DataGridPage.xaml.cs index 1893c4e5e7..9b7bf3ab16 100644 --- a/samples/ControlCatalog/Pages/DataGridPage.xaml.cs +++ b/samples/ControlCatalog/Pages/DataGridPage.xaml.cs @@ -17,7 +17,7 @@ namespace ControlCatalog.Pages this.InitializeComponent(); var dataGridSortDescription = DataGridSortDescription.FromPath(nameof(Country.Region), ListSortDirection.Ascending, new ReversedStringComparer()); - var colelctionView1 = new DataGridCollectionView(Countries.All); + var collectionView1 = new DataGridCollectionView(Countries.All); colelctionView1.SortDescriptions.Add(dataGridSortDescription); var dg1 = this.FindControl("dataGrid1"); dg1.IsReadOnly = true; From d1fef034869c651ca37c86f70dac345c3a700202 Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Thu, 17 Sep 2020 15:28:50 -0400 Subject: [PATCH 39/46] Make SwitchSortDirection public --- .../Collections/DataGridSortDescription.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs index fe115f5012..662ff91329 100644 --- a/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs +++ b/src/Avalonia.Controls.DataGrid/Collections/DataGridSortDescription.cs @@ -28,7 +28,7 @@ namespace Avalonia.Collections return seq.ThenBy(o => o, Comparer); } - internal virtual DataGridSortDescription SwitchSortDirection() + public virtual DataGridSortDescription SwitchSortDirection() { return this; } @@ -242,7 +242,7 @@ namespace Avalonia.Collections } } - internal override DataGridSortDescription SwitchSortDirection() + public override DataGridSortDescription SwitchSortDirection() { var newDirection = _direction == ListSortDirection.Ascending ? ListSortDirection.Descending : ListSortDirection.Ascending; return new DataGridPathSortDescription(this, newDirection); From 59db76f21416910360c91e1690952696696a0e2d Mon Sep 17 00:00:00 2001 From: Maksym Katsydan Date: Thu, 17 Sep 2020 16:24:55 -0400 Subject: [PATCH 40/46] Fix build errors --- samples/ControlCatalog/Pages/DataGridPage.xaml.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/samples/ControlCatalog/Pages/DataGridPage.xaml.cs b/samples/ControlCatalog/Pages/DataGridPage.xaml.cs index 9b7bf3ab16..2a30f4d91b 100644 --- a/samples/ControlCatalog/Pages/DataGridPage.xaml.cs +++ b/samples/ControlCatalog/Pages/DataGridPage.xaml.cs @@ -18,7 +18,7 @@ namespace ControlCatalog.Pages var dataGridSortDescription = DataGridSortDescription.FromPath(nameof(Country.Region), ListSortDirection.Ascending, new ReversedStringComparer()); var collectionView1 = new DataGridCollectionView(Countries.All); - colelctionView1.SortDescriptions.Add(dataGridSortDescription); + collectionView1.SortDescriptions.Add(dataGridSortDescription); var dg1 = this.FindControl("dataGrid1"); dg1.IsReadOnly = true; dg1.LoadingRow += Dg1_LoadingRow; @@ -26,12 +26,12 @@ namespace ControlCatalog.Pages { var property = ((a.Column as DataGridBoundColumn)?.Binding as Binding).Path; if (property == dataGridSortDescription.PropertyPath - && !colelctionView1.SortDescriptions.Contains(dataGridSortDescription)) + && !collectionView1.SortDescriptions.Contains(dataGridSortDescription)) { - colelctionView1.SortDescriptions.Add(dataGridSortDescription); + collectionView1.SortDescriptions.Add(dataGridSortDescription); } }; - dg1.Items = colelctionView1; + dg1.Items = collectionView1; var dg2 = this.FindControl("dataGridGrouping"); dg2.IsReadOnly = true; From e574acd85f831360e97350d7a6a1d57620de684d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 18 Sep 2020 09:03:10 +0200 Subject: [PATCH 41/46] Mark TabOnceActiveElement value as nullable. --- src/Avalonia.Input/KeyboardNavigation.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Input/KeyboardNavigation.cs b/src/Avalonia.Input/KeyboardNavigation.cs index 722215f8b7..6ef3c4fd60 100644 --- a/src/Avalonia.Input/KeyboardNavigation.cs +++ b/src/Avalonia.Input/KeyboardNavigation.cs @@ -25,12 +25,11 @@ namespace Avalonia.Input /// attached property set to , this property /// defines to which child the focus should move. /// - public static readonly AttachedProperty TabOnceActiveElementProperty = - AvaloniaProperty.RegisterAttached( + public static readonly AttachedProperty TabOnceActiveElementProperty = + AvaloniaProperty.RegisterAttached( "TabOnceActiveElement", typeof(KeyboardNavigation)); - /// /// Defines the IsTabStop attached property. /// @@ -68,7 +67,7 @@ namespace Avalonia.Input /// /// The container. /// The active element for the container. - public static IInputElement GetTabOnceActiveElement(InputElement element) + public static IInputElement? GetTabOnceActiveElement(InputElement element) { return element.GetValue(TabOnceActiveElementProperty); } @@ -78,7 +77,7 @@ namespace Avalonia.Input /// /// The container. /// The active element for the container. - public static void SetTabOnceActiveElement(InputElement element, IInputElement value) + public static void SetTabOnceActiveElement(InputElement element, IInputElement? value) { element.SetValue(TabOnceActiveElementProperty, value); } From 3f47f6dba35ae9954b8970f9b5586736f44c3f4c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 18 Sep 2020 09:08:08 +0200 Subject: [PATCH 42/46] Added failing test for AvaloniaList.CopyTo. `Can_CopyTo_Array_Of_Same_Type` passes but `Can_CopyTo_Array_Of_Base_Type` fails. --- ...Controls.UnitTests.net47.v3.ncrunchproject | 1 + .../Collections/AvaloniaListTests.cs | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/.ncrunch/Avalonia.Controls.UnitTests.net47.v3.ncrunchproject b/.ncrunch/Avalonia.Controls.UnitTests.net47.v3.ncrunchproject index e9d39b0c74..f30a20df78 100644 --- a/.ncrunch/Avalonia.Controls.UnitTests.net47.v3.ncrunchproject +++ b/.ncrunch/Avalonia.Controls.UnitTests.net47.v3.ncrunchproject @@ -3,5 +3,6 @@ MissingOrIgnoredProjectReference + Avalonia.Controls.UnitTests.TimePickerTests \ No newline at end of file diff --git a/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs b/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs index 19700cadab..d5ac01a092 100644 --- a/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs +++ b/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel; @@ -334,5 +335,27 @@ namespace Avalonia.Base.UnitTests.Collections Assert.True(raised); } + + [Fact] + public void Can_CopyTo_Array_Of_Same_Type() + { + var target = new AvaloniaList { "foo", "bar", "baz" }; + var result = new string[3]; + + target.CopyTo(result, 0); + + Assert.Equal(target, result); + } + + [Fact] + public void Can_CopyTo_Array_Of_Base_Type() + { + var target = new AvaloniaList { "foo", "bar", "baz" }; + var result = new object[3]; + + ((IList)target).CopyTo(result, 0); + + Assert.Equal(target, result); + } } } From 8350ab1399a310782982c586cc1a829280a7c6ec Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 17 Sep 2020 17:42:48 +0200 Subject: [PATCH 43/46] Copy ICollection.CopyTo implementation from the BCL. Did not handle differing types. --- src/Avalonia.Base/Collections/AvaloniaList.cs | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Base/Collections/AvaloniaList.cs b/src/Avalonia.Base/Collections/AvaloniaList.cs index f201cfab1f..d43b4e04bb 100644 --- a/src/Avalonia.Base/Collections/AvaloniaList.cs +++ b/src/Avalonia.Base/Collections/AvaloniaList.cs @@ -543,7 +543,73 @@ namespace Avalonia.Collections /// void ICollection.CopyTo(Array array, int index) { - _inner.CopyTo((T[])array, index); + if (array == null) + { + throw new ArgumentNullException(nameof(array)); + } + + if (array.Rank != 1) + { + throw new ArgumentException("Multi-dimensional arrays are not supported."); + } + + if (array.GetLowerBound(0) != 0) + { + throw new ArgumentException("Non-zero lower bounds are not supported."); + } + + if (index < 0) + { + throw new ArgumentException("Invalid index."); + } + + if (array.Length - index < Count) + { + throw new ArgumentException("The target array is too small."); + } + + if (array is T[] tArray) + { + _inner.CopyTo(tArray, index); + } + else + { + // + // Catch the obvious case assignment will fail. + // We can't find all possible problems by doing the check though. + // For example, if the element type of the Array is derived from T, + // we can't figure out if we can successfully copy the element beforehand. + // + Type targetType = array.GetType().GetElementType()!; + Type sourceType = typeof(T); + if (!(targetType.IsAssignableFrom(sourceType) || sourceType.IsAssignableFrom(targetType))) + { + throw new ArgumentException("Invalid array type"); + } + + // + // We can't cast array of value type to object[], so we don't support + // widening of primitive types here. + // + object[] objects = array as object[]; + if (objects == null) + { + throw new ArgumentException("Invalid array type"); + } + + int count = _inner.Count; + try + { + for (int i = 0; i < count; i++) + { + objects[index++] = _inner[i]; + } + } + catch (ArrayTypeMismatchException) + { + throw new ArgumentException("Invalid array type"); + } + } } /// From 89684e5b94d51b432a4476d089c1f1a2df8782a6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 17 Sep 2020 17:46:07 +0200 Subject: [PATCH 44/46] Short-circuit setting source to current value. --- src/Avalonia.Controls/Selection/InternalSelectionModel.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Avalonia.Controls/Selection/InternalSelectionModel.cs b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs index a28e4b2785..fcdaf44166 100644 --- a/src/Avalonia.Controls/Selection/InternalSelectionModel.cs +++ b/src/Avalonia.Controls/Selection/InternalSelectionModel.cs @@ -63,6 +63,11 @@ namespace Avalonia.Controls.Selection private protected override void SetSource(IEnumerable? value) { + if (Source == value) + { + return; + } + object?[]? oldSelection = null; if (Source is object && value is object) From 544686b78d99674bf50984ce5e97008a8a3d319a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 18 Sep 2020 11:41:04 +0200 Subject: [PATCH 45/46] Don't try to SelectAll with single selection. `Toggle` doesn't mean multiple selection. --- src/Avalonia.Controls/Primitives/SelectingItemsControl.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index c954f9fd4a..e34b3b145f 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -491,8 +491,7 @@ namespace Avalonia.Controls.Primitives if (ItemCount > 0 && Match(keymap.SelectAll) && - (((SelectionMode & SelectionMode.Multiple) != 0) || - (SelectionMode & SelectionMode.Toggle) != 0)) + SelectionMode.HasFlag(SelectionMode.Multiple)) { Selection.SelectAll(); e.Handled = true; From 08a8badd7afbf87c45d985de815b82b9d64fc6f0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 18 Sep 2020 00:28:56 +0200 Subject: [PATCH 46/46] Don't try to realize index -1. `Algorithm_GetAnchorForTargetElement` can return -1 and when it does so we try to realize that item, which obviously doesn't exist, so boom. --- src/Avalonia.Layout/FlowLayoutAlgorithm.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Layout/FlowLayoutAlgorithm.cs b/src/Avalonia.Layout/FlowLayoutAlgorithm.cs index cd7f725f18..eace54d2e0 100644 --- a/src/Avalonia.Layout/FlowLayoutAlgorithm.cs +++ b/src/Avalonia.Layout/FlowLayoutAlgorithm.cs @@ -211,7 +211,7 @@ namespace Avalonia.Layout anchorPosition = new Point(anchorBounds.X, anchorBounds.Y); } } - else + else if (anchorIndex >= 0) { // It is possible to end up in a situation during a collection change where GetAnchorForTargetElement returns an index // which is not in the realized range. Eg. insert one item at index 0 for a grid layout.