From d62785bc975f0489b7e7fbd95d75d40f5c1b0c15 Mon Sep 17 00:00:00 2001 From: Diana Petcheva Date: Fri, 13 Feb 2026 22:36:42 +0200 Subject: [PATCH] fix(ui5-menu): close menu with f4, Alt + ArrowUp/Down when opened by SplitButton --- packages/main/cypress/specs/Menu.cy.tsx | 88 +++++++++++++++++++ .../cypress/support/commands/Menu.commands.ts | 17 ++++ packages/main/src/Menu.ts | 11 ++- packages/main/src/SplitButton.ts | 8 ++ 4 files changed, 122 insertions(+), 2 deletions(-) diff --git a/packages/main/cypress/specs/Menu.cy.tsx b/packages/main/cypress/specs/Menu.cy.tsx index d0e0ace3e496..68434210afc1 100644 --- a/packages/main/cypress/specs/Menu.cy.tsx +++ b/packages/main/cypress/specs/Menu.cy.tsx @@ -1,4 +1,5 @@ import Button from "../../src/Button.js"; +import SplitButton from "../../src/SplitButton.js"; import Menu from "../../src/Menu.js"; import MenuItem from "../../src/MenuItem.js"; import MenuItemGroup from "../../src/MenuItemGroup.js"; @@ -349,6 +350,93 @@ describe("Menu interaction", () => { .ui5MenuItemCheckShiftClickAndPress("[text='Outside']", "not.have.attr"); }); + it("should close menu with Alt+ArrowDown when opened by a SplitButton", () => { + cy.mount( + <> + Open Menu + + + + + + ); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .ui5MenuOpen(); + + cy.get("[ui5-menu] > [ui5-menu-item]") + .as("items"); + + cy.get("@items") + .first() + .should("be.focused") + .realPress(["Alt", "ArrowDown"]); + + cy.get("@menu") + .ui5MenuClosed(); + }); + + it("should close menu with Alt+ArrowUp when opened by a SplitButton", () => { + cy.mount( + <> + Open Menu + + + + + + ); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .ui5MenuOpen(); + + cy.get("[ui5-menu] > [ui5-menu-item]") + .as("items"); + + cy.get("@items") + .first() + .should("be.focused") + .realPress(["Alt", "ArrowUp"]); + + cy.get("@menu") + .ui5MenuClosed(); + }); + + it("should close menu with F4 when opened by a SplitButton", () => { + cy.mount( + <> + Open Menu + + + + + + ); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .ui5MenuOpen(); + + cy.get("[ui5-menu] > [ui5-menu-item]") + .as("items"); + + cy.get("@items") + .first() + .should("be.focused") + .realPress("F4"); + + cy.get("@menu") + .ui5MenuClosed(); + }); + describe("Event firing", () => { it("Event firing - 'ui5-item-click' after 'click' on menu item", () => { cy.mount( diff --git a/packages/main/cypress/support/commands/Menu.commands.ts b/packages/main/cypress/support/commands/Menu.commands.ts index 26f934654906..bc04fabf9513 100644 --- a/packages/main/cypress/support/commands/Menu.commands.ts +++ b/packages/main/cypress/support/commands/Menu.commands.ts @@ -33,6 +33,22 @@ Cypress.Commands.add("ui5MenuOpened", { prevSubject: true }, subject => { .and("have.attr", "open"); }); +Cypress.Commands.add("ui5MenuClosed", { prevSubject: true }, subject => { + cy.wrap(subject) + .as("menu"); + + cy.get("@menu") + .should("not.have.attr", "open"); + + cy.get("@menu") + .shadow() + .find("[ui5-responsive-popover]") + .should($rp => { + expect($rp.is(":popover-open")).to.be.false; + }) + .and("not.have.attr", "open"); +}); + Cypress.Commands.add("ui5MenuItemClick", { prevSubject: true }, subject => { cy.get(subject) .as("item") @@ -104,6 +120,7 @@ declare global { interface Chainable { ui5MenuOpen(options?: { opener?: string }): Chainable ui5MenuOpened(): Chainable + ui5MenuClosed(): Chainable ui5MenuItemClick(): Chainable ui5MenuItemPress(key: any): Chainable ui5MenuItemCheckShiftClickAndPress(menuItem: string, shouldStatement: string): Chainable diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index a11c114dcd8e..0cab11a2bb56 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -10,6 +10,7 @@ import { isEnter, isTabNext, isTabPrevious, + isShow, } from "@ui5/webcomponents-base/dist/Keys.js"; import { isPhone, @@ -29,6 +30,7 @@ import type MenuItem from "./MenuItem.js"; import { isInstanceOfMenuItem } from "./MenuItem.js"; import { isInstanceOfMenuItemGroup } from "./MenuItemGroup.js"; import { isInstanceOfMenuSeparator } from "./MenuSeparator.js"; +import { isInstanceOfSplitButton } from "./SplitButton.js"; import type PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js"; import type PopoverPlacement from "./types/PopoverPlacement.js"; import type { @@ -280,6 +282,10 @@ class Menu extends UI5Element { return this.shadowRoot!.querySelector("[ui5-list]"); } + get _opener() { + return typeof this.opener === "string" ? document.getElementById(this.opener) : this.opener; + } + /** Returns menu item groups */ get _menuItemGroups() { return this.items.filter(isInstanceOfMenuItemGroup); @@ -438,6 +444,7 @@ class Menu extends UI5Element { _itemKeyDown(e: KeyboardEvent) { const isTabNextPrevious = isTabNext(e) || isTabPrevious(e); + const isShowKey = isShow(e); const item = e.target as MenuItem; if (!isInstanceOfMenuItem(item)) { @@ -447,7 +454,7 @@ class Menu extends UI5Element { const isEndContentNavigation = isRight(e) || isLeft(e); const shouldOpenMenu = this.isRtl ? isLeft(e) : isRight(e); - if (isEnter(e) || isTabNextPrevious) { + if (isEnter(e) || isTabNextPrevious || isShowKey) { e.preventDefault(); } @@ -457,7 +464,7 @@ class Menu extends UI5Element { if (shouldOpenMenu) { this._openItemSubMenu(item, false); - } else if (isTabNextPrevious) { + } else if (isTabNextPrevious || (isShowKey && this._opener && isInstanceOfSplitButton(this._opener))) { this._close(); } } diff --git a/packages/main/src/SplitButton.ts b/packages/main/src/SplitButton.ts index 1af8e659a46c..f4073f39e732 100644 --- a/packages/main/src/SplitButton.ts +++ b/packages/main/src/SplitButton.ts @@ -1,5 +1,6 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; import type { DefaultSlot } from "@ui5/webcomponents-base/dist/UI5Element.js"; +import createInstanceChecker from "@ui5/webcomponents-base/dist/util/createInstanceChecker.js"; import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js"; import event from "@ui5/webcomponents-base/dist/decorators/event-strict.js"; import property from "@ui5/webcomponents-base/dist/decorators/property.js"; @@ -491,6 +492,10 @@ class SplitButton extends UI5Element { return SplitButton.i18nBundle.getText(SPLIT_BUTTON_ARROW_BUTTON_TOOLTIP); } + get isSplitButton(): boolean { + return true; + } + get ariaLabelText() { return [SplitButton.i18nBundle.getText(SPLIT_BUTTON_DESCRIPTION), SplitButton.i18nBundle.getText(SPLIT_BUTTON_KEYBOARD_HINT)].join(" "); } @@ -502,3 +507,6 @@ export default SplitButton; export type { SplitButtonAccessibilityAttributes, }; + +// Todo - change it to isInstanceOfMenuButton and change the function to check for the presence of opensMenu getter? +export const isInstanceOfSplitButton = createInstanceChecker("isSplitButton");