Merge pull request #3177 from AvaloniaUI/fixes/3148-virtualization-out-of-range

Make sure ItemsPresenter is updated before selection change.

src/Avalonia.Base/Utilities/MathUtilities.cs

@@ -159,6 +159,11 @@ namespace Avalonia.Utilities
         /// <returns>The clamped value.</returns>
         public static int Clamp(int val, int min, int max)
         {
+            if (min > max)
+            {
+                throw new ArgumentException($"{min} cannot be greater than {max}.");
+            }
+
             if (val < min)
             {
                 return min;

src/Avalonia.Controls/ItemsControl.cs

@@ -359,6 +359,12 @@ namespace Avalonia.Controls
             UpdateItemCount();
             RemoveControlItemsFromLogicalChildren(oldValue);
             AddControlItemsToLogicalChildren(newValue);
+
+            if (Presenter != null)
+            {
+                Presenter.Items = newValue;
+            }
+
             SubscribeToItems(newValue);
         }
 
@@ -370,6 +376,8 @@ namespace Avalonia.Controls
         /// <param name="e">The event args.</param>
         protected virtual void ItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
         {
+            UpdateItemCount();
+
             switch (e.Action)
             {
                 case NotifyCollectionChangedAction.Add:
@@ -381,7 +389,7 @@ namespace Avalonia.Controls
                     break;
             }
 
-            UpdateItemCount();
+            Presenter?.ItemsChanged(e);
 
             var collection = sender as ICollection;
             PseudoClasses.Set(":empty", collection == null || collection.Count == 0);

src/Avalonia.Controls/Presenters/IItemsPresenter.cs

@@ -1,12 +1,19 @@
 // Copyright (c) The Avalonia Project. All rights reserved.
 // Licensed under the MIT license. See licence.md file in the project root for full license information.
 
+using System.Collections;
+using System.Collections.Specialized;
+
 namespace Avalonia.Controls.Presenters
 {
     public interface IItemsPresenter : IPresenter
     {
+        IEnumerable Items { get; set; }
+
         IPanel Panel { get; }
 
+        void ItemsChanged(NotifyCollectionChangedEventArgs e);
+
         void ScrollIntoView(object item);
     }
 }

src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs

@@ -63,7 +63,7 @@ namespace Avalonia.Controls.Presenters
                 _itemsSubscription?.Dispose();
                 _itemsSubscription = null;
 
-                if (_createdPanel && value is INotifyCollectionChanged incc)
+                if (!IsHosted && _createdPanel && value is INotifyCollectionChanged incc)
                 {
                     _itemsSubscription = incc.WeakSubscribe(ItemsCollectionChanged);
                 }
@@ -130,6 +130,8 @@ namespace Avalonia.Controls.Presenters
             private set;
         }
 
+        protected bool IsHosted => TemplatedParent is IItemsPresenterHost;
+
         /// <inheritdoc/>
         public override sealed void ApplyTemplate()
         {
@@ -144,6 +146,15 @@ namespace Avalonia.Controls.Presenters
         {
         }
 
+        /// <inheritdoc/>
+        void IItemsPresenter.ItemsChanged(NotifyCollectionChangedEventArgs e)
+        {
+            if (Panel != null)
+            {
+                ItemsChanged(e);
+            }
+        }
+
         /// <summary>
         /// Creates the <see cref="ItemContainerGenerator"/> for the control.
         /// </summary>
@@ -215,7 +226,7 @@ namespace Avalonia.Controls.Presenters
 
             _createdPanel = true;
 
-            if (_itemsSubscription == null && Items is INotifyCollectionChanged incc)
+            if (!IsHosted && _itemsSubscription == null && Items is INotifyCollectionChanged incc)
             {
                 _itemsSubscription = incc.WeakSubscribe(ItemsCollectionChanged);
             }

src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

@@ -302,13 +302,24 @@ namespace Avalonia.Controls.Primitives
         /// <inheritdoc/>
         protected override void ItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
         {
-            base.ItemsCollectionChanged(sender, e);
-
             if (_updateCount > 0)
             {
+                base.ItemsCollectionChanged(sender, e);
                 return;
             }
 
+            switch (e.Action)
+            {
+                case NotifyCollectionChangedAction.Add:
+                    _selection.ItemsInserted(e.NewStartingIndex, e.NewItems.Count);
+                    break;
+                case NotifyCollectionChangedAction.Remove:
+                    _selection.ItemsRemoved(e.OldStartingIndex, e.OldItems.Count);
+                    break;
+            }
+
+            base.ItemsCollectionChanged(sender, e);
+
             switch (e.Action)
             {
                 case NotifyCollectionChangedAction.Add:
@@ -318,14 +329,12 @@ namespace Avalonia.Controls.Primitives
                     }
                     else
                     {
-                        _selection.ItemsInserted(e.NewStartingIndex, e.NewItems.Count);
                         UpdateSelectedItem(_selection.First(), false);
                     }
 
                     break;
 
                 case NotifyCollectionChangedAction.Remove:
-                    _selection.ItemsRemoved(e.OldStartingIndex, e.OldItems.Count);
                     UpdateSelectedItem(_selection.First(), false);
                     ResetSelectedItems();
                     break;

tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs

@@ -12,6 +12,7 @@ using Xunit;
 using System.Collections.ObjectModel;
 using Avalonia.UnitTests;
 using Avalonia.Input;
+using System.Collections.Generic;
 
 namespace Avalonia.Controls.UnitTests
 {
@@ -104,6 +105,28 @@ namespace Avalonia.Controls.UnitTests
             Assert.Equal(new[] { child }, target.GetLogicalChildren());
         }
 
+        [Fact]
+        public void Added_Container_Should_Have_LogicalParent_Set_To_ItemsControl()
+        {
+            var item = new Border();
+            var items = new ObservableCollection<Border>();
+
+            var target = new ItemsControl
+            {
+                Template = GetTemplate(),
+                Items = items,
+            };
+
+            var root = new TestRoot(true, target);
+
+            root.Measure(new Size(100, 100));
+            root.Arrange(new Rect(0, 0, 100, 100));
+
+            items.Add(item);
+
+            Assert.Equal(target, item.Parent);
+        }
+
         [Fact]
         public void Control_Item_Should_Be_Removed_From_Logical_Children_Before_ApplyTemplate()
         {
@@ -522,6 +545,36 @@ namespace Avalonia.Controls.UnitTests
             }
         }
 
+        [Fact]
+        public void Presenter_Items_Should_Be_In_Sync()
+        {
+            var target = new ItemsControl
+            {
+                Template = GetTemplate(),
+                Items = new object[]
+                {
+                    new Button(),
+                    new Button(),
+                },
+            };
+
+            var root = new TestRoot { Child = target };
+            var otherPanel = new StackPanel();
+
+            target.ApplyTemplate();
+            target.Presenter.ApplyTemplate();
+            
+            target.ItemContainerGenerator.Materialized += (s, e) =>
+            {
+                Assert.IsType<Canvas>(e.Containers[0].Item);
+            };
+
+            target.Items = new[]
+            {
+                new Canvas()
+            };
+        }
+
         private class Item
         {
             public Item(string value)

tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs

@@ -1,6 +1,7 @@
 // Copyright (c) The Avalonia Project. All rights reserved.
 // Licensed under the MIT license. See licence.md file in the project root for full license information.
 
+using System.Collections;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
@@ -14,6 +15,7 @@ using Avalonia.Data;
 using Avalonia.Input;
 using Avalonia.Interactivity;
 using Avalonia.Markup.Data;
+using Avalonia.Styling;
 using Avalonia.UnitTests;
 using Moq;
 using Xunit;
@@ -1104,6 +1106,45 @@ namespace Avalonia.Controls.UnitTests.Primitives
             Assert.True(raised);
         }
 
+        [Fact]
+        public void AutoScrollToSelectedItem_On_Reset_Works()
+        {
+            // Issue #3148
+            using (UnitTestApplication.Start(TestServices.StyledWindow))
+            {
+                var items = new ResettingCollection(100);
+
+                var target = new ListBox
+                {
+                    Items = items,
+                    ItemTemplate = new FuncDataTemplate<string>((x, _) =>
+                        new TextBlock
+                        {
+                            Text = x,
+                            Width = 100,
+                            Height = 10
+                        }),
+                    AutoScrollToSelectedItem = true,
+                    VirtualizationMode = ItemVirtualizationMode.Simple,
+                };
+
+                var root = new TestRoot(true, target);
+                root.Measure(new Size(100, 100));
+                root.Arrange(new Rect(0, 0, 100, 100));
+
+                Assert.True(target.Presenter.Panel.Children.Count > 0);
+                Assert.True(target.Presenter.Panel.Children.Count < 100);
+
+                target.SelectedItem = "Item99";
+
+                // #3148 triggered here.
+                items.Reset(new[] { "Item99" });
+
+                Assert.Equal(0, target.SelectedIndex);
+                Assert.Equal(1, target.Presenter.Panel.Children.Count);
+            }
+        }
+
         [Fact]
         public void Can_Set_Both_SelectedItem_And_SelectedItems_During_Initialization()
         {
@@ -1152,6 +1193,7 @@ namespace Avalonia.Controls.UnitTests.Primitives
                     Name = "itemsPresenter",
                     [~ItemsPresenter.ItemsProperty] = control[~ItemsControl.ItemsProperty],
                     [~ItemsPresenter.ItemsPanelProperty] = control[~ItemsControl.ItemsPanelProperty],
+                    [~ItemsPresenter.VirtualizationModeProperty] = control[~ListBox.VirtualizationModeProperty],
                 }.RegisterInNameScope(scope));
         }
 
@@ -1196,5 +1238,24 @@ namespace Avalonia.Controls.UnitTests.Primitives
                 return base.MoveSelection(direction, wrap);
             }
         }
+
+        private class ResettingCollection : List<string>, INotifyCollectionChanged
+        {
+            public ResettingCollection(int itemCount)
+            {
+                AddRange(Enumerable.Range(0, itemCount).Select(x => $"Item{x}"));
+            }
+
+            public void Reset(IEnumerable<string> items)
+            {
+                Clear();
+                AddRange(items);
+                CollectionChanged?.Invoke(
+                    this,
+                    new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
+            }
+
+            public event NotifyCollectionChangedEventHandler CollectionChanged;
+        }
     }
 }

tests/Avalonia.UnitTests/TestRoot.cs

@@ -24,8 +24,19 @@ namespace Avalonia.UnitTests
         }
 
         public TestRoot(IControl child)
+            : this(false, child)
+        {
+            Child = child;
+        }
+
+        public TestRoot(bool useGlobalStyles, IControl child)
             : this()
         {
+            if (useGlobalStyles)
+            {
+                StylingParent = UnitTestApplication.Current;
+            }
+
             Child = child;
         }