feat(radio-button-group, checkbox-group): add id prop
#7195
Conversation
Added id prop allows consumers to set a custom id on the group component, otherwise it will be set to a randomly generated GUID. `id` prop is needed to set hint text and validation ids for accessibility and testing purposes. fix #7191
…alidation Removes legacy props from being passed to the component with new validation designs.
| </CarbonProvider> | ||
| ); | ||
| }; | ||
|
|
||
| WithNewValidation.args = { | ||
| label: "Radiobutton 1", |
There was a problem hiding this comment.
praise: good idea to make these args so we can change them via the story's controls 👍
| const { current: uniqueId } = useRef(id || guid()); | ||
| const inputHintId = useRef(legendHelp ? `${uniqueId}-hint` : undefined); |
There was a problem hiding this comment.
suggestion: to ensure that uniqueId and inputHintId update if either the legendHelp or id props change, it might be better to compute it outside of the useRef hook:
| const { current: uniqueId } = useRef(id || guid()); | |
| const inputHintId = useRef(legendHelp ? `${uniqueId}-hint` : undefined); | |
| const internalId = useRef(guid()); | |
| const uniqueId = id || internalId.current; | |
| const inputHintId = legendHelp ? `${uniqueId}-hint` : undefined; |
This way, inputHintId will be re-calculated when the component re-renders, making it reactive if legendHelp or id change, rather than being calculated only on the initial render.
| const inputHintId = useRef(legendHelp ? `${uniqueId}-hint` : undefined); | ||
|
|
There was a problem hiding this comment.
suggestion: similarly to RadioButtonGroup, it might be worth us making uniqueId and inputHintId reactive instead of a ref, so it updates to account that either the id or legendHelp props could change.
| const inputHintId = useRef(legendHelp ? `${uniqueId}-hint` : undefined); | |
| const internalId = useRef(guid()); | |
| const uniqueId = id || internalId.current; | |
| const inputHintId = legendHelp ? `${uniqueId}-hint` : undefined; |
| {(error || warning) && ( | ||
| <ErrorBorder warning={!!(!error && warning)} inline={inline} /> | ||
| )} | ||
| <StyledCheckboxGroup | ||
| data-component="checkbox-group" | ||
| data-role="checkbox-group" | ||
| legendInline={legendInline} |
There was a problem hiding this comment.
praise: good spot that this contradicted our documentation 👍
fix #7191
Proposed behaviour
idprop toRadioButtonGroupandCheckboxGroup, if not provided it will be set to a randomly generated GUID. Thisidis used to set hint text and validation ids, needed for accessibility and testing purposes.legendInlineinCheckboxGrouphas no effect with new validation.Current behaviour
RadioButtonGroupandCheckboxGroupdo not supportidprop.legendInlineinCheckboxGrouphas unexpected behaviour with new validation.Checklist
d.tsfile added or updated if requiredQA
Additional context
Testing instructions