-
Notifications
You must be signed in to change notification settings - Fork 8.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make the Pane inactive border respect the theme.window.applicationTheme
#14486
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice-n-easy
@@ -4175,6 +4175,22 @@ namespace winrt::TerminalApp::implementation | |||
const auto theme = _settings.GlobalSettings().CurrentTheme(); | |||
auto requestedTheme{ theme.RequestedTheme() }; | |||
|
|||
{ | |||
// Update the brushes that Pane's use... | |||
Pane::SetupResources(requestedTheme); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scary, updating globals like that, but I think I get how it is safe!
Co-authored-by: Carlos Zamora <[email protected]>
Hello @zadjii-msft! Because this pull request has the 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 (
|
🎉 Handy links: |
We couldn't do this before, because
App::Current().Resources().Lookup()
would always return the OS theme version of a resource. That thread has lengthy details on why.FORTUNATELY FOR US, this isn't the first time we've dealt with this. We've come up with a workaround in the past, and we can just use it again here.
Closes #3917
This will also make #3061 easy 😉