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

[joy-ui][Radio][Input] Fix inheritance of disabled prop #39934

Merged
merged 3 commits into from
Dec 11, 2023

Conversation

sai6855
Copy link
Contributor

@sai6855 sai6855 commented Nov 20, 2023

Before

https://codesandbox.io/s/vigorous-leaf-l4ynj3?file=/src/index.tsx

After

https://codesandbox.io/s/elastic-sunset-7fykcs?file=/src/Demo.tsx

In both Input and Radio disabled prop from theme were taking precedence over disabled prop from FormControl. This PR fixes the issue.

Sorry, something went wrong.

@sai6855 sai6855 marked this pull request as draft November 20, 2023 09:19
@mui-bot
Copy link

mui-bot commented Nov 20, 2023

Netlify deploy preview

https://deploy-preview-39934--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 0ab5742

@@ -302,6 +302,7 @@ const Radio = React.forwardRef(function Radio(inProps, ref) {
const name = inProps.name || radioGroup?.name || nameProp;
const disableIcon = inProps.disableIcon || radioGroup?.disableIcon || disableIconProp;
const overlay = inProps.overlay || radioGroup?.overlay || overlayProp;
const isDisabled = inProps.disabled || formControl?.disabled || disabledProp;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const isDisabled = inProps.disabled || formControl?.disabled || disabledProp;
const disabled = inProps.disabled || formControl?.disabled || disabledProp;

Let's keep disabled as a const name, we never use is as a prefix.

Copy link
Contributor Author

@sai6855 sai6855 Nov 20, 2023

Choose a reason for hiding this comment

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

there is already a variable named disabled, hence went with isDisabled. if not for isDisabled what do you suggest?. I made logic inline like before.

const { getInputProps, checked, disabled, focusVisible } = useSwitch(useRadioProps);
.

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I left one comment on the naming, the logic looks good. Have you checked if this is implemented across the other input components?

@mnajdova mnajdova added bug 🐛 Something doesn't work component: radio This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy labels Nov 20, 2023
@sai6855 sai6855 changed the title [joy-ui][Radio] Fix inheritance of disabled prop [joy-ui][Radio][Input] Fix inheritance of disabled prop Nov 20, 2023
@sai6855
Copy link
Contributor Author

sai6855 commented Nov 20, 2023

Have you checked if this is implemented across the other input components?

Switch, Checkbox,Select were looking good. Input was also having same issue, fixed in this commit 015b614

@sai6855 sai6855 marked this pull request as ready for review November 20, 2023 10:34
@danilo-leal danilo-leal merged commit 303d2b6 into mui:master Dec 11, 2023
@oliviertassinari oliviertassinari added the component: text field This is the name of the generic UI component, not the React module! label Nov 12, 2024
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: radio This is the name of the generic UI component, not the React module! component: text field This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants