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]