diff --git a/.changeset/wild-maps-grow.md b/.changeset/wild-maps-grow.md new file mode 100644 index 00000000000..e3b514572c5 --- /dev/null +++ b/.changeset/wild-maps-grow.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +fix(FormControl): allow required check boxes in CheckboxGroup diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-colorblind-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-colorblind-linux.png index b880220d43f..bf302ee24bb 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-colorblind-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-dimmed-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-dimmed-linux.png index 1fde5aaa284..3a9224423ab 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-dimmed-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-high-contrast-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-high-contrast-linux.png index 218388f0412..7da7e53386a 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-high-contrast-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-linux.png index b880220d43f..bf302ee24bb 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-tritanopia-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-tritanopia-linux.png index b880220d43f..bf302ee24bb 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-tritanopia-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-colorblind-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-colorblind-linux.png index effa238bfe5..fbcb27c49e0 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-colorblind-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-high-contrast-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-high-contrast-linux.png index 56b4431cfa3..4f4d140484e 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-high-contrast-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-linux.png index effa238bfe5..973fb44a7de 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-linux.png differ diff --git a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-tritanopia-linux.png b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-tritanopia-linux.png index effa238bfe5..fbcb27c49e0 100644 Binary files a/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-tritanopia-linux.png and b/.playwright/snapshots/components/CheckboxGroup.test.ts-snapshots/CheckboxGroup-Default-light-tritanopia-linux.png differ diff --git a/packages/react/src/CheckboxGroup/CheckboxGroup.stories.tsx b/packages/react/src/CheckboxGroup/CheckboxGroup.stories.tsx index ac38b1af2b2..cc40e5f3595 100644 --- a/packages/react/src/CheckboxGroup/CheckboxGroup.stories.tsx +++ b/packages/react/src/CheckboxGroup/CheckboxGroup.stories.tsx @@ -100,7 +100,7 @@ Playground.argTypes = { export const Default = () => ( Choices - + Choice one diff --git a/packages/react/src/FormControl/FormControl.tsx b/packages/react/src/FormControl/FormControl.tsx index 95129dc516f..257163c753a 100644 --- a/packages/react/src/FormControl/FormControl.tsx +++ b/packages/react/src/FormControl/FormControl.tsx @@ -18,6 +18,7 @@ import FormControlLabel from './_FormControlLabel' import FormControlLeadingVisual from './_FormControlLeadingVisual' import FormControlValidation from './_FormControlValidation' import {FormControlContextProvider} from './_FormControlContext' +import {warning} from '../utils/warning' export type FormControlProps = { children?: React.ReactNode @@ -62,26 +63,21 @@ const FormControl = React.forwardRef( const inputProps = React.isValidElement(InputComponent) && InputComponent.props const isChoiceInput = React.isValidElement(InputComponent) && (InputComponent.type === Checkbox || InputComponent.type === Radio) + const isRadioInput = React.isValidElement(InputComponent) && InputComponent.type === Radio if (InputComponent) { - if (inputProps?.id) { - // eslint-disable-next-line no-console - console.warn( - `instead of passing the 'id' prop directly to the input component, it should be passed to the parent component, `, - ) - } - if (inputProps?.disabled) { - // eslint-disable-next-line no-console - console.warn( - `instead of passing the 'disabled' prop directly to the input component, it should be passed to the parent component, `, - ) - } - if (inputProps?.required) { - // eslint-disable-next-line no-console - console.warn( - `instead of passing the 'required' prop directly to the input component, it should be passed to the parent component, `, - ) - } + warning( + inputProps?.id, + `instead of passing the 'id' prop directly to the input component, it should be passed to the parent component, `, + ) + warning( + inputProps?.disabled, + `instead of passing the 'disabled' prop directly to the input component, it should be passed to the parent component, `, + ) + warning( + inputProps?.required, + `instead of passing the 'required' prop directly to the input component, it should be passed to the parent component, `, + ) } if (!slots.label) { @@ -92,24 +88,20 @@ const FormControl = React.forwardRef( } if (isChoiceInput) { - if (slots.validation) { - // eslint-disable-next-line no-console - console.warn( - 'Validation messages are not rendered for an individual checkbox or radio. The validation message should be shown for all options.', - ) - } + warning( + !!slots.validation, + 'Validation messages are not rendered for an individual checkbox or radio. The validation message should be shown for all options.', + ) - if (childrenWithoutSlots.find(child => React.isValidElement(child) && child.props?.required)) { - // eslint-disable-next-line no-console - console.warn('An individual checkbox or radio cannot be a required field.') - } + warning( + isRadioInput && childrenWithoutSlots.find(child => React.isValidElement(child) && child.props?.required), + 'An individual radio cannot be a required field.', + ) } else { - if (slots.leadingVisual) { - // eslint-disable-next-line no-console - console.warn( - 'A leading visual is only rendered for a checkbox or radio form control. If you want to render a leading visual inside of your input, check if your input supports a leading visual.', - ) - } + warning( + !!slots.leadingVisual, + 'A leading visual is only rendered for a checkbox or radio form control. If you want to render a leading visual inside of your input, check if your input supports a leading visual.', + ) } const isLabelHidden = slots.label?.props.visuallyHidden @@ -138,11 +130,14 @@ const FormControl = React.forwardRef( InputComponent as React.ReactElement<{ id: string disabled: boolean + required: boolean ['aria-describedby']: string }>, { id, disabled, + // allow checkboxes to be required + required: required && !isRadioInput, ['aria-describedby']: captionId as string, }, )} diff --git a/packages/react/src/__tests__/FormControl.test.tsx b/packages/react/src/__tests__/FormControl.test.tsx index e3d9e50b86e..3adcdfc5dda 100644 --- a/packages/react/src/__tests__/FormControl.test.tsx +++ b/packages/react/src/__tests__/FormControl.test.tsx @@ -5,7 +5,9 @@ import axe from 'axe-core' import { Autocomplete, Checkbox, + CheckboxGroup, FormControl, + Radio, Select, Textarea, TextInput, @@ -376,13 +378,13 @@ describe('FormControl', () => { consoleSpy.mockRestore() }) - it('should warn users if they pass `required` to a checkbox or radio', async () => { + it('should warn users if they pass `required` to a radio', async () => { const consoleSpy = jest.spyOn(global.console, 'warn').mockImplementation() render( {LABEL_TEXT} - + {CAPTION_TEXT} , ) @@ -390,6 +392,53 @@ describe('FormControl', () => { expect(consoleSpy).toHaveBeenCalled() consoleSpy.mockRestore() }) + + it('should allow required prop to individual checkbox', async () => { + const {getByRole} = render( + + {LABEL_TEXT} + + {CAPTION_TEXT} + , + ) + + expect(getByRole('checkbox')).toBeRequired() + }) + + it('should not add required prop to individual radio', async () => { + const {getByRole} = render( + + {LABEL_TEXT} + + {CAPTION_TEXT} + , + ) + + expect(getByRole('radio')).not.toBeRequired() + }) + + it('should allow required prop on checkbox if part of CheckboxGroup', async () => { + const {getByTestId} = render( + + Checkboxes + + + Checkbox one + + + + Checkbox two + + + + Checkbox three + + , + ) + + expect(getByTestId('checkbox-1')).toBeRequired() + expect(getByTestId('checkbox-2')).not.toBeRequired() + }) }) it('should have no axe violations', async () => {