Skip to content

[a11y] ContentSwitcher - High Contrast Mode #19336

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mariyageorge01
Copy link
Contributor

@mariyageorge01 mariyageorge01 commented May 7, 2025

Closes #19140

Fixed the ContentSwitcher Windows High Contrast mode style issues.

Changelog

New

  • Added styles to fix issues in HCM.

Changed

  • NA

Removed

  • NA

Testing / Reviewing

  • Pull the branch locally and ensure that ContentSwitcher works fine in HCM.

PR Checklist

As the author of this PR, before marking ready for review, confirm you:

  • Reviewed every line of the diff
  • Updated documentation and storybook examples - NA
  • Wrote passing tests that cover this change - NA
  • Addressed any impact on accessibility (a11y)
  • Tested for cross-browser consistency
  • Validated that this code is ready for review and status checks should pass

More details can be found in the pull request guide

Copy link

netlify bot commented May 7, 2025

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit de042e7
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/6827022a9016e900086de736
😎 Deploy Preview https://deploy-preview-19336--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

netlify bot commented May 7, 2025

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit de042e7
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/6827022af6e01b000805d6ec
😎 Deploy Preview https://deploy-preview-19336--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

codecov bot commented May 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.85%. Comparing base (bdc774e) to head (de042e7).
Report is 48 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #19336   +/-   ##
=======================================
  Coverage   84.85%   84.85%           
=======================================
  Files         371      371           
  Lines       14413    14413           
  Branches     4694     4716   +22     
=======================================
  Hits        12230    12230           
+ Misses       2037     2036    -1     
- Partials      146      147    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mariyageorge01 mariyageorge01 marked this pull request as ready for review May 7, 2025 16:51
@mariyageorge01 mariyageorge01 requested review from a team as code owners May 7, 2025 16:51
@wkeese
Copy link
Contributor

wkeese commented May 9, 2025

FYI, looks like we were working on the invisible icon problem in parallel, although attached to different ticket numbers. I think I have a fix in #19352 that eliminates the need for these mixin calls in general:

@include high-contrast-mode('icon-fill');
@include high-contrast-mode('icon-fill-selected')

I am curious if this part of your PR actually worked for you:

    @if ($type == 'selected') {
      background-color: SelectedItem;
    }

I ask because I thought that background-color was ignored in Windows High Contrast mode.

@mariyageorge01
Copy link
Contributor Author

FYI, looks like we were working on the invisible icon problem in parallel, although attached to different ticket numbers. I think I have a fix in #19352 that eliminates the need for these mixin calls in general:

@include high-contrast-mode('icon-fill');
@include high-contrast-mode('icon-fill-selected')

Cool 😎 (I will take a look into that PR). @wkeese Should I wait for your PR to be merged before merging mine (in case mine gets approved)?

I am curious if this part of your PR actually worked for you:

@if ($type == 'selected') {
  background-color: SelectedItem;
}

I ask because I thought that background-color was ignored in Windows High Contrast mode.

I am on Mac and when forced-colors is enabled, I get:

Screenshot 2025-05-09 at 6 21 23 PM

@wkeese
Copy link
Contributor

wkeese commented May 10, 2025

Cool 😎 (I will take a look into that PR). @wkeese Should I wait for your PR to be merged before merging mine (in case mine gets approved)?

I think that makes sense, then you can remove the parts setting icon-fill.

I am on Mac and when forced-colors is enabled, I get:

Screenshot 2025-05-09 at 6 21 23 PM

Oh you're right, although it's complicated.

According to my tests on Windows 11 (in high contrast), a normal background color like this is ignored:

@media screen and (-ms-high-contrast: active), (forced-colors: active) {
    .cds--content-switcher-btn::after {
        background-color: red;
    }
}

But when you set it to one of the high-contrast colors (like as done in your PR), it works:

@media screen and (-ms-high-contrast: active), (forced-colors: active) {
    .cds--content-switcher-btn::after {
        background-color: SelectedItem;
    }
}

PS: I added this to my PR so I think you can get rid of the @include high-contrast-mode('selected') too:

// Force a background-color for selected buttons etc., otherwise the user won't know they are selected.
@include custom-property.declaration('background-selected', SelectedItem);
@include custom-property.declaration(
  'background-selected-hover',
  SelectedItem
);
@include custom-property.declaration(
  'layer-selected-inverse',
  SelectedItem
);

- fix hcm style for ContentSwitcher

Contributes-to: carbon-design-system#19140
Signed-off-by: Mariya George [email protected]
- highlight selected tab

Contributes-to: carbon-design-system#19140
Signed-off-by: Mariya George [email protected]
@2nikhiltom
Copy link
Contributor

@mariyageorge01 thanks for your PR - this looks good, just some conflicts that needs attention

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[a11y]: [Windows High Contrast Mode]: [ContentSwitcher] Icons are not visible and selected item has no borders
3 participants