From 91dd518f245c6ac505657c6c30387d6a9aae1783 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 9 Sep 2024 15:17:14 -0400 Subject: [PATCH 01/17] Add `aria-describedby` to `TextInput` when `LeadingVisual` === `string` --- packages/react/src/TextInput/TextInput.tsx | 9 ++++++++- .../internal/components/TextInputInnerVisualSlot.tsx | 10 ++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/react/src/TextInput/TextInput.tsx b/packages/react/src/TextInput/TextInput.tsx index e2d2acd0736..c077dda66e8 100644 --- a/packages/react/src/TextInput/TextInput.tsx +++ b/packages/react/src/TextInput/TextInput.tsx @@ -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' @@ -96,6 +96,7 @@ const TextInput = React.forwardRef( const focusInput: MouseEventHandler = () => { inputRef.current?.focus() } + const leadingVisualId = useId() const handleInputFocus = useCallback( (e: React.FocusEvent) => { setIsInputFocused(true) @@ -137,6 +138,7 @@ const TextInput = React.forwardRef( visualPosition="leading" showLoadingIndicator={showLeadingLoadingIndicator} hasLoadingIndicator={typeof loading === 'boolean'} + id={typeof LeadingVisual === 'string' ? leadingVisualId : undefined} > {typeof LeadingVisual !== 'string' && isValidElementType(LeadingVisual) ? : LeadingVisual} @@ -149,6 +151,11 @@ const TextInput = React.forwardRef( aria-required={required} aria-invalid={validationStatus === 'error' ? 'true' : undefined} {...inputProps} + aria-describedby={ + typeof LeadingVisual === 'string' + ? `${leadingVisualId} ${inputProps['aria-describedby']}` + : inputProps['aria-describedby'] + } data-component="input" /> -> = ({children, hasLoadingIndicator, showLoadingIndicator, visualPosition}) => { +> = ({children, hasLoadingIndicator, showLoadingIndicator, visualPosition, id}) => { if ((!children && !hasLoadingIndicator) || (visualPosition === 'leading' && !children && !showLoadingIndicator)) { return null } if (!hasLoadingIndicator) { - return {children} + return ( + + {children} + + ) } return ( From 2725c7d628a0ee279b36ac35c7731dd48f9b94b2 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 9 Sep 2024 16:00:10 -0400 Subject: [PATCH 02/17] Update to include `TrailingVisual`, icons --- .../TextInput/TextInput.features.stories.tsx | 56 +++++++++++-------- packages/react/src/TextInput/TextInput.tsx | 19 +++++-- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/packages/react/src/TextInput/TextInput.features.stories.tsx b/packages/react/src/TextInput/TextInput.features.stories.tsx index 672be82b4e7..253daaafc10 100644 --- a/packages/react/src/TextInput/TextInput.features.stories.tsx +++ b/packages/react/src/TextInput/TextInput.features.stories.tsx @@ -94,31 +94,39 @@ export const Required = () => ( ) -export const WithLeadingVisual = () => ( - - - Default label - - - - Enter monies - - - -) +export const WithLeadingVisual = () => { + const Checkmark = () => -export const WithTrailingIcon = () => ( - - - Default label - - - - Enter monies - - - -) + return ( + + + Default label + + + + Enter monies + + + + ) +} + +export const WithTrailingIcon = () => { + const Checkmark = () => + + return ( + + + Default label + + + + Enter monies + + + + ) +} export const WithTrailingAction = () => { const [value, setValue] = useState('sample text') diff --git a/packages/react/src/TextInput/TextInput.tsx b/packages/react/src/TextInput/TextInput.tsx index c077dda66e8..7a93f751a21 100644 --- a/packages/react/src/TextInput/TextInput.tsx +++ b/packages/react/src/TextInput/TextInput.tsx @@ -97,6 +97,16 @@ const TextInput = React.forwardRef( inputRef.current?.focus() } const leadingVisualId = useId() + const trailingVisualId = useId() + + const inputDescribedBy = [ + inputProps['aria-describedby'], + LeadingVisual && leadingVisualId, + TrailingVisual && trailingVisualId, + ] + .filter(String) + .join('') + const handleInputFocus = useCallback( (e: React.FocusEvent) => { setIsInputFocused(true) @@ -138,7 +148,7 @@ const TextInput = React.forwardRef( visualPosition="leading" showLoadingIndicator={showLeadingLoadingIndicator} hasLoadingIndicator={typeof loading === 'boolean'} - id={typeof LeadingVisual === 'string' ? leadingVisualId : undefined} + id={leadingVisualId} > {typeof LeadingVisual !== 'string' && isValidElementType(LeadingVisual) ? : LeadingVisual} @@ -151,17 +161,14 @@ const TextInput = React.forwardRef( aria-required={required} aria-invalid={validationStatus === 'error' ? 'true' : undefined} {...inputProps} - aria-describedby={ - typeof LeadingVisual === 'string' - ? `${leadingVisualId} ${inputProps['aria-describedby']}` - : inputProps['aria-describedby'] - } + aria-describedby={inputDescribedBy} data-component="input" /> {typeof TrailingVisual !== 'string' && isValidElementType(TrailingVisual) ? ( From f754eda7508d23bd5e93c7b5f99f7c2bb98625b3 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 9 Sep 2024 16:28:21 -0400 Subject: [PATCH 03/17] Add tests, update snapshots --- .../TextInput/TextInput.features.stories.tsx | 2 +- packages/react/src/TextInput/TextInput.tsx | 12 +- .../react/src/__tests__/TextInput.test.tsx | 46 +++++++ .../__snapshots__/Autocomplete.test.tsx.snap | 4 +- .../__snapshots__/TextInput.test.tsx.snap | 117 +++++++++++------- 5 files changed, 125 insertions(+), 56 deletions(-) diff --git a/packages/react/src/TextInput/TextInput.features.stories.tsx b/packages/react/src/TextInput/TextInput.features.stories.tsx index 253daaafc10..474515242a2 100644 --- a/packages/react/src/TextInput/TextInput.features.stories.tsx +++ b/packages/react/src/TextInput/TextInput.features.stories.tsx @@ -112,7 +112,7 @@ export const WithLeadingVisual = () => { } export const WithTrailingIcon = () => { - const Checkmark = () => + const Checkmark = () => return ( diff --git a/packages/react/src/TextInput/TextInput.tsx b/packages/react/src/TextInput/TextInput.tsx index 7a93f751a21..bdd78ada214 100644 --- a/packages/react/src/TextInput/TextInput.tsx +++ b/packages/react/src/TextInput/TextInput.tsx @@ -99,13 +99,11 @@ const TextInput = React.forwardRef( const leadingVisualId = useId() const trailingVisualId = useId() - const inputDescribedBy = [ - inputProps['aria-describedby'], - LeadingVisual && leadingVisualId, - TrailingVisual && trailingVisualId, - ] - .filter(String) - .join('') + const inputDescribedBy = + [inputProps['aria-describedby'], LeadingVisual && leadingVisualId, TrailingVisual && trailingVisualId] + .filter(String) + .join(' ') + .trim() || undefined const handleInputFocus = useCallback( (e: React.FocusEvent) => { diff --git a/packages/react/src/__tests__/TextInput.test.tsx b/packages/react/src/__tests__/TextInput.test.tsx index 866e00cb09f..d21613d7be8 100644 --- a/packages/react/src/__tests__/TextInput.test.tsx +++ b/packages/react/src/__tests__/TextInput.test.tsx @@ -236,4 +236,50 @@ describe('TextInput', () => { const {getByRole} = HTMLRender() expect(getByRole('textbox')).toHaveAttribute('aria-invalid', 'true') }) + + it('should include the leadingVisual as part of the input accessible description', () => { + const {getByRole} = HTMLRender() + expect(getByRole('textbox')).toHaveAccessibleDescription('Search') + }) + + it('should include the leadingVisual icon as part of the input accessible description', () => { + const Icon = () => + + const {getByRole} = HTMLRender() + const icon = getByRole('img') + + 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() + expect(getByRole('textbox')).toHaveAccessibleDescription('Search') + }) + + it('should include the trailingVisual icon as part of the input accessible description', () => { + const Icon = () => + + const {getByRole} = HTMLRender() + const icon = getByRole('img') + + 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() + expect(getByRole('textbox')).toHaveAccessibleDescription('$ Currency') + }) + + it('should keep the passed aria-describedby value', () => { + const {getByRole} = HTMLRender( + <> + value + + , + ) + expect(getByRole('textbox').getAttribute('aria-describedby')).toContain('passedValue') + expect(getByRole('textbox')).toHaveAccessibleDescription('value leading trailing') + }) }) diff --git a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap index 9f40a07fbc8..da52e114226 100644 --- a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap @@ -378,7 +378,7 @@ exports[`snapshots renders a loading state 1`] = ` > Loading diff --git a/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap index abf6f01ebad..a1d3bd001d8 100644 --- a/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap @@ -795,6 +795,7 @@ exports[`TextInput renders leadingVisual 1`] = ` >
Trailing
Trailing