fix(FormControl): allow required check boxes (#5027)

* fix(FormControl): allow required check boxes in CheckboxGroup

* Create wild-maps-grow.md

* test(vrt): update snapshots

* Docs(FormControl): use TextInput instead of CheckBox in default story

* fix(FormControl): allow required prop on individual checkboxes

* fix(FormControl): use internal warning instead of console.warning

* fix(FormControl): rever console.error label change

---------

Co-authored-by: francinelucca <francinelucca@users.noreply.github.com>

.changeset/wild-maps-grow.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+fix(FormControl): allow required check boxes in CheckboxGroup

packages/react/src/CheckboxGroup/CheckboxGroup.stories.tsx

@@ -100,7 +100,7 @@ Playground.argTypes = {
 export const Default = () => (
   <CheckboxGroup>
     <CheckboxGroup.Label>Choices</CheckboxGroup.Label>
-    <FormControl>
+    <FormControl required>
       <Checkbox value="one" defaultChecked />
       <FormControl.Label>Choice one</FormControl.Label>
     </FormControl>

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<HTMLDivElement, FormControlProps>(
     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, <FormControl>`,
-        )
-      }
-      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, <FormControl>`,
-        )
-      }
-      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, <FormControl>`,
-        )
-      }
+      warning(
+        inputProps?.id,
+        `instead of passing the 'id' prop directly to the input component, it should be passed to the parent component, <FormControl>`,
+      )
+      warning(
+        inputProps?.disabled,
+        `instead of passing the 'disabled' prop directly to the input component, it should be passed to the parent component, <FormControl>`,
+      )
+      warning(
+        inputProps?.required,
+        `instead of passing the 'required' prop directly to the input component, it should be passed to the parent component, <FormControl>`,
+      )
     }
 
     if (!slots.label) {
@@ -92,24 +88,20 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
     }
 
     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<HTMLDivElement, FormControlProps>(
                   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,
                   },
                 )}

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,20 +378,67 @@ 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(
           <FormControl required>
             <FormControl.Label>{LABEL_TEXT}</FormControl.Label>
-            <Checkbox required />
+            <Radio value="radio" name="radio" required />
             <FormControl.Caption>{CAPTION_TEXT}</FormControl.Caption>
           </FormControl>,
         )
 
         expect(consoleSpy).toHaveBeenCalled()
         consoleSpy.mockRestore()
       })
+
+      it('should allow required prop to individual checkbox', async () => {
+        const {getByRole} = render(
+          <FormControl required>
+            <FormControl.Label>{LABEL_TEXT}</FormControl.Label>
+            <Checkbox />
+            <FormControl.Caption>{CAPTION_TEXT}</FormControl.Caption>
+          </FormControl>,
+        )
+
+        expect(getByRole('checkbox')).toBeRequired()
+      })
+
+      it('should not add required prop to individual radio', async () => {
+        const {getByRole} = render(
+          <FormControl required>
+            <FormControl.Label>{LABEL_TEXT}</FormControl.Label>
+            <Radio value="radio" name="radio" />
+            <FormControl.Caption>{CAPTION_TEXT}</FormControl.Caption>
+          </FormControl>,
+        )
+
+        expect(getByRole('radio')).not.toBeRequired()
+      })
+
+      it('should allow required prop on checkbox if part of CheckboxGroup', async () => {
+        const {getByTestId} = render(
+          <CheckboxGroup>
+            <CheckboxGroup.Label>Checkboxes</CheckboxGroup.Label>
+            <FormControl required>
+              <Checkbox value="checkOne" data-testid="checkbox-1" />
+              <FormControl.Label>Checkbox one</FormControl.Label>
+            </FormControl>
+            <FormControl>
+              <Checkbox value="checkTwo" data-testid="checkbox-2" />
+              <FormControl.Label>Checkbox two</FormControl.Label>
+            </FormControl>
+            <FormControl>
+              <Checkbox value="checkThree" />
+              <FormControl.Label>Checkbox three</FormControl.Label>
+            </FormControl>
+          </CheckboxGroup>,
+        )
+
+        expect(getByTestId('checkbox-1')).toBeRequired()
+        expect(getByTestId('checkbox-2')).not.toBeRequired()
+      })
     })
 
     it('should have no axe violations', async () => {