-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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] Fix running click event on disabled #36892
Conversation
Netlify deploy previewhttps://deploy-preview-36892--material-ui.netlify.app/ Bundle size report |
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.
👍 Thanks, sorry I didn't get to this before this week's release 🙏 @sai6855
@mj12albert no problem :) |
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 suspect #36867 should be fixed in the Autocomplete, hence this PR is not going in the right direction: #36892 (comment).
if (onClick) { | ||
if (onClick && !fcs.disabled) { |
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.
Why? It's not clear if it's the desired behavior:
export default function App() {
return (
<div
onClick={() => {
console.log("root");
}}
>
<input
disabled
onClick={() => {
console.log("input");
}}
/>
</div>
);
}
https://codesandbox.io/s/amazing-banzai-xc225m?file=/src/App.js
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.
🤔 This looked ok to me as a plain <input>
doesn't fire onClick
when it's disabled: https://codesandbox.io/s/async-https-n9x4y3-onclick-disabled-n9x4y3 @oliviertassinari
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 there can be startAdornment, endAdornment, and renderSuffix. IMHO, we shouldn't try to make InputBase behave like an input because it's not: it's a root > input
like structure. Instead, if we were to stay close to the normal DOM behavior we would avoid surprises for people that are familiar with the platform behavior.
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.
we shouldn't try to make InputBase behave like an input because it's not
OK this makes sense 👌 will try to find a fix within Autocomplete
This reverts commit 9118505.
This reverts commit 9118505.
To complement, #36892 (comment). I didn't realize this also changed A clear example: import * as React from "react";
import TextField from "@mui/material/TextField";
export default function BasicTextFields() {
return (
<TextField
onClick={() => {
console.log("textField");
}}
helperText="hello"
/>
);
} https://codesandbox.io/s/autumn-http-cn5nrc?file=/Demo.tsx:0-257 clicking on the helper text doesn't trigger the onClick listener anymore ❌. |
This reverts commit 9118505.
closes #36867
before : https://codesandbox.io/s/zen-meadow-lr2flp?file=/demo.tsx
after : https://codesandbox.io/s/beautiful-panka-24o4h2?file=/demo.tsx
Actual issue lies in
TextField
component not inAutocomplete
. Right nowonClick
event ofTextField
runs even ifTextField
is disabled. This PR fixes it.