Merge pull request #3936 from AvaloniaUI/fixes/3934-scroll-to-wrong-item

Fix scrolling to incorrect item in SelectingItemsControl.

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<IndexPath>? _selectedIndicesCached;
         private IReadOnlyList<object?>? _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;
             }
         }
 

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<string>((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<ListBox>((parent, scope) =>

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()
         {