radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.87k stars 823 forks source link

[Slider] Incorrect range mapping #1966

Open latin-1 opened 1 year ago

latin-1 commented 1 year ago

Bug report

Dragging the thumb or clicking on the track.

Current Behavior

Current Behavior

Expected behavior

Expected Behavior

Reproducible example

CodeSandbox Template

Suggested solution

Modify the mapping function.

$$ \frac12W{thumb}+V{percent}(W{track}-W{thumb}) $$

Additional context

Other implementations:

Note: There is also another type of slider:

Another Type

Your environment

Software Name(s) Version
Radix Package(s) @radix-ui/react-slider 1.1.0
React n/a
Browser
Assistive tech
Node n/a
npm/yarn
Operating System
benoitgrelard commented 1 year ago

Hey @latin-1, the current implementation is by design. We interpolate the size of the thumb over the whole range so it doesn't get out of bounds (outside of its natural bounding box).

What is "correct" is up to interpretation, for example, our implementation matches the native input range on MacOS:

CleanShot 2023-02-20 at 15 57 22@2x

That said, I agree that this is perhaps a little opinionated, and something we have discussed about revisiting at some point in the future, by potentially providing a way to opt into different positioning strategies.

latin-1 commented 1 year ago

@benoitgrelard You misunderstood what I meant.

The native one and yours do not behave in the same way. For example, if I press the mouse at the center of the thumb and then drag to the right edge, the cursor should remain at the center. If I click close to the edge, it should seek to the track edge (the min/max value) rather than a value close to the edge. That's how the native one act.

latin-1 commented 1 year ago

The third picture is just for reference. It's not related to this issue.

latin-1 commented 1 year ago
// current behavior:
value = event.offsetX / tracker.width
// expected behavior:
value = clamp((event.offsetX - thumb.width / 2) / (tracker.width - thumb.width), 0, 1)
benoitgrelard commented 1 year ago

That's not the behaviour I see, you can clearly see it jump here:

https://user-images.githubusercontent.com/1539897/220157300-0800a81e-1f4f-4f5b-98ae-1acebde6e921.mp4

latin-1 commented 1 year ago

@benoitgrelard I mean close to, especially, the distance between the cursor and the edge should less than half of the thumb size.

latin-1 commented 1 year ago

and in your video, after seek, the cursor is at the center of the thumb. but it's not true for radix-ui

benoitgrelard commented 1 year ago

I'm sorry I'm not really following. Your initial diagram are showing one thing, and you're talking about something else now which isn't super clear. The position of the the native one seems to jump too depending on where the cursor is when starting the slide.

benoitgrelard commented 1 year ago

and in your video, after seek, the cursor is at the center of the thumb. but it's not true for radix-ui

Yeah I see that, but I don't think one is necessarily better than the other, that's just a slight different in implementation. We interpolate the thumb width over the whole range, ours is entirely driven on the pointer position.

latin-1 commented 1 year ago

Yeah I see that, but I don't think one is necessarily better than the other, that's just a slight different in implementation. We interpolate the thumb width over the whole range, ours is entirely driven on the pointer position.

I don't think so. No one other than radix-ui implements the slider in this way. You can try to increase the thumb size and see the weird behavior. While dragging, the thumb does not follows the cursor properly.

benoitgrelard commented 1 year ago

While dragging, the thumb does not follows the cursor properly.

I know it doesn't, as I said "we interpolate the thumb width over the whole range" and because we keep it in-bound it will look as if it's moving slightly under the pointer.

latin-1 commented 1 year ago

In our project, we use a large thumb (~100px) and that is quite problematic.

latin-1 commented 1 year ago

and because we keep it in-bound

It's not related to whatever in-bound or out-bound. No one other than radix-ui implements the in-bound slider in this way.

benoitgrelard commented 1 year ago

I haven't seen many examples of libraries even having an in-bound layout like this. Do you know any so we can compare?

latin-1 commented 1 year ago

I haven't seen many examples of libraries even having an in-bound layout like this.

I don't have either. But as far as I can tell, the iOS one, the Web's native one, the one in Adobe Spectrum, are behave in the same way, which is different from yours.

benoitgrelard commented 1 year ago

Here, to make sure we are talking about the same thing. This is what I mean by:

