Skip to content

fix: make tick slider handles align to ticks #1289

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

fix: make tick slider handles align to ticks #1289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
496f4fc
fix: make tick slider handles align to ticks perfectly
lazd Oct 8, 2021
2d57a20
feat: only use round slider values in demo, update value display
lazd Oct 8, 2021
0f45713
Merge branch 'main' into slider-ticks
lazd Nov 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions components/slider/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,12 +418,12 @@ governing permissions and limitations under the License.
/* 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));
width: calc(100% + var(--spectrum-slider-handle-width-adjusted));
Copy link
Contributor

@adixon-adobe adixon-adobe Oct 8, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@lazd lazd Oct 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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:

image

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.

Copy link
Contributor

@adixon-adobe adixon-adobe Oct 9, 2021

Choose a reason for hiding this comment

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

Awesome -- makes total tense. Nice work!


position: absolute;
top: calc(var(--spectrum-slider-track-height) / 2 - 1px);

margin-left: calc(var(--spectrum-slider-handle-width) / 2 * -1);
margin-left: calc(var(--spectrum-slider-handle-width-adjusted) / 2 * -1);
}

.spectrum-Slider-trackContainer {
Expand Down
50 changes: 32 additions & 18 deletions components/slider/metadata/slider.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,19 +188,22 @@ examples:
<div class="spectrum-Slider-value" role="textbox" aria-readonly="true" aria-labelledby="spectrum-Slider-label-14">14</div>
</div>
<div class="spectrum-Slider-controls">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-ticks">
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
</div>
<div class="spectrum-Slider-handle" style="left: 40%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" id="spectrum-Slider-input-14">
<div class="spectrum-Slider-trackContainer">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-track"></div>
</div>
<div class="spectrum-Slider-handleContainer">
<div class="spectrum-Slider-handle" style="left: 75%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" id="spectrum-Slider-input-14">
</div>
</div>
<div class="spectrum-Slider-track"></div>
</div>
</div>
<div class="spectrum-Slider spectrum-Slider--tick is-disabled">
Expand All @@ -209,19 +212,22 @@ examples:
<div class="spectrum-Slider-value" role="textbox" aria-readonly="true" aria-labelledby="spectrum-Slider-label-15">14</div>
</div>
<div class="spectrum-Slider-controls">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-ticks">
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
<div class="spectrum-Slider-tick"></div>
</div>
<div class="spectrum-Slider-handle" style="left: 40%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" disabled id="spectrum-Slider-input-15">
<div class="spectrum-Slider-trackContainer">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-track"></div>
</div>
<div class="spectrum-Slider-handleContainer">
<div class="spectrum-Slider-handle" style="left: 75%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" id="spectrum-Slider-input-14">
</div>
</div>
<div class="spectrum-Slider-track"></div>
</div>
</div>
- id: slider-tick
Expand All @@ -234,7 +240,6 @@ examples:
<div class="spectrum-Slider-value" role="textbox" aria-readonly="true" aria-labelledby="spectrum-Slider-label-12">14</div>
</div>
<div class="spectrum-Slider-controls">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-ticks">
<div class="spectrum-Slider-tick">
<div class="spectrum-Slider-tickLabel">0</div>
Expand All @@ -255,10 +260,15 @@ examples:
<div class="spectrum-Slider-tickLabel">100</div>
</div>
</div>
<div class="spectrum-Slider-handle" style="left: 40%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" id="spectrum-Slider-input-12">
<div class="spectrum-Slider-trackContainer">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-track"></div>
</div>
<div class="spectrum-Slider-handleContainer">
<div class="spectrum-Slider-handle" style="left: 40%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" id="spectrum-Slider-input-12">
</div>
</div>
<div class="spectrum-Slider-track"></div>
</div>
</div>
<div class="spectrum-Slider is-disabled">
Expand All @@ -267,7 +277,6 @@ examples:
<div class="spectrum-Slider-value" role="textbox" aria-readonly="true" aria-labelledby="spectrum-Slider-label-13">14</div>
</div>
<div class="spectrum-Slider-controls">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-ticks">
<div class="spectrum-Slider-tick">
<div class="spectrum-Slider-tickLabel">0</div>
Expand All @@ -288,10 +297,15 @@ examples:
<div class="spectrum-Slider-tickLabel">100</div>
</div>
</div>
<div class="spectrum-Slider-handle" style="left: 40%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" disabled id="spectrum-Slider-input-13">
<div class="spectrum-Slider-trackContainer">
<div class="spectrum-Slider-track"></div>
<div class="spectrum-Slider-track"></div>
</div>
<div class="spectrum-Slider-handleContainer">
<div class="spectrum-Slider-handle" style="left: 40%;">
<input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" id="spectrum-Slider-input-12">
</div>
</div>
<div class="spectrum-Slider-track"></div>
</div>
</div>
- id: slider-player
Expand Down
9 changes: 7 additions & 2 deletions site/resources/js/enhancement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ function makeDoubleSlider(slider) {
var sliderOffsetLeft = slider.offsetLeft + slider.offsetParent.offsetLeft;

var x = Math.max(Math.min(e.x - sliderOffsetLeft, sliderOffsetWidth), 0);
var percent = (x / sliderOffsetWidth) * 100;
var percent = Math.round((x / sliderOffsetWidth) * 100);

if (isRTL()) {
percent = 100 - percent;
Expand Down Expand Up @@ -545,6 +545,7 @@ function makeSlider(slider) {
var handle = handles[0];
var isColor = slider.classList.contains('spectrum-Slider--color');
var fill = slider.querySelector('.spectrum-Slider-fill');
var value = slider.querySelector('.spectrum-Slider-value');

if (handles.length > 1) {
makeDoubleSlider(slider);
Expand Down Expand Up @@ -575,12 +576,16 @@ function makeSlider(slider) {
var sliderOffsetLeft = slider.offsetLeft + slider.offsetParent.offsetLeft;

var x = Math.max(Math.min(e.x - sliderOffsetLeft, sliderOffsetWidth), 0);
var percent = (x / sliderOffsetWidth) * 100;
var percent = Math.round((x / sliderOffsetWidth) * 100);

if (isRTL()) {
percent = 100 - percent;
}

if (value) {
value.innerText = percent;
}

if (leftTrack && rightTrack && !isColor) {
leftTrack.style.width = percent + '%';
rightTrack.style.width = (100 - percent) + '%';
Expand Down