Skip to content

Make the Pane inactive border respect the theme.window.applicationTheme#14486

Merged
2 commits merged intomainfrom
dev/migrie/b/3917-finally
Dec 9, 2022
Merged

Make the Pane inactive border respect the theme.window.applicationTheme#14486
2 commits merged intomainfrom
dev/migrie/b/3917-finally

Conversation

@zadjii-msft
Copy link
Member

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 😉

@ghost ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Dec 2, 2022
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Nice-n-easy

@carlos-zamora carlos-zamora added the Needs-Second It's a PR that needs another sign-off label Dec 9, 2022
@ghost ghost requested review from DHowett, PankajBhojwani and lhecker December 9, 2022 18:45

{
// Update the brushes that Pane's use...
Pane::SetupResources(requestedTheme);
Copy link
Member

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 <carlos.zamora@microsoft.com>
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Dec 9, 2022
@ghost
Copy link

ghost commented Dec 9, 2022

Hello @zadjii-msft!

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.

@ghost ghost merged commit 84bb98b into main Dec 9, 2022
@ghost ghost deleted the dev/migrie/b/3917-finally branch December 9, 2022 21:26
@ghost
Copy link

ghost commented Jan 24, 2023

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

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-UserInterface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Second It's a PR that needs another sign-off Priority-3 A description (P3) Product-Terminal The new Windows Terminal. zBugBash-Consider

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panel split background ignores requestedTheme, follows system app mode

3 participants