[Slider] Consider using touch-action rather than preventDefault

[dbismut]

Bug report

Current Behavior

The Slider component uses event.preventDefault() when touch starts to prevent scrolling on mobile. https://github.com/radix-ui/primitives/blob/fb723f441f8c03b902f6ee511a017cbcb79060d5/packages/react/slider/src/Slider.tsx#L457

This generates two admittedly minor issues:

1. It prevents vertical scrolling when the scroll starts from the Slider Thumb. This is acceptable when the slider is vertical, but in the case of an horizontal slider, I don't think the slider should refrain the user from scrolling vertically.

2. TouchStart events aren't cancellable in Chrome when they're passive, and React doesn't have handlers supporting non passive events. In other words, this line generates the following warning in Chrome (responsive mode):

Suggested solution

Option 1 An easy fix to remove the Chrome error would be to check if the event is cancellable. It would be just ignoring the issue, but at least you don't get the red warning in the console ;)

if (event.cancelable) event.preventDefault()

Option 2 A better fix would be to avoid using preventDefault and rather use touch-action css property. You could use touch-action: pan-y on the horizontal slider (ie the browser will allow vertical scrolling) and touch-action: pan-x for the vertical slider (ie the browser will allow horizontal scrolling).

[benoitgrelard]

Thanks again for the suggestion David. I'll let @jjenzz catchup with the discussion and the suggestion here and we'll figure something out. 👍

[asherccohen]

On a side note, while working on this fix, it would be nice to have an additional onFinalChange prop that allows triggering an action only when user ends moving the slider. This acts like a debounced effect, very useful for dispatching redux actions or network mutations.

[jjenzz]

@asherccohen I'm not sure if we would provide an API like that. We try to provide a minimal API surface that gives you all the information you need to be able to achieve the requirements of your product. In your case, you can debounce our onValueChange yourself: https://codesandbox.io/s/wonderful-brown-xrddb?file=/App.js.

Having said that, it does sound a lot like the distinction between onChange and onInput. Our current behaviour is more like native onInput so maybe we could consider onValueInput and change our onValueChange to fire on blur (like native events). If that would be useful, please feel free to create a separate feature request.

[asherccohen]

Thanks @jjenzz, I added a request here https://github.com/radix-ui/primitives/issues/903