From 8e29811c91ff6f4e6ecedd1bc817041d3e1070f8 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 13 May 2020 23:01:41 +0200 Subject: [PATCH 1/2] Added failing test for #3934. --- .../ListBoxTests.cs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index d43385142d..a7679ba388 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -367,6 +367,46 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Clicking_Item_Should_Raise_BringIntoView_For_Correct_Control() + { + // Issue #3934 + var items = Enumerable.Range(0, 10).Select(x => $"Item {x}").ToArray(); + var target = new ListBox + { + Template = ListBoxTemplate(), + Items = items, + ItemTemplate = new FuncDataTemplate((x, _) => new TextBlock { Height = 10 }), + SelectionMode = SelectionMode.AlwaysSelected, + VirtualizationMode = ItemVirtualizationMode.None, + }; + + Prepare(target); + + // First an item that is not index 0 must be selected. + _mouse.Click(target.Presenter.Panel.Children[1]); + Assert.Equal(new IndexPath(1), target.Selection.AnchorIndex); + + // We're going to be clicking on item 9. + var item = (ListBoxItem)target.Presenter.Panel.Children[9]; + var raised = 0; + + // Make sure a RequestBringIntoView event is raised for item 9. It won't be handled + // by the ScrollContentPresenter as the item is already visible, so we don't need + // handledEventsToo: true. Issue #3934 failed here because item 0 was being scrolled + // into view due to SelectionMode.AlwaysSelected. + target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => + { + Assert.Same(item, e.TargetObject); + ++raised; + }); + + // Click item 9. + _mouse.Click(item); + + Assert.Equal(1, raised); + } + private FuncControlTemplate ListBoxTemplate() { return new FuncControlTemplate((parent, scope) => From 7f87c2e74ab0c3f2b00cdce441264e3520cfeb23 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 13 May 2020 23:27:11 +0200 Subject: [PATCH 2/2] Don't raise SelectionModel.PropertyChanged during update. --- src/Avalonia.Controls/SelectionModel.cs | 35 +++++++----- .../SelectionModelTests.cs | 54 +++++++++++++++++++ 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/src/Avalonia.Controls/SelectionModel.cs b/src/Avalonia.Controls/SelectionModel.cs index dd4934f9e5..93699583e6 100644 --- a/src/Avalonia.Controls/SelectionModel.cs +++ b/src/Avalonia.Controls/SelectionModel.cs @@ -20,6 +20,7 @@ namespace Avalonia.Controls private bool _singleSelect; private bool _autoSelect; private int _operationCount; + private IndexPath _oldAnchorIndex; private IReadOnlyList? _selectedIndicesCached; private IReadOnlyList? _selectedItemsCached; private SelectionModelChildrenRequestedEventArgs? _childrenRequestedEventArgs; @@ -142,6 +143,8 @@ namespace Avalonia.Controls } set { + var oldValue = AnchorIndex; + if (value != null) { SelectionTreeHelper.TraverseIndexPath( @@ -155,7 +158,10 @@ namespace Avalonia.Controls _rootNode.AnchorIndex = -1; } - RaisePropertyChanged("AnchorIndex"); + if (_operationCount == 0 && oldValue != AnchorIndex) + { + RaisePropertyChanged("AnchorIndex"); + } } } @@ -633,19 +639,18 @@ namespace Avalonia.Controls _selectedIndicesCached = null; _selectedItemsCached = null; - // Raise SelectionChanged event if (e != null) { SelectionChanged?.Invoke(this, e); - } - RaisePropertyChanged(nameof(SelectedIndex)); - RaisePropertyChanged(nameof(SelectedIndices)); + RaisePropertyChanged(nameof(SelectedIndex)); + RaisePropertyChanged(nameof(SelectedIndices)); - if (_rootNode.Source != null) - { - RaisePropertyChanged(nameof(SelectedItem)); - RaisePropertyChanged(nameof(SelectedItems)); + if (_rootNode.Source != null) + { + RaisePropertyChanged(nameof(SelectedItem)); + RaisePropertyChanged(nameof(SelectedItems)); + } } } @@ -785,6 +790,7 @@ namespace Avalonia.Controls { if (_operationCount++ == 0) { + _oldAnchorIndex = AnchorIndex; _rootNode.BeginOperation(); } } @@ -808,13 +814,16 @@ namespace Avalonia.Controls var changeSet = new SelectionModelChangeSet(changes); e = changeSet.CreateEventArgs(); } - } - OnSelectionChanged(e); + OnSelectionChanged(e); + + if (_oldAnchorIndex != AnchorIndex) + { + RaisePropertyChanged(nameof(AnchorIndex)); + } - if (_operationCount == 0) - { _rootNode.Cleanup(); + _oldAnchorIndex = default; } } diff --git a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs index 246ff723a1..ebf9c40012 100644 --- a/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/SelectionModelTests.cs @@ -1458,6 +1458,60 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(1, raised); } + [Fact] + public void Batch_Update_Does_Not_Raise_PropertyChanged_Until_Operation_Finished() + { + var data = new[] { "foo", "bar", "baz", "qux" }; + var target = new SelectionModel { Source = data }; + var raised = 0; + + target.SelectedIndex = new IndexPath(1); + + Assert.Equal(new IndexPath(1), target.AnchorIndex); + + target.PropertyChanged += (s, e) => ++raised; + + using (target.Update()) + { + target.ClearSelection(); + + Assert.Equal(0, raised); + + target.AnchorIndex = new IndexPath(2); + + Assert.Equal(0, raised); + + target.SelectedIndex = new IndexPath(3); + + Assert.Equal(0, raised); + } + + Assert.Equal(new IndexPath(3), target.AnchorIndex); + Assert.Equal(5, raised); + } + + [Fact] + public void Batch_Update_Does_Not_Raise_PropertyChanged_If_Nothing_Changed() + { + var data = new[] { "foo", "bar", "baz", "qux" }; + var target = new SelectionModel { Source = data }; + var raised = 0; + + target.SelectedIndex = new IndexPath(1); + + Assert.Equal(new IndexPath(1), target.AnchorIndex); + + target.PropertyChanged += (s, e) => ++raised; + + using (target.Update()) + { + target.ClearSelection(); + target.SelectedIndex = new IndexPath(1); + } + + Assert.Equal(0, raised); + } + [Fact] public void AutoSelect_Selects_When_Enabled() {