Skip to content

fix(Combobox): add aria attributes on error #2327

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.

Sign up for GitHub

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

Open
chornonoh-vova wants to merge 1 commit into
base: develop
Choose a base branch
from
+194 −19
Open

fix(Combobox): add aria attributes on error #2327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
174 changes: 174 additions & 0 deletions packages/react/src/components/Combobox/Combobox.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1909,3 +1909,177 @@ test('should have no axe violations with description and error when expanded', a
const results = await axe(comboboxRef.current!);
expect(results).toHaveNoViolations();
});

test('should set aria-invalid on combobox input when error is provided', () => {
render(
<Combobox label="label" error="This field is required">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

expect(screen.getByRole('combobox')).toBeInvalid();
});

test('should not set aria-invalid on combobox input when no error is provided', () => {
render(
<Combobox label="label">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

expect(screen.getByRole('combobox')).not.toBeInvalid();
});

test('should update aria-invalid when error prop is dynamically added', () => {
const { rerender } = render(
<Combobox label="label">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

expect(screen.getByRole('combobox')).not.toBeInvalid();

rerender(
<Combobox label="label" error="This field is required">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

expect(screen.getByRole('combobox')).toBeInvalid();
});

test('should update aria-invalid when error prop is dynamically removed', () => {
const { rerender } = render(
<Combobox label="label" error="This field is required">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

expect(screen.getByRole('combobox')).toBeInvalid();

rerender(
<Combobox label="label">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

expect(screen.getByRole('combobox')).not.toBeInvalid();
});

test('should set aria-describedby on listbox pointing to error id when error is provided', () => {
render(
<Combobox label="label" error="This field is required" id="combo">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

const listbox = screen.getByRole('listbox');
expect(listbox).toHaveAttribute(
'aria-describedby',
expect.stringContaining('combo-error')
);
});

test('should set aria-describedby on listbox pointing to both description and error ids', () => {
render(
<Combobox
label="label"
description="This is a helpful description"
error="This field is required"
id="combo"
>
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

const listbox = screen.getByRole('listbox');
const ariaDescribedby = listbox.getAttribute('aria-describedby');
expect(ariaDescribedby).toContain('combo-description');
expect(ariaDescribedby).toContain('combo-error');
});

test('should not set aria-describedby on listbox when no error or description is provided', () => {
render(
<Combobox label="label">
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

const listbox = screen.getByRole('listbox');
expect(listbox).not.toHaveAttribute('aria-describedby');
});

test('should set aria-describedby on listbox including external aria-describedby', () => {
render(
<Combobox
label="label"
error="This field is required"
aria-describedby="external-id"
id="combo"
>
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

const listbox = screen.getByRole('listbox');
const ariaDescribedby = listbox.getAttribute('aria-describedby');
expect(ariaDescribedby).toContain('external-id');
expect(ariaDescribedby).toContain('combo-error');
});

test('should have matching aria-describedby on both combobox input and listbox', () => {
render(
<Combobox
label="label"
description="This is a helpful description"
error="This field is required"
aria-describedby="external-id"
id="combo"
>
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

const combobox = screen.getByRole('combobox');
const listbox = screen.getByRole('listbox');
expect(combobox.getAttribute('aria-describedby')).toBe(
listbox.getAttribute('aria-describedby')
);
});

test('should have no axe violations with error', async () => {
const comboboxRef = createRef<HTMLDivElement>();
render(
<Combobox label="label" error="This field is required" ref={comboboxRef}>
<ComboboxOption>Apple</ComboboxOption>
<ComboboxOption>Banana</ComboboxOption>
<ComboboxOption>Cantaloupe</ComboboxOption>
</Combobox>
);

expect(comboboxRef.current).toBeTruthy();
const results = await axe(comboboxRef.current!);
expect(results).toHaveNoViolations();
});
39 changes: 20 additions & 19 deletions packages/react/src/components/Combobox/Combobox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,10 @@ interface ComboboxOption {
removeOptionLabel?: string;
}

interface BaseComboboxProps
extends Omit<
React.InputHTMLAttributes<HTMLInputElement>,
'value' | 'defaultValue'
> {
interface BaseComboboxProps extends Omit<
React.InputHTMLAttributes<HTMLInputElement>,
'value' | 'defaultValue'
> {
label: ContentNode;
options?: ComboboxOption[];
requiredText?: React.ReactNode;
Expand Down Expand Up @@ -614,6 +613,20 @@ const Combobox = forwardRef<
<NoMatchingOptions />
);

const errorId = `${id}-error`;
const descriptionId = `${id}-description`;
let describedby = ariaDescribedby;
if (description) {
describedby = addIdRef(describedby, descriptionId);
}
if (error) {
describedby = addIdRef(describedby, errorId);
}
const inputProps = {
...props,
'aria-describedby': describedby
};

const comboboxListbox = (
// eslint-disable-next-line
// @ts-expect-error
Expand All @@ -625,6 +638,7 @@ const Combobox = forwardRef<
})}
role={noMatchingOptions ? 'presentation' : 'listbox'}
aria-labelledby={noMatchingOptions ? undefined : `${id}-label`}
aria-describedby={describedby}
id={`${id}-listbox`}
value={multiselect ? selectedValues : selectedValues[0]}
onMouseDown={handleComboboxOptionMouseDown}
Expand All @@ -642,20 +656,6 @@ const Combobox = forwardRef<
</Listbox>
);

const errorId = `${id}-error`;
const descriptionId = `${id}-description`;
let describedby = ariaDescribedby;
if (description) {
describedby = addIdRef(describedby, descriptionId);
}
if (error) {
describedby = addIdRef(describedby, errorId);
}
const inputProps = {
...props,
'aria-describedby': describedby
};

return (
<div
id={id}
Expand Down Expand Up @@ -735,6 +735,7 @@ const Combobox = forwardRef<
value={inputValue}
role="combobox"
disabled={disabled}
aria-invalid={error ? true : undefined}
aria-autocomplete={!isAutoComplete ? 'none' : 'list'}
aria-controls={`${id}-listbox`}
aria-expanded={open}
Expand Down
Loading