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.

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

Use useId to generate unique ids for accessibility attributes. #4055

Merged
merged 1 commit into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions __tests__/src/components/CanvasInfo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ describe('CanvasInfo', () => {
canvasLabel="The Canvas Label"
canvasDescription="The Canvas Description"
canvasMetadata={metadata}
id="xyz"
/>,
);
});
Expand Down Expand Up @@ -45,7 +44,7 @@ describe('CanvasInfo', () => {
describe('when metadata is not present', () => {
beforeEach(() => {
render(
<CanvasInfo id="xyz" />,
<CanvasInfo />,
);
});

Expand Down
1 change: 0 additions & 1 deletion __tests__/src/components/CollectionInfo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { CollectionInfo } from '../../../src/components/CollectionInfo';
function createWrapper(props) {
return render(
<CollectionInfo
id="test"
collectionPath={[1, 2]}
showCollectionDialog={() => {}}
{...props}
Expand Down
3 changes: 1 addition & 2 deletions __tests__/src/components/ManifestInfo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ describe('ManifestInfo', () => {
beforeEach(() => {
render(
<ManifestInfo
id="xyz"
manifestLabel="The Manifest Label"
manifestDescription="The Manifest Description"
manifestMetadata={metadata}
Expand Down Expand Up @@ -42,7 +41,7 @@ describe('ManifestInfo', () => {
describe('when metadata is not present', () => {
beforeEach(() => {
render(
<ManifestInfo id="xyz" />,
<ManifestInfo />,
);
});

Expand Down
2 changes: 1 addition & 1 deletion __tests__/src/components/WindowTopMenuButton.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ describe('WindowTopMenuButton', () => {
render(<Subject />);
await user.click(screen.getByLabelText('Window views & thumbnail display'));
// when 'open' is true, aria-owns is set to the id of the window
expect(screen.getByLabelText('Window views & thumbnail display')).toHaveAttribute('aria-owns', 'window-menu_xyz'); // eslint-disable-line testing-library/no-node-access
expect(screen.getByLabelText('Window views & thumbnail display')).toHaveAttribute('aria-owns'); // eslint-disable-line testing-library/no-node-access
});
});
14 changes: 7 additions & 7 deletions __tests__/src/components/WorkspaceOptionsMenu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,18 @@ describe('WorkspaceOptionsMenu', () => {

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

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

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

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

it('fires the correct callbacks on menu close', async () => {
Expand Down
11 changes: 6 additions & 5 deletions src/components/CanvasInfo.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useId } from 'react';
import PropTypes from 'prop-types';
import Typography from '@mui/material/Typography';
import { useTranslation } from 'react-i18next';
Expand All @@ -13,24 +14,25 @@ export function CanvasInfo({
canvasDescription = null,
canvasLabel = null,
canvasMetadata = [],
id,
index = 1,
totalSize = 1,
}) {
const { t } = useTranslation();
const id = useId();
const titleId = useId();
const pluginProps = arguments[0]; // eslint-disable-line prefer-rest-params

return (
<CollapsibleSection
id={`${id}-currentItem-${index}`}
id={id}
label={t('currentItem', { context: `${index + 1}/${totalSize}` })}
>
{canvasLabel && (
<Typography
aria-labelledby={
`${id}-currentItem-${index} ${id}-currentItem-${index}-heading`
`${id} ${titleId}`
}
id={`${id}-currentItem-${index}-heading`}
id={titleId}
variant="h4"
component="h5"
>
Expand All @@ -56,7 +58,6 @@ CanvasInfo.propTypes = {
canvasDescription: PropTypes.string,
canvasLabel: PropTypes.string,
canvasMetadata: PropTypes.array, // eslint-disable-line react/forbid-prop-types
id: PropTypes.string.isRequired,
index: PropTypes.number,
totalSize: PropTypes.number,
};
12 changes: 7 additions & 5 deletions src/components/CollectionInfo.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useId } from 'react';
import PropTypes from 'prop-types';
import Button from '@mui/material/Button';
import Typography from '@mui/material/Typography';
Expand All @@ -9,9 +10,11 @@
* CollectionInfo
*/
export function CollectionInfo({
collectionLabel = null, collectionPath = [], id, showCollectionDialog, windowId = null,
collectionLabel = null, collectionPath = [], showCollectionDialog, windowId = null,
}) {
const { t } = useTranslation();
const id = useId();
const titleId = useId();

/**
* Show the containing collection.
Expand All @@ -26,13 +29,13 @@

return (
<CollapsibleSection
id={`${id}-collection`}
id={id}
label={t('collection')}
>
{collectionLabel && (
<Typography
aria-labelledby={`${id}-resource ${id}-resource-heading`}
id={`${id}-resource-heading`}
aria-labelledby={`${id} ${titleId}`}
id={titleId}

Check warning on line 38 in src/components/CollectionInfo.js

View check run for this annotation

Codecov / codecov/patch

src/components/CollectionInfo.js#L37-L38

Added lines #L37 - L38 were not covered by tests
variant="h4"
>
{collectionLabel}
Expand All @@ -53,7 +56,6 @@
CollectionInfo.propTypes = {
collectionLabel: PropTypes.string,
collectionPath: PropTypes.arrayOf(PropTypes.string),
id: PropTypes.string.isRequired,
showCollectionDialog: PropTypes.func.isRequired,
windowId: PropTypes.string,
};
7 changes: 4 additions & 3 deletions src/components/ManifestInfo.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useId } from 'react';
import PropTypes from 'prop-types';
import Typography from '@mui/material/Typography';
import { useTranslation } from 'react-i18next';
Expand All @@ -10,12 +11,13 @@ import { PluginHook } from './PluginHook';
* ManifestInfo
*/
export function ManifestInfo({
manifestDescription = null, manifestLabel = null, manifestMetadata = [], manifestSummary = null, id,
manifestDescription = null, manifestLabel = null, manifestMetadata = [], manifestSummary = null,
...rest
}) {
const { t } = useTranslation();
const id = useId();
const pluginProps = {
id, manifestDescription, manifestLabel, manifestMetadata, manifestSummary, ...rest,
manifestDescription, manifestLabel, manifestMetadata, manifestSummary, ...rest,
};

return (
Expand Down Expand Up @@ -56,7 +58,6 @@ export function ManifestInfo({
}

ManifestInfo.propTypes = {
id: PropTypes.string.isRequired,
manifestDescription: PropTypes.string,
manifestLabel: PropTypes.string,
manifestMetadata: PropTypes.array, // eslint-disable-line react/forbid-prop-types
Expand Down
15 changes: 9 additions & 6 deletions src/components/ManifestRelatedLinks.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useId } from 'react';
import PropTypes from 'prop-types';
import { styled } from '@mui/material/styles';
import Typography from '@mui/material/Typography';
Expand All @@ -20,26 +21,29 @@ const StyledDl = styled('dl')(({ theme }) => ({
*/
export function ManifestRelatedLinks({
homepage = null,
id,
manifestUrl = null,
related = null,
renderings = null,
seeAlso = null,
...rest
}) {
const { t } = useTranslation();
const id = useId();
const titleId = useId();

const pluginProps = {
homepage, id, manifestUrl, related, renderings, seeAlso, t, ...rest,
homepage, manifestUrl, related, renderings, seeAlso, t, ...rest,
};

return (
<CollapsibleSection
id={`${id}-related`}
aria-labelledby={titleId}
id={id}
label={t('related')}
>
<Typography
aria-labelledby={`${id}-related ${id}-related-heading`}
id={`${id}-related-heading`}
aria-labelledby={`${id} ${titleId}`}
id={titleId}
variant="h4"
component="h5"
>
Expand Down Expand Up @@ -129,7 +133,6 @@ ManifestRelatedLinks.propTypes = {
label: PropTypes.string,
value: PropTypes.string,
})),
id: PropTypes.string.isRequired,
manifestUrl: PropTypes.string,
related: PropTypes.arrayOf(PropTypes.shape({
format: PropTypes.string,
Expand Down
5 changes: 3 additions & 2 deletions src/components/SearchHit.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useMemo } from 'react';
import { useEffect, useId, useMemo } from 'react';
import { useEffectEvent } from 'use-effect-event';
import PropTypes from 'prop-types';
import Button from '@mui/material/Button';
Expand Down Expand Up @@ -86,11 +86,12 @@ export function SearchHit({
);
});

const canvasLabelHtmlId = useId();

if (focused && !selected) return null;

const renderedHit = focused ? hit : hit && truncatedHit;
const truncated = hit && (renderedHit.before !== hit.before || renderedHit.after !== hit.after);
const canvasLabelHtmlId = `${companionWindowId}-${index}`;
const ownerState = {
adjacent, focused, selected, windowSelected,
};
Expand Down
7 changes: 4 additions & 3 deletions src/components/WindowListButton.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState } from 'react';
import { useId, useState } from 'react';
import PropTypes from 'prop-types';
import BookmarksIcon from '@mui/icons-material/BookmarksSharp';
import { useTranslation } from 'react-i18next';
Expand All @@ -11,6 +11,7 @@ import MiradorMenuButton from '../containers/MiradorMenuButton';
export function WindowListButton({ disabled = false, windowCount }) {
const { t } = useTranslation();
const [windowListAnchor, setWindowListAnchor] = useState(null);
const id = useId();

/** */
const handleClose = () => { setWindowListAnchor(null); };
Expand All @@ -22,7 +23,7 @@ export function WindowListButton({ disabled = false, windowCount }) {
<MiradorMenuButton
aria-haspopup="true"
aria-label={t('listAllOpenWindows')}
aria-owns={windowListAnchor ? 'window-list' : null}
aria-owns={windowListAnchor ? id : null}
selected={Boolean(windowListAnchor)}
disabled={disabled}
badge
Expand All @@ -42,7 +43,7 @@ export function WindowListButton({ disabled = false, windowCount }) {
{Boolean(windowListAnchor) && (
<WindowList
anchorEl={windowListAnchor}
id="window-list"
id={id}
open={Boolean(windowListAnchor)}
handleClose={handleClose}
/>
Expand Down
11 changes: 6 additions & 5 deletions src/components/WindowSideBarCanvasPanel.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useRef } from 'react';
import { useId, useRef } from 'react';
import PropTypes from 'prop-types';
import { styled } from '@mui/material/styles';
import Tabs from '@mui/material/Tabs';
Expand Down Expand Up @@ -49,6 +49,7 @@ export function WindowSideBarCanvasPanel({
}) {
const { t } = useTranslation();
const containerRef = useRef();
const tabPanelId = useId();

/** */
const handleSequenceChange = (event) => {
Expand Down Expand Up @@ -126,15 +127,15 @@ export function WindowSideBarCanvasPanel({
textColor="primary"
>
{showToc && (
<Tooltip title={t('tableOfContentsList')} value="tableOfContents"><Tab sx={{ minWidth: 'auto' }} value="tableOfContents" aria-label={t('tableOfContentsList')} aria-controls={`tab-panel-${id}`} icon={<TocIcon style={{ transform: 'scale(-1, 1)' }} />} /></Tooltip>
<Tooltip title={t('tableOfContentsList')} value="tableOfContents"><Tab sx={{ minWidth: 'auto' }} value="tableOfContents" aria-label={t('tableOfContentsList')} aria-controls={tabPanelId} icon={<TocIcon style={{ transform: 'scale(-1, 1)' }} />} /></Tooltip>
)}
<Tooltip title={t('itemList')} value="item"><Tab sx={{ minWidth: 'auto' }} value="item" aria-label={t('itemList')} aria-controls={`tab-panel-${id}`} icon={<ItemListIcon />} /></Tooltip>
<Tooltip title={t('thumbnailList')} value="thumbnail"><Tab sx={{ minWidth: 'auto' }} value="thumbnail" aria-label={t('thumbnailList')} aria-controls={`tab-panel-${id}`} icon={<ThumbnailListIcon />} /></Tooltip>
<Tooltip title={t('itemList')} value="item"><Tab sx={{ minWidth: 'auto' }} value="item" aria-label={t('itemList')} aria-controls={tabPanelId} icon={<ItemListIcon />} /></Tooltip>
<Tooltip title={t('thumbnailList')} value="thumbnail"><Tab sx={{ minWidth: 'auto' }} value="thumbnail" aria-label={t('thumbnailList')} aria-controls={tabPanelId} icon={<ThumbnailListIcon />} /></Tooltip>
</Tabs>
</>
)}
>
<div id={`tab-panel-${id}`}>
<div id={tabPanelId}>
{ collection && (
<Button
fullWidth
Expand Down
8 changes: 4 additions & 4 deletions src/components/WindowSideBarInfoPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
key={canvasId}
>
<CanvasInfo
id={id}
canvasId={canvasId}
companionWindowId={id}
index={index}
totalSize={canvasIds.length}
windowId={windowId}
Expand All @@ -57,16 +57,16 @@
}
{ collectionPath.length > 0 && (
<CompanionWindowSection>
<CollectionInfo id={id} windowId={windowId} />
<CollectionInfo companionWindowId={id} windowId={windowId} />

Check warning on line 60 in src/components/WindowSideBarInfoPanel.js

View check run for this annotation

Codecov / codecov/patch

src/components/WindowSideBarInfoPanel.js#L60

Added line #L60 was not covered by tests
</CompanionWindowSection>
)}

<CompanionWindowSection>
<ManifestInfo id={id} windowId={windowId} />
<ManifestInfo companionWindowId={id} windowId={windowId} />
</CompanionWindowSection>

<CompanionWindowSection>
<ManifestRelatedLinks id={id} windowId={windowId} />
<ManifestRelatedLinks companionWindowId={id} windowId={windowId} />
</CompanionWindowSection>
</CompanionWindow>
);
Expand Down
4 changes: 2 additions & 2 deletions src/components/WindowTopBarPluginMenu.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useContext, useState } from 'react';
import { useContext, useId, useState } from 'react';
import PropTypes from 'prop-types';
import MoreVertIcon from '@mui/icons-material/MoreVertSharp';
import Menu from '@mui/material/Menu';
Expand All @@ -18,6 +18,7 @@ export function WindowTopBarPluginMenu({
const pluginProps = arguments[0]; // eslint-disable-line prefer-rest-params
const [anchorEl, setAnchorEl] = useState(null);
const [open, setOpen] = useState(false);
const windowPluginMenuId = useId();

/** */
const handleMenuClick = (event) => {
Expand All @@ -31,7 +32,6 @@ export function WindowTopBarPluginMenu({
setOpen(false);
};

const windowPluginMenuId = `window-plugin-menu_${windowId}`;
if (!PluginComponents || PluginComponents.length === 0) return null;

return (
Expand Down
4 changes: 2 additions & 2 deletions src/components/WindowTopMenuButton.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState } from 'react';
import { useId, useState } from 'react';
import PropTypes from 'prop-types';
import { useTranslation } from 'react-i18next';
import WindowTopMenu from '../containers/WindowTopMenu';
Expand All @@ -11,6 +11,7 @@ export function WindowTopMenuButton({ classes = {}, windowId }) {
const { t } = useTranslation();
const [anchorEl, setAnchorEl] = useState(null);
const [open, setOpen] = useState(false);
const menuId = useId();

/** */
const handleMenuClick = (event) => {
Expand All @@ -24,7 +25,6 @@ export function WindowTopMenuButton({ classes = {}, windowId }) {
setOpen(false);
};

const menuId = `window-menu_${windowId}`;
return (
<>
<MiradorMenuButton
Expand Down
Loading
Loading