Skip to content

Commit

Permalink
707: Improved unit tests (and accessibility for the components themse…
Browse files Browse the repository at this point in the history
…lves) by making use of accessible roles and labels
  • Loading branch information
tombogle committed Feb 23, 2024
1 parent b5c3580 commit 9ea642a
Show file tree
Hide file tree
Showing 8 changed files with 481 additions and 477 deletions.
304 changes: 151 additions & 153 deletions lib/platform-bible-react/dist/index.cjs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lib/platform-bible-react/dist/index.cjs.map

Large diffs are not rendered by default.

590 changes: 292 additions & 298 deletions lib/platform-bible-react/dist/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lib/platform-bible-react/dist/index.js.map

Large diffs are not rendered by default.

16 changes: 14 additions & 2 deletions lib/platform-bible-react/src/components/grid-menu.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,17 @@ function MenuColumn(
}: MenuColumnProps & { className: string | undefined }, // Ensure className is provided and of type string
) {
return (
<Grid id={id} item xs="auto" className={`papi-menu-column ${className ?? ''}`}>
<h3 className={`papi-menu-column-header ${className ?? ''}`}>{metadata.label}</h3>
<Grid
id={id}
item
xs="auto"
role="menu"
aria-label={id}
className={`papi-menu-column ${className ?? ''}`}
>
<h3 aria-label={metadata.label} className={`papi-menu-column-header ${className ?? ''}`}>
{metadata.label}
</h3>
{/* It would seem as though this List component were unnecessary, since it only contains one
thing, but the "dense" property does affect the layout of the items (in a way I don't fully
understand). There might be a better way. */}
Expand All @@ -69,6 +78,7 @@ export default function GridMenu({
id,
}: GridMenuProps) {
const { columns } = multiColumnMenu;

const columnNumbers = new Map<number, ColumnInfo>();
Object.getOwnPropertyNames(columns).forEach((columnName: string) => {
// We know for sure there is a (boolean) property 'isExtensible' that we are not interested in.
Expand Down Expand Up @@ -100,6 +110,8 @@ export default function GridMenu({
spacing={0}
className={`papi-multi-column-menu ${className ?? ''}`}
columns={sortedColumns.length}
role="menu"
aria-label="GridMenu"
id={id}
>
{sortedColumns.map((col, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ describe('GridMenu', () => {
render(
<GridMenu multiColumnMenu={menuData.mainMenu} commandHandler={RememberLastMenuCommand} />,
);

const sendReceiveProjectsItem = screen.queryByText('%sendReceiveProjects%');
expect(sendReceiveProjectsItem).toBeDefined();

sendReceiveProjectsItem && fireEvent.click(sendReceiveProjectsItem);
const sendReceiveProjectsItem = screen.getByRole('menuitem', { name: '%sendReceiveProjects%' });

fireEvent.click(sendReceiveProjectsItem);

expect(numberOfCommandsHandled).toBe(1);
expect(lastCommandHandled).toBe('paratext.sendReceiveProjects');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { PlatformMenus } from 'platform-bible-utils';
import { render, screen } from '@testing-library/react';
import '@testing-library/jest-dom';
import { PropsWithChildren } from 'react';
import GridMenu from './grid-menu.component';
import { MouseEventHandler, PropsWithChildren } from 'react';
import NonValidatingDocumentCombiner from '../test-utils/non-validating-document-combiner';
import * as jsonMenu from './sample.composed.full.menu.json';

Expand All @@ -20,7 +20,6 @@ jest.mock('@mui/material', () => {
}: {
divider: boolean | undefined;
className: string | undefined;
onClick?: MouseEventHandler<HTMLLIElement> | undefined;
} & PropsWithChildren) => {
const dividerStyle = divider ? ' hasDivider' : '';
return (
Expand All @@ -32,9 +31,6 @@ jest.mock('@mui/material', () => {
};
});

function HandleMenuCommandNoOp() {
}

describe('GridMenu renders', () => {
const topMenuCombiner = new NonValidatingDocumentCombiner(jsonMenu, {
copyDocuments: false,
Expand All @@ -44,21 +40,26 @@ describe('GridMenu renders', () => {
// Assert the type that schema validation should have already sorted out
// eslint-disable-next-line no-type-assertion/no-type-assertion
const menuData = topMenuCombiner.output as PlatformMenus;
render(
<GridMenu multiColumnMenu={menuData.mainMenu} commandHandler={HandleMenuCommandNoOp} />,
);
render(<GridMenu multiColumnMenu={menuData.mainMenu} commandHandler={() => {}} />);

it('column label correctly', () => {
const expectedColumns = [
screen.queryByText('%mainMenu_Paratext%'),
screen.queryByText('%mainMenu_Window%'),
screen.queryByText('%mainMenu_Layout%'),
screen.queryByText('%mainMenu_Help%'),
{
html: screen.getByRole('menu', { name: 'paratext.paratext' }),
label: '%mainMenu_Paratext%',
},
{ html: screen.getByRole('menu', { name: 'platform.window' }), label: '%mainMenu_Window%' },
{ html: screen.getByRole('menu', { name: 'platform.layout' }), label: '%mainMenu_Layout%' },
{ html: screen.getByRole('menu', { name: 'platform.help' }), label: '%mainMenu_Help%' },
];

expectedColumns.forEach((column) => {
expect(column).toBeInTheDocument();
expect(column).toHaveAttribute(
const { html, label } = column;
expect(html).toBeInTheDocument();
const labelElement = screen.getByLabelText(label);
expect(html).toContainElement(labelElement);
expect(labelElement).toHaveTextContent(label);
expect(labelElement).toHaveAttribute(
'class',
expect.stringMatching(/\bpapi-menu-column-header\b/),
);
Expand Down Expand Up @@ -90,9 +91,9 @@ describe('GridMenu renders', () => {
});

it('the last group in a column without a final separator', () => {
const htmlForAddParatextVideoItem = allMenuItems.map((i) => i.outerHTML).find((html) => html &&
/%video_AddParatextVideo%/.test(html),
);
const htmlForAddParatextVideoItem = allMenuItems
.map((i) => i.outerHTML)
.find((html) => html && /%video_AddParatextVideo%/.test(html));

expect(htmlForAddParatextVideoItem).toBeDefined();
expect(htmlForAddParatextVideoItem).not.toMatch('hasDivider');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export default function GroupedMenuItemList(
const divKey = firstItem.item.group;

return (
<div key={divKey}>
<div key={divKey} role="menu" aria-label="divKey">
{/* I can't find anyway to "destructure" items.map. Asked ChatGPT, Google, and co-workers. */}
{/* eslint-disable-next-line react/destructuring-assignment */}
{items.map((itemInfo, index) => {
Expand Down

0 comments on commit 9ea642a

Please sign in to comment.