From 2982b418135c9a34979ee2aef0ed5142e5fcbadc Mon Sep 17 00:00:00 2001 From: Stephane Delcroix Date: Fri, 15 Mar 2024 10:15:41 +0100 Subject: [PATCH] [C] use ResourcesChanged to propagate Theme instead of subscribing to ThemeChanged event, use the ResourcesChanged mechanism of DynamicResource to propagate the change - related to #8713 - alternative to #19931 - fixes #18505 --- Microsoft.Maui-dev.sln | 2 +- src/Controls/src/Core/AppThemeBinding.cs | 57 +++++++++++-------- .../src/Core/Application/Application.cs | 10 ++++ src/Controls/src/Core/Shell/Shell.cs | 2 + .../tests/Core.UnitTests/AppThemeTests.cs | 7 +++ .../Core.UnitTests/DynamicResourceTests.cs | 15 +++-- 6 files changed, 62 insertions(+), 31 deletions(-) diff --git a/Microsoft.Maui-dev.sln b/Microsoft.Maui-dev.sln index 5269c698cefb..c38c4fe99ab0 100644 --- a/Microsoft.Maui-dev.sln +++ b/Microsoft.Maui-dev.sln @@ -241,7 +241,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "UITest.Appium", "src\TestUt EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "UITest.NUnit", "src\TestUtils\src\UITest.NUnit\UITest.NUnit.csproj", "{A307B624-48D4-494E-A70D-5B3CDF6620CF}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Controls.SourceGen.UnitTests", "src\Controls\tests\SourceGen.UnitTests\Conrtrols.SourceGen.UnitTests\Controls.SourceGen.UnitTests.csproj", "{06747B55-91DB-47F5-B7A2-56526C28A0D3}" +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Controls.SourceGen.UnitTests", "src\Controls\tests\SourceGen.UnitTests\Controls.SourceGen.UnitTests\Controls.SourceGen.UnitTests.csproj", "{06747B55-91DB-47F5-B7A2-56526C28A0D3}" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SourceGen.UnitTests", "src\Controls\tests\SourceGen.UnitTests\SourceGen.UnitTests.csproj", "{BC7F7C82-694F-4B97-86FC-273FB3FACA25}" EndProject diff --git a/src/Controls/src/Core/AppThemeBinding.cs b/src/Controls/src/Core/AppThemeBinding.cs index f97d4b296a31..7d0f8c470065 100644 --- a/src/Controls/src/Core/AppThemeBinding.cs +++ b/src/Controls/src/Core/AppThemeBinding.cs @@ -8,10 +8,37 @@ namespace Microsoft.Maui.Controls { class AppThemeBinding : BindingBase { + class AppThemeProxy : Element + { + public AppThemeProxy(Element parent, AppThemeBinding binding) + { + _parent = parent; + Binding = binding; + this.SetDynamicResource(AppThemeProperty, "__ApplicationTheme__"); + ((IElementDefinition)parent).AddResourcesChangedListener(OnParentResourcesChanged); + } + + public static BindableProperty AppThemeProperty = BindableProperty.Create("AppTheme", typeof(AppTheme), typeof(AppThemeBinding), AppTheme.Unspecified, + propertyChanged: (bindable, oldValue, newValue) => ((AppThemeProxy)bindable).OnAppThemeChanged()); + readonly Element _parent; + + public void OnAppThemeChanged() + { + Binding.Apply(false); + } + + public AppThemeBinding Binding { get; } + + public void Unsubscribe() + { + ((IElementDefinition)_parent).RemoveResourcesChangedListener(OnParentResourcesChanged); + } + } + WeakReference _weakTarget; BindableProperty _targetProperty; - bool _attached; SetterSpecificity specificity; + AppThemeProxy _appThemeProxy; internal override BindingBase Clone() { @@ -34,7 +61,6 @@ internal override void Apply(bool fromTarget) { base.Apply(fromTarget); ApplyCore(); - SetAttached(true); } internal override void Apply(object context, BindableObject bindObj, BindableProperty targetProperty, bool fromBindingContextChanged, SetterSpecificity specificity) @@ -44,12 +70,15 @@ internal override void Apply(object context, BindableObject bindObj, BindablePro base.Apply(context, bindObj, targetProperty, fromBindingContextChanged, specificity); this.specificity = specificity; ApplyCore(false); - SetAttached(true); + _appThemeProxy = new AppThemeProxy(bindObj as Element, this); + } internal override void Unapply(bool fromBindingContextChanged = false) { - SetAttached(false); + _appThemeProxy?.Unsubscribe(); + _appThemeProxy = null; + base.Unapply(fromBindingContextChanged); _weakTarget = null; _targetProperty = null; @@ -62,7 +91,6 @@ void ApplyCore(bool dispatch = false) { if (_weakTarget == null || !_weakTarget.TryGetTarget(out var target)) { - SetAttached(false); return; } @@ -147,24 +175,5 @@ target is VisualElement ve && _ => _isLightSet ? Light : Default, }; } - - void SetAttached(bool value) - { - var app = Application.Current; - if (app != null && _attached != value) - { - if (value) - { - // Going from false -> true - app.RequestedThemeChanged += OnRequestedThemeChanged; - } - else - { - // Going from true -> false - app.RequestedThemeChanged -= OnRequestedThemeChanged; - } - _attached = value; - } - } } } diff --git a/src/Controls/src/Core/Application/Application.cs b/src/Controls/src/Core/Application/Application.cs index 53b7d3b9674d..b2036a240baa 100644 --- a/src/Controls/src/Core/Application/Application.cs +++ b/src/Controls/src/Core/Application/Application.cs @@ -101,6 +101,15 @@ public Page? MainPage _singleWindowMainPage = value; + if (value is not null) + { + OnParentResourcesChanged(this.GetMergedResources()); + OnParentResourcesChanged([new KeyValuePair("__ApplicationTheme__", _lastAppTheme)]); + ((IElementDefinition)this).AddResourcesChangedListener(value.OnParentResourcesChanged); + } else if (MainPage is not null) + { + ((IElementDefinition)this).RemoveResourcesChangedListener(MainPage.OnParentResourcesChanged); + } if (Windows.Count == 1) { Windows[0].Page = value; @@ -243,6 +252,7 @@ void TriggerThemeChangedActual() _themeChangedFiring = true; _lastAppTheme = newTheme; + OnParentResourcesChanged([new KeyValuePair("__ApplicationTheme__", newTheme)]); _weakEventManager.HandleEvent(this, new AppThemeChangedEventArgs(newTheme), nameof(RequestedThemeChanged)); } finally diff --git a/src/Controls/src/Core/Shell/Shell.cs b/src/Controls/src/Core/Shell/Shell.cs index eb46e39bd0ac..ad4738e139b3 100644 --- a/src/Controls/src/Core/Shell/Shell.cs +++ b/src/Controls/src/Core/Shell/Shell.cs @@ -827,10 +827,12 @@ private protected override void OnHandlerChangingCore(HandlerChangingEventArgs a return; if (args.NewHandler == null) +#pragma warning disable CS0618 // Type or member is obsolete Application.Current.RequestedThemeChanged -= OnRequestedThemeChanged; if (args.NewHandler != null && args.OldHandler == null) Application.Current.RequestedThemeChanged += OnRequestedThemeChanged; +#pragma warning restore CS0618 // Type or member is obsolete } private void OnRequestedThemeChanged(object sender, AppThemeChangedEventArgs e) diff --git a/src/Controls/tests/Core.UnitTests/AppThemeTests.cs b/src/Controls/tests/Core.UnitTests/AppThemeTests.cs index 2d6e1eedc926..a458cbf89380 100644 --- a/src/Controls/tests/Core.UnitTests/AppThemeTests.cs +++ b/src/Controls/tests/Core.UnitTests/AppThemeTests.cs @@ -34,6 +34,7 @@ public void ThemeChangeUsingSetAppThemeColor() { Text = "Green on Light, Red on Dark" }; + app.LoadPage(new ContentPage {Content = label}); label.SetAppThemeColor(Label.TextColorProperty, Colors.Green, Colors.Red); Assert.Equal(Colors.Green, label.TextColor); @@ -46,11 +47,14 @@ public void ThemeChangeUsingSetAppThemeColor() [Fact] public void ThemeChangeUsingSetAppTheme() { + var label = new Label { Text = "Green on Light, Red on Dark" }; + app.LoadPage(new ContentPage {Content = label}); + label.SetAppTheme(Label.TextColorProperty, Colors.Green, Colors.Red); Assert.Equal(Colors.Green, label.TextColor); @@ -66,6 +70,7 @@ public void ThemeChangeUsingSetBinding() { Text = "Green on Light, Red on Dark" }; + app.LoadPage(new ContentPage {Content = label}); label.SetBinding(Label.TextColorProperty, new AppThemeBinding { Light = Colors.Green, Dark = Colors.Red }); Assert.Equal(Colors.Green, label.TextColor); @@ -82,6 +87,7 @@ public void ThemeChangeUsingUserAppTheme() { Text = "Green on Light, Red on Dark" }; + app.LoadPage(new ContentPage {Content = label}); label.SetAppThemeColor(Label.TextColorProperty, Colors.Green, Colors.Red); Assert.Equal(Colors.Green, label.TextColor); @@ -225,6 +231,7 @@ public void NullApplicationCurrentFallsBackToEssentials() { Text = "Green on Light, Red on Dark" }; + app.LoadPage(new ContentPage {Content = label}); label.SetAppThemeColor(Label.TextColorProperty, Colors.Green, Colors.Red); diff --git a/src/Controls/tests/Core.UnitTests/DynamicResourceTests.cs b/src/Controls/tests/Core.UnitTests/DynamicResourceTests.cs index 09c85bbf3182..1d937131fe0a 100644 --- a/src/Controls/tests/Core.UnitTests/DynamicResourceTests.cs +++ b/src/Controls/tests/Core.UnitTests/DynamicResourceTests.cs @@ -107,8 +107,9 @@ public void SettingResourceTriggersValueChanged() var label = new Label(); label.SetDynamicResource(Label.TextProperty, "foo"); Assert.Equal(Label.TextProperty.DefaultValue, label.Text); - label.Resources = new ResourceDictionary(); - label.Resources.Add("foo", "FOO"); + label.Resources = new ResourceDictionary { + { "foo", "FOO" } + }; Assert.Equal("FOO", label.Text); } @@ -126,8 +127,9 @@ public void AddingAResourceDictionaryTriggersValueChangedForExistingValues() [Fact] public void ValueChangedTriggeredOnSubscribeIfKeyAlreadyExists() { - var label = new Label(); - label.Resources = new ResourceDictionary { { "foo", "FOO" } }; + var label = new Label { + Resources = new ResourceDictionary { { "foo", "FOO" } } + }; Assert.Equal(Label.TextProperty.DefaultValue, label.Text); label.SetDynamicResource(Label.TextProperty, "foo"); Assert.Equal("FOO", label.Text); @@ -136,8 +138,9 @@ public void ValueChangedTriggeredOnSubscribeIfKeyAlreadyExists() [Fact] public void RemoveDynamicResourceStopsUpdating() { - var label = new Label(); - label.Resources = new ResourceDictionary { { "foo", "FOO" } }; + var label = new Label { + Resources = new ResourceDictionary { { "foo", "FOO" } } + }; Assert.Equal(Label.TextProperty.DefaultValue, label.Text); label.SetDynamicResource(Label.TextProperty, "foo"); Assert.Equal("FOO", label.Text);