From b024bb463b9f729b3bc176e9917cd14acbb6d46d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Aug 2023 13:41:30 +0200 Subject: [PATCH 1/6] Add tests for #12620. One failing. --- .../ItemsControlTests.cs | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs index 58e4ec1b75..0a8ea7c7b2 100644 --- a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs @@ -14,6 +14,7 @@ using Avalonia.Input; using Avalonia.Layout; using Avalonia.LogicalTree; using Avalonia.Markup.Xaml.Templates; +using Avalonia.Media; using Avalonia.Styling; using Avalonia.UnitTests; using Avalonia.VisualTree; @@ -128,6 +129,102 @@ namespace Avalonia.Controls.UnitTests Assert.Same(container.Theme, theme); } + [Fact] + public void ItemContainerTheme_Can_Be_Changed() + { + using var app = Start(); + + var theme1 = new ControlTheme + { + TargetType = typeof(ContentPresenter), + Setters = { new Setter(ContentPresenter.BackgroundProperty, Brushes.Red) } + }; + + var theme2 = new ControlTheme + { + TargetType = typeof(ContentPresenter), + Setters = { new Setter(ContentPresenter.BackgroundProperty, Brushes.Green) } + }; + + var target = CreateTarget( + itemsSource: new[] { "Foo" }, + itemContainerTheme: theme1); + + var container = GetContainer(target); + + Assert.Same(container.Theme, theme1); + Assert.Equal(container.Background, Brushes.Red); + + target.ItemContainerTheme = theme2; + + container = GetContainer(target); + Assert.Same(container.Theme, theme2); + Assert.Equal(container.Background, Brushes.Green); + } + + [Fact] + public void ItemContainerTheme_Can_Be_Changed_Virtualizing() + { + using var app = Start(); + + var theme1 = new ControlTheme + { + TargetType = typeof(ContentPresenter), + Setters = { new Setter(ContentPresenter.BackgroundProperty, Brushes.Red) } + }; + + var theme2 = new ControlTheme + { + TargetType = typeof(ContentPresenter), + Setters = { new Setter(ContentPresenter.BackgroundProperty, Brushes.Green) } + }; + + var itemsPanel = new FuncTemplate(() => new VirtualizingStackPanel()); + var target = CreateTarget( + itemsSource: new[] { "Foo" }, + itemContainerTheme: theme1, + itemsPanel: itemsPanel); + + var container = GetContainer(target); + + Assert.Same(container.Theme, theme1); + Assert.Equal(container.Background, Brushes.Red); + + target.ItemContainerTheme = theme2; + Layout(target); + + container = GetContainer(target); + Assert.Same(container.Theme, theme2); + Assert.Equal(container.Background, Brushes.Green); + } + + [Fact] + public void ItemContainerTheme_Can_Be_Cleared() + { + using var app = Start(); + + var theme = new ControlTheme + { + TargetType = typeof(ContentPresenter), + Setters = { new Setter(ContentPresenter.BackgroundProperty, Brushes.Red) } + }; + + var target = CreateTarget( + itemsSource: new[] { "Foo" }, + itemContainerTheme: theme); + + var container = GetContainer(target); + + Assert.Same(container.Theme, theme); + Assert.Equal(container.Background, Brushes.Red); + + target.ItemContainerTheme = null; + + container = GetContainer(target); + Assert.Null(container.Theme); + Assert.Null(container.Background); + } + [Fact] public void Container_Should_Have_LogicalParent_Set_To_ItemsControl() { @@ -851,6 +948,7 @@ namespace Avalonia.Controls.UnitTests IList? itemsSource = null, ControlTheme? itemContainerTheme = null, IDataTemplate? itemTemplate = null, + ITemplate? itemsPanel = null, IEnumerable? dataTemplates = null, bool performLayout = true) { @@ -861,6 +959,7 @@ namespace Avalonia.Controls.UnitTests itemsSource: itemsSource, itemContainerTheme: itemContainerTheme, itemTemplate: itemTemplate, + itemsPanel: itemsPanel, dataTemplates: dataTemplates, performLayout: performLayout); } From 754bdd4220e91cae5c76466cc98d88c41c761075 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Aug 2023 13:44:00 +0200 Subject: [PATCH 2/6] Handle changing ItemContainerTheme properly. Previously only handled the case where the `ItemContainerTheme` was changing from null -> non-null. Fixes #12620 --- src/Avalonia.Controls/ItemsControl.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 83aa88a7b6..47b943ed2e 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -665,12 +665,10 @@ namespace Avalonia.Controls { var itemContainerTheme = ItemContainerTheme; - if (itemContainerTheme is not null && - !container.IsSet(ThemeProperty) && - StyledElement.GetStyleKey(container) == itemContainerTheme.TargetType) - { + if (itemContainerTheme is null) + container.Theme = null; + else if (GetStyleKey(container) == itemContainerTheme.TargetType) container.Theme = itemContainerTheme; - } if (item is not Control) container.DataContext = item; From f347eaf7112b0672eb3123fa9b36293be84fdb60 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Aug 2023 14:05:00 +0200 Subject: [PATCH 3/6] Accept base TargetType in ItemContainerTheme --- src/Avalonia.Controls/ItemsControl.cs | 2 +- .../ItemsControlTests.cs | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 47b943ed2e..e8bd3b91a3 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -667,7 +667,7 @@ namespace Avalonia.Controls if (itemContainerTheme is null) container.Theme = null; - else if (GetStyleKey(container) == itemContainerTheme.TargetType) + else if (itemContainerTheme.TargetType?.IsAssignableFrom(GetStyleKey(container)) == true) container.Theme = itemContainerTheme; if (item is not Control) diff --git a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs index 0a8ea7c7b2..a0feabec61 100644 --- a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs @@ -129,6 +129,20 @@ namespace Avalonia.Controls.UnitTests Assert.Same(container.Theme, theme); } + [Fact] + public void Container_Should_Have_Theme_Set_To_ItemContainerTheme_With_Base_TargetType() + { + using var app = Start(); + var theme = new ControlTheme { TargetType = typeof(Control) }; + var target = CreateTarget( + itemsSource: new[] { "Foo" }, + itemContainerTheme: theme); + + var container = GetContainer(target); + + Assert.Same(container.Theme, theme); + } + [Fact] public void ItemContainerTheme_Can_Be_Changed() { From 03ff228a6d3ba5f3b0ab7678332de52ca22b389f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Aug 2023 14:46:54 +0200 Subject: [PATCH 4/6] Preserve container themes set locally. --- src/Avalonia.Controls/ItemsControl.cs | 28 ++++++++++--- .../ItemsControlTests.cs | 39 +++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index e8bd3b91a3..b7785162ec 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -67,6 +67,9 @@ namespace Avalonia.Controls public static readonly StyledProperty DisplayMemberBindingProperty = AvaloniaProperty.Register(nameof(DisplayMemberBinding)); + private static readonly AttachedProperty AppliedItemContainerTheme = + AvaloniaProperty.RegisterAttached("HasAppliedItemContainerTheme"); + /// /// Gets or sets the to use for binding to the display member of each item. /// @@ -663,12 +666,27 @@ namespace Avalonia.Controls internal void PrepareItemContainer(Control container, object? item, int index) { - var itemContainerTheme = ItemContainerTheme; + // If the container has no theme set, or we've already applied our ItemContainerTheme + // (and it hasn't changed since) then we're in control of the container's Theme and may + // need to update it. + if (!container.IsSet(ThemeProperty) || container.GetValue(AppliedItemContainerTheme) == container.Theme) + { + var itemContainerTheme = ItemContainerTheme; - if (itemContainerTheme is null) - container.Theme = null; - else if (itemContainerTheme.TargetType?.IsAssignableFrom(GetStyleKey(container)) == true) - container.Theme = itemContainerTheme; + if (itemContainerTheme?.TargetType?.IsAssignableFrom(GetStyleKey(container)) == true) + { + // We have an ItemContainerTheme and it matches the container. Set the Theme + // property, and mark the container as having had ItemContainerTheme applied. + container.SetCurrentValue(ThemeProperty, itemContainerTheme); + container.SetValue(AppliedItemContainerTheme, itemContainerTheme); + } + else + { + // Otherwise clear the theme and the HasAppliedItemContainerTheme property. + container.ClearValue(ThemeProperty); + container.ClearValue(AppliedItemContainerTheme); + } + } if (item is not Control) container.DataContext = item; diff --git a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs index a0feabec61..7d3c1fe4f4 100644 --- a/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ItemsControlTests.cs @@ -239,6 +239,45 @@ namespace Avalonia.Controls.UnitTests Assert.Null(container.Background); } + [Fact] + public void ItemContainerTheme_Should_Not_Override_LocalValue_Theme() + { + using var app = Start(); + + var theme1 = new ControlTheme + { + TargetType = typeof(ContentPresenter), + Setters = { new Setter(ContentPresenter.BackgroundProperty, Brushes.Red) } + }; + + var theme2 = new ControlTheme + { + TargetType = typeof(Control), + Setters = { new Setter(ContentPresenter.BackgroundProperty, Brushes.Green) } + }; + + var items = new object[] + { + new ContentPresenter(), + new ContentPresenter + { + Theme = theme2 + }, + }; + + var target = CreateTarget( + itemsSource: items, + itemContainerTheme: theme1); + + Assert.Same(theme1, GetContainer(target, 0).Theme); + Assert.Same(theme2, GetContainer(target, 1).Theme); + + target.ItemContainerTheme = null; + + Assert.Null(GetContainer(target, 0).Theme); + Assert.Same(theme2, GetContainer(target, 1).Theme); + } + [Fact] public void Container_Should_Have_LogicalParent_Set_To_ItemsControl() { From a48e4e7cdcaf8100a0b5692fc2d64066b1203e4c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Aug 2023 14:51:25 +0200 Subject: [PATCH 5/6] Fix comment. --- src/Avalonia.Controls/ItemsControl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index b7785162ec..8c40cc8335 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -682,7 +682,7 @@ namespace Avalonia.Controls } else { - // Otherwise clear the theme and the HasAppliedItemContainerTheme property. + // Otherwise clear the theme and the AppliedItemContainerTheme property. container.ClearValue(ThemeProperty); container.ClearValue(AppliedItemContainerTheme); } From f8ea38c90e78f3607adbd88cbffc1322efc57faa Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Aug 2023 14:55:59 +0200 Subject: [PATCH 6/6] Fix property name. --- src/Avalonia.Controls/ItemsControl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 8c40cc8335..41e9e7c111 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -68,7 +68,7 @@ namespace Avalonia.Controls AvaloniaProperty.Register(nameof(DisplayMemberBinding)); private static readonly AttachedProperty AppliedItemContainerTheme = - AvaloniaProperty.RegisterAttached("HasAppliedItemContainerTheme"); + AvaloniaProperty.RegisterAttached("AppliedItemContainerTheme"); /// /// Gets or sets the to use for binding to the display member of each item.