Use ThemeLookup for the SUI bg too

[zadjii-msft]

I can't exactly repro #14559. I suspect that's due to #14567 having been merged. This, however, seemed related. Without this, we'll use the App's RequestedTheme (the one that can't be changed at runtime), rather than the user's requestedTheme. That will do weird things, like make the BG of the SUI dark, with white expanders.

I think this should close #14559.

[zadjii-msft]

@carlos-zamora can you check this out and make sure #14559 doesn't repro anymore?

[carlos-zamora]

Issue no longer repros. Thanks!

I did notice this though:

I forget if this one's been around for a while, or if the fix is to apply the theme manually to the entire visual tree.

Note: OS theme is dark

[zadjii-msft]

or if the fix is to apply the theme manually to the entire visual tree

probably that.... yea. Uhg. I'll take another spin through that and find the popup root.

[ghost]

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[zadjii-msft]

I'll take another spin through that and find the popup root

This might be Hard to do generically. Consider the code we use for the popups:

        auto themingLambda{ [this](const Windows::Foundation::IInspectable& sender, const RoutedEventArgs&) {
            auto theme{ _settings.GlobalSettings().CurrentTheme() };
            auto requestedTheme{ theme.RequestedTheme() };
            auto element{ sender.try_as<winrt::Windows::UI::Xaml::FrameworkElement>() };
            while (element)
            {
                element.RequestedTheme(requestedTheme);
                element = element.Parent().try_as<winrt::Windows::UI::Xaml::FrameworkElement>();
            }
        } };

Popups belong to a different XAML root than the rest of the app. So we can go up the tree from the SUI and apply the requested theme to that whole tree, but we'll never get up to the popup root from the SUI. We'd need a literal popup to get there, and that we won't have access to, till it's opened.

I'm inclined to resolve that sub-point as "meh". There may be a way to get to it, but "meh".

[carlos-zamora]

I'll take another spin through that and find the popup root

This might be Hard to do generically. Consider the code we use for the popups:

        auto themingLambda{ [this](const Windows::Foundation::IInspectable& sender, const RoutedEventArgs&) {
            auto theme{ _settings.GlobalSettings().CurrentTheme() };
            auto requestedTheme{ theme.RequestedTheme() };
            auto element{ sender.try_as<winrt::Windows::UI::Xaml::FrameworkElement>() };
            while (element)
            {
                element.RequestedTheme(requestedTheme);
                element = element.Parent().try_as<winrt::Windows::UI::Xaml::FrameworkElement>();
            }
        } };

Popups belong to a different XAML root than the rest of the app. So we can go up the tree from the SUI and apply the requested theme to that whole tree, but we'll never get up to the popup root from the SUI. We'd need a literal popup to get there, and that we won't have access to, till it's opened.

I'm inclined to resolve that sub-point as "meh". There may be a way to get to it, but "meh".

Could it be a part of the SettingContainer? Maybe in OnApplyTemplate()?

Regardless, I'm ok with shipping this as-is. But we should have a bug tracking it imo :/

[DHowett]

Could it be a part of the SettingContainer? Maybe in OnApplyTemplate()?

There's no popup when the template is applied; it only exists when the user interacts. I think this is a very deep well and a big waste of engineering effort to fix honestly. XAML has let us down. :)

[carlos-zamora]

Could it be a part of the SettingContainer? Maybe in OnApplyTemplate()?

There's no popup when the template is applied; it only exists when the user interacts. I think this is a very deep well and a big waste of engineering effort to fix honestly. XAML has let us down. :)

Oh right, I forgot Mike said that. Fair enough.

[ghost]

🎉Windows Terminal Preview v1.17.1023 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download