Conversation
Add a new test project Uno.Extensions.Core.WinUI.Tests with runtime tests verifying the ElementThemeChanged fix for system theme tracking: - When saved theme is System and OS theme changes, ThemeChanged reports AppTheme.System (not Dark/Light) - Saved theme setting is not overwritten during OS theme changes - Multiple OS theme changes continue reporting System correctly - Explicit (non-System) saved themes do not take the System shortcut Also adds InternalsVisibleTo for the test assembly and wires the test project into the RuntimeTests runner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where switching OS themes while the app is set to follow the system theme (AppTheme.System) would "freeze" the app on the detected theme. The root cause was that ElementThemeChanged called InternalSetThemeAsync, which set an explicit RequestedTheme, overriding ElementTheme.Default and preventing future OS theme tracking.
Changes:
- Added an early-return guard in
ThemeService.ElementThemeChangedthat preservesElementTheme.Defaultwhen the saved theme isSystem, firingThemeChangedwithAppTheme.Systeminstead of callingInternalSetThemeAsync. - Added a new test project (
Uno.Extensions.Core.WinUI.Tests) with unit tests verifying correct behavior for system and explicit theme scenarios. - Exposed internals to the new test project via
InternalsVisibleTo.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
ThemeService.cs |
Early-return guard in ElementThemeChanged to avoid overriding ElementTheme.Default when following system theme |
Uno.Extensions.Core.WinUI.Tests.csproj |
New test project for Core UI extensions |
Given_ThemeService.cs |
Runtime tests verifying the fix for system and explicit theme switching |
SynchronousDispatcher.cs |
Test helper implementing IDispatcher synchronously |
InMemorySettings.cs |
Test helper implementing ISettings with in-memory dictionary |
Uno.Extensions.Core.WinUI.csproj |
Added InternalsVisibleTo for the new test project |
Uno.Extensions.RuntimeTests.csproj |
Added project reference to the new test project |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-moss-0c5b8040f-3037.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-moss-0c5b8040f-3037.eastus2.azurestaticapps.net |
PR Type
What kind of change does this PR introduce?
What is the current behavior?
When the user's saved theme preference is
System(follow OS theme) and the OS theme changes (e.g. Light → Dark), theElementThemeChangedhandler inThemeServicecallsInternalSetThemeAsync, which sets an explicitRequestedTheme(e.g.ElementTheme.Dark) on the root element. This overridesElementTheme.Default, effectively "freezing" the app on that theme and preventing future OS theme changes from being tracked.What is the new behavior?
When the saved theme is
AppTheme.SystemandElementThemeChangedfires, the handler now:ThemeChangedevent withAppTheme.System(instead of the specific Dark/Light value)InternalSetThemeAsync, preservingElementTheme.Defaulton the root element so the app continues to follow the OS themeA new runtime test project (
Uno.Extensions.Core.WinUI.Tests) has been added with tests verifying:ThemeChangedreportsAppTheme.System(not Dark/Light) when following system themePR Checklist
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information
The fix is a small early-return guard in
ThemeService.ElementThemeChanged(11 lines added toThemeService.cs). The test project follows the existingNavigation.UI.Testspattern and is wired into theRuntimeTestsrunner.Internal Issue (If applicable):