diff --git a/CHANGELOG.md b/CHANGELOG.md index 92ca7f27b..d0b8de9f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,19 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ## [Unreleased] +### Added + +- `` + - `role` property: `menu` (default), `navigation`, `list`, `listbox`, `none` or `presentation` get special support, other ARIA roles are set as they are + - `navigation` is rendered as `list`, the `nav` wrapper must be added by yourself + - sets `roleStructure` of `` children automatically according to the `role` if it is not given, otherwise a console warning is emitted if it does not fit + - emits a console warning if an explicitly set `role` of a `` child does not fit to its (explicitly or automatically set) `roleStructure` + - sets `role="listitem"` (lists) or `role="none"` (listboxes) on `` children, and `aria-hidden` if the divider has no `title`; explicitly set values are kept, except `role="separator"` +- `` + - accepts all attributes of the `li` element, e.g. `role` and `aria-hidden` +- `` + - `role` property: `menu` (default) or `listbox`, forwarded to the included `` and used for `aria-haspopup` of the toggler + ### Fixed - Fix misused menu roles in selection popups and overlays diff --git a/src/components/ContextOverlay/ContextMenu.stories.tsx b/src/components/ContextOverlay/ContextMenu.stories.tsx index 98b59a751..b04b66e13 100644 --- a/src/components/ContextOverlay/ContextMenu.stories.tsx +++ b/src/components/ContextOverlay/ContextMenu.stories.tsx @@ -23,11 +23,12 @@ const Template: StoryFn = (args) => ( export const Default = Template.bind({}); Default.args = { + role: "menu", children: [ - , - - - + , + + + , ], }; diff --git a/src/components/ContextOverlay/ContextMenu.tsx b/src/components/ContextOverlay/ContextMenu.tsx index 1966851d2..3bfcd39a7 100644 --- a/src/components/ContextOverlay/ContextMenu.tsx +++ b/src/components/ContextOverlay/ContextMenu.tsx @@ -5,7 +5,7 @@ import { CLASSPREFIX as eccgui } from "../../configuration/constants"; import { ValidIconName } from "../Icon/canonicalIconNames"; import { IconButton, IconButtonProps } from "../Icon/IconButton"; import { TestableComponent } from "../interfaces"; -import Menu from "../Menu/Menu"; +import Menu, { MenuProps } from "../Menu/Menu"; import ContextOverlay, { ContextOverlayProps } from "./ContextOverlay"; @@ -19,6 +19,12 @@ export interface ContextMenuProps extends TestableComponent { * They will be wrapped in a `Menu` element automatically. */ children?: React.JSX.Element | React.JSX.Element[]; + /** + * Role of the included `Menu` element. + * It defines the `roleStructure` of the `MenuItem` children, e.g. use `listbox` for selectable options. + * It is also used for `aria-haspopup` of the toggler element. + */ + role?: "menu" | "listbox"; /** * Toggler that need to be used to display menu. * If a valid icon name is used then the icon element is displayed. @@ -46,6 +52,11 @@ export interface ContextMenuProps extends TestableComponent { * Props to spread to `ContextOverlay` that is used to display the dropdown. */ contextOverlayProps?: Partial>; + /** + * Props to spread to the included `Menu` element, e.g. additional `aria-*` attributes. + * Use the `role` property of `ContextMenu` to set the role of the menu. + */ + menuProps?: Partial>; /** * Disables the button to open the menu. */ @@ -63,9 +74,11 @@ export interface ContextMenuProps extends TestableComponent { export const ContextMenu = ({ children, className = "", + role = "menu", togglerElement = "item-moremenu", togglerText = "Show more options", contextOverlayProps, + menuProps, disabled, togglerLarge = false, togglerSize, @@ -94,11 +107,15 @@ export const ContextMenu = ({ return ( {children}} + content={ + + {children} + + } + popupKind={role === "listbox" ? PopupKind.LISTBOX : PopupKind.MENU} disabled={!!disabled} usePlaceholder={!preventPlaceholder} > diff --git a/src/components/ContextOverlay/tests/ContextMenu.test.tsx b/src/components/ContextOverlay/tests/ContextMenu.test.tsx index 00ad29ad3..021b9ccc1 100644 --- a/src/components/ContextOverlay/tests/ContextMenu.test.tsx +++ b/src/components/ContextOverlay/tests/ContextMenu.test.tsx @@ -4,6 +4,7 @@ import { fireEvent, render, screen } from "@testing-library/react"; import "@testing-library/jest-dom"; import { CLASSPREFIX as eccgui } from "../../../configuration/constants"; +import MenuItem from "../../Menu/MenuItem"; import ContextMenu from "./../ContextMenu"; import { Default as ContextMenuStory } from "./../ContextMenu.stories"; @@ -43,4 +44,49 @@ describe("ContextMenu", () => { expect(screen.getByRole("button")).toHaveAttribute("aria-haspopup", "menu"); expect(screen.getByRole("menu")).toBeVisible(); }); + it("should use role `menu` for the menu by default", async () => { + const { container } = render(); + fireEvent.click(container.getElementsByClassName(overlayWrapper)[0]); + expect(await screen.findByRole("menu")).toBeInTheDocument(); + }); + it("should forward `role` to the menu", async () => { + const { container } = render( + + + , + ); + fireEvent.click(container.getElementsByClassName(overlayWrapper)[0]); + expect(await screen.findByRole("listbox")).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "First option" })).toBeInTheDocument(); + }); + it("should forward `menuProps` to the menu", async () => { + const { container } = render( + + + , + ); + fireEvent.click(container.getElementsByClassName(overlayWrapper)[0]); + const listbox = await screen.findByRole("listbox"); + expect(listbox).toHaveAttribute("aria-multiselectable", "true"); + expect(listbox).toHaveAttribute("aria-label", "Options"); + expect(listbox).toHaveClass("custom-menu"); + }); + it.each([true, false])( + "should set `aria-haspopup` according to role `listbox` (preventPlaceholder: %s)", + async (preventPlaceholder) => { + const { container } = render( + + + , + ); + expect(screen.getByRole("button")).toHaveAttribute("aria-haspopup", "listbox"); + fireEvent.click(container.getElementsByClassName(overlayWrapper)[0]); + expect(await screen.findByRole("listbox")).toBeVisible(); + expect(screen.getByRole("button")).toHaveAttribute("aria-haspopup", "listbox"); + }, + ); }); diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 5f6f8563c..e9935ea6e 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -1,14 +1,160 @@ -import React from "react"; -import { Menu as BlueprintMenu, MenuProps as BlueprintMenuProps } from "@blueprintjs/core"; +import React, { AriaRole } from "react"; +import { + Menu as BlueprintMenu, + MenuItem as BlueprintMenuItem, + MenuProps as BlueprintMenuProps, +} from "@blueprintjs/core"; import { CLASSPREFIX as eccgui } from "../../configuration/constants"; -export type MenuProps = BlueprintMenuProps; +import MenuDivider, { MenuDividerProps } from "./MenuDivider"; +import MenuItem, { MenuItemProps } from "./MenuItem"; + +/** + * Roles that are supported by the automatic `roleStructure` handling of the `MenuItem` children. + */ +type SupportedMenuRole = "menu" | "navigation" | "list" | "listbox" | "none" | "presentation"; + +export interface MenuProps extends BlueprintMenuProps { + /** + * Sets the ARIA role of the menu. + * It may become mandatory in a future version. + * Setting a `role` checks whether `MenuItem` and `MenuDivider` children use a correct `roleStructure`, or sets it automatically. + * The automatic checks only work if `MenuItem` and `MenuDivider` are used as direct children or inside a `Fragment`. + * The checks only work for `menu`, `navigation`, `list`, `listbox`, `none` and `presentation`. + * Other roles are applied as they are, and the children are not processed. + */ + role?: SupportedMenuRole | AriaRole; +} + +const roleMapping: Partial> = { + navigation: "list", +}; + +const allowedRoleStructure: Record[]> = { + menu: ["menuitem"], + navigation: ["listitem"], + list: ["listitem"], + listbox: ["listoption"], + none: ["none"], + presentation: ["none"], +}; + +/** + * Allowed values for an explicitly set `role` on `MenuItem` children, regarding their (explicitly or automatically set) `roleStructure`. + * Blueprint sets this `role` on the inner target element of the item, where it overwrites the role defined by `roleStructure`, + * so it must fit to the role of the `li` element that is defined by `roleStructure`. + * A `roleStructure` without entry does not restrict the item role. + */ +const allowedItemRole: Partial, AriaRole[]>> = { + menuitem: ["menuitem", "menuitemcheckbox", "menuitemradio"], + listitem: ["link", "button", "none", "presentation"], + // the `li` element already has the `option` role, so the target element must not have an own role + listoption: ["none", "presentation"], +}; + +/** + * `MenuDivider` children get this role if their `role="separator"` is not allowed inside the (mapped) menu role. + */ +const dividerRoleMapping: Partial> = { + list: "listitem", + listbox: "none", +}; + +/** + * Blueprint uses this `roleStructure` by default, so it does not need to be set explicitly. + */ +const defaultRoleStructure: MenuItemProps["roleStructure"] = "menuitem"; + +/** + * Represents a list of items, used as menu, navigation, options, or only for presentation. + * It sets automatically `roleStructure` for its direct `MenuItem` children (also inside fragments) regarding the set `role`, + * or it emits a console warning if it does not fit to an already set `roleStructure`. + * It also emits a console warning if an explicitly set `role` of a `MenuItem` does not fit to its `roleStructure`. + * On `MenuDivider` children without role or with `role="separator"` it sets `role="listitem"` in lists + * (`role="list"` or `role="navigation"`) and `role="none"` in listboxes, because `separator` is not allowed there. + * It also sets `aria-hidden` then if the divider has no `title` and no own `aria-hidden` value. + * It does not add a `nav` wrapper on `role="navigation"`, you need to do it on your own. + */ +export const Menu = ({ children, className = "", role = "menu", ...restProps }: MenuProps) => { + const isSupportedRole = Object.prototype.hasOwnProperty.call(allowedRoleStructure, role); + const roleMapped = (isSupportedRole && roleMapping[role as SupportedMenuRole]) || role; + const subroles = isSupportedRole ? allowedRoleStructure[role as SupportedMenuRole] : undefined; + const dividerRole = Object.prototype.hasOwnProperty.call(dividerRoleMapping, roleMapped) + ? dividerRoleMapping[roleMapped] + : undefined; + + // warnings are collected while rendering and emitted by an effect, so they are not repeated on each re-render + const warnings: string[] = []; + + const processItems = ( + nodes: React.ReactNode, + subroles: NonNullable[], + ): React.ReactNode => + React.Children.map(nodes, (child) => { + if (React.isValidElement<{ children?: React.ReactNode }>(child) && child.type === React.Fragment) { + return {processItems(child.props.children, subroles)}; + } + if (React.isValidElement(child) && child.type === MenuDivider) { + if (!dividerRole) { + return child; + } + const { role: ownRole, title, "aria-hidden": ariaHidden } = child.props; + return React.cloneElement(child, { + // `separator` is not allowed here, other explicitly set roles are kept + ...(!ownRole || ownRole === "separator" ? { role: dividerRole } : {}), + ...(!title && ariaHidden === undefined ? { "aria-hidden": true } : {}), + }); + } + if ( + !React.isValidElement(child) || + (child.type !== MenuItem && child.type !== BlueprintMenuItem) + ) { + return child; + } + const { roleStructure, role: itemRole } = child.props; + const roleStructureUsed = roleStructure ?? subroles[0]; + const itemRoles = allowedItemRole[roleStructureUsed]; + if (itemRole && itemRoles && !itemRoles.includes(itemRole)) { + warnings.push( + `Menu: MenuItem with role="${itemRole}" does not fit to its roleStructure="${roleStructureUsed}"${ + roleStructure ? "" : ` (set automatically for Menu role="${role}")` + }, allowed: ${itemRoles.map((allowedRole) => `"${allowedRole}"`).join(", ")}.`, + ); + } + if (!roleStructure) { + // prevent unnecessary cloning if Blueprint's default is used anyway + return subroles[0] === defaultRoleStructure + ? child + : React.cloneElement(child, { roleStructure: subroles[0] }); + } + if (!subroles.includes(roleStructure)) { + warnings.push( + `Menu: MenuItem with roleStructure="${roleStructure}" does not fit into Menu with role="${role}", allowed: ${subroles + .map((subrole) => `"${subrole}"`) + .join(", ")}.`, + ); + } + return child; + }); + + const items = subroles ? processItems(children, subroles) : children; + + // only emit warnings again if they changed + const warningsKey = warnings.join("\n"); + React.useEffect(() => { + if (!warningsKey) { + return; + } + warningsKey.split("\n").forEach((warning) => { + // eslint-disable-next-line no-console + console.warn(warning); + }); + }, [warningsKey]); -export const Menu = ({ children, className = "", ...restProps }: MenuProps) => { return ( - - {children} + + {items} ); }; diff --git a/src/components/Menu/MenuDivider.tsx b/src/components/Menu/MenuDivider.tsx index 27385dccc..17a900e6b 100644 --- a/src/components/Menu/MenuDivider.tsx +++ b/src/components/Menu/MenuDivider.tsx @@ -1,15 +1,40 @@ import React from "react"; -import { MenuDivider as BlueprintMenuDivider, MenuDividerProps as BlueprintMenuDividerProps } from "@blueprintjs/core"; +import { + Classes as BlueprintClasses, + H6 as BlueprintSeparatorTitle, + MenuDividerProps as BlueprintMenuDividerProps, +} from "@blueprintjs/core"; +import classNames from "classnames"; import { CLASSPREFIX as eccgui } from "../../configuration/constants"; -export type MenuDividerProps = BlueprintMenuDividerProps; +/** + * Properties of the Blueprint `MenuDivider` and all attributes of the `li` element. + * Use `role="listitem"` if the parent `Menu` is a list, because `separator` is not allowed there. + * `Menu` sets it automatically, and `aria-hidden` for dividers without `title`. + */ +export interface MenuDividerProps + extends + BlueprintMenuDividerProps, + Omit, "title" | "children" | "className"> {} -export const MenuDivider = ({ children, className = "", ...restProps }: MenuDividerProps) => { +/** + * Divider or section header inside `Menu`. + * It uses the same markup and classes like the Blueprint `MenuDivider`, but supports other roles than `separator`. + */ +export const MenuDivider = ({ className, role = "separator", title, titleId, ...otherLiProps }: MenuDividerProps) => { return ( - - {children} - +
  • + {title && {title}} +
  • ); }; diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index e34b2a4e6..74a415ac1 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -34,10 +34,18 @@ export interface MenuItemProps * Visual intent color to apply to element. */ intent?: BlueprintMenuItemProps["intent"] | "accent"; + /** + * Defines the ARIA role structure that is used for the menu item element and its inner target element. + * It is recommended to set it, even if it is optional right now. + * The value is checked if a `role` is set on the parent `Menu`. + * It may become mandatory in a future version. + */ + roleStructure?: BlueprintMenuItemProps["roleStructure"]; } /** * Single item, used as child inside `Menu`. + * It is recommended to use it as a direct child and not to wrap it inside other elements, except `Fragment`. */ export const MenuItem = React.forwardRef( ({ children, className = "", icon, onClick, href, text, tooltip, intent, ...restProps }, ref) => { diff --git a/src/components/Menu/Stories/Menu.stories.tsx b/src/components/Menu/Stories/Menu.stories.tsx index 2c2d4d4b7..699b9ec87 100644 --- a/src/components/Menu/Stories/Menu.stories.tsx +++ b/src/components/Menu/Stories/Menu.stories.tsx @@ -25,11 +25,47 @@ const MenuExample: StoryFn = (args) => ( export const Default = MenuExample.bind({}); Default.args = { + role: "menu", children: ( <> {ContentMenuStory.args.children} - + ), }; + +/** + * Use `role="navigation"` for a list of links, e.g. as main or sidebar navigation. + * The `MenuItem` elements need `roleStructure="listitem"`, it is set automatically if missing. + * `MenuDivider` elements need `role="listitem"` because `separator` is not allowed inside a list, + * it is set automatically if missing. Dividers without `title` get `aria-hidden` automatically. + * `Menu` does not add a `nav` element, so you need to wrap it on your own. + */ +const NavigationExample: StoryFn = (args) => ( + + + +); + +export const Navigation = NavigationExample.bind({}); +Navigation.args = { + role: "navigation", + children: [ + , + , + , + , + , + ], +}; diff --git a/src/components/Menu/Stories/MenuItem.stories.tsx b/src/components/Menu/Stories/MenuItem.stories.tsx index f7e9e121b..024e8fc4b 100644 --- a/src/components/Menu/Stories/MenuItem.stories.tsx +++ b/src/components/Menu/Stories/MenuItem.stories.tsx @@ -29,7 +29,7 @@ export default { const MenuExample: StoryFn = (args) => ( - + @@ -39,10 +39,11 @@ export const Default = MenuExample.bind({}); Default.args = { children: ( <> - - + + ), text: "Parent option", + roleStructure: "menuitem", key: "m4", }; diff --git a/src/components/Menu/tests/Menu.test.tsx b/src/components/Menu/tests/Menu.test.tsx new file mode 100644 index 000000000..8fad19c2a --- /dev/null +++ b/src/components/Menu/tests/Menu.test.tsx @@ -0,0 +1,634 @@ +import React from "react"; +import { Classes as BlueprintClasses, MenuItem as BlueprintMenuItem } from "@blueprintjs/core"; +import { render } from "@testing-library/react"; + +import "@testing-library/jest-dom"; + +import { CLASSPREFIX as eccgui } from "../../../configuration/constants"; +import Menu, { MenuProps } from "../Menu"; +import MenuDivider from "../MenuDivider"; +import MenuItem, { MenuItemProps } from "../MenuItem"; + +type MenuRole = NonNullable; +type RoleStructure = NonNullable; + +/** + * Blueprint renders `roleStructure` as roles on the `li` and the inner target element. + * Each `roleStructure` value results in an unique combination. + */ +const expectedDomRoles: Record = { + menuitem: { li: "none", target: "menuitem" }, + listitem: { li: null, target: null }, + listoption: { li: "option", target: null }, + none: { li: "none", target: null }, +}; + +const getDomRoles = (container: HTMLElement) => + Array.from(container.querySelectorAll(`.${eccgui}-menu__item`)).map((target) => ({ + li: target.closest("li")!.getAttribute("role"), + target: target.getAttribute("role"), + })); + +describe("Menu", () => { + let warnSpy: jest.SpyInstance; + + beforeEach(() => { + warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + describe("role", () => { + it("should use role 'menu' by default", () => { + const { container } = render( + + + , + ); + const list = container.querySelector(`.${eccgui}-menu__list`); + expect(list).toHaveAttribute("role", "menu"); + }); + + it("should map role 'navigation' to 'list'", () => { + const { container } = render( + + + , + ); + const list = container.querySelector(`.${eccgui}-menu__list`); + expect(list).toHaveAttribute("role", "list"); + }); + + it.each(["menu", "list", "listbox", "none", "presentation"])( + "should keep role '%s' unchanged", + (role) => { + const { container } = render( + + + , + ); + const list = container.querySelector(`.${eccgui}-menu__list`); + expect(list).toHaveAttribute("role", role); + }, + ); + + it.each(["group", "tree", "toString"])( + "should render unsupported role '%s' as it is without processing the children", + (role) => { + const { container } = render( + + + + + , + ); + const list = container.querySelector(`.${eccgui}-menu__list`); + expect(list).toHaveAttribute("role", role); + // children keep their own or Blueprint's default `roleStructure` + expect(getDomRoles(container)).toEqual([expectedDomRoles.listoption, expectedDomRoles.menuitem]); + expect(container.querySelector(`.${eccgui}-menu__divider`)).toHaveAttribute("role", "separator"); + expect(warnSpy).not.toHaveBeenCalled(); + }, + ); + + it("should keep custom class names", () => { + const { container } = render(); + const list = container.querySelector(`.${eccgui}-menu__list`); + expect(list).toHaveClass("custom-class"); + }); + }); + + describe("roleStructure autoset", () => { + it.each<[MenuRole, RoleStructure]>([ + ["menu", "menuitem"], + ["navigation", "listitem"], + ["list", "listitem"], + ["listbox", "listoption"], + ["none", "none"], + ["presentation", "none"], + ])("should set roleStructure for role '%s' to '%s' if not set", (role, expectedRoleStructure) => { + const { container } = render( + + + + , + ); + expect(getDomRoles(container)).toEqual([ + expectedDomRoles[expectedRoleStructure], + expectedDomRoles[expectedRoleStructure], + ]); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("should not clone MenuItems if Blueprint's default roleStructure 'menuitem' is used anyway", () => { + const cloneSpy = jest.spyOn(React, "cloneElement"); + try { + const { container } = render( + + + + , + ); + expect(getDomRoles(container)).toEqual([expectedDomRoles.menuitem, expectedDomRoles.menuitem]); + expect(cloneSpy).not.toHaveBeenCalled(); + } finally { + cloneSpy.mockRestore(); + } + }); + + it("should set roleStructure also for Blueprint MenuItem children", () => { + const { container } = render( + + + , + ); + expect(getDomRoles(container)).toEqual([expectedDomRoles.listoption]); + }); + + it("should not overwrite an allowed roleStructure", () => { + const { container } = render( + + + , + ); + expect(getDomRoles(container)).toEqual([expectedDomRoles.listitem]); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("should only set roleStructure on items without one", () => { + const { container } = render( + + + + , + ); + expect(getDomRoles(container)).toEqual([expectedDomRoles.listitem, expectedDomRoles.listitem]); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("should set roleStructure for MenuItems nested in fragments", () => { + const { container } = render( + + <> + + <> + + + {[, ]} + + , + ); + expect(getDomRoles(container)).toEqual(Array(4).fill(expectedDomRoles.listoption)); + }); + + it("should leave other children untouched", () => { + const { container } = render( + + + +
  • custom
  • + {"text"} + {null} +
    , + ); + expect(getDomRoles(container)).toEqual([expectedDomRoles.listitem]); + expect(container.querySelector(".custom-child")).toBeInTheDocument(); + expect(container).toHaveTextContent("text"); + expect(warnSpy).not.toHaveBeenCalled(); + }); + }); + + describe("MenuDivider", () => { + const getDividers = (container: HTMLElement) => + Array.from(container.querySelectorAll(`.${eccgui}-menu__divider`)); + + it.each(["list", "navigation"])( + "should set role 'listitem' and aria-hidden on divider without title for role '%s'", + (role) => { + const { container } = render( + + + + + , + ); + const [divider] = getDividers(container); + expect(divider.tagName).toBe("LI"); + expect(divider).toHaveAttribute("role", "listitem"); + expect(divider).toHaveAttribute("aria-hidden", "true"); + expect(divider).toHaveClass(BlueprintClasses.MENU_DIVIDER); + }, + ); + + it.each(["list", "navigation"])( + "should set role 'listitem' but no aria-hidden on divider with title for role '%s'", + (role) => { + const { container } = render( + + + + + , + ); + const [divider] = getDividers(container); + expect(divider).toHaveAttribute("role", "listitem"); + expect(divider).not.toHaveAttribute("aria-hidden"); + expect(divider).toHaveClass(BlueprintClasses.MENU_HEADER); + expect(divider.querySelector("h6")).toHaveTextContent("Section"); + expect(divider.querySelector("h6")).toHaveAttribute("id", "section-title"); + }, + ); + + it("should set role 'listitem' also on dividers inside fragments", () => { + const { container } = render( + + <> + + + + + , + ); + const [divider] = getDividers(container); + expect(divider).toHaveAttribute("role", "listitem"); + expect(divider).toHaveAttribute("aria-hidden", "true"); + }); + + it("should overwrite an explicit separator role in lists", () => { + const { container } = render( + + + , + ); + expect(getDividers(container)[0]).toHaveAttribute("role", "listitem"); + }); + + it("should keep other explicitly set roles in lists", () => { + const { container } = render( + + + , + ); + expect(getDividers(container)[0]).toHaveAttribute("role", "presentation"); + }); + + it("should keep an explicitly set aria-hidden value in lists", () => { + const { container } = render( + + + + , + ); + const [withoutTitle, withTitle] = getDividers(container); + expect(withoutTitle).toHaveAttribute("aria-hidden", "false"); + expect(withoutTitle).toHaveAttribute("role", "listitem"); + expect(withTitle).toHaveAttribute("aria-hidden", "true"); + }); + + it("should set role 'none' and aria-hidden on divider without title for role 'listbox'", () => { + const { container } = render( + + + + + + + , + ); + const [withoutRole, withSeparator, withTitle] = getDividers(container); + expect(withoutRole).toHaveAttribute("role", "none"); + expect(withoutRole).toHaveAttribute("aria-hidden", "true"); + expect(withSeparator).toHaveAttribute("role", "none"); + expect(withSeparator).toHaveAttribute("aria-hidden", "true"); + expect(withTitle).toHaveAttribute("role", "none"); + expect(withTitle).not.toHaveAttribute("aria-hidden"); + }); + + it("should keep explicitly set divider values for role 'listbox'", () => { + const { container } = render( + + + , + ); + const [divider] = getDividers(container); + expect(divider).toHaveAttribute("role", "presentation"); + expect(divider).toHaveAttribute("aria-hidden", "false"); + }); + + it.each(["menu", "none", "presentation"])( + "should keep role 'separator' on divider for role '%s'", + (role) => { + const { container } = render( + + + + , + ); + getDividers(container).forEach((divider) => { + expect(divider).toHaveAttribute("role", "separator"); + expect(divider).not.toHaveAttribute("aria-hidden"); + }); + }, + ); + + it("should render all children of the list as listitems", () => { + const { container } = render( + + + + + + + , + ); + const list = container.querySelector(`.${eccgui}-menu__list`)!; + Array.from(list.children).forEach((child) => { + expect(child.tagName).toBe("LI"); + expect([null, "listitem"]).toContain(child.getAttribute("role")); + }); + }); + }); + + describe("MenuItem role check", () => { + it.each<[RoleStructure, string]>([ + ["menuitem", "menuitem"], + ["menuitem", "menuitemcheckbox"], + ["menuitem", "menuitemradio"], + ["listitem", "link"], + ["listitem", "button"], + ["listitem", "none"], + ["listitem", "presentation"], + ["listoption", "none"], + ["listoption", "presentation"], + ])("should not warn for roleStructure '%s' with allowed item role '%s'", (roleStructure, itemRole) => { + render( + + + , + ); + // only the roleStructure warning is expected because it does not fit into `role="none"` + expect( + warnSpy.mock.calls.filter((call) => call[0].includes(`MenuItem with role="${itemRole}"`)), + ).toHaveLength(0); + }); + + it.each<[RoleStructure, string]>([ + ["menuitem", "link"], + ["menuitem", "option"], + ["listitem", "menuitem"], + ["listitem", "listitem"], + ["listitem", "option"], + ["listoption", "option"], + ["listoption", "menuitemcheckbox"], + ])("should warn for roleStructure '%s' with not allowed item role '%s'", (roleStructure, itemRole) => { + render( + + + , + ); + const roleWarnings = warnSpy.mock.calls + .map((call) => call[0] as string) + .filter((warning) => warning.includes(`MenuItem with role="${itemRole}"`)); + expect(roleWarnings).toHaveLength(1); + expect(roleWarnings[0]).toContain(`does not fit to its roleStructure="${roleStructure}",`); + expect(roleWarnings[0]).not.toContain("set automatically"); + }); + + it.each<[MenuRole, RoleStructure, string]>([ + ["menu", "menuitem", "link"], + ["navigation", "listitem", "menuitem"], + ["list", "listitem", "option"], + ["listbox", "listoption", "option"], + ])( + "should check the item role against the automatically set roleStructure for role '%s'", + (role, roleStructure, itemRole) => { + render( + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0][0]).toContain( + `MenuItem with role="${itemRole}" does not fit to its roleStructure="${roleStructure}" (set automatically for Menu role="${role}")`, + ); + }, + ); + + it("should check the item role against an explicitly set roleStructure, not against the menu role", () => { + render( + + + , + ); + const warnings = warnSpy.mock.calls.map((call) => call[0] as string); + expect(warnings).toHaveLength(2); + expect(warnings.some((warning) => warning.includes('roleStructure="listoption" does not fit'))).toBe(true); + expect( + warnings.some((warning) => + warning.includes('role="menuitemcheckbox" does not fit to its roleStructure="listoption"'), + ), + ).toBe(true); + }); + + it.each(["none", "presentation"])( + "should not restrict the item role for roleStructure 'none' (role '%s')", + (role) => { + render( + + + + , + ); + expect(warnSpy).not.toHaveBeenCalled(); + }, + ); + + it("should not check the item role for unsupported menu roles", () => { + render( + + + , + ); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("should not warn if no item role is set", () => { + render( + + + , + ); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("should list the allowed item roles in the warning", () => { + render( + + + , + ); + expect(warnSpy.mock.calls[0][0]).toContain('allowed: "none", "presentation"'); + }); + + it("should keep the item role and the automatically set roleStructure", () => { + const { container } = render( + + + , + ); + expect(getDomRoles(container)).toEqual([{ li: "option", target: "option" }]); + }); + + it("should check the item role of Blueprint MenuItem children and inside fragments too", () => { + render( + + + <> + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(2); + }); + }); + + describe("roleStructure check", () => { + it.each<[MenuRole, RoleStructure]>([ + ["menu", "listitem"], + ["menu", "listoption"], + ["navigation", "menuitem"], + ["list", "listoption"], + ["listbox", "menuitem"], + ["none", "listitem"], + ["presentation", "menuitem"], + ])("should warn for role '%s' with not allowed roleStructure '%s'", (role, roleStructure) => { + render( + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0][0]).toContain(`roleStructure="${roleStructure}"`); + expect(warnSpy.mock.calls[0][0]).toContain(`role="${role}"`); + }); + + it("should keep the not allowed roleStructure value", () => { + const { container } = render( + + + , + ); + expect(getDomRoles(container)).toEqual([expectedDomRoles.listoption]); + }); + + it("should list the allowed values in the warning", () => { + render( + + + , + ); + expect(warnSpy.mock.calls[0][0]).toContain('allowed: "listoption"'); + }); + + it("should warn once per not allowed item", () => { + render( + + + + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(2); + }); + + it("should not repeat warnings on re-render", () => { + const { rerender } = render( + + + , + ); + rerender( + + + , + ); + rerender( + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it("should warn again if the mismatches changed", () => { + const { rerender } = render( + + + , + ); + rerender( + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(2); + expect(warnSpy.mock.calls[1][0]).toContain('roleStructure="listitem"'); + }); + + it("should not warn after the mismatch was fixed", () => { + const { rerender } = render( + + + , + ); + rerender( + + + , + ); + rerender( + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it("should check Blueprint MenuItem children too", () => { + render( + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it("should check MenuItems nested in fragments", () => { + render( + + <> + + <> + + + + , + ); + expect(warnSpy).toHaveBeenCalledTimes(2); + }); + + it("should not check MenuItems in submenus", () => { + render( + + + + + , + ); + expect(warnSpy).not.toHaveBeenCalled(); + }); + }); +});