From 77d5ba92c03aa58288bf0d50ef805f7d6487e67a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 17 May 2020 12:09:11 +0200 Subject: [PATCH 01/11] Update BenchmarkDotNet, --- tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj b/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj index 5fc3dce671..251894e324 100644 --- a/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj +++ b/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj @@ -18,7 +18,7 @@ - + From 42859fae894a20553a2415f110235d9092ab97cb Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 17 May 2020 23:23:50 +0200 Subject: [PATCH 02/11] Refactored resources. Split resource nodes into two types: - `IResourceHost` represents controls and `Application` - `IResourceProvider` represents resource dictionaries and styles, these are owned by `IResourceHost`s Dynamic resources are now always resolved from an `IResourceHost`: if an `IResourceProvider` doesn't have a host then a dynamic resource in the resource provider will not be resolved. Resource providers no longer have a `ResourcesChanged` event, instead they notify their `IResourceHost` owner of a resource change by calling the `NotifyHostedResourcesChanged` method. --- src/Avalonia.Controls/Application.cs | 58 ++---- src/Avalonia.Controls/TopLevel.cs | 4 +- .../Controls/IResourceDictionary.cs | 4 +- .../Controls/IResourceHost.cs | 32 ++++ .../Controls/IResourceNode.cs | 28 ++- .../Controls/IResourceProvider.cs | 41 +++-- .../Controls/ISetResourceParent.cs | 27 --- .../Controls/ResourceDictionary.cs | 164 +++++++++-------- .../Controls/ResourceNodeExtensions.cs | 103 +++++++++-- src/Avalonia.Styling/IStyledElement.cs | 8 +- src/Avalonia.Styling/StyledElement.cs | 57 ++---- src/Avalonia.Styling/Styling/IStyle.cs | 2 +- src/Avalonia.Styling/Styling/IStyleHost.cs | 2 +- src/Avalonia.Styling/Styling/Style.cs | 71 ++++---- src/Avalonia.Styling/Styling/Styles.cs | 165 +++++++----------- .../DynamicResourceExtension.cs | 32 +++- .../MarkupExtensions/ResourceInclude.cs | 52 ++---- .../StaticResourceExtension.cs | 18 +- .../Styling/StyleInclude.cs | 66 +++---- .../Xaml/ResourceDictionaryTests.cs | 19 -- .../ResourceDictionaryTests.cs | 152 +++++++++++----- .../Avalonia.Styling.UnitTests/StyleTests.cs | 28 +++ .../StyledElementTests.cs | 59 ++----- .../StyledElementTests_Resources.cs | 17 -- .../Avalonia.Styling.UnitTests/StylesTests.cs | 111 ++++-------- 25 files changed, 683 insertions(+), 637 deletions(-) create mode 100644 src/Avalonia.Styling/Controls/IResourceHost.cs delete mode 100644 src/Avalonia.Styling/Controls/ISetResourceParent.cs diff --git a/src/Avalonia.Controls/Application.cs b/src/Avalonia.Controls/Application.cs index 02f47e07b4..f0fe98a4b7 100644 --- a/src/Avalonia.Controls/Application.cs +++ b/src/Avalonia.Controls/Application.cs @@ -30,7 +30,7 @@ namespace Avalonia /// method. /// - Tracks the lifetime of the application. /// - public class Application : AvaloniaObject, IDataContextProvider, IGlobalDataTemplates, IGlobalStyles, IResourceNode + public class Application : AvaloniaObject, IDataContextProvider, IGlobalDataTemplates, IGlobalStyles, IResourceHost { /// /// The application-global data templates. @@ -129,26 +129,13 @@ namespace Avalonia /// public IResourceDictionary Resources { - get => _resources ?? (Resources = new ResourceDictionary()); + get => _resources ??= new ResourceDictionary(this); set { - Contract.Requires(value != null); - - var hadResources = false; - - if (_resources != null) - { - hadResources = _resources.Count > 0; - _resources.ResourcesChanged -= ThisResourcesChanged; - } - + value = value ?? throw new ArgumentNullException(nameof(value)); + _resources?.RemoveOwner(this); _resources = value; - _resources.ResourcesChanged += ThisResourcesChanged; - - if (hadResources || _resources.Count > 0) - { - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); - } + _resources.AddOwner(this); } } @@ -161,23 +148,15 @@ namespace Avalonia /// /// Global styles apply to all windows in the application. /// - public Styles Styles - { - get - { - if (_styles == null) - { - _styles = new Styles(this); - _styles.ResourcesChanged += ThisResourcesChanged; - } - - return _styles; - } - } + public Styles Styles => _styles ??= new Styles(this); /// bool IDataTemplateHost.IsDataTemplatesInitialized => _dataTemplates != null; + /// + bool IResourceNode.HasResources => (_resources?.HasResources ?? false) || + (((IResourceNode?)_styles)?.HasResources ?? false); + /// /// Gets the styling parent of the application, which is null. /// @@ -185,13 +164,7 @@ namespace Avalonia /// bool IStyleHost.IsStylesInitialized => _styles != null; - - /// - bool IResourceProvider.HasResources => _resources?.Count > 0; - - /// - IResourceNode IResourceNode.ResourceParent => null; - + /// /// Application lifetime, use it for things like setting the main window and exiting the app from code /// Currently supported lifetimes are: @@ -219,13 +192,18 @@ namespace Avalonia public virtual void Initialize() { } /// - bool IResourceProvider.TryGetResource(object key, out object value) + bool IResourceNode.TryGetResource(object key, out object value) { value = null; return (_resources?.TryGetResource(key, out value) ?? false) || Styles.TryGetResource(key, out value); } + void IResourceHost.NotifyHostedResourcesChanged(ResourcesChangedEventArgs e) + { + ResourcesChanged?.Invoke(this, e); + } + void IStyleHost.StylesAdded(IReadOnlyList styles) { _stylesAdded?.Invoke(styles); @@ -282,8 +260,6 @@ namespace Avalonia try { _notifyingResourcesChanged = true; - (_resources as ISetResourceParent)?.ParentResourcesChanged(e); - (_styles as ISetResourceParent)?.ParentResourcesChanged(e); ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); } finally diff --git a/src/Avalonia.Controls/TopLevel.cs b/src/Avalonia.Controls/TopLevel.cs index d17f3b0423..b6eee290ce 100644 --- a/src/Avalonia.Controls/TopLevel.cs +++ b/src/Avalonia.Controls/TopLevel.cs @@ -127,11 +127,11 @@ namespace Avalonia.Controls x => (x as InputElement)?.GetObservable(CursorProperty) ?? Observable.Empty()) .Switch().Subscribe(cursor => PlatformImpl?.SetCursor(cursor?.PlatformCursor)); - if (((IStyleHost)this).StylingParent is IResourceProvider applicationResources) + if (((IStyleHost)this).StylingParent is IResourceHost applicationResources) { WeakSubscriptionManager.Subscribe( applicationResources, - nameof(IResourceProvider.ResourcesChanged), + nameof(IResourceHost.ResourcesChanged), this); } } diff --git a/src/Avalonia.Styling/Controls/IResourceDictionary.cs b/src/Avalonia.Styling/Controls/IResourceDictionary.cs index 240f9b08bf..3a68dde31e 100644 --- a/src/Avalonia.Styling/Controls/IResourceDictionary.cs +++ b/src/Avalonia.Styling/Controls/IResourceDictionary.cs @@ -1,11 +1,13 @@ using System.Collections.Generic; +#nullable enable + namespace Avalonia.Controls { /// /// An indexed dictionary of resources. /// - public interface IResourceDictionary : IResourceProvider, IDictionary + public interface IResourceDictionary : IResourceProvider, IDictionary { /// /// Gets a collection of child resource dictionaries. diff --git a/src/Avalonia.Styling/Controls/IResourceHost.cs b/src/Avalonia.Styling/Controls/IResourceHost.cs new file mode 100644 index 0000000000..fed9359d94 --- /dev/null +++ b/src/Avalonia.Styling/Controls/IResourceHost.cs @@ -0,0 +1,32 @@ +using System; + +#nullable enable + +namespace Avalonia.Controls +{ + /// + /// Represents an element which hosts resources. + /// + /// + /// This interface is implemented by and `Application`. + /// + public interface IResourceHost : IResourceNode + { + /// + /// Raised when the resources change on the element or an ancestor of the element. + /// + event EventHandler ResourcesChanged; + + /// + /// Notifies the resource host that one or more of its hosted resources has changed. + /// + /// The event args. + /// + /// This method will be called automatically by the framework, you should not need to call + /// this method yourself. It is called when the resources hosted by this element have + /// changed, and is usually called by a resource dictionary or style hosted by the element + /// in response to a resource being added or removed. + /// + void NotifyHostedResourcesChanged(ResourcesChangedEventArgs e); + } +} diff --git a/src/Avalonia.Styling/Controls/IResourceNode.cs b/src/Avalonia.Styling/Controls/IResourceNode.cs index b6a6cdc3e3..73bfeaf161 100644 --- a/src/Avalonia.Styling/Controls/IResourceNode.cs +++ b/src/Avalonia.Styling/Controls/IResourceNode.cs @@ -1,15 +1,35 @@ using System; +#nullable enable + namespace Avalonia.Controls { /// - /// Represents resource provider in a tree. + /// Represents an object that can be queried for resources. /// - public interface IResourceNode : IResourceProvider + /// + /// The interface represents a common interface for both controls that host resources + /// () and resource providers such as + /// (see ). + /// + public interface IResourceNode { /// - /// Gets the parent resource node, if any. + /// Gets a value indicating whether the object has resources. + /// + bool HasResources { get; } + + /// + /// Tries to find a resource within the object. /// - IResourceNode ResourceParent { get; } + /// The resource key. + /// + /// When this method returns, contains the value associated with the specified key, + /// if the key is found; otherwise, null. + /// + /// + /// True if the resource if found, otherwise false. + /// + bool TryGetResource(object key, out object? value); } } diff --git a/src/Avalonia.Styling/Controls/IResourceProvider.cs b/src/Avalonia.Styling/Controls/IResourceProvider.cs index cbaacee012..27a8c2da68 100644 --- a/src/Avalonia.Styling/Controls/IResourceProvider.cs +++ b/src/Avalonia.Styling/Controls/IResourceProvider.cs @@ -1,33 +1,42 @@ using System; +using Avalonia.Styling; + +#nullable enable namespace Avalonia.Controls { /// - /// Represents an object that can be queried for resources. + /// Represents an object that can be queried for resources but does not appear in the logical tree. /// - public interface IResourceProvider + /// + /// This interface is implemented by , and + /// + /// + public interface IResourceProvider : IResourceNode { /// - /// Raised when resources in the provider are changed. + /// Gets the owner of the resource provider. + /// + /// + /// If multiple owners are added, returns the first. + /// + IResourceHost? Owner { get; } + + /// + /// Raised when the of the resource provider changes. /// - event EventHandler ResourcesChanged; + event EventHandler OwnerChanged; /// - /// Gets a value indicating whether the element has resources. + /// Adds an owner to the resource provider. /// - bool HasResources { get; } + /// The owner. + void AddOwner(IResourceHost owner); /// - /// Tries to find a resource within the provider. + /// Removes a resource provider owner. /// - /// The resource key. - /// - /// When this method returns, contains the value associated with the specified key, - /// if the key is found; otherwise, null. - /// - /// - /// True if the resource if found, otherwise false. - /// - bool TryGetResource(object key, out object value); + /// The owner. + void RemoveOwner(IResourceHost owner); } } diff --git a/src/Avalonia.Styling/Controls/ISetResourceParent.cs b/src/Avalonia.Styling/Controls/ISetResourceParent.cs deleted file mode 100644 index a1264adc34..0000000000 --- a/src/Avalonia.Styling/Controls/ISetResourceParent.cs +++ /dev/null @@ -1,27 +0,0 @@ -namespace Avalonia.Controls -{ - /// - /// Defines an interface through which an 's parent can be set. - /// - /// - /// You should not usually need to use this interface - it is for internal use only. - /// - public interface ISetResourceParent : IResourceNode - { - /// - /// Sets the resource parent. - /// - /// The parent. - void SetParent(IResourceNode parent); - - /// - /// Notifies the resource node that a change has been made to the resources in its parent. - /// - /// The event args. - /// - /// This method will be called automatically by the framework, you should not need to call - /// this method yourself. - /// - void ParentResourcesChanged(ResourcesChangedEventArgs e); - } -} diff --git a/src/Avalonia.Styling/Controls/ResourceDictionary.cs b/src/Avalonia.Styling/Controls/ResourceDictionary.cs index ff84ab0b73..cedd116e92 100644 --- a/src/Avalonia.Styling/Controls/ResourceDictionary.cs +++ b/src/Avalonia.Styling/Controls/ResourceDictionary.cs @@ -1,21 +1,20 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; -using System.Linq; using Avalonia.Collections; +using Avalonia.Metadata; + +#nullable enable namespace Avalonia.Controls { /// /// An indexed dictionary of resources. /// - public class ResourceDictionary : AvaloniaDictionary, - IResourceDictionary, - IResourceNode, - ISetResourceParent + public class ResourceDictionary : AvaloniaDictionary, IResourceDictionary { - private IResourceNode _parent; - private AvaloniaList _mergedDictionaries; + private IResourceHost? _owner; + private AvaloniaList? _mergedDictionaries; /// /// Initializes a new instance of the class. @@ -25,10 +24,28 @@ namespace Avalonia.Controls CollectionChanged += OnCollectionChanged; } - /// - public event EventHandler ResourcesChanged; + /// + /// Initializes a new instance of the class. + /// + public ResourceDictionary(IResourceHost owner) + : this() + { + Owner = owner; + } + + public IResourceHost? Owner + { + get => _owner; + private set + { + if (_owner != value) + { + _owner = value; + OwnerChanged?.Invoke(this, EventArgs.Empty); + } + } + } - /// public IList MergedDictionaries { get @@ -40,71 +57,51 @@ namespace Avalonia.Controls _mergedDictionaries.ForEachItem( x => { - if (x is ISetResourceParent setParent) - { - setParent.SetParent(this); - setParent.ParentResourcesChanged(new ResourcesChangedEventArgs()); - } - - if (x.HasResources) + if (Owner is object) { - OnResourcesChanged(); + x.AddOwner(Owner); } - - x.ResourcesChanged += MergedDictionaryResourcesChanged; }, x => { - if (x is ISetResourceParent setParent) - { - setParent.SetParent(null); - setParent.ParentResourcesChanged(new ResourcesChangedEventArgs()); - } - - if (x.HasResources) + if (Owner is object) { - OnResourcesChanged(); + x.RemoveOwner(Owner); } - - (x as ISetResourceParent)?.SetParent(null); - x.ResourcesChanged -= MergedDictionaryResourcesChanged; - }, - () => { }); + }, null); } return _mergedDictionaries; } } - /// - bool IResourceProvider.HasResources + bool IResourceNode.HasResources { - get => Count > 0 || (_mergedDictionaries?.Any(x => x.HasResources) ?? false); - } - - /// - IResourceNode IResourceNode.ResourceParent => _parent; + get + { + if (Count > 0) + { + return true; + } - /// - void ISetResourceParent.ParentResourcesChanged(ResourcesChangedEventArgs e) - { - NotifyMergedDictionariesResourcesChanged(e); - ResourcesChanged?.Invoke(this, e); - } + if (_mergedDictionaries?.Count > 0) + { + foreach (var i in _mergedDictionaries) + { + if (i.HasResources) + { + return true; + } + } + } - /// - void ISetResourceParent.SetParent(IResourceNode parent) - { - if (_parent != null && parent != null) - { - throw new InvalidOperationException("The ResourceDictionary already has a parent."); + return false; } - - _parent = parent; } - /// - public bool TryGetResource(object key, out object value) + public event EventHandler? OwnerChanged; + + public bool TryGetResource(object key, out object? value) { if (TryGetValue(key, out value)) { @@ -125,32 +122,63 @@ namespace Avalonia.Controls return false; } - private void OnResourcesChanged() + void IResourceProvider.AddOwner(IResourceHost owner) { - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); + owner = owner ?? throw new ArgumentNullException(nameof(owner)); + + if (Owner != null) + { + throw new InvalidOperationException("The ResourceDictionary already has a parent."); + } + + Owner = owner; + + var hasResources = Count > 0; + + if (_mergedDictionaries is object) + { + foreach (var i in _mergedDictionaries) + { + i.AddOwner(owner); + hasResources |= i.HasResources; + } + } + + if (hasResources) + { + owner.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); + } } - private void NotifyMergedDictionariesResourcesChanged(ResourcesChangedEventArgs e) + void IResourceProvider.RemoveOwner(IResourceHost owner) { - if (_mergedDictionaries != null) + owner = owner ?? throw new ArgumentNullException(nameof(owner)); + + if (Owner == owner) { - for (var i = _mergedDictionaries.Count - 1; i >= 0; --i) + Owner = null; + + var hasResources = Count > 0; + + if (_mergedDictionaries is object) { - if (_mergedDictionaries[i] is ISetResourceParent merged) + foreach (var i in _mergedDictionaries) { - merged.ParentResourcesChanged(e); + i.RemoveOwner(owner); + hasResources |= i.HasResources; } } + + if (hasResources) + { + owner.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); + } } } private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { - var ev = new ResourcesChangedEventArgs(); - NotifyMergedDictionariesResourcesChanged(ev); - OnResourcesChanged(); + Owner?.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); } - - private void MergedDictionaryResourcesChanged(object sender, ResourcesChangedEventArgs e) => OnResourcesChanged(); } } diff --git a/src/Avalonia.Styling/Controls/ResourceNodeExtensions.cs b/src/Avalonia.Styling/Controls/ResourceNodeExtensions.cs index 19a16f86c4..7771629e0d 100644 --- a/src/Avalonia.Styling/Controls/ResourceNodeExtensions.cs +++ b/src/Avalonia.Styling/Controls/ResourceNodeExtensions.cs @@ -1,6 +1,9 @@ using System; +using Avalonia.LogicalTree; using Avalonia.Reactive; +#nullable enable + namespace Avalonia.Controls { public static class ResourceNodeExtensions @@ -11,8 +14,11 @@ namespace Avalonia.Controls /// The control. /// The resource key. /// The resource, or if not found. - public static object FindResource(this IResourceNode control, object key) + public static object? FindResource(this IResourceHost control, object key) { + control = control ?? throw new ArgumentNullException(nameof(control)); + key = key ?? throw new ArgumentNullException(nameof(key)); + if (control.TryFindResource(key, out var value)) { return value; @@ -28,16 +34,16 @@ namespace Avalonia.Controls /// The resource key. /// On return, contains the resource if found, otherwise null. /// True if the resource was found; otherwise false. - public static bool TryFindResource(this IResourceNode control, object key, out object value) + public static bool TryFindResource(this IResourceHost control, object key, out object? value) { - Contract.Requires(control != null); - Contract.Requires(key != null); + control = control ?? throw new ArgumentNullException(nameof(control)); + key = key ?? throw new ArgumentNullException(nameof(key)); - var current = control; + IResourceHost? current = control; while (current != null) { - if (current is IResourceNode host) + if (current is IResourceHost host) { if (host.TryGetResource(key, out value)) { @@ -45,24 +51,35 @@ namespace Avalonia.Controls } } - current = current.ResourceParent; + current = (current as IStyledElement)?.StylingParent as IResourceHost; } value = null; return false; } - public static IObservable GetResourceObservable(this IResourceNode target, object key) + public static IObservable GetResourceObservable(this IStyledElement control, object key) + { + control = control ?? throw new ArgumentNullException(nameof(control)); + key = key ?? throw new ArgumentNullException(nameof(key)); + + return new ResourceObservable(control, key); + } + + public static IObservable GetResourceObservable(this IResourceProvider resourceProvider, object key) { - return new ResourceObservable(target, key); + resourceProvider = resourceProvider ?? throw new ArgumentNullException(nameof(resourceProvider)); + key = key ?? throw new ArgumentNullException(nameof(key)); + + return new FloatingResourceObservable(resourceProvider, key); } - private class ResourceObservable : LightweightObservableBase + private class ResourceObservable : LightweightObservableBase { - private readonly IResourceNode _target; + private readonly IStyledElement _target; private readonly object _key; - public ResourceObservable(IResourceNode target, object key) + public ResourceObservable(IStyledElement target, object key) { _target = target; _key = key; @@ -78,7 +95,7 @@ namespace Avalonia.Controls _target.ResourcesChanged -= ResourcesChanged; } - protected override void Subscribed(IObserver observer, bool first) + protected override void Subscribed(IObserver observer, bool first) { observer.OnNext(_target.FindResource(_key)); } @@ -88,5 +105,65 @@ namespace Avalonia.Controls PublishNext(_target.FindResource(_key)); } } + + private class FloatingResourceObservable : LightweightObservableBase + { + private readonly IResourceProvider _target; + private readonly object _key; + private IResourceHost? _owner; + + public FloatingResourceObservable(IResourceProvider target, object key) + { + _target = target; + _key = key; + } + + protected override void Initialize() + { + _target.OwnerChanged += OwnerChanged; + _owner = _target.Owner; + } + + protected override void Deinitialize() + { + _target.OwnerChanged -= OwnerChanged; + _owner = null; + } + + protected override void Subscribed(IObserver observer, bool first) + { + if (_target.Owner is object) + { + observer.OnNext(_target.Owner?.FindResource(_key)); + } + } + + private void PublishNext() + { + PublishNext(_target.Owner?.FindResource(_key)); + } + + private void OwnerChanged(object sender, EventArgs e) + { + if (_owner is object) + { + _owner.ResourcesChanged -= ResourcesChanged; + } + + _owner = _target.Owner; + + if (_owner is object) + { + _owner.ResourcesChanged += ResourcesChanged; + } + + PublishNext(); + } + + private void ResourcesChanged(object sender, ResourcesChangedEventArgs e) + { + PublishNext(); + } + } } } diff --git a/src/Avalonia.Styling/IStyledElement.cs b/src/Avalonia.Styling/IStyledElement.cs index d4d0f179c3..610c743a3e 100644 --- a/src/Avalonia.Styling/IStyledElement.cs +++ b/src/Avalonia.Styling/IStyledElement.cs @@ -9,8 +9,7 @@ namespace Avalonia IStyleable, IStyleHost, ILogical, - IResourceProvider, - IResourceNode, + IResourceHost, IDataContextProvider { /// @@ -18,6 +17,11 @@ namespace Avalonia /// event EventHandler Initialized; + /// + /// Raised when resources on the element are changed. + /// + event EventHandler ResourcesChanged; + /// /// Gets a value that indicates whether the element has finished initialization. /// diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index 1a4bc5b3a3..febb8ec817 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -212,7 +212,6 @@ namespace Avalonia if (_styles is null) { _styles = new Styles(this); - _styles.ResourcesChanged += ThisResourcesChanged; NotifyResourcesChanged(new ResourcesChangedEventArgs()); } @@ -225,32 +224,13 @@ namespace Avalonia /// public IResourceDictionary Resources { - get => _resources ?? (Resources = new ResourceDictionary()); + get => _resources ??= new ResourceDictionary(this); set { value = value ?? throw new ArgumentNullException(nameof(value)); - - var hadResources = false; - - if (_resources != null) - { - (_resources as ISetResourceParent)?.SetParent(null); - hadResources = _resources.Count > 0; - _resources.ResourcesChanged -= ThisResourcesChanged; - } - + _resources?.RemoveOwner(this); _resources = value; - _resources.ResourcesChanged += ThisResourcesChanged; - - if (value is ISetResourceParent setParent && setParent.ResourceParent == null) - { - setParent.SetParent(this); - } - - if (hadResources || _resources.Count > 0) - { - NotifyResourcesChanged(new ResourcesChangedEventArgs()); - } + _resources.AddOwner(this); } } @@ -312,10 +292,8 @@ namespace Avalonia IAvaloniaReadOnlyList ILogical.LogicalChildren => LogicalChildren; /// - bool IResourceProvider.HasResources => _resources?.Count > 0 || Styles.HasResources; - - /// - IResourceNode? IResourceNode.ResourceParent => ((IStyleHost)this).StylingParent as IResourceNode; + bool IResourceNode.HasResources => (_resources?.HasResources ?? false) || + (((IResourceNode?)_styles)?.HasResources ?? false); /// IAvaloniaReadOnlyList IStyleable.Classes => Classes; @@ -407,7 +385,10 @@ namespace Avalonia void ILogical.NotifyResourcesChanged(ResourcesChangedEventArgs e) => NotifyResourcesChanged(e); /// - bool IResourceProvider.TryGetResource(object key, out object? value) + void IResourceHost.NotifyHostedResourcesChanged(ResourcesChangedEventArgs e) => NotifyResourcesChanged(e); + + /// + bool IResourceNode.TryGetResource(object key, out object? value) { value = null; return (_resources?.TryGetResource(key, out value) ?? false) || @@ -446,6 +427,7 @@ namespace Avalonia { old.ResourcesChanged -= ThisResourcesChanged; } + if (Parent != null) { Parent.ResourcesChanged += ThisResourcesChanged; @@ -804,24 +786,9 @@ namespace Avalonia } } - private void NotifyResourcesChanged(ResourcesChangedEventArgs e) + private void NotifyResourcesChanged(ResourcesChangedEventArgs? e = null) { - if (_notifyingResourcesChanged) - { - return; - } - - try - { - _notifyingResourcesChanged = true; - (_resources as ISetResourceParent)?.ParentResourcesChanged(e); - (_styles as ISetResourceParent)?.ParentResourcesChanged(e); - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); - } - finally - { - _notifyingResourcesChanged = false; - } + ResourcesChanged?.Invoke(this, e ?? new ResourcesChangedEventArgs()); } private void ThisResourcesChanged(object sender, ResourcesChangedEventArgs e) diff --git a/src/Avalonia.Styling/Styling/IStyle.cs b/src/Avalonia.Styling/Styling/IStyle.cs index 738a69cb88..78fbe0f2b5 100644 --- a/src/Avalonia.Styling/Styling/IStyle.cs +++ b/src/Avalonia.Styling/Styling/IStyle.cs @@ -8,7 +8,7 @@ namespace Avalonia.Styling /// /// Defines the interface for styles. /// - public interface IStyle : IResourceNode + public interface IStyle { /// /// Gets a collection of child styles. diff --git a/src/Avalonia.Styling/Styling/IStyleHost.cs b/src/Avalonia.Styling/Styling/IStyleHost.cs index 6953940a71..360b40d9a1 100644 --- a/src/Avalonia.Styling/Styling/IStyleHost.cs +++ b/src/Avalonia.Styling/Styling/IStyleHost.cs @@ -27,7 +27,7 @@ namespace Avalonia.Styling /// /// Gets the parent style host element. /// - IStyleHost StylingParent { get; } + IStyleHost? StylingParent { get; } /// /// Called when styles are added to or a nested styles collection. diff --git a/src/Avalonia.Styling/Styling/Style.cs b/src/Avalonia.Styling/Styling/Style.cs index de0506163e..ae866ebf46 100644 --- a/src/Avalonia.Styling/Styling/Style.cs +++ b/src/Avalonia.Styling/Styling/Style.cs @@ -11,9 +11,9 @@ namespace Avalonia.Styling /// /// Defines a style. /// - public class Style : AvaloniaObject, IStyle, ISetResourceParent + public class Style : AvaloniaObject, IStyle, IResourceProvider { - private IResourceNode? _parent; + private IResourceHost? _owner; private IResourceDictionary? _resources; private List? _setters; private List? _animations; @@ -34,8 +34,18 @@ namespace Avalonia.Styling Selector = selector(null); } - /// - public event EventHandler? ResourcesChanged; + public IResourceHost? Owner + { + get => _owner; + private set + { + if (_owner != value) + { + _owner = value; + OwnerChanged?.Invoke(this, EventArgs.Empty); + } + } + } /// /// Gets or sets a dictionary of style resources. @@ -47,20 +57,18 @@ namespace Avalonia.Styling { value = value ?? throw new ArgumentNullException(nameof(value)); - var hadResources = false; - - if (_resources != null) - { - hadResources = _resources.HasResources; - _resources.ResourcesChanged -= ResourceDictionaryChanged; - } + var hadResources = _resources?.HasResources ?? false; _resources = value; - _resources.ResourcesChanged += ResourceDictionaryChanged; - if (hadResources || _resources.HasResources) + if (Owner is object) { - ((ISetResourceParent)this).ParentResourcesChanged(new ResourcesChangedEventArgs()); + _resources.AddOwner(Owner); + + if (hadResources || _resources.HasResources) + { + Owner.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); + } } } } @@ -81,15 +89,11 @@ namespace Avalonia.Styling /// public IList Animations => _animations ??= new List(); - /// - IResourceNode? IResourceNode.ResourceParent => _parent; - - /// - bool IResourceProvider.HasResources => _resources?.Count > 0; - + bool IResourceNode.HasResources => _resources?.Count > 0; IReadOnlyList IStyle.Children => Array.Empty(); - /// + public event EventHandler? OwnerChanged; + public SelectorMatchResult TryAttach(IStyleable target, IStyleHost? host) { target = target ?? throw new ArgumentNullException(nameof(target)); @@ -107,7 +111,6 @@ namespace Avalonia.Styling return match.Result; } - /// public bool TryGetResource(object key, out object? result) { result = null; @@ -130,26 +133,28 @@ namespace Avalonia.Styling } } - /// - void ISetResourceParent.ParentResourcesChanged(ResourcesChangedEventArgs e) + void IResourceProvider.AddOwner(IResourceHost owner) { - ResourcesChanged?.Invoke(this, e); - } + owner = owner ?? throw new ArgumentNullException(nameof(owner)); - /// - void ISetResourceParent.SetParent(IResourceNode parent) - { - if (_parent != null && parent != null) + if (Owner != null) { throw new InvalidOperationException("The Style already has a parent."); } - _parent = parent; + Owner = owner; + _resources?.AddOwner(owner); } - private void ResourceDictionaryChanged(object sender, ResourcesChangedEventArgs e) + void IResourceProvider.RemoveOwner(IResourceHost owner) { - ResourcesChanged?.Invoke(this, e); + owner = owner ?? throw new ArgumentNullException(nameof(owner)); + + if (Owner == owner) + { + Owner = null; + _resources?.RemoveOwner(owner); + } } } } diff --git a/src/Avalonia.Styling/Styling/Styles.cs b/src/Avalonia.Styling/Styling/Styles.cs index ac355fd8b3..1058b863f0 100644 --- a/src/Avalonia.Styling/Styling/Styles.cs +++ b/src/Avalonia.Styling/Styling/Styles.cs @@ -2,7 +2,6 @@ using System; using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; -using System.Linq; using Avalonia.Collections; using Avalonia.Controls; @@ -13,10 +12,13 @@ namespace Avalonia.Styling /// /// A style that consists of a number of child styles. /// - public class Styles : AvaloniaObject, IAvaloniaList, IStyle, ISetResourceParent + public class Styles : AvaloniaObject, + IAvaloniaList, + IStyle, + IResourceProvider { private readonly AvaloniaList _styles = new AvaloniaList(); - private IResourceNode? _parent; + private IResourceHost? _owner; private IResourceDictionary? _resources; private Dictionary?>? _cache; private bool _notifyingResourcesChanged; @@ -27,22 +29,29 @@ namespace Avalonia.Styling _styles.CollectionChanged += OnCollectionChanged; } - public Styles(IResourceNode parent) + public Styles(IResourceHost owner) : this() { - _parent = parent; + Owner = owner; } public event NotifyCollectionChangedEventHandler? CollectionChanged; + public event EventHandler? OwnerChanged; - /// - public event EventHandler? ResourcesChanged; - - /// public int Count => _styles.Count; - /// - public bool HasResources => _resources?.Count > 0 || this.Any(x => x.HasResources); + public IResourceHost? Owner + { + get => _owner; + private set + { + if (_owner != value) + { + _owner = value; + OwnerChanged?.Invoke(this, EventArgs.Empty); + } + } + } /// /// Gets or sets a dictionary of style resources. @@ -54,43 +63,53 @@ namespace Avalonia.Styling { value = value ?? throw new ArgumentNullException(nameof(Resources)); - var hadResources = false; - - if (_resources != null) + if (Owner is object) { - hadResources = _resources.Count > 0; - _resources.ResourcesChanged -= NotifyResourcesChanged; + _resources?.RemoveOwner(Owner); } _resources = value; - _resources.ResourcesChanged += NotifyResourcesChanged; - if (hadResources || _resources.Count > 0) + if (Owner is object) { - ((ISetResourceParent)this).ParentResourcesChanged(new ResourcesChangedEventArgs()); + _resources.AddOwner(Owner); } } } - /// - IResourceNode? IResourceNode.ResourceParent => _parent; - - /// bool ICollection.IsReadOnly => false; - /// + bool IResourceNode.HasResources + { + get + { + if (Count > 0) + { + return true; + } + + foreach (var i in this) + { + if (i is IResourceProvider p && p.HasResources) + { + return true; + } + } + + return false; + } + } + IStyle IReadOnlyList.this[int index] => _styles[index]; IReadOnlyList IStyle.Children => this; - /// public IStyle this[int index] { get => _styles[index]; set => _styles[index] = value; } - /// public SelectorMatchResult TryAttach(IStyleable target, IStyleHost? host) { _cache ??= new Dictionary?>(); @@ -142,7 +161,7 @@ namespace Avalonia.Styling for (var i = Count - 1; i >= 0; --i) { - if (this[i].TryGetResource(key, out value)) + if (this[i] is IResourceProvider p && p.TryGetResource(key, out value)) { return true; } @@ -203,20 +222,29 @@ namespace Avalonia.Styling IEnumerator IEnumerable.GetEnumerator() => _styles.GetEnumerator(); /// - void ISetResourceParent.SetParent(IResourceNode parent) + void IResourceProvider.AddOwner(IResourceHost owner) { - if (_parent != null && parent != null) + owner = owner ?? throw new ArgumentNullException(nameof(owner)); + + if (Owner != null) { - throw new InvalidOperationException("The Style already has a parent."); + throw new InvalidOperationException("The Styles already has a owner."); } - _parent = parent; + Owner = owner; + _resources?.AddOwner(owner); } /// - void ISetResourceParent.ParentResourcesChanged(ResourcesChangedEventArgs e) + void IResourceProvider.RemoveOwner(IResourceHost owner) { - NotifyResourcesChanged(e); + owner = owner ?? throw new ArgumentNullException(nameof(owner)); + + if (Owner == owner) + { + Owner = null; + _resources?.RemoveOwner(owner); + } } private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) @@ -241,22 +269,15 @@ namespace Avalonia.Styling { var style = (IStyle)items[i]; - if (style.ResourceParent == null && style is ISetResourceParent setParent) + if (Owner is object && style is IResourceProvider resourceProvider) { - setParent.SetParent(this); - setParent.ParentResourcesChanged(new ResourcesChangedEventArgs()); + resourceProvider.AddOwner(Owner); } - if (style.HasResources) - { - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); - } - - style.ResourcesChanged += NotifyResourcesChanged; _cache = null; } - GetHost()?.StylesAdded(ToReadOnlyList(items)); + (Owner as IStyleHost)?.StylesAdded(ToReadOnlyList(items)); } void Remove(IList items) @@ -265,22 +286,15 @@ namespace Avalonia.Styling { var style = (IStyle)items[i]; - if (style.ResourceParent == this && style is ISetResourceParent setParent) + if (Owner is object && style is IResourceProvider resourceProvider) { - setParent.SetParent(null); - setParent.ParentResourcesChanged(new ResourcesChangedEventArgs()); + resourceProvider.RemoveOwner(Owner); } - if (style.HasResources) - { - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); - } - - style.ResourcesChanged -= NotifyResourcesChanged; _cache = null; } - GetHost()?.StylesRemoved(ToReadOnlyList(items)); + (Owner as IStyleHost)?.StylesRemoved(ToReadOnlyList(items)); } switch (e.Action) @@ -301,50 +315,5 @@ namespace Avalonia.Styling CollectionChanged?.Invoke(this, e); } - - private IStyleHost? GetHost() - { - var node = _parent; - - while (node != null) - { - if (node is IStyleHost host) - { - return host; - } - - node = node.ResourceParent; - } - - return null; - } - - private void NotifyResourcesChanged(object sender, ResourcesChangedEventArgs e) - { - NotifyResourcesChanged(e); - } - - private void NotifyResourcesChanged(ResourcesChangedEventArgs e) - { - if (_notifyingResourcesChanged) - { - return; - } - - try - { - _notifyingResourcesChanged = true; - foreach (var child in this) - { - (child as ISetResourceParent)?.ParentResourcesChanged(e); - } - - ResourcesChanged?.Invoke(this, e); - } - finally - { - _notifyingResourcesChanged = false; - } - } } } diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/DynamicResourceExtension.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/DynamicResourceExtension.cs index 1e493ee2e8..d0d4aae9b8 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/DynamicResourceExtension.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/DynamicResourceExtension.cs @@ -1,15 +1,15 @@ using System; -using System.ComponentModel; -using System.Linq; -using System.Reactive.Linq; using Avalonia.Controls; using Avalonia.Data; +#nullable enable + namespace Avalonia.Markup.Xaml.MarkupExtensions { public class DynamicResourceExtension : IBinding { - private IResourceNode _anchor; + private IStyledElement? _anchor; + private IResourceProvider? _resourceProvider; public DynamicResourceExtension() { @@ -20,32 +20,46 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions ResourceKey = resourceKey; } - public object ResourceKey { get; set; } + public object? ResourceKey { get; set; } public IBinding ProvideValue(IServiceProvider serviceProvider) { var provideTarget = serviceProvider.GetService(); - if (!(provideTarget.TargetObject is IResourceNode)) + if (!(provideTarget.TargetObject is IStyledElement)) { - _anchor = serviceProvider.GetFirstParent(); + _anchor = serviceProvider.GetFirstParent(); + + if (_anchor is null) + { + _resourceProvider = serviceProvider.GetFirstParent(); + } } return this; } - InstancedBinding IBinding.Initiate( + InstancedBinding? IBinding.Initiate( IAvaloniaObject target, AvaloniaProperty targetProperty, object anchor, bool enableDataValidation) { - var control = target as IResourceNode ?? _anchor; + if (ResourceKey is null) + { + return null; + } + + var control = target as IStyledElement ?? _anchor as IStyledElement; if (control != null) { return InstancedBinding.OneWay(control.GetResourceObservable(ResourceKey)); } + else if (_resourceProvider is object) + { + return InstancedBinding.OneWay(_resourceProvider.GetResourceObservable(ResourceKey)); + } return null; } diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs index 0d56942645..87e2ba3a2e 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs @@ -2,18 +2,17 @@ using System.ComponentModel; using Avalonia.Controls; +#nullable enable + namespace Avalonia.Markup.Xaml.MarkupExtensions { /// /// Loads a resource dictionary from a specified URL. /// - public class ResourceInclude : IResourceNode, ISetResourceParent + public class ResourceInclude : IResourceProvider { - private IResourceNode _parent; - private Uri _baseUri; - private IResourceDictionary _loaded; - - public event EventHandler ResourcesChanged; + private Uri? _baseUri; + private IResourceDictionary? _loaded; /// /// Gets the loaded resource dictionary. @@ -26,53 +25,34 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions { var loader = new AvaloniaXamlLoader(); _loaded = (IResourceDictionary)loader.Load(Source, _baseUri); - - (_loaded as ISetResourceParent)?.SetParent(this); - _loaded.ResourcesChanged += ResourcesChanged; - - if (_loaded.HasResources) - { - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); - } } return _loaded; } } + public IResourceHost? Owner => Loaded.Owner; + /// /// Gets or sets the source URL. /// - public Uri Source { get; set; } - - /// - bool IResourceProvider.HasResources => Loaded.HasResources; + public Uri? Source { get; set; } - /// - IResourceNode IResourceNode.ResourceParent => _parent; + bool IResourceNode.HasResources => Loaded.HasResources; - /// - bool IResourceProvider.TryGetResource(object key, out object value) + public event EventHandler OwnerChanged { - return Loaded.TryGetResource(key, out value); + add => Loaded.OwnerChanged += value; + remove => Loaded.OwnerChanged -= value; } - /// - void ISetResourceParent.SetParent(IResourceNode parent) + bool IResourceNode.TryGetResource(object key, out object? value) { - if (_parent != null && parent != null) - { - throw new InvalidOperationException("The ResourceInclude already has a parent."); - } - - _parent = parent; + return Loaded.TryGetResource(key, out value); } - /// - void ISetResourceParent.ParentResourcesChanged(ResourcesChangedEventArgs e) - { - (_loaded as ISetResourceParent)?.ParentResourcesChanged(e); - } + void IResourceProvider.AddOwner(IResourceHost owner) => Loaded.AddOwner(owner); + void IResourceProvider.RemoveOwner(IResourceHost owner) => Loaded.RemoveOwner(owner); public ResourceInclude ProvideValue(IServiceProvider serviceProvider) { diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs index b34123a5de..6d9fac638c 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs @@ -4,6 +4,7 @@ using System.ComponentModel; using System.Reflection; using Avalonia.Controls; using Avalonia.Markup.Data; +using Avalonia.Markup.Xaml.XamlIl.Runtime; namespace Avalonia.Markup.Xaml.MarkupExtensions { @@ -22,15 +23,22 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions public object ProvideValue(IServiceProvider serviceProvider) { - // Look upwards though the ambient context for IResourceProviders which might be able - // to give us the resource. - foreach (var resourceProvider in serviceProvider.GetParents()) + var stack = serviceProvider.GetService(); + + // Look upwards though the ambient context for IResourceHosts and IResourceProviders + // which might be able to give us the resource. + foreach (var e in stack.Parents) { - if (resourceProvider.TryGetResource(ResourceKey, out var value)) + object value; + + if (e is IResourceHost host && host.TryGetResource(ResourceKey, out value)) + { + return value; + } + else if (e is IResourceProvider provider && provider.TryGetResource(ResourceKey, out value)) { return value; } - } // The resource still hasn't been found, so add a delayed one-time binding. diff --git a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs index 3d3d9bc08e..2b39263ee9 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs @@ -10,32 +10,30 @@ namespace Avalonia.Markup.Xaml.Styling /// /// Includes a style from a URL. /// - public class StyleInclude : IStyle, ISetResourceParent + public class StyleInclude : IStyle, IResourceProvider { - private Uri _baseUri; + private readonly Uri _baseUri; private IStyle[]? _loaded; - private IResourceNode? _parent; /// /// Initializes a new instance of the class. /// - /// + /// The base URL for the XAML context. public StyleInclude(Uri baseUri) { _baseUri = baseUri; } + /// + /// Initializes a new instance of the class. + /// + /// The XAML service provider. public StyleInclude(IServiceProvider serviceProvider) { _baseUri = serviceProvider.GetContextBaseUri(); } - - /// - public event EventHandler ResourcesChanged - { - add {} - remove {} - } + + public IResourceHost? Owner => (Loaded as IResourceProvider)?.Owner; /// /// Gets or sets the source URL. @@ -53,7 +51,6 @@ namespace Avalonia.Markup.Xaml.Styling { var loader = new AvaloniaXamlLoader(); var loaded = (IStyle)loader.Load(Source, _baseUri); - (loaded as ISetResourceParent)?.SetParent(this); _loaded = new[] { loaded }; } @@ -61,35 +58,42 @@ namespace Avalonia.Markup.Xaml.Styling } } - /// - bool IResourceProvider.HasResources => Loaded.HasResources; - - /// - IResourceNode? IResourceNode.ResourceParent => _parent; + bool IResourceNode.HasResources => (Loaded as IResourceProvider)?.HasResources ?? false; IReadOnlyList IStyle.Children => _loaded ?? Array.Empty(); - /// - public SelectorMatchResult TryAttach(IStyleable target, IStyleHost? host) => Loaded.TryAttach(target, host); - - /// - public bool TryGetResource(object key, out object? value) => Loaded.TryGetResource(key, out value); - - /// - void ISetResourceParent.ParentResourcesChanged(ResourcesChangedEventArgs e) + public event EventHandler OwnerChanged { - (Loaded as ISetResourceParent)?.ParentResourcesChanged(e); + add + { + if (Loaded is IResourceProvider rp) + { + rp.OwnerChanged += value; + } + } + remove + { + if (Loaded is IResourceProvider rp) + { + rp.OwnerChanged -= value; + } + } } - /// - void ISetResourceParent.SetParent(IResourceNode parent) + public SelectorMatchResult TryAttach(IStyleable target, IStyleHost? host) => Loaded.TryAttach(target, host); + + public bool TryGetResource(object key, out object? value) { - if (_parent != null && parent != null) + if (Loaded is IResourceProvider p) { - throw new InvalidOperationException("The Style already has a parent."); + return p.TryGetResource(key, out value); } - _parent = parent; + value = null; + return false; } + + void IResourceProvider.AddOwner(IResourceHost owner) => (Loaded as IResourceProvider)?.AddOwner(owner); + void IResourceProvider.RemoveOwner(IResourceHost owner) => (Loaded as IResourceProvider)?.RemoveOwner(owner); } } diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ResourceDictionaryTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ResourceDictionaryTests.cs index e5fd5ed37e..1c82e46b3c 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ResourceDictionaryTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ResourceDictionaryTests.cs @@ -30,25 +30,6 @@ namespace Avalonia.Markup.Xaml.UnitTests.Xaml } } - [Fact] - public void DynamicResource_Works_In_ResourceDictionary() - { - using (StyledWindow()) - { - var xaml = @" - - Red - -"; - var loader = new AvaloniaXamlLoader(); - var resources = (ResourceDictionary)loader.Load(xaml); - var brush = (SolidColorBrush)resources["RedBrush"]; - - Assert.Equal(Colors.Red, brush.Color); - } - } - [Fact] public void DynamicResource_Finds_Resource_In_Parent_Dictionary() { diff --git a/tests/Avalonia.Styling.UnitTests/ResourceDictionaryTests.cs b/tests/Avalonia.Styling.UnitTests/ResourceDictionaryTests.cs index c66952f428..84b0d09b61 100644 --- a/tests/Avalonia.Styling.UnitTests/ResourceDictionaryTests.cs +++ b/tests/Avalonia.Styling.UnitTests/ResourceDictionaryTests.cs @@ -7,6 +7,20 @@ namespace Avalonia.Styling.UnitTests { public class ResourceDictionaryTests { + [Fact] + public void Cannot_Add_Null_Key() + { + var target = new ResourceDictionary(); + Assert.Throws(() => target.Add(null, "null")); + } + + [Fact] + public void Can_Add_Null_Value() + { + var target = new ResourceDictionary(); + target.Add("null", null); + } + [Fact] public void TryGetResource_Should_Find_Resource() { @@ -77,66 +91,97 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void ResourcesChanged_Should_Be_Raised_On_Resource_Add() + public void NotifyHostedResourcesChanged_Should_Be_Called_On_AddOwner() { - var target = new ResourceDictionary(); - var raised = false; + var host = new Mock(); + var target = new ResourceDictionary { { "foo", "bar" } }; + + ((IResourceProvider)target).AddOwner(host.Object); - target.ResourcesChanged += (_, __) => raised = true; + host.Verify(x => x.NotifyHostedResourcesChanged(It.IsAny())); + } + + [Fact] + public void NotifyHostedResourcesChanged_Should_Be_Called_On_RemoveOwner() + { + var host = new Mock(); + var target = new ResourceDictionary { { "foo", "bar" } }; + + ((IResourceProvider)target).AddOwner(host.Object); + host.ResetCalls(); + ((IResourceProvider)target).RemoveOwner(host.Object); + + host.Verify(x => x.NotifyHostedResourcesChanged(It.IsAny())); + } + + [Fact] + public void NotifyHostedResourcesChanged_Should_Be_Called_On_Resource_Add() + { + var host = new Mock(); + var target = new ResourceDictionary(host.Object); + + host.ResetCalls(); target.Add("foo", "bar"); - Assert.True(raised); + host.Verify(x => x.NotifyHostedResourcesChanged(It.IsAny())); } [Fact] - public void ResourcesChanged_Should_Be_Raised_On_MergedDictionary_Add() + public void NotifyHostedResourcesChanged_Should_Be_Called_On_MergedDictionary_Add() { - var target = new ResourceDictionary(); - var raised = false; + var host = new Mock(); + var target = new ResourceDictionary(host.Object); - target.ResourcesChanged += (_, __) => raised = true; + host.ResetCalls(); target.MergedDictionaries.Add(new ResourceDictionary { { "foo", "bar" }, }); - Assert.True(raised); + host.Verify( + x => x.NotifyHostedResourcesChanged(It.IsAny()), + Times.Once); } [Fact] - public void ResourcesChanged_Should_Not_Be_Raised_On_Empty_MergedDictionary_Add() + public void NotifyHostedResourcesChanged_Should_Not_Be_Called_On_Empty_MergedDictionary_Add() { - var target = new ResourceDictionary(); - var raised = false; + var host = new Mock(); + var target = new ResourceDictionary(host.Object); - target.ResourcesChanged += (_, __) => raised = true; + host.ResetCalls(); target.MergedDictionaries.Add(new ResourceDictionary()); - Assert.False(raised); + host.Verify( + x => x.NotifyHostedResourcesChanged(It.IsAny()), + Times.Never); } [Fact] - public void ResourcesChanged_Should_Be_Raised_On_MergedDictionary_Remove() + public void NotifyHostedResourcesChanged_Should_Be_Called_On_MergedDictionary_Remove() { - var target = new ResourceDictionary + var host = new Mock(); + var target = new ResourceDictionary(host.Object) { MergedDictionaries = { new ResourceDictionary { { "foo", "bar" } }, } }; - var raised = false; - target.ResourcesChanged += (_, __) => raised = true; + host.ResetCalls(); target.MergedDictionaries.RemoveAt(0); - Assert.True(raised); + host.Verify( + x => x.NotifyHostedResourcesChanged(It.IsAny()), + Times.Once); } [Fact] - public void ResourcesChanged_Should_Be_Raised_On_MergedDictionary_Resource_Add() + public void NotifyHostedResourcesChanged_Should_Be_Called_On_MergedDictionary_Resource_Add() { - var target = new ResourceDictionary + var host = new Mock(); + var target = new ResourceDictionary(host.Object) { MergedDictionaries = { @@ -144,44 +189,63 @@ namespace Avalonia.Styling.UnitTests } }; - var raised = false; - - target.ResourcesChanged += (_, __) => raised = true; + host.ResetCalls(); ((IResourceDictionary)target.MergedDictionaries[0]).Add("foo", "bar"); - Assert.True(raised); + host.Verify( + x => x.NotifyHostedResourcesChanged(It.IsAny()), + Times.Once); } [Fact] - public void MergedDictionary_ParentResourcesChanged_Should_Be_Called_On_Resource_Add() + public void Sets_Added_MergedDictionary_Owner() { - var target = new ResourceDictionary(); - var merged = new Mock(); + var host = new Mock(); - target.MergedDictionaries.Add(merged.Object); - merged.ResetCalls(); + var target = new ResourceDictionary(host.Object); + target.MergedDictionaries.Add(new ResourceDictionary()); - target.Add("foo", "bar"); + Assert.Same(host.Object, target.Owner); + Assert.Same(host.Object, ((ResourceDictionary)target.MergedDictionaries[0]).Owner); + } - merged.Verify( - x => x.ParentResourcesChanged(It.IsAny()), - Times.Once); + [Fact] + public void AddOwner_Sets_MergedDictionary_Owner() + { + var host = new Mock(); + + var target = new ResourceDictionary + { + MergedDictionaries = + { + new ResourceDictionary(), + } + }; + + ((IResourceProvider)target).AddOwner(host.Object); + + Assert.Same(host.Object, target.Owner); + Assert.Same(host.Object, ((ResourceDictionary)target.MergedDictionaries[0]).Owner); } [Fact] - public void MergedDictionary_ParentResourcesChanged_Should_Be_Called_On_NotifyResourceChanged() + public void RemoveOwner_Clears_MergedDictionary_Owner() { - var target = new ResourceDictionary(); - var merged = new Mock(); + var host = new Mock(); - target.MergedDictionaries.Add(merged.Object); - merged.ResetCalls(); + var target = new ResourceDictionary(host.Object) + { + MergedDictionaries = + { + new ResourceDictionary(), + } + }; - ((ISetResourceParent)target).ParentResourcesChanged(new ResourcesChangedEventArgs()); + ((IResourceProvider)target).RemoveOwner(host.Object); - merged.Verify( - x => x.ParentResourcesChanged(It.IsAny()), - Times.Once); + Assert.Null(target.Owner); + Assert.Null(((ResourceDictionary)target.MergedDictionaries[0]).Owner); } + } } diff --git a/tests/Avalonia.Styling.UnitTests/StyleTests.cs b/tests/Avalonia.Styling.UnitTests/StyleTests.cs index 086986a52e..3e2bd68cf0 100644 --- a/tests/Avalonia.Styling.UnitTests/StyleTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyleTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using Avalonia.Controls; using Avalonia.Data; using Avalonia.UnitTests; +using Moq; using Xunit; namespace Avalonia.Styling.UnitTests @@ -420,6 +421,33 @@ namespace Avalonia.Styling.UnitTests } } + [Fact] + public void Should_Set_Owner_On_Assigned_Resources() + { + var host = new Mock(); + var target = new Style(); + ((IResourceProvider)target).AddOwner(host.Object); + + var resources = new Mock(); + target.Resources = resources.Object; + + resources.Verify(x => x.AddOwner(host.Object), Times.Once); + } + + [Fact] + public void Should_Set_Owner_On_Assigned_Resources_2() + { + var host = new Mock(); + var target = new Style(); + + var resources = new Mock(); + target.Resources = resources.Object; + + host.ResetCalls(); + ((IResourceProvider)target).AddOwner(host.Object); + resources.Verify(x => x.AddOwner(host.Object), Times.Once); + } + private class Class1 : Control { public static readonly StyledProperty FooProperty = diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs index e336fbf838..857815f5b4 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs @@ -492,20 +492,20 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void Resources_Parent_Is_Set() + public void Resources_Owner_Is_Set() { var target = new TestControl(); - Assert.Same(target, ((IResourceNode)target.Resources).ResourceParent); + Assert.Same(target, ((ResourceDictionary)target.Resources).Owner); } [Fact] public void Assigned_Resources_Parent_Is_Set() { - var resources = new ResourceDictionary(); - var target = new TestControl { Resources = resources }; + var resources = new Mock(); + var target = new TestControl { Resources = resources.Object }; - Assert.Same(target, ((IResourceNode)resources).ResourceParent); + resources.Verify(x => x.AddOwner(target)); } [Fact] @@ -522,58 +522,25 @@ namespace Avalonia.Styling.UnitTests } [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 Styles_Parent_Is_Set() + public void Styles_Owner_Is_Set() { var target = new TestControl(); - Assert.Same(target, ((IResourceNode)target.Styles).ResourceParent); + Assert.Same(target, target.Styles.Owner); } [Fact] - public void Changing_Parent_Notifies_Styles_ParentResourcesChanged() + public void Changing_Parent_Raises_ResourcesChanged() { - var style = new Mock(); - var setResourceParent = style.As(); - var target = new TestControl { Styles = { style.Object } }; + var target = new TestControl(); var parent = new Decorator { Resources = { { "foo", "bar" } } }; + var raised = 0; - 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 } }; + target.ResourcesChanged += (s, e) => ++raised; - setResourceParent.ResetCalls(); - target.Resources.Add("foo", "bar"); + parent.Child = target; - setResourceParent.Verify(x => - x.ParentResourcesChanged(It.IsAny()), - Times.Once); + Assert.Equal(1, raised); } [Fact] diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs index 4b86c00cc2..8a79a867d0 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs @@ -217,23 +217,6 @@ 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) => diff --git a/tests/Avalonia.Styling.UnitTests/StylesTests.cs b/tests/Avalonia.Styling.UnitTests/StylesTests.cs index a61b178d7b..a4afcd34f9 100644 --- a/tests/Avalonia.Styling.UnitTests/StylesTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StylesTests.cs @@ -8,103 +8,59 @@ namespace Avalonia.Styling.UnitTests public class StylesTests { [Fact] - public void Adding_Style_With_Resources_Should_Raise_ResourceChanged() + public void Adding_Style_Should_Set_Owner() { - var style = new Style - { - Resources = { { "foo", "bar" } }, - }; + var host = new Mock(); + var target = new Styles(host.Object); + var style = new Mock(); + var rp = style.As(); - var target = new Styles(); - var raised = false; + host.ResetCalls(); + target.Add(style.Object); - target.ResourcesChanged += (_, __) => raised = true; - target.Add(style); - - Assert.True(raised); + rp.Verify(x => x.AddOwner(host.Object)); } [Fact] - public void Removing_Style_With_Resources_Should_Raise_ResourceChanged() + public void Removing_Style_Should_Clear_Owner() { - var target = new Styles - { - new Style - { - Resources = { { "foo", "bar" } }, - } - }; + var host = new Mock(); + var target = new Styles(host.Object); + var style = new Mock(); + var rp = style.As(); - var raised = false; + host.ResetCalls(); + target.Add(style.Object); + target.Remove(style.Object); - target.ResourcesChanged += (_, __) => raised = true; - target.Clear(); - - Assert.True(raised); + rp.Verify(x => x.RemoveOwner(host.Object)); } [Fact] - public void Adding_Style_Without_Resources_Should_Not_Raise_ResourceChanged() + public void Should_Set_Owner_On_Assigned_Resources() { - var style = new Style(); - var target = new Styles(); - var raised = false; + var host = new Mock(); + var target = new Style(); + ((IResourceProvider)target).AddOwner(host.Object); - target.ResourcesChanged += (_, __) => raised = true; - target.Add(style); + var resources = new Mock(); + target.Resources = resources.Object; - Assert.False(raised); + resources.Verify(x => x.AddOwner(host.Object), Times.Once); } [Fact] - public void Adding_Resource_Should_Raise_Child_ResourceChanged() + public void Should_Set_Owner_On_Assigned_Resources_2() { - Style child; - var target = new Styles - { - (child = new Style()), - }; - - var raised = false; - - child.ResourcesChanged += (_, __) => raised = true; - target.Resources.Add("foo", "bar"); - - Assert.True(raised); - } - - [Fact] - public void Adding_Resource_To_Sibling_Style_Should_Raise_ResourceChanged() - { - Style style1; - Style style2; - var target = new Styles - { - (style1 = new Style()), - (style2 = new Style()), - }; - - var raised = false; - - style2.ResourcesChanged += (_, __) => raised = true; - style1.Resources.Add("foo", "bar"); - - Assert.True(raised); - } - - [Fact] - public void ParentResourcesChanged_Should_Be_Propagated_To_Children() - { - var childStyle = new Mock(); - var setResourceParent = childStyle.As(); - var target = new Styles { childStyle.Object }; + var host = new Mock(); + var target = new Styles(); - setResourceParent.ResetCalls(); - ((ISetResourceParent)target).ParentResourcesChanged(new ResourcesChangedEventArgs()); + var resources = new Mock(); + target.Resources = resources.Object; - setResourceParent.Verify(x => x.ParentResourcesChanged( - It.IsAny()), - Times.Once); + host.ResetCalls(); + ((IResourceProvider)target).AddOwner(host.Object); + resources.Verify(x => x.AddOwner(host.Object), Times.Once); } [Fact] @@ -124,8 +80,7 @@ namespace Avalonia.Styling.UnitTests } }; - var result = target.FindResource("foo"); - + Assert.True(target.TryGetResource("foo", out var result)); Assert.Equal("bar", result); } } From eec2a05a26c0f1f03a33ff1ce53990095c791928 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 18 May 2020 11:30:28 +0200 Subject: [PATCH 03/11] Don't use ResourcesChanged to propagate. Instead of each control listening to its parent `ResourcesChanged` event, use the logical tree to propagate `ResourcesChanged` events: - When attaching to the logical tree, piggyback on the `AttachedToLogicalTree` traversal - At other times, propagate by calling `NotifyResourcesChanged` --- src/Avalonia.Styling/StyledElement.cs | 63 ++++++++++--------- .../StyledElementTests_Resources.cs | 43 +++++++------ 2 files changed, 56 insertions(+), 50 deletions(-) diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index febb8ec817..2a7fdb7a0a 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -205,19 +205,7 @@ namespace Avalonia /// each styled element may in addition define its own styles which are applied to the styled element /// itself and its children. /// - public Styles Styles - { - get - { - if (_styles is null) - { - _styles = new Styles(this); - NotifyResourcesChanged(new ResourcesChangedEventArgs()); - } - - return _styles; - } - } + public Styles Styles => _styles ??= new Styles(this); /// /// Gets or sets the styled element's resource dictionary. @@ -423,18 +411,6 @@ namespace Avalonia OnDetachedFromLogicalTreeCore(e); } - if (old != null) - { - old.ResourcesChanged -= ThisResourcesChanged; - } - - if (Parent != null) - { - Parent.ResourcesChanged += ThisResourcesChanged; - } - - NotifyResourcesChanged(new ResourcesChangedEventArgs()); - var newRoot = FindLogicalRoot(this); if (newRoot is object) @@ -442,6 +418,14 @@ namespace Avalonia var e = new LogicalTreeAttachmentEventArgs(newRoot, this, parent); OnAttachedToLogicalTreeCore(e); } + else + { + // If we were attached to the logical tree, we piggyback on the tree traversal + // there to raise resources changed notifications. If we're being removed from + // the logical tree or attached to a control which is not rooted, then traverse + // the tree raising notifications now. + NotifyResourcesChanged(); + } #nullable disable RaisePropertyChanged( @@ -638,6 +622,7 @@ namespace Avalonia _logicalRoot = e.Root; ApplyStyling(); + NotifyResourcesChanged(propagate: false); OnAttachedToLogicalTree(e); AttachedToLogicalTree?.Invoke(this, e); @@ -786,14 +771,30 @@ namespace Avalonia } } - private void NotifyResourcesChanged(ResourcesChangedEventArgs? e = null) + private void NotifyResourcesChanged( + ResourcesChangedEventArgs? e = null, + bool propagate = true) { - ResourcesChanged?.Invoke(this, e ?? new ResourcesChangedEventArgs()); - } + if (ResourcesChanged is object) + { + e ??= new ResourcesChangedEventArgs(); + ResourcesChanged(this, e); + } - private void ThisResourcesChanged(object sender, ResourcesChangedEventArgs e) - { - NotifyResourcesChanged(e); + if (propagate && _logicalChildren is object) + { + var count = _logicalChildren.Count; + + if (count > 0) + { + e ??= new ResourcesChangedEventArgs(); + + for (var i = 0; i < count; ++i) + { + _logicalChildren[i].NotifyResourcesChanged(e); + } + } + } } private static IReadOnlyList RecurseStyles(IReadOnlyList styles) diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs index 8a79a867d0..b3289f3804 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs @@ -154,21 +154,42 @@ namespace Avalonia.Controls.UnitTests }; var raisedOnTarget = false; - var raisedOnPresenter = false; var raisedOnChild = false; target.Measure(Size.Infinity); target.ResourcesChanged += (_, __) => raisedOnTarget = true; - target.Presenter.ResourcesChanged += (_, __) => raisedOnPresenter = true; child.ResourcesChanged += (_, __) => raisedOnChild = true; target.Resources.Add("foo", "bar"); Assert.True(raisedOnTarget); - Assert.True(raisedOnPresenter); Assert.True(raisedOnChild); } + [Fact] + public void Adding_Resource_Should_Call_Not_Raise_ResourceChanged_On_Template_Children() + { + Border child; + + var target = new ContentControl + { + Content = child = new Border(), + Template = ContentControlTemplate(), + }; + + var raisedOnTarget = false; + var raisedOnPresenter = false; + + target.Measure(Size.Infinity); + target.ResourcesChanged += (_, __) => raisedOnTarget = true; + target.Presenter.ResourcesChanged += (_, __) => raisedOnPresenter = true; + + target.Resources.Add("foo", "bar"); + + Assert.True(raisedOnTarget); + Assert.False(raisedOnPresenter); + } + [Fact] public void Adding_Resource_To_Styles_Should_Raise_ResourceChanged() { @@ -201,22 +222,6 @@ namespace Avalonia.Controls.UnitTests Assert.True(raised); } - [Fact] - public void Setting_Logical_Parent_Raises_Child_ResourcesChanged() - { - var parent = new ContentControl(); - var child = new StyledElement(); - - ((ISetLogicalParent)child).SetParent(parent); - var raisedOnChild = false; - - child.ResourcesChanged += (_, __) => raisedOnChild = true; - - parent.Resources.Add("foo", "bar"); - - Assert.True(raisedOnChild); - } - private IControlTemplate ContentControlTemplate() { return new FuncControlTemplate((x, scope) => From d138523b8e2f0c31b03b142cdaea20d63a4f4807 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 18 May 2020 14:04:38 +0200 Subject: [PATCH 04/11] Set owner on Styles children. And added some unit tests. --- src/Avalonia.Styling/Styling/Styles.cs | 16 ++++++++ .../DynamicResourceExtensionTests.cs | 39 +++++++++++++++++++ .../Avalonia.Styling.UnitTests/StylesTests.cs | 30 +++++++++++++- 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Styling/Styling/Styles.cs b/src/Avalonia.Styling/Styling/Styles.cs index 1058b863f0..f9cf3910f3 100644 --- a/src/Avalonia.Styling/Styling/Styles.cs +++ b/src/Avalonia.Styling/Styling/Styles.cs @@ -233,6 +233,14 @@ namespace Avalonia.Styling Owner = owner; _resources?.AddOwner(owner); + + foreach (var child in this) + { + if (child is IResourceProvider r) + { + r.AddOwner(owner); + } + } } /// @@ -244,6 +252,14 @@ namespace Avalonia.Styling { Owner = null; _resources?.RemoveOwner(owner); + + foreach (var child in this) + { + if (child is IResourceProvider r) + { + r.RemoveOwner(owner); + } + } } } diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs index 7c017584e4..412b5ec083 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs @@ -592,6 +592,45 @@ namespace Avalonia.Markup.Xaml.UnitTests.MarkupExtensions } } + [Fact] + public void DynamicResource_Can_Be_Found_In_Nested_Style_File() + { + var style1Xaml = @" + + +"; + var style2Xaml = @" +"; + using (StyledWindow( + ("test:style1.xaml", style1Xaml), + ("test:style2.xaml", style2Xaml))) + { + var xaml = @" + + + + + +"; + + var loader = new AvaloniaXamlLoader(); + var window = (Window)loader.Load(xaml); + var border = window.FindControl("border"); + var borderBrush = (ISolidColorBrush)border.Background; + + Assert.NotNull(borderBrush); + Assert.Equal(0xffff0000, borderBrush.Color.ToUint32()); + } + } + [Fact] public void Control_Property_Is_Updated_When_Parent_Is_Changed() { diff --git a/tests/Avalonia.Styling.UnitTests/StylesTests.cs b/tests/Avalonia.Styling.UnitTests/StylesTests.cs index a4afcd34f9..9d3704c91d 100644 --- a/tests/Avalonia.Styling.UnitTests/StylesTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StylesTests.cs @@ -40,7 +40,7 @@ namespace Avalonia.Styling.UnitTests public void Should_Set_Owner_On_Assigned_Resources() { var host = new Mock(); - var target = new Style(); + var target = new Styles(); ((IResourceProvider)target).AddOwner(host.Object); var resources = new Mock(); @@ -63,6 +63,34 @@ namespace Avalonia.Styling.UnitTests resources.Verify(x => x.AddOwner(host.Object), Times.Once); } + [Fact] + public void Should_Set_Owner_On_Child_Style() + { + var host = new Mock(); + var target = new Styles(); + ((IResourceProvider)target).AddOwner(host.Object); + + var style = new Mock(); + var resourceProvider = style.As(); + target.Add(style.Object); + + resourceProvider.Verify(x => x.AddOwner(host.Object), Times.Once); + } + + [Fact] + public void Should_Set_Owner_On_Child_Style_2() + { + var host = new Mock(); + var target = new Styles(); + + var style = new Mock(); + var resourceProvider = style.As(); + target.Add(style.Object); + + host.ResetCalls(); + ((IResourceProvider)target).AddOwner(host.Object); + resourceProvider.Verify(x => x.AddOwner(host.Object), Times.Once); + } [Fact] public void Finds_Resource_In_Merged_Dictionary() { From 21bddb1ced9747ac171ff12e5748902dcfbf39c0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 19 May 2020 09:12:08 +0200 Subject: [PATCH 05/11] Added test that fails after resource refactor. `Changing_Resource_In_Templated_Parent_Should_Affect_Templated_Child` now fails because resource changed messages no longer traverse into templated children. --- .../Primitives/TemplatedControlTests.cs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs index d069753e85..2930098371 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs @@ -10,6 +10,7 @@ using Avalonia.Styling; using Avalonia.UnitTests; using Avalonia.VisualTree; using Xunit; +using Avalonia.Media; namespace Avalonia.Controls.UnitTests.Primitives { @@ -512,6 +513,68 @@ namespace Avalonia.Controls.UnitTests.Primitives } } + [Fact] + public void Templated_Child_Should_Find_Resource_In_TemplatedParent() + { + var target = new ContentControl + { + Resources = + { + { "red", Brushes.Red }, + }, + Template = new FuncControlTemplate((x, scope) => + { + var result = new ContentPresenter + { + Name = "PART_ContentPresenter", + [!ContentPresenter.ContentProperty] = x[!ContentControl.ContentProperty], + }.RegisterInNameScope(scope); + + result.Bind(ContentPresenter.BackgroundProperty, result.GetResourceObservable("red")); + + return result; + }), + }; + + target.ApplyTemplate(); + + var contentPresenter = Assert.IsType(target.GetVisualChildren().Single()); + Assert.Same(Brushes.Red, contentPresenter.Background); + } + + [Fact] + public void Changing_Resource_In_Templated_Parent_Should_Affect_Templated_Child() + { + var target = new ContentControl + { + Resources = + { + { "red", Brushes.Red }, + }, + Template = new FuncControlTemplate((x, scope) => + { + var result = new ContentPresenter + { + Name = "PART_ContentPresenter", + [!ContentPresenter.ContentProperty] = x[!ContentControl.ContentProperty], + }.RegisterInNameScope(scope); + + result.Bind(ContentPresenter.BackgroundProperty, result.GetResourceObservable("red")); + + return result; + }), + }; + + target.ApplyTemplate(); + + var contentPresenter = Assert.IsType(target.GetVisualChildren().Single()); + Assert.Same(Brushes.Red, contentPresenter.Background); + + target.Resources["red"] = Brushes.Green; + + Assert.Same(Brushes.Green, contentPresenter.Background); + } + private static IControl ScrollingContentControlTemplate(ContentControl control, INameScope scope) { return new Border From 0f5c97109bb159b46ad4f33fba23bb19b915de92 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 19 May 2020 09:12:16 +0200 Subject: [PATCH 06/11] Fix outdated naming. --- .../StyledElementTests.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs index 857815f5b4..ef5a097f87 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs @@ -101,7 +101,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void AttachedToLogicalParent_Should_Be_Called_When_Added_To_Tree() + public void AttachedToLogicalTree_Should_Be_Called_When_Added_To_Tree() { var root = new TestRoot(); var parent = new Border(); @@ -128,9 +128,9 @@ namespace Avalonia.Styling.UnitTests Assert.True(childRaised); Assert.True(grandchildRaised); } - + [Fact] - public void AttachedToLogicalParent_Should_Be_Called_Before_Parent_Change_Signalled() + public void AttachedToLogicalTree_Should_Be_Called_Before_Parent_Change_Signalled() { var root = new TestRoot(); var child = new Border(); @@ -150,7 +150,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void AttachedToLogicalParent_Should_Not_Be_Called_With_GlobalStyles_As_Root() + public void AttachedToLogicalTree_Should_Not_Be_Called_With_GlobalStyles_As_Root() { var globalStyles = Mock.Of(); var root = new TestRoot { StylingParent = globalStyles }; @@ -169,7 +169,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void AttachedToLogicalParent_Should_Have_Source_Set() + public void AttachedToLogicalTree_Should_Have_Source_Set() { var root = new TestRoot(); var canvas = new Canvas(); @@ -191,7 +191,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void AttachedToLogicalParent_Should_Have_Parent_Set() + public void AttachedToLogicalTree_Should_Have_Parent_Set() { var root = new TestRoot(); var canvas = new Canvas(); @@ -213,7 +213,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void DetachedFromLogicalParent_Should_Be_Called_When_Removed_From_Tree() + public void DetachedFromLogicalTree_Should_Be_Called_When_Removed_From_Tree() { var root = new TestRoot(); var parent = new Border(); @@ -239,7 +239,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void DetachedFromLogicalParent_Should_Not_Be_Called_With_GlobalStyles_As_Root() + public void DetachedFromLogicalTree_Should_Not_Be_Called_With_GlobalStyles_As_Root() { var globalStyles = Mock.Of(); var root = new TestRoot { StylingParent = globalStyles }; @@ -259,7 +259,7 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void Parent_Should_Be_Null_When_DetachedFromLogicalParent_Called() + public void Parent_Should_Be_Null_When_DetachedFromLogicalTree_Called() { var target = new TestControl(); var root = new TestRoot(target); From 7022c137e74f942e3f1b21b633c778f41af2eeac Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 19 May 2020 09:31:36 +0200 Subject: [PATCH 07/11] Propagate ResourcesChanged to template children. Makes `Changing_Resource_In_Templated_Parent_Should_Affect_Templated_Child` pass again. --- .../Primitives/TemplatedControl.cs | 15 ++++++++ src/Avalonia.Styling/StyledElement.cs | 37 +++++++++++++------ .../StyledElementTests_Resources.cs | 24 ------------ 3 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index 97d01a38bd..820d5777f5 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -287,6 +287,21 @@ namespace Avalonia.Controls.Primitives return this; } + protected sealed override void NotifyChildResourcesChanged(ResourcesChangedEventArgs e) + { + var count = VisualChildren.Count; + + for (var i = 0; i < count; ++i) + { + if (VisualChildren[i] is ILogical logical) + { + logical.NotifyResourcesChanged(e); + } + } + + base.NotifyChildResourcesChanged(e); + } + /// protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) { diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index 2a7fdb7a0a..8a536485a1 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -493,6 +493,28 @@ namespace Avalonia } } + /// + /// Notifies child controls that a change has been made to resources that apply to them. + /// + /// The event args. + protected virtual void NotifyChildResourcesChanged(ResourcesChangedEventArgs e) + { + if (_logicalChildren is object) + { + var count = _logicalChildren.Count; + + if (count > 0) + { + e ??= new ResourcesChangedEventArgs(); + + for (var i = 0; i < count; ++i) + { + _logicalChildren[i].NotifyResourcesChanged(e); + } + } + } + } + /// /// Called when the styled element is added to a rooted logical tree. /// @@ -781,19 +803,10 @@ namespace Avalonia ResourcesChanged(this, e); } - if (propagate && _logicalChildren is object) + if (propagate) { - var count = _logicalChildren.Count; - - if (count > 0) - { - e ??= new ResourcesChangedEventArgs(); - - for (var i = 0; i < count; ++i) - { - _logicalChildren[i].NotifyResourcesChanged(e); - } - } + e ??= new ResourcesChangedEventArgs(); + NotifyChildResourcesChanged(e); } } diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs index b3289f3804..05315380fd 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests_Resources.cs @@ -166,30 +166,6 @@ namespace Avalonia.Controls.UnitTests Assert.True(raisedOnChild); } - [Fact] - public void Adding_Resource_Should_Call_Not_Raise_ResourceChanged_On_Template_Children() - { - Border child; - - var target = new ContentControl - { - Content = child = new Border(), - Template = ContentControlTemplate(), - }; - - var raisedOnTarget = false; - var raisedOnPresenter = false; - - target.Measure(Size.Infinity); - target.ResourcesChanged += (_, __) => raisedOnTarget = true; - target.Presenter.ResourcesChanged += (_, __) => raisedOnPresenter = true; - - target.Resources.Add("foo", "bar"); - - Assert.True(raisedOnTarget); - Assert.False(raisedOnPresenter); - } - [Fact] public void Adding_Resource_To_Styles_Should_Raise_ResourceChanged() { From b6aec8f4e039f8291679b11d379151ccd0530945 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 19 May 2020 09:33:41 +0200 Subject: [PATCH 08/11] Propagate ResourcesChanged to visual layers. --- .../Primitives/VisualLayerManager.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/VisualLayerManager.cs b/src/Avalonia.Controls/Primitives/VisualLayerManager.cs index 385e7cc567..86aeb5c62a 100644 --- a/src/Avalonia.Controls/Primitives/VisualLayerManager.cs +++ b/src/Avalonia.Controls/Primitives/VisualLayerManager.cs @@ -55,8 +55,15 @@ namespace Avalonia.Controls.Primitives ((ILogical)layer).NotifyAttachedToLogicalTree(new LogicalTreeAttachmentEventArgs(_logicalRoot, layer, this)); InvalidateArrange(); } - - + + protected override void NotifyChildResourcesChanged(ResourcesChangedEventArgs e) + { + foreach (var l in _layers) + ((ILogical)l).NotifyResourcesChanged(e); + + base.NotifyChildResourcesChanged(e); + } + protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) { base.OnAttachedToLogicalTree(e); @@ -74,7 +81,6 @@ namespace Avalonia.Controls.Primitives ((ILogical)l).NotifyDetachedFromLogicalTree(e); } - protected override Size MeasureOverride(Size availableSize) { foreach (var l in _layers) From dcdaf5e0bf775cb02060d48a2952ce0176c540d6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 19 May 2020 22:27:20 +0200 Subject: [PATCH 09/11] Don't raise ResourceChanged on reparenting... ...when the new parent isn't attached to the tree. This prevents us sending a load of useless `ResourcesChanged` events. --- src/Avalonia.Styling/StyledElement.cs | 10 ++++-- .../Primitives/TemplatedControlTests.cs | 2 ++ .../DynamicResourceExtensionTests.cs | 35 ++++++++++--------- .../StyledElementTests.cs | 4 +-- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index 8a536485a1..09991f2ece 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -418,12 +418,16 @@ namespace Avalonia var e = new LogicalTreeAttachmentEventArgs(newRoot, this, parent); OnAttachedToLogicalTreeCore(e); } - else + else if (parent is null) { // If we were attached to the logical tree, we piggyback on the tree traversal // there to raise resources changed notifications. If we're being removed from - // the logical tree or attached to a control which is not rooted, then traverse - // the tree raising notifications now. + // the logical tree, then traverse the tree raising notifications now. + // + // We don't raise resources changed notifications if we're being attached to a + // non-rooted control beacuse it's unlikely that dynamic resources need to be + // correct until the control is added to the tree, and it causes a *lot* of + // notifications. NotifyResourcesChanged(); } diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs index 2930098371..58d2eedb1f 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs @@ -536,6 +536,7 @@ namespace Avalonia.Controls.UnitTests.Primitives }), }; + var root = new TestRoot(target); target.ApplyTemplate(); var contentPresenter = Assert.IsType(target.GetVisualChildren().Single()); @@ -565,6 +566,7 @@ namespace Avalonia.Controls.UnitTests.Primitives }), }; + var root = new TestRoot(target); target.ApplyTemplate(); var contentPresenter = Assert.IsType(target.GetVisualChildren().Single()); diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs index 412b5ec083..ed3e4c986b 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs @@ -634,33 +634,36 @@ namespace Avalonia.Markup.Xaml.UnitTests.MarkupExtensions [Fact] public void Control_Property_Is_Updated_When_Parent_Is_Changed() { - var xaml = @" - - + #ff506070 - + -"; +"; - var loader = new AvaloniaXamlLoader(); - var userControl = (UserControl)loader.Load(xaml); - var border = userControl.FindControl("border"); + var loader = new AvaloniaXamlLoader(); + var window = (Window)loader.Load(xaml); + var border = window.FindControl("border"); - DelayedBinding.ApplyBindings(border); + DelayedBinding.ApplyBindings(border); - var brush = (SolidColorBrush)border.Background; - Assert.Equal(0xff506070, brush.Color.ToUint32()); + var brush = (SolidColorBrush)border.Background; + Assert.Equal(0xff506070, brush.Color.ToUint32()); - userControl.Content = null; + window.Content = null; - Assert.Null(border.Background); + Assert.Null(border.Background); - userControl.Content = border; + window.Content = border; - brush = (SolidColorBrush)border.Background; - Assert.Equal(0xff506070, brush.Color.ToUint32()); + brush = (SolidColorBrush)border.Background; + Assert.Equal(0xff506070, brush.Color.ToUint32()); + } } [Fact] diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs index ef5a097f87..fcbccba695 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs @@ -530,9 +530,9 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void Changing_Parent_Raises_ResourcesChanged() + public void Adding_To_Logical_Tree_Raises_ResourcesChanged() { - var target = new TestControl(); + var target = new TestRoot(); var parent = new Decorator { Resources = { { "foo", "bar" } } }; var raised = 0; From 0a1cb6fdf8a0ce490162d70be0671ad5f31afe8d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 19 May 2020 22:39:57 +0200 Subject: [PATCH 10/11] Added ResourcesChangedEventArgs.Empty. We don't need to allocate a new instance each time. --- src/Avalonia.Controls/Application.cs | 2 +- src/Avalonia.Styling/Controls/ResourceDictionary.cs | 6 +++--- src/Avalonia.Styling/Controls/ResourcesChangedEventArgs.cs | 1 + src/Avalonia.Styling/StyledElement.cs | 6 +++--- src/Avalonia.Styling/Styling/Style.cs | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Controls/Application.cs b/src/Avalonia.Controls/Application.cs index f0fe98a4b7..3bf72460df 100644 --- a/src/Avalonia.Controls/Application.cs +++ b/src/Avalonia.Controls/Application.cs @@ -260,7 +260,7 @@ namespace Avalonia try { _notifyingResourcesChanged = true; - ResourcesChanged?.Invoke(this, new ResourcesChangedEventArgs()); + ResourcesChanged?.Invoke(this, ResourcesChangedEventArgs.Empty); } finally { diff --git a/src/Avalonia.Styling/Controls/ResourceDictionary.cs b/src/Avalonia.Styling/Controls/ResourceDictionary.cs index cedd116e92..c56dd74143 100644 --- a/src/Avalonia.Styling/Controls/ResourceDictionary.cs +++ b/src/Avalonia.Styling/Controls/ResourceDictionary.cs @@ -146,7 +146,7 @@ namespace Avalonia.Controls if (hasResources) { - owner.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); + owner.NotifyHostedResourcesChanged(ResourcesChangedEventArgs.Empty); } } @@ -171,14 +171,14 @@ namespace Avalonia.Controls if (hasResources) { - owner.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); + owner.NotifyHostedResourcesChanged(ResourcesChangedEventArgs.Empty); } } } private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { - Owner?.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); + Owner?.NotifyHostedResourcesChanged(ResourcesChangedEventArgs.Empty); } } } diff --git a/src/Avalonia.Styling/Controls/ResourcesChangedEventArgs.cs b/src/Avalonia.Styling/Controls/ResourcesChangedEventArgs.cs index 8e3acc0e88..ee2ece8a26 100644 --- a/src/Avalonia.Styling/Controls/ResourcesChangedEventArgs.cs +++ b/src/Avalonia.Styling/Controls/ResourcesChangedEventArgs.cs @@ -4,5 +4,6 @@ namespace Avalonia.Controls { public class ResourcesChangedEventArgs : EventArgs { + public static new readonly ResourcesChangedEventArgs Empty = new ResourcesChangedEventArgs(); } } diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index 09991f2ece..bdd01924f1 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -509,7 +509,7 @@ namespace Avalonia if (count > 0) { - e ??= new ResourcesChangedEventArgs(); + e ??= ResourcesChangedEventArgs.Empty; for (var i = 0; i < count; ++i) { @@ -803,13 +803,13 @@ namespace Avalonia { if (ResourcesChanged is object) { - e ??= new ResourcesChangedEventArgs(); + e ??= ResourcesChangedEventArgs.Empty; ResourcesChanged(this, e); } if (propagate) { - e ??= new ResourcesChangedEventArgs(); + e ??= ResourcesChangedEventArgs.Empty; NotifyChildResourcesChanged(e); } } diff --git a/src/Avalonia.Styling/Styling/Style.cs b/src/Avalonia.Styling/Styling/Style.cs index ae866ebf46..00819ef7be 100644 --- a/src/Avalonia.Styling/Styling/Style.cs +++ b/src/Avalonia.Styling/Styling/Style.cs @@ -67,7 +67,7 @@ namespace Avalonia.Styling if (hadResources || _resources.HasResources) { - Owner.NotifyHostedResourcesChanged(new ResourcesChangedEventArgs()); + Owner.NotifyHostedResourcesChanged(ResourcesChangedEventArgs.Empty); } } } From ab9522f83dfa1ae82189ed61a0e2e66c1ee2fe15 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 20 May 2020 12:20:36 +0200 Subject: [PATCH 11/11] Fix silly mistake. --- src/Avalonia.Styling/Styling/Styles.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Styling/Styling/Styles.cs b/src/Avalonia.Styling/Styling/Styles.cs index f9cf3910f3..7c79060930 100644 --- a/src/Avalonia.Styling/Styling/Styles.cs +++ b/src/Avalonia.Styling/Styling/Styles.cs @@ -83,7 +83,7 @@ namespace Avalonia.Styling { get { - if (Count > 0) + if (_resources?.Count > 0) { return true; }