microsoft / TemplateStudio

Template Studio accelerates the creation of new WinUI 3, WPF, and UWP apps using a wizard-based experience.
Other
2.7k stars 461 forks source link

Dark mode titlebar #4628

Closed Piguelo closed 1 year ago

Piguelo commented 1 year ago

Describe the bug

  1. Titlebar caption color remains black in dark mode.
  2. Titlebar buttons remains black after minimize and restore app in dark mode.
  3. Selected NavigationViewItem text color remains black after start the app in dark mode when Content property is binded.

To Reproduce

  1. Start a new project

    Project type: Navigation Pane

    Design pattern: MVVM Toolkit

    Pages: Blank Settings

    Features: Settings Storage MSIX Packaging Theme Selection

    1. Run it and switch to dark mode.
    2. Minimize and restore to see bug N°2.
    3. NavigationViewItem bug:

ShellPage.xaml

            <NavigationView.MenuItems>
                <NavigationViewItem
                    Content="{x:Bind ViewModel.MainMenuTitle}"
                    helpers:NavigationHelper.NavigateTo="testTS53.ViewModels.MainViewModel">
                    <NavigationViewItem.Icon>
                        <FontIcon
                            FontFamily="{StaticResource SymbolThemeFontFamily}"
                            Glyph="&#xe7c3;"/>
                    </NavigationViewItem.Icon>
                </NavigationViewItem>
            </NavigationView.MenuItems>

ShellViewModel.cs

    private string _mainMenuTitle = "Binded Title";
    public string MainMenuTitle
    {
        get => _mainMenuTitle;
        set => SetProperty(ref _mainMenuTitle, value);
    }

Additional context

Bug 1: Titlebar caption color _bug_1_title_caption

Bug 2: Titlebar buttons _bug_2_title_buttons

Bug 3: NavigationViewItem text color _bug_3_menuitem

Applies to the following platforms:

About your setup

scorpion3399 commented 1 year ago

Hello, I have also experienced the first 2 bugs. However for me just switching the Theme from Light to Default (and the other way around) produces the bug. Maybe the TitleBarHelper Class, that is going to be removed at some point as MS says, has the issue?

Edit: Visual Studio Version: 17.4.5 Template Studio Wizard Version: 5.1 (The App was built with this version) Windows Build: 10.0.19041.0

If there is any other information you would like feel free to ask

Piguelo commented 1 year ago

Hello, I have also experienced the first 2 bugs. However for me just switching the Theme from Light to Default (and the other way around) produces the bug. Maybe the TitleBarHelper Class, that is going to be removed at some point as MS says, has the issue?

Edit: Visual Studio Version: 17.4.5 Template Studio Wizard Version: 5.1 (The App was built with this version) Windows Build: 10.0.19041.0

If there is any other information you would like feel free to ask

