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

UI is broken in system high contrast mode #5360

Closed
mimi89999 opened this issue Apr 15, 2020 · 20 comments · Fixed by #6833 or #6915
Closed

UI is broken in system high contrast mode #5360

mimi89999 opened this issue Apr 15, 2020 · 20 comments · Fixed by #6833 or #6915
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Tracking-External This bug isn't resolved, but it's following an external workitem.
Milestone

Comments

@mimi89999
Copy link

Environment

Windows build number: 10.0.19603.1000
Windows Terminal version (if applicable): 0.10.781.0

Steps to reproduce

Turn on high contrast mode.

Expected behavior

UI elements should become easier to distinguish.

Actual behavior

Terminal UI elements are hard to identify. It took me some time to find the + button because it looks like the Windows logo or the start menu button and not like a +. Dropdown button looks more like a checkbox than a dropdown button or arrow pointing down.

Screenshot_win10_2020-04-15_17:10:46

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 15, 2020
@zadjii-msft
Copy link
Member

Okay yea this is pretty bad. I would have thought that this might have to be fixed on the upstream TabView. However, looking at MUX#1663, that high-contrast tabview looks just fine.

@zadjii-msft zadjii-msft added Area-User Interface 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 Apr 15, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 15, 2020
@zadjii-msft
Copy link
Member

Wait now I'm reading that thread more, we might be able to get this by just updating to the latest MUX.

This comment: microsoft/microsoft-ui-xaml#1663 (comment) made me think that they might have just solved this for us 😆

@zadjii-msft zadjii-msft added this to the Terminal v1.x milestone Apr 15, 2020
@jtippet
Copy link
Contributor

jtippet commented Apr 15, 2020

@zadjii-msft - I just stopped by the Terminal github project to send you guys an email to ask you to update to the latest MUX, so I can start contributing some PRs to smooth out Terminal's HC experience. I promise mimi8999 is not in cahoots with me, and this issue getting filed today is a total coincidence ;)

XAML has a global "I understand high contrast" flag named ApplicationHighContrastAdjustment. For Terminal, it's currently in HC-ignorant mode, which means MUX is more aggressive about ignoring your colors and automatically backplating text. This is not wrong, but it will make it difficult to reach perfect beauty. Beyond updating MUX, the next best thing you can do is toggle that flag, then fix the issues that it was hiding (invisible dialog box buttons, menu text, wrong titlebar color, etc.)

I'm happy to contribute wherever you can use help -- the whole reason I fixed MUX's TabView was so I could subsequently make my terminals not ugly.

@mimi89999
Copy link
Author

@jtippet Were you able to build Terminal with latest MUX release? I wasn't.

@jtippet
Copy link
Contributor

jtippet commented Apr 15, 2020

I didn't attempt that. I've only prototyped the other half - cleaning up button labels and menu text.

@DHowett-MSFT DHowett-MSFT added Tracking-External This bug isn't resolved, but it's following an external workitem. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 17, 2020
@DHowett-MSFT
Copy link
Contributor

Triaging this for 1.x. If WinUI 2.4 lands before that, we may get it as part of a dependency updating pass.

@WSLUser
Copy link
Contributor

WSLUser commented Apr 21, 2020

Since I was looking into bumping deps, I did try updating to the preview release (we are actually on a 2.2. preview so going to 2.4 preview seems acceptable) but ran into problems attempting to build. I can update the other stuff but while debug mode is fine, the release mode has an issue compiling conpty in preview mode of VS 2019 and a problem finding the generated .lib in stable. Not sure what exactly is at play but my fork is available to test with. Also, you need to statically assign paths for instead of using wildcards in the build process. VS2019 won't accept the wildcards (building Terminal becomes even worse without letting VS statically assign the build paths).

@DHowett-MSFT
Copy link
Contributor

Thanks for looking into this! I’m a bit concerned. We are using 2.3, for what it’s worth, and even though it says “prerelease” it’s definitely the full retail payload. There’s a depressing reason for that... but in short, the “prerelease” builds put their DLLs inside our package, and the retail ones do not, and we need that copy of their DLL.

Which version of VS2019 are you seeing this with? The team has been building on 2019 since it came out, and hasn’t seen any trouble with the wildcards.

1 similar comment
@DHowett-MSFT
Copy link
Contributor

Thanks for looking into this! I’m a bit concerned. We are using 2.3, for what it’s worth, and even though it says “prerelease” it’s definitely the full retail payload. There’s a depressing reason for that... but in short, the “prerelease” builds put their DLLs inside our package, and the retail ones do not, and we need that copy of their DLL.

Which version of VS2019 are you seeing this with? The team has been building on 2019 since it came out, and hasn’t seen any trouble with the wildcards.

@jtippet
Copy link
Contributor

jtippet commented Jul 7, 2020

@DHowett-MSFT - may I request updating MUX again to 2.5.0-prerelease.200609001? That version contains a couple important fixes to unblock a fix for this issue:

