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

[docs][Slider] Prevent long presses from triggering context menu or text selection #1339

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

onehanddev
Copy link
Contributor

@onehanddev onehanddev commented Jan 15, 2025

@mui-bot
Copy link

mui-bot commented Jan 15, 2025

Netlify deploy preview

https://deploy-preview-1339--base-ui.netlify.app/

Generated by 🚫 dangerJS against 7c25836

Copy link

netlify bot commented Jan 15, 2025

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 8006317
🔍 Latest deploy log https://app.netlify.com/sites/base-ui/deploys/67891142971a9300081244e7
😎 Deploy Preview https://deploy-preview-1339--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@onehanddev onehanddev marked this pull request as ready for review January 15, 2025 09:17
@zannager zannager added component: slider This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation labels Jan 15, 2025
@onehanddev onehanddev changed the title (Docs/Slider) Prevent Context Menu on Long Press for Slider Thumb on Touch Devices [Docs/Slider] Prevent Context Menu on Long Press for Slider Thumb on Touch Devices Jan 16, 2025
@mj12albert
Copy link
Member

@onehanddev Would you mind trying to move the styles to the Track? It's still possible to text-select the track 😅 though much harder than before, at 15s here:

slider-user-select.mp4

@onehanddev
Copy link
Contributor Author

@onehanddev Would you mind trying to move the styles to the Track? It's still possible to text-select the track 😅 though much harder than before, at 15s here:

slider-user-select.mp4

@mj12albert I’ve applied the styles to all slider elements, as moving them to just the Track didn’t resolve the issue. This is because touch events are registered separately for each element. The issue should now be fully resolved. Let me know if you notice anything else! 😊

@mj12albert mj12albert changed the title [Docs/Slider] Prevent Context Menu on Long Press for Slider Thumb on Touch Devices [docs][Slider] Prevent long presses from triggering context menu or text selection Jan 17, 2025
@@ -4,6 +4,8 @@
align-items: center;
width: 14rem;
padding-block: 0.75rem;
touch-action: none;
Copy link
Member

Choose a reason for hiding this comment

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

👍 makes total sense to put this on Control actually, though it isn't needed on any descendents:

This means that in practice, touch-action is typically applied only to top-level elements which have some custom behavior, without needing to specify touch-action explicitly on any of that element's descendants

https://developer.mozilla.org/en-US/docs/Web/CSS/touch-action

(user-select is though, it doesn't inherit)

Copy link
Member

@mj12albert mj12albert left a comment

Choose a reason for hiding this comment

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

LGTM ~ just need to remove the redundant touch-actions @onehanddev

Though on iOS it seems impossible to fully prevent unwanted text selection in CSS alone, at 9s here I was trying to press the edge of .Control:

RPReplay_Final1737098312.mp4

As described in the RA codebase here:

Safari on iOS starts selecting text on long press. The only way to avoid this, it seems, is to add user-select: none to the entire page. Adding it to the pressable element prevents that element from being selected, but nearby elements may still receive selection. We add user-select: none on touch start, and remove it again on touch end to prevent this. This must be implemented using global state to avoid race conditions between multiple elements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: slider This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Slider] Holding the thumb after pointerdown on touch devices opens context menu
4 participants