-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[autocomplete] Allow tooltip text selection #36503
[autocomplete] Allow tooltip text selection #36503
Conversation
Netlify deploy previewhttps://deploy-preview-36503--material-ui.netlify.app/ Bundle size report |
I was trying to write an other test to check the user's onMouse text highlight/selection behavior but was not successful. |
@@ -1007,13 +1007,25 @@ export default function useAutocomplete(props) { | |||
|
|||
// Prevent input blur when interacting with the combobox | |||
const handleMouseDown = (event) => { | |||
const isToolTipElement = event.target?.parentElement?.getAttribute('role')?.includes('tooltip'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this check is too specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, see totally your point.
I'm working on it and looking at that other issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But regards to this issue only, I made a change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliviertassinari in case you missed it, I just made it general in a way that if user clicks anywhere outside the Autocomplete component, it prevents focusing. so then user is able to select text for instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change makes sense to me 👌 cc @michaldudak
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, seems OK to me.
Co-authored-by: Amirreza safehian <amirreza@RIV-D421TWWQ9V.local>
Fix #33195
Codesandbox: https://codesandbox.io/s/inputadornments-demo-material-ui-forked-j0gvpo?file=/demo.tsx