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

Improve conditional rendering in Radios #658

Conversation

robpataki
Copy link

Screenshots

ConditionalRadios

@daniel-ac-martin
Copy link
Owner

I can't remember the details but I think there are some issues with this sort of approach and SSR / isomorphic rendering.

My gut instinct is that we should run the conditional reveal off of the DOM's existing state, rather than creating another copy of the state in React. We do this in the CheckBoxes component, but we have to use a bit of a hack to get the component to re-render. - https://github.com/daniel-ac-martin/NotGovUK/blob/master/components/checkboxes/src/Checkbox.tsx#L23

Checkboxes is easier though, as the changes can all be captured on the individual checkbox.
For Radios we might need to might need to supply each of the individual Radio components with a shared onChange handler, to ensure we capture the change. (And then apply the useState hack on the overarching Radios component.)
But I'd need to have a play around to be sure.

@robpataki
Copy link
Author

robpataki commented Jan 24, 2023

Hey @daniel-ac-martin, I've done some digging and I'm back :)

It does indeed seem to be easier with checkboxes, as they trigger the change event, and also change their own internal "checked" attribute when the user checks and unchecks them.

With radios it is a different story - only one change event is ever triggered by a single radio input, and the radio inputs "virtually within" the same radio group don't seem to communicate at all which radio is checked and who is not.
The MDN docs do mention this as the expected behaviour for radios and checkboxes:

Depending on the kind of element being changed and the way the user interacts with the element, the change event fires at a different moment:

  • When a element is checked or unchecked (by clicking or using the keyboard);
  • When a element is checked (but not when unchecked);

I ram a quick test with your components, and I can verify this behaviour, as per the below screen grab:
radios-vs-checkboxes-checked

There does not seem to be a native way to capture radio input "checkedness" on the individual input elements, hence I went for a solution that manages the state on the parent level - in the Radio Group's parent component. This seems to be a popular solution in and outside of React - and certainly for my current project the solution is working fine. I am not aware of the SSR/Isomorphic rendering related issues, can you please share some resources?

Let me know your thoughts.

@robpataki
Copy link
Author

For reference, the HTML specs for radios and checkboxes are here:

I found the MDN explanation on the change event more useful than the HTML spec, but I do find the definition of the radio and checkbox type inputs interesting:

  • Radio: The input element represents a control that, when used in conjunction with other input elements, forms a radio button group in which only one control can have its checkedness state set to true.
  • Checkbox: The input element represents a two-state control that represents the element's checkedness state. If the element's checkedness state is true, the control represents a positive selection, and if it is false, a negative selection.

@robpataki robpataki mentioned this pull request Feb 6, 2023
@daniel-ac-martin
Copy link
Owner

Closing in favour of #695.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants