From 065f3b76bb2d2e07082175164f61b2f3ba03b864 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Mon, 9 Sep 2019 20:49:33 +0100 Subject: [PATCH 1/6] add a repro to control catalog. --- samples/ControlCatalog/Pages/ListBoxPage.xaml | 4 +++- .../ControlCatalog/Pages/ListBoxPage.xaml.cs | 21 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/samples/ControlCatalog/Pages/ListBoxPage.xaml b/samples/ControlCatalog/Pages/ListBoxPage.xaml index 49e9aafc4a..b1b3112e60 100644 --- a/samples/ControlCatalog/Pages/ListBoxPage.xaml +++ b/samples/ControlCatalog/Pages/ListBoxPage.xaml @@ -10,12 +10,14 @@ HorizontalAlignment="Center" Spacing="16"> - + + + Single Multiple diff --git a/samples/ControlCatalog/Pages/ListBoxPage.xaml.cs b/samples/ControlCatalog/Pages/ListBoxPage.xaml.cs index 8a67766c76..cdbf8fd2b6 100644 --- a/samples/ControlCatalog/Pages/ListBoxPage.xaml.cs +++ b/samples/ControlCatalog/Pages/ListBoxPage.xaml.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.ObjectModel; using System.Linq; using System.Reactive; @@ -27,7 +28,7 @@ namespace ControlCatalog.Pages public PageViewModel() { - Items = new ObservableCollection(Enumerable.Range(1, 10).Select(i => GenerateItem())); + Items = new ObservableCollection(Enumerable.Range(1, 10000).Select(i => GenerateItem())); SelectedItems = new ObservableCollection(); AddItemCommand = ReactiveCommand.Create(() => Items.Add(GenerateItem())); @@ -39,16 +40,34 @@ namespace ControlCatalog.Pages Items.Remove(SelectedItems[0]); } }); + + SelectRandomItemCommand = ReactiveCommand.Create(() => + { + var random = new Random(); + + SelectedItem = Items[random.Next(Items.Count - 1)]; + }); } public ObservableCollection Items { get; } + private string _selectedItem; + + public string SelectedItem + { + get { return _selectedItem; } + set { this.RaiseAndSetIfChanged(ref _selectedItem, value); } + } + + public ObservableCollection SelectedItems { get; } public ReactiveCommand AddItemCommand { get; } public ReactiveCommand RemoveItemCommand { get; } + public ReactiveCommand SelectRandomItemCommand { get; } + public SelectionMode SelectionMode { get => _selectionMode; From e9ae26e09c3aa08003e3cf3a02a7723c48d96c50 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Mon, 9 Sep 2019 20:49:46 +0100 Subject: [PATCH 2/6] fix scrolling to selected item. --- src/Avalonia.Controls/Primitives/SelectingItemsControl.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 44ae89fdbc..b53b9f73b6 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -858,7 +858,7 @@ namespace Avalonia.Controls.Primitives if (AutoScrollToSelectedItem) { - ScrollIntoView(_selectedIndex); + ScrollIntoView(_selectedItem); } } } @@ -1046,6 +1046,11 @@ namespace Avalonia.Controls.Primitives removed?.Select(x => ElementAt(Items, x)).ToArray() ?? Array.Empty()); RaiseEvent(e); } + + if(AutoScrollToSelectedItem) + { + ScrollIntoView(_selectedItem); + } } private void UpdateSelectedItems(Action action) From cce605af493864b9e996cef1441fc70ccd546c50 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 9 Sep 2019 23:51:30 +0200 Subject: [PATCH 3/6] Remove unused/bad code. Removed the code that was causing the TreeView problems described in #2944. This code was never actually called except in `TreeView` because a plain `ItemsControl` can't be used with virtualization (it uses untyped `ItemContainerGenerator` so items can't be recycled) and every other derived class derives from SelectingItemsControl which overrides this method and doesn't call the base implementation. In addition there were no unit tests covering this code. Fixes #2944 --- src/Avalonia.Controls/ItemsControl.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 902e55bde6..0fe7291835 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -302,19 +302,6 @@ namespace Avalonia.Controls /// The details of the containers. protected virtual void OnContainersRecycled(ItemContainerEventArgs e) { - var toRemove = new List(); - - foreach (var container in e.Containers) - { - // If the item is its own container, then it will be removed from the logical tree - // when it is removed from the Items collection. - if (container?.ContainerControl != container?.Item) - { - toRemove.Add(container.ContainerControl); - } - } - - LogicalChildren.RemoveAll(toRemove); } /// From d636c7647e2deb92fd0f84e40fd5cb4b92b17ffc Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Mon, 9 Sep 2019 23:14:11 +0100 Subject: [PATCH 4/6] only require a single call to ScrollIntoView --- src/Avalonia.Controls/Primitives/SelectingItemsControl.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index b53b9f73b6..336bcffa54 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -855,11 +855,6 @@ namespace Avalonia.Controls.Primitives _selectedItem = ElementAt(Items, _selectedIndex); RaisePropertyChanged(SelectedIndexProperty, -1, _selectedIndex, BindingPriority.LocalValue); RaisePropertyChanged(SelectedItemProperty, null, _selectedItem, BindingPriority.LocalValue); - - if (AutoScrollToSelectedItem) - { - ScrollIntoView(_selectedItem); - } } } From 9e6d3db5207d7d79d1551ec50b6c34fc9759334a Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Mon, 9 Sep 2019 23:14:20 +0100 Subject: [PATCH 5/6] fix whitespace. --- 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 336bcffa54..340947d2e1 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -1042,7 +1042,7 @@ namespace Avalonia.Controls.Primitives RaiseEvent(e); } - if(AutoScrollToSelectedItem) + if (AutoScrollToSelectedItem) { ScrollIntoView(_selectedItem); } From 6f44dfac5b42a5240cb92e01c374369c6858de02 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 10 Sep 2019 00:18:11 +0200 Subject: [PATCH 6/6] Added unit test for #2952. --- .../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 bc002174ec..f4f500b802 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -894,6 +894,33 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal("Qux", target.SelectedItem); } + [Fact] + public void AutoScrollToSelectedItem_Causes_Scroll_To_SelectedItem() + { + var items = new ObservableCollection + { + "Foo", + "Bar", + "Baz" + }; + + var target = new ListBox + { + Template = Template(), + Items = items, + }; + + target.ApplyTemplate(); + target.Presenter.ApplyTemplate(); + + var raised = false; + target.AddHandler(Control.RequestBringIntoViewEvent, (s, e) => raised = true); + + target.SelectedIndex = 2; + + Assert.True(raised); + } + private FuncControlTemplate Template() { return new FuncControlTemplate((control, scope) =>