Skip to content

Conversation

AndyMDoyle
Copy link
Contributor

@AndyMDoyle AndyMDoyle commented Jan 27, 2021

Fixes #3291

Sorry, my bad.

I didn't realise that ThemeHelper was setting the theme on elements by capturing a reference to the MainPage. I assumed it was relying on Application.RequestedTheme and changing this.

Balance is restored, and ThemeHelper has had some unused code removed. ThemeHelper.RootTheme has been refactored slightly to make it responsible for reading the value from one place (LocalSettings), and setting ThemeHelper.RootTheme will persist this change back to LocalSettings and call ApplyTheme() to have the current theme applied across the application.

…page can be captured for dynamic theme switching
@yaira2 yaira2 requested a review from tsvietOK January 27, 2021 18:24
@tsvietOK tsvietOK added the changes requested Changes are needed for this pull request label Feb 1, 2021
@gave92 gave92 requested a review from tsvietOK February 13, 2021 19:51
@tsvietOK tsvietOK added ready to merge Pull requests that are approved and ready to merge and removed changes requested Changes are needed for this pull request labels Feb 13, 2021
@yaira2 yaira2 changed the title Fix 3291 - Initialize theme after MainPage is constructed Fixed an issue where changing the theme settings wasn't working correctly Feb 14, 2021
@yaira2 yaira2 merged commit 6881985 into files-community:main Feb 14, 2021
@yaira2
Copy link
Member

yaira2 commented Feb 14, 2021

@AndyMDoyle Thank you!

@gave92 gave92 mentioned this pull request Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants