-
Notifications
You must be signed in to change notification settings - Fork 207
fix(slider): offset variant border radius bug fix #3611
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): offset variant border radius bug fix #3611
Conversation
🦋 Changeset detectedLatest commit: 54c3f11 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 |
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.
Nice! Looks good when pulling down the branch and stepping through the styles in the inspector. ✨
🚀 Deployed on https://pr-3611--spectrum-css.netlify.app |
File metricsSummaryTotal size: 2.25 MB*
slider
* An ASCII character in UTF-8 is 8 bits or 1 byte. |
c68f4e3
to
d2272ea
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.
Glad this was a nice quick fix! Looks like this just needs a rebase now.
This is really nitpicky, but I think when this merges, we should change this commit from feat
to fix
- it is a bug/regression fix, as opposed to something new for slider. In my mind, fix
also corresponds to the minor version bump a bit better than feat
does.
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.
I'm so glad we're looking into this one! There are a few things here that I noticed that I thought were really odd, which is holding back my approval:
- First, this isn't a problem in SWC, and after taking a quick glance, I really can't figure out why. The
&:last-of-type::before
selector seems to be working great there. I would think that any changes we make here would carry over fine to SWC and not cause problems, but I'm wondering if it's worth more of an investigation on how what appears to be the same CSS is working differently between the two environments. - Second, and maybe more importantly, this is throwing some regressions into the range variant, here it has a border-radius on the right side of the middle segment:
In comparison, this is how it looked before, with the border-radius only being applied to the end segments:
* Lastly, can you see the color diffs between the top and bottom image? For some reason the build preview and your local branch are running slightly different colors (honestly higher contrast in your branch tbh) than main... but that seems unexpected unless I missed something? That might end up being completely unrelated to your PR specifically, but noting it just in case?Ok it looks like there were some slider contrast things added here, so that doesn't seem like it's relevant here!
60c1060
to
54c3f11
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.
Thanks for looking into this one!
* feat(actionbutton): use s2 colors in spectrum-two theme (#3606) * feat(actionbutton): use closer to s2 colors in spectrum-two theme Requested colors update for action button, aligning the colors closer to the S2 design, post-foundations. In the foundations spectrum-two theme: - Removes the border - Changes the default background colors to match s2 specs - Updates the background colors used for static black and static white SWC-497 * fix(actionbutton): fix high contrast styles for selected disabled The selected + disabled button was not showing up as the disabled colors in high contrast mode. Fixed by adjusting the source order slightly in the high contrast media query so disabled is after selected and takes precedence. * fix(search): update disabled state in spectrum two (#3593) Co-authored-by: rise-erpelding <[email protected]> Co-authored-by: [ Cassondra ] <[email protected]> * chore(deps): bump the npm_and_yarn group with 2 updates (#3618) Bumps the npm_and_yarn group with 2 updates: [@babel/helpers](https://github.com/babel/babel/tree/HEAD/packages/babel-helpers) and [@babel/runtime](https://github.com/babel/babel/tree/HEAD/packages/babel-runtime). Updates `@babel/helpers` from 7.26.0 to 7.26.10 - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.26.10/packages/babel-helpers) Updates `@babel/runtime` from 7.24.4 to 7.26.10 - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.26.10/packages/babel-runtime) --- updated-dependencies: - dependency-name: "@babel/helpers" dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: "@babel/runtime" dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: update release script install settings * fix(button): adjust s2 static colors [SWC-496] (#3613) * chore: release (#3619) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(slider): corrects contrast bug caused by template default arg (#3614) * fix(stepper): fast follows for focus/focus+hover/keyboardFocus borders (#3621) * fix(stepper): focus/focus+hover/keyboardFocus border colors * chore(stepper): add changeset * fix(slider): offset variant border radius bug fix (#3611) * feat(slider): offset variant border radius bug fix * feat(slider): fix range slider * fix(combobox): border color fast follows swc-582 (#3609) * fix(combobox): correct s1/legacy container variable * fix(combobox): fast follow border color remapping - add mods for s2 foundations disabled picker button BG/border colors - correct disabled+read-only border color - add read-only border custom property - fixes express read-only border from gray-500 to gray-400 - update metadata.json * chore(combobox): create changeset * chore: release (#3623) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore: patch tj-actions vulnerability (#3626) * fix(alertbanner): change system variable from spectrum to legacy (#3624) * fix(alertbanner): change system: spectrum to legacy * chore(alertbanner): create changeset * test(checkbox): add more coverage for checkbox (#3625) * chore(checkbox): add isHovered state to checkbox - adds isHovered shared type and control to checkbox stories - adds several isHovered test cases - updates checkbox template to include isHovered arg * chore(form): fix the fieldgroup component input and labels * chore: release (#3631) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(checkbox): add invalid+checked+hover checkbox styles (#3617) - add missing ::before pseudo to target the before element in the invalid/checked/hover state - update metadata.json - create changeset * chore: release (#3632) * chore: release --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix: undefined and duplicated variable after merging main fix(slider): remove duplicated values Remove a large number of duplicate values causing stylelint "Unexpected duplicate" warnings. It looks like things got doubled up somehow in a previous rebase or merge. This included duplicate t-shirt size classes. Also moves root styles block under the custom property definitions to be consistent with other components. fix(combobox): fixes undefined and duplicated values --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: TaraT <[email protected]> Co-authored-by: rise-erpelding <[email protected]> Co-authored-by: [ Cassondra ] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Cory Dransfeldt <[email protected]> Co-authored-by: Marissa Huysentruyt <[email protected]> Co-authored-by: aramos-adobe <[email protected]>
Slider: offset variant track fix
The border radius styles were not being applied to the second instance of the
spectrum-Slider-track
when the offset variant is activated. The reason for this bug is because when theoffset
is selected, the template structure changes asspectrum-Slider-fill
gets added to the slider.Adding a sibling combinator
&~.spectrum-Slider-track
tospectrum-Slider-track
when offset is activated resolved the issue.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
Regression testing
Validate:
Screenshots
To-do list