feat: improve RadioButton and RadioButtonGroup types
#16648
Conversation
Changes: - Export prop interfaces for `RadioButton` and `RadioButtonGroup`. - Narrow `onChange` argument type for `RadioButtonGroupProps`. - Reference `RadioButtonProps` for `RadioButtonGroup` types where we are expecting a `RadioButton` value. - Add missing types for component `refs` and event handlers in `RadioButton` and `RadioButtonGroup`. - Simplify `getRadioButtons()` with a type assertion on `children` argument and refactoring to use an early return statement.
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -98,80 +98,85 @@ export interface RadioButtonProps | |||
| required?: boolean; | |||
| } | |||
|
|
|||
| const RadioButton = React.forwardRef((props: RadioButtonProps, ref) => { | |||
There was a problem hiding this comment.
ref is now correctly typed.
Most of the following diff below is due to prettier's formatting adjusting the indentation. I recommend viewing with ⚙️ whitespace changes hidden.
| value: RadioButtonProps['value'], | ||
| name: RadioButtonProps['name'], |
There was a problem hiding this comment.
We can directly reference other types within the same interface when the expected value is the same (e.g., props.name and props.value are what we're passing to the onChange handler).
Referencing types this way provides more context and reduces the work later, should RadioButton types change in the future.
This is also how it was originally typed (See DefinitelyTyped)
| const innerLabelClasses = classNames( | ||
| `${prefix}--radio-button__label-text`, | ||
| { | ||
| [`${prefix}--visually-hidden`]: hideLabel, | ||
| } | ||
| ); |
There was a problem hiding this comment.
This was an automatic prettier adjustment.
| const uid = useId('radio-button'); | ||
| const uniqueId = id || uid; | ||
|
|
||
| function handleOnChange(event: React.ChangeEvent<HTMLInputElement>) { |
There was a problem hiding this comment.
As a bonus, I added some missing types while working in these files.
| @@ -87,10 +88,11 @@ export interface RadioButtonGroupProps | |||
| * the group changes | |||
| */ | |||
| onChange?: ( | |||
| selection: React.ReactNode, | |||
There was a problem hiding this comment.
The selection will be the same as the RadioButton value. ReactNode is too broad.
| if (!selected && (radioButton as ReactElement)?.props.checked) { | ||
| newProps.checked = true; | ||
| const mappedChildren = React.Children.map( | ||
| children as ReactElement<RadioButtonProps>, |
There was a problem hiding this comment.
children is now typed as a RadioButton allowing us to inherit the types from RadioButtonProps. Previously, all prop values referenced in this function were typed as any.
This way, we no longer need to keep asserting the type of radioButton.
| if (!radioButton) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The null-check from 🔴 184 was moved up here as an early return.
| @@ -230,7 +241,7 @@ const RadioButtonGroup = React.forwardRef( | |||
| const divRef = useRef<HTMLDivElement>(null); | |||
|
|
|||
| // Slug is always size `mini` | |||
| let normalizedSlug; | |||
| let normalizedSlug: ReactElement | undefined; | |||
There was a problem hiding this comment.
This can be done across all components for Type Safety and Clarity
e582548
In #16186, the
selectionargument ofRadioButtonGroup'sonChangeprop was changed toReactNodeinstead ofstring | numberas written in the PR description. In addition to fixing that I have added some missing types and improved others.Changelog
RadioButtonandRadioButtonGroup.onChangeargument type forRadioButtonGroupProps.RadioButtonPropsforRadioButtonGrouptypes where we are expecting aRadioButtonvalue.refsand event handlers inRadioButtonandRadioButtonGroup.getRadioButtons()with a type assertion onchildrenargument and refactoring to use an early return statement.Testing / Reviewing
RadioButtonandRadioButtonGroupfor type errorsRadioButtonsused inside of a group (e.g., radio buttons are still correctly displayed and the group value changes when radio selections change.)