With that version of MUX, Terminal is very close to being beautiful in all HC themes. Here's a peek at what it looks like with the newer MUX and a change to set Terminal to ApplicationHighContrastAdjustment::None:
image

Compare that to the screenshot at the top of this thread, and I think it's clearly an improvement. The menu is rather less embarrassing too:

Current 2.5.0 + AHCA::None
image image

As a bonus, by updating to 2.5.0, you'll also get rounded tab feet, which is catnip for tab nerds:
image

@mimi89999
Copy link
Author

Is there any progress on this issue? The latest preview from the Store still looks really bad

Screenshot_win10_2020-07-07_11:53:22

@mimi89999
Copy link
Author

Also, I noticed that some icons in the taskbar change to monochrome and others don't. Why?

@zadjii-msft
Copy link
Member

@jtippet absolutely lets update! Kinda funny that we only just got on 2.4 (#5778) a few days before the 2.5 prerelease came out 😃

@ghost ghost added the In-PR This issue has a related PR label Jul 7, 2020
@jtippet
Copy link
Contributor

jtippet commented Jul 7, 2020

Is there any progress on this issue? The latest preview from the Store still looks really bad

There's definitely been progress -- comparing your original screenshot with your current one, you can see that the tab titles look better. That's fix 1663 in MUX, which already shipped in the store version of Terminal. The menu will be improved a bit once #6819 lands, and then there's really only one more line of code to add to Terminal to make everything look like the screenshot I shared.

Also, I noticed that some icons in the taskbar change to monochrome and others don't. Why?

The OS never automatically generates monochrome icons from a full-color icon. It would rarely look great, and in some cases, would make an unreadable mess.

For the app launcher icons in the center of the taskbar, the shell relies on the app to provide a hand-crafted high contrast icon. But only UWP has the technology to designate an icon as being for high contrast -- classic win32 resources don't even have the concept. (Classic win32 can only tailor icons for display scale and color bitdepth, the latter of which is entirely obsolete, since nobody's rocking an EGA display anymore.) So a UWP app can display colorful icons when HC is disabled, while automatically switching to monochrome icons when HC is enabled. But a Win32 app has to choose one or the other, and can't adapt to the HC setting. That's why, in your screenshot, the File Explorer icon is not monochrome: File Explorer is a win32 app.

Furthermore, an app will only change its icon if the app developer's art department actually troubled themselves to provide a high contrast version of the icon. That's why, in your screenshot, Terminal does not have a monochrome icon: the Terminal art department did not provide one. Although if you have more pixel talents than I, perhaps the Terminal team would accept a contribution of beautiful new HC icons? See here.

Hopefully, as Project Reunion makes UWP technology more readily available, more developers will include high contrast icons with their apps.

@mimi89999
Copy link
Author

There's definitely been progress -- comparing your original screenshot with your current one, you can see that the tab titles look better. That's fix 1663 in MUX, which already shipped in the store version of Terminal. The menu will be improved a bit once #6819 lands, and then there's really only one more line of code to add to Terminal to make everything look like the screenshot I shared.

Ah. OK. I see that the tabs look good now, but the + and arrow down buttons still have a background. I thought that they also would be fixed since in the animation in microsoft/microsoft-ui-xaml#1663 (comment) those buttons also don't have the background anymore.

For the app launcher icons in the center of the taskbar, the shell relies on the app to provide a hand-crafted high contrast icon. But only UWP has the technology to designate an icon as being for high contrast...

Thanks for this long explanation. Hopefully Terminal will get a monochrome icon soon 😉
As for old Win32 apps, I saw that MS made a nonofficial API for dark theme especially for Explorer. AFAIK only Explorer and Recorder are using it. Somebody managed to enable it: https://github.com/ysc3839/win32-darkmode. Maybe they will do the same for icons? I don't see them anywhere close to rewriting old Win32 apps. Anyway, that's off topic here...

@jtippet
Copy link
Contributor

jtippet commented Jul 7, 2020

I thought that they also would be fixed since in the animation in microsoft/microsoft-ui-xaml#1663 (comment) those buttons also don't have the background anymore.

Yeah, I fixed the standard TabView [+] icon that ships with MUX. But Terminal doesn't use the standard icon. It builds its own from scratch, so it can have the dropdown menu. Don't worry though: I'm fixing Terminal's [+] button too: #6812

As for old Win32 apps, I saw that MS made a nonofficial API for dark theme especially for Explorer.

You can definitely write code to detect whether HC is enabled, and if so, query the background color. You don't even need undocumented APIs -- just query SPI_GETHIGHCONTRAST. So you can already write a win32 app that draws a beautiful icon in all modes. But the taskbar problem that you're seeing is different: the app isn't drawing the icons, so there's no amount of code you can add to the app to fix the problem. There are various OS surfaces (taskbar, start menu, alt+tab, task manager, ...) where the OS draws an icon on behalf of your app. In those cases, the OS needs to know which of your icons to use. UWP has a sophisticated metadata system, so for example you can describe an icon as being designed for 200% DPI + high contrast black theme + fr_CA locale. The OS will inventory all your icons, sift through the metadata, then select the best icon for the current situation. Compare that to Win32, where the icon can only talk about pixels and color depth -- there's simply no way for you to say that a particular icon is designed to look great in HC black theme.

