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.

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

feat(Row): improve VoiceOver compatibility #1079

Merged
merged 6 commits into from
Apr 29, 2024
Merged

feat(Row): improve VoiceOver compatibility #1079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2aa6b6e
WEB-1724 feat(Row): improve VoiceOver compatibility
atabel Apr 11, 2024
2fc4a14
fix test
atabel Apr 11, 2024
0b3e27d
fix titleAs in Rows
atabel Apr 12, 2024
f9dede2
increase sheet tests timeout
atabel Apr 12, 2024
f74083f
Merge branch 'master' into atoledano-screen-reader
atabel Apr 19, 2024
d67f944
improve reading order and extract extra text for link aria-label
atabel Apr 19, 2024
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
148 changes: 145 additions & 3 deletions src/__tests__/list-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import {
IconPlayFilled,
IconShopRegular,
IconTrashCanRegular,
Stack,
Tag,
Text2,
ThemeContextProvider,
} from '..';
import {makeTheme} from './test-utils';
Expand Down Expand Up @@ -152,9 +155,8 @@ test('Row list with radio buttons', async () => {
</ThemeContextProvider>
);

const radioBanana = screen.getByRole('radio', {name: 'Banana'});
// alternate accessible selector
const radioApple = screen.getByLabelText('Apple');
const radioBanana = screen.getByRole('radio', {name: 'Banana bananabanana'});
const radioApple = screen.getByRole('radio', {name: 'Apple appleapple'});

expect(radioBanana).not.toBeChecked();
expect(radioApple).not.toBeChecked();
Expand Down Expand Up @@ -309,3 +311,143 @@ test('Row list with iconButton', async () => {
expect(iconButtonOnPressSpy).toHaveBeenCalledTimes(1);
expect(logEventSpy).toHaveBeenCalledWith({name: 'icon-button-tracking-event'});
});

test('Text content is read by screen readers in the right order in Rows with link', () => {
render(
<ThemeContextProvider theme={makeTheme()}>
<RowList>
<Row
headline={<Tag type="promo">Headline</Tag>}
title="Title"
subtitle="Subtitle"
description="Description"
detail="Detail"
extra={
<Stack space={4}>
<Text2 regular>Extra line 1</Text2>
<Text2 regular>Extra line 2</Text2>
</Stack>
}
href="/"
/>
</RowList>
</ThemeContextProvider>
);

const row = screen.getByRole('link', {
// WARN: There should be a space between the extra lines, but jsdom doesn't support .innerText method, so we fallback to .textContent https://github.com/jsdom/jsdom/issues/1245
name: 'Title Headline Subtitle Description Extra line 1Extra line 2 Detail',
});
expect(row).toBeInTheDocument();
});

test('Text content is read by screen readers in the right order in Rows with button', () => {
render(
<ThemeContextProvider theme={makeTheme()}>
<RowList>
<Row
headline={<Tag type="promo">Headline</Tag>}
title="Title"
subtitle="Subtitle"
description="Description"
detail="Detail"
extra={
<Stack space={4}>
<Text2 regular>Extra line 1</Text2>
<Text2 regular>Extra line 2</Text2>
</Stack>
}
onPress={() => {}}
/>
</RowList>
</ThemeContextProvider>
);

const row = screen.getByRole('button', {
name: 'Title Headline Subtitle Description Extra line 1 Extra line 2 Detail',
});
expect(row).toBeInTheDocument();
});

test('Text content is read by screen readers in the right order in Rows with checkbox', () => {
render(
<ThemeContextProvider theme={makeTheme()}>
<RowList>
<Row
headline={<Tag type="promo">Headline</Tag>}
title="Title"
subtitle="Subtitle"
description="Description"
extra={
<Stack space={4}>
<Text2 regular>Extra line 1</Text2>
<Text2 regular>Extra line 2</Text2>
</Stack>
}
checkbox={{defaultValue: false}}
/>
</RowList>
</ThemeContextProvider>
);

const row = screen.getByRole('checkbox', {
name: 'Title Headline Subtitle Description Extra line 1 Extra line 2',
});
expect(row).toBeInTheDocument();
});

test('Text content is read by screen readers in the right order in Rows with switch', () => {
render(
<ThemeContextProvider theme={makeTheme()}>
<RowList>
<Row
headline={<Tag type="promo">Headline</Tag>}
title="Title"
subtitle="Subtitle"
description="Description"
extra={
<Stack space={4}>
<Text2 regular>Extra line 1</Text2>
<Text2 regular>Extra line 2</Text2>
</Stack>
}
switch={{defaultValue: false}}
/>
</RowList>
</ThemeContextProvider>
);

const row = screen.getByRole('switch', {
name: 'Title Headline Subtitle Description Extra line 1 Extra line 2',
});
expect(row).toBeInTheDocument();
});

test('Text content is read by screen readers in the right order in Rows with radio', () => {
render(
<ThemeContextProvider theme={makeTheme()}>
<RadioGroup name="radio-group">
<RowList>
<Row
headline={<Tag type="promo">Headline</Tag>}
title="Title"
subtitle="Subtitle"
description="Description"
extra={
<Stack space={4}>
<Text2 regular>Extra line 1</Text2>
<Text2 regular>Extra line 2</Text2>
</Stack>
}
radioValue="radio1"
/>
</RowList>
</RadioGroup>
</ThemeContextProvider>
);

const row = screen.getByRole('radio', {
name: 'Title Headline Subtitle Description Extra line 1 Extra line 2',
});
expect(row).toBeInTheDocument();
});
26 changes: 13 additions & 13 deletions src/__tests__/sheet-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test('Sheet', async () => {

await userEvent.click(closeButton);
await waitForElementToBeRemoved(sheet);
}, 20000);
}, 30000);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's hope it doesn't fail again :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤞


