Update the shape of our custom NewTab button to match MUX's TabView button #6766
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DHowett
reviewed
Jul 2, 2020
DHowett
approved these changes
Jul 2, 2020
zadjii-msft
approved these changes
Jul 6, 2020
zadjii-msft
added
Area-UserInterface
|
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 (
|
ghost
merged commit 
1 task
DHowett
pushed a commit
that referenced
this pull request
Jul 7, 2020
Update colors of our custom NewTab button to match MUX's TabView button MUX has a NewTab button, but Terminal uses a homemade lookalike. The version in Terminal doesn't use the same brush color resources as MUX's button, so it looks very slightly different. This PR updates Terminal's button to use the exact same colors that MUX uses. I literally copied these brush names out of MUX source code. ## References This is the color version of the layout fix #6766 This is a prerequisite for fixing #5360 ## Detailed Description of the Pull Request / Additional comments The real reason that this matters is that once you flip on `ApplicationHighContrastAdjustment::None`, the existing colors will not work at all. The existing brushes are themed to black foreground on a black background when High Contrast (HC) Black theme is enabled. The only thing that's saving you is `ApplicationHighContrastAdjustment::Auto` is automatically backplating the glyphs on the buttons, which (by design) hides the fact that the colors are poor. The backplates are those ugly squares inside the buttons on the HC themes. Before I can push a PR that disables automatic backplating (set `ApplicationHighContrastAdjustment` to `None`), we'll need to select better brushes that work in HC mode. MUX has already selected brushes that work great in all modes, so it just makes sense to use their brushes. The one very subtle difference here is that, for non-HC themes, the glyph's foreground has a bit more contrast when the button is in hovered/pressed states. Again this slight difference hardly matters now, but using the correct brushes will become critical when we try to remove the HC backplating. Closes #6812
DHowett
pushed a commit
that referenced
this pull request
Jul 17, 2020
…utton (#6766) The MUX TabView control has a uniquely-shaped [+] button. TerminalApp doesn't use it: instead, it has a SplitView button that is styled to look like MUX's official button. However, it doesn't get the button's shape right. This PR updates TerminalApp's custom button to look more like MUX's. The difference is that MUX only rounds the top two corners, and it uses a bigger radius. Without matching MUX's radius, the upper-left corner of the button makes an awkward asymmetric divot with the abutting tab. There's also a spot in the lower-left corner that just looks like someone accidentally spilled a few pixels on the floor. Current appearance before this PR:  New appearance with this PR:  Most important deltas highlighted with red circles:  Note that this PR does *not* attempt to fix the colors. The colors are also just slightly different from what MUX uses. I'll save that for a separate PR, since all those screenshots would clutter this up this PR. (cherry picked from commit d350a89)
DHowett
pushed a commit
that referenced
this pull request
Jul 17, 2020
Update colors of our custom NewTab button to match MUX's TabView button MUX has a NewTab button, but Terminal uses a homemade lookalike. The version in Terminal doesn't use the same brush color resources as MUX's button, so it looks very slightly different. This PR updates Terminal's button to use the exact same colors that MUX uses. I literally copied these brush names out of MUX source code. ## References This is the color version of the layout fix #6766 This is a prerequisite for fixing #5360 ## Detailed Description of the Pull Request / Additional comments The real reason that this matters is that once you flip on `ApplicationHighContrastAdjustment::None`, the existing colors will not work at all. The existing brushes are themed to black foreground on a black background when High Contrast (HC) Black theme is enabled. The only thing that's saving you is `ApplicationHighContrastAdjustment::Auto` is automatically backplating the glyphs on the buttons, which (by design) hides the fact that the colors are poor. The backplates are those ugly squares inside the buttons on the HC themes. Before I can push a PR that disables automatic backplating (set `ApplicationHighContrastAdjustment` to `None`), we'll need to select better brushes that work in HC mode. MUX has already selected brushes that work great in all modes, so it just makes sense to use their brushes. The one very subtle difference here is that, for non-HC themes, the glyph's foreground has a bit more contrast when the button is in hovered/pressed states. Again this slight difference hardly matters now, but using the correct brushes will become critical when we try to remove the HC backplating. Closes #6812 (cherry picked from commit 4faa104)
|
🎉This issue was addressed in #6812, which has now been successfully released as Handy links: |
|
🎉 Handy links: |
|
🎉This issue was addressed in #6812, which has now been successfully released as Handy links: |
|
🎉 Handy links: |
|
🎉This issue was addressed in #6812, which has now been successfully released as Handy links: |
|
🎉 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The MUX TabView control has a uniquely-shaped [+] button. TerminalApp
doesn't use it: instead, it has a SplitView button that is styled to
look like MUX's official button. However, it doesn't get the button's
shape right. This PR updates TerminalApp's custom button to look more
like MUX's.
The difference is that MUX only rounds the top two corners, and it uses
a bigger radius. Without matching MUX's radius, the upper-left corner
of the button makes an awkward asymmetric divot with the abutting tab.
There's also a spot in the lower-left corner that just looks like
someone accidentally spilled a few pixels on the floor.
Current appearance before this PR:
New appearance with this PR:
Most important deltas highlighted with red circles:
Note that this PR does not attempt to fix the colors. The colors are
also just slightly different from what MUX uses. I'll save that for a
separate PR, since all those screenshots would clutter this up this PR.