there is a workaround for v5.2

    public static void UpdateTitleBar(ElementTheme theme)
    {
        if (App.MainWindow.ExtendsContentIntoTitleBar)
        {
            if (theme == ElementTheme.Default)
            {
                theme = Application.Current.RequestedTheme == ApplicationTheme.Light ? ElementTheme.Light : ElementTheme.Dark;
            }

            Application.Current.Resources["WindowCaptionForeground"] = theme switch
            {
                ElementTheme.Dark => new SolidColorBrush(Colors.White),
                ElementTheme.Light => new SolidColorBrush(Colors.Black),
                _ => new SolidColorBrush(Colors.Transparent)
            };

            Application.Current.Resources["WindowCaptionForegroundDisabled"] = theme switch
            {
                ElementTheme.Dark => new SolidColorBrush(Color.FromArgb(0x66, 0xFF, 0xFF, 0xFF)),
                ElementTheme.Light => new SolidColorBrush(Color.FromArgb(0x66, 0x00, 0x00, 0x00)),
                _ => new SolidColorBrush(Colors.Transparent)
            };

            Application.Current.Resources["WindowCaptionButtonBackgroundPointerOver"] = theme switch
            {
                ElementTheme.Dark => new SolidColorBrush(Color.FromArgb(0x33, 0xFF, 0xFF, 0xFF)),
                ElementTheme.Light => new SolidColorBrush(Color.FromArgb(0x33, 0x00, 0x00, 0x00)),
                _ => new SolidColorBrush(Colors.Transparent)
            };

            Application.Current.Resources["WindowCaptionButtonBackgroundPressed"] = theme switch
            {
                ElementTheme.Dark => new SolidColorBrush(Color.FromArgb(0x66, 0xFF, 0xFF, 0xFF)),
                ElementTheme.Light => new SolidColorBrush(Color.FromArgb(0x66, 0x00, 0x00, 0x00)),
                _ => new SolidColorBrush(Colors.Transparent)
            };

            Application.Current.Resources["WindowCaptionButtonStrokePointerOver"] = theme switch
            {
                ElementTheme.Dark => new SolidColorBrush(Colors.White),
                ElementTheme.Light => new SolidColorBrush(Colors.Black),
                _ => new SolidColorBrush(Colors.Transparent)
            };

            Application.Current.Resources["WindowCaptionButtonStrokePressed"] = theme switch
            {
                ElementTheme.Dark => new SolidColorBrush(Colors.White),
                ElementTheme.Light => new SolidColorBrush(Colors.Black),
                _ => new SolidColorBrush(Colors.Transparent)
            };

            Application.Current.Resources["WindowCaptionBackground"] = new SolidColorBrush(Colors.Transparent);
            Application.Current.Resources["WindowCaptionBackgroundDisabled"] = new SolidColorBrush(Colors.Transparent);

            var hwnd = WinRT.Interop.WindowNative.GetWindowHandle(App.MainWindow);
            if (hwnd == GetActiveWindow())
            {
                SendMessage(hwnd, WMACTIVATE, WAINACTIVE, IntPtr.Zero);
                SendMessage(hwnd, WMACTIVATE, WAACTIVE, IntPtr.Zero);
            }
            else
            {
                SendMessage(hwnd, WMACTIVATE, WAACTIVE, IntPtr.Zero);
                SendMessage(hwnd, WMACTIVATE, WAINACTIVE, IntPtr.Zero);
            }
        }
    }

TitleBarHelper no longer exists in v5.3

mikebattista commented 1 year ago

@scorpion3399 so you're hitting the first 2 bugs using a project with code from the 5.1 extension? Does your project have the TitleBarHelper class?

5.3 removed the TitleBarHelper since the underlying bugs were fixed in the Windows App SDK.

Both you and @Piguelo are on Windows 10, though, which might be related. I can't repro on Windows 11.

scorpion3399 commented 1 year ago

Firstly thank you @mikebattista and for @Piguelo for helping me.

@Piguelo your solution works. Thank you.

@mikebattista YES the project is using code from the 5.1 extension and YES it has the TitleBarHelper class.

I did the following tests:

I also created a new Test project with the Settings Page using Template Studio 5.3 and Microsoft.WindowsAppSDK 1.2.230118.102 in Win10 (that does NOT contain TitleBarHelper) and the bug is fixed.

My last resource will be finding the differences in the MS created source code of version 5.1 (my App) and 5.3 (the Tes) that could have some connection with this bug (ThemeSelector, the Navigation and Activation files and possibly the ShellPage).

mikebattista commented 1 year ago

Thanks for the analysis to help narrow down the repro.

The release changelog is at https://github.com/microsoft/TemplateStudio/releases which can give you some clues as to what changed between releases.

pratikone commented 1 year ago

@scorpion3399 do check this PR out. You will have to delete code from similar places. https://github.com/microsoft/TemplateStudio/commit/c4b1b77b504a1ebafa780244906e4d547a16ef1f

you have to remove titlebar workaround from bunch of places like SetRequestedThemeAsync and OnLoaded and add WindowCaptionBackground as Transparent resource

scorpion3399 commented 1 year ago

Thanks @pratikone . My issue has been resolved.

mikebattista commented 1 year ago

So your app based now on the 5.3/WinAppSDK 1.2.3 title bar code, works on both Win10 and Win11?

scorpion3399 commented 1 year ago

