-
Notifications
You must be signed in to change notification settings - Fork 198
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): use high contrast properties and fix ramp handle circle #2752
Conversation
🦋 Changeset detectedLatest commit: c233023 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🚀 Deployed on https://pr-2752--spectrum-css.netlify.app |
File metricsSummaryTotal size: 4.64 MB* Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.
Detailsslider
* Results are not gzipped or minified. * An ASCII character in UTF-8 is 8 bits or 1 byte. |
d44be12
to
540d7e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
540d7e8
to
e622311
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took another peek after the rebase conflicts and this is still looking good! Also checked in RTL. Nice work!
e622311
to
e29a340
Compare
Updates the Slider high contrast styles to use the --highcontrast prefixed custom properties. Some were being set with --spectrum prefixed custom properties. In the process, it was also found that the ramp variant was missing the handle outer circle (gap between the ramp background and handle), that created contrast between the handle in the background. This was particularly noticeable on hover/active, as the handle was blending into the same ramp highlight color. The fix was already present but the nested .spectrum-Slider--ramp selector was missing an '&' to make it the same element as the .spectrum-Slider
e29a340
to
c233023
Compare
Description
Updates the Slider high contrast styles to use all
--highcontrast
prefixed custom properties. It was noticed that some were being set with--spectrum
prefixed custom properties. Some were duplicative, as they already had--highcontrast
prefixed custom properties being set (which take precendence in thevar()
fallback chains). A few needed a rename.In the process, it was also found that the ramp variant was missing the handle outer circle (gap between the ramp background and handle), that created contrast between the handle in the background. This was particularly noticeable on hover/active, as the handle was blending into the same ramp highlight color. The fix was already present but the nested
.spectrum-Slider--ramp
selector was missing an&
to make it the same element as the.spectrum-Slider
CSS-760
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
Review note: The only changes made here are within the
@media (forced-colors: active)
. The rest of the files modified are automatic prettier fixes, including changes to whitespace.Regression testing
Validate:
Screenshots
"Ramp" variant high contrast bug before and after fix:
To-do list