Let FormControl accept any input (#1968)

* renders any non-FormControl components passed into FormControl in the spot we'd normally render a Primer input component * adds changeset * appease the linter * Update docs/content/FormControl.mdx Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com> * addresses PR feedback Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
primer · Mar 16, 2022 · 1b01485 · 1b01485
1 parent 0487ecb
commit 1b01485
Show file tree

Hide file tree

Showing 9 changed files with 137 additions and 74 deletions.
diff --git a/.changeset/spotty-eagles-help.md b/.changeset/spotty-eagles-help.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Instead of rendering unexpected FormControl children before the rest of the content, we render them in the same spot we'd normally render a Primer input component
diff --git a/docs/content/FormControl.mdx b/docs/content/FormControl.mdx
@@ -91,6 +91,89 @@ const DifferentInputs = () => {
 render(DifferentInputs)
 ```
 
+### With a custom input
+
+<Note variant="warning">
+
+When rendering an input other than a form component from Primer, you must manually pass the attributes that make the form control accessible:
+
+- The input should have an ID
+- `FormControl.Label` should be associated with the text input by using `htmlFor`
+- If there is a caption, the input should be associated with the caption by passing the message's ID to `aria-describedby`
+- If there is a validation message, the input should be associated with the message by passing the message's ID to `aria-describedby`
+- If there is both a caption and a validation message, the input should be associated with the message by passing the both the validation message's ID and the caption's ID to `aria-describedby`. Example: `aria-describedby="caption-id validation-id"`
+- If the input's value is invalid, `aria-invalid={true}` should be passed to the input.
+- If the input is disabled, `disabled` should be passed.
+- If the input is required, `required` should be passed.
+
+When rendering a custom checkbox or radio component, you must also pass `layout="horizontal"` to the `FormControl` component.
+
+</Note>
+
+```javascript live noinline
+const CustomTextInput = props => <input type="text" {...props} />
+const CustomCheckboxInput = props => <input type="checkbox" {...props} />
+const FormControlWithCustomInput = () => {
+  const [value, setValue] = React.useState('mona lisa')
+  const [validationResult, setValidationResult] = React.useState()
+  const doesValueContainSpaces = inputValue => /\s/g.test(inputValue)
+  const handleInputChange = e => {
+    setValue(e.currentTarget.value)
+  }
+
+  React.useEffect(() => {
+    if (doesValueContainSpaces(value)) {
+      setValidationResult('noSpaces')
+    } else if (value) {
+      setValidationResult('validName')
+    }
+  }, [value])
+
+  return (
+    <Box display="grid" gridGap={3}>
+      <FormControl>
+        <FormControl.Label htmlFor="custom-input">GitHub handle</FormControl.Label>
+        <CustomTextInput
+          id="custom-input"
+          aria-describedby="custom-input-caption custom-input-validation"
+          aria-invalid={validationResult === 'noSpaces'}
+          onChange={handleInputChange}
+        />
+        {validationResult === 'noSpaces' && (
+          <FormControl.Validation id="custom-input-validation" variant="error">
+            GitHub handles cannot contain spaces
+          </FormControl.Validation>
+        )}
+        {validationResult === 'validName' && (
+          <FormControl.Validation id="custom-input-validation" variant="success">
+            Valid name
+          </FormControl.Validation>
+        )}
+        <FormControl.Caption id="custom-input-caption">
+          With or without "@". For example "monalisa" or "@monalisa"
+        </FormControl.Caption>
+      </FormControl>
+
+      <CheckboxGroup>
+        <CheckboxGroup.Label>Checkboxes</CheckboxGroup.Label>
+        <FormControl layout="horizontal">
+          <CustomCheckboxInput id="custom-checkbox-one" value="checkOne" />
+          <FormControl.Label htmlFor="custom-checkbox-one">Checkbox one</FormControl.Label>
+          <FormControl.Caption id="custom-checkbox-one-caption">Hint text for checkbox one</FormControl.Caption>
+        </FormControl>
+        <FormControl layout="horizontal">
+          <CustomCheckboxInput id="custom-checkbox-two" value="checkTwo" />
+          <FormControl.Label htmlFor="custom-checkbox-two">Checkbox two</FormControl.Label>
+          <FormControl.Caption id="custom-checkbox-two-caption">Hint text for checkbox two</FormControl.Caption>
+        </FormControl>
+      </CheckboxGroup>
+    </Box>
+  )
+}
+
+render(FormControlWithCustomInput)
+```
+
 ### With checkbox and radio inputs
 
 ```jsx live

diff --git a/src/FormControl/FormControl.tsx b/src/FormControl/FormControl.tsx
@@ -24,6 +24,11 @@ export type FormControlProps = {
    * If true, the user must specify a value for the input before the owning form can be submitted
    */
   required?: boolean
+  /**
+   * The direction the content flows.
+   * Vertical layout is used by default, and horizontal layout is used for checkbox and radio inputs.
+   */
+  layout?: 'horizontal' | 'vertical'
 } & SxProp
 
 export interface FormControlContext extends Pick<FormControlProps, 'disabled' | 'id' | 'required'> {
@@ -32,7 +37,7 @@ export interface FormControlContext extends Pick<FormControlProps, 'disabled' |
 }
 
 const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
-  ({children, disabled: disabledProp, id: idProp, required, sx}, ref) => {
+  ({children, disabled: disabledProp, layout, id: idProp, required, sx}, ref) => {
     const expectedInputComponents = [Autocomplete, Checkbox, Radio, Select, TextInput, TextInputWithTokens, Textarea]
     const choiceGroupContext = useContext(CheckboxOrRadioGroupContext)
     const disabled = choiceGroupContext?.disabled || disabledProp
@@ -56,20 +61,7 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
     const isChoiceInput =
       React.isValidElement(InputComponent) && (InputComponent.type === Checkbox || InputComponent.type === Radio)
 
-    if (!InputComponent) {
-      // eslint-disable-next-line no-console
-      console.warn(
-        `To correctly render this field with the correct ARIA attributes passed to the input, please pass one of the component from @primer/react as a direct child of the FormControl component: ${expectedInputComponents.reduce(
-          (acc, componentName) => {
-            acc += `\n- ${componentName.displayName}`
-
-            return acc
-          },
-          ''
-        )}`,
-        'If you are using a custom input component, please be sure to follow WCAG guidelines to make your form control accessible.'
-      )
-    } else {
+    if (InputComponent) {
       if (inputProps?.id) {
         // eslint-disable-next-line no-console
         console.warn(
@@ -135,7 +127,7 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
         {slots => {
           const isLabelHidden = React.isValidElement(slots.Label) && slots.Label.props.visuallyHidden
 
-          return isChoiceInput ? (
+          return isChoiceInput || layout === 'horizontal' ? (
             <Box ref={ref} display="flex" alignItems={slots.LeadingVisual ? 'center' : undefined} sx={sx}>
               <Box sx={{'> input': {marginLeft: 0, marginRight: 0}}}>
                 {React.isValidElement(InputComponent) &&
@@ -183,13 +175,8 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
               display="flex"
               flexDirection="column"
               width="100%"
-              sx={{...(isLabelHidden ? {'> *:not(label) + *': {marginTop: 2}} : {'> * + *': {marginTop: 2}}), ...sx}}
+              sx={{...(isLabelHidden ? {'> *:not(label) + *': {marginTop: 1}} : {'> * + *': {marginTop: 1}}), ...sx}}
             >
-              {React.Children.toArray(children).filter(
-                child =>
-                  React.isValidElement(child) &&
-                  !expectedInputComponents.some(inputComponent => child.type === inputComponent)
-              )}
               {slots.Label}
               {React.isValidElement(InputComponent) &&
                 React.cloneElement(InputComponent, {
@@ -199,6 +186,11 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
                   validationStatus,
                   ['aria-describedby']: [validationMessageId, captionId].filter(Boolean).join(' ')
                 })}
+              {React.Children.toArray(children).filter(
+                child =>
+                  React.isValidElement(child) &&
+                  !expectedInputComponents.some(inputComponent => child.type === inputComponent)
+              )}
               {validationChild && <ValidationAnimationContainer show>{slots.Validation}</ValidationAnimationContainer>}
               {slots.Caption}
             </Box>
@@ -209,6 +201,10 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
   }
 )
 
+FormControl.defaultProps = {
+  layout: 'vertical'
+}
+
 export default Object.assign(FormControl, {
   Caption: FormControlCaption,
   Label: FormControlLabel,

diff --git a/src/FormControl/_FormControlCaption.tsx b/src/FormControl/_FormControlCaption.tsx
@@ -4,10 +4,10 @@ import InputCaption from '../_InputCaption'
 import {FormControlContext} from './FormControl'
 import {Slot} from './slots'
 
-const FormControlCaption: React.FC<SxProp> = ({children, sx}) => (
+const FormControlCaption: React.FC<{id?: string} & SxProp> = ({children, sx, id}) => (
   <Slot name="Caption">
     {({captionId, disabled}: FormControlContext) => (
-      <InputCaption id={captionId} disabled={disabled} sx={sx}>
+      <InputCaption id={id || captionId} disabled={disabled} sx={sx}>
         {children}
       </InputCaption>
     )}

diff --git a/src/FormControl/_FormControlLabel.tsx b/src/FormControl/_FormControlLabel.tsx
@@ -11,10 +11,16 @@ export type Props = {
   visuallyHidden?: boolean
 } & SxProp
 
-const FormControlLabel: React.FC<Props> = ({children, visuallyHidden, sx}) => (
+const FormControlLabel: React.FC<{htmlFor?: string} & Props> = ({children, htmlFor, visuallyHidden, sx}) => (
   <Slot name="Label">
     {({disabled, id, required}: FormControlContext) => (
-      <InputLabel htmlFor={id} visuallyHidden={visuallyHidden} required={required} disabled={disabled} sx={sx}>
+      <InputLabel
+        htmlFor={htmlFor || id}
+        visuallyHidden={visuallyHidden}
+        required={required}
+        disabled={disabled}
+        sx={sx}
+      >
         {children}
       </InputLabel>
     )}

diff --git a/src/FormControl/_FormControlValidation.tsx b/src/FormControl/_FormControlValidation.tsx
@@ -7,12 +7,13 @@ import {Slot} from './slots'
 
 export type FormControlValidationProps = {
   variant: FormValidationStatus
+  id?: string
 } & SxProp
 
-const FormControlValidation: React.FC<FormControlValidationProps> = ({children, variant, sx}) => (
+const FormControlValidation: React.FC<FormControlValidationProps> = ({children, variant, sx, id}) => (
   <Slot name="Validation">
     {({validationMessageId}: FormControlContext) => (
-      <InputValidation validationStatus={variant} id={validationMessageId} sx={sx}>
+      <InputValidation validationStatus={variant} id={id || validationMessageId} sx={sx}>
         {children}
       </InputValidation>
     )}

diff --git a/src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx b/src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx
@@ -1,5 +1,5 @@
 import React from 'react'
-import {Box, Checkbox, FormControl, Radio, useSSRSafeId} from '..'
+import {Box, useSSRSafeId} from '..'
 import ValidationAnimationContainer from '../_ValidationAnimationContainer'
 import CheckboxOrRadioGroupCaption from './_CheckboxOrRadioGroupCaption'
 import CheckboxOrRadioGroupLabel from './_CheckboxOrRadioGroupLabel'
@@ -56,7 +56,6 @@ const CheckboxOrRadioGroup: React.FC<CheckboxOrRadioGroupProps> = ({
   required,
   sx
 }) => {
-  const expectedInputComponents = [Checkbox, Radio]
   const labelChild = React.Children.toArray(children).find(
     child => React.isValidElement(child) && child.type === CheckboxOrRadioGroupLabel
   )
@@ -69,25 +68,6 @@ const CheckboxOrRadioGroup: React.FC<CheckboxOrRadioGroupProps> = ({
   const id = useSSRSafeId(idProp)
   const validationMessageId = validationChild && `${id}-validationMessage`
   const captionId = captionChild && `${id}-caption`
-  const checkIfOnlyContainsChoiceInputs = () => {
-    const formControlComponentChildren = React.Children.toArray(children)
-      .filter(child => React.isValidElement(child) && child.type === FormControl)
-      .map(formControlComponent =>
-        React.isValidElement(formControlComponent) ? formControlComponent.props.children : []
-      )
-      .flat()
-
-    return Boolean(
-      React.Children.toArray(formControlComponentChildren).find(child =>
-        expectedInputComponents.some(inputComponent => React.isValidElement(child) && child.type === inputComponent)
-      )
-    )
-  }
-
-  if (!checkIfOnlyContainsChoiceInputs()) {
-    // eslint-disable-next-line no-console
-    console.warn('Only `Checkbox` and `Radio` form controls should be used in a `CheckboxOrRadioGroup`.')
-  }
 
   if (!labelChild && !ariaLabelledby) {
     // eslint-disable-next-line no-console

diff --git a/src/__tests__/FormControl.test.tsx b/src/__tests__/FormControl.test.tsx
@@ -257,31 +257,6 @@ describe('FormControl', () => {
     })
 
     describe('warnings', () => {
-      it('should warn users if they do not pass an input', async () => {
-        render(
-          <SSRProvider>
-            <FormControl>
-              <FormControl.Label>{LABEL_TEXT}</FormControl.Label>
-              <FormControl.Caption>{CAPTION_TEXT}</FormControl.Caption>
-            </FormControl>
-          </SSRProvider>
-        )
-
-        expect(mockWarningFn).toHaveBeenCalled()
-      })
-      it('should warn users if they try to render a choice (checkbox or radio) input', async () => {
-        render(
-          <SSRProvider>
-            <FormControl>
-              <FormControl.Label>{LABEL_TEXT}</FormControl.Label>
-              <Checkbox />
-              <FormControl.Caption>{CAPTION_TEXT}</FormControl.Caption>
-            </FormControl>
-          </SSRProvider>
-        )
-
-        expect(mockWarningFn).toHaveBeenCalled()
-      })
       it('should log an error if a user does not pass a label', async () => {
         render(
           <SSRProvider>

diff --git a/src/stories/FormControl.stories.tsx b/src/stories/FormControl.stories.tsx
@@ -139,6 +139,23 @@ export const UsingRadioInput = (args: Args) => (
   </FormControl>
 )
 
+export const UsingCustomInput = (args: Args) => (
+  <FormControl {...args}>
+    <FormControl.Label htmlFor="custom-input">Name</FormControl.Label>
+    <input
+      type="text"
+      id="custom-input"
+      aria-describedby="custom-input-caption custom-input-validation"
+      disabled={args.disabled}
+      required={args.required}
+    />
+    <FormControl.Caption id="custom-input-caption">Your first name</FormControl.Caption>
+    <FormControl.Validation variant="success" id="custom-input-validation">
+      Not a valid name
+    </FormControl.Validation>
+  </FormControl>
+)
+
 export const WithLeadingVisual = (args: Args) => (
   <FormControl {...args}>
     <FormControl.Label>Selectable choice</FormControl.Label>