From 5b252620b8d87343c20471ea19902c013945f9bf Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 15:36:32 +0200 Subject: [PATCH 01/24] 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/24] 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/24] 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/24] 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/24] 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 f4017162cd5ca4c65cd92a2a7f7a90c638505640 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 11 Sep 2020 20:34:42 +0200 Subject: [PATCH 06/24] 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 07/24] 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 d84a98f65ac04f03f40bbc4b33c9201fc660e1cc Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 12 Sep 2020 00:17:15 +0200 Subject: [PATCH 08/24] 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 22580652fa6f0893c90f1db544dd69eafb980f13 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 10:11:30 +0200 Subject: [PATCH 09/24] 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 10/24] 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 c01c7c937838265fc8031934c1bca7703ec9952b Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Tue, 15 Sep 2020 02:50:27 -0700 Subject: [PATCH 11/24] opengl es 3.1, 3.0 and 2.0 --- src/Avalonia.OpenGL/EglDisplay.cs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.OpenGL/EglDisplay.cs b/src/Avalonia.OpenGL/EglDisplay.cs index 7b194e4346..ca7e9b66c3 100644 --- a/src/Avalonia.OpenGL/EglDisplay.cs +++ b/src/Avalonia.OpenGL/EglDisplay.cs @@ -67,13 +67,38 @@ namespace Avalonia.OpenGL { Attributes = new[] { - EGL_CONTEXT_CLIENT_VERSION, 2, + EGL_CONTEXT_MAJOR_VERSION, 3, + EGL_CONTEXT_MINOR_VERSION, 1, + EGL_NONE + }, + Api = EGL_OPENGL_ES_API, + RenderableTypeBit = EGL_OPENGL_ES3_BIT, + Version = new GlVersion(GlProfileType.OpenGLES, 3, 1) + }, + new + { + Attributes = new[] + { + EGL_CONTEXT_MAJOR_VERSION, 3, + EGL_CONTEXT_MINOR_VERSION, 0, + EGL_NONE + }, + Api = EGL_OPENGL_ES_API, + RenderableTypeBit = EGL_OPENGL_ES3_BIT, + Version = new GlVersion(GlProfileType.OpenGLES, 3, 0) + }, + new + { + Attributes = new[] + { + EGL_CONTEXT_MAJOR_VERSION, 2, + EGL_CONTEXT_MINOR_VERSION, 0, EGL_NONE }, Api = EGL_OPENGL_ES_API, RenderableTypeBit = EGL_OPENGL_ES2_BIT, Version = new GlVersion(GlProfileType.OpenGLES, 2, 0) - } + }, }) { if (!_egl.BindApi(cfg.Api)) From a6d83cd32a8e373b7b00f58b18b2947f135da3fa Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Tue, 15 Sep 2020 06:12:47 -0700 Subject: [PATCH 12/24] support any gles version with angle using angle options. --- src/Avalonia.OpenGL/AngleOptions.cs | 8 +++ src/Avalonia.OpenGL/EglDisplay.cs | 63 +++++++++++----------- src/Avalonia.X11/Glx/GlxDisplay.cs | 2 +- src/Avalonia.X11/Glx/GlxPlatformFeature.cs | 4 +- src/Avalonia.X11/X11Platform.cs | 4 +- 5 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/Avalonia.OpenGL/AngleOptions.cs b/src/Avalonia.OpenGL/AngleOptions.cs index 84744288ed..462bd56cbe 100644 --- a/src/Avalonia.OpenGL/AngleOptions.cs +++ b/src/Avalonia.OpenGL/AngleOptions.cs @@ -10,6 +10,14 @@ namespace Avalonia.OpenGL DirectX11 } + public IList GlProfiles { get; set; } = new List + { + new GlVersion(GlProfileType.OpenGLES, 3, 2), + new GlVersion(GlProfileType.OpenGLES, 3, 1), + new GlVersion(GlProfileType.OpenGLES, 3, 0), + new GlVersion(GlProfileType.OpenGLES, 2, 0) + }; + public IList AllowedPlatformApis { get; set; } = null; } } diff --git a/src/Avalonia.OpenGL/EglDisplay.cs b/src/Avalonia.OpenGL/EglDisplay.cs index ca7e9b66c3..9edcaf2bc0 100644 --- a/src/Avalonia.OpenGL/EglDisplay.cs +++ b/src/Avalonia.OpenGL/EglDisplay.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Runtime.InteropServices; using Avalonia.Platform.Interop; using static Avalonia.OpenGL.EglConsts; @@ -61,45 +62,45 @@ namespace Avalonia.OpenGL if (!_egl.Initialize(_display, out var major, out var minor)) throw OpenGlException.GetFormattedException("eglInitialize", _egl); - foreach (var cfg in new[] + var glProfiles = AvaloniaLocator.Current.GetService()?.GlProfiles + ?? new[] + { + new GlVersion(GlProfileType.OpenGLES, 3, 2), + new GlVersion(GlProfileType.OpenGLES, 3, 1), + new GlVersion(GlProfileType.OpenGLES, 3, 0), + new GlVersion(GlProfileType.OpenGLES, 2, 0) + }; + + var cfgs = glProfiles.Select(x => { - new - { - Attributes = new[] - { - EGL_CONTEXT_MAJOR_VERSION, 3, - EGL_CONTEXT_MINOR_VERSION, 1, - EGL_NONE - }, - Api = EGL_OPENGL_ES_API, - RenderableTypeBit = EGL_OPENGL_ES3_BIT, - Version = new GlVersion(GlProfileType.OpenGLES, 3, 1) - }, - new + var typeBit = EGL_OPENGL_ES3_BIT; + + switch (x.Major) { - Attributes = new[] - { - EGL_CONTEXT_MAJOR_VERSION, 3, - EGL_CONTEXT_MINOR_VERSION, 0, - EGL_NONE - }, - Api = EGL_OPENGL_ES_API, - RenderableTypeBit = EGL_OPENGL_ES3_BIT, - Version = new GlVersion(GlProfileType.OpenGLES, 3, 0) - }, - new + case 2: + typeBit = EGL_OPENGL_ES2_BIT; + break; + + case 1: + typeBit = EGL_OPENGL_ES_BIT; + break; + } + + return new { Attributes = new[] { - EGL_CONTEXT_MAJOR_VERSION, 2, - EGL_CONTEXT_MINOR_VERSION, 0, + EGL_CONTEXT_MAJOR_VERSION, x.Major, + EGL_CONTEXT_MINOR_VERSION, x.Minor, EGL_NONE }, Api = EGL_OPENGL_ES_API, - RenderableTypeBit = EGL_OPENGL_ES2_BIT, - Version = new GlVersion(GlProfileType.OpenGLES, 2, 0) - }, - }) + RenderableTypeBit = typeBit, + Version = x + }; + }); + + foreach (var cfg in cfgs) { if (!_egl.BindApi(cfg.Api)) continue; diff --git a/src/Avalonia.X11/Glx/GlxDisplay.cs b/src/Avalonia.X11/Glx/GlxDisplay.cs index 903d6b570b..b82895d12c 100644 --- a/src/Avalonia.X11/Glx/GlxDisplay.cs +++ b/src/Avalonia.X11/Glx/GlxDisplay.cs @@ -18,7 +18,7 @@ namespace Avalonia.X11.Glx public XVisualInfo* VisualInfo => _visual; public GlxContext DeferredContext { get; } public GlxInterface Glx { get; } = new GlxInterface(); - public GlxDisplay(X11Info x11, List probeProfiles) + public GlxDisplay(X11Info x11, IList probeProfiles) { _x11 = x11; _probeProfiles = probeProfiles.ToList(); diff --git a/src/Avalonia.X11/Glx/GlxPlatformFeature.cs b/src/Avalonia.X11/Glx/GlxPlatformFeature.cs index e3250e6733..ad3a54bcc1 100644 --- a/src/Avalonia.X11/Glx/GlxPlatformFeature.cs +++ b/src/Avalonia.X11/Glx/GlxPlatformFeature.cs @@ -12,7 +12,7 @@ namespace Avalonia.X11.Glx public GlxContext DeferredContext { get; private set; } public IGlContext MainContext => DeferredContext; - public static bool TryInitialize(X11Info x11, List glProfiles) + public static bool TryInitialize(X11Info x11, IList glProfiles) { var feature = TryCreate(x11, glProfiles); if (feature != null) @@ -24,7 +24,7 @@ namespace Avalonia.X11.Glx return false; } - public static GlxGlPlatformFeature TryCreate(X11Info x11, List glProfiles) + public static GlxGlPlatformFeature TryCreate(X11Info x11, IList glProfiles) { try { diff --git a/src/Avalonia.X11/X11Platform.cs b/src/Avalonia.X11/X11Platform.cs index 7f3255d4da..d7bd81db98 100644 --- a/src/Avalonia.X11/X11Platform.cs +++ b/src/Avalonia.X11/X11Platform.cs @@ -103,7 +103,7 @@ namespace Avalonia public bool UseDBusMenu { get; set; } public bool UseDeferredRendering { get; set; } = true; - public List GlProfiles { get; set; } = new List + public IList GlProfiles { get; set; } = new List { new GlVersion(GlProfileType.OpenGL, 4, 0), new GlVersion(GlProfileType.OpenGL, 3, 2), @@ -113,7 +113,7 @@ namespace Avalonia new GlVersion(GlProfileType.OpenGLES, 2, 0) }; - public List GlxRendererBlacklist { get; set; } = new List + public IList GlxRendererBlacklist { get; set; } = new List { // llvmpipe is a software GL rasterizer. If it's returned by glGetString, // that usually means that something in the system is horribly misconfigured From b73ba990777c9727cad4efcd64b968c33b86f336 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 15 Sep 2020 15:16:34 +0200 Subject: [PATCH 13/24] 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 14/24] 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 15/24] 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 16/24] 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 17/24] 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 0fefe3b5af4529624378f47e8cbbd9e980045164 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 16 Sep 2020 03:02:07 -0700 Subject: [PATCH 18/24] dont use egl 3.2 and 3.1 as they are still wip. --- src/Avalonia.OpenGL/AngleOptions.cs | 2 -- src/Avalonia.OpenGL/EglDisplay.cs | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/Avalonia.OpenGL/AngleOptions.cs b/src/Avalonia.OpenGL/AngleOptions.cs index 462bd56cbe..0807eb7ab4 100644 --- a/src/Avalonia.OpenGL/AngleOptions.cs +++ b/src/Avalonia.OpenGL/AngleOptions.cs @@ -12,8 +12,6 @@ namespace Avalonia.OpenGL public IList GlProfiles { get; set; } = new List { - new GlVersion(GlProfileType.OpenGLES, 3, 2), - new GlVersion(GlProfileType.OpenGLES, 3, 1), new GlVersion(GlProfileType.OpenGLES, 3, 0), new GlVersion(GlProfileType.OpenGLES, 2, 0) }; diff --git a/src/Avalonia.OpenGL/EglDisplay.cs b/src/Avalonia.OpenGL/EglDisplay.cs index 9edcaf2bc0..7f41e75d6a 100644 --- a/src/Avalonia.OpenGL/EglDisplay.cs +++ b/src/Avalonia.OpenGL/EglDisplay.cs @@ -65,8 +65,6 @@ namespace Avalonia.OpenGL var glProfiles = AvaloniaLocator.Current.GetService()?.GlProfiles ?? new[] { - new GlVersion(GlProfileType.OpenGLES, 3, 2), - new GlVersion(GlProfileType.OpenGLES, 3, 1), new GlVersion(GlProfileType.OpenGLES, 3, 0), new GlVersion(GlProfileType.OpenGLES, 2, 0) }; From 14f314341ed61f2bf66fb79f9be57b3b51c8766b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 12:21:13 +0200 Subject: [PATCH 19/24] 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 20/24] 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 8e1c27756582614885fff6d280a7634f0d86e032 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 16 Sep 2020 03:40:50 -0700 Subject: [PATCH 21/24] update to release of skiasharp. --- build/SkiaSharp.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/SkiaSharp.props b/build/SkiaSharp.props index d7d04c7971..f2e7df36cd 100644 --- a/build/SkiaSharp.props +++ b/build/SkiaSharp.props @@ -1,6 +1,6 @@  - - + + From 1b4fe4f5632b3ddb47bec68d43dcaa4e97675cbc Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 16 Sep 2020 13:55:59 +0200 Subject: [PATCH 22/24] 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 23/24] 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 24/24] 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);