Skip to content

Commit 20f09b5

Browse files
mperrottisiddharthkp
authored andcommitted
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>
1 parent 809a2bf commit 20f09b5

File tree

9 files changed

+137
-74
lines changed

9 files changed

+137
-74
lines changed

.changeset/spotty-eagles-help.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': patch
3+
---
4+
5+
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

docs/content/FormControl.mdx

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,89 @@ const DifferentInputs = () => {
9191
render(DifferentInputs)
9292
```
9393

94+
### With a custom input
95+
96+
<Note variant="warning">
97+
98+
When rendering an input other than a form component from Primer, you must manually pass the attributes that make the form control accessible:
99+
100+
- The input should have an ID
101+
- `FormControl.Label` should be associated with the text input by using `htmlFor`
102+
- If there is a caption, the input should be associated with the caption by passing the message's ID to `aria-describedby`
103+
- If there is a validation message, the input should be associated with the message by passing the message's ID to `aria-describedby`
104+
- 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"`
105+
- If the input's value is invalid, `aria-invalid={true}` should be passed to the input.
106+
- If the input is disabled, `disabled` should be passed.
107+
- If the input is required, `required` should be passed.
108+
109+
When rendering a custom checkbox or radio component, you must also pass `layout="horizontal"` to the `FormControl` component.
110+
111+
</Note>
112+
113+
```javascript live noinline
114+
const CustomTextInput = props => <input type="text" {...props} />
115+
const CustomCheckboxInput = props => <input type="checkbox" {...props} />
116+
const FormControlWithCustomInput = () => {
117+
const [value, setValue] = React.useState('mona lisa')
118+
const [validationResult, setValidationResult] = React.useState()
119+
const doesValueContainSpaces = inputValue => /\s/g.test(inputValue)
120+
const handleInputChange = e => {
121+
setValue(e.currentTarget.value)
122+
}
123+
124+
React.useEffect(() => {
125+
if (doesValueContainSpaces(value)) {
126+
setValidationResult('noSpaces')
127+
} else if (value) {
128+
setValidationResult('validName')
129+
}
130+
}, [value])
131+
132+
return (
133+
<Box display="grid" gridGap={3}>
134+
<FormControl>
135+
<FormControl.Label htmlFor="custom-input">GitHub handle</FormControl.Label>
136+
<CustomTextInput
137+
id="custom-input"
138+
aria-describedby="custom-input-caption custom-input-validation"
139+
aria-invalid={validationResult === 'noSpaces'}
140+
onChange={handleInputChange}
141+
/>
142+
{validationResult === 'noSpaces' && (
143+
<FormControl.Validation id="custom-input-validation" variant="error">
144+
GitHub handles cannot contain spaces
145+
</FormControl.Validation>
146+
)}
147+
{validationResult === 'validName' && (
148+
<FormControl.Validation id="custom-input-validation" variant="success">
149+
Valid name
150+
</FormControl.Validation>
151+
)}
152+
<FormControl.Caption id="custom-input-caption">
153+
With or without "@". For example "monalisa" or "@monalisa"
154+
</FormControl.Caption>
155+
</FormControl>
156+
157+
<CheckboxGroup>
158+
<CheckboxGroup.Label>Checkboxes</CheckboxGroup.Label>
159+
<FormControl layout="horizontal">
160+
<CustomCheckboxInput id="custom-checkbox-one" value="checkOne" />
161+
<FormControl.Label htmlFor="custom-checkbox-one">Checkbox one</FormControl.Label>
162+
<FormControl.Caption id="custom-checkbox-one-caption">Hint text for checkbox one</FormControl.Caption>
163+
</FormControl>
164+
<FormControl layout="horizontal">
165+
<CustomCheckboxInput id="custom-checkbox-two" value="checkTwo" />
166+
<FormControl.Label htmlFor="custom-checkbox-two">Checkbox two</FormControl.Label>
167+
<FormControl.Caption id="custom-checkbox-two-caption">Hint text for checkbox two</FormControl.Caption>
168+
</FormControl>
169+
</CheckboxGroup>
170+
</Box>
171+
)
172+
}
173+
174+
render(FormControlWithCustomInput)
175+
```
176+
94177
### With checkbox and radio inputs
95178

96179
```jsx live

src/FormControl/FormControl.tsx

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ export type FormControlProps = {
2424
* If true, the user must specify a value for the input before the owning form can be submitted
2525
*/
2626
required?: boolean
27+
/**
28+
* The direction the content flows.
29+
* Vertical layout is used by default, and horizontal layout is used for checkbox and radio inputs.
30+
*/
31+
layout?: 'horizontal' | 'vertical'
2732
} & SxProp
2833

2934
export interface FormControlContext extends Pick<FormControlProps, 'disabled' | 'id' | 'required'> {
@@ -32,7 +37,7 @@ export interface FormControlContext extends Pick<FormControlProps, 'disabled' |
3237
}
3338

3439
const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
35-
({children, disabled: disabledProp, id: idProp, required, sx}, ref) => {
40+
({children, disabled: disabledProp, layout, id: idProp, required, sx}, ref) => {
3641
const expectedInputComponents = [Autocomplete, Checkbox, Radio, Select, TextInput, TextInputWithTokens, Textarea]
3742
const choiceGroupContext = useContext(CheckboxOrRadioGroupContext)
3843
const disabled = choiceGroupContext?.disabled || disabledProp
@@ -56,20 +61,7 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
5661
const isChoiceInput =
5762
React.isValidElement(InputComponent) && (InputComponent.type === Checkbox || InputComponent.type === Radio)
5863

59-
if (!InputComponent) {
60-
// eslint-disable-next-line no-console
61-
console.warn(
62-
`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(
63-
(acc, componentName) => {
64-
acc += `\n- ${componentName.displayName}`
65-
66-
return acc
67-
},
68-
''
69-
)}`,
70-
'If you are using a custom input component, please be sure to follow WCAG guidelines to make your form control accessible.'
71-
)
72-
} else {
64+
if (InputComponent) {
7365
if (inputProps?.id) {
7466
// eslint-disable-next-line no-console
7567
console.warn(
@@ -135,7 +127,7 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
135127
{slots => {
136128
const isLabelHidden = React.isValidElement(slots.Label) && slots.Label.props.visuallyHidden
137129

138-
return isChoiceInput ? (
130+
return isChoiceInput || layout === 'horizontal' ? (
139131
<Box ref={ref} display="flex" alignItems={slots.LeadingVisual ? 'center' : undefined} sx={sx}>
140132
<Box sx={{'> input': {marginLeft: 0, marginRight: 0}}}>
141133
{React.isValidElement(InputComponent) &&
@@ -183,13 +175,8 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
183175
display="flex"
184176
flexDirection="column"
185177
width="100%"
186-
sx={{...(isLabelHidden ? {'> *:not(label) + *': {marginTop: 2}} : {'> * + *': {marginTop: 2}}), ...sx}}
178+
sx={{...(isLabelHidden ? {'> *:not(label) + *': {marginTop: 1}} : {'> * + *': {marginTop: 1}}), ...sx}}
187179
>
188-
{React.Children.toArray(children).filter(
189-
child =>
190-
React.isValidElement(child) &&
191-
!expectedInputComponents.some(inputComponent => child.type === inputComponent)
192-
)}
193180
{slots.Label}
194181
{React.isValidElement(InputComponent) &&
195182
React.cloneElement(InputComponent, {
@@ -199,6 +186,11 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
199186
validationStatus,
200187
['aria-describedby']: [validationMessageId, captionId].filter(Boolean).join(' ')
201188
})}
189+
{React.Children.toArray(children).filter(
190+
child =>
191+
React.isValidElement(child) &&
192+
!expectedInputComponents.some(inputComponent => child.type === inputComponent)
193+
)}
202194
{validationChild && <ValidationAnimationContainer show>{slots.Validation}</ValidationAnimationContainer>}
203195
{slots.Caption}
204196
</Box>
@@ -209,6 +201,10 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
209201
}
210202
)
211203

204+
FormControl.defaultProps = {
205+
layout: 'vertical'
206+
}
207+
212208
export default Object.assign(FormControl, {
213209
Caption: FormControlCaption,
214210
Label: FormControlLabel,

src/FormControl/_FormControlCaption.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import InputCaption from '../_InputCaption'
44
import {FormControlContext} from './FormControl'
55
import {Slot} from './slots'
66

7-
const FormControlCaption: React.FC<SxProp> = ({children, sx}) => (
7+
const FormControlCaption: React.FC<{id?: string} & SxProp> = ({children, sx, id}) => (
88
<Slot name="Caption">
99
{({captionId, disabled}: FormControlContext) => (
10-
<InputCaption id={captionId} disabled={disabled} sx={sx}>
10+
<InputCaption id={id || captionId} disabled={disabled} sx={sx}>
1111
{children}
1212
</InputCaption>
1313
)}

src/FormControl/_FormControlLabel.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,16 @@ export type Props = {
1111
visuallyHidden?: boolean
1212
} & SxProp
1313

14-
const FormControlLabel: React.FC<Props> = ({children, visuallyHidden, sx}) => (
14+
const FormControlLabel: React.FC<{htmlFor?: string} & Props> = ({children, htmlFor, visuallyHidden, sx}) => (
1515
<Slot name="Label">
1616
{({disabled, id, required}: FormControlContext) => (
17-
<InputLabel htmlFor={id} visuallyHidden={visuallyHidden} required={required} disabled={disabled} sx={sx}>
17+
<InputLabel
18+
htmlFor={htmlFor || id}
19+
visuallyHidden={visuallyHidden}
20+
required={required}
21+
disabled={disabled}
22+
sx={sx}
23+
>
1824
{children}
1925
</InputLabel>
2026
)}

src/FormControl/_FormControlValidation.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@ import {Slot} from './slots'
77

88
export type FormControlValidationProps = {
99
variant: FormValidationStatus
10+
id?: string
1011
} & SxProp
1112

12-
const FormControlValidation: React.FC<FormControlValidationProps> = ({children, variant, sx}) => (
13+
const FormControlValidation: React.FC<FormControlValidationProps> = ({children, variant, sx, id}) => (
1314
<Slot name="Validation">
1415
{({validationMessageId}: FormControlContext) => (
15-
<InputValidation validationStatus={variant} id={validationMessageId} sx={sx}>
16+
<InputValidation validationStatus={variant} id={id || validationMessageId} sx={sx}>
1617
{children}
1718
</InputValidation>
1819
)}

src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react'
2-
import {Box, Checkbox, FormControl, Radio, useSSRSafeId} from '..'
2+
import {Box, useSSRSafeId} from '..'
33
import ValidationAnimationContainer from '../_ValidationAnimationContainer'
44
import CheckboxOrRadioGroupCaption from './_CheckboxOrRadioGroupCaption'
55
import CheckboxOrRadioGroupLabel from './_CheckboxOrRadioGroupLabel'
@@ -56,7 +56,6 @@ const CheckboxOrRadioGroup: React.FC<CheckboxOrRadioGroupProps> = ({
5656
required,
5757
sx
5858
}) => {
59-
const expectedInputComponents = [Checkbox, Radio]
6059
const labelChild = React.Children.toArray(children).find(
6160
child => React.isValidElement(child) && child.type === CheckboxOrRadioGroupLabel
6261
)
@@ -69,25 +68,6 @@ const CheckboxOrRadioGroup: React.FC<CheckboxOrRadioGroupProps> = ({
6968
const id = useSSRSafeId(idProp)
7069
const validationMessageId = validationChild && `${id}-validationMessage`
7170
const captionId = captionChild && `${id}-caption`
72-
const checkIfOnlyContainsChoiceInputs = () => {
73-
const formControlComponentChildren = React.Children.toArray(children)
74-
.filter(child => React.isValidElement(child) && child.type === FormControl)
75-
.map(formControlComponent =>
76-
React.isValidElement(formControlComponent) ? formControlComponent.props.children : []
77-
)
78-
.flat()
79-
80-
return Boolean(
81-
React.Children.toArray(formControlComponentChildren).find(child =>
82-
expectedInputComponents.some(inputComponent => React.isValidElement(child) && child.type === inputComponent)
83-
)
84-
)
85-
}
86-
87-
if (!checkIfOnlyContainsChoiceInputs()) {
88-
// eslint-disable-next-line no-console
89-
console.warn('Only `Checkbox` and `Radio` form controls should be used in a `CheckboxOrRadioGroup`.')
90-
}
9171

9272
if (!labelChild && !ariaLabelledby) {
9373
// eslint-disable-next-line no-console

src/__tests__/FormControl.test.tsx

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -257,31 +257,6 @@ describe('FormControl', () => {
257257
})
258258

259259
describe('warnings', () => {
260-
it('should warn users if they do not pass an input', async () => {
261-
render(
262-
<SSRProvider>
263-
<FormControl>
264-
<FormControl.Label>{LABEL_TEXT}</FormControl.Label>
265-
<FormControl.Caption>{CAPTION_TEXT}</FormControl.Caption>
266-
</FormControl>
267-
</SSRProvider>
268-
)
269-
270-
expect(mockWarningFn).toHaveBeenCalled()
271-
})
272-
it('should warn users if they try to render a choice (checkbox or radio) input', async () => {
273-
render(
274-
<SSRProvider>
275-
<FormControl>
276-
<FormControl.Label>{LABEL_TEXT}</FormControl.Label>
277-
<Checkbox />
278-
<FormControl.Caption>{CAPTION_TEXT}</FormControl.Caption>
279-
</FormControl>
280-
</SSRProvider>
281-
)
282-
283-
expect(mockWarningFn).toHaveBeenCalled()
284-
})
285260
it('should log an error if a user does not pass a label', async () => {
286261
render(
287262
<SSRProvider>

src/stories/FormControl.stories.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,23 @@ export const UsingRadioInput = (args: Args) => (
139139
</FormControl>
140140
)
141141

142+
export const UsingCustomInput = (args: Args) => (
143+
<FormControl {...args}>
144+
<FormControl.Label htmlFor="custom-input">Name</FormControl.Label>
145+
<input
146+
type="text"
147+
id="custom-input"
148+
aria-describedby="custom-input-caption custom-input-validation"
149+
disabled={args.disabled}
150+
required={args.required}
151+
/>
152+
<FormControl.Caption id="custom-input-caption">Your first name</FormControl.Caption>
153+
<FormControl.Validation variant="success" id="custom-input-validation">
154+
Not a valid name
155+
</FormControl.Validation>
156+
</FormControl>
157+
)
158+
142159
export const WithLeadingVisual = (args: Args) => (
143160
<FormControl {...args}>
144161
<FormControl.Label>Selectable choice</FormControl.Label>

0 commit comments

Comments
 (0)