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 tab contrast colors when in high contrast #18109

Merged
merged 3 commits into from
Nov 4, 2024

Conversation

carlos-zamora
Copy link
Member

Summary of the Pull Request

Originally, the XAML resources were being applied on the TabView's ResourceDictionary directly. However, high contrast mode has a few weird scenarios as it basically reduces the color palette to just a few colors to ensure high contrast. This PR now stores the resources onto the ThemeDictionaries so that we have more control over the colors used.

References and Relevant Issues

Closes #17913
Closes #13067 (fixed by 5be0056)

Validation Steps Performed

Compared the following scenarios to WinUI 2 gallery's TabView when in High Contrast mode:
✅ (Un)selected tab
✅ hover over x of (un)selected tab
✅ hover over unselected tab

This makes the tabs still adhere to the WT theme. However, that doesn't
necessarily guarantee that the colors will be in high contrast.
@carlos-zamora carlos-zamora added the Needs-Discussion Something that requires a team discussion before we can proceed label Oct 25, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Accessibility Issues related to accessibility Priority-1 A description (P1) Product-Terminal The new Windows Terminal. labels Oct 25, 2024
@carlos-zamora
Copy link
Member Author

Demo

Selected vs unselected

image

Unselected (hovered)

image
image

Unselected (Hover over X)

image
image

Selected (Hover over X)

image
image

@carlos-zamora
Copy link
Member Author

Discussion

The OS allows users to set a contrast theme:
{C32FE595-6B43-4FE6-9F47-F2D2C004E24C}

This is achieved by using the following XAML resource keys:
{D84E0F24-DC90-4C1C-9892-57B9A8A5B6A6}

However, Windows Terminal offers a lot of built-in customizability, namely through color schemes and theming. In fact, the default theme is:

{
    "name": "dark",
    "window": {
        "applicationTheme": "dark"
    },
    "tab": {
        "background": "terminalBackground",
        "unfocusedBackground": "#00000000"
    },
    "tabRow": {
        "unfocusedBackground": "#333333FF"
    }
},

So, the question is, how do we balance the two? What I've done here is ignore the OS theme, basically. The main one I'm looking at in the default theme is tab.background="terminalBackground" since that's a dynamic value. There may be some scenarios where high contrast may not be guaranteed. Though, the default color scheme with default theme works, and that's really the important one.

CC @zadjii-msft as the father of WT Theming

@carlos-zamora carlos-zamora removed the Needs-Discussion Something that requires a team discussion before we can proceed label Oct 28, 2024
@carlos-zamora
Copy link
Member Author

Discusion

Discussed as a team. The current solution works just fine. We'll keep an eye on the tab row and other reported High Contrast issues for the future.

@DHowett DHowett merged commit 72df7ac into main Nov 4, 2024
18 of 20 checks passed
@DHowett DHowett deleted the dev/cazamor/a11y/high-contrast-tab-bg branch November 4, 2024 22:51
carlos-zamora added a commit that referenced this pull request Dec 11, 2024
If we colored a tab, then switched to another tab, there's a bug that
the unselected tab loses its color. This was introduced in PR #18109.
This PR fixes that by actually applying the selected color to the tab
(whoops). Additionally, I removed setting the
"TabViewItemHeaderCloseButtonBackground" resource because it looked
weird (see comment in PR).

Closes #18226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal.
Projects
None yet
3 participants