test('RadioListSheet', async () => {
const selectSpy = jest.fn();
Expand Down Expand Up @@ -95,7 +95,7 @@ test('RadioListSheet', async () => {

await waitForElementToBeRemoved(sheet);
expect(selectSpy).toHaveBeenCalledWith('1');
}, 20000);
}, 30000);

test('ActionsListSheet', async () => {
const selectSpy = jest.fn();
Expand Down Expand Up @@ -144,7 +144,7 @@ test('ActionsListSheet', async () => {

await waitForElementToBeRemoved(sheet);
expect(selectSpy).toHaveBeenCalledWith('1');
}, 20000);
}, 30000);

test('InfoSheet', async () => {
const TestComponent = () => {
Expand Down Expand Up @@ -193,7 +193,7 @@ test('InfoSheet', async () => {

const items = await within(itemList).findAllByRole('listitem');
expect(items).toHaveLength(2);
}, 20000);
}, 30000);

test('ActionsSheet', async () => {
const onPressButtonSpy = jest.fn();
Expand Down Expand Up @@ -249,7 +249,7 @@ test('ActionsSheet', async () => {

await waitForElementToBeRemoved(sheet);
expect(onPressButtonSpy).toHaveBeenCalledWith('SECONDARY');
}, 20000);
}, 30000);

test('showSheet INFO', async () => {
const resultSpy = jest.fn();
Expand Down Expand Up @@ -277,7 +277,7 @@ test('showSheet INFO', async () => {

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith(undefined);
}, 20000);
}, 30000);

test('showSheet ACTIONS_LIST', async () => {
const resultSpy = jest.fn();
Expand Down Expand Up @@ -309,7 +309,7 @@ test('showSheet ACTIONS_LIST', async () => {

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith({action: 'SUBMIT', selectedId: '2'});
}, 20000);
}, 30000);

test('showSheet ACTIONS_LIST dismiss', async () => {
const resultSpy = jest.fn();
Expand Down Expand Up @@ -340,7 +340,7 @@ test('showSheet ACTIONS_LIST dismiss', async () => {

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith({action: 'DISMISS'});
}, 20000);
}, 30000);

test('showSheet RADIO_LIST', async () => {
const resultSpy = jest.fn();
Expand Down Expand Up @@ -374,7 +374,7 @@ test('showSheet RADIO_LIST', async () => {

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith({action: 'SUBMIT', selectedId: '2'});
}, 20000);
}, 30000);

test('showSheet RADIO_LIST dismiss', async () => {
const resultSpy = jest.fn();
Expand Down Expand Up @@ -405,7 +405,7 @@ test('showSheet RADIO_LIST dismiss', async () => {

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith({action: 'DISMISS'});
}, 20000);
}, 30000);

test('showSheet ACTIONS', async () => {
const resultSpy = jest.fn();
Expand Down Expand Up @@ -444,7 +444,7 @@ test('showSheet ACTIONS', async () => {

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith({action: 'LINK'});
}, 20000);
}, 30000);

test('showSheet ACTIONS dismiss', async () => {
const resultSpy = jest.fn();
Expand Down Expand Up @@ -476,7 +476,7 @@ test('showSheet ACTIONS dismiss', async () => {

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith({action: 'DISMISS'});
}, 20000);
}, 30000);

test('showSheet fails if SheetRoot is not rendered', async () => {
await expect(
Expand Down Expand Up @@ -743,4 +743,4 @@ test('showSheet with native implementation fallbacks to web if native fails', as

await waitForElementToBeRemoved(sheet);
expect(resultSpy).toHaveBeenCalledWith({action: 'LINK'});
}, 20000);
}, 30000);
3 changes: 3 additions & 0 deletions src/box.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type Props = {
/** "data-" prefix is automatically added. For example, use "testid" instead of "data-testid" */
dataAttributes?: DataAttributes;
'aria-label'?: string;
'aria-hidden'?: React.HTMLAttributes<HTMLAnchorElement>['aria-hidden'];
};

const Box = React.forwardRef<HTMLDivElement, Props>(
Expand All @@ -40,6 +41,7 @@ const Box = React.forwardRef<HTMLDivElement, Props>(
role,
dataAttributes,
'aria-label': ariaLabel,
'aria-hidden': ariaHidden,
},
ref
) => {
Expand All @@ -64,6 +66,7 @@ const Box = React.forwardRef<HTMLDivElement, Props>(
{...getPrefixedDataAttributes(dataAttributes)}
role={role}
aria-label={ariaLabel}
aria-hidden={ariaHidden}
ref={ref}
className={classnames(className, paddingClasses)}
style={{
Expand Down
4 changes: 2 additions & 2 deletions src/icons/icon-error.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const IconErrorO2 = ({size = 48}: Props): JSX.Element => {
const {platformOverrides} = useTheme();

return (
<svg width={size} height={size} viewBox="0 0 64 64" overflow="visible">
<svg role="presentation" width={size} height={size} viewBox="0 0 64 64" overflow="visible">
<g
stroke={vars.colors.error}
fill={vars.colors.error}
Expand Down Expand Up @@ -62,7 +62,7 @@ const IconErrorDefault = ({size = 48}: Props): JSX.Element => {
const {platformOverrides} = useTheme();

return (
<svg width={size} height={size} viewBox="0 0 64 64" overflow="visible">
<svg role="presentation" width={size} height={size} viewBox="0 0 64 64" overflow="visible">
<g
stroke={vars.colors.error}
fill="none"
Expand Down
4 changes: 2 additions & 2 deletions src/icons/icon-info.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type Props = {

const IconInfoO2 = ({size = 48}: Props): JSX.Element => {
return (
<svg width={size} height={size} viewBox="0 0 64 64">
<svg role="presentation" width={size} height={size} viewBox="0 0 64 64">
<g
stroke={vars.colors.brand}
fill={vars.colors.brand}
Expand All @@ -32,7 +32,7 @@ const IconInfoO2 = ({size = 48}: Props): JSX.Element => {

const IconInfoDefault = ({size = 48}: Props): JSX.Element => {
return (
<svg width={size} height={size} viewBox="0 0 64 64">
<svg role="presentation" width={size} height={size} viewBox="0 0 64 64">
<g fill={vars.colors.brand}>
<path
fillRule="nonzero"
Expand Down
2 changes: 1 addition & 1 deletion src/icons/icon-success-vivo-new.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const IconSuccessVivoNew = ({size = 48}: Props): JSX.Element => {
const gradientId = useAriaId();

return (
<svg width={size} height={size} viewBox="0 0 64 64" fill="none">
<svg role="presentation" width={size} height={size} viewBox="0 0 64 64" fill="none">
<circle
cx="32"
cy="32"
Expand Down
2 changes: 1 addition & 1 deletion src/icons/icon-success-vivo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const IconSuccessVivo = ({size = 48}: Props): JSX.Element => {
const {platformOverrides} = useTheme();

return (
<svg width={size} height={size} viewBox="0 0 64 64" overflow="visible">
<svg role="presentation" width={size} height={size} viewBox="0 0 64 64" overflow="visible">
<g
fill={isInverse ? vars.colors.inverse : vars.colors.brand}
stroke={isInverse ? vars.colors.inverse : vars.colors.brand}
Expand Down
Loading
Loading