Skip to content
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.

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

[Slider] Holding the thumb after pointerdown on touch devices opens context menu #1295

Open
mj12albert opened this issue Jan 7, 2025 · 11 comments · May be fixed by #1339
Open

[Slider] Holding the thumb after pointerdown on touch devices opens context menu #1295

mj12albert opened this issue Jan 7, 2025 · 11 comments · May be fixed by #1339
Assignees
Labels
bug 🐛 Something doesn't work component: slider This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation

Comments

@mj12albert
Copy link
Member

Devtools responsive mode:

Screen.Recording.2025-01-07.at.6.36.51.pm.mov

iOS:

RPReplay_Final1736235686.MP4
@mj12albert mj12albert added component: slider This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work labels Jan 7, 2025
@mj12albert
Copy link
Member Author

Probably a long press is causing this

@onehanddev
Copy link
Contributor

Hi @mj12albert,
I would love to debug and fix this issue, can you please assign it to me ? Thanks !

@mj12albert mj12albert self-assigned this Jan 7, 2025
@mj12albert
Copy link
Member Author

mj12albert commented Jan 13, 2025

I think there's two ways we could go about this:

  1. Leave it to the user to use user-select: none on the thumb. Radix puts it on the Root in their docs demo. For us it would have to be on the thumb in order to not interfere with selecting text rendered by Slider.Value.

  2. Try to suppress the context menu without interfering with text selection, even if the thumb renders text, like RA does: https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/textSelection.ts . This could get pretty complicated, IMO even if the thumb renders text I'd almost never try to or want to select the text? A similar situation as our tabs demo applying user-select: none unconditionally on the Tab: https://github.com/mui/base-ui/blob/master/docs/src/app/(public)/(content)/react/components/tabs/demos/hero/css-modules/index.module.css#L29

@colmtuite
Copy link
Contributor

Option 1 seems fine

@github-project-automation github-project-automation bot moved this to Backlog in Base UI Jan 13, 2025
@colmtuite colmtuite moved this from Backlog to Selected in Base UI Jan 13, 2025
@mj12albert mj12albert added the docs Improvements or additions to the documentation label Jan 13, 2025
@mj12albert
Copy link
Member Author

@onehanddev We decided to just handle it using user-select in the demo styles, are you still interested in making a PR?

@onehanddev
Copy link
Contributor

@mj12albert Sure, I will be creating the PR, Can you please assign it to me ?

@onehanddev
Copy link
Contributor

Hi @mj12albert, I might be overlooking something, but applying user-select: none; on Slider.Thumb or any ancestor elements doesn’t seem to resolve this issue. I also tested similar behavior on the Radix UI and React Spectrum demo pages, and the context menu still opens when pressing and holding on a touch screen device. Here’s a screen recording demonstrating the issue.

Screen.Recording.2025-01-14.at.8.27.37.PM.mov

@colmtuite
Copy link
Contributor

@onehanddev It needs touch-action: none; too

@onehanddev
Copy link
Contributor

It tried that too but it still didn't work. Is it working for you ? @colmtuite

@colmtuite
Copy link
Contributor

colmtuite commented Jan 15, 2025

@onehanddev dev tools emulator is not reliable. Testing Radix on iOS it seems to work ok.

@onehanddev
Copy link
Contributor

@colmtuite Yeah, I was able to reproduce it in mobile device, and raised the PR, please feel free to review it, Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: slider This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
Status: Selected
3 participants