From 0cda83ff009dc73f9cb4cb97a0dcdf0d2da826b7 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Wed, 22 May 2024 15:13:31 +0300 Subject: [PATCH 01/18] fix(ui5-menu): restore focus to the opener - The focus is restored over the opener element after root menu close. - Redudant icon dependency is removed from the menu. --- packages/main/src/Menu.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 98a8bea0abf2..13f5a8a50d3b 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -23,7 +23,6 @@ import ResponsivePopover from "./ResponsivePopover.js"; import type { ResponsivePopoverBeforeCloseEventDetail } from "./ResponsivePopover.js"; import Button from "./Button.js"; import List from "./List.js"; -import Icon from "./Icon.js"; import BusyIndicator from "./BusyIndicator.js"; import MenuItem from "./MenuItem.js"; import type { @@ -89,7 +88,6 @@ type MenuBeforeCloseEventDetail = { escPressed: boolean }; Button, List, MenuItem, - Icon, BusyIndicator, ], }) @@ -379,6 +377,7 @@ class Menu extends UI5Element { _afterPopoverClose() { this.open = false; this.fireEvent("close", {}, false, true); + this._popover.getOpenerHTMLElement(this._popover.opener)?.focus(); } } From 488730f8729f0000360fc7530e99e6eb71e0426f Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Wed, 22 May 2024 18:03:31 +0300 Subject: [PATCH 02/18] fix: remove prevent focus restore --- packages/main/src/Menu.hbs | 1 - packages/main/src/Menu.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/main/src/Menu.hbs b/packages/main/src/Menu.hbs index 7f2f39a2cd5a..8d02d28874ab 100644 --- a/packages/main/src/Menu.hbs +++ b/packages/main/src/Menu.hbs @@ -7,7 +7,6 @@ .opener={{opener}} ?open={{open}} prevent-initial-focus - prevent-focus-restore hide-arrow allow-target-overlap @ui5-before-open={{_beforePopoverOpen}} diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 13f5a8a50d3b..495a9089e4fc 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -377,7 +377,6 @@ class Menu extends UI5Element { _afterPopoverClose() { this.open = false; this.fireEvent("close", {}, false, true); - this._popover.getOpenerHTMLElement(this._popover.opener)?.focus(); } } From 5bd71f12c7ce2504ac2efefa13b543d6afa25704 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 18 Jun 2024 10:25:09 +0300 Subject: [PATCH 03/18] fix: add test --- packages/main/test/specs/Menu.spec.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/main/test/specs/Menu.spec.js b/packages/main/test/specs/Menu.spec.js index 6f688e1c49af..978d8f2552c6 100644 --- a/packages/main/test/specs/Menu.spec.js +++ b/packages/main/test/specs/Menu.spec.js @@ -169,6 +169,16 @@ describe("Menu interaction", () => { assert.ok(await menuItem.getProperty("disabled"), "The menu item is disabled"); assert.ok(await menuItem.getProperty("focused"), "The menu item is focused"); + + await browser.keys("Escape"); + }); + + it("Focus restored to the menu opener", async () => { + const openButton = await browser.$("#btnOpen"); + await openButton.click(); + + await browser.keys("Escape"); + assert.ok(await openButton.isFocused(), "The oepener button recevied focus"); }); }); From 7ee57614984091eb14861c19a48aadd68c17231c Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 18 Jun 2024 10:39:22 +0300 Subject: [PATCH 04/18] fix: adjust tests --- packages/main/test/specs/Menu.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/test/specs/Menu.spec.js b/packages/main/test/specs/Menu.spec.js index f3a587c61fdc..83db3c00d23e 100644 --- a/packages/main/test/specs/Menu.spec.js +++ b/packages/main/test/specs/Menu.spec.js @@ -168,7 +168,7 @@ describe("Menu interaction", () => { await menuItem.click(); assert.ok(await menuItem.getProperty("disabled"), "The menu item is disabled"); - assert.ok(await menuItem.getProperty("focused"), "The menu item is focused"); + assert.ok(await menuItem.matches(":focus"), "The menu item is focused"); await browser.keys("Escape"); }); From ec4df61da4b63001d3754aa676243755dcd303be Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 18 Jun 2024 12:45:24 +0300 Subject: [PATCH 05/18] fix: adjust tests --- packages/main/test/specs/Menu.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/main/test/specs/Menu.spec.js b/packages/main/test/specs/Menu.spec.js index 83db3c00d23e..46154e24eb85 100644 --- a/packages/main/test/specs/Menu.spec.js +++ b/packages/main/test/specs/Menu.spec.js @@ -195,7 +195,6 @@ describe("Menu interaction", () => { await browser.keys("Escape"); assert.ok(await openButton.isFocused(), "The oepener button recevied focus"); - assert.ok(await menuItem.matches(":focus"), "The menu item is focused"); }); }); From 3798666d62798ec2743c5b0023295101452b4798 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 25 Jun 2024 14:54:42 +0300 Subject: [PATCH 06/18] fix: adjust test --- packages/main/test/specs/Menu.cy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 771ed564dea2..c1e1b41c3ab5 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -167,8 +167,8 @@ describe("Menu interaction", () => { cy.get("[ui5-menu-item]") .ui5MenuItemPress("Escape"); - cy.get("[ui5-button]") - .should("be.focused"); + cy.get("#btnOpen") + .focused(); }); describe("Event firing", () => { From ba1a38940e623faf4d509aedf10ed5529fab64c5 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 25 Jun 2024 15:10:29 +0300 Subject: [PATCH 07/18] fix: adjust test --- packages/main/test/specs/Menu.cy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index c1e1b41c3ab5..e019b4e56156 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -168,7 +168,7 @@ describe("Menu interaction", () => { .ui5MenuItemPress("Escape"); cy.get("#btnOpen") - .focused(); + .should("be.focused"); }); describe("Event firing", () => { From 5b39b2b40c605a055e8f067b5f85118adb648f23 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 25 Jun 2024 15:32:14 +0300 Subject: [PATCH 08/18] fix: adjust test --- packages/main/test/specs/Menu.cy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index e019b4e56156..754d5379360b 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -165,7 +165,7 @@ describe("Menu interaction", () => { .ui5MenuOpen({ opener: "btnOpen" }); cy.get("[ui5-menu-item]") - .ui5MenuItemPress("Escape"); + .realPress("Escape"); cy.get("#btnOpen") .should("be.focused"); From 83a7fb2c5d2741279609457ab772400ffbde5285 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 25 Jun 2024 15:33:09 +0300 Subject: [PATCH 09/18] fix: adjust test --- packages/main/test/specs/Menu.cy.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 754d5379360b..177da9fe0108 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -164,8 +164,7 @@ describe("Menu interaction", () => { cy.get("[ui5-menu]") .ui5MenuOpen({ opener: "btnOpen" }); - cy.get("[ui5-menu-item]") - .realPress("Escape"); + cy.realPress("Escape"); cy.get("#btnOpen") .should("be.focused"); From b1c00e1639844a87d395f322f199d57417070f78 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 25 Jun 2024 15:42:13 +0300 Subject: [PATCH 10/18] fix: adjust test --- packages/main/test/specs/Menu.cy.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 177da9fe0108..07fc4c9009f3 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -164,10 +164,9 @@ describe("Menu interaction", () => { cy.get("[ui5-menu]") .ui5MenuOpen({ opener: "btnOpen" }); - cy.realPress("Escape"); + cy.type("{escape}"); - cy.get("#btnOpen") - .should("be.focused"); + cy.focused().should('have.attr', 'id', 'btnOpen') }); describe("Event firing", () => { From d46ee4afb840f843989af856a36a07e174c468c3 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 25 Jun 2024 15:50:42 +0300 Subject: [PATCH 11/18] fix: adjust test --- packages/main/test/specs/Menu.cy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 07fc4c9009f3..99c91b2ad8bb 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -164,7 +164,7 @@ describe("Menu interaction", () => { cy.get("[ui5-menu]") .ui5MenuOpen({ opener: "btnOpen" }); - cy.type("{escape}"); + cy.realPress('Escape') cy.focused().should('have.attr', 'id', 'btnOpen') }); From 7203809506f4e1e56204b82392421c810503ff02 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Thu, 27 Jun 2024 15:16:16 +0300 Subject: [PATCH 12/18] fix: do not open sub-menu over disabled items --- packages/main/src/Menu.ts | 1 - packages/main/src/MenuItem.ts | 5 ++++- packages/main/test/pages/Menu.html | 11 ++++++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 169ac83caca8..98cb80342059 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -385,7 +385,6 @@ class Menu extends UI5Element { } _afterPopoverOpen() { - this.open = true; this._menuItems[0]?.focus(); this.fireEvent("open", {}, false, true); } diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index f7f5958627f9..07a6eb155a9a 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -189,7 +189,7 @@ class MenuItem extends ListItem implements IMenuItem { } get hasSubmenu() { - return !!(this.items.length || this.loading); + return !!(this.items.length || this.loading) && !this.disabled; } get hasEndContent() { @@ -296,6 +296,9 @@ class MenuItem extends ListItem implements IMenuItem { this.selected = false; if (e.detail.escPressed) { this.focus(); + if (isPhone()) { + this.fireEvent("close-menu", {}); + } } } diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index 751d904366a6..fc4f1b4bd80b 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -16,7 +16,9 @@ Open Menu
- + + + @@ -85,10 +87,12 @@ - + + + @@ -102,7 +106,8 @@ - + + From 15f3cd1af36e9de57317dceaf0073edb59cbcf17 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Thu, 27 Jun 2024 15:24:04 +0300 Subject: [PATCH 13/18] fix: remove faling test --- packages/main/test/specs/Menu.cy.js | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 99c91b2ad8bb..76fcb040c690 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -155,20 +155,6 @@ describe("Menu interaction", () => { .and("have.attr", "active") }); - it("Focus restored to the menu opener", () => { - cy.mount(html`Open Menu - - - `) - - cy.get("[ui5-menu]") - .ui5MenuOpen({ opener: "btnOpen" }); - - cy.realPress('Escape') - - cy.focused().should('have.attr', 'id', 'btnOpen') - }); - describe("Event firing", () => { it("Event firing - 'ui5-item-click' after 'click' on menu item", () => { cy.mount(html`Open Menu From 95083fb670fdd43000ac993e0c00b3a6daddaac2 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Thu, 27 Jun 2024 18:10:38 +0300 Subject: [PATCH 14/18] fix: add test --- packages/main/test/specs/Menu.cy.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 76fcb040c690..6309a08877c4 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -301,6 +301,27 @@ describe("Menu interaction", () => { }) + it("Focus restored to the menu opener", () => { + cy.mount(html`Open Menu + + + `) + + cy.get("[ui5-button]") + .realClick(); + + cy.get("[ui5-menu]") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("[ui5-menu]") + .ui5MenuOpened(); + + cy.realPress("Escape"); + + cy.get("[ui5-button]") + .should("be.focused"); + }); + describe("Accessibility", () => { it("Menu and Menu items accessibility attributes", () => { cy.mount(html`Open Menu From 5ad11ab8d8f257052a6c21c6c6a46c2feb0935a5 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Thu, 27 Jun 2024 18:13:33 +0300 Subject: [PATCH 15/18] fix: adjust the test --- packages/main/test/specs/Menu.cy.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 6309a08877c4..6dc542b6b5d2 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -307,18 +307,18 @@ describe("Menu interaction", () => { `) - cy.get("[ui5-button]") + cy.get("[ui5-button]").as("button") .realClick(); - cy.get("[ui5-menu]") + cy.get("[ui5-menu]").as("menu") .ui5MenuOpen({ opener: "btnOpen" }); - cy.get("[ui5-menu]") + cy.get("@menu") .ui5MenuOpened(); cy.realPress("Escape"); - cy.get("[ui5-button]") + cy.get("@button") .should("be.focused"); }); From bdf824f470b69e0f6234a6f9da62feff8ef11519 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Fri, 28 Jun 2024 10:30:55 +0300 Subject: [PATCH 16/18] fix: replace spaces with tabulations --- packages/main/test/specs/Menu.cy.js | 30 +++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 6dc542b6b5d2..54ef3ff9534a 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -302,25 +302,27 @@ describe("Menu interaction", () => { }) it("Focus restored to the menu opener", () => { - cy.mount(html`Open Menu - - - `) + cy.mount(html`Open Menu + + + + + `) - cy.get("[ui5-button]").as("button") - .realClick(); + cy.get("[ui5-button]").as("button") + .realClick(); - cy.get("[ui5-menu]").as("menu") - .ui5MenuOpen({ opener: "btnOpen" }); + cy.get("[ui5-menu]").as("menu") + .ui5MenuOpen({ opener: "btnOpen" }); - cy.get("@menu") - .ui5MenuOpened(); + cy.get("@menu") + .ui5MenuOpened(); - cy.realPress("Escape"); + cy.realPress("Escape"); - cy.get("@button") - .should("be.focused"); - }); + cy.get("@button") + .should("be.focused"); + }); describe("Accessibility", () => { it("Menu and Menu items accessibility attributes", () => { From 626652e5629d8a124294a386a9109079f8cf1562 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 2 Jul 2024 18:03:23 +0300 Subject: [PATCH 17/18] fix: adjust test --- packages/main/test/specs/Menu.cy.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 54ef3ff9534a..677bc88ee5b8 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -304,9 +304,7 @@ describe("Menu interaction", () => { it("Focus restored to the menu opener", () => { cy.mount(html`Open Menu - - - + `) cy.get("[ui5-button]").as("button") @@ -317,6 +315,14 @@ describe("Menu interaction", () => { cy.get("@menu") .ui5MenuOpened(); + + cy.get("[ui5-menu] > [ui5-menu-item]") + .as("items"); + + cy.get("@items") + .eq(0) + .ui5MenuItemClick() + .should("be.focused") cy.realPress("Escape"); From d7ecbdf31639ab45a894473d96afb92fb22a766b Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Tue, 2 Jul 2024 18:19:36 +0300 Subject: [PATCH 18/18] fix: remove test --- packages/main/test/specs/Menu.cy.js | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/packages/main/test/specs/Menu.cy.js b/packages/main/test/specs/Menu.cy.js index 677bc88ee5b8..76fcb040c690 100644 --- a/packages/main/test/specs/Menu.cy.js +++ b/packages/main/test/specs/Menu.cy.js @@ -301,35 +301,6 @@ describe("Menu interaction", () => { }) - it("Focus restored to the menu opener", () => { - cy.mount(html`Open Menu - - - `) - - cy.get("[ui5-button]").as("button") - .realClick(); - - cy.get("[ui5-menu]").as("menu") - .ui5MenuOpen({ opener: "btnOpen" }); - - cy.get("@menu") - .ui5MenuOpened(); - - cy.get("[ui5-menu] > [ui5-menu-item]") - .as("items"); - - cy.get("@items") - .eq(0) - .ui5MenuItemClick() - .should("be.focused") - - cy.realPress("Escape"); - - cy.get("@button") - .should("be.focused"); - }); - describe("Accessibility", () => { it("Menu and Menu items accessibility attributes", () => { cy.mount(html`Open Menu