From 7345336b9a81fa9d41ddae9084e93c8e30014f2b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 19 Feb 2020 10:51:45 +0100 Subject: [PATCH 1/4] Added failing unit tests for #3590. --- .../DynamicResourceExtensionTests.cs | 131 ++++++++++++++++++ .../StyledElementTests.cs | 47 +++++++ .../StyledElementTests_Resources.cs | 19 ++- 3 files changed, 196 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs index 96955539c1..adeae810c6 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs @@ -627,6 +627,137 @@ namespace Avalonia.Markup.Xaml.UnitTests.MarkupExtensions Assert.Equal(0xff506070, brush.Color.ToUint32()); } + [Fact] + public void Resource_With_DynamicResource_Is_Updated_When_Added_To_Parent() + { + var xaml = @" + + + + + + +"; + + var loader = new AvaloniaXamlLoader(); + var userControl = (UserControl)loader.Load(xaml); + var border = userControl.FindControl("border"); + + DelayedBinding.ApplyBindings(border); + + var brush = (SolidColorBrush)border.Background; + Assert.Equal(0u, brush.Color.ToUint32()); + + brush.GetObservable(SolidColorBrush.ColorProperty).Subscribe(_ => { }); + + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window + { + Resources = + { + { "color", Colors.Red } + }, + Content = userControl, + }; + + window.Show(); + + Assert.Equal(Colors.Red, brush.Color); + } + } + + [Fact] + public void MergedDictionary_Resource_With_DynamicResource_Is_Updated_When_Added_To_Parent() + { + var xaml = @" + + + + + + + + + + + + +"; + + var loader = new AvaloniaXamlLoader(); + var userControl = (UserControl)loader.Load(xaml); + var border = userControl.FindControl("border"); + + DelayedBinding.ApplyBindings(border); + + var brush = (SolidColorBrush)border.Background; + Assert.Equal(0u, brush.Color.ToUint32()); + + brush.GetObservable(SolidColorBrush.ColorProperty).Subscribe(_ => { }); + + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window + { + Resources = + { + { "color", Colors.Red } + }, + Content = userControl, + }; + + window.Show(); + + Assert.Equal(Colors.Red, brush.Color); + } + } + + [Fact] + public void Style_Resource_With_DynamicResource_Is_Updated_When_Added_To_Parent() + { + var xaml = @" + + + + + + +"; + + var loader = new AvaloniaXamlLoader(); + var userControl = (UserControl)loader.Load(xaml); + var border = userControl.FindControl("border"); + + DelayedBinding.ApplyBindings(border); + + var brush = (SolidColorBrush)border.Background; + Assert.Equal(0u, brush.Color.ToUint32()); + + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window + { + Resources = + { + { "color", Colors.Red } + }, + Content = userControl, + }; + + window.Show(); + + Assert.Equal(Colors.Red, brush.Color); + } + } + private IDisposable StyledWindow(params (string, string)[] assets) { var services = TestServices.StyledWindow.With( diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs index ae519ed674..3a676529c7 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs @@ -489,6 +489,53 @@ namespace Avalonia.Styling.UnitTests called); } + [Fact] + public void Changing_Parent_Notifies_Resources_ParentResourcesChanged() + { + var resources = new Mock(); + var setResourceParent = resources.As(); + var target = new TestControl { Resources = resources.Object }; + var parent = new Decorator { Resources = { { "foo", "bar" } } }; + + setResourceParent.ResetCalls(); + parent.Child = target; + + setResourceParent.Verify(x => + x.ParentResourcesChanged(It.IsAny()), + Times.Once); + } + + [Fact] + public void Changing_Parent_Notifies_Styles_ParentResourcesChanged() + { + var style = new Mock(); + var setResourceParent = style.As(); + var target = new TestControl { Styles = { style.Object } }; + var parent = new Decorator { Resources = { { "foo", "bar" } } }; + + setResourceParent.ResetCalls(); + parent.Child = target; + + setResourceParent.Verify(x => + x.ParentResourcesChanged(It.IsAny()), + Times.Once); + } + + [Fact] + public void Changing_Resources_Notifies_Styles() + { + var style = new Mock(); + var setResourceParent = style.As(); + var target = new TestControl { Styles = { style.Object } }; + + setResourceParent.ResetCalls(); + target.Resources.Add("foo", "bar"); + + setResourceParent.Verify(x => + x.ParentResourcesChanged(It.IsAny()), + Times.Once); + } + private interface IDataContextEvents { event EventHandler DataContextBeginUpdate; diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs index b8d53b38b2..ee275ca4ba 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs @@ -205,7 +205,7 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void Setting_Logical_Parent_Subscribes_To_Parents_ResourceChanged_Event() + public void Setting_Logical_Parent_Raises_Child_ResourcesChanged() { var parent = new ContentControl(); var child = new StyledElement(); @@ -220,6 +220,23 @@ namespace Avalonia.Controls.UnitTests Assert.True(raisedOnChild); } + [Fact] + public void Setting_Logical_Parent_Raises_Style_ResourcesChanged() + { + var style = new Style(x => x.OfType()); + var parent = new ContentControl(); + var child = new StyledElement { Styles = { style } }; + + ((ISetLogicalParent)child).SetParent(parent); + var raised = false; + + style.ResourcesChanged += (_, __) => raised = true; + + parent.Resources.Add("foo", "bar"); + + Assert.True(raised); + } + private IControlTemplate ContentControlTemplate() { return new FuncControlTemplate((x, scope) => From 4f2c2159938a4edab0f7ccac33f0cdcf80885662 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 20 Feb 2020 14:27:33 +0100 Subject: [PATCH 2/4] Pass ResourceChanged messages to child styles. Fixes #3590. --- src/Avalonia.Styling/StyledElement.cs | 40 ++++++++++--- src/Avalonia.Styling/Styling/Styles.cs | 40 +++++++------ .../StyledElementTests.cs | 60 +++++++++++++++++++ .../Avalonia.Styling.UnitTests/StylesTests.cs | 26 ++++---- 4 files changed, 125 insertions(+), 41 deletions(-) diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index 120a53c664..e9e9ca4d5a 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -67,6 +67,7 @@ namespace Avalonia private Subject _styleDetach = new Subject(); private ITemplatedControl _templatedParent; private bool _dataContextUpdating; + private bool _notifyingResourcesChanged; /// /// Initializes static members of the class. @@ -235,6 +236,7 @@ namespace Avalonia } _styles.ResourcesChanged += ThisResourcesChanged; + NotifyResourcesChanged(new ResourcesChangedEventArgs()); } } } @@ -253,6 +255,7 @@ namespace Avalonia if (_resources != null) { + (_resources as ISetResourceParent)?.SetParent(null); hadResources = _resources.Count > 0; _resources.ResourcesChanged -= ThisResourcesChanged; } @@ -260,9 +263,14 @@ namespace Avalonia _resources = value; _resources.ResourcesChanged += ThisResourcesChanged; + if (value is ISetResourceParent setParent && setParent.ResourceParent == null) + { + setParent.SetParent(this); + } + if (hadResources || _resources.Count > 0) { - ((ILogical)this).NotifyResourcesChanged(new ResourcesChangedEventArgs()); + NotifyResourcesChanged(new ResourcesChangedEventArgs()); } } } @@ -407,10 +415,7 @@ namespace Avalonia } /// - void ILogical.NotifyResourcesChanged(ResourcesChangedEventArgs e) - { - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); - } + void ILogical.NotifyResourcesChanged(ResourcesChangedEventArgs e) => NotifyResourcesChanged(e); /// bool IResourceProvider.TryGetResource(object key, out object value) @@ -456,7 +461,8 @@ namespace Avalonia { Parent.ResourcesChanged += ThisResourcesChanged; } - ((ILogical)this).NotifyResourcesChanged(new ResourcesChangedEventArgs()); + + NotifyResourcesChanged(new ResourcesChangedEventArgs()); if (Parent is ILogicalRoot || Parent?.IsAttachedToLogicalTree == true || this is ILogicalRoot) { @@ -721,9 +727,29 @@ namespace Avalonia } } + private void NotifyResourcesChanged(ResourcesChangedEventArgs e) + { + if (_notifyingResourcesChanged) + { + return; + } + + try + { + _notifyingResourcesChanged = true; + (_resources as ISetResourceParent)?.ParentResourcesChanged(e); + (_styles as ISetResourceParent)?.ParentResourcesChanged(e); + ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); + } + finally + { + _notifyingResourcesChanged = false; + } + } + private void ThisResourcesChanged(object sender, ResourcesChangedEventArgs e) { - ((ILogical)this).NotifyResourcesChanged(e); + NotifyResourcesChanged(e); } } } diff --git a/src/Avalonia.Styling/Styling/Styles.cs b/src/Avalonia.Styling/Styling/Styles.cs index fd38c39650..1929c24017 100644 --- a/src/Avalonia.Styling/Styling/Styles.cs +++ b/src/Avalonia.Styling/Styling/Styles.cs @@ -20,6 +20,7 @@ namespace Avalonia.Styling private IResourceDictionary _resources; private AvaloniaList _styles = new AvaloniaList(); private Dictionary> _cache; + private bool _notifyingResourcesChanged; public Styles() { @@ -38,7 +39,7 @@ namespace Avalonia.Styling ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); } - x.ResourcesChanged += SubResourceChanged; + x.ResourcesChanged += NotifyResourcesChanged; _cache = null; }, x => @@ -54,7 +55,7 @@ namespace Avalonia.Styling ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); } - x.ResourcesChanged -= SubResourceChanged; + x.ResourcesChanged -= NotifyResourcesChanged; _cache = null; }, () => { }); @@ -90,11 +91,11 @@ namespace Avalonia.Styling if (_resources != null) { hadResources = _resources.Count > 0; - _resources.ResourcesChanged -= ResourceDictionaryChanged; + _resources.ResourcesChanged -= NotifyResourcesChanged; } _resources = value; - _resources.ResourcesChanged += ResourceDictionaryChanged; + _resources.ResourcesChanged += NotifyResourcesChanged; if (hadResources || _resources.Count > 0) { @@ -261,34 +262,35 @@ namespace Avalonia.Styling /// void ISetResourceParent.ParentResourcesChanged(ResourcesChangedEventArgs e) { - ResourcesChanged?.Invoke(this, e); + NotifyResourcesChanged(e); } - private void ResourceDictionaryChanged(object sender, ResourcesChangedEventArgs e) + private void NotifyResourcesChanged(object sender, ResourcesChangedEventArgs e) { - foreach (var child in this) - { - (child as ISetResourceParent)?.ParentResourcesChanged(e); - } - - ResourcesChanged?.Invoke(this, e); + NotifyResourcesChanged(e); } - private void SubResourceChanged(object sender, ResourcesChangedEventArgs e) + private void NotifyResourcesChanged(ResourcesChangedEventArgs e) { - var foundSource = false; + if (_notifyingResourcesChanged) + { + return; + } - foreach (var child in this) + try { - if (foundSource) + _notifyingResourcesChanged = true; + foreach (var child in this) { (child as ISetResourceParent)?.ParentResourcesChanged(e); } - foundSource |= child == sender; + ResourcesChanged?.Invoke(this, e); + } + finally + { + _notifyingResourcesChanged = false; } - - ResourcesChanged?.Invoke(this, e); } } } diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs index 3a676529c7..d349d9c3e0 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs @@ -489,6 +489,36 @@ namespace Avalonia.Styling.UnitTests called); } + [Fact] + public void Resources_Parent_Is_Set() + { + var target = new TestControl(); + + Assert.Same(target, ((IResourceNode)target.Resources).ResourceParent); + } + + [Fact] + public void Assigned_Resources_Parent_Is_Set() + { + var resources = new ResourceDictionary(); + var target = new TestControl { Resources = resources }; + + Assert.Same(target, ((IResourceNode)resources).ResourceParent); + } + + [Fact] + public void Assigning_Resources_Raises_ResourcesChanged() + { + var resources = new ResourceDictionary { { "foo", "bar" } }; + var target = new TestControl(); + var raised = 0; + + target.ResourcesChanged += (s, e) => ++raised; + target.Resources = resources; + + Assert.Equal(1, raised); + } + [Fact] public void Changing_Parent_Notifies_Resources_ParentResourcesChanged() { @@ -505,6 +535,36 @@ namespace Avalonia.Styling.UnitTests Times.Once); } + [Fact] + public void Styles_Parent_Is_Set() + { + var target = new TestControl(); + + Assert.Same(target, ((IResourceNode)target.Styles).ResourceParent); + } + + [Fact] + public void Assigned_Styles_Parent_Is_Set() + { + var styles = new Styles(); + var target = new TestControl { Styles = styles }; + + Assert.Same(target, ((IResourceNode)styles).ResourceParent); + } + + [Fact] + public void Assigning_Styles_Raises_ResourcesChanged() + { + var styles = new Styles { Resources = { { "foo", "bar" } } }; + var target = new TestControl(); + var raised = 0; + + target.ResourcesChanged += (s, e) => ++raised; + target.Styles = styles; + + Assert.Equal(1, raised); + } + [Fact] public void Changing_Parent_Notifies_Styles_ParentResourcesChanged() { diff --git a/tests/Avalonia.Styling.UnitTests/StylesTests.cs b/tests/Avalonia.Styling.UnitTests/StylesTests.cs index 82b6b81759..e921763495 100644 --- a/tests/Avalonia.Styling.UnitTests/StylesTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StylesTests.cs @@ -3,6 +3,7 @@ using System; using Avalonia.Controls; +using Moq; using Xunit; namespace Avalonia.Styling.UnitTests @@ -76,7 +77,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void Adding_Resource_To_Younger_Sibling_Style_Should_Raise_ResourceChanged() + public void Adding_Resource_To_Sibling_Style_Should_Raise_ResourceChanged() { Style style1; Style style2; @@ -95,25 +96,20 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void Adding_Resource_To_Older_Sibling_Style_Should_Raise_ResourceChanged() + public void ParentResourcesChanged_Should_Be_Propagated_To_Children() { - Style style1; - Style style2; - var target = new Styles - { - (style1 = new Style()), - (style2 = new Style()), - }; - - var raised = false; + var childStyle = new Mock(); + var setResourceParent = childStyle.As(); + var target = new Styles { childStyle.Object }; - style1.ResourcesChanged += (_, __) => raised = true; - style2.Resources.Add("foo", "bar"); + setResourceParent.ResetCalls(); + ((ISetResourceParent)target).ParentResourcesChanged(new ResourcesChangedEventArgs()); - Assert.False(raised); + setResourceParent.Verify(x => x.ParentResourcesChanged( + It.IsAny()), + Times.Once); } - [Fact] public void Finds_Resource_In_Merged_Dictionary() { From ac8e8db72bf4a607b5518068ffb5e329b47e7f44 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 25 Feb 2020 16:00:33 +0100 Subject: [PATCH 3/4] Make Application handle style resources properly. --- src/Avalonia.Controls/Application.cs | 38 ++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Application.cs b/src/Avalonia.Controls/Application.cs index c49f7c3d09..0ae615fe23 100644 --- a/src/Avalonia.Controls/Application.cs +++ b/src/Avalonia.Controls/Application.cs @@ -44,6 +44,7 @@ namespace Avalonia private readonly Styler _styler = new Styler(); private Styles _styles; private IResourceDictionary _resources; + private bool _notifyingResourcesChanged; /// /// Defines the property. @@ -160,7 +161,20 @@ namespace Avalonia /// /// Global styles apply to all windows in the application. /// - public Styles Styles => _styles ?? (_styles = new Styles()); + public Styles Styles + { + get + { + if (_styles == null) + { + _styles = new Styles(); + ((ISetResourceParent)_styles).SetParent(this); + _styles.ResourcesChanged += ThisResourcesChanged; + } + + return _styles; + } + } /// bool IDataTemplateHost.IsDataTemplatesInitialized => _dataTemplates != null; @@ -233,9 +247,29 @@ namespace Avalonia } + private void NotifyResourcesChanged(ResourcesChangedEventArgs e) + { + if (_notifyingResourcesChanged) + { + return; + } + + try + { + _notifyingResourcesChanged = true; + (_resources as ISetResourceParent)?.ParentResourcesChanged(e); + (_styles as ISetResourceParent)?.ParentResourcesChanged(e); + ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); + } + finally + { + _notifyingResourcesChanged = false; + } + } + private void ThisResourcesChanged(object sender, ResourcesChangedEventArgs e) { - ResourcesChanged?.Invoke(this, e); + NotifyResourcesChanged(e); } private string _name; From 097eeb646fe83c6c9651ddd8110cdcc7a248825f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 25 Feb 2020 16:21:34 +0100 Subject: [PATCH 4/4] Don't allow StyledElement.Styles to be set. It's not settable in Application and I can't see a reason it should be in `StyledElement`. Also add a `Styles` ctor that takes a parent. --- src/Avalonia.Controls/Application.cs | 3 +-- src/Avalonia.Styling/StyledElement.cs | 24 ++++--------------- src/Avalonia.Styling/Styling/Styles.cs | 6 +++++ .../StyledElementTests.cs | 22 ----------------- 4 files changed, 12 insertions(+), 43 deletions(-) diff --git a/src/Avalonia.Controls/Application.cs b/src/Avalonia.Controls/Application.cs index 0ae615fe23..d7b1aec0b7 100644 --- a/src/Avalonia.Controls/Application.cs +++ b/src/Avalonia.Controls/Application.cs @@ -167,8 +167,7 @@ namespace Avalonia { if (_styles == null) { - _styles = new Styles(); - ((ISetResourceParent)_styles).SetParent(this); + _styles = new Styles(this); _styles.ResourcesChanged += ThisResourcesChanged; } diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index e9e9ca4d5a..f15f38a5c0 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -215,29 +215,15 @@ namespace Avalonia /// public Styles Styles { - get { return _styles ?? (Styles = new Styles()); } - set + get { - Contract.Requires(value != null); - - if (_styles != value) + if (_styles == null) { - if (_styles != null) - { - (_styles as ISetResourceParent)?.SetParent(null); - _styles.ResourcesChanged -= ThisResourcesChanged; - } - - _styles = value; - - if (value is ISetResourceParent setParent && setParent.ResourceParent == null) - { - setParent.SetParent(this); - } - + _styles = new Styles(this); _styles.ResourcesChanged += ThisResourcesChanged; - NotifyResourcesChanged(new ResourcesChangedEventArgs()); } + + return _styles; } } diff --git a/src/Avalonia.Styling/Styling/Styles.cs b/src/Avalonia.Styling/Styling/Styles.cs index 1929c24017..bef4ef2ee0 100644 --- a/src/Avalonia.Styling/Styling/Styles.cs +++ b/src/Avalonia.Styling/Styling/Styles.cs @@ -61,6 +61,12 @@ namespace Avalonia.Styling () => { }); } + public Styles(IResourceNode parent) + : this() + { + _parent = parent; + } + public event NotifyCollectionChangedEventHandler CollectionChanged { add => _styles.CollectionChanged += value; diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs index d349d9c3e0..c6c48f9aa8 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs @@ -543,28 +543,6 @@ namespace Avalonia.Styling.UnitTests Assert.Same(target, ((IResourceNode)target.Styles).ResourceParent); } - [Fact] - public void Assigned_Styles_Parent_Is_Set() - { - var styles = new Styles(); - var target = new TestControl { Styles = styles }; - - Assert.Same(target, ((IResourceNode)styles).ResourceParent); - } - - [Fact] - public void Assigning_Styles_Raises_ResourcesChanged() - { - var styles = new Styles { Resources = { { "foo", "bar" } } }; - var target = new TestControl(); - var raised = 0; - - target.ResourcesChanged += (s, e) => ++raised; - target.Styles = styles; - - Assert.Equal(1, raised); - } - [Fact] public void Changing_Parent_Notifies_Styles_ParentResourcesChanged() {