We interpolate the thumb width over the whole range

Current one (in bounds), with the compensation of the thumb width:

https://user-images.githubusercontent.com/1539897/220164099-15437cc2-5dce-4783-b776-c09781b131ca.mp4


If we didn't do compensate to keep in bounds (what most implementations do, thumb goes outside of bounding box):

https://user-images.githubusercontent.com/1539897/220164144-3d5366cb-7a23-473b-9547-393c0624850f.mp4


So you can see that what you are describing is directly related to the compensation to keep in-bounds.

latin-1 commented 1 year ago

So you can see that what you are describing is directly related to the compensation to keep in-bounds.

I don't think so. For example, React Spectrum, which is in-bound, acts like the second one with a slightly longer track. That's the common pattern for in-bound sliders. The Web's native one does the same.

AFAIK, the only other implementation interpolate the position over whole range, is the WAI-ARIA's color viewer slider. But the multi-thumb slider example from WAI-ARIA just follows the native one. It's hard to say which is recommended from WAI-ARIA.

latin-1 commented 1 year ago

https://www.w3.org/WAI/ARIA/apg/patterns/slider/examples/slider-color-viewer/ https://www.w3.org/WAI/ARIA/apg/patterns/slider-multithumb/examples/slider-multithumb/

benoitgrelard commented 1 year ago

It's not in-bounds right? Or am I missing something?

https://user-images.githubusercontent.com/1539897/220168428-d27561de-d8c8-407c-a2a4-5f8d70986954.mp4

latin-1 commented 1 year ago

@benoitgrelard https://react-spectrum.adobe.com/react-spectrum/Slider.html I mean this one.

benoitgrelard commented 1 year ago

Oh I see weird, their react-aria seems to be out-bounds, but the react-spectrum one which is built on top seems to be in-bounds as you point out. I can't figure out what they're doing to change that though looking at the source… 🤔

benoitgrelard commented 1 year ago

I think I found it, a combination of margin compensation and width:

CleanShot 2023-02-20 at 17 36 47@2x
benoitgrelard commented 1 year ago

Yep, if I deactivate our compensation, we can still achieve in-bounds using negative margins like this:

https://user-images.githubusercontent.com/1539897/220171672-6c44113a-22cf-4d6f-929a-c2396ce5c30f.mp4

Can you confirm this is what you'd be after?

latin-1 commented 1 year ago

Yes that's what I mean. Currently, the only way to achieve this is, using a zero-width thumb container.

benoitgrelard commented 1 year ago

Ok, thanks for confirming. So one thing we could do potentially is offer a way to opt-out of the thumb compensation. Would that be good for you?

latin-1 commented 1 year ago

Yes and no. I still think the current behavior is problematic though. Anyway, I'm OK with a new option. It's also useful for constructing an out-bounds slider.

benoitgrelard commented 1 year ago

What would this not enable?

latin-1 commented 1 year ago

I'm fine with either as long as the behavior is well documented.

hasparus commented 1 year ago

Hello, I'd like to chime in here, if I can.

Isn't current behaviour a bit problematic with a thicker range? As the range doesn't end in the middle of the thumb, if both have a border radius, we'll see a gap between them for value close to the minimum.

image

@latin-1's suggested solution would solve this problem.

image

I worked around it by turning the thumb into 0 width container with the :after, but that gets me an out of bounds slider, and I'd rather have an in-bounds one but without the gap.

Rephrasing for better understanding: I like the current Thumb position, but not the current Range width.

benoitgrelard commented 1 year ago

Can you post a sandbox of your case @hasparus?

hasparus commented 1 year ago

Sure thing @benoitgrelard. Here you go. I forked the CodeSandbox created by latin-1 and changed the height.

https://user-images.githubusercontent.com/15332326/221632283-7b3c6102-b64b-46f8-bac4-5ead3407c14c.mov

pacocoursey commented 1 year ago

In favor of disabling the compensation (which seems to be possible via user CSS anyways, as you showed) based on the simple rule: if I press down at x/y on the thumb, my cursor should still be at x/y when I release. Right now it feels unexpected because 1px of mouse movement does not equal 1px of thumb movement.

adri1wald commented 1 year ago

