fix(ui5-menu): improve focus handling

packages/main/src/Menu.hbs

@@ -74,6 +74,7 @@
 				@mouseover={{../_itemMouseOver}}
 				@mouseout={{../_itemMouseOut}}
 				@keydown={{../_itemKeyDown}}
+				@_focused={{../_onfocusin}}
 			>
 				<div class="ui5-menu-item-text">
 				{{#if this.item.hasDummyIcon}}

packages/main/src/Menu.ts

@@ -284,6 +284,12 @@ class Menu extends UI5Element {
 	@property({ type: Object, defaultValue: undefined })
 	_parentMenuItem?: MenuItem;
 
+	/**
+	 * Stores parent menu item DOM representation (if there is such).
+	 */
+	@property({ type: Object, defaultValue: undefined })
+	_opener?: HTMLElement;
+
 	/**
 	 * Stores menu item that have sub-menu opened.
 	 */
@@ -400,15 +406,10 @@ class Menu extends UI5Element {
 		}
 		if (!this._isSubMenu) {
 			this._parentMenuItem = undefined;
+			this._opener = undefined;
 		}
 		const popover = await this._createPopover();
-		popover.initialFocus = "";
-		for (let index = 0; index < this._currentItems.length; index++) {
-			if (!this._currentItems[index].item.disabled) {
-				popover.initialFocus = `${this._id}-menu-item-${index}`;
-				break;
-			}
-		}
+		popover.initialFocus = `${this._id}-menu-item-0`;
 		popover.showAt(opener);
 	}
 
@@ -422,8 +423,7 @@ class Menu extends UI5Element {
 			if (isPhone()) {
 				this._parentItemsStack = [];
 			}
-			this._popover.close();
-			this._popover.resetFocus();
+			this._popover.close(false, false, true);
 		}
 	}
 
@@ -459,13 +459,14 @@ class Menu extends UI5Element {
 		});
 	}
 
