Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Runtime updating of AppTheme values #10442

Merged
merged 10 commits into from
Apr 28, 2020
Merged

Runtime updating of AppTheme values #10442

merged 10 commits into from
Apr 28, 2020

Conversation

jfversluis
Copy link
Member

Description of Change

This implements the runtime updating of AppTheme values.

Issues Resolved

API Changes

Added:

  • void BindableObject.SetOnAppTheme(T Light, T Dark, T Default = default);
  • void BindableObject.SetAppThemeColor(Color Light, Color Dark, Color Default = default);

Platforms Affected

  • Core/XAML (all platforms)

Behavioral/Visual Changes

The INotifyPropertyChanged was removed from the API. It doesn't make much sense to have that on there (anymore). People that were using that might see breaks.

Before/After Screenshots

iOS

ezgif com-video-to-gif

Android

ezgif com-video-to-gif (1)

Testing Procedure

Go to the right gallery page(s) and switch appearance

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jfversluis jfversluis added t/bug 🐛 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. a/darkmode labels Apr 24, 2020
@jfversluis jfversluis added this to the 4.6.0 milestone Apr 24, 2020
@jfversluis jfversluis added this to To do in v4.6.0 via automation Apr 24, 2020
@@ -275,8 +275,7 @@ void SetEffectiveFlowDirection(EffectiveFlowDirection value, bool fireFlowDirect

internal VisualElement()
{
if (Device.Flags?.IndexOf(ExperimentalFlags.AppThemeExperimental) > 0)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was removed intentionally. It caused some unexpected behavior

@jfversluis jfversluis added the API-change Heads-up to reviewers that this PR may contain an API change label Apr 24, 2020
@samhouts samhouts added this to Ready for Review (PRs) in Sprint 169 Apr 24, 2020
Sprint 169 automation moved this from Ready for Review (PRs) to In progress Apr 24, 2020
Xamarin.Forms.Core/Application.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Platform.iOS/Renderers/PageRenderer.cs Outdated Show resolved Hide resolved
@jfversluis jfversluis moved this from In progress to Ready for Review (PRs) in Sprint 169 Apr 26, 2020
@samhouts samhouts requested a review from rmarinho April 27, 2020 21:40
Sprint 169 automation moved this from Ready for Review (PRs) to In progress Apr 28, 2020
Copy link
Member

@samhouts samhouts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there might be an exception raised on Android when you rotate. Check UI tests.

@jfversluis
Copy link
Member Author

Nice one! Was due to the experimental flag validation while raising the event. Should be fixed now 🤞

@@ -274,7 +275,7 @@
<ItemGroup />
<Target Name="_CopyNUnitTestAdapterFiles" AfterTargets="Build">
<ItemGroup>
<_NUnitTestAdapterFiles Include="$(NuGetPackageRoot)NUnit3TestAdapter\%(Version)\build\net35\**" Condition="@(PackageReference -> '%(Identity)') == 'NUnit3TestAdapter'" InProject="False" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this have changed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh. No. But it's added again on the next line but it just escapes the > to a gt;. I'm guessing that's an inconsistency between VS versions/VSWin and VSMac? I'll revert to be sure

Sprint 169 automation moved this from In progress to Ready for Review (PRs) Apr 28, 2020
@samhouts samhouts merged commit 6c40121 into 4.6.0 Apr 28, 2020
v4.6.0 automation moved this from To do to Done Apr 28, 2020
Sprint 169 automation moved this from Ready for Review (PRs) to Done Apr 28, 2020
@samhouts samhouts deleted the apptheme-runtimeupdate branch April 28, 2020 18:31
@samhouts samhouts added this to Done in DarkMode/AppThemes Apr 28, 2020
@samhouts samhouts removed this from Done in DarkMode/AppThemes May 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/darkmode a/Xaml </> API-change Heads-up to reviewers that this PR may contain an API change blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. ControlGallery p/iOS 🍎 t/bug 🐛
Projects
No open projects
Sprint 169
  
Done
v4.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants