Skip to content

Commit f04609f

Browse files
Merge pull request #4055 from ProjectMirador/a11y-ids
Use useId to generate unique ids for accessibility attributes.
2 parents e5470d6 + c88eb43 commit f04609f

24 files changed

+98
-80
lines changed

__tests__/src/components/CanvasInfo.test.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ describe('CanvasInfo', () => {
1414
canvasLabel="The Canvas Label"
1515
canvasDescription="The Canvas Description"
1616
canvasMetadata={metadata}
17-
id="xyz"
1817
/>,
1918
);
2019
});
@@ -45,7 +44,7 @@ describe('CanvasInfo', () => {
4544
describe('when metadata is not present', () => {
4645
beforeEach(() => {
4746
render(
48-
<CanvasInfo id="xyz" />,
47+
<CanvasInfo />,
4948
);
5049
});
5150

__tests__/src/components/CollectionInfo.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { CollectionInfo } from '../../../src/components/CollectionInfo';
66
function createWrapper(props) {
77
return render(
88
<CollectionInfo
9-
id="test"
109
collectionPath={[1, 2]}
1110
showCollectionDialog={() => {}}
1211
{...props}

__tests__/src/components/ManifestInfo.test.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ describe('ManifestInfo', () => {
88
beforeEach(() => {
99
render(
1010
<ManifestInfo
11-
id="xyz"
1211
manifestLabel="The Manifest Label"
1312
manifestDescription="The Manifest Description"
1413
manifestMetadata={metadata}
@@ -42,7 +41,7 @@ describe('ManifestInfo', () => {
4241
describe('when metadata is not present', () => {
4342
beforeEach(() => {
4443
render(
45-
<ManifestInfo id="xyz" />,
44+
<ManifestInfo />,
4645
);
4746
});
4847

__tests__/src/components/WindowTopMenuButton.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,6 @@ describe('WindowTopMenuButton', () => {
4747
render(<Subject />);
4848
await user.click(screen.getByLabelText('Window views & thumbnail display'));
4949
// when 'open' is true, aria-owns is set to the id of the window
50-
expect(screen.getByLabelText('Window views & thumbnail display')).toHaveAttribute('aria-owns', 'window-menu_xyz'); // eslint-disable-line testing-library/no-node-access
50+
expect(screen.getByLabelText('Window views & thumbnail display')).toHaveAttribute('aria-owns'); // eslint-disable-line testing-library/no-node-access
5151
});
5252
});

__tests__/src/components/WorkspaceOptionsMenu.test.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,18 @@ describe('WorkspaceOptionsMenu', () => {
4848

4949
it('renders the export dialog when export option is clicked', async () => {
5050
render(<Subject anchorEl={screen.getByTestId('menu-trigger-button')} open />);
51-
expect(document.querySelector('#workspace-export')).not.toBeInTheDocument(); // eslint-disable-line testing-library/no-node-access
51+
expect(screen.queryByRole('heading', { name: 'Export workspace' })).not.toBeInTheDocument();
5252

53-
await user.click(screen.getAllByRole('menuitem')[0]);
54-
expect(document.querySelector('#workspace-export')).toBeInTheDocument(); // eslint-disable-line testing-library/no-node-access
53+
await user.click(screen.getByRole('menuitem', { name: 'Export workspace' }));
54+
expect(screen.getByRole('heading', { name: 'Export workspace' })).toBeInTheDocument();
5555
});
5656

57-
it('renders the import dialog when imporrt option is clicked', async () => {
57+
it('renders the import dialog when import option is clicked', async () => {
5858
render(<Subject anchorEl={screen.getByTestId('menu-trigger-button')} open />);
59-
expect(document.querySelector('#workspace-import')).not.toBeInTheDocument(); // eslint-disable-line testing-library/no-node-access
59+
expect(screen.queryByRole('heading', { name: 'Import workspace' })).not.toBeInTheDocument();
6060

61-
await user.click(screen.getAllByRole('menuitem')[1]);
62-
expect(document.querySelector('#workspace-import')).toBeInTheDocument(); // eslint-disable-line testing-library/no-node-access
61+
await user.click(screen.getByRole('menuitem', { name: 'Import workspace' }));
62+
expect(screen.getByRole('heading', { name: 'Import workspace' })).toBeInTheDocument();
6363
});
6464

6565
it('fires the correct callbacks on menu close', async () => {

src/components/CanvasInfo.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useId } from 'react';
12
import PropTypes from 'prop-types';
23
import Typography from '@mui/material/Typography';
34
import { useTranslation } from 'react-i18next';
@@ -13,24 +14,25 @@ export function CanvasInfo({
1314
canvasDescription = null,
1415
canvasLabel = null,
1516
canvasMetadata = [],
16-
id,
1717
index = 1,
1818
totalSize = 1,
1919
}) {
2020
const { t } = useTranslation();
21+
const id = useId();
22+
const titleId = useId();
2123
const pluginProps = arguments[0]; // eslint-disable-line prefer-rest-params
2224

2325
return (
2426
<CollapsibleSection
25-
id={`${id}-currentItem-${index}`}
27+
id={id}
2628
label={t('currentItem', { context: `${index + 1}/${totalSize}` })}
2729
>
2830
{canvasLabel && (
2931
<Typography
3032
aria-labelledby={
31-
`${id}-currentItem-${index} ${id}-currentItem-${index}-heading`
33+
`${id} ${titleId}`
3234
}
33-
id={`${id}-currentItem-${index}-heading`}
35+
id={titleId}
3436
variant="h4"
3537
component="h5"
3638
>
@@ -56,7 +58,6 @@ CanvasInfo.propTypes = {
5658
canvasDescription: PropTypes.string,
5759
canvasLabel: PropTypes.string,
5860
canvasMetadata: PropTypes.array, // eslint-disable-line react/forbid-prop-types
59-
id: PropTypes.string.isRequired,
6061
index: PropTypes.number,
6162
totalSize: PropTypes.number,
6263
};

src/components/CollectionInfo.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useId } from 'react';
12
import PropTypes from 'prop-types';
23
import Button from '@mui/material/Button';
34
import Typography from '@mui/material/Typography';
@@ -9,9 +10,11 @@ import CollapsibleSection from '../containers/CollapsibleSection';
910
* CollectionInfo
1011
*/
1112
export function CollectionInfo({
12-
collectionLabel = null, collectionPath = [], id, showCollectionDialog, windowId = null,
13+
collectionLabel = null, collectionPath = [], showCollectionDialog, windowId = null,
1314
}) {
1415
const { t } = useTranslation();
16+
const id = useId();
17+
const titleId = useId();
1518

1619
/**
1720
* Show the containing collection.
@@ -26,13 +29,13 @@ export function CollectionInfo({
2629

2730
return (
2831
<CollapsibleSection
29-
id={`${id}-collection`}
32+
id={id}
3033
label={t('collection')}
3134
>
3235
{collectionLabel && (
3336
<Typography
34-
aria-labelledby={`${id}-resource ${id}-resource-heading`}
35-
id={`${id}-resource-heading`}
37+
aria-labelledby={`${id} ${titleId}`}
38+
id={titleId}
3639
variant="h4"
3740
>
3841
{collectionLabel}
@@ -53,7 +56,6 @@ export function CollectionInfo({
5356
CollectionInfo.propTypes = {
5457
collectionLabel: PropTypes.string,
5558
collectionPath: PropTypes.arrayOf(PropTypes.string),
56-
id: PropTypes.string.isRequired,
5759
showCollectionDialog: PropTypes.func.isRequired,
5860
windowId: PropTypes.string,
5961
};

src/components/ManifestInfo.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useId } from 'react';
12
import PropTypes from 'prop-types';
23
import Typography from '@mui/material/Typography';
34
import { useTranslation } from 'react-i18next';
@@ -10,12 +11,13 @@ import { PluginHook } from './PluginHook';
1011
* ManifestInfo
1112
*/
1213
export function ManifestInfo({
13-
manifestDescription = null, manifestLabel = null, manifestMetadata = [], manifestSummary = null, id,
14+
manifestDescription = null, manifestLabel = null, manifestMetadata = [], manifestSummary = null,
1415
...rest
1516
}) {
1617
const { t } = useTranslation();
18+
const id = useId();
1719
const pluginProps = {
18-
id, manifestDescription, manifestLabel, manifestMetadata, manifestSummary, ...rest,
20+
manifestDescription, manifestLabel, manifestMetadata, manifestSummary, ...rest,
1921
};
2022

2123
return (
@@ -56,7 +58,6 @@ export function ManifestInfo({
5658
}
5759

5860
ManifestInfo.propTypes = {
59-
id: PropTypes.string.isRequired,
6061
manifestDescription: PropTypes.string,
6162
manifestLabel: PropTypes.string,
6263
manifestMetadata: PropTypes.array, // eslint-disable-line react/forbid-prop-types

src/components/ManifestRelatedLinks.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useId } from 'react';
12
import PropTypes from 'prop-types';
23
import { styled } from '@mui/material/styles';
34
import Typography from '@mui/material/Typography';
@@ -20,26 +21,29 @@ const StyledDl = styled('dl')(({ theme }) => ({
2021
*/
2122
export function ManifestRelatedLinks({
2223
homepage = null,
23-
id,
2424
manifestUrl = null,
2525
related = null,
2626
renderings = null,
2727
seeAlso = null,
2828
...rest
2929
}) {
3030
const { t } = useTranslation();
31+
const id = useId();
32+
const titleId = useId();
33+
3134
const pluginProps = {
32-
homepage, id, manifestUrl, related, renderings, seeAlso, t, ...rest,
35+
homepage, manifestUrl, related, renderings, seeAlso, t, ...rest,
3336
};
3437

3538
return (
3639
<CollapsibleSection
37-
id={`${id}-related`}
40+
aria-labelledby={titleId}
41+
id={id}
3842
label={t('related')}
3943
>
4044
<Typography
41-
aria-labelledby={`${id}-related ${id}-related-heading`}
42-
id={`${id}-related-heading`}
45+
aria-labelledby={`${id} ${titleId}`}
46+
id={titleId}
4347
variant="h4"
4448
component="h5"
4549
>
@@ -129,7 +133,6 @@ ManifestRelatedLinks.propTypes = {
129133
label: PropTypes.string,
130134
value: PropTypes.string,
131135
})),
132-
id: PropTypes.string.isRequired,
133136
manifestUrl: PropTypes.string,
134137
related: PropTypes.arrayOf(PropTypes.shape({
135138
format: PropTypes.string,

src/components/SearchHit.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useMemo } from 'react';
1+
import { useEffect, useId, useMemo } from 'react';
22
import { useEffectEvent } from 'use-effect-event';
33
import PropTypes from 'prop-types';
44
import Button from '@mui/material/Button';
@@ -86,11 +86,12 @@ export function SearchHit({
8686
);
8787
});
8888

89+
const canvasLabelHtmlId = useId();
90+
8991
if (focused && !selected) return null;
9092

9193
const renderedHit = focused ? hit : hit && truncatedHit;
9294
const truncated = hit && (renderedHit.before !== hit.before || renderedHit.after !== hit.after);
93-
const canvasLabelHtmlId = `${companionWindowId}-${index}`;
9495
const ownerState = {
9596
adjacent, focused, selected, windowSelected,
9697
};

0 commit comments

Comments
 (0)