Create RadioGroup component

[acelaya]

Alternative implementation to #1682, which does not use the RadioButton component from #1681

Add a new RadioGroup component with role="radiogroup" that renders a group of items with role="radio" in it.

This implementation is inspired by how popular component libraries like MUI and Healdess UI do it, but is more simplistic for now.

Todo

• Add tests
• Follow behavior described in https://www.w3.org/WAI/ARIA/apg/patterns/radio/
• Consider removing the items prop and providing radios via <RadioGroup.Radio /> components.

Implementation decisions

1. To simplify the implementation for now, the list of radios is passed via the items={[...]} prop as an array of objects.

   We could alternatively take a more declarative approach as we did with Select, and instead expose a <RadioGroup.Radio /> component that gets the selected value and handlers via context, and lets the rest of the props be passed by the consumers:

   <RadioGroup {...}>
     <RadioGroup.Radio value="1" label="Foo" />
     <RadioGroup.Radio value="2" label="Something" subtitle="Something else but longer" />
     <RadioGroup.Radio value="3" label="Disabled" disabled />
   </RadioGroup>

   EDIT: This is what I finally did, as we found two benefits for this approach:

   • More customizable children.
   • More flexibility when arranging radios without the limitations of direction prop.

2. I initially had the intention to build this component on top of RadioButton (see Create RadioButtonGroup component #1682), but once I got hands-on, I realized it was impractical for a few reasons:

   • Every radio component needs to be more complex than the RadioButton allows. This forces the use of role="radio" so that it can be focused and interacted with the keyboard, but then we end up with a radio containing another radio, which I don't think is correct from an accessibility point of view.
   • A workaround for the above would involve allowing a more customizable layout in RadioButton, via classes or similar, but it feels like a hack just for the sake of using RadioButton.
   • RadioButton provides its own disabling capability via disabled prop, which among other things adds a 70% opacity to the component. If we do something similar in the wrapper, we would end up adding-up the 70% opacity twice just in part of the component, making it look like part of it is "more disabled".

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (530e958) to head (306653f).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #1683   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        66    +2     
  Lines         1084      1114   +30     
  Branches       420       434   +14     
=========================================
+ Hits          1084      1114   +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[robertknight]

To simplify the implementation for now, the list of radios is passed via the items={[...]} prop as an array of objects.

The prop is called inputs, which seems obvious enough (inputs, items and options would all work). The main restriction of doing it this way is that you can't add extra content to children. Do we have a use case for that?

[acelaya]

To simplify the implementation for now, the list of radios is passed via the items={[...]} prop as an array of objects.

The prop is called inputs, which seems obvious enough (inputs, items and options would all work). The main restriction of doing it this way is that you can't add extra content to children. Do we have a use case for that?

Not as far as I know

[robertknight]

Generally this looks very good. Some notes:

• The US spelling of "Labeled" as a single "l". We should use that everywhere. aria-labelledby is a mistake (see links in my comments)
• Setting inputs via an array will work for the moment, but it means we can't add custom content to items, so we may have to revise the API if that use case comes up
• The intro of the documentation needs a revision. I suggest we link to the APG pattern for cases where our components are implementations of those patterns. These guides describe the purpose, expected keyboard behavior and semantic labeling. If we choose to deviate from aspects of a pattern, we should note why.

[robertknight]

Keyboard navigation works as expected when the radio group is focused using tab, but I found that in Safari if I focused an item using the mouse and then navigated using the keyboard, the focus ring was not always rendered. In Chrome the focus ring is rendered as expected. I think this issue could be deferred to a subsequent PR.

[acelaya]

• The intro of the documentation needs a revision. I suggest we link to the APG pattern for cases where our components are implementations of those patterns. These guides describe the purpose, expected keyboard behavior and semantic labeling. If we choose to deviate from aspects of a pattern, we should note why.

Good point. I see some differences on how it is described there and the behavior I implemented. I will fix those.

[robertknight]

LGTM 👍