mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.93k stars 32.27k forks source link

[joy-ui][slider] Slider is draggable beyond bounds (touch) #39362

Open pixelass opened 1 year ago

pixelass commented 1 year ago

Duplicates

Latest version

Steps to reproduce 🕹

Link to live example: https://mui.com/joy-ui/react-slider/

Steps:

  1. open example
  2. scroll to first example
  3. open dev tools with touch
  4. drag ouside the bounds of root

Current behavior 😯

On touch devices the dragging of the slider works far beyond the root element.

Screenshot: the gaps either drag the upper or lower slider

image

If an interactive element is in the gap, the behavior is as expected image

Screencast

https://github.com/mui/material-ui/assets/1148334/131104b7-b517-4da1-9536-b173e56b0a06

https://github.com/mui/material-ui/assets/1148334/d22467f1-6572-472f-a364-3b34f1f2a027

Expected behavior 🤔

I expect to be able do click on an area outside the slider bounds without triggering the slider itself, so that I can perform actions like scrolling.

Context 🔦

I have a page with several slider stacked in a box. I want to scroll the page on mobile, but it is impossible because each slider is somehow blocking me and triggering the slider instead of scrolling.

This breaks my UI/UX.

Your environment 🌎

npx @mui/envinfo ``` System: OS: Windows 10 10.0.22621 Binaries: Node: 18.5.0 - C:\Program Files\nodejs\node.EXE @emotion/styled: 11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.17 @mui/core-downloads-tracker: 5.14.12 @mui/icons-material: 5.14.11 => 5.14.11 @mui/joy: 5.0.0-beta.9 => 5.0.0-beta.9 @mui/material: ^5.14.11 => 5.14.11 @mui/private-theming: 5.14.12 @mui/styled-engine: 5.14.12 @mui/system: 5.14.12 @mui/types: 7.2.5 @mui/utils: 5.14.12 @types/react: ^18.2.23 => 18.2.23 react: 18.2.0 => 18.2.0 react-dom: 18.2.0 => 18.2.0 typescript: ^5.2.2 => 5.2.2 ```
pixelass commented 1 year ago

It looks like the issue comes from useSlider via base-ui. It can be reproduced here: https://mui.com/base-ui/react-slider/

brijeshb42 commented 1 year ago

I notice that the trigger area to capture dragging/touch expands to the box that encapsulates the slider. But on an actual device, I also found that this is not really an issue as you need buffer zone for even the slightest touch interaction. @michaldudak What do you think? It's reproducible on desktop with touch emulation.

michaldudak commented 1 year ago

Yes, the area to click is a bit larger than the track and thumb to make it easier to aim. Perhaps we can make it a bit smaller, but that's up to Joy UI maintainers (cc @siriwatknp).

gitstart commented 1 year ago

@michaldudak @siriwatknp we would like to pick this up.

michaldudak commented 1 year ago

Let's discuss the desired behavior with @siriwatknp first.

pixelass commented 1 year ago

Here's another reason why this is currently a problem:

I have a colorpicker and when I close it, the slider get's triggered. (tested in dev-tools, so I'm not sure if this even applies on a real mobile device)

https://github.com/mui/material-ui/assets/1148334/c7431b52-b8f8-4863-a635-567ef6bb9008

brijeshb42 commented 1 year ago

Have you tested the experience on a real mobile device to see if the scenario is same there as well?

siriwatknp commented 1 year ago

This is to have a minimum touch area for touchscreen devices. @zanivan @danilo-leal Do you have a source to follow the minimum touch area that we can refer to?

@pixelass For workaround, this can be customize via sx or theme.

<Slider
    sx={{ "--Slider-size": "32px" }}
  />

https://codesandbox.io/s/fervent-aryabhata-l6dccw?file=/src/Demo.tsx:252-536

danilo-leal commented 1 year ago

Yeah! There is some touch target documentation out there, but to sum it up, for small area components such as a slider's thumb, I think between 42 and 48px is a healthy target.

One thing that I think it'd be helpful there, though, is having the focus treatment (potentially on mobile only) fill the entire touch space; otherwise, it's indeed confusing to understand why the slider works if you're "apparently" not touching it. 🤔

siriwatknp commented 1 year ago

Yeah! There is some touch target documentation out there, but to sum it up, for small area components such as a slider's thumb, I think between 42 and 48px is a healthy target.

One thing that I think it'd be helpful there, though, is having the focus treatment (potentially on mobile only) fill the entire touch space; otherwise, it's indeed confusing to understand why the slider works if you're "apparently" not touching it. 🤔

The current touch area is 42px. I'd wait for @zanivan's comment. If we agree to go with 42px, I will close this issue.

zanivan commented 1 year ago

@siriwatknp I would rather use 44px just for the accessibility sake. Would that work?

pixelass commented 1 year ago

I think Google/Lighhouse expects 48px based on the Android guidelines: https://support.google.com/accessibility/android/answer/7101858?hl=en

There was a very detailed discussion on this: https://github.com/GoogleChrome/lighthouse/issues/4358

This is also mentioned in the material-design specs: https://m2.material.io/design/usability/accessibility.html#assistive-technology

Have you tested the experience on a real mobile device to see if the scenario is same there as well?

I tested this and it is noticeable but not as crucial since iOS seems to block certain touches when a scroll happens. I haven't tested in on an Android though.

pixelass commented 1 year ago

I did some further testing on iOS.

Go to the "track" section of the docs. There seems to be a difference in the slider being focused. There is definitely a certain annoyance and potential danger in making unwanted adjustments while scrolling.

But overall on a real device is seems a "a lot" better than on the dev tools or iOS simulator.