Skip to content
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

Fix a couple issues with experimental.useBackgroundImageForWindow #14456

Merged
4 commits merged into from
Nov 29, 2022

Conversation

zadjii-msft
Copy link
Member

This fixes two issues with experimental.useBackgroundImageForWindow I discovered while looking at #14260

  • It looks like the opacity of the whole-window BG image wouldn't hot reload if the path didn't.
  • set useBGForWindow:true, focus a pane with an image, then set it to useBGForWindow:false, and observe a pane with <100 opacity. You'll be able to see the BG image left behind!

These are pretty easy to miss, so I can see how it happened.

I don't think this technically closes that thread, though. Ultimately, I think OP's settings were just wrong (and possible didn't hot-reload). There's another, trickier bit I'm discussing in that thread, that might deserve its own separate follow-up for discussion.

@zadjii-msft zadjii-msft changed the title This is one of the bugs I found, the residual image after disabling one Fix a couple issues with experimental.useBackgroundImageForWindow Nov 28, 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.

Thanks for fixing these. Just be sure to fix the changes to .github before merging (assuming somebody else comes along and approves)

@@ -6,6 +6,7 @@ File | Purpose | Format | Info
[reject.txt](reject.txt) | Remove words from the dictionary (after allow) | grep pattern matching whole dictionary words | [reject](https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples%3A-reject)
[excludes.txt](excludes.txt) | Files to ignore entirely | perl regular expression | [excludes](https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples%3A-excludes)
[patterns/*.txt](patterns/) | Patterns to ignore from checked lines | perl regular expression (order matters, first match wins) | [patterns](https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples%3A-patterns)
[candidate.patterns](candidate.patterns) | Patterns that might be worth adding to [patterns.txt](patterns.txt) | perl regular expression with optional comment block introductions (all matches will be suggested) | [candidates](https://github.com/check-spelling/check-spelling/wiki/Feature:-Suggest-patterns)
Copy link
Member

Choose a reason for hiding this comment

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

Sorry about this. You'll need to merge main to get "right" with this changeset again. I should have done the "merge base" trick I did for PowerToys... but with 330 branches on our repo it would have been quite hard.

Copy link
Member

Choose a reason for hiding this comment

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

I decided to just do it

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Nov 29, 2022
@ghost
Copy link

ghost commented Nov 29, 2022

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.

@ghost ghost merged commit f2eed92 into main Nov 29, 2022
@ghost ghost deleted the dev/migrie/b/14260-bgImageForWindow branch November 29, 2022 23:09
DHowett pushed a commit that referenced this pull request Dec 12, 2022
…14456)

This fixes two issues with `experimental.useBackgroundImageForWindow` I discovered while looking at #14260

* It looks like the opacity of the whole-window BG image wouldn't hot reload if the path didn't.
* > set useBGForWindow:true, focus a pane with an image, then set it to useBGForWindow:false, and observe a pane with <100 opacity. You'll be able to see the BG image left behind!

These are pretty easy to miss, so I can see how it happened.

I don't think this _technically_ closes that thread, though. Ultimately, I think OP's settings were just wrong (and possible didn't hot-reload). There's another, trickier bit I'm discussing in that thread, that might deserve its own separate follow-up for discussion.

(cherry picked from commit f2eed92)
Service-Card-Id: 87207792
Service-Version: 1.16
@ghost
Copy link

ghost commented Dec 14, 2022

🎉Windows Terminal Preview v1.16.3463.0 and v1.16.3464.0 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
AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants