diff --git a/.changeset/spotty-eagles-help.md b/.changeset/spotty-eagles-help.md new file mode 100644 index 00000000000..2593685d9e9 --- /dev/null +++ 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 index 9bd343de66c..b93291853b1 100644 --- a/docs/content/FormControl.mdx +++ b/docs/content/FormControl.mdx @@ -91,6 +91,89 @@ const DifferentInputs = () => { render(DifferentInputs) ``` +### With a custom input + + + +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. + + + +```javascript live noinline +const CustomTextInput = props => +const CustomCheckboxInput = 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 ( + + + GitHub handle + + {validationResult === 'noSpaces' && ( + + GitHub handles cannot contain spaces + + )} + {validationResult === 'validName' && ( + + Valid name + + )} + + With or without "@". For example "monalisa" or "@monalisa" + + + + + Checkboxes + + + Checkbox one + Hint text for checkbox one + + + + Checkbox two + Hint text for checkbox two + + + + ) +} + +render(FormControlWithCustomInput) +``` + ### With checkbox and radio inputs ```jsx live diff --git a/src/FormControl/FormControl.tsx b/src/FormControl/FormControl.tsx index 7ed7dac30f6..7e022763fbb 100644 --- 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 { @@ -32,7 +37,7 @@ export interface FormControlContext extends Pick( - ({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( 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( {slots => { const isLabelHidden = React.isValidElement(slots.Label) && slots.Label.props.visuallyHidden - return isChoiceInput ? ( + return isChoiceInput || layout === 'horizontal' ? ( input': {marginLeft: 0, marginRight: 0}}}> {React.isValidElement(InputComponent) && @@ -183,13 +175,8 @@ const FormControl = React.forwardRef( 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( validationStatus, ['aria-describedby']: [validationMessageId, captionId].filter(Boolean).join(' ') })} + {React.Children.toArray(children).filter( + child => + React.isValidElement(child) && + !expectedInputComponents.some(inputComponent => child.type === inputComponent) + )} {validationChild && {slots.Validation}} {slots.Caption} @@ -209,6 +201,10 @@ const FormControl = React.forwardRef( } ) +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 index 26ab27a5162..12197ef383d 100644 --- 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 = ({children, sx}) => ( +const FormControlCaption: React.FC<{id?: string} & SxProp> = ({children, sx, id}) => ( {({captionId, disabled}: FormControlContext) => ( - + {children} )} diff --git a/src/FormControl/_FormControlLabel.tsx b/src/FormControl/_FormControlLabel.tsx index e6a0021f095..48876830259 100644 --- a/src/FormControl/_FormControlLabel.tsx +++ b/src/FormControl/_FormControlLabel.tsx @@ -11,10 +11,16 @@ export type Props = { visuallyHidden?: boolean } & SxProp -const FormControlLabel: React.FC = ({children, visuallyHidden, sx}) => ( +const FormControlLabel: React.FC<{htmlFor?: string} & Props> = ({children, htmlFor, visuallyHidden, sx}) => ( {({disabled, id, required}: FormControlContext) => ( - + {children} )} diff --git a/src/FormControl/_FormControlValidation.tsx b/src/FormControl/_FormControlValidation.tsx index 7ea94e25ec3..b2653c3c7d4 100644 --- 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 = ({children, variant, sx}) => ( +const FormControlValidation: React.FC = ({children, variant, sx, id}) => ( {({validationMessageId}: FormControlContext) => ( - + {children} )} diff --git a/src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx b/src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx index fb1c963c236..7b50d5ab1fc 100644 --- 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 = ({ 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 = ({ 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 index 34724e7c730..4f79f92bfdd 100644 --- 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( - - - {LABEL_TEXT} - {CAPTION_TEXT} - - - ) - - expect(mockWarningFn).toHaveBeenCalled() - }) - it('should warn users if they try to render a choice (checkbox or radio) input', async () => { - render( - - - {LABEL_TEXT} - - {CAPTION_TEXT} - - - ) - - expect(mockWarningFn).toHaveBeenCalled() - }) it('should log an error if a user does not pass a label', async () => { render( diff --git a/src/stories/FormControl.stories.tsx b/src/stories/FormControl.stories.tsx index 1fd255c41c7..cd29a9e1de6 100644 --- a/src/stories/FormControl.stories.tsx +++ b/src/stories/FormControl.stories.tsx @@ -139,6 +139,23 @@ export const UsingRadioInput = (args: Args) => ( ) +export const UsingCustomInput = (args: Args) => ( + + Name + + Your first name + + Not a valid name + + +) + export const WithLeadingVisual = (args: Args) => ( Selectable choice