Add radio/checkbox group accessibility#2739
Conversation
Fix radio group doc example to use display toggles
| * Wraps the group in a `EuiFormFieldset` which adds an `EuiLegend` for titling the whole group. | ||
| * Accepts an `EuiFormLegendProps` shape. | ||
| */ | ||
| legend?: EuiFormLegendProps; |
There was a problem hiding this comment.
Should legends be required for radio groups?
There was a problem hiding this comment.
I don't think it's necessary to break current implementations for an additive property. It's possible consumers could have already fixed this with their own implementation of fieldset and legend.
There was a problem hiding this comment.
If they've already fixed it for their own solutions, then it's likely it'd be an easy migration to this new form. If you're worried about it not being, we could allow legend={{}} as a valid option that effectively does nothing other than prove you (an implementing developer) thought about the legend.
I just worry about us adding these capabilities in building accessible pages but people not taking advantage of them because they don't see the benefit themselves without a forcing mechanism.
There was a problem hiding this comment.
Our strategy is to be as out-of-the-way as possible with updates and be pro-engineer rather than pro-feature with updates (regardless of what that feature or bug fix is). It's easy when you think about ONLY fixing Kibana. It's not so simple when we talk about ANY site using EUI. We can target a deprecation if we want and change it at a later time, but we always give six months for that kind of thing.
This is a very broad team strategy we've used to remove friction and keep people happy and using EUI in general.
elizabetdev
left a comment
There was a problem hiding this comment.
Tested locally and everything looks good to me! 🎉
Allow passing
legendprop to wrap these groups in EuiFormFieldsetTo properly label groups of radio buttons, there needs to be a fieldset and legend (even if that legend is visually hidden). This PR adds a
legendprop that accepts the same object as EuiFormFieldset.legend and if provided will wrap the radios in this component otherwise it just returns a div as before.I added this as an option for checkboxes as well since checkbox groups could have an over-arching label but by a11y rules an input cannot have 2
<label>elements attached to it. Therefore it needs to be alegend.Which will also help with #2493
Incidentals
During this addition, there were a couple fixes/enhancements made to these components.
1. Radio and checkbox labels are now
inline-block. Before, the label was block display meaning even the white space on the right was clickable. This could have caused unintentional clicks by the user.2. Extended the EuiCheckboxGroup.options type to extend EuiCheckbox. Before, the options would only accept
id, label, disabled. Now consumers can pass anything accepted by EuiCheckbox (including data-test-subj).Checklist
[ ] Checked for breaking changes and labeled appropriately