Sure thing @benoitgrelard. Here you go. I forked the CodeSandbox created by latin-1 and changed the height.

Screen.Recording.2023-02-27.at.18.11.10.mov

We also have this issue in our implementation. Ideally the Range end would be exactly in the middle of the Thumb across the whole "range"

If you zoom in you can even see this in the current demo

Screenshot 2023-07-10 at 21 21 56
joicesena commented 1 year ago

I also have this issue in my implementation, exactly what @adri1wald described above.

dreamboyx1 commented 1 year ago

Just want to +1 to update the slider to allows fine tuning. Right now it feels too sloppy and unpredictable for things like trying to finesse the volume of an audio wave form in a live performance. I've been looking at react slider (https://zillow.github.io/react-slider/#reactslider). The thing I love about this one is that when you click and release anywhere on the thumb, without moving the mouse, the slider value does not update. And, when you drag, the pointer sticks to the initial x position relative to the thumb. Or, I should say, the thumb sticks to the pointer. It feels more natural and responsive, IMO.

Anyway, I love this library a lot. Thank you for your hard work.

imalgrab commented 8 months ago

Hey, I see there are no updates nor open PRs. Have someone found a workaround other than using different lib? Thanks for all the work and contribution into radix-ui 🙇🏼

mark-gl commented 8 months ago

Ark/Zag's slider has a 'thumbAlignment' prop that lets you pick between 'contain' (similar to Radix's current behaviour) and 'center' (which behaves like the third slider style mentioned here, similar to MUI).

I've tried to replicate this with the Radix slider component here. If it's a suitable solution, I could open a PR.

the-me-0 commented 7 months ago

Ark/Zag's slider has a 'thumbAlignment' prop that lets you pick between 'contain' (similar to Radix's current behaviour) and 'center' (which behaves like the third slider style mentioned here, similar to MUI).

I've tried to replicate this with the Radix slider component here. If it's a suitable solution, I could open a PR.

Hi @mark-gl ! I can say your PR would be useful to me at least ! I don't know a lot about radix as I mainly use overlays, but I've been annoyed by this thumb alignment correction for some time now. I would really love to have a prop for it !

ADTC commented 7 months ago

I came here because it's super annoying that the thumb doesn't stay exactly under the cursor. It keeps slipping away. Watch this video:

https://github.com/radix-ui/primitives/assets/6047296/fb76c7c9-7703-4490-92f7-f4d2d651505d

The contain version is incorrectly implemented. As you can see above, observe how the thumb slips slowly from the grab hand cursor as I move the mouse while dragging the thumb. Also observe how it jerks away from the cursor when I try to grab it (by clicking and dragging again after a previous release of the mouse button).

While the center version is a nice feature to have, it doesn't solve the above problem. It's just a different UI of the slider. The contain version should actually be fixed to this correct implementation. The semantic range should be adjusted to compensate for half the width of the thumb on either side, while the visible range contains the thumb as it already does.

Expected Behavior

This will ensure that the thumb doesn't slip from under the mouse cursor. It will stay exactly where I grabbed it, no matter which part of the slider I move to. (This is what this issue is actually about. It's a bug report, not a feature request.)

PS: If you think about this, it's really just the same as center with extra visual padding on the track that's half the width of the thumb. So it looks like it's in-bound but behaves like it's out-bound. With the center option, I can at least add some CSS to the track to achieve the desired effect (looking inbound while being outbound). But it doesn't look like it would be possible in contain because the mouse behavior itself is broken and no amount of CSS on the track can fix it.

To clarify, we start first with the center option which contains the real track from min to max:

Another Type

Then we just have to add two fake tracks to the two sides of the real track. The fake ones would each be half the width of the thumb. This would give the compensate version.

mark-gl commented 7 months ago

The sliders I linked (Ark/Zag) appear to behave similarly to Radix in the contain mode (the cursor doesn't stay at the same point on the thumb when dragging), so it's not just Radix. However, the approach you're describing (which appears to be how the Mantine slider and Windows/macOS native sliders behave) seems to be more common.

Maybe it would make sense as a third alignment option in the PR, as I'm not sure about copying the name contain from those libraries and then having it behave differently.

ADTC commented 7 months ago

@mark-gl Regarding the naming. How about: