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

[Tooltip] Long press select text on iOS #23466

Merged
merged 14 commits into from
Nov 12, 2020
Merged

Conversation

hmaddisb
Copy link
Contributor

I added a useRef for keeping track of old WebkitUserSelect. If placement of introducing the ref is incorrect, feedback is much appreciated!

Closes #23232

@mui-pr-bot
Copy link

mui-pr-bot commented Nov 10, 2020

Details of bundle changes

Generated by 🚫 dangerJS against d5e8760

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: tooltip This is the name of the generic UI component, not the React module! labels Nov 10, 2020
@hmaddisb
Copy link
Contributor Author

@oliviertassinari hi, haven't seen this type of failure before. is there something I have done incorrectly?

@oliviertassinari
Copy link
Member

We work on this problem at #23464.

@hmaddisb
Copy link
Contributor Author

ok, thank you for the information!

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Looks good. The cleanup is a bit too eager though.

packages/material-ui/src/Tooltip/Tooltip.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tooltip/Tooltip.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tooltip/Tooltip.js Show resolved Hide resolved
packages/material-ui/src/Tooltip/Tooltip.js Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari dismissed eps1lon’s stale review November 11, 2020 17:11

bug fixed and test case added

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

great cleanup

@eps1lon
Copy link
Member

eps1lon commented Nov 12, 2020

Did somebody confirm this works on iOS? It's not working on iPadOS. The PR description might still be correct since iOS and iPadOS are different operating systems. Just want to make sure we don't think this fixed it on all touch devices.

@oliviertassinari
Copy link
Member

I can confirm the PR fixes the behavior on these two simulators:

Capture d’écran 2020-11-12 à 14 25 46

Capture d’écran 2020-11-12 à 14 25 40

@eps1lon
Copy link
Member

eps1lon commented Nov 12, 2020

I can confirm the PR fixes the behavior on these two simulators:

Capture d’écran 2020-11-12 à 14 25 46 Capture d’écran 2020-11-12 à 14 25 40

Let's hope they're more accurate than firefox' touch simulators.

@eps1lon eps1lon merged commit a282900 into mui:next Nov 12, 2020
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: tooltip This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tooltip] Long press select text on iOS
4 participants