-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Render hidden form input fields for Checkbox
, Switch
and RadioGroup
components
#3095
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
68f0913
to
e3d1a5d
Compare
By default, the hidden form fields render an `<input type="hidden" />`. However, we have some components where we explicitly want to change the type for example `checkbox` or `radio` types. I first tried to encode this information in the data itself. That requires us to use symbols so that we don't accidentally choose a key that actually exists in the data. An overrides key solves this for our use cases. The component is also internal, so nothing is exposed to the user.
In case of checkboxes and radio elements, we will only render the hidden inputs if: 1. You pass a `name` prop to make it form-aware 2. When the checkbox / radio is "checked" This is now changed to always render the hidden input as long as the `name` prop is passed to make it form-aware.
e3d1a5d
to
7037b35
Compare
Checkbox
, Switch
and RadioGroup
components
@@ -315,7 +315,8 @@ function RadioGroupFn<TTag extends ElementType = typeof DEFAULT_RADIO_GROUP_TAG, | |||
{name != null && ( | |||
<FormFields | |||
disabled={disabled} | |||
data={value != null ? { [name]: value || 'on' } : {}} | |||
data={{ [name]: value || 'on' }} |
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 know it doesn't technically matter but it seems weird to me that this produces a hidden radio input where the value="on" (it's not checked so it's not going to show up in form data).
I feel like I would've expected to see three radios in the DOM with the same name, the three values, and only the appropriate one checked. Instead of mutating a single radio.
Maybe it doesn't matter though? Thoughts?
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.
It's a bit odd for sure, but if you submit a form with <input type="radio" name="foo" />
without a value, then it will be submitted with a value of "on"
.
We could try to move the hidden inputs to each Radio
component itself, but only the checked ones will be submitted anyway so this feels better. Then we don't have to render n
hidden radio inputs.
This PR ensures that we always render the hidden form input elements for the
Checkbox
,Switch
andRadioGroup
components.The reason why we didn't render them was because they weren't included in the
FormData
anyway as long as they are not "checked". However, this makes it harder if you want to get access to the elements if you use some form library that uses the inputs in the form. Additionally, this also means that we were adding and removing elements from the DOM.Now with this change, they will always be there and the
checked
attribute will be updated behind the scenes instead. Now, the hidden inputs will always be available in the form data.Note: if you don't pass the
name
prop, the hidden form inputs won't be rendered (same behavior as before).Fixes: #2977