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

[Checkbox] Fix form submission with empty value #28423

Merged
merged 2 commits into from
Sep 23, 2021
Merged

[Checkbox] Fix form submission with empty value #28423

merged 2 commits into from
Sep 23, 2021

Conversation

garronej
Copy link
Contributor

@garronej garronej commented Sep 18, 2021

Hello,

Mui checkboxes, when used in a <form>, don't submit their values.

Screen.Recording.2021-09-18.at.15.58.09.mov

Playground link

This is due to the fact that, if we put a value prop to the <input />, even if it's undefined, it ends up as an html property and at the time of submiting the form will be interpreted as a void string overriding the checked value.

This PR fixes it but maybe it would be better to fix it upstream.

Best regards,

PS: The new landing page is very cool.

@mui-pr-bot
Copy link

mui-pr-bot commented Sep 18, 2021

Details of bundle changes

Generated by 🚫 dangerJS against 6552202

@oliviertassinari
Copy link
Member

@garronej Cool. Could you add a test case for it? With the unstyled effort, we will likely refactor this part of the codebase.

@oliviertassinari oliviertassinari changed the title Fixes checkboxes [Checkbox] Fix form submission withe empty value Sep 18, 2021
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: checkbox This is the name of the generic UI component, not the React module! labels Sep 18, 2021
@eps1lon eps1lon added the PR: needs test The pull request can't be merged label Sep 19, 2021
@garronej
Copy link
Contributor Author

Cool. Could you add a test case for it?

@oliviertassinari done 👍🏻

@oliviertassinari oliviertassinari removed the PR: needs test The pull request can't be merged label Sep 21, 2021
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.

Thanks for the test case

@oliviertassinari oliviertassinari changed the title [Checkbox] Fix form submission withe empty value [Checkbox] Fix form submission with empty value Sep 23, 2021
@oliviertassinari oliviertassinari merged commit e1ec7e1 into mui:master Sep 23, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 23, 2021

Sebastian is off, merging as the change is well tested (it fails without the fix) and there is a clear reproduction. @garronej Thanks, well done

@eps1lon
Copy link
Member

eps1lon commented Sep 27, 2021

Referencing facebook/react#17590 here which is why we need this workaround.

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: checkbox 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.

None yet

5 participants