Skip to content

Commit

Permalink
TextInput: Add LeadingVisual/TrailingVisual to be part of the acc…
Browse files Browse the repository at this point in the history
…essible description in `TextInput` (#4939)

* Add `aria-describedby` to `TextInput` when `LeadingVisual` === `string`

* Update to include `TrailingVisual`, icons

* Add tests, update snapshots

* Axe fix: Add `aria-label` to story

* Add changeset

* Add loading to description

* Add new prop `loaderText`, reference loading with text input

* Update changeset

* Update packages/react/src/TextInput/TextInput.tsx

Co-authored-by: Ian Sanders <iansan5653@github.com>

* Add `aria-hidden`

* Update docs note, test snapshots

* format

* Fix tests

* Update snapshots again

* Update snapshots again

* Move `visuallyhidden` span

* Update snapshots

---------

Co-authored-by: Ian Sanders <iansan5653@github.com>
  • Loading branch information
TylerJDev and iansan5653 authored Oct 17, 2024
1 parent bd1f1c2 commit 9936add
Show file tree
Hide file tree
Showing 10 changed files with 915 additions and 1,033 deletions.
5 changes: 5 additions & 0 deletions .changeset/five-windows-invent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Adds `aria-describedby` for `LeadingVisual` and `TrailingVisual` in `TextInput`; adds new prop `loaderText` to convey loading state to screen readers
6 changes: 6 additions & 0 deletions packages/react/src/TextInput/TextInput.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@
"defaultValue": "",
"description": "<div>Which position to render the loading indicator</div> <ul> <li> 'auto' (default): at the end of the input, unless a `leadingVisual` is passed. Then, it will render at the beginning </li> <li>'leading': at the beginning of the input</li> <li>'trailing': at the end of the input</li> </ul>"
},
{
"name": "loaderText",
"type": "string",
"defaultValue": "Loading",
"description": "Text for screen readers to convey the loading state, should be descriptive and explain what is loading. This prop should only be used if there is visible context explaining what is loading, to ensure that context is provided to all users."
},
{
"name": "leadingVisual",
"type": "string | React.ComponentType",
Expand Down
56 changes: 32 additions & 24 deletions packages/react/src/TextInput/TextInput.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,31 +98,39 @@ export const Required = () => (
</Box>
)

export const WithLeadingVisual = () => (
<Box as="form">
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<TextInput leadingVisual={CheckIcon} />
</FormControl>
<FormControl>
<FormControl.Label>Enter monies</FormControl.Label>
<TextInput leadingVisual="$" />
</FormControl>
</Box>
)
export const WithLeadingVisual = () => {
const Checkmark = () => <CheckIcon aria-label="Checkmark" />

export const WithTrailingIcon = () => (
<Box>
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<TextInput trailingVisual={CheckIcon} />
</FormControl>
<FormControl>
<FormControl.Label>Enter monies</FormControl.Label>
<TextInput trailingVisual="minutes" placeholder="200" />
</FormControl>
</Box>
)
return (
<Box as="form">
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<TextInput leadingVisual={Checkmark} />
</FormControl>
<FormControl>
<FormControl.Label>Enter monies</FormControl.Label>
<TextInput leadingVisual="$" />
</FormControl>
</Box>
)
}

export const WithTrailingIcon = () => {
const Checkmark = () => <CheckIcon aria-label="Checkmark" />

return (
<Box>
<FormControl>
<FormControl.Label>Default label</FormControl.Label>
<TextInput trailingVisual={Checkmark} />
</FormControl>
<FormControl>
<FormControl.Label>Enter monies</FormControl.Label>
<TextInput trailingVisual="minutes" placeholder="200" />
</FormControl>
</Box>
)
}

export const WithTrailingAction = () => {
const [value, setValue] = useState('sample text')
Expand Down
24 changes: 23 additions & 1 deletion packages/react/src/TextInput/TextInput.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {MouseEventHandler} from 'react'
import React, {useCallback, useState} from 'react'
import React, {useCallback, useState, useId} from 'react'
import {isValidElementType} from 'react-is'
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
import {clsx} from 'clsx'
Expand All @@ -11,6 +11,7 @@ import type {StyledWrapperProps} from '../internal/components/TextInputWrapper'
import TextInputWrapper from '../internal/components/TextInputWrapper'
import TextInputAction from '../internal/components/TextInputInnerAction'
import UnstyledTextInput from '../internal/components/UnstyledTextInput'
import VisuallyHidden from '../_VisuallyHidden'

export type TextInputNonPassthroughProps = {
/** @deprecated Use `leadingVisual` or `trailingVisual` prop instead */
Expand All @@ -24,6 +25,8 @@ export type TextInputNonPassthroughProps = {
* 'trailing': at the end of the input
**/
loaderPosition?: 'auto' | 'leading' | 'trailing'
/** Text for screen readers to convey the loading state */
loaderText?: string
/**
* A visual that renders inside the input before the typing area
*/
Expand Down Expand Up @@ -67,6 +70,7 @@ const TextInput = React.forwardRef<HTMLInputElement, TextInputProps>(
disabled,
loading,
loaderPosition = 'auto',
loaderText = 'Loading',
monospace,
validationStatus,
sx: sxProp,
Expand Down Expand Up @@ -96,6 +100,18 @@ const TextInput = React.forwardRef<HTMLInputElement, TextInputProps>(
const focusInput: MouseEventHandler = () => {
inputRef.current?.focus()
}
const leadingVisualId = useId()
const trailingVisualId = useId()
const loadingId = useId()

const inputDescribedBy =
clsx(
inputProps['aria-describedby'],
LeadingVisual && leadingVisualId,
TrailingVisual && trailingVisualId,
loading && loadingId,
) || undefined

const handleInputFocus = useCallback(
(e: React.FocusEvent<HTMLInputElement>) => {
setIsInputFocused(true)
Expand Down Expand Up @@ -137,6 +153,8 @@ const TextInput = React.forwardRef<HTMLInputElement, TextInputProps>(
visualPosition="leading"
showLoadingIndicator={showLeadingLoadingIndicator}
hasLoadingIndicator={typeof loading === 'boolean'}
id={leadingVisualId}
data-testid="text-input-leading-visual"
>
{typeof LeadingVisual !== 'string' && isValidElementType(LeadingVisual) ? <LeadingVisual /> : LeadingVisual}
</TextInputInnerVisualSlot>
Expand All @@ -149,12 +167,16 @@ const TextInput = React.forwardRef<HTMLInputElement, TextInputProps>(
aria-required={required}
aria-invalid={validationStatus === 'error' ? 'true' : undefined}
{...inputProps}
aria-describedby={inputDescribedBy}
data-component="input"
/>
{loading && <VisuallyHidden id={loadingId}>{loaderText}</VisuallyHidden>}
<TextInputInnerVisualSlot
visualPosition="trailing"
showLoadingIndicator={showTrailingLoadingIndicator}
hasLoadingIndicator={typeof loading === 'boolean'}
id={trailingVisualId}
data-testid="text-input-trailing-visual"
>
{typeof TrailingVisual !== 'string' && isValidElementType(TrailingVisual) ? (
<TrailingVisual />
Expand Down
68 changes: 68 additions & 0 deletions packages/react/src/__tests__/TextInput.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,4 +256,72 @@ describe('TextInput', () => {
const {getByRole} = HTMLRender(<TextInput onChange={onChange} aria-invalid="true" value="" />)
expect(getByRole('textbox')).toHaveAttribute('aria-invalid', 'true')
})

it('should include the leadingVisual as part of the input accessible description', () => {
const {getByRole} = HTMLRender(<TextInput leadingVisual="Search" />)
expect(getByRole('textbox')).toHaveAccessibleDescription('Search')
})

it('should include the leadingVisual icon as part of the input accessible description', () => {
const Icon = () => <SearchIcon aria-label="Search" />

const {getByRole} = HTMLRender(<TextInput leadingVisual={Icon} />)
const icon = getByRole('img', {hidden: true})

expect(getByRole('textbox')).toHaveAttribute('aria-describedby', icon.parentElement?.id)
expect(icon).toHaveAccessibleName('Search')
})

it('should include the trailingVisual as part of the input accessible description', () => {
const {getByRole} = HTMLRender(<TextInput trailingVisual="Search" />)
expect(getByRole('textbox')).toHaveAccessibleDescription('Search')
})

it('should include the trailingVisual icon as part of the input accessible description', () => {
const Icon = () => <SearchIcon aria-label="Search" />

const {getByRole} = HTMLRender(<TextInput trailingVisual={Icon} />)
const icon = getByRole('img', {hidden: true})

expect(getByRole('textbox')).toHaveAttribute('aria-describedby', icon.parentElement?.id)
expect(icon).toHaveAccessibleName('Search')
})

it('should include both the leadingVisual and trailingVisual as part of the input accessible description', () => {
const {getByRole} = HTMLRender(<TextInput leadingVisual="$" trailingVisual="Currency" />)
expect(getByRole('textbox')).toHaveAccessibleDescription('$ Currency')
})

it('should keep the passed aria-describedby value', () => {
const {getByRole} = HTMLRender(
<>
<span id="passedValue">value</span>
<TextInput leadingVisual="leading" trailingVisual="trailing" aria-describedby="passedValue" />
</>,
)
expect(getByRole('textbox').getAttribute('aria-describedby')).toContain('passedValue')
expect(getByRole('textbox')).toHaveAccessibleDescription('value leading trailing')
})

it('should include the loading indicator as part of the input accessible description', () => {
const {getByRole} = HTMLRender(<TextInput loading />)
expect(getByRole('textbox')).toHaveAccessibleDescription('Loading')
})

it('should include the leadingVisual and loading indicator as part of the input accessible description', () => {
const {getByRole} = HTMLRender(
<TextInput loading loaderText="Loading search items" loaderPosition="trailing" leadingVisual="Search" />,
)
expect(getByRole('textbox')).toHaveAccessibleDescription('Search Loading search items')
})

it('should include the trailingVisual and loading indicator as part of the input accessible description', () => {
const {getByRole} = HTMLRender(<TextInput loading loaderPosition="leading" trailingVisual="Search" />)
expect(getByRole('textbox')).toHaveAccessibleDescription('Search Loading')
})

it('should not have an aria-describedby if there is no leadingVisual, trailingVisual, or loading indicator', () => {
const {getByRole} = HTMLRender(<TextInput />)
expect(getByRole('textbox')).not.toHaveAttribute('aria-describedby')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ exports[`snapshots renders a loading state 1`] = `
>
<svg
aria-hidden={true}
aria-labelledby=":r1v:"
aria-labelledby=":r2h:"
className="c3"
fill="none"
height="32px"
Expand All @@ -404,7 +404,7 @@ exports[`snapshots renders a loading state 1`] = `
</svg>
<span
className="c4"
id=":r1v:"
id=":r2h:"
>
Loading
</span>
Expand Down
Loading

0 comments on commit 9936add

Please sign in to comment.