From c81a595eebeb3c8cd4a8eb351f3d6eb0878d6002 Mon Sep 17 00:00:00 2001 From: Adam Tirella Date: Tue, 12 Dec 2023 09:41:06 -0800 Subject: [PATCH 1/3] fix(menu-item): Improve keyboard navigability when `href` populated --- .../calcite-components/src/components/menu-item/menu-item.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/calcite-components/src/components/menu-item/menu-item.tsx b/packages/calcite-components/src/components/menu-item/menu-item.tsx index bac34ddeec4..e1c4cd4e45d 100644 --- a/packages/calcite-components/src/components/menu-item/menu-item.tsx +++ b/packages/calcite-components/src/components/menu-item/menu-item.tsx @@ -275,7 +275,9 @@ export class CalciteMenuItem implements LoadableComponent, T9nComponent, Localiz ) { this.open = !this.open; } - event.preventDefault(); + if (!this.href || event.target === this.dropdownActionEl) { + event.preventDefault(); + } } else if (event.key === "Escape") { if (this.open) { this.open = false; From 1470f7c81512cb1131f5639c1cd528f9c0fdfb68 Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 12 Dec 2023 17:50:08 -0800 Subject: [PATCH 2/3] Clean up --- .../src/components/menu-item/menu-item.e2e.ts | 33 +++++++++++++++++++ .../src/components/menu-item/menu-item.tsx | 16 ++++----- .../calcite-components/src/demos/menu.html | 7 ++++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/packages/calcite-components/src/components/menu-item/menu-item.e2e.ts b/packages/calcite-components/src/components/menu-item/menu-item.e2e.ts index f8beb28371b..057d12bf700 100644 --- a/packages/calcite-components/src/components/menu-item/menu-item.e2e.ts +++ b/packages/calcite-components/src/components/menu-item/menu-item.e2e.ts @@ -50,6 +50,39 @@ describe("calcite-menu-item", () => { expect(eventSpy).toHaveReceivedEventTimes(1); }); + describe("href support", () => { + const testHref = "#nature"; + const testEl = ``; + + it("should navigate to a new url when href provided and user interacts with click", async () => { + const page = await newE2EPage(); + await page.setContent(html`${testEl}`); + const originalUrl = page.url(); + await page.waitForChanges(); + + const menuItem = await page.find("calcite-menu-item"); + await page.waitForChanges(); + await menuItem.click(); + await page.waitForChanges(); + const newUrl = page.url(); + expect(newUrl).toEqual(originalUrl + testHref); + }); + + it("should navigate to a new url when href provided and user interacts with `enter` key", async () => { + const page = await newE2EPage(); + await page.setContent(html`${testEl}`); + const originalUrl = page.url(); + await page.waitForChanges(); + + await page.keyboard.press("Tab"); + await page.waitForChanges(); + await page.keyboard.press("Enter"); + await page.waitForChanges(); + const newUrl = page.url(); + expect(newUrl).toEqual(originalUrl + testHref); + }); + }); + it("should emit calciteMenuItemSelect event when user select the text area of the component using Enter or Space key", async () => { const page = await newE2EPage(); await page.setContent(html` diff --git a/packages/calcite-components/src/components/menu-item/menu-item.tsx b/packages/calcite-components/src/components/menu-item/menu-item.tsx index e1c4cd4e45d..b3289737e74 100644 --- a/packages/calcite-components/src/components/menu-item/menu-item.tsx +++ b/packages/calcite-components/src/components/menu-item/menu-item.tsx @@ -267,15 +267,15 @@ export class CalciteMenuItem implements LoadableComponent, T9nComponent, Localiz private keyDownHandler = async (event: KeyboardEvent): Promise => { // opening and closing of submenu is handled here. Any other functionality is bubbled to parent menu. + const targetIsDropdown = event.target === this.dropdownActionEl; if (event.key === " " || event.key === "Enter") { - this.selectMenuItem(event); - if ( - this.hasSubmenu && - (!this.href || (this.href && event.target === this.dropdownActionEl)) - ) { + if (this.hasSubmenu && (!this.href || (this.href && targetIsDropdown))) { this.open = !this.open; } - if (!this.href || event.target === this.dropdownActionEl) { + if (!(this.href && targetIsDropdown) && event.key !== "Enter") { + this.selectMenuItem(event); + } + if (event.key === " " || (this.href && targetIsDropdown)) { event.preventDefault(); } } else if (event.key === "Escape") { @@ -288,7 +288,7 @@ export class CalciteMenuItem implements LoadableComponent, T9nComponent, Localiz } else if (event.key === "ArrowDown" || event.key === "ArrowUp") { event.preventDefault(); if ( - (event.target === this.dropdownActionEl || !this.href) && + (targetIsDropdown || !this.href) && this.hasSubmenu && !this.open && this.layout === "horizontal" @@ -311,7 +311,7 @@ export class CalciteMenuItem implements LoadableComponent, T9nComponent, Localiz } else if (event.key === "ArrowRight") { event.preventDefault(); if ( - (event.target === this.dropdownActionEl || !this.href) && + (targetIsDropdown || !this.href) && this.hasSubmenu && !this.open && this.layout === "vertical" diff --git a/packages/calcite-components/src/demos/menu.html b/packages/calcite-components/src/demos/menu.html index 0f17c1c94c6..b5c523e4639 100644 --- a/packages/calcite-components/src/demos/menu.html +++ b/packages/calcite-components/src/demos/menu.html @@ -440,4 +440,11 @@

+ From 79d20575c757e9466c4517bb28469078e0423784 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 13 Dec 2023 14:04:09 -0800 Subject: [PATCH 3/3] Clean up --- .../src/components/menu-item/menu-item.tsx | 47 ++++++++----------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/packages/calcite-components/src/components/menu-item/menu-item.tsx b/packages/calcite-components/src/components/menu-item/menu-item.tsx index b3289737e74..fe7c342d585 100644 --- a/packages/calcite-components/src/components/menu-item/menu-item.tsx +++ b/packages/calcite-components/src/components/menu-item/menu-item.tsx @@ -266,63 +266,54 @@ export class CalciteMenuItem implements LoadableComponent, T9nComponent, Localiz }; private keyDownHandler = async (event: KeyboardEvent): Promise => { - // opening and closing of submenu is handled here. Any other functionality is bubbled to parent menu. + const { hasSubmenu, href, layout, open, submenuItems } = this; + const key = event.key; const targetIsDropdown = event.target === this.dropdownActionEl; - if (event.key === " " || event.key === "Enter") { - if (this.hasSubmenu && (!this.href || (this.href && targetIsDropdown))) { - this.open = !this.open; + if (key === " " || key === "Enter") { + if (hasSubmenu && (!href || (href && targetIsDropdown))) { + this.open = !open; } - if (!(this.href && targetIsDropdown) && event.key !== "Enter") { + if (!(href && targetIsDropdown) && key !== "Enter") { this.selectMenuItem(event); } - if (event.key === " " || (this.href && targetIsDropdown)) { + if (key === " " || (href && targetIsDropdown)) { event.preventDefault(); } - } else if (event.key === "Escape") { - if (this.open) { + } else if (key === "Escape") { + if (open) { this.open = false; return; } this.calciteInternalMenuItemKeyEvent.emit({ event }); event.preventDefault(); - } else if (event.key === "ArrowDown" || event.key === "ArrowUp") { + } else if (key === "ArrowDown" || key === "ArrowUp") { event.preventDefault(); - if ( - (targetIsDropdown || !this.href) && - this.hasSubmenu && - !this.open && - this.layout === "horizontal" - ) { + if ((targetIsDropdown || !href) && hasSubmenu && !open && layout === "horizontal") { this.open = true; return; } this.calciteInternalMenuItemKeyEvent.emit({ event, - children: this.submenuItems, - isSubmenuOpen: this.open && this.hasSubmenu, + children: submenuItems, + isSubmenuOpen: open && hasSubmenu, }); - } else if (event.key === "ArrowLeft") { + } else if (key === "ArrowLeft") { event.preventDefault(); this.calciteInternalMenuItemKeyEvent.emit({ event, - children: this.submenuItems, + children: submenuItems, isSubmenuOpen: true, }); - } else if (event.key === "ArrowRight") { + } else if (key === "ArrowRight") { event.preventDefault(); - if ( - (targetIsDropdown || !this.href) && - this.hasSubmenu && - !this.open && - this.layout === "vertical" - ) { + if ((targetIsDropdown || !href) && hasSubmenu && !open && layout === "vertical") { this.open = true; return; } this.calciteInternalMenuItemKeyEvent.emit({ event, - children: this.submenuItems, - isSubmenuOpen: this.open && this.hasSubmenu, + children: submenuItems, + isSubmenuOpen: open && hasSubmenu, }); } };