diff --git a/packages/main/src/Menu.hbs b/packages/main/src/Menu.hbs index 997ce02eb3b2..fb5ed5f4e613 100644 --- a/packages/main/src/Menu.hbs +++ b/packages/main/src/Menu.hbs @@ -38,7 +38,7 @@ icon="decline" design="Transparent" aria-label="{{labelClose}}" - @click={{close}} + @click={{_closeAll}} > @@ -67,6 +67,7 @@ accessible-role="menuitem" .additionalText="{{this.item._additionalText}}" tooltip="{{this.item.tooltip}}" + selected="{{this.item.subMenuOpened}}" ?disabled={{this.item.disabled}} ?starts-section={{this.item.startsSection}} ?selected={{this.item.subMenuOpened}} diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index da010d3ec7eb..b38b0689fb35 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -11,7 +11,6 @@ import { } from "@ui5/webcomponents-base/dist/Keys.js"; import { isPhone, - isTablet, isDesktop, } from "@ui5/webcomponents-base/dist/Device.js"; import { getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js"; @@ -265,13 +264,6 @@ class Menu extends UI5Element { @property({ type: Object, multiple: true }) _currentItems!: Array; - /** - * Stores a list of parent menu items for each sub-menu (on phone). - * @private - */ - @property({ type: Object, multiple: true }) - _parentItemsStack!: Array; - /** * Stores the ResponsivePopover instance */ @@ -347,7 +339,7 @@ class Menu extends UI5Element { } get isSubMenuOpened() { - return !!this._parentMenuItem; + return this._parentMenuItem && this._popover?.isOpen(); } get menuHeaderTextPhone() { @@ -355,7 +347,7 @@ class Menu extends UI5Element { } onBeforeRendering() { - !isPhone() && this._prepareCurrentItems(this.items); + this._prepareCurrentItems(this.items); const itemsWithChildren = this.itemsWithChildren; const itemsWithIcon = this.itemsWithIcon; @@ -385,7 +377,7 @@ class Menu extends UI5Element { if (this.open) { const opener = this.getOpener(); - if (opener) { + if (opener && !this.isSubMenuOpened) { this.showAt(opener); } } else { @@ -400,17 +392,14 @@ class Menu extends UI5Element { * @public */ async showAt(opener: HTMLElement): Promise { - if (isPhone()) { - this._prepareCurrentItems(this.items); - this._parentItemsStack = []; - } if (!this._isSubMenu) { this._parentMenuItem = undefined; this._opener = undefined; } + const busyWithoutItems = !this._parentMenuItem?.items.length && this._parentMenuItem?.busy; const popover = await this._createPopover(); popover.initialFocus = `${this._id}-menu-item-0`; - popover.showAt(opener); + popover.showAt(opener, busyWithoutItems); } /** @@ -419,17 +408,14 @@ class Menu extends UI5Element { * @public */ close(): void { - if (this._popover) { - if (isPhone()) { - this._parentItemsStack = []; - } - this._popover.close(false, false, true); - } + this._popover?.close(false, false, true); } async _createPopover() { - const staticAreaItemDomRef = await this.getStaticAreaItemDomRef(); - this._popover = staticAreaItemDomRef!.querySelector("[ui5-responsive-popover]")!; + if (!this._popover) { + const staticAreaItemDomRef = await this.getStaticAreaItemDomRef(); + this._popover = staticAreaItemDomRef!.querySelector("[ui5-responsive-popover]")!; + } return this._popover; } @@ -439,14 +425,12 @@ class Menu extends UI5Element { } _navigateBack() { - const parentMenuItem = this._parentItemsStack.pop(); + this._closeItemSubMenu(this._parentMenuItem as MenuItem, true); + } - this.focus(); - if (parentMenuItem) { - const parentMenuItemParent = parentMenuItem.parentElement as MenuItem; - this._prepareCurrentItems(parentMenuItemParent.items); - this._parentMenuItem = this._parentItemsStack.length ? this._parentItemsStack[this._parentItemsStack.length - 1] : undefined; - } + _closeAll() { + const mainMenu = this._findMainMenu(this); + mainMenu?.close(); } _prepareCurrentItems(items: Array) { @@ -460,6 +444,9 @@ class Menu extends UI5Element { } _createSubMenu(item: MenuItem, opener: HTMLElement) { + if (item._subMenu) { + return; + } const ctor = this.constructor as typeof Menu; const subMenu = document.createElement(ctor.getMetadata().getTag()) as Menu; @@ -488,7 +475,7 @@ class Menu extends UI5Element { _openItemSubMenu(item: MenuItem, opener: HTMLElement) { const mainMenu = this._findMainMenu(item); - mainMenu.fireEvent("before-open", { + mainMenu?.fireEvent("before-open", { item, }, false, false); item._subMenu!.showAt(opener); @@ -514,8 +501,6 @@ class Menu extends UI5Element { if (forceClose || !parentItem._preventSubMenuClose) { subMenu.close(); - subMenu.remove(); - parentItem._subMenu = undefined; if (keyboard) { subMenu._opener?.focus(); } @@ -525,7 +510,7 @@ class Menu extends UI5Element { } } - _prepareSubMenuDesktopTablet(item: MenuItem, opener: HTMLElement) { + _prepareSubMenu(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); @@ -540,12 +525,6 @@ class Menu extends UI5Element { } } - _prepareSubMenuPhone(item: MenuItem) { - this._prepareCurrentItems(item.items); - this._parentMenuItem = item; - this._parentItemsStack.push(item); - } - _onfocusin(e: FocusEvent): void { const target = e.target as HTMLElement; const menuListItem = target.hasAttribute("ui5-menu-li") @@ -561,7 +540,7 @@ class Menu extends UI5Element { // Sets the new timeout this._timeout = setTimeout(() => { - this._prepareSubMenuDesktopTablet(item, opener); + this._prepareSubMenu(item, opener); }, MENU_OPEN_DELAY); } @@ -620,7 +599,7 @@ class Menu extends UI5Element { const opener = e.target as OpenerStandardListItem; const item = opener.associatedItem; - item.hasSubmenu && this._prepareSubMenuDesktopTablet(item, opener); + item.hasSubmenu && this._prepareSubMenu(item, opener); } else if (shouldCloseMenu && this._isSubMenu && this._parentMenuItem) { const parentMenuItemParent = this._parentMenuItem.parentElement as Menu; parentMenuItemParent._closeItemSubMenu(this._parentMenuItem, true, true); @@ -634,9 +613,6 @@ class Menu extends UI5Element { if (!item.hasSubmenu) { // click on an item that doesn't have sub-items fires an "item-click" event if (!this._isSubMenu) { - if (isPhone()) { - this._parentMenuItem = undefined; - } // fire event if the click is on top-level menu item const prevented = !this.fireEvent("item-click", { "item": item, @@ -666,22 +642,22 @@ class Menu extends UI5Element { mainMenu._popover!.close(); } } - } else if (isPhone()) { - // prepares and opens sub-menu on phone - this._prepareSubMenuPhone(item); - } else if (isTablet()) { - // prepares and opens sub-menu on tablet - this._prepareSubMenuDesktopTablet(item, opener); + } else { + this._prepareSubMenu(item, opener); } } - _findMainMenu(item: MenuItem) { - let parentMenu = item.parentElement as Menu; - while (parentMenu._parentMenuItem) { - parentMenu = parentMenu._parentMenuItem.parentElement as Menu; + _findMainMenu(element: MenuItem | Menu) { + let menu = this._isMenu(element) ? element as Menu : element.parentElement as Menu; + while (menu && menu._parentMenuItem) { + menu = menu._parentMenuItem.parentElement as Menu; } - return parentMenu; + return menu; + } + + _isMenu(element: HTMLElement) { + return element.hasAttribute("ui5-menu"); } _beforePopoverOpen(e: CustomEvent) { diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 5a34b2f96397..585860e0dd99 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -168,7 +168,7 @@ class MenuItem extends UI5Element { } get subMenuOpened() { - return !!this._subMenu; + return !!this._subMenu?._popover?.isOpen(); } get _additionalText() { diff --git a/packages/main/src/NavigationMenu.ts b/packages/main/src/NavigationMenu.ts index 704810a689a2..1f5a4f5c59aa 100644 --- a/packages/main/src/NavigationMenu.ts +++ b/packages/main/src/NavigationMenu.ts @@ -3,8 +3,6 @@ import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js"; import { isDesktop, - isPhone, - isTablet, } from "@ui5/webcomponents-base/dist/Device.js"; import type { ListItemClickEventDetail } from "./List.js"; import Menu from "./Menu.js"; @@ -67,6 +65,10 @@ class NavigationMenu extends Menu { @slot({ "default": true, type: HTMLElement, invalidateOnChildChange: true }) declare items: Array; + _isMenu(element: HTMLElement) { + return element.hasAttribute("ui5-navigation-menu"); + } + _itemMouseOver(e: MouseEvent) { if (isDesktop()) { // respect mouseover only on desktop @@ -124,14 +126,9 @@ class NavigationMenu extends Menu { mainMenu._popover!.close(); } - if (isPhone()) { - // prepares and opens sub-menu on phone - this._prepareSubMenuPhone(item); - } else if (isTablet()) { - // prepares and opens sub-menu on tablet - this._prepareSubMenuDesktopTablet(item, opener); - } + this._prepareSubMenu(item, opener); } + get accSideNavigationPopoverHiddenText() { return NavigationMenu.i18nBundle.getText(NAVIGATION_MENU_POPOVER_HIDDEN_TEXT); } diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index 42a3ffb4900d..72779f1b1bf9 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -15,7 +15,7 @@ Open Menu
- + @@ -119,7 +119,7 @@ menu.addEventListener("ui5-item-click", function(event) { const item = event.detail.item; selectionInput.value = item.text; - if (item && item.text === "New File") { + if (item && item.text === "New File(selection prevented)") { event.preventDefault(); } }); diff --git a/packages/main/test/specs/Menu.spec.js b/packages/main/test/specs/Menu.spec.js index b9d6eebacdd6..0369b8b5fd9f 100644 --- a/packages/main/test/specs/Menu.spec.js +++ b/packages/main/test/specs/Menu.spec.js @@ -59,24 +59,22 @@ describe("Menu interaction", () => { listItems[3].click(); // open sub-menu - await submenuList.$("ui5-menu").waitForExist({ + await submenuList.$("ui5-menu:nth-of-type(1)").waitForExist({ timeout: 1000, - timeoutMsg: "The second level sub-menu is should be created" + timeoutMsg: "First sub-menu is created" }) assert.ok(await submenuList.$("ui5-menu"), "The second level sub-menu is being created"); // new ui5-menu element is created for the sub-menu - await browser.keys("ArrowLeft"); // back to main menu - await browser.keys("ArrowDown"); // go to the next menu item (close sub-menu) + listItems[4].click(); // open sub-menu - await submenuList.$("ui5-menu").waitForExist({ - reverse: true, + await submenuList.$("ui5-menu:nth-of-type(2)").waitForExist({ timeout: 1000, - timeoutMsg: "The second level sub-menu is should be destroyed" + timeoutMsg: "Second sub-menu is created" }) - assert.strictEqual(await submenuList.$$("ui5-menu").length, 0, - "The second level sub-menu is being destroyed"); // sub-menu ui5-menu element is destroyed + assert.strictEqual(await submenuList.$$("ui5-menu").length, 2, + "Two sub-menus are present"); }); it("Event firing after 'click' on menu item", async () => { @@ -92,7 +90,7 @@ describe("Menu interaction", () => { await listItems[0].click({x: 1, y: 1}); - assert.strictEqual(await selectionInput.getAttribute("value"), "New File", "Click on first item fires an event"); + assert.strictEqual(await selectionInput.getAttribute("value"), "New File(selection prevented)", "Click on first item fires an event"); }); it("Event firing after [Space] on menu item", async () => { @@ -107,7 +105,7 @@ describe("Menu interaction", () => { await browser.keys("Space"); - assert.strictEqual(await selectionInput.getAttribute("value"), "New File", "Pressing [Space] on first item fires an event"); + assert.strictEqual(await selectionInput.getAttribute("value"), "New File(selection prevented)", "Pressing [Space] on first item fires an event"); }); it("Event firing after [Enter] on menu item", async () => { @@ -122,7 +120,7 @@ describe("Menu interaction", () => { await browser.keys("Enter"); - assert.strictEqual(await selectionInput.getAttribute("value"), "New File", "Pressing [Enter] on first item fires an event"); + assert.strictEqual(await selectionInput.getAttribute("value"), "New File(selection prevented)", "Pressing [Enter] on first item fires an event"); }); it("Events firing on open/close of the menu", async () => { @@ -180,7 +178,7 @@ describe("Menu interaction", () => { await browser.pause(100); const menuPopover = await browser.$("ui5-static-area-item:last-of-type").shadow$("ui5-responsive-popover"); - const newFileItem = await menuPopover.$("ui5-menu-li[accessible-name='New File']"); + const newFileItem = await menuPopover.$("ui5-menu-li[accessible-name='New File(selection prevented)']"); newFileItem.click(); await browser.pause(100); @@ -217,11 +215,11 @@ describe("Menu Accessibility", () => { assert.strictEqual(await list.getAttribute("accessible-role"), "menu", "There is proper 'menu' role for the menu list"); assert.strictEqual(await listItems[0].getAttribute("accessible-role"), "menuitem", "There is proper 'menuitem' role for the menu list items"); - assert.strictEqual(await listItems[0].getAttribute("tooltip"), "Select a file", "There is a tooltip"); + assert.strictEqual(await listItems[0].getAttribute("tooltip"), "Select a file - prevent default", "There is a tooltip"); assert.strictEqual(await listItems[2].shadow$(".ui5-li-root").getAttribute("aria-haspopup"), "menu", "There is an aria-haspopup attribute"); assert.strictEqual( await listItems[0].getAttribute("accessible-name"), - "New File Opens a file explorer", + "New File(selection prevented) Opens a file explorer", "There is additional description added"); }); });