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(slider): corrects contrast bug caused by template default arg #3614

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

cdransf
Copy link
Member

@cdransf cdransf commented Mar 11, 2025

Description

Corrects the contrast issue observed between the fill element and slider track that caused the fill element to be difficult to see.

  • Adjusts colors per design feedback.
  • Removes default control/template value that caused contrast issue in dark modew.

Validation steps

  1. Open Storybook URL
  2. Navigate to slider
  3. Change theme from light to dark
  4. Enable testing preview
  5. Scroll to filled offset and verify offset and track have sufficient contrast

Screenshots

image

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. ✨

@cdransf cdransf added size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. skip_vrt Add to a PR to skip running VRT (but still pass the action) ready-for-review labels Mar 11, 2025
@cdransf cdransf self-assigned this Mar 11, 2025
Copy link

changeset-bot bot commented Mar 11, 2025

⚠️ No Changeset found

Latest commit: ca1eb32

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Mar 11, 2025

File metrics

Summary

Total size: 2.25 MB*

🎉 No changes detected in any packages

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

Copy link
Contributor

github-actions bot commented Mar 11, 2025

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

@cdransf cdransf force-pushed the cdransf/slider-contrast-fix branch from f56782b to e7e160b Compare March 11, 2025 15:50
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

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

Thanks for digging into the gray values- glad we got them corrected!

Looking at prod, is this PR supposed to address main? I think I see the same issue, so if we fix it on main, could we just cherry pick over to spectrum-two?

Screenshot 2025-03-11 at 11 49 54 AM

@cdransf cdransf force-pushed the cdransf/slider-contrast-fix branch 2 times, most recently from adbf4c0 to 1de25b8 Compare March 11, 2025 16:06
@cdransf cdransf changed the base branch from spectrum-two to main March 11, 2025 16:07
@cdransf cdransf force-pushed the cdransf/slider-contrast-fix branch 5 times, most recently from 25b7240 to b05f885 Compare March 11, 2025 16:21
@cdransf cdransf force-pushed the cdransf/slider-contrast-fix branch 2 times, most recently from 0e2b62a to 54562e4 Compare March 11, 2025 17:28
@cdransf
Copy link
Member Author

cdransf commented Mar 11, 2025

Thanks for digging into the gray values- glad we got them corrected!

Looking at prod, is this PR supposed to address main? I think I see the same issue, so if we fix it on main, could we just cherry pick over to spectrum-two?

Absolutely! I lifted everything over to main and squashed it down so we can cherry-pick in a follow up PR. ✨

@cdransf cdransf force-pushed the cdransf/slider-contrast-fix branch from 54562e4 to 6200ea4 Compare March 11, 2025 23:12
@cdransf cdransf force-pushed the cdransf/slider-contrast-fix branch from 6200ea4 to f5fb64c Compare March 12, 2025 19:16
@castastrophe castastrophe force-pushed the main branch 10 times, most recently from c68f4e3 to d2272ea Compare March 12, 2025 21:56
@castastrophe castastrophe force-pushed the cdransf/slider-contrast-fix branch from f5fb64c to ca1eb32 Compare March 12, 2025 22:11
@castastrophe castastrophe enabled auto-merge (squash) March 12, 2025 22:14
@castastrophe castastrophe merged commit ae8a126 into main Mar 12, 2025
18 of 24 checks passed
@castastrophe castastrophe deleted the cdransf/slider-contrast-fix branch March 12, 2025 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants