Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make FormControl externally extensible (via hook) #3632

Merged
merged 22 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2d9c093
Make `FormControl` extensible via context + hook
iansan5653 Aug 14, 2023
380c757
Create sweet-turtles-applaud.md
iansan5653 Aug 14, 2023
41d53c0
Dont call hook in `InlineAutocomplete` since it gets called farther d…
iansan5653 Aug 14, 2023
86daf8b
Don't call hook in `Autocomplete`
iansan5653 Aug 14, 2023
6b34654
Add comments on imports
iansan5653 Aug 14, 2023
e88f28d
Remove old comment
iansan5653 Aug 14, 2023
ebb0ba5
Remove `id` override from `AutocompleteInput`
iansan5653 Aug 14, 2023
2a73a98
Merge branch 'main' of https://github.com/primer/react into formcontr…
iansan5653 Aug 14, 2023
bac5870
Change version bump to `minor`
iansan5653 Aug 14, 2023
b32e2dc
Revert "Remove `id` override from `AutocompleteInput`"
iansan5653 Aug 14, 2023
eb900da
Suppress warning when passed ID == context ID
iansan5653 Aug 14, 2023
11120db
Don't export `FormControlContext` directly
iansan5653 Aug 18, 2023
8e470af
Don't export `FormControlContext` type
iansan5653 Aug 21, 2023
edf8fe4
Revert component changes
iansan5653 Aug 22, 2023
5ff0518
Move warning & cloning logic back into `FormControl` :(
iansan5653 Aug 22, 2023
cda6870
Allow calling `useForwardedProps` without passing a props object
iansan5653 Aug 22, 2023
cbcb273
Expose `useFormControlForwardedProps` directly
iansan5653 Sep 1, 2023
ffedcc1
Add tests for `useFormControlForwardedProps`
iansan5653 Sep 1, 2023
38bcc16
Add story for `useFormControlForwardedProps`
iansan5653 Sep 1, 2023
b21bb48
Fix import
iansan5653 Sep 5, 2023
b5677e5
Update snapshot
iansan5653 Sep 5, 2023
51b5e83
Fix `InlineAutocomplete`
iansan5653 Sep 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/sweet-turtles-applaud.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@primer/react": minor
---

Allow consumers to make components that are compatible with `FormControl` by reading forwarded props in from the `useFormControlForwardedProps` hook

<!-- Changed components: FormControl -->
28 changes: 6 additions & 22 deletions src/FormControl/FormControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import Textarea from '../Textarea'
import {CheckboxOrRadioGroupContext} from '../internal/components/CheckboxOrRadioGroup'
import ValidationAnimationContainer from '../internal/components/ValidationAnimationContainer'
import {get} from '../constants'
import InlineAutocomplete from '../drafts/InlineAutocomplete'
import {useSlots} from '../hooks/useSlots'
import {SxProp} from '../sx'
import {useSSRSafeId} from '../utils/ssr'
import {useId} from '../hooks/useId'
import FormControlCaption from './_FormControlCaption'
import FormControlLabel from './_FormControlLabel'
import FormControlLeadingVisual from './_FormControlLeadingVisual'
import FormControlValidation from './_FormControlValidation'
import {FormControlContextProvider} from './_FormControlContext'

export type FormControlProps = {
children?: React.ReactNode
Expand All @@ -40,13 +40,6 @@ export type FormControlProps = {
layout?: 'horizontal' | 'vertical'
} & SxProp

export interface FormControlContext extends Pick<FormControlProps, 'disabled' | 'id' | 'required'> {
captionId?: string
validationMessageId?: string
}

export const FormControlContext = React.createContext<FormControlContext>({})

const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
({children, disabled: disabledProp, layout = 'vertical', id: idProp, required, sx}, ref) => {
const [slots, childrenWithoutSlots] = useSlots(children, {
Expand All @@ -55,19 +48,10 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
leadingVisual: FormControlLeadingVisual,
validation: FormControlValidation,
})
const expectedInputComponents = [
Autocomplete,
Checkbox,
Radio,
Select,
TextInput,
TextInputWithTokens,
Textarea,
InlineAutocomplete,
]
const expectedInputComponents = [Autocomplete, Checkbox, Radio, Select, TextInput, TextInputWithTokens, Textarea]
const choiceGroupContext = useContext(CheckboxOrRadioGroupContext)
const disabled = choiceGroupContext.disabled || disabledProp
const id = useSSRSafeId(idProp)
const id = useId(idProp)
const validationMessageId = slots.validation ? `${id}-validationMessage` : undefined
const captionId = slots.caption ? `${id}-caption` : undefined
const validationStatus = slots.validation?.props.variant
Expand Down Expand Up @@ -130,7 +114,7 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
const isLabelHidden = slots.label?.props.visuallyHidden

return (
<FormControlContext.Provider
<FormControlContextProvider
value={{
captionId,
disabled,
Expand Down Expand Up @@ -222,7 +206,7 @@ const FormControl = React.forwardRef<HTMLDivElement, FormControlProps>(
{slots.caption}
</Box>
)}
</FormControlContext.Provider>
</FormControlContextProvider>
)
},
)
Expand Down
4 changes: 2 additions & 2 deletions src/FormControl/_FormControlCaption.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from 'react'
import InputCaption from '../internal/components/InputCaption'
import {SxProp} from '../sx'
import {FormControlContext} from './FormControl'
import {useFormControlContext} from './_FormControlContext'

const FormControlCaption: React.FC<React.PropsWithChildren<{id?: string} & SxProp>> = ({children, sx, id}) => {
const {captionId, disabled} = React.useContext(FormControlContext)
const {captionId, disabled} = useFormControlContext()
return (
<InputCaption id={id || captionId || ''} disabled={disabled} sx={sx}>
{children}
Expand Down
51 changes: 51 additions & 0 deletions src/FormControl/_FormControlContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import {createContext, useContext} from 'react'
import {FormValidationStatus} from '../utils/types/FormValidationStatus'
import {FormControlProps} from './FormControl'

interface FormControlContext extends Pick<FormControlProps, 'disabled' | 'id' | 'required'> {
captionId?: string
validationMessageId?: string
validationStatus?: FormValidationStatus
}

const FormControlContext = createContext<FormControlContext | null>(null)

export const FormControlContextProvider = FormControlContext.Provider

/** This is the private/internal interface for subcomponents of `FormControl`. */
export function useFormControlContext(): FormControlContext {
return useContext(FormControlContext) ?? {}
}

interface FormControlForwardedProps extends Omit<FormControlContext, 'captionId' | 'validationMessageId'> {
['aria-describedby']?: string
}

/**
* Make any component compatible with `FormControl`'s automatic wiring up of accessibility attributes & validation by
* reading the props from this hook and merging them with the passed-in props. If used outside of `FormControl`, this
* hook has no effect.
*
* @param externalProps The external props passed to this component. If provided, these props will be merged with the
* `FormControl` props, with external props taking priority.
*/
export function useFormControlForwardedProps<P>(externalProps: P): P & FormControlForwardedProps
/**
* Make any component compatible with `FormControl`'s automatic wiring up of accessibility attributes & validation by
* reading the props from this hook and handling them / assigning them to the underlying form control. If used outside
* of `FormControl`, this hook has no effect.
*/
export function useFormControlForwardedProps(): FormControlForwardedProps
export function useFormControlForwardedProps(externalProps: FormControlForwardedProps = {}) {
const context = useContext(FormControlContext)
if (!context) return externalProps

return {
disabled: context.disabled,
id: context.id,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One downside of this is that if two components inside the same FormControl call this hook, they could have the same ID. But that's an invalid structure anyway - FormControl is singular so this shouldn't be a problem.

required: context.required,
validationStatus: context.validationStatus,
['aria-describedby']: [context.validationMessageId, context.captionId].filter(Boolean).join(' ') || undefined,
...externalProps,
}
}
4 changes: 2 additions & 2 deletions src/FormControl/_FormControlLabel.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react'
import InputLabel from '../internal/components/InputLabel'
import {SxProp} from '../sx'
import {FormControlContext} from './FormControl'
import {useFormControlContext} from './_FormControlContext'

export type Props = {
/**
Expand All @@ -14,7 +14,7 @@ export type Props = {
const FormControlLabel: React.FC<
React.PropsWithChildren<{htmlFor?: string} & React.ComponentProps<typeof InputLabel> & Props>
> = ({as, children, htmlFor, id, visuallyHidden, sx, ...props}) => {
const {disabled, id: formControlId, required} = React.useContext(FormControlContext)
const {disabled, id: formControlId, required} = useFormControlContext()

/**
* Ensure we can pass through props correctly, since legend/span accept no defined 'htmlFor'
Expand Down
4 changes: 2 additions & 2 deletions src/FormControl/_FormControlLeadingVisual.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import React from 'react'
import Box from '../Box'
import {get} from '../constants'
import {SxProp} from '../sx'
import {FormControlContext} from './FormControl'
import {useFormControlContext} from './_FormControlContext'

const FormControlLeadingVisual: React.FC<React.PropsWithChildren<SxProp>> = ({children, sx}) => {
const {disabled, captionId} = React.useContext(FormControlContext)
const {disabled, captionId} = useFormControlContext()
return (
<Box
color={disabled ? 'fg.muted' : 'fg.default'}
Expand Down
4 changes: 2 additions & 2 deletions src/FormControl/_FormControlValidation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react'
import InputValidation from '../internal/components/InputValidation'
import {SxProp} from '../sx'
import {FormValidationStatus} from '../utils/types/FormValidationStatus'
import {FormControlContext} from './FormControl'
import {useFormControlContext} from './_FormControlContext'

export type FormControlValidationProps = {
variant: FormValidationStatus
Expand All @@ -15,7 +15,7 @@ const FormControlValidation: React.FC<React.PropsWithChildren<FormControlValidat
sx,
id,
}) => {
const {validationMessageId} = React.useContext(FormControlContext)
const {validationMessageId} = useFormControlContext()
return (
<InputValidation validationStatus={variant} id={id || validationMessageId || ''} sx={sx}>
{children}
Expand Down
1 change: 1 addition & 0 deletions src/FormControl/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export {useFormControlForwardedProps} from './_FormControlContext'
export {default} from './FormControl'
67 changes: 67 additions & 0 deletions src/FormControl/useFormControlForwardedProps.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import React from 'react'
import {Meta} from '@storybook/react'
import {BaseStyles, FormControl, ThemeProvider, theme, useFormControlForwardedProps} from '..'

export default {
title: 'Hooks/useFormControlForwardedProps',
decorators: [
Story => {
return (
<ThemeProvider theme={theme}>
<BaseStyles>
<Story />
</BaseStyles>
</ThemeProvider>
)
},
],
argTypes: {
disabled: {
type: 'boolean',
},
required: {
type: 'boolean',
},
label: {
type: 'string',
},
caption: {
type: 'string',
},
type: {
control: {
type: 'select',
description: "Type of the input, showing how the `type` prop can be forwarded to the input's props",
},
options: ['text', 'number', 'password', 'email', 'search', 'tel', 'url'],
},
},
} as Meta

interface ArgTypes {
disabled: boolean
required: boolean
label: string
caption: string
type: string
}

/** A custom input that is not a Primer `TextInput` but still supports autowiring with `FormControl`. */
const CustomInput = (externalProps: {type: string}) => {
const props = useFormControlForwardedProps(externalProps)
return <input {...props} />
}

export const AutowiredCustomInput = ({
label = 'Custom input',
caption = 'This is not a Primer input, but it still has `aria-describedby` and similar attributes applied automatically',
required = false,
disabled = false,
type = 'text',
}: ArgTypes) => (
<FormControl disabled={disabled} required={required}>
<FormControl.Label>{label}</FormControl.Label>
<CustomInput type={type} />
{caption && <FormControl.Caption>{caption}</FormControl.Caption>}
</FormControl>
)
64 changes: 62 additions & 2 deletions src/__tests__/FormControl.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import React from 'react'
import {render} from '@testing-library/react'
import {render, renderHook} from '@testing-library/react'

Check failure on line 2 in src/__tests__/FormControl.test.tsx

View workflow job for this annotation

GitHub Actions / type-check (17)

Module '"@testing-library/react"' has no exported member 'renderHook'.
import {axe, toHaveNoViolations} from 'jest-axe'
import {Autocomplete, Checkbox, FormControl, Select, SSRProvider, Textarea, TextInput, TextInputWithTokens} from '..'
import {
Autocomplete,
Checkbox,
FormControl,
Select,
SSRProvider,
Textarea,
TextInput,
TextInputWithTokens,
useFormControlForwardedProps,
} from '..'
import {MarkGithubIcon} from '@primer/octicons-react'
expect.extend(toHaveNoViolations)

Expand Down Expand Up @@ -446,3 +456,53 @@
})
})
})

describe('useFormControlForwardedProps', () => {
describe('when used outside FormControl', () => {
test('returns empty object when no props object passed', () => {
const result = renderHook(() => useFormControlForwardedProps())
expect(result.result.current).toEqual({})
})

test('returns passed props object instance when passed', () => {
const props = {id: 'test-id'}
const result = renderHook(() => useFormControlForwardedProps(props))
expect(result.result.current).toBe(props)
})
})

test('provides context value when no props object is passed', () => {
const id = 'test-id'

const {result} = renderHook(() => useFormControlForwardedProps(), {
wrapper: ({children}) => (

Check failure on line 478 in src/__tests__/FormControl.test.tsx

View workflow job for this annotation

GitHub Actions / type-check (17)

Binding element 'children' implicitly has an 'any' type.
<FormControl id={id} disabled required>
<FormControl.Label>Label</FormControl.Label>
{children}
</FormControl>
),
})

expect(result.current.disabled).toBe(true)
expect(result.current.id).toBe(id)
expect(result.current.required).toBe(true)
})

test('merges with props object, overriding to prioritize props when conflicting', () => {
const props = {id: 'override-id', xyz: 'someValue'}

const {result} = renderHook(() => useFormControlForwardedProps(props), {
wrapper: ({children}) => (

Check failure on line 495 in src/__tests__/FormControl.test.tsx

View workflow job for this annotation

GitHub Actions / type-check (17)

Binding element 'children' implicitly has an 'any' type.
<FormControl id="form-control-id" disabled>
<FormControl.Label>Label</FormControl.Label>
{children}
</FormControl>
),
})

expect(result.current.disabled).toBe(true)
expect(result.current.id).toBe(props.id)
expect(result.current.required).toBeFalsy()
expect(result.current.xyz).toBe(props.xyz)
})
})
1 change: 0 additions & 1 deletion src/drafts/InlineAutocomplete/InlineAutocomplete.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ const InlineAutocomplete = ({
children,
tabInsertsSuggestions = false,
suggestionsPlacement = 'below',
// Forward accessibility props so it works with FormControl
...forwardProps
Copy link
Contributor Author

@iansan5653 iansan5653 Aug 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forwarding isn't necessary to make it work with FormControl anymore since the underlying TextInput or Textarea will handle it, but I still forward the HTML props just for backwards compat

}: InlineAutocompleteProps & React.ComponentProps<'textarea' | 'input'>) => {
const inputRef = useRef<HTMLInputElement & HTMLTextAreaElement>(null)
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export type {FilterListProps, FilterListItemProps} from './FilterList'
export {default as Flash} from './Flash'
export type {FlashProps} from './Flash'
export {default as FormControl} from './FormControl'
export {useFormControlForwardedProps} from './FormControl'
export {default as Header} from './Header'
export type {HeaderProps, HeaderItemProps, HeaderLinkProps} from './Header'
export {default as Heading} from './Heading'
Expand Down
Loading