From a26b1ae30be8927eba19257efbb09eaf7a66993f Mon Sep 17 00:00:00 2001 From: Lidiya Georgieva Date: Tue, 4 Feb 2025 14:52:56 +0200 Subject: [PATCH 1/7] fix(ui5-side-navigation): "Space" key triggers links Pressing "Space" key triggers click on the items with external links. In addition inappropriate exclamation mark usage removed. fixes: #10654 fixes: #10714 --- .../fiori/cypress/specs/SideNavigation.cy.tsx | 56 +++++++++++++++++++ packages/fiori/src/SideNavigation.ts | 14 +++-- .../src/SideNavigationSelectableItemBase.ts | 12 ++++ packages/fiori/test/pages/SideNavigation.html | 1 + 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/packages/fiori/cypress/specs/SideNavigation.cy.tsx b/packages/fiori/cypress/specs/SideNavigation.cy.tsx index 08672208add1..93d6a9f0becd 100644 --- a/packages/fiori/cypress/specs/SideNavigation.cy.tsx +++ b/packages/fiori/cypress/specs/SideNavigation.cy.tsx @@ -308,6 +308,62 @@ describe("Side Navigation interaction", () => { }); }); + it("Tests link opening with mouse click", () => { + cy.mount( + + + + + + + + ); + + cy.url().should("not.include", "#test"); + cy.get("#unselectableItemWithLink").shadow().find("a").invoke("removeAttr", "target"); + + cy.get("#unselectableItemWithLink").realClick(); + + cy.url().should("include", "#test"); + + // Remove #test from the URL + cy.window().then(win => { + win.history.pushState({}, "", win.location.pathname + win.location.search); + }); + + cy.url().should("not.include", "#test"); + }); + + it("Tests link opening with Enter", () => { + cy.mount( + + + + + + + + ); + + cy.url().should("not.include", "#test"); + cy.get("#unselectableItemWithLink").shadow().find("a").invoke("removeAttr", "target"); + + // cy.get("#unselectableItemWithLink").trigger("keydown", { key: "Enter" }); + cy.get("#unselectableItemWithLink").realPress("Enter"); + + cy.url().should("include", "#test"); + + // Remove #test from the URL + cy.window().then(win => { + win.history.pushState({}, "", win.location.pathname + win.location.search); + }); + + cy.url().should("not.include", "#test"); + }); + + // it("Tests link opening with Space", () => { + // }); + it("Tests 'selection-change' event", () => { cy.mount( diff --git a/packages/fiori/src/SideNavigation.ts b/packages/fiori/src/SideNavigation.ts index 33c66887444f..3e4d41c7cf1c 100644 --- a/packages/fiori/src/SideNavigation.ts +++ b/packages/fiori/src/SideNavigation.ts @@ -492,17 +492,19 @@ class SideNavigation extends UI5Element { let itemDomRef; - if (isInstanceOfSideNavigationItemBase(item)) { - itemDomRef = item.getDomRef()!; + if (isInstanceOfSideNavigationItemBase(item) && item.getDomRef()) { + itemDomRef = item.getDomRef(); } else { itemDomRef = item; } - const { marginTop, marginBottom } = window.getComputedStyle(itemDomRef); - itemsHeight += itemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom); + if (itemDomRef) { + const { marginTop, marginBottom } = window.getComputedStyle(itemDomRef); + itemsHeight += itemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom); - if (itemsHeight > listHeight) { - item.classList.add("ui5-sn-item-hidden"); + if (itemsHeight > listHeight) { + item.classList.add("ui5-sn-item-hidden"); + } } }); diff --git a/packages/fiori/src/SideNavigationSelectableItemBase.ts b/packages/fiori/src/SideNavigationSelectableItemBase.ts index 2cae53b00af0..9ab84f33bd34 100644 --- a/packages/fiori/src/SideNavigationSelectableItemBase.ts +++ b/packages/fiori/src/SideNavigationSelectableItemBase.ts @@ -206,6 +206,18 @@ class SideNavigationSelectableItemBase extends SideNavigationItemBase { _onkeyup(e: KeyboardEvent) { if (isSpace(e)) { this._activate(e); + + if (this.href && !e.defaultPrevented) { + const customEvent = new MouseEvent("click"); + + customEvent.stopImmediatePropagation(); + if (this.getDomRef()!.querySelector("a")) { + this.getDomRef()!.querySelector("a")!.dispatchEvent(customEvent); + } else { + // when Side Navigation is collapsed and it is first level item we have directly element + this.getDomRef()!.dispatchEvent(customEvent); + } + } } } diff --git a/packages/fiori/test/pages/SideNavigation.html b/packages/fiori/test/pages/SideNavigation.html index e75ab5dfefa1..670287db1238 100644 --- a/packages/fiori/test/pages/SideNavigation.html +++ b/packages/fiori/test/pages/SideNavigation.html @@ -47,6 +47,7 @@ + From f4f333ee5eae3d9792a5da1b725d542150a0a44d Mon Sep 17 00:00:00 2001 From: Lidiya Georgieva Date: Tue, 4 Feb 2025 15:58:44 +0200 Subject: [PATCH 2/7] chore: split fixes --- packages/fiori/src/SideNavigation.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/fiori/src/SideNavigation.ts b/packages/fiori/src/SideNavigation.ts index 3e4d41c7cf1c..33c66887444f 100644 --- a/packages/fiori/src/SideNavigation.ts +++ b/packages/fiori/src/SideNavigation.ts @@ -492,19 +492,17 @@ class SideNavigation extends UI5Element { let itemDomRef; - if (isInstanceOfSideNavigationItemBase(item) && item.getDomRef()) { - itemDomRef = item.getDomRef(); + if (isInstanceOfSideNavigationItemBase(item)) { + itemDomRef = item.getDomRef()!; } else { itemDomRef = item; } - if (itemDomRef) { - const { marginTop, marginBottom } = window.getComputedStyle(itemDomRef); - itemsHeight += itemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom); + const { marginTop, marginBottom } = window.getComputedStyle(itemDomRef); + itemsHeight += itemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom); - if (itemsHeight > listHeight) { - item.classList.add("ui5-sn-item-hidden"); - } + if (itemsHeight > listHeight) { + item.classList.add("ui5-sn-item-hidden"); } }); From 4621578dcee060fc74150c88eace405c57235361 Mon Sep 17 00:00:00 2001 From: Lidiya Georgieva Date: Wed, 5 Feb 2025 08:58:03 +0200 Subject: [PATCH 3/7] chore: fix tests --- .../fiori/cypress/specs/SideNavigation.cy.tsx | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/packages/fiori/cypress/specs/SideNavigation.cy.tsx b/packages/fiori/cypress/specs/SideNavigation.cy.tsx index 93d6a9f0becd..f5ee2682380a 100644 --- a/packages/fiori/cypress/specs/SideNavigation.cy.tsx +++ b/packages/fiori/cypress/specs/SideNavigation.cy.tsx @@ -312,15 +312,11 @@ describe("Side Navigation interaction", () => { cy.mount( - - - - + ); cy.url().should("not.include", "#test"); - cy.get("#unselectableItemWithLink").shadow().find("a").invoke("removeAttr", "target"); cy.get("#unselectableItemWithLink").realClick(); @@ -328,7 +324,7 @@ describe("Side Navigation interaction", () => { // Remove #test from the URL cy.window().then(win => { - win.history.pushState({}, "", win.location.pathname + win.location.search); + win.history.back(); }); cy.url().should("not.include", "#test"); @@ -336,33 +332,51 @@ describe("Side Navigation interaction", () => { it("Tests link opening with Enter", () => { cy.mount( - - - - - - + + + ); cy.url().should("not.include", "#test"); - cy.get("#unselectableItemWithLink").shadow().find("a").invoke("removeAttr", "target"); - // cy.get("#unselectableItemWithLink").trigger("keydown", { key: "Enter" }); - cy.get("#unselectableItemWithLink").realPress("Enter"); + cy.get("#focusStart").realClick(); + cy.realPress("ArrowDown"); + cy.realPress("Enter"); cy.url().should("include", "#test"); // Remove #test from the URL cy.window().then(win => { - win.history.pushState({}, "", win.location.pathname + win.location.search); + win.history.back(); }); cy.url().should("not.include", "#test"); }); - // it("Tests link opening with Space", () => { - // }); + it("Tests link opening with Space", () => { + cy.mount( + + + + + ); + + cy.url().should("not.include", "#test"); + + cy.get("#focusStart").realClick(); + cy.realPress("ArrowDown"); + cy.realPress("Space"); + + cy.url().should("include", "#test"); + + // Remove #test from the URL + cy.window().then(win => { + win.history.back(); + }); + + cy.url().should("not.include", "#test"); + }); it("Tests 'selection-change' event", () => { cy.mount( From 4e5d308c85ce17862a277dce7e47eec2fb6462a7 Mon Sep 17 00:00:00 2001 From: Lidiya Georgieva Date: Wed, 5 Feb 2025 15:02:24 +0200 Subject: [PATCH 4/7] chore: add keyup test --- .../fiori/cypress/specs/SideNavigation.cy.tsx | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/fiori/cypress/specs/SideNavigation.cy.tsx b/packages/fiori/cypress/specs/SideNavigation.cy.tsx index f5ee2682380a..75c865b6feea 100644 --- a/packages/fiori/cypress/specs/SideNavigation.cy.tsx +++ b/packages/fiori/cypress/specs/SideNavigation.cy.tsx @@ -358,7 +358,7 @@ describe("Side Navigation interaction", () => { cy.mount( - + ); @@ -376,6 +376,25 @@ describe("Side Navigation interaction", () => { }); cy.url().should("not.include", "#test"); + + cy.get("#focusStart").realClick(); + cy.realPress("ArrowDown"); + cy.realPress("Space"); + cy.get("#linkItem").should("be.focused"); + + // act + cy.focused().trigger("keydown", { + key: "Enter", + }); + + cy.url().should("include", "#test"); + + // Remove #test from the URL + cy.window().then(win => { + win.history.back(); + }); + + cy.url().should("not.include", "#test"); }); it("Tests 'selection-change' event", () => { From dd8905dfdf417e56d1473d8452176b6fadb74417 Mon Sep 17 00:00:00 2001 From: Lidiya Georgieva Date: Thu, 6 Feb 2025 11:04:12 +0200 Subject: [PATCH 5/7] chore: adapt test --- packages/fiori/test/specs/SideNavigation.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/fiori/test/specs/SideNavigation.spec.js b/packages/fiori/test/specs/SideNavigation.spec.js index fa047e61df08..4786212e15a3 100644 --- a/packages/fiori/test/specs/SideNavigation.spec.js +++ b/packages/fiori/test/specs/SideNavigation.spec.js @@ -227,7 +227,7 @@ describe("Component Behavior", () => { assert.strictEqual(await sideNavigationFixedTree.getAttribute("aria-roledescription"), roleDescription, "Role description of the SideNavigation fixed tree element is correctly set"); assert.notExists(await items[10].getAttribute("aria-roledescription"), "Role description of the SideNavigation fixed tree item is not set"); assert.strictEqual(await items[10].getAttribute("aria-haspopup"), "tree", "There is 'aria-haspopup' with correct value"); - assert.notExists(await items[11].getAttribute("aria-haspopup"), "There is no 'aria-haspopup'"); + assert.notExists(await items[12].getAttribute("aria-haspopup"), "There is no 'aria-haspopup'"); // popup await browser.$("#item2").click(); From 65dce299bd34aa90addd36ad027fef3372268b9d Mon Sep 17 00:00:00 2001 From: Lidiya Georgieva Date: Thu, 6 Feb 2025 11:47:22 +0200 Subject: [PATCH 6/7] chore: fix test --- packages/fiori/test/specs/SideNavigation.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fiori/test/specs/SideNavigation.spec.js b/packages/fiori/test/specs/SideNavigation.spec.js index 4786212e15a3..95e0bc32988f 100644 --- a/packages/fiori/test/specs/SideNavigation.spec.js +++ b/packages/fiori/test/specs/SideNavigation.spec.js @@ -225,8 +225,8 @@ describe("Component Behavior", () => { // fixed items assert.strictEqual(await sideNavigationFixedTree.getAttribute("aria-roledescription"), roleDescription, "Role description of the SideNavigation fixed tree element is correctly set"); - assert.notExists(await items[10].getAttribute("aria-roledescription"), "Role description of the SideNavigation fixed tree item is not set"); - assert.strictEqual(await items[10].getAttribute("aria-haspopup"), "tree", "There is 'aria-haspopup' with correct value"); + assert.notExists(await items[11].getAttribute("aria-roledescription"), "Role description of the SideNavigation fixed tree item is not set"); + assert.strictEqual(await items[11].getAttribute("aria-haspopup"), "tree", "There is 'aria-haspopup' with correct value"); assert.notExists(await items[12].getAttribute("aria-haspopup"), "There is no 'aria-haspopup'"); // popup From 454646106f8c7ff298549cf1bc2b6f87c364158b Mon Sep 17 00:00:00 2001 From: Lidiya Georgieva Date: Fri, 7 Feb 2025 11:33:57 +0200 Subject: [PATCH 7/7] chore: fix test --- packages/fiori/cypress/specs/SideNavigation.cy.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/fiori/cypress/specs/SideNavigation.cy.tsx b/packages/fiori/cypress/specs/SideNavigation.cy.tsx index f7a2836d3404..bb177d285c1b 100644 --- a/packages/fiori/cypress/specs/SideNavigation.cy.tsx +++ b/packages/fiori/cypress/specs/SideNavigation.cy.tsx @@ -379,12 +379,11 @@ describe("Side Navigation interaction", () => { cy.get("#focusStart").realClick(); cy.realPress("ArrowDown"); - cy.realPress("Space"); cy.get("#linkItem").should("be.focused"); // act - cy.focused().trigger("keydown", { - key: "Enter", + cy.focused().trigger("keyup", { + key: " ", }); cy.url().should("include", "#test");