@ghost ghost removed the In-PR This issue has a related PR label Jul 7, 2020
DHowett pushed a commit that referenced this issue 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
ghost pushed a commit that referenced this issue Jul 7, 2020
See: https://github.com/microsoft/microsoft-ui-xaml/releases/tag/v2.5.0-prerelease.200609001

> ### Notable Changes:
> 
>     Resize tab view items only once the pointer has left the TabViewItem strip (microsoft/microsoft-ui-xaml#2569)
>     Align TabView visuals with Edge (microsoft/microsoft-ui-xaml#2201)
>     Fix background of MenuFlyout in white high contrast (microsoft/microsoft-ui-xaml#2446)
>     TabView: Make TabViewItem consume the TabViewItemHeaderForeground theme resource (microsoft/microsoft-ui-xaml#2348)
>     TabView: Add tooltips to its scrolling buttons. (microsoft/microsoft-ui-xaml#2369)


* [x] Related to #5360 (@jtippet confirms that this alone does not close it.)
* [x] I work here
@ghost ghost added the In-PR This issue has a related PR label Jul 8, 2020
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements and removed In-PR This issue has a related PR labels Jul 8, 2020
DHowett pushed a commit that referenced this issue Jul 8, 2020
This PR enables `ApplicationHighContrastAdjustment::None`.  Doing this
disables a set of mitigations in XAML designed to band-aid apps that
were never explicitly designed for High Contrast (HC) modes.  Terminal
now has full control of and responsibility for its appearance in HC
mode.  This allows Terminal to look a lot better.

On paper, we should be able to set `HighContrastAdjustment="None"` on
the `<Application>` element.  But that doesn't have any effect.  I don't
know if this is a bug in `<Toolkit:XamlApplication>` or somewhere else.
So instead I set the property in codebehind, which is not as ideal, but
does at least work.  I'd love to a way to move this into App.xaml.

The Find box had a couple stray styles to override the ToggleButton's
foreground color.  With backplating removed, these styles became
actively harmful (white foreground on highlight color background), so I
just removed them.  The built-in style for ToggleButton is perfect
as-is.

Closes #5360
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jul 8, 2020
@ghost ghost added In-PR This issue has a related PR and removed Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. labels Jul 14, 2020
DHowett pushed a commit that referenced this issue Jul 14, 2020
This commit adds image assets for High Contrast mode

Tagging this issue so it contains a nice list of all the recent HC
fixes: #5360

I made several changes to DHowett's script and added it to the repo:
* Add support for generating high contrast icons
* Add the ability to easily edit the "intermediate" (previously "zbase")
  files for manual hinting
* Appease the spellchecker

I created new SVGs for HC mode. There's one SVG for both Black and White
modes -- I just invert the colors. Then I manually hinted the generated
bitmaps for the production icons. I didn't bother hinting the Dev/Pre
ones, so the text does get unreadable at small sizes.

View the original images in #6915.

Co-authored-by: Jeffrey Tippet <[email protected]>
Co-authored-by: Dustin L. Howett <[email protected]>
Closes #6822
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 14, 2020
DHowett pushed a commit that referenced this issue 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)
DHowett pushed a commit that referenced this issue Jul 20, 2020
This commit adds image assets for High Contrast mode

Tagging this issue so it contains a nice list of all the recent HC
fixes: #5360

I made several changes to DHowett's script and added it to the repo:
* Add support for generating high contrast icons
* Add the ability to easily edit the "intermediate" (previously "zbase")
  files for manual hinting
* Appease the spellchecker

I created new SVGs for HC mode. There's one SVG for both Black and White
modes -- I just invert the colors. Then I manually hinted the generated
bitmaps for the production icons. I didn't bother hinting the Dev/Pre
ones, so the text does get unreadable at small sizes.

View the original images in #6915.

Co-authored-by: Jeffrey Tippet <[email protected]>
Co-authored-by: Dustin L. Howett <[email protected]>
Closes #6822

(cherry picked from commit bd93cb5)
@ghost
Copy link

ghost commented Jul 21, 2020

🎉This issue was addressed in #6915, which has now been successfully released as Windows Terminal v1.1.2021.0.:tada:

Handy links:

1 similar comment
@ghost
Copy link

ghost commented Jul 22, 2020

🎉This issue was addressed in #6915, which has now been successfully released as Windows Terminal v1.1.2021.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 22, 2020

🎉This issue was addressed in #6915, which has now been successfully released as Windows Terminal Preview v1.2.2022.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 22, 2020

🎉This issue was addressed in #6833, which has now been successfully released as Windows Terminal Preview v1.2.2022.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Tracking-External This bug isn't resolved, but it's following an external workitem.
Projects
None yet
5 participants