From b14aa8276dbcce32a1a5f857a334e0c6d14a58e5 Mon Sep 17 00:00:00 2001 From: Davide Giacometti Date: Wed, 17 Apr 2024 16:39:19 +0200 Subject: [PATCH] [Settings]Theme override fix and cleanup (#32362) * theme override fix and cleanup * test fix --- .../ViewModelTests/General.cs | 11 --- .../Helpers/{Utils.cs => WindowHelper.cs} | 11 ++- .../Settings.UI/Services/ThemeService.cs | 48 +++++++++ .../Settings.UI/SettingsXAML/App.xaml.cs | 99 ++----------------- .../SettingsXAML/MainWindow.xaml.cs | 16 ++- .../SettingsXAML/OobeWindow.xaml.cs | 11 ++- .../SettingsXAML/Views/GeneralPage.xaml.cs | 3 - .../ViewModels/DashboardViewModel.cs | 2 - .../ViewModels/GeneralViewModel.cs | 17 +--- 9 files changed, 89 insertions(+), 129 deletions(-) rename src/settings-ui/Settings.UI/Helpers/{Utils.cs => WindowHelper.cs} (84%) create mode 100644 src/settings-ui/Settings.UI/Services/ThemeService.cs diff --git a/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/General.cs b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/General.cs index 11eaefb195..56da0a21bb 100644 --- a/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/General.cs +++ b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/General.cs @@ -53,7 +53,6 @@ namespace ViewModelTests runAsUserText: "GeneralSettings_RunningAsUserText", isElevated: false, isAdmin: false, - updateTheme: UpdateUIThemeMethod, ipcMSGCallBackFunc: sendMockIPCConfigMSG, ipcMSGRestartAsAdminMSGCallBackFunc: sendRestartAdminIPCMessage, ipcMSGCheckForUpdatesCallBackFunc: sendCheckForUpdatesIPCMessage, @@ -83,7 +82,6 @@ namespace ViewModelTests "GeneralSettings_RunningAsUserText", false, false, - UpdateUIThemeMethod, sendMockIPCConfigMSG, sendRestartAdminIPCMessage, sendCheckForUpdatesIPCMessage, @@ -120,7 +118,6 @@ namespace ViewModelTests "GeneralSettings_RunningAsUserText", false, false, - UpdateUIThemeMethod, sendMockIPCConfigMSG, sendRestartAdminIPCMessage, sendCheckForUpdatesIPCMessage, @@ -152,7 +149,6 @@ namespace ViewModelTests "GeneralSettings_RunningAsUserText", false, false, - UpdateUIThemeMethod, sendMockIPCConfigMSG, sendRestartAdminIPCMessage, sendCheckForUpdatesIPCMessage, @@ -186,7 +182,6 @@ namespace ViewModelTests "GeneralSettings_RunningAsUserText", false, false, - UpdateUIThemeMethod, sendMockIPCConfigMSG, sendRestartAdminIPCMessage, sendCheckForUpdatesIPCMessage, @@ -217,7 +212,6 @@ namespace ViewModelTests "GeneralSettings_RunningAsUserText", false, false, - UpdateUIThemeMethod, sendMockIPCConfigMSG, sendRestartAdminIPCMessage, sendCheckForUpdatesIPCMessage, @@ -243,10 +237,5 @@ namespace ViewModelTests Assert.IsTrue(modules.PowerLauncher); Assert.IsTrue(modules.ColorPicker); } - - public static int UpdateUIThemeMethod(string themeName) - { - return 0; - } } } diff --git a/src/settings-ui/Settings.UI/Helpers/Utils.cs b/src/settings-ui/Settings.UI/Helpers/WindowHelper.cs similarity index 84% rename from src/settings-ui/Settings.UI/Helpers/Utils.cs rename to src/settings-ui/Settings.UI/Helpers/WindowHelper.cs index 43a406feba..440d6cf902 100644 --- a/src/settings-ui/Settings.UI/Helpers/Utils.cs +++ b/src/settings-ui/Settings.UI/Helpers/WindowHelper.cs @@ -6,10 +6,11 @@ using System; using System.IO; using System.Runtime.InteropServices; using System.Text.Json; +using Microsoft.UI.Xaml; namespace Microsoft.PowerToys.Settings.UI.Helpers { - internal sealed class Utils + internal sealed class WindowHelper { private static string _placementPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), @"Microsoft\PowerToys\settings-placement.json"); @@ -45,5 +46,13 @@ namespace Microsoft.PowerToys.Settings.UI.Helpers { } } + + public static void SetTheme(Window window, ElementTheme theme) + { + if (window.Content is FrameworkElement rootElement) + { + rootElement.RequestedTheme = theme; + } + } } } diff --git a/src/settings-ui/Settings.UI/Services/ThemeService.cs b/src/settings-ui/Settings.UI/Services/ThemeService.cs new file mode 100644 index 0000000000..e817c275e4 --- /dev/null +++ b/src/settings-ui/Settings.UI/Services/ThemeService.cs @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using Microsoft.PowerToys.Settings.UI.Library; +using Microsoft.PowerToys.Settings.UI.Library.Interfaces; +using Microsoft.UI.Xaml; + +namespace Microsoft.PowerToys.Settings.UI.Services +{ + public class ThemeService + { + private readonly ISettingsRepository _generalSettingsRepository; + + public event EventHandler ThemeChanged; + + public ElementTheme Theme { get; private set; } = ElementTheme.Default; + + public ThemeService(ISettingsRepository generalSettingsRepository) + { + _generalSettingsRepository = generalSettingsRepository; + Theme = GetTheme(); + } + + public void ApplyTheme() + { + Theme = GetTheme(); + ThemeChanged?.Invoke(null, Theme); + } + + private ElementTheme GetTheme() + { + switch (_generalSettingsRepository.SettingsConfig.Theme.ToUpperInvariant()) + { + case "LIGHT": + return ElementTheme.Light; + case "DARK": + return ElementTheme.Dark; + case "SYSTEM": + return ElementTheme.Default; + default: + ManagedCommon.Logger.LogError($"Unexpected theme name: {_generalSettingsRepository.SettingsConfig.Theme}"); + return ElementTheme.Default; + } + } + } +} diff --git a/src/settings-ui/Settings.UI/SettingsXAML/App.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/App.xaml.cs index b00a018e8a..3a844aa644 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/App.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/App.xaml.cs @@ -5,20 +5,16 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Globalization; using System.IO; using System.Linq; -using System.Net.Http.Json; -using System.Runtime.InteropServices; using System.Text.Json; -using System.Threading; using System.Threading.Tasks; -using Common.UI; using interop; using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Helpers; using Microsoft.PowerToys.Settings.UI.Library; using Microsoft.PowerToys.Settings.UI.Library.Telemetry.Events; +using Microsoft.PowerToys.Settings.UI.Services; using Microsoft.PowerToys.Settings.UI.Views; using Microsoft.PowerToys.Telemetry; using Microsoft.UI.Xaml; @@ -73,8 +69,6 @@ namespace Microsoft.PowerToys.Settings.UI public static Action IPCMessageReceivedCallback { get; set; } - private static bool loggedImmersiveDarkException; - /// /// Initializes a new instance of the class. /// Initializes the singleton application object. This is the first line of authored code @@ -259,19 +253,6 @@ namespace Microsoft.PowerToys.Settings.UI ShellPage.OpenFlyoutCallback(p); } } - - if (SelectedTheme() == ElementTheme.Default) - { - try - { - themeListener = new ThemeListener(); - themeListener.ThemeChanged += (_) => HandleThemeChange(); - } - catch (Exception ex) - { - Logger.LogError($"HandleThemeChange exception. Please install .NET 4.", ex); - } - } } /// @@ -311,7 +292,7 @@ namespace Microsoft.PowerToys.Settings.UI ShowMessageDialog("The application is running in Debug mode.", "DEBUG"); #else /* If we try to run Settings as a standalone app, it will start PowerToys.exe if not running and open Settings again through it in the Dashboard page. */ - SettingsDeepLink.OpenSettings(SettingsDeepLink.SettingsWindow.Dashboard, true); + Common.UI.SettingsDeepLink.OpenSettings(Common.UI.SettingsDeepLink.SettingsWindow.Dashboard, true); Exit(); #endif } @@ -347,92 +328,24 @@ namespace Microsoft.PowerToys.Settings.UI return ipcmanager; } - public static ElementTheme SelectedTheme() - { - switch (SettingsRepository.GetInstance(settingsUtils).SettingsConfig.Theme.ToUpper(CultureInfo.InvariantCulture)) - { - case "DARK": return ElementTheme.Dark; - case "LIGHT": return ElementTheme.Light; - default: return ElementTheme.Default; - } - } - public static bool IsDarkTheme() { - var selectedTheme = SelectedTheme(); - return selectedTheme == ElementTheme.Dark || (selectedTheme == ElementTheme.Default && ThemeHelpers.GetAppTheme() == AppTheme.Dark); - } - - public static void HandleThemeChange() - { - try - { - bool isDark = IsDarkTheme(); - - if (settingsWindow != null) - { - var hWnd = WinRT.Interop.WindowNative.GetWindowHandle(settingsWindow); - ThemeHelpers.SetImmersiveDarkMode(hWnd, isDark); - } - - if (oobeWindow != null) - { - var hWnd = WinRT.Interop.WindowNative.GetWindowHandle(oobeWindow); - ThemeHelpers.SetImmersiveDarkMode(hWnd, isDark); - } - - if (SelectedTheme() == ElementTheme.Default) - { - themeListener = new ThemeListener(); - themeListener.ThemeChanged += (_) => HandleThemeChange(); - } - else if (themeListener != null) - { - themeListener.Dispose(); - themeListener = null; - } - } - catch (Exception e) - { - if (!loggedImmersiveDarkException) - { - Logger.LogError($"HandleThemeChange exception. Please install .NET 4.", e); - loggedImmersiveDarkException = true; - } - } + return ThemeService.Theme == ElementTheme.Dark || (ThemeService.Theme == ElementTheme.Default && ThemeHelpers.GetAppTheme() == AppTheme.Dark); } public static int UpdateUIThemeMethod(string themeName) { - switch (themeName?.ToUpperInvariant()) - { - case "LIGHT": - // OobeShellPage.OobeShellHandler.RequestedTheme = ElementTheme.Light; - ShellPage.ShellHandler.RequestedTheme = ElementTheme.Light; - break; - case "DARK": - // OobeShellPage.OobeShellHandler.RequestedTheme = ElementTheme.Dark; - ShellPage.ShellHandler.RequestedTheme = ElementTheme.Dark; - break; - case "SYSTEM": - // OobeShellPage.OobeShellHandler.RequestedTheme = ElementTheme.Default; - ShellPage.ShellHandler.RequestedTheme = ElementTheme.Default; - break; - default: - Logger.LogError($"Unexpected theme name: {themeName}"); - break; - } - - HandleThemeChange(); return 0; } private static ISettingsUtils settingsUtils = new SettingsUtils(); + private static ThemeService themeService = new ThemeService(SettingsRepository.GetInstance(settingsUtils)); + + public static ThemeService ThemeService => themeService; private static MainWindow settingsWindow; private static OobeWindow oobeWindow; private static FlyoutWindow flyoutWindow; - private static ThemeListener themeListener; public static void ClearSettingsWindow() { diff --git a/src/settings-ui/Settings.UI/SettingsXAML/MainWindow.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/MainWindow.xaml.cs index b3dec9c601..c665e3a21a 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/MainWindow.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/MainWindow.xaml.cs @@ -27,6 +27,9 @@ namespace Microsoft.PowerToys.Settings.UI var bootTime = new System.Diagnostics.Stopwatch(); bootTime.Start(); + App.ThemeService.ThemeChanged += OnThemeChanged; + App.ThemeService.ApplyTheme(); + ShellPage.SetElevationStatus(App.IsElevated); ShellPage.SetIsUserAnAdmin(App.IsUserAnAdmin); @@ -36,7 +39,7 @@ namespace Microsoft.PowerToys.Settings.UI AppWindow appWindow = AppWindow.GetFromWindowId(windowId); appWindow.SetIcon("Assets\\Settings\\icon.ico"); - var placement = Utils.DeserializePlacementOrDefault(hWnd); + var placement = WindowHelper.DeserializePlacementOrDefault(hWnd); if (createHidden) { placement.ShowCmd = NativeMethods.SW_HIDE; @@ -202,7 +205,7 @@ namespace Microsoft.PowerToys.Settings.UI private void Window_Closed(object sender, WindowEventArgs args) { var hWnd = WinRT.Interop.WindowNative.GetWindowHandle(this); - Utils.SerializePlacement(hWnd); + WindowHelper.SerializePlacement(hWnd); if (App.GetOobeWindow() == null) { @@ -213,6 +216,8 @@ namespace Microsoft.PowerToys.Settings.UI args.Handled = true; NativeMethods.ShowWindow(hWnd, NativeMethods.SW_HIDE); } + + App.ThemeService.ThemeChanged -= OnThemeChanged; } private void Window_Activated(object sender, WindowActivatedEventArgs args) @@ -221,11 +226,16 @@ namespace Microsoft.PowerToys.Settings.UI { this.Activated -= Window_Activated; var hWnd = WinRT.Interop.WindowNative.GetWindowHandle(this); - var placement = Utils.DeserializePlacementOrDefault(hWnd); + var placement = WindowHelper.DeserializePlacementOrDefault(hWnd); NativeMethods.SetWindowPlacement(hWnd, ref placement); } } + private void OnThemeChanged(object sender, ElementTheme theme) + { + WindowHelper.SetTheme(this, theme); + } + internal void EnsurePageIsSelected() { ShellPage.EnsurePageIsSelected(); diff --git a/src/settings-ui/Settings.UI/SettingsXAML/OobeWindow.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/OobeWindow.xaml.cs index 096a76b8a1..ff18d65917 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/OobeWindow.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/OobeWindow.xaml.cs @@ -4,7 +4,6 @@ using System; using interop; -using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Helpers; using Microsoft.PowerToys.Settings.UI.OOBE.Enums; using Microsoft.PowerToys.Settings.UI.OOBE.Views; @@ -36,6 +35,9 @@ namespace Microsoft.PowerToys.Settings.UI public OobeWindow(PowerToysModules initialModule) { + App.ThemeService.ThemeChanged += OnThemeChanged; + App.ThemeService.ApplyTheme(); + this.InitializeComponent(); // Set window icon @@ -133,6 +135,13 @@ namespace Microsoft.PowerToys.Settings.UI { mainWindow.CloseHiddenWindow(); } + + App.ThemeService.ThemeChanged -= OnThemeChanged; + } + + private void OnThemeChanged(object sender, ElementTheme theme) + { + WindowHelper.SetTheme(this, theme); } private void Dispose(bool disposing) diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/GeneralPage.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/Views/GeneralPage.xaml.cs index e1d345b616..ed29e2e9f6 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/GeneralPage.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/GeneralPage.xaml.cs @@ -8,11 +8,9 @@ using System.Threading.Tasks; using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Helpers; using Microsoft.PowerToys.Settings.UI.Library; -using Microsoft.PowerToys.Settings.UI.OOBE.Views; using Microsoft.PowerToys.Settings.UI.ViewModels; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; -using Windows.Storage.Pickers; namespace Microsoft.PowerToys.Settings.UI.Views { @@ -73,7 +71,6 @@ namespace Microsoft.PowerToys.Settings.UI.Views loader.GetString("GeneralSettings_RunningAsUserText"), ShellPage.IsElevated, ShellPage.IsUserAnAdmin, - App.UpdateUIThemeMethod, ShellPage.SendDefaultIPCMessage, ShellPage.SendRestartAdminIPCMessage, ShellPage.SendCheckForUpdatesIPCMessage, diff --git a/src/settings-ui/Settings.UI/ViewModels/DashboardViewModel.cs b/src/settings-ui/Settings.UI/ViewModels/DashboardViewModel.cs index cd489b4230..b2d6d226f0 100644 --- a/src/settings-ui/Settings.UI/ViewModels/DashboardViewModel.cs +++ b/src/settings-ui/Settings.UI/ViewModels/DashboardViewModel.cs @@ -64,8 +64,6 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels UpdatingSettings updatingSettingsConfig = UpdatingSettings.LoadSettings(); UpdateAvailable = updatingSettingsConfig != null && (updatingSettingsConfig.State == UpdatingSettings.UpdatingState.ReadyToInstall || updatingSettingsConfig.State == UpdatingSettings.UpdatingState.ReadyToDownload); - - App.UpdateUIThemeMethod(generalSettingsConfig.Theme); } private void AddDashboardListItem(ModuleType moduleType) diff --git a/src/settings-ui/Settings.UI/ViewModels/GeneralViewModel.cs b/src/settings-ui/Settings.UI/ViewModels/GeneralViewModel.cs index 6782423db2..8d55056f64 100644 --- a/src/settings-ui/Settings.UI/ViewModels/GeneralViewModel.cs +++ b/src/settings-ui/Settings.UI/ViewModels/GeneralViewModel.cs @@ -7,7 +7,6 @@ using System.Diagnostics; using System.Globalization; using System.IO; using System.IO.Abstractions; -using System.Net.NetworkInformation; using System.Reflection; using System.Runtime.CompilerServices; using System.Text.Json; @@ -48,8 +47,6 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels public ButtonClickCommand UpdateNowButtonEventHandler { get; set; } - public Func UpdateUIThemeCallBack { get; } - public Func SendConfigMSG { get; } public Func SendRestartAsAdminConfigMSG { get; } @@ -68,7 +65,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels private SettingsBackupAndRestoreUtils settingsBackupAndRestoreUtils = SettingsBackupAndRestoreUtils.Instance; - public GeneralViewModel(ISettingsRepository settingsRepository, string runAsAdminText, string runAsUserText, bool isElevated, bool isAdmin, Func updateTheme, Func ipcMSGCallBackFunc, Func ipcMSGRestartAsAdminMSGCallBackFunc, Func ipcMSGCheckForUpdatesCallBackFunc, string configFileSubfolder = "", Action dispatcherAction = null, Action hideBackupAndRestoreMessageAreaAction = null, Action doBackupAndRestoreDryRun = null, Func> pickSingleFolderDialog = null, object resourceLoader = null) + public GeneralViewModel(ISettingsRepository settingsRepository, string runAsAdminText, string runAsUserText, bool isElevated, bool isAdmin, Func ipcMSGCallBackFunc, Func ipcMSGRestartAsAdminMSGCallBackFunc, Func ipcMSGCheckForUpdatesCallBackFunc, string configFileSubfolder = "", Action dispatcherAction = null, Action hideBackupAndRestoreMessageAreaAction = null, Action doBackupAndRestoreDryRun = null, Func> pickSingleFolderDialog = null, object resourceLoader = null) { CheckForUpdatesEventHandler = new ButtonClickCommand(CheckForUpdatesClick); RestartElevatedButtonEventHandler = new ButtonClickCommand(RestartElevated); @@ -97,9 +94,6 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels SendCheckForUpdatesConfigMSG = ipcMSGCheckForUpdatesCallBackFunc; SendRestartAsAdminConfigMSG = ipcMSGRestartAsAdminMSGCallBackFunc; - // set the callback function value to update the UI theme. - UpdateUIThemeCallBack = updateTheme; - // Update Settings file folder: _settingsConfigFileFolder = configFileSubfolder; @@ -440,14 +434,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels _themeIndex = value; - try - { - UpdateUIThemeCallBack(GeneralSettingsConfig.Theme); - } - catch (Exception e) - { - Logger.LogError("Exception encountered when changing Settings theme", e); - } + App.ThemeService.ApplyTheme(); NotifyPropertyChanged(); }