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

[TextField][InputLabel] Remove pointer-events: none property #30493

Merged
merged 4 commits into from
Jan 10, 2022

Conversation

hbjORbj
Copy link
Member

@hbjORbj hbjORbj commented Jan 4, 2022

Closes #30488

Issue:

  • When using TextField and its InputLabelProps prop, which is passed to InputLabel component, event handlers (e.g., onClick, onFocus) passed to InputLabelProps don't work.
  • Code sandbox

Soluition:

@hbjORbj hbjORbj added bug 🐛 Something doesn't work package: material-ui Specific to @mui/material labels Jan 4, 2022
@mui-pr-bot
Copy link

mui-pr-bot commented Jan 4, 2022

Details of bundle changes

Generated by 🚫 dangerJS against ae3f9ce

@hbjORbj hbjORbj changed the title [InputLabel] Remove pointer-events: none property [TextField][InputLabel] Remove pointer-events: none property Jan 4, 2022
@hbjORbj hbjORbj changed the title [TextField][InputLabel] Remove pointer-events: none property [InputLabel] Remove pointer-events: none property Jan 4, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 4, 2022

👍 for doing the investigation work on why this style is here in the first place. However, while in v5 we now correctly use the for attribute to avoid the initial bug, we added the for to fix an a11y issue, it seems to be not enough. I see two changes in the new behavior:

  1. On hover, we don't set the hover CSS:

hover

  1. The textbox is no longer focused on mousedown.

I would classify these two as regressions, at least very close to.

In https://deploy-preview-30493--material-ui.netlify.app/components/autocomplete/#combo-box, it's definitely a regression, the popup does no longer open on focus.

Looking at #30488 "I want to render a Tooltip wrapped help icon on the label of the outlined input.", this seems like a OK-ish use case. It's not the best UX I would argue (=> use the helper text), but still valid. What might work is to only apply this pointer-events: none CSS if the label is shrunk. Another option: we label the issue as "won't fix" and say, use the helper text, could work too I think.

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

As @oliviertassinari mentioned, hover does not display style is a bug.

@hbjORbj
Copy link
Member Author

hbjORbj commented Jan 6, 2022

@oliviertassinari @siriwatknp

What might work is to only apply this pointer-events: none CSS if the label is shrunk.

I think this is a reasonable compromise. Please check out codesandbox.

@hbjORbj hbjORbj changed the title [InputLabel] Remove pointer-events: none property [TextField][InputLabel] Remove pointer-events: none property Jan 6, 2022
@hbjORbj hbjORbj added the component: text field This is the name of the generic UI component, not the React module! label Jan 6, 2022
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Nice fix.

@siriwatknp siriwatknp merged commit 22c14e3 into mui:master Jan 10, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 10, 2022

Screenshot 2022-01-10 at 21 47 51

https://deploy-preview-30493--material-ui.netlify.app/components/text-fields/#customization

wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
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: text field This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
4 participants