From d19f159932490fa65b5cd277b50f50ff5cb3bcd4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 2 May 2019 16:14:50 +0200 Subject: [PATCH 1/4] Added failing tests for #2432. --- .../Presenters/CarouselPresenterTests.cs | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs b/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs index c5ff64b0e3..828f96b806 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs @@ -193,6 +193,113 @@ namespace Avalonia.Controls.UnitTests.Presenters Assert.Empty(target.Panel.Children); } + [Fact] + public void Should_Handle_Inserting_Items_At_SelectedItem() + { + ObservableCollection items = new ObservableCollection + { + "item1", + "item2", + "item3", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 0, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + target.SelectedIndex = 1; + + items.Insert(1, "item1a"); + + var containers = target.ItemContainerGenerator.Containers.Select(x => (x.Index, (string)x.Item)); + + Assert.Equal( + new[] + { + (0, "item1"), + (1, "item1a"), + (2, "item2"), + }, + containers); + + var visibleContainer = (ContentPresenter)target.Panel.Children.Single(x => x.IsVisible); + Assert.Equal("item1a", visibleContainer.Content); + } + + [Fact] + public void Should_Handle_Inserting_Items_Before_SelectedItem() + { + ObservableCollection items = new ObservableCollection + { + "item1", + "item2", + "item3", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 0, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + target.SelectedIndex = 2; + + items.Insert(1, "item1a"); + + var actual = target.ItemContainerGenerator.Containers.Select(x => (x.Index, (string)x.Item)); + + Assert.Equal( + new[] + { + (0, "item1"), + (3, "item3"), + }, + actual); + + var visibleContainer = (ContentPresenter)target.Panel.Children.Single(x => x.IsVisible); + Assert.Equal("item3", visibleContainer.Content); + } + + [Fact] + public void Should_Handle_Inserting_Items_After_SelectedItem() + { + ObservableCollection items = new ObservableCollection + { + "item1", + "item2", + "item3", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 0, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + target.SelectedIndex = 1; + target.SelectedIndex = 0; + + items.Insert(1, "item1a"); + + var actual = target.ItemContainerGenerator.Containers.Select(x => (x.Index, (string)x.Item)); + + Assert.Equal( + new[] + { + (0, "item1"), + (2, "item2"), + }, + actual); + } + private class TestItem : ContentControl { } From 76c3d0c03d9b2457cc312db577258f3c9a93013d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 2 May 2019 16:16:29 +0200 Subject: [PATCH 2/4] Handle item insertion in CarouselPresenter. Fixes #2432. --- .../Presenters/CarouselPresenter.cs | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index ba2c7c91d8..f9f8d58430 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -38,7 +38,6 @@ namespace Avalonia.Controls.Presenters Carousel.PageTransitionProperty.AddOwner(); private int _selectedIndex = -1; - // private Task _current; private Task _currentTransition; private int _queuedTransitionIndex = -1; @@ -105,12 +104,33 @@ namespace Avalonia.Controls.Presenters /// protected override void ItemsChanged(NotifyCollectionChangedEventArgs e) { + var generator = ItemContainerGenerator; + switch (e.Action) { + case NotifyCollectionChangedAction.Add: + var end = e.NewStartingIndex + e.NewItems.Count; + + if (end < Items.Count()) + { + generator.InsertSpace(e.NewStartingIndex, e.NewItems.Count); + } + + if (SelectedIndex >= e.NewStartingIndex && SelectedIndex < end) + { + var newItem = GetOrCreateContainer(SelectedIndex); + + foreach (var control in Panel.Children) + { + control.IsVisible = control == newItem; + } + } + + break; + case NotifyCollectionChangedAction.Remove: if (!IsVirtualized) { - var generator = ItemContainerGenerator; var containers = generator.RemoveRange(e.OldStartingIndex, e.OldItems.Count); Panel.Children.RemoveAll(containers.Select(x => x.ContainerControl)); @@ -122,7 +142,6 @@ namespace Avalonia.Controls.Presenters case NotifyCollectionChangedAction.Reset: { - var generator = ItemContainerGenerator; var containers = generator.Containers.ToList(); generator.Clear(); Panel.Children.RemoveAll(containers.Select(x => x.ContainerControl)); From 5745b72222af8e359ae45725d32c348f0c95d627 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 3 May 2019 11:02:10 +0200 Subject: [PATCH 3/4] Started refactoring CarouselPresenter. Make it react correctly to changes in the source collection. Fixes #2432. --- .../Presenters/CarouselPresenter.cs | 121 +-- .../Presenters/ItemContainerSync.cs | 125 +++ .../Presenters/ItemVirtualizerNone.cs | 48 +- .../Presenters/ItemsPresenterBase.cs | 9 +- .../CarouselTests.cs | 38 +- .../Presenters/CarouselPresenterTests.cs | 802 +++++++++++++----- 6 files changed, 818 insertions(+), 325 deletions(-) create mode 100644 src/Avalonia.Controls/Presenters/ItemContainerSync.cs diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index f9f8d58430..bdc4d29400 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -46,6 +46,7 @@ namespace Avalonia.Controls.Presenters /// static CarouselPresenter() { + IsVirtualizedProperty.Changed.AddClassHandler(x => x.IsVirtualizedChanged); SelectedIndexProperty.Changed.AddClassHandler(x => x.SelectedIndexChanged); } @@ -93,78 +94,72 @@ namespace Avalonia.Controls.Presenters set { SetValue(PageTransitionProperty, value); } } - /// protected override void PanelCreated(IPanel panel) { -#pragma warning disable 4014 - MoveToPage(-1, SelectedIndex); -#pragma warning restore 4014 + base.PanelCreated(panel); } /// protected override void ItemsChanged(NotifyCollectionChangedEventArgs e) { - var generator = ItemContainerGenerator; - - switch (e.Action) + if (!IsVirtualized) { - case NotifyCollectionChangedAction.Add: - var end = e.NewStartingIndex + e.NewItems.Count; - - if (end < Items.Count()) - { - generator.InsertSpace(e.NewStartingIndex, e.NewItems.Count); - } + base.ItemsChanged(e); - if (SelectedIndex >= e.NewStartingIndex && SelectedIndex < end) - { - var newItem = GetOrCreateContainer(SelectedIndex); + if (Items == null || SelectedIndex >= Items.Count()) + { + SelectedIndex = Items.Count() - 1; + } - foreach (var control in Panel.Children) + foreach (var c in ItemContainerGenerator.Containers) + { + c.ContainerControl.IsVisible = c.Index == SelectedIndex; + } + } + else if (SelectedIndex != -1 && Panel != null) + { + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + if (e.NewStartingIndex > SelectedIndex) { - control.IsVisible = control == newItem; + return; } - } - - break; - - case NotifyCollectionChangedAction.Remove: - if (!IsVirtualized) - { - var containers = generator.RemoveRange(e.OldStartingIndex, e.OldItems.Count); - Panel.Children.RemoveAll(containers.Select(x => x.ContainerControl)); - -#pragma warning disable 4014 - MoveToPage(-1, SelectedIndex); -#pragma warning restore 4014 - } - break; + break; + case NotifyCollectionChangedAction.Remove: + if (e.OldStartingIndex > SelectedIndex) + { + return; + } + break; + case NotifyCollectionChangedAction.Replace: + if (e.OldStartingIndex > SelectedIndex || + e.OldStartingIndex + e.OldItems.Count - 1 < SelectedIndex) + { + return; + } + break; + case NotifyCollectionChangedAction.Move: + if (e.OldStartingIndex > SelectedIndex && + e.NewStartingIndex > SelectedIndex) + { + return; + } + break; + } - case NotifyCollectionChangedAction.Reset: - { - var containers = generator.Containers.ToList(); - generator.Clear(); - Panel.Children.RemoveAll(containers.Select(x => x.ContainerControl)); + if (Items == null || SelectedIndex >= Items.Count()) + { + SelectedIndex = Items.Count() - 1; + } -#pragma warning disable 4014 - var newIndex = SelectedIndex; + Panel.Children.Clear(); + ItemContainerGenerator.Clear(); - if(SelectedIndex < 0) - { - if(Items != null && Items.Count() > 0) - { - newIndex = 0; - } - else - { - newIndex = -1; - } - } - - MoveToPage(-1, newIndex); -#pragma warning restore 4014 - } - break; + if (SelectedIndex != -1) + { + GetOrCreateContainer(SelectedIndex); + } } } @@ -231,6 +226,18 @@ namespace Avalonia.Controls.Presenters return container; } + /// + /// Called when the property changes. + /// + /// The event args. + private void IsVirtualizedChanged(AvaloniaPropertyChangedEventArgs e) + { + if (Panel != null) + { + ItemsChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + } + } + /// /// Called when the property changes. /// diff --git a/src/Avalonia.Controls/Presenters/ItemContainerSync.cs b/src/Avalonia.Controls/Presenters/ItemContainerSync.cs new file mode 100644 index 0000000000..035d404dec --- /dev/null +++ b/src/Avalonia.Controls/Presenters/ItemContainerSync.cs @@ -0,0 +1,125 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.Specialized; +using Avalonia.Controls.Generators; +using Avalonia.Controls.Utils; + +namespace Avalonia.Controls.Presenters +{ + internal static class ItemContainerSync + { + public static void ItemsChanged( + ItemsPresenterBase owner, + IEnumerable items, + NotifyCollectionChangedEventArgs e) + { + var generator = owner.ItemContainerGenerator; + var panel = owner.Panel; + + if (panel == null) + { + return; + } + + void Add() + { + if (e.NewStartingIndex + e.NewItems.Count < items.Count()) + { + generator.InsertSpace(e.NewStartingIndex, e.NewItems.Count); + } + + AddContainers(owner, e.NewStartingIndex, e.NewItems); + } + + void Remove() + { + RemoveContainers(panel, generator.RemoveRange(e.OldStartingIndex, e.OldItems.Count)); + } + + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + Add(); + break; + + case NotifyCollectionChangedAction.Remove: + Remove(); + break; + + case NotifyCollectionChangedAction.Replace: + RemoveContainers(panel, generator.Dematerialize(e.OldStartingIndex, e.OldItems.Count)); + var containers = AddContainers(owner, e.NewStartingIndex, e.NewItems); + + var i = e.NewStartingIndex; + + foreach (var container in containers) + { + panel.Children[i++] = container.ContainerControl; + } + + break; + + case NotifyCollectionChangedAction.Move: + Remove(); + Add(); + break; + + case NotifyCollectionChangedAction.Reset: + RemoveContainers(panel, generator.Clear()); + + if (items != null) + { + AddContainers(owner, 0, items); + } + + break; + } + } + + private static IList AddContainers( + ItemsPresenterBase owner, + int index, + IEnumerable items) + { + var generator = owner.ItemContainerGenerator; + var result = new List(); + var panel = owner.Panel; + + foreach (var item in items) + { + var i = generator.Materialize(index++, item, owner.MemberSelector); + + if (i.ContainerControl != null) + { + if (i.Index < panel.Children.Count) + { + // TODO: This will insert at the wrong place when there are null items. + panel.Children.Insert(i.Index, i.ContainerControl); + } + else + { + panel.Children.Add(i.ContainerControl); + } + } + + result.Add(i); + } + + return result; + } + + private static void RemoveContainers( + IPanel panel, + IEnumerable items) + { + foreach (var i in items) + { + if (i.ContainerControl != null) + { + panel.Children.Remove(i.ContainerControl); + } + } + } + } +} diff --git a/src/Avalonia.Controls/Presenters/ItemVirtualizerNone.cs b/src/Avalonia.Controls/Presenters/ItemVirtualizerNone.cs index 8947b30e63..413855bcc6 100644 --- a/src/Avalonia.Controls/Presenters/ItemVirtualizerNone.cs +++ b/src/Avalonia.Controls/Presenters/ItemVirtualizerNone.cs @@ -60,53 +60,7 @@ namespace Avalonia.Controls.Presenters public override void ItemsChanged(IEnumerable items, NotifyCollectionChangedEventArgs e) { base.ItemsChanged(items, e); - - var generator = Owner.ItemContainerGenerator; - var panel = Owner.Panel; - - switch (e.Action) - { - case NotifyCollectionChangedAction.Add: - if (e.NewStartingIndex + e.NewItems.Count < Items.Count()) - { - generator.InsertSpace(e.NewStartingIndex, e.NewItems.Count); - } - - AddContainers(e.NewStartingIndex, e.NewItems); - break; - - case NotifyCollectionChangedAction.Remove: - RemoveContainers(generator.RemoveRange(e.OldStartingIndex, e.OldItems.Count)); - break; - - case NotifyCollectionChangedAction.Replace: - RemoveContainers(generator.Dematerialize(e.OldStartingIndex, e.OldItems.Count)); - var containers = AddContainers(e.NewStartingIndex, e.NewItems); - - var i = e.NewStartingIndex; - - foreach (var container in containers) - { - panel.Children[i++] = container.ContainerControl; - } - - break; - - case NotifyCollectionChangedAction.Move: - // TODO: Handle move in a more efficient manner. At the moment we just - // drop through to Reset to recreate all the containers. - - case NotifyCollectionChangedAction.Reset: - RemoveContainers(generator.Clear()); - - if (Items != null) - { - AddContainers(0, Items); - } - - break; - } - + ItemContainerSync.ItemsChanged(Owner, items, e); Owner.InvalidateMeasure(); } diff --git a/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs b/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs index e9dc75a236..b4b792139d 100644 --- a/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs +++ b/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs @@ -7,6 +7,7 @@ using System.Collections.Specialized; using Avalonia.Collections; using Avalonia.Controls.Generators; using Avalonia.Controls.Templates; +using Avalonia.Controls.Utils; using Avalonia.Styling; namespace Avalonia.Controls.Presenters @@ -205,7 +206,13 @@ namespace Avalonia.Controls.Presenters /// has been set, the items collection has been modified, or the panel has been created. /// /// A description of the change. - protected abstract void ItemsChanged(NotifyCollectionChangedEventArgs e); + /// + /// The panel is guaranteed to be created when this method is called. + /// + protected virtual void ItemsChanged(NotifyCollectionChangedEventArgs e) + { + ItemContainerSync.ItemsChanged(this, Items, e); + } /// /// Creates the when is called for the first diff --git a/tests/Avalonia.Controls.UnitTests/CarouselTests.cs b/tests/Avalonia.Controls.UnitTests/CarouselTests.cs index f36e5864f6..2070b4b66a 100644 --- a/tests/Avalonia.Controls.UnitTests/CarouselTests.cs +++ b/tests/Avalonia.Controls.UnitTests/CarouselTests.cs @@ -76,25 +76,6 @@ namespace Avalonia.Controls.UnitTests Assert.Single(target.ItemContainerGenerator.Containers); } - [Fact] - public void Should_Not_Remove_NonCurrent_Page_When_IsVirtualized_False() - { - var target = new Carousel - { - Template = new FuncControlTemplate(CreateTemplate), - Items = new[] { "foo", "bar" }, - IsVirtualized = false, - SelectedIndex = 0, - }; - - target.ApplyTemplate(); - target.Presenter.ApplyTemplate(); - - Assert.Single(target.ItemContainerGenerator.Containers); - target.SelectedIndex = 1; - Assert.Equal(2, target.ItemContainerGenerator.Containers.Count()); - } - [Fact] public void Selected_Item_Changes_To_First_Item_When_Items_Property_Changes() { @@ -115,9 +96,9 @@ namespace Avalonia.Controls.UnitTests target.ApplyTemplate(); target.Presenter.ApplyTemplate(); - Assert.Single(target.GetLogicalChildren()); + Assert.Equal(3, target.GetLogicalChildren().Count()); - var child = target.GetLogicalChildren().Single(); + var child = target.GetLogicalChildren().First(); Assert.IsType(child); Assert.Equal("Foo", ((TextBlock)child).Text); @@ -127,7 +108,7 @@ namespace Avalonia.Controls.UnitTests target.Items = newItems; - child = target.GetLogicalChildren().Single(); + child = target.GetLogicalChildren().First(); Assert.IsType(child); Assert.Equal("Bar", ((TextBlock)child).Text); @@ -146,7 +127,8 @@ namespace Avalonia.Controls.UnitTests var target = new Carousel { Template = new FuncControlTemplate(CreateTemplate), - Items = items + Items = items, + IsVirtualized = true, }; target.ApplyTemplate(); @@ -190,9 +172,9 @@ namespace Avalonia.Controls.UnitTests target.ApplyTemplate(); target.Presenter.ApplyTemplate(); - Assert.Single(target.GetLogicalChildren()); + Assert.Equal(3, target.GetLogicalChildren().Count()); - var child = target.GetLogicalChildren().Single(); + var child = target.GetLogicalChildren().First(); Assert.IsType(child); Assert.Equal("Foo", ((TextBlock)child).Text); @@ -254,16 +236,16 @@ namespace Avalonia.Controls.UnitTests target.ApplyTemplate(); target.Presenter.ApplyTemplate(); - Assert.Single(target.GetLogicalChildren()); + Assert.Equal(3, target.GetLogicalChildren().Count()); - var child = target.GetLogicalChildren().Single(); + var child = target.GetLogicalChildren().First(); Assert.IsType(child); Assert.Equal("Foo", ((TextBlock)child).Text); items.RemoveAt(0); - child = target.GetLogicalChildren().Single(); + child = target.GetLogicalChildren().First(); Assert.IsType(child); Assert.Equal("Bar", ((TextBlock)child).Text); diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs b/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs index 828f96b806..2cc9965740 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/CarouselPresenterTests.cs @@ -8,6 +8,7 @@ using Avalonia.Controls.Presenters; using Avalonia.Controls.Templates; using Xunit; using System.Collections.ObjectModel; +using System.Collections; namespace Avalonia.Controls.UnitTests.Presenters { @@ -49,255 +50,672 @@ namespace Avalonia.Controls.UnitTests.Presenters Assert.IsType>(target.ItemContainerGenerator); } - [Fact] - public void Setting_SelectedIndex_Should_Show_Page() + public class Virtualized { - var target = new CarouselPresenter + [Fact] + public void Should_Initially_Materialize_Selected_Container() { - Items = new[] { "foo", "bar" }, - SelectedIndex = 0, - }; + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + SelectedIndex = 0, + IsVirtualized = true, + }; - target.ApplyTemplate(); + target.ApplyTemplate(); - Assert.IsType(target.Panel.Children[0]); - Assert.Equal("foo", ((ContentPresenter)target.Panel.Children[0]).Content); - } + AssertSingle(target); + } - [Fact] - public void Changing_SelectedIndex_Should_Show_Page() - { - var target = new CarouselPresenter + [Fact] + public void Should_Initially_Materialize_Nothing_If_No_Selected_Container() { - Items = new[] { "foo", "bar" }, - SelectedIndex = 0, - }; + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + IsVirtualized = true, + }; - target.ApplyTemplate(); - target.SelectedIndex = 1; + target.ApplyTemplate(); - Assert.IsType(target.Panel.Children[0]); - Assert.Equal("bar", ((ContentPresenter)target.Panel.Children[0]).Content); - } + Assert.Empty(target.Panel.Children); + Assert.Empty(target.ItemContainerGenerator.Containers); + } - [Fact] - public void Should_Remove_NonCurrent_Page_When_IsVirtualized_True() - { - var target = new CarouselPresenter + [Fact] + public void Switching_To_Virtualized_Should_Reset_Containers() { - Items = new[] { "foo", "bar" }, - IsVirtualized = true, - SelectedIndex = 0, - }; + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + SelectedIndex = 0, + IsVirtualized = false, + }; - target.ApplyTemplate(); - Assert.Single(target.ItemContainerGenerator.Containers); - target.SelectedIndex = 1; - Assert.Single(target.ItemContainerGenerator.Containers); - } + target.ApplyTemplate(); + target.IsVirtualized = true; - [Fact] - public void Should_Not_Remove_NonCurrent_Page_When_IsVirtualized_False() - { - var target = new CarouselPresenter + AssertSingle(target); + } + + [Fact] + public void Changing_SelectedIndex_Should_Show_Page() { - Items = new[] { "foo", "bar" }, - IsVirtualized = false, - SelectedIndex = 0, - }; + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + SelectedIndex = 0, + IsVirtualized = true, + }; - target.ApplyTemplate(); - Assert.Single(target.ItemContainerGenerator.Containers); - Assert.Single(target.Panel.Children); - target.SelectedIndex = 1; - Assert.Equal(2, target.ItemContainerGenerator.Containers.Count()); - Assert.Equal(2, target.Panel.Children.Count); - target.SelectedIndex = 0; - Assert.Equal(2, target.ItemContainerGenerator.Containers.Count()); - Assert.Equal(2, target.Panel.Children.Count); - } + target.ApplyTemplate(); + AssertSingle(target); - [Fact] - public void Should_Remove_Controls_When_IsVirtualized_Is_False() - { - ObservableCollection items = new ObservableCollection(); - - var target = new CarouselPresenter + target.SelectedIndex = 1; + AssertSingle(target); + } + + [Fact] + public void Should_Remove_NonCurrent_Page() { - Items = items, - SelectedIndex = 0, - IsVirtualized = false, - }; + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + IsVirtualized = true, + SelectedIndex = 0, + }; - target.ApplyTemplate(); - target.SelectedIndex = 0; - items.Add("foo"); - target.SelectedIndex = 0; + target.ApplyTemplate(); + AssertSingle(target); - Assert.Single(target.ItemContainerGenerator.Containers); - Assert.Single(target.Panel.Children); + target.SelectedIndex = 1; + AssertSingle(target); - items.Add("bar"); - Assert.Single(target.ItemContainerGenerator.Containers); - Assert.Single(target.Panel.Children); + } - target.SelectedIndex = 1; - Assert.Equal(2, target.ItemContainerGenerator.Containers.Count()); - Assert.Equal(2, target.Panel.Children.Count); + [Fact] + public void Should_Handle_Inserting_Item_At_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - items.Remove(items[0]); - Assert.Single(target.ItemContainerGenerator.Containers); - Assert.Single(target.Panel.Children); + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; - items.Remove(items[0]); - Assert.Empty(target.ItemContainerGenerator.Containers); - Assert.Empty(target.Panel.Children); - } + target.ApplyTemplate(); - [Fact] - public void Should_Have_Correct_ItemsContainer_Index() - { - ObservableCollection items = new ObservableCollection(); + items.Insert(1, "item1a"); + AssertSingle(target); + } - var target = new CarouselPresenter + [Fact] + public void Should_Handle_Inserting_Item_Before_SelectedItem() { - Items = items, - SelectedIndex = 0, - IsVirtualized = false, - }; + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - target.ApplyTemplate(); - target.SelectedIndex = 0; - items.Add("foo"); - target.SelectedIndex = 0; - - Assert.Single(target.ItemContainerGenerator.Containers); - Assert.Single(target.Panel.Children); - - items.Add("bar"); - Assert.Single(target.ItemContainerGenerator.Containers); - Assert.Single(target.Panel.Children); - - target.SelectedIndex = 1; - Assert.Equal(2, target.ItemContainerGenerator.Containers.Count()); - Assert.Equal(2, target.Panel.Children.Count); - Assert.Equal(0, target.ItemContainerGenerator.Containers.First().Index); - - items.Remove(items[0]); - Assert.Single(target.ItemContainerGenerator.Containers); - Assert.Single(target.Panel.Children); - Assert.Equal(0, target.ItemContainerGenerator.Containers.First().Index); - - items.Remove(items[0]); - Assert.Empty(target.ItemContainerGenerator.Containers); - Assert.Empty(target.Panel.Children); - } + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 2, + IsVirtualized = true, + }; - [Fact] - public void Should_Handle_Inserting_Items_At_SelectedItem() - { - ObservableCollection items = new ObservableCollection + target.ApplyTemplate(); + + items.Insert(1, "item1a"); + AssertSingle(target); + } + + [Fact] + public void Should_Do_Nothing_When_Inserting_Item_After_SelectedItem() { - "item1", - "item2", - "item3", - }; + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - var target = new CarouselPresenter + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; + + target.ApplyTemplate(); + var child = AssertSingle(target); + items.Insert(2, "after"); + Assert.Same(child, AssertSingle(target)); + } + + [Fact] + public void Should_Handle_Removing_Item_At_SelectedItem() { - Items = items, - SelectedIndex = 0, - IsVirtualized = false, - }; + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - target.ApplyTemplate(); - target.SelectedIndex = 1; + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; - items.Insert(1, "item1a"); + target.ApplyTemplate(); - var containers = target.ItemContainerGenerator.Containers.Select(x => (x.Index, (string)x.Item)); + items.RemoveAt(1); + AssertSingle(target); + } - Assert.Equal( - new[] + [Fact] + public void Should_Handle_Removing_Item_Before_SelectedItem() + { + var items = new ObservableCollection { - (0, "item1"), - (1, "item1a"), - (2, "item2"), - }, - containers); + "item0", + "item1", + "item2", + }; - var visibleContainer = (ContentPresenter)target.Panel.Children.Single(x => x.IsVisible); - Assert.Equal("item1a", visibleContainer.Content); - } + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; - [Fact] - public void Should_Handle_Inserting_Items_Before_SelectedItem() - { - ObservableCollection items = new ObservableCollection + target.ApplyTemplate(); + + items.RemoveAt(0); + AssertSingle(target); + } + + [Fact] + public void Should_Do_Nothing_When_Removing_Item_After_SelectedItem() { - "item1", - "item2", - "item3", - }; + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - var target = new CarouselPresenter + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; + + target.ApplyTemplate(); + var child = AssertSingle(target); + items.RemoveAt(2); + Assert.Same(child, AssertSingle(target)); + } + + [Fact] + public void Should_Handle_Removing_SelectedItem_When_Its_Last() { - Items = items, - SelectedIndex = 0, - IsVirtualized = false, - }; + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - target.ApplyTemplate(); - target.SelectedIndex = 2; + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 2, + IsVirtualized = true, + }; + + target.ApplyTemplate(); + + items.RemoveAt(2); + Assert.Equal(1, target.SelectedIndex); + AssertSingle(target); + } + + [Fact] + public void Should_Handle_Removing_Last_Item() + { + var items = new ObservableCollection + { + "item0", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 0, + IsVirtualized = true, + }; + + target.ApplyTemplate(); + + items.RemoveAt(0); + Assert.Empty(target.Panel.Children); + Assert.Empty(target.ItemContainerGenerator.Containers); + } + + [Fact] + public void Should_Handle_Replacing_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; + + target.ApplyTemplate(); + + items[1] = "replaced"; + AssertSingle(target); + } + + [Fact] + public void Should_Do_Nothing_When_Replacing_Non_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - items.Insert(1, "item1a"); + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; + + target.ApplyTemplate(); + var child = AssertSingle(target); + items[0] = "replaced"; + Assert.Same(child, AssertSingle(target)); + } - var actual = target.ItemContainerGenerator.Containers.Select(x => (x.Index, (string)x.Item)); + [Fact] + public void Should_Handle_Moving_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; - Assert.Equal( - new[] + var target = new CarouselPresenter { - (0, "item1"), - (3, "item3"), - }, - actual); + Items = items, + SelectedIndex = 1, + IsVirtualized = true, + }; + + target.ApplyTemplate(); + + items.Move(1, 0); + AssertSingle(target); + } - var visibleContainer = (ContentPresenter)target.Panel.Children.Single(x => x.IsVisible); - Assert.Equal("item3", visibleContainer.Content); + private static IControl AssertSingle(CarouselPresenter target) + { + var items = (IList)target.Items; + var index = target.SelectedIndex; + var content = items[index]; + var child = Assert.Single(target.Panel.Children); + var presenter = Assert.IsType(child); + var container = Assert.Single(target.ItemContainerGenerator.Containers); + var visible = Assert.Single(target.Panel.Children.Where(x => x.IsVisible)); + + Assert.Same(child, container.ContainerControl); + Assert.Same(child, visible); + Assert.Equal(content, presenter.Content); + Assert.Equal(content, container.Item); + Assert.Equal(index, container.Index); + + return child; + } } - [Fact] - public void Should_Handle_Inserting_Items_After_SelectedItem() + public class NonVirtualized { - ObservableCollection items = new ObservableCollection + [Fact] + public void Should_Initially_Materialize_All_Containers() { - "item1", - "item2", - "item3", - }; + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + IsVirtualized = false, + }; - var target = new CarouselPresenter + target.ApplyTemplate(); + AssertAll(target); + } + + [Fact] + public void Should_Initially_Show_Selected_Item() { - Items = items, - SelectedIndex = 0, - IsVirtualized = false, - }; + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + SelectedIndex = 1, + IsVirtualized = false, + }; - target.ApplyTemplate(); - target.SelectedIndex = 1; - target.SelectedIndex = 0; + target.ApplyTemplate(); + AssertAll(target); + } + + [Fact] + public void Switching_To_Non_Virtualized_Should_Reset_Containers() + { + var target = new CarouselPresenter + { + Items = new[] { "foo", "bar" }, + SelectedIndex = 0, + IsVirtualized = true, + }; - items.Insert(1, "item1a"); + target.ApplyTemplate(); + target.IsVirtualized = false; - var actual = target.ItemContainerGenerator.Containers.Select(x => (x.Index, (string)x.Item)); + AssertAll(target); + } - Assert.Equal( - new[] + [Fact] + public void Changing_SelectedIndex_Should_Show_Page() + { + var target = new CarouselPresenter { - (0, "item1"), - (2, "item2"), - }, - actual); + Items = new[] { "foo", "bar" }, + SelectedIndex = 0, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + AssertAll(target); + + target.SelectedIndex = 1; + AssertAll(target); + } + + [Fact] + public void Should_Handle_Inserting_Item_At_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items.Insert(1, "item1a"); + AssertAll(target); + } + + [Fact] + public void Should_Handle_Inserting_Item_Before_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 2, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items.Insert(1, "item1a"); + AssertAll(target); + } + + [Fact] + public void Should_Do_Handle_Inserting_Item_After_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + items.Insert(2, "after"); + AssertAll(target); + } + + [Fact] + public void Should_Handle_Removing_Item_At_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items.RemoveAt(1); + AssertAll(target); + } + + [Fact] + public void Should_Handle_Removing_Item_Before_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items.RemoveAt(0); + AssertAll(target); + } + + [Fact] + public void Should_Handle_Removing_Item_After_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + items.RemoveAt(2); + AssertAll(target); + } + + [Fact] + public void Should_Handle_Removing_SelectedItem_When_Its_Last() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 2, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items.RemoveAt(2); + Assert.Equal(1, target.SelectedIndex); + AssertAll(target); + } + + [Fact] + public void Should_Handle_Removing_Last_Item() + { + var items = new ObservableCollection + { + "item0", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 0, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items.RemoveAt(0); + Assert.Empty(target.Panel.Children); + Assert.Empty(target.ItemContainerGenerator.Containers); + } + + [Fact] + public void Should_Handle_Replacing_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items[1] = "replaced"; + AssertAll(target); + } + + [Fact] + public void Should_Handle_Moving_SelectedItem() + { + var items = new ObservableCollection + { + "item0", + "item1", + "item2", + }; + + var target = new CarouselPresenter + { + Items = items, + SelectedIndex = 1, + IsVirtualized = false, + }; + + target.ApplyTemplate(); + + items.Move(1, 0); + AssertAll(target); + } + + private static void AssertAll(CarouselPresenter target) + { + var items = (IList)target.Items; + + Assert.Equal(items?.Count ?? 0, target.Panel.Children.Count); + Assert.Equal(items?.Count ?? 0, target.ItemContainerGenerator.Containers.Count()); + + for (var i = 0; i < items?.Count; ++i) + { + var content = items[i]; + var child = target.Panel.Children[i]; + var presenter = Assert.IsType(child); + var container = target.ItemContainerGenerator.ContainerFromIndex(i); + + Assert.Same(child, container); + Assert.Equal(i == target.SelectedIndex, child.IsVisible); + Assert.Equal(content, presenter.Content); + Assert.Equal(i, target.ItemContainerGenerator.IndexFromContainer(container)); + } + } } private class TestItem : ContentControl From e63fc96c848d9f89ec9dc49f15514defdc6a76ff Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 5 May 2019 21:12:01 +0200 Subject: [PATCH 4/4] Fix failing test. And remove empty method. --- src/Avalonia.Controls/Presenters/CarouselPresenter.cs | 5 ----- .../Primitives/SelectingItemsControl.cs | 11 ++++++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs index bdc4d29400..f887b987e0 100644 --- a/src/Avalonia.Controls/Presenters/CarouselPresenter.cs +++ b/src/Avalonia.Controls/Presenters/CarouselPresenter.cs @@ -94,11 +94,6 @@ namespace Avalonia.Controls.Presenters set { SetValue(PageTransitionProperty, value); } } - protected override void PanelCreated(IPanel panel) - { - base.PanelCreated(panel); - } - /// protected override void ItemsChanged(NotifyCollectionChangedEventArgs e) { diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index a64dbe0546..280c3ad93a 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -325,14 +325,19 @@ namespace Avalonia.Controls.Primitives if (_updateCount == 0) { + var newIndex = -1; + if (SelectedIndex != -1) { - SelectedIndex = IndexOf((IEnumerable)e.NewValue, SelectedItem); + newIndex = IndexOf((IEnumerable)e.NewValue, SelectedItem); } - else if (AlwaysSelected && Items != null && Items.Cast().Any()) + + if (AlwaysSelected && Items != null && Items.Cast().Any()) { - SelectedIndex = 0; + newIndex = 0; } + + SelectedIndex = newIndex; } }