fix: make tick slider handles align to ticks #1289
Conversation
| /* backwards compatibility: these are not required, but they make the handle go to the edegs and align right */ | ||
| .spectrum-Slider-handleContainer, | ||
| .spectrum-Slider-trackContainer { | ||
| width: calc(100% + var(--spectrum-slider-handle-width-adjusted)); |
There was a problem hiding this comment.
These changes will effectively widen the overall slider bounding box right (potentially leading to clients needing layout changes)?
I'm wondering if it'd be helpful to add another css variable for this context (so clients can reset it to 0 for the old behavior more easily, particularly in the web component implementation), and also potentially making a major version bump since this is likely a breaking change for some clients. I also know that the spectrum designs on this are unclear about what the behavior should be (though this change certainly seems correct to me). I posted in the spectrum design slack channel and can follow up there to make sure the designs are less ambiguous.
There was a problem hiding this comment.
It does not widen the bounding box actually, because those elements are positioned absolute inside of the slider's overall container. It does, however, overlap the bounding box visually. This could result in half of the slider handle being invisible if it's placed in a scrollable panel (with overflow-y: auto) that didn't have enough padding and the handle was at 0 or 100:
As far as backwards compatibility, the containers that make this change are optional, and existing markup works and looks exactly as it did before, so there is no breaking change in this PR.
There was a problem hiding this comment.
Awesome -- makes total tense. Nice work!
lazd
changed the title
|
I suspect PR #1547 will end up superseding this so I've highlighted this PR there to ensure changes address this concern. |
Indeed, I had this on my radar in https://jira.corp.adobe.com/browse/CSS-332 Clean up old Slider PRs. [feat: make tick slider handles align to ticks (follow up on #1289)] |
|
Thank so much for tackling this work. Closing it out in favor of the token migration effort but we're making sure this update is integrated. 🎉 |
Description
This PR adds two optional containers to contain the tracks and the handle, making handles align with all ticks most of the time (different sizes and positions of ticks are subject to off-by-one errors, but it's much closer than before). It also fixes the position of the first and last ticks, which were off by 1px.
Finally, it adds some JS to update the value of the slider in the docs on drag and restricts slider values to whole numbers.
Fixes #927
How and where has this been tested?
Screenshots
100%
75%
50%
To-do list