fix(windows): unify standard window title bar theming#87
fix(windows): unify standard window title bar theming#87debugtheworldbot merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces window title bar theme synchronization across multiple windows and adds dynamic corner radius adjustments for the StatsPopupWindow. The review identifies opportunities to reduce code duplication by extracting theme logic into a base class and recommends direct access to named XAML elements instead of using FindName.
| Loaded += OnLoaded; | ||
| Closed += OnClosed; | ||
| ThemeManager.Instance.ThemeChanged += OnThemeChanged; | ||
| } | ||
|
|
||
| private void OnLoaded(object sender, RoutedEventArgs e) | ||
| { | ||
| ApplyWindowTitleBarTheme(); | ||
| } | ||
|
|
||
| private void OnClosed(object? sender, EventArgs e) | ||
| { | ||
| ThemeManager.Instance.ThemeChanged -= OnThemeChanged; | ||
| } | ||
|
|
||
| private void OnThemeChanged() | ||
| { | ||
| Dispatcher.BeginInvoke(new Action(ApplyWindowTitleBarTheme)); | ||
| } | ||
|
|
||
| private void ApplyWindowTitleBarTheme() | ||
| { | ||
| var handle = new WindowInteropHelper(this).Handle; | ||
| NativeInterop.TrySetImmersiveDarkMode(handle, ThemeManager.Instance.IsDarkTheme); | ||
| } |
There was a problem hiding this comment.
This block of code for handling title bar theming is duplicated across MouseCalibrationWindow, NotificationSettingsWindow, and SettingsWindow. To improve maintainability and reduce code duplication, consider extracting this logic into a common base class.
For example, you could create a ThemedWindow base class that inherits from Window and contains this logic:
// In a new file, e.g., Views/ThemedWindow.cs
public class ThemedWindow : Window
{
public ThemedWindow()
{
Loaded += OnLoaded;
Closed += OnClosed;
ThemeManager.Instance.ThemeChanged += OnThemeChanged;
}
private void OnLoaded(object sender, RoutedEventArgs e)
{
ApplyWindowTitleBarTheme();
}
private void OnClosed(object? sender, EventArgs e)
{
ThemeManager.Instance.ThemeChanged -= OnThemeChanged;
}
private void OnThemeChanged()
{
Dispatcher.BeginInvoke(new Action(ApplyWindowTitleBarTheme));
}
private void ApplyWindowTitleBarTheme()
{
var handle = new WindowInteropHelper(this).Handle;
NativeInterop.TrySetImmersiveDarkMode(handle, ThemeManager.Instance.IsDarkTheme);
}
}Then, you could change MouseCalibrationWindow, NotificationSettingsWindow, and SettingsWindow to inherit from ThemedWindow instead of Window. This would also require updating the root element in their respective XAML files (e.g., <local:ThemedWindow ...>). This would centralize the theme handling logic, making it easier to maintain.
| if (FindName("RootBorder") is System.Windows.Controls.Border rootBorder) | ||
| { | ||
| rootBorder.CornerRadius = _isWindowMode ? new CornerRadius(0) : new CornerRadius(8); | ||
| } |
There was a problem hiding this comment.
Since RootBorder is defined with x:Name in the corresponding XAML file, the XAML compiler will generate a field for it in this partial class. You can access it directly instead of using FindName(), which is more efficient and results in cleaner code.
RootBorder.CornerRadius = _isWindowMode ? new CornerRadius(0) : new CornerRadius(8);
Summary