CommunityToolkit
Feat: Implement ThemeListener for winui3
Fixes
Closes #135
PR Type
What kind of change does this PR introduce?
Bugfix
What is the current behavior?
It does not work under WinUI3.
What is the new behavior?
This PR implements the broken ThemeListener, closes #135. I really need a working ThemeListener for handling Dark/Light theme changes. I am not concerned with HighContrast at the time being.
PR Checklist
Please check if your PR fulfills the following requirements:
- [x] Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
- [x] Based off latest main branch of toolkit
- [x] Tested code with current supported SDKs
- [x] Tests for the changes have been added (if applicable)
- [x] Header has been added to all new source files
- [x] Contains NO breaking changes
Other information
@HO-COOH please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@dotnet-policy-service agree [company="{your company}"]Options:
- (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
- (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"Contributor License Agreement
@dotnet-policy-service agree
@Arlodotexe we should look at reducing the verbosity of the build and seeing if we can hook into the checkbox somehow or something...
@HO-COOH thanks for the PR! The errors are about the setup around the PInvoke:
26>C:\a\Windows\Windows\components\Helpers\src\ThemeListenerHelperWindow.cs(16,23): error CA1060: Move pinvokes to native methods class (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1060) [C:\a\Windows\Windows\components\Helpers\src\CommunityToolkit.WinUI.Helpers.csproj::TargetFramework=net8.0-windows10.0.22621.0]
26>C:\a\Windows\Windows\components\Helpers\src\ThemeListenerHelperWindow.cs(214,6): error CA2101: Specify marshaling for P/Invoke string arguments (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2101) [C:\a\Windows\Windows\components\Helpers\src\CommunityToolkit.WinUI.Helpers.csproj::TargetFramework=net8.0-windows10.0.22621.0]
Are you familiar with the C#/Win32 project? That's the best way these days to setup PInvoke a bit more painlessly. It'd be great if we could just swap over to that and avoid a bunch of the code here from the PR for the native methods.
@niels9001 forgot that we didn't do this for TitleBar yet either: https://github.com/CommunityToolkit/Labs-Windows/blob/main/components/TitleBar/src/NativeMethods.cs - though once 1.6 is out, we may not worry about it.
@Arlodotexe we should look at reducing the verbosity of the build and seeing if we can hook into the checkbox somehow or something...
@HO-COOH thanks for the PR! The errors are about the setup around the PInvoke:
26>C:\a\Windows\Windows\components\Helpers\src\ThemeListenerHelperWindow.cs(16,23): error CA1060: Move pinvokes to native methods class (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1060) [C:\a\Windows\Windows\components\Helpers\src\CommunityToolkit.WinUI.Helpers.csproj::TargetFramework=net8.0-windows10.0.22621.0] 26>C:\a\Windows\Windows\components\Helpers\src\ThemeListenerHelperWindow.cs(214,6): error CA2101: Specify marshaling for P/Invoke string arguments (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2101) [C:\a\Windows\Windows\components\Helpers\src\CommunityToolkit.WinUI.Helpers.csproj::TargetFramework=net8.0-windows10.0.22621.0]Are you familiar with the C#/Win32 project? That's the best way these days to setup PInvoke a bit more painlessly. It'd be great if we could just swap over to that and avoid a bunch of the code here from the PR for the native methods.
@niels9001 forgot that we didn't do this for TitleBar yet either: https://github.com/CommunityToolkit/Labs-Windows/blob/main/components/TitleBar/src/NativeMethods.cs - though once 1.6 is out, we may not worry about it.
I do have some knowledge about it. I am concerned about whether it will be compatible with other builds. Will after installing this nuget still builds on like WASM builds?
@HO-COOH Have you ever tried ‘ThemeSettings’ WinRT API for the high contrast aware? Looks like you’re trying to create a window and hook an event change of window style. Also CsWin32 is a generator of P/Invoke so it shouldn't make a problem.
var windowId = MainWindow.Instance.AppWindow.Id;
ThemeSettings = ThemeSettings.CreateForWindowId(windowId);
ThemeSettings.Changed += (s, e) => { IsHighContrastChanged?.Invoke(this, s.HighContrast); };
you can check out there and try it out. https://github.com/files-community/Files/blob/a891cafa3a9378f2e3b51b04b5b3f3eb11981e14/src/Files.App/Services/App/AppThemeModeService.cs
To be honest, this isn’t handy too but more easy to handle imo.
Could it be that something is blocking the basic functionality? (things like) Theme adaptation should be as native as possible and I am convinced there's no need for a complex solution. Often less = more
My solution is way too easy. But maintainers may have to amend some spec to cope with the limitation that requires window IDs to hook up. Like we can have Register function.
Could it be that something is blocking the basic functionality? (things like) Theme adaptation should be as native as possible and I am convinced there's no need for a complex solution. Often less = more
The UISettings.ColorValueChanged api does not work in winui3 packaged app. See this issue
I thought it did (I made this service)? https://github.com/files-community/Files/blob/main/src/Files.App/Services/App/AppThemeModeService.cs#L124
I thought it did (I made this service)? https://github.com/files-community/Files/blob/main/src/Files.App/Services/App/AppThemeModeService.cs#L124
Is this self-contained or packaged? I believe it's self-contained, because I can directly run files.exe in the app's folder.
Then it should not be working on Windows 10. My video (and also Microsoft's own doc) is clear enough
This code is completely non trim/AOT-safe and needs a rewrite to be considered.
I am not familiar with this. Is there any doc I can look into?
This code is completely non trim/AOT-safe and needs a rewrite to be considered.
I am not familiar with this. Is there any doc I can look into?
I just open a PR on your repo, can you take a look?
I think Win32MessageLoop is too heavy. We can use RegNotifyChangeKeyValue to impl this. Here is a sample
public partial class ThemeListener
{
private const UIntPtr HKEY_CURRENT_USER = 0x80000001;
private const uint KEY_NOTIFY = 0x0010;
private const uint KEY_READ = ((0x00020000) | (0x0001) | (0x0008) | (0x0010)) & (~0x00100000);
public Action<bool>? ThemeChanged;
public async Task Loop(CancellationToken cancellationToken)
{
IntPtr subKeyName = Marshal.StringToCoTaskMemAnsi("SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Themes\\Personalize");
IntPtr hKeyRef = Marshal.AllocCoTaskMem(8);
int hr = RegOpenKeyExA(HKEY_CURRENT_USER, subKeyName, 0, KEY_NOTIFY | KEY_READ, hKeyRef);
Marshal.FreeCoTaskMem(subKeyName);
if (hr != 0)
{
Marshal.FreeCoTaskMem(hKeyRef);
return;
}
IntPtr hKey = Marshal.ReadIntPtr(hKeyRef);
Marshal.FreeCoTaskMem(hKeyRef);
subKeyName = Marshal.StringToCoTaskMemAnsi("AppsUseLightTheme");
IntPtr dwSizeRef = Marshal.AllocCoTaskMem(4);
IntPtr valueRef = Marshal.AllocCoTaskMem(4);
Marshal.WriteInt32(dwSizeRef, 4);
while (true)
{
RegNotifyChangeKeyValue(hKey, 1, 0x00000004, IntPtr.Zero, 0);
RegQueryValueExA(hKey, subKeyName, IntPtr.Zero, IntPtr.Zero, valueRef, dwSizeRef);
int value = Marshal.ReadInt32(valueRef);
bool result = value > 0;
if (cancellationToken.IsCancellationRequested)
{
break;
}
ThemeChanged?.Invoke(result);
}
Marshal.FreeCoTaskMem(subKeyName);
Marshal.FreeCoTaskMem(dwSizeRef);
Marshal.FreeCoTaskMem(valueRef);
RegCloseKey(hKey);
}
[LibraryImport("Advapi32.dll")]
private static partial int RegOpenKeyExA(UIntPtr hKey, IntPtr lpSubKey, uint ulOptions, uint samDesired, IntPtr phkResult);
[LibraryImport("Advapi32.dll")]
private static partial int RegQueryValueExA(IntPtr hKey, IntPtr lpValueName, IntPtr lpReserved, IntPtr lpType, IntPtr lpData, IntPtr lpcbData);
[LibraryImport("Advapi32.dll")]
private static partial int RegNotifyChangeKeyValue(IntPtr hKey, uint bWatchSubtree, uint dwNotifyFilter, IntPtr hEvent, uint fAsynchronous);
[LibraryImport("Advapi32.dll")]
private static partial int RegCloseKey(IntPtr hKey);
}
Packaged or not doesn't matter, if you disable virtualization of registry it should work and my app also work in that case. But you got the point, it doesn't work for most cases.
disable virtualization
This means the package can be harder to pass(or never pass) windows store verification.
@RERASER If you look at this, your method does not work if packaged.
It works on my packaged app, perhaps it is affected by the system version. My build version is 26100.2454.
Why not use Microsoft.win32.SystemEvents？
/// <summary>
/// A standard Win32 window proc for our broadcast window.
/// </summary>
private IntPtr WindowProc(IntPtr hWnd, int msg, nint wParam, nint lParam)
{
switch (msg)
{
case Interop.User32.WM_SETTINGCHANGE:
string? newString;
IntPtr newStringPtr = lParam;
if (lParam != 0)
{
newString = Marshal.PtrToStringUni(lParam);
if (newString != null)
{
newStringPtr = Marshal.StringToHGlobalUni(newString);
}
}
Interop.User32.PostMessageW(_windowHandle, Interop.User32.WM_REFLECT + msg, wParam, newStringPtr);
break;
case Interop.User32.WM_REFLECT + Interop.User32.WM_SETTINGCHANGE:
try
{
OnUserPreferenceChanging(msg - Interop.User32.WM_REFLECT, wParam, lParam);
OnUserPreferenceChanged(msg - Interop.User32.WM_REFLECT, wParam, lParam);
}
finally
{
try
{
if (lParam != 0)
{
Marshal.FreeHGlobal(lParam);
}
}
catch (Exception e)
{
Debug.Fail("Exception occurred while freeing memory: " + e.ToString());
}
}
break;
Why not use Microsoft.win32.SystemEvents？
@cnbluefire You are adding a whole new package as dependency. Plus I tried it, the UserPreferenceChangedEventArgs.Category gives UserPreferenceCategory.General when I toggle the theme, therefore a registry check is still necessary.
QQ for the folks on this thread, where do folks use the ThemeListener over just listening to the ActualThemeChanged event?
QQ for the folks on this thread, where do folks use the ThemeListener over just listening to the
ActualThemeChangedevent?
Because it is supposed to listen to system theme, not app theme which can be override by user (also more and more app supports this feature)
Using ShouldAppsUseDarkMode in the window message loop would be alternative to reg key.
[DllImport("uxtheme.dll", SetLastError = true, EntryPoint = "#132")]
public static extern bool ShouldAppsUseDarkMode();
Using ShouldAppsUseDarkMode in the window message loop would be alternative to reg key.
[DllImport("uxtheme.dll", SetLastError = true, EntryPoint = "#132")] public static extern bool ShouldAppsUseDarkMode();
Take a look at https://github.com/microsoft/WindowsAppSDK/issues/41#issuecomment-1945997050
Well, I know, all of them are undocumented, the key also may not be present likewise. I'm saying combining (respecting the reg key if any and fall backing to SystemUsesLightTheme too) might be a thorough way of doing this.
Suggestion: why not use UISettings and its ColorValuesChanged event. Example:
using Windows.UI.ViewManagement;
UISettings uiSettings = new UISettings();
uiSettings.ColorValuesChanged += (sender, args) =>
{
// This runs on a background thread, so dispatch to UI thread
DispatcherQueue.GetForCurrentThread().TryEnqueue(() =>
{
// Apply theme-specific changes here,
// Somewhere else, track your theme in an app theme service,
// track your window, apply to all open windows
// - no escape from window tracking in WinUI anyway :-)
});
};
It works well, except for the fact that you get the event 2 times for each color change (the mystery of WinUI code and why things are always different from logic), but you can just dedupe. No P/Invoke, fully public API, no hacks no tricks.
