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(asset): add whcm values for asset background color #2369

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

jenndiaz
Copy link
Contributor

@jenndiaz jenndiaz commented Dec 14, 2023

Description

Addresses SWC issue 3376
"When running in high contrast mode the file and folder asset types should use the high contrast mode colours so they are always visible no matter the theme chosen"

Fix:

  • use currentColor on the background fill of the file and folder background when forced-colors: active
  • currentColor matches the inherited color property which unlike svg fill is overwritten with WHCM values

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

Open the docs site and emulate WHCM

  • asset Folder and File are visible in a dark color scheme
  • asset Folder and File are visible in a light color scheme

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

Before

Light
Screenshot 2023-12-15 at 1 38 07 PM

Dark
Screenshot 2023-12-15 at 1 38 20 PM

After

Light
Screenshot 2023-12-15 at 1 37 41 PM

Dark
Screenshot 2023-12-15 at 1 37 52 PM

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

Copy link
Contributor

github-actions bot commented Dec 14, 2023

🚀 Deployed on https://pr-2369--spectrum-css.netlify.app

Copy link
Contributor

github-actions bot commented Dec 14, 2023

File metrics

Summary

Total size: 3.97 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@jenndiaz jenndiaz marked this pull request as ready for review December 15, 2023 20:44
@jenndiaz jenndiaz requested review from pfulton, mdt2 and jawinn December 15, 2023 20:44
Copy link
Collaborator

@mdt2 mdt2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@castastrophe castastrophe force-pushed the jenndiaz/css-645-asset-whcm-fix branch from 432c4d0 to 83957c1 Compare December 19, 2023 20:13
@castastrophe
Copy link
Collaborator

Looks good - should we add a VRT for forced colors to protect from possible regressions in future?

@castastrophe castastrophe merged commit 4c1c959 into main Dec 20, 2023
10 checks passed
@castastrophe castastrophe deleted the jenndiaz/css-645-asset-whcm-fix branch December 20, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants