Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/cascadia/TerminalSettingsEditor/MainPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,28 @@ using namespace winrt::Windows::Foundation::Collections;

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
// GH#19927 - Walk the visual tree of the given element to find any
// Popups and set the theme on their Child element. In XAML Islands,
// popup children render in the PopupRoot (separate from the content
// tree), so they don't inherit our page's RequestedTheme
static void _setThemeOnPopups(const winrt::Windows::UI::Xaml::DependencyObject& element,
const winrt::Windows::UI::Xaml::ElementTheme theme)
{
const auto childCount = winrt::Windows::UI::Xaml::Media::VisualTreeHelper::GetChildrenCount(element);
for (int32_t i = 0; i < childCount; i++)
{
const auto child = winrt::Windows::UI::Xaml::Media::VisualTreeHelper::GetChild(element, i);
if (const auto popup = child.try_as<winrt::Windows::UI::Xaml::Controls::Primitives::Popup>())
{
if (const auto popupChild = popup.Child().try_as<winrt::Windows::UI::Xaml::FrameworkElement>())
{
popupChild.RequestedTheme(theme);
}
}
_setThemeOnPopups(child, theme);
}
}

static WUX::Controls::FontIcon _fontIconForNavTag(const std::wstring_view navTag)
{
WUX::Controls::FontIcon icon{};
Expand Down Expand Up @@ -350,6 +372,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_Navigate(tag, BreadcrumbSubPage::None);
}
}

// GH#19927 - Theme the search box's internal popup now that the
// visual tree is ready and control templates are applied
const auto& theme = _settingsSource.GlobalSettings().CurrentTheme();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(is it guaranteed that there is a theme?)

const bool hasThemeForSettings{ theme.Settings() != nullptr };
const auto& requestedTheme = hasThemeForSettings ? theme.Settings().RequestedTheme() : theme.RequestedTheme();
Comment on lines +378 to +380
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const auto& theme = _settingsSource.GlobalSettings().CurrentTheme();
const bool hasThemeForSettings{ theme.Settings() != nullptr };
const auto& requestedTheme = hasThemeForSettings ? theme.Settings().RequestedTheme() : theme.RequestedTheme();
const auto theme = _settingsSource.GlobalSettings().CurrentTheme();
const auto hasThemeForSettings{ theme.Settings() != nullptr };
const auto requestedTheme = hasThemeForSettings ? theme.Settings().RequestedTheme() : theme.RequestedTheme();

_setThemeOnPopups(SettingsSearchBox(), requestedTheme);
}

// Function Description:
Expand Down Expand Up @@ -1105,6 +1134,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

RequestedTheme(requestedTheme);

// GH#19927 - Theme the search box's internal popup so its
// dropdown matches the app theme instead of the system theme
_setThemeOnPopups(SettingsSearchBox(), requestedTheme);

// Mica gets it's appearance from the app's theme, not necessarily the
// Page's theme. In the case of dark app, light settings, mica will be a
// dark color, and the text will also be dark, making the UI _very_ hard
Expand Down
Loading