Sorry for the late response. Yes it does. However it created another issue that is currently open. https://github.com/microsoft/TemplateStudio/issues/4630

mikebattista commented 1 year ago

Thanks. @Piguelo if you update to the latest versions is this issue resolved for you as well?

Piguelo commented 1 year ago

Thanks. @Piguelo if you update to the latest versions is this issue resolved for you as well?

What do you mean by "update to the latest versions"?

i did uninstall/install TemplateStudioForWinUICs.vsix. I tested it in a new project, but i see no changes, all bugs are still present.

danvim commented 1 year ago

I have the opposite issue. My title on a light theme stays white. My Win11's system theme is Dark.

EDIT: I think I can reproduce this issue reliably and believe the title color depends on the system theme on app startup. I'm using extension version 5.3.

  1. Set system color to Dark.
  2. Debug a newly generated Template Studios WinUI app.
  3. Notice the title color always stays white no matter the app selected theme.
  4. Change system color to Light.
  5. Reopen app and notice the title color stays black.
mikebattista commented 1 year ago

@pratikone will need to take a look when he can as he owns the custom title bar platform APIs.

I've tried reproing these titlebar issues on a couple different Windows versions but haven't been able to repro myself. I believe the issues exist, but I haven't been able to find the repro condition.

@pratikone if you can repro these we'll need to consider additional platform fixes and/or workarounds in the templates. Or worst case, we should consider bringing back the TitleBarHelper until we can pinpoint the issues.

danvim commented 1 year ago

I think I found the problem.

private void MainWindow_Activated(object sender, WindowActivatedEventArgs args)
{
    var resource = args.WindowActivationState == WindowActivationState.Deactivated ? "WindowCaptionForegroundDisabled" : "WindowCaptionForeground";

    AppTitleBarText.Foreground = (SolidColorBrush)App.Current.Resources[resource];
}

On debug, App.Current.RequestedTheme is always the same as Windows's system theme. I tried setting this, but it cannot be set at runtime.

image

image

I can't find where WindowCaptionForeground is defined, so it I'm assuming it's something native. It's only logical that App.Current.Resources is themed after App.Current.RequestedTheme. Since App.Current.RequestedTheme is always the system theme, it doesn't make sense for the title bar to be themed properly using the generated code.

A workaround is simply removing AppTitleBarText.Foreground = (SolidColorBrush)App.Current.Resources[resource]; so it can be themed properly.

danvim commented 1 year ago

https://github.com/microsoft/TemplateStudio/issues/4630#issuecomment-1451504834 Fixed it for me

Piguelo commented 1 year ago

#4630 (comment) Fixed it for me

it does fix bug 1 and 2 for me.

i'm still trying to find a workaround for NavigationViewItem binded title issue.

https://user-images.githubusercontent.com/91241030/227730693-894b1f20-5198-4b14-847c-23b003a5bb7a.mp4

KiyanYang commented 1 year ago

I find a workaround for NavigationViewItem binded title issue. I don’t know the specific reason, just obtained by trying.

In Views/ShellPage.xaml.cs, add eventHandler to NavigationViewControl.Loaded event, like this

public sealed partial class ShellPage : Page
{
+   public event RoutedEventHandler NavigationViewLoaded;

    public ShellPage(ShellViewModel viewModel)
    {
        ...

        // Add this line
+       NavigationViewControl.Loaded += (sender, e) => NavigationViewLoaded?.Invoke(sender, e);
    }

    ...
}

In Activation/DefaultActivationHandler.cs, change HandleInternalAsync method to

protected async override Task HandleInternalAsync(LaunchActivatedEventArgs args)
{
    if (App.MainWindow.Content is ShellPage shellPage)
    {
        shellPage.NavigationViewLoaded += (sender, e) =>
        {
            _navigationService.NavigateTo(typeof(MainViewModel).FullName!, args.Arguments);
        };
    }

    await Task.CompletedTask;
}
Piguelo commented 1 year ago

it works, thank you @KiyanYang

Mysterious-Dev commented 1 year ago

With recent merged PR, this issue can be closed ?

pratikone commented 1 year ago

this issue has been fixed by #4638 as well as #4666. Will be available in next release.