feat(ui5-menu): improve accessibility (#11846)

This PR introduces the following accessibility improvements in ui5-menu component:

Adds aria-label attributes on menu item group wrappers, which already have role="group". The labels are according to the group's checkMode property.

Adds the possibility to check/uncheck menu items in groups when the checkMode property is set to Single or Multiple. This is achieved by holding down the Shift key while selecting (with mouse or keyboard) the menu items in these groups. Selecting menu items that don't belong to any group, or items inside groups with the checkMode property set to None will always close the menu regardless if Shift is pressed or not.

Fixes: #11594

Author: NHristov-sap

packages/main/cypress/specs/Menu.cy.tsx

@@ -286,6 +286,68 @@ describe("Menu interaction", () => {
 			.should("be.focused");
 	});
 
+	it("should not close menu when selecting 'single' checkable menu item with 'Shift'", () => {
+		cy.mount(
+			<>
+				<Button id="btnOpen">Open Menu</Button>
+				<Menu opener="btnOpen">
+					<MenuItemGroup checkMode="Single">
+						<MenuItem text="Single"></MenuItem>
+					</MenuItemGroup>
+				</Menu>
+			</>
+		);
+
+		cy.get("[ui5-menu]")
+			.ui5MenuItemCheckShiftClickAndPress("[text='Single']", "have.attr");
+	});
+
+	it("should not close menu when selecting 'multi' checkable menu item with 'Shift'", () => {
+		cy.mount(
+			<>
+				<Button id="btnOpen">Open Menu</Button>
+				<Menu opener="btnOpen">
+					<MenuItemGroup checkMode="Multiple">
+						<MenuItem text="Multiple"></MenuItem>
+					</MenuItemGroup>
+				</Menu>
+			</>
+		);
+
+		cy.get("[ui5-menu]")
+			.ui5MenuItemCheckShiftClickAndPress("[text='Multiple']", "have.attr");
+	});
+
+	it("should close menu when selecting non checkable menu item with 'Shift'", () => {
+		cy.mount(
+			<>
+				<Button id="btnOpen">Open Menu</Button>
+				<Menu opener="btnOpen">
+					<MenuItemGroup checkMode="None">
+						<MenuItem text="None"></MenuItem>
+					</MenuItemGroup>
+				</Menu>
+			</>
+		);
+
+		cy.get("[ui5-menu]")
+			.ui5MenuItemCheckShiftClickAndPress("[text='None']", "not.have.attr");
+	});
+
+	it("should close menu when selecting menu item, not in a group, with 'Shift'", () => {
+		cy.mount(
+			<>
+				<Button id="btnOpen">Open Menu</Button>
+				<Menu opener="btnOpen">
+					<MenuItem text="Outside"></MenuItem>
+				</Menu>
+			</>
+		);
+
+		cy.get("[ui5-menu]")
+			.ui5MenuItemCheckShiftClickAndPress("[text='Outside']", "not.have.attr");
+	});
+
 	describe("Event firing", () => {
 		it("Event firing - 'ui5-item-click' after 'click' on menu item", () => {
 			cy.mount(

packages/main/cypress/support/commands.ts

@@ -67,11 +67,7 @@ declare global {
 	namespace Cypress {
 		interface Chainable {
 			ui5SimulateDevice(device?: SimulationDevices): Chainable<void>
-			ui5MenuOpen(options?: { opener?: string }): Chainable<void>
-			ui5MenuOpened(): Chainable<void>
-			ui5MenuItemClick(): Chainable<void>
 			ui5DOMRef(): Chainable<void>
-			ui5MenuItemPress(key: any): Chainable<void>
 			ui5CalendarGetDay(calendarSelector: string, timestamp: string): Chainable<JQuery<HTMLElement>>
 			ui5CalendarGetMonth(calendarSelector: string, timestamp: string): Chainable<JQuery<HTMLElement>>
 			ui5CalendarShowYearRangePicker(): Chainable<void>

packages/main/cypress/support/commands/Menu.commands.ts

@@ -44,9 +44,69 @@ Cypress.Commands.add("ui5MenuItemClick", { prevSubject: true }, subject => {
 
 Cypress.Commands.add("ui5MenuItemPress", { prevSubject: true }, (subject, key) => {
 	cy.get(subject)
-		.should("have.focused")
+		.should("be.focused")
 		.and("be.visible");
 
 	// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
 	cy.realPress(key);
 });
+
+Cypress.Commands.add("ui5MenuItemCheckShiftClickAndPress", { prevSubject: true }, (subject, menuItem, shouldStatement) => {
+	cy.get(subject)
+		.as("menu");
+
+	cy.get("@menu")
+		.find(menuItem)
+		.as("menuItem");
+
+	cy.get("@menu")
+		.ui5MenuOpen();
+
+	cy.get("@menuItem")
+		.should("be.visible")
+		.and("be.focused");
+
+	cy.get("@menuItem")
+		.realClick({ shiftKey: true });
+
+	cy.get("@menu")
+		.should(shouldStatement, "open");
+
+	cy.get("@menu")
+		.ui5MenuOpen();
+
+	cy.get("@menuItem")
+		.should("be.visible")
+		.and("be.focused");
+
+	cy.get("@menuItem")
+		.realPress(["Shift", "Enter"]);
+
+	cy.get("@menu")
+		.should(shouldStatement, "open");
+
+	cy.get("@menu")
+		.ui5MenuOpen();
+
+	cy.get("@menuItem")
+		.should("be.visible")
+		.and("be.focused");
+
+	cy.get("@menuItem")
+		.realPress(["Shift", "Space"]);
+
+	cy.get("@menu")
+		.should(shouldStatement, "open");
+});
+
+declare global {
+	namespace Cypress {
+		interface Chainable {
+			ui5MenuOpen(options?: { opener?: string }): Chainable<void>
+			ui5MenuOpened(): Chainable<void>
+			ui5MenuItemClick(): Chainable<void>
+			ui5MenuItemPress(key: any): Chainable<void>
+			ui5MenuItemCheckShiftClickAndPress(menuItem: string, shouldStatement: string): Chainable<void>
+		}
+	}
+}

packages/main/src/ListItemBase.ts

@@ -146,11 +146,11 @@ class ListItemBase extends UI5Element implements ITabbable {
 			return;
 		}
 
-		if (isSpace(e)) {
+		if (this._isSpace(e)) {
 			e.preventDefault();
 		}
 
-		if (isEnter(e)) {
+		if (this._isEnter(e)) {
 			this.fireItemPress(e);
 		}
 	}
@@ -159,7 +159,7 @@ class ListItemBase extends UI5Element implements ITabbable {
 		if (this.getFocusDomRef()!.matches(":has(:focus-within)")) {
 			return;
 		}
-		if (isSpace(e)) {
+		if (this._isSpace(e)) {
 			this.fireItemPress(e);
 		}
 	}
@@ -171,6 +171,20 @@ class ListItemBase extends UI5Element implements ITabbable {
 		this.fireItemPress(e);
 	}
 
+	/**
+	 * Override from subcomponent, if needed
+	 */
+	_isSpace(e: KeyboardEvent) {
+		return isSpace(e);
+	}
+
+	/**
+	 * Override from subcomponent, if needed
+	 */
+	_isEnter(e: KeyboardEvent) {
+		return isEnter(e);
+	}
+
 	fireItemPress(e: Event) {
 		if (this.disabled || !this._pressable) {
 			return;

packages/main/src/Menu.ts

@@ -7,11 +7,8 @@ import {
 	isLeft,
 	isRight,
 	isEnter,
-	isSpace,
 	isTabNext,
 	isTabPrevious,
-	isDown,
-	isUp,
 } from "@ui5/webcomponents-base/dist/Keys.js";
 import {
 	isPhone,
@@ -415,7 +412,7 @@ class Menu extends UI5Element {
 
 			if (!prevented) {
 				item._updateCheckedState();
-				this._popover && item.fireDecoratorEvent("close-menu");
+				this._popover && !item._shiftPressed && item.fireDecoratorEvent("close-menu");
 			}
 		} else {
 			this._openItemSubMenu(item);
@@ -430,12 +427,8 @@ class Menu extends UI5Element {
 			return;
 		}
 
-		const menuItemInMenu = this._allMenuItems.includes(item);
-		const isItemNavigation = isUp(e) || isDown(e);
-		const isItemSelection = isEnter(e) || isSpace(e);
 		const isEndContentNavigation = isRight(e) || isLeft(e);
 		const shouldOpenMenu = this.isRtl ? isLeft(e) : isRight(e);
-		const shouldCloseMenu = menuItemInMenu && !(isItemNavigation || isItemSelection || isEndContentNavigation);
 
 		if (isEnter(e) || isTabNextPrevious) {
 			e.preventDefault();
@@ -447,7 +440,7 @@ class Menu extends UI5Element {
 
 		if (shouldOpenMenu) {
 			this._openItemSubMenu(item);
-		} else if ((shouldCloseMenu || isTabNextPrevious)) {
+		} else if (isTabNextPrevious) {
 			this._close();
 		}
 	}

packages/main/src/MenuItem.ts

@@ -10,6 +10,9 @@ import {
 	isRight,
 	isEnter,
 	isSpace,
+	isEnterShift,
+	isSpaceShift,
+	isShift,
 	isTabNext,
 	isTabPrevious,
 	isDown,
@@ -314,6 +317,7 @@ class MenuItem extends ListItem implements IMenuItem {
 	static i18nBundle: I18nBundle;
 
 	_itemNavigation: ItemNavigation;
+	_shiftPressed: boolean = false;
 
 	constructor() {
 		super();
@@ -337,6 +341,10 @@ class MenuItem extends ListItem implements IMenuItem {
 		});
 	}
 
+	get _isCheckable() {
+		return this._checkMode !== MenuItemGroupCheckMode.None;
+	}
+
 	_navigateToEndContent(shouldNavigateToPreviousItem: boolean) {
 		const navigatableItems = this._navigableItems;
 		const item = shouldNavigateToPreviousItem
@@ -534,22 +542,40 @@ class MenuItem extends ListItem implements IMenuItem {
 		this._closeOtherSubMenus(item);
 	}
 
+	_isSpace(e: KeyboardEvent) {
+		this._shiftPressed = this._isCheckable && isSpaceShift(e);
+		return isSpace(e) || isSpaceShift(e);
+	}
+
+	_isEnter(e: KeyboardEvent) {
+		this._shiftPressed = this._isCheckable && isEnterShift(e);
+		return isEnter(e) || isEnterShift(e);
+	}
+
+	_onclick(e: MouseEvent) {
+		this._shiftPressed = this._isCheckable && e.shiftKey;
+		super._onclick(e);
+	}
+
 	_itemKeyDown(e: KeyboardEvent) {
 		const item = e.target as MenuItem;
 		const itemInMenuItems = this._allMenuItems.includes(item);
 		const isTabNextPrevious = isTabNext(e) || isTabPrevious(e);
-		const isItemNavigation = isUp(e) || isDown(e);
-		const isItemSelection = isSpace(e) || isEnter(e);
-		const shouldOpenMenu = this.isRtl ? isLeft(e) : isRight(e);
-		const shouldCloseMenu = !(isItemNavigation || isItemSelection || shouldOpenMenu) || isTabNextPrevious;
+		const shouldCloseMenu = this.isRtl ? isRight(e) : isLeft(e);
 
-		if (itemInMenuItems && shouldCloseMenu) {
+		if (itemInMenuItems && (isTabNextPrevious || shouldCloseMenu)) {
 			this._close();
 			this.focus();
 			e.stopPropagation();
 		}
 	}
 
+	_itemKeyUp(e: KeyboardEvent) {
+		if (isShift(e)) {
+			this._shiftPressed = false;
+		}
+	}
+
 	_endContentKeyDown(e: KeyboardEvent) {
 		const shouldNavigateOutOfEndContent = isUp(e) || isDown(e);
 

packages/main/src/MenuItemGroup.ts

@@ -2,12 +2,19 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
 import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
 import property from "@ui5/webcomponents-base/dist/decorators/property.js";
 import slot from "@ui5/webcomponents-base/dist/decorators/slot.js";
+import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js";
 import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
+import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
 import type MenuItem from "./MenuItem.js";
 import { isInstanceOfMenuItem } from "./MenuItem.js";
 import MenuItemGroupTemplate from "./MenuItemGroupTemplate.js";
 import MenuItemGroupCheckMode from "./types/MenuItemGroupCheckMode.js";
 import type { IMenuItem } from "./Menu.js";
+import {
+	MENU_ITEM_GROUP_NONE_ACCESSIBLE_NAME,
+	MENU_ITEM_GROUP_SINGLE_ACCESSIBLE_NAME,
+	MENU_ITEM_GROUP_MULTI_ACCESSIBLE_NAME,
+} from "./generated/i18n/i18n-defaults.js";
 
 /**
  * @class
@@ -62,6 +69,22 @@ class MenuItemGroup extends UI5Element implements IMenuItem {
 	@slot({ "default": true, type: HTMLElement, invalidateOnChildChange: true })
 	items!: Array<IMenuItem>;
 
+	@i18n("@ui5/webcomponents")
+	static i18nBundle: I18nBundle;
+
+	get ariaLabelText(): string | undefined {
+		switch (this.checkMode) {
+		case MenuItemGroupCheckMode.None:
+			return MenuItemGroup.i18nBundle.getText(MENU_ITEM_GROUP_NONE_ACCESSIBLE_NAME);
+		case MenuItemGroupCheckMode.Single:
+			return MenuItemGroup.i18nBundle.getText(MENU_ITEM_GROUP_SINGLE_ACCESSIBLE_NAME);
+		case MenuItemGroupCheckMode.Multiple:
+			return MenuItemGroup.i18nBundle.getText(MENU_ITEM_GROUP_MULTI_ACCESSIBLE_NAME);
+		default:
+			return undefined;
+		}
+	}
+
 	get isGroup(): boolean {
 		return true;
 	}

packages/main/src/MenuItemGroupTemplate.tsx

@@ -4,6 +4,7 @@ export default function MenuItemGroupTemplate(this: MenuItemGroup) {
 	return (
 		<div
 			role="group"
+			aria-label={this.ariaLabelText}
 			onui5-check={this._handleItemCheck}
 		>
 			<slot></slot>

packages/main/src/MenuItemTemplate.tsx

@@ -138,6 +138,7 @@ function listItemPostContent(this: MenuItem) {
 						loadingDelay={this.loadingDelay}
 						onMouseOver={this._itemMouseOver}
 						onKeyDown={this._itemKeyDown}
+						onKeyUp={this._itemKeyUp}
 						// handles event from slotted children
 						onui5-close-menu={this._close}
 						onui5-exit-end-content={this._navigateOutOfEndContent}

packages/main/src/i18n/messagebundle.properties

@@ -643,6 +643,15 @@ MENU_CLOSE_BUTTON_ARIA_LABEL=Decline
 #XACT: ARIA information for the Menu popover
 MENU_POPOVER_ACCESSIBLE_NAME=Select an option from the menu
 
+#XACT: ARIA information for the Menu Item Group with itemSelectionMode "None"
+MENU_ITEM_GROUP_NONE_ACCESSIBLE_NAME=Contains Non-Selectable Items
+
+#XACT: ARIA information for the Menu Item Group with itemSelectionMode "SingleSelect"
+MENU_ITEM_GROUP_SINGLE_ACCESSIBLE_NAME=Contains Selectable Items
+
+#XACT: ARIA information for the Menu Item Group with itemSelectionMode "MultiSelect"
+MENU_ITEM_GROUP_MULTI_ACCESSIBLE_NAME=Contains Multi-Selectable Items
+
 #XACT: ARIA announcement for roldesecription attribute of Dialog header
 DIALOG_HEADER_ARIA_ROLE_DESCRIPTION=Interactive Header