-	_createSubMenu(item: MenuItem, openerId: string) {
+	_createSubMenu(item: MenuItem, opener: HTMLElement) {
 		const ctor = this.constructor as typeof Menu;
 		const subMenu = document.createElement(ctor.getMetadata().getTag()) as Menu;
 
 		subMenu._isSubMenu = true;
-		subMenu.setAttribute("id", `submenu-${openerId}`);
+		subMenu.setAttribute("id", `submenu-${opener.id}`);
 		subMenu._parentMenuItem = item;
+		subMenu._opener = opener;
 		subMenu.busy = item.busy;
 		subMenu.busyDelay = item.busyDelay;
 		const fragment = this._clonedItemsFragment(item);
@@ -485,51 +486,54 @@ class Menu extends UI5Element {
 		return fragment;
 	}
 
-	_openItemSubMenu(item: MenuItem, opener: HTMLElement, actionId: string) {
+	_openItemSubMenu(item: MenuItem, opener: HTMLElement) {
 		const mainMenu = this._findMainMenu(item);
 		mainMenu.fireEvent<MenuBeforeOpenEventDetail>("before-open", {
 			item,
 		}, false, false);
 		item._subMenu!.showAt(opener);
 		item._preventSubMenuClose = true;
 		this._openedSubMenuItem = item;
-		this._subMenuOpenerId = actionId;
+		this._subMenuOpenerId = opener.id;
 	}
 
-	_closeItemSubMenu(item: MenuItem, forceClose = false) {
+	_closeItemSubMenu(item: MenuItem, forceClose = false, keyboard = false) {
 		if (item) {
 			if (forceClose) {
 				item._preventSubMenuClose = false;
-				this._closeSubMenuPopover(item._subMenu!, true);
+				this._closeSubMenuPopover(item._subMenu!, forceClose, keyboard);
 			} else {
 				setTimeout(() => this._closeSubMenuPopover(item._subMenu!), 0);
 			}
 		}
 	}
 
-	_closeSubMenuPopover(subMenu: Menu, forceClose = false) {
+	_closeSubMenuPopover(subMenu: Menu, forceClose = false, keyboard = false) {
 		if (subMenu) {
 			const parentItem = subMenu._parentMenuItem!;
 
 			if (forceClose || !parentItem._preventSubMenuClose) {
 				subMenu.close();
 				subMenu.remove();
 				parentItem._subMenu = undefined;
+				if (keyboard) {
+					subMenu._opener?.focus();
+				}
 				this._openedSubMenuItem = undefined;
 				this._subMenuOpenerId = "";
 			}
 		}
 	}
 
-	_prepareSubMenuDesktopTablet(item: MenuItem, opener: HTMLElement, actionId: string) {
-		if (actionId !== this._subMenuOpenerId || (item && item.hasSubmenu)) {
+	_prepareSubMenuDesktopTablet(item: MenuItem, opener: HTMLElement) {
+		if (opener.id !== this._subMenuOpenerId || (item && item.hasSubmenu)) {
 			// close opened sub-menu if there is any opened
 			this._closeItemSubMenu(this._openedSubMenuItem!, true);
 		}
 		if (item && item.hasSubmenu) {
 			// create new sub-menu
-			this._createSubMenu(item, actionId);
-			this._openItemSubMenu(item, opener, actionId);
+			this._createSubMenu(item, opener);
+			this._openItemSubMenu(item, opener);
 		}
 		if (this._parentMenuItem) {
 			this._parentMenuItem._preventSubMenuClose = true;
@@ -542,46 +546,44 @@ class Menu extends UI5Element {
 		this._parentItemsStack.push(item);
 	}
 
-	_startOpenTimeout(item: MenuItem, opener: OpenerStandardListItem, hoverId: string) {
-		// If theres already a timeout, clears it
-		this._clearTimeout();
+	_onfocusin(e: FocusEvent): void {
+		const target = e.target as HTMLElement;
+		const menuListItem = target.hasAttribute("ui5-menu-li")
+			? target as MenuListItem
+			: (target.getRootNode() as ShadowRoot).host as MenuListItem;
+		const item = menuListItem.associatedItem as MenuItem;
+		const mainMenu = this._findMainMenu(item);
+		mainMenu?.fireEvent("item-focus", { ref: menuListItem, item });
+	}
+
+	_startOpenTimeout(item: MenuItem, opener: OpenerStandardListItem) {
+		clearTimeout(this._timeout);
 
 		// Sets the new timeout
 		this._timeout = setTimeout(() => {
-			this._prepareSubMenuDesktopTablet(item, opener, hoverId);
+			this._prepareSubMenuDesktopTablet(item, opener);
 		}, MENU_OPEN_DELAY);
 	}
 
 	_startCloseTimeout(item: MenuItem) {
-		// If theres already a timeout, clears it
-		this._clearTimeout();
+		clearTimeout(this._timeout);
 
 		// Sets the new timeout
 		this._timeout = setTimeout(() => {
 			this._closeItemSubMenu(item);
 		}, MENU_CLOSE_DELAY);
 	}
 
-	_clearTimeout() {
-		if (this._timeout) {
-			clearTimeout(this._timeout);
-		}
-	}
-
 	_itemMouseOver(e: MouseEvent) {
 		if (isDesktop()) {
 			// respect mouseover only on desktop
 			const opener = e.target as OpenerStandardListItem;
 			const item = opener.associatedItem;
-			const hoverId = opener.getAttribute("id")!;
 
 			opener.focus();
 
-			// If there is a pending close operation, cancel it
-			this._clearTimeout();
-
 			// Opens submenu with 300ms delay
-			this._startOpenTimeout(item, opener, hoverId);
+			this._startOpenTimeout(item, opener);
 		}
 	}
 
@@ -596,8 +598,7 @@ class Menu extends UI5Element {
 			const opener = e.target as OpenerStandardListItem;
 			const item = opener.associatedItem;
 
-			// If there is a pending open operation, cancel it
-			this._clearTimeout();
+			clearTimeout(this._timeout);
 
 			// Close submenu with 400ms delay
 			if (item && item.hasSubmenu && item._subMenu) {
@@ -609,28 +610,26 @@ class Menu extends UI5Element {
 	}
 
 	_itemKeyDown(e: KeyboardEvent) {
-		const isMenuClose = this.isRtl ? isRight(e) : isLeft(e);
-		const isMenuOpen = this.isRtl ? isLeft(e) : isRight(e);
+		const shouldCloseMenu = this.isRtl ? isRight(e) : isLeft(e);
+		const shouldOpenMenu = this.isRtl ? isLeft(e) : isRight(e);
 
 		if (isEnter(e)) {
 			e.preventDefault();
 		}
-		if (isMenuOpen) {
+		if (shouldOpenMenu) {
 			const opener = e.target as OpenerStandardListItem;
 			const item = opener.associatedItem;
-			const hoverId = opener.getAttribute("id")!;
 
-			item.hasSubmenu && this._prepareSubMenuDesktopTablet(item, opener, hoverId);
-		} else if (isMenuClose && this._isSubMenu && this._parentMenuItem) {
+			item.hasSubmenu && this._prepareSubMenuDesktopTablet(item, opener);
+		} else if (shouldCloseMenu && this._isSubMenu && this._parentMenuItem) {
 			const parentMenuItemParent = this._parentMenuItem.parentElement as Menu;
-			parentMenuItemParent._closeItemSubMenu(this._parentMenuItem, true);
+			parentMenuItemParent._closeItemSubMenu(this._parentMenuItem, true, true);
 		}
 	}
 
 	_itemClick(e: CustomEvent<ListItemClickEventDetail>) {
 		const opener = e.detail.item as OpenerStandardListItem;
 		const item = opener.associatedItem;
-		const actionId = opener.getAttribute("id")!;
 
 		if (!item.hasSubmenu) {
 			// click on an item that doesn't have sub-items fires an "item-click" event
@@ -672,7 +671,7 @@ class Menu extends UI5Element {
 			this._prepareSubMenuPhone(item);
 		} else if (isTablet()) {
 			// prepares and opens sub-menu on tablet
-			this._prepareSubMenuDesktopTablet(item, opener, actionId);
+			this._prepareSubMenuDesktopTablet(item, opener);
 		}
 	}
 

packages/main/src/NavigationMenu.ts

@@ -72,7 +72,6 @@ class NavigationMenu extends Menu {
 			// respect mouseover only on desktop
 			const opener = e.target as OpenerStandardListItem;
 			let item = opener.associatedItem;
-			const hoverId = opener.getAttribute("id")!;
 
 			if (!item) {
 				// for nested <a>
@@ -82,11 +81,8 @@ class NavigationMenu extends Menu {
 				}
 			}
 
-			// If there is a pending close operation, cancel it
-			this._clearTimeout();
-
 			// Opens submenu with 300ms delay
-			this._startOpenTimeout(item, opener, hoverId);
+			this._startOpenTimeout(item, opener);
 		}
 	}
 
@@ -109,7 +105,6 @@ class NavigationMenu extends Menu {
 	_itemClick(e: CustomEvent<ListItemClickEventDetail>) {
 		const opener = e.detail.item as OpenerStandardListItem;
 		const item = opener.associatedItem;
-		const actionId = opener.getAttribute("id")!;
 		const mainMenu = this._findMainMenu(item);
 		const prevented = !mainMenu.fireEvent<MenuItemClickEventDetail>("item-click", {
 			"item": item,
@@ -134,7 +129,7 @@ class NavigationMenu extends Menu {
 			this._prepareSubMenuPhone(item);
 		} else if (isTablet()) {
 			// prepares and opens sub-menu on tablet
-			this._prepareSubMenuDesktopTablet(item, opener, actionId);
+			this._prepareSubMenuDesktopTablet(item, opener);
 		}
 	}
 	get accSideNavigationPopoverHiddenText() {

packages/main/test/pages/Menu.html

@@ -15,14 +15,13 @@
 <body class="bg">
 	<ui5-button id="btnOpen">Open Menu</ui5-button> <br/>
 	<ui5-menu id="menu" header-text="My ui5-menu">
-
 		<ui5-menu-item text="New File" accessible-name="Opens a file explorer" additional-text="Ctrl+Alt+Shift+N" tooltip="Select a file" icon="add-document"></ui5-menu-item>
 		<ui5-menu-item text="New Folder with very long title for a menu item" additional-text="Ctrl+F" icon="add-folder" disabled></ui5-menu-item>
 		<ui5-menu-item text="Open" icon="open-folder" starts-section busy-delay="100" busy>
 			<ui5-menu-item text="Open Locally" icon="open-folder" additional-text="Ctrl+K">
 				<ui5-menu-item text="Open from C"></ui5-menu-item>
 				<ui5-menu-item text="Open from D"></ui5-menu-item>
-				<ui5-menu-item text="Open from E"></ui5-menu-item>
+				<ui5-menu-item text="Open from E" disabled></ui5-menu-item>
 			</ui5-menu-item>
 			<ui5-menu-item text="Open from SAP Cloud" additional-text="Ctrl+L"></ui5-menu-item>
 		</ui5-menu-item>
@@ -60,6 +59,24 @@
 	<ui5-title level="H5" class="header-title">Event logger</ui5-title>
 	<ui5-input id="eventLogger" style="width: 100%;"></ui5-input>
 
+	<ui5-button id="btnOpen2">Open Menu</ui5-button> <br/>
+	<ui5-menu id="menu2" header-text="My ui5-menu">
+		<ui5-menu-item text="New Folder" additional-text="Ctrl+F" icon="add-folder" disabled></ui5-menu-item>
+		<ui5-menu-item text="Open" icon="open-folder" starts-section>
+			<ui5-menu-item text="Open Locally" icon="open-folder" additional-text="Ctrl+K"></ui5-menu-item>
+			<ui5-menu-item text="Open from SAP Cloud" additional-text="Ctrl+L" disabled></ui5-menu-item>
+		</ui5-menu-item>
+		<ui5-menu-item text="Preferences" icon="action-settings" starts-section disabled></ui5-menu-item>
+		<ui5-menu-item text="Exit" icon="journey-arrive"></ui5-menu-item>
+	</ui5-menu>
+
+	<ui5-popover id="detailsPopover">
+			<div>
+				<ui5-label for="detailsLink">For more information:</ui5-label>
+			</div>
+			<ui5-link id="detailsLink" href="https://en.wikipedia.org/wiki/Google_logo" target="_blank"></ui5-link>
+	</ui5-popover>
+
 	<script>
 		btnOpen.accessibilityAttributes = {
 			hasPopup: "Menu",
@@ -114,6 +131,7 @@
 		});
 
 		menu.addEventListener("ui5-after-open", function() {
+			shouldOpenDetailsPopover = true;
 			var opener = menu.opener.id ? menu.opener.id : menu.opener;
 			openerInput.value = opener;
 			eventLogger.value += "* [after-open] ";
@@ -125,6 +143,7 @@
 		});
 
 		menu.addEventListener("ui5-after-close", function() {
+			shouldOpenDetailsPopover = false;
 			openerInput.value = "";
 			btnToggleOpen.pressed = false;
 			eventLogger.value += "* [after-close]";
@@ -135,6 +154,48 @@
 			window["sap-ui-webcomponents-bundle"].applyDirection();
 		});
 
+		btnOpen2.accessibilityAttributes = {
+			hasPopup: "Menu",
+		};
+
+		btnOpen2.addEventListener("click", function() {
+			menu2.showAt(btnOpen2);
+		});
+
+		let shouldOpenDetailsPopover = false;
+
+		detailsPopover.addEventListener("ui5-before-close", function() {
+			shouldOpenDetailsPopover = false;
+		});
+
+		detailsPopover.addEventListener("ui5-after-close", function() {
+			shouldOpenDetailsPopover = true;
+		});
+
+		menu2.addEventListener("ui5-after-open", function() {
+			shouldOpenDetailsPopover = true;
+		});
+
+		menu2.addEventListener("ui5-after-close", function() {
+			shouldOpenDetailsPopover = false;
+		});
+
+		menu2.addEventListener("ui5-item-focus", function(event) {
+			const item = event.detail.item;
+			const opener = event.detail.ref;
+
+			if (detailsPopover.isOpen()) {
+				detailsPopover.close(false, false, true);
+			}
+
+			if (item.disabled && shouldOpenDetailsPopover) {
+				const link = detailsPopover.querySelector("#detailsLink");
+				link.textContent = item.text;
+				detailsPopover.showAt(opener, true)
+				link.focus();
+			}
+		});
+
 	</script>
 </body>
 </html>

packages/main/test/specs/Menu.spec.js

@@ -28,7 +28,7 @@ describe("Menu interaction", () => {
 	it("Top level menu items appearance", async () => {
 		await browser.url(`test/pages/Menu.html`);
 		const openButton = await browser.$("#btnOpen");
-		const menuItems = await browser.$$("ui5-menu>ui5-menu-item");
+		const menuItems = await browser.$$("ui5-menu[id='menu']>ui5-menu-item");
 
 		openButton.click();