From e4ab16218e563607dbb62ba219f36a4088849c58 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 20 Jul 2026 14:10:47 -0700 Subject: [PATCH 1/2] refactor: drop custom keyboard events --- .../src/components/card-group/card-group.tsx | 69 ++++++++++------- .../components/src/components/card/card.tsx | 13 ---- .../chip-group/chip-group.browser.e2e.tsx | 24 +++++- .../src/components/chip-group/chip-group.tsx | 39 ++++++---- .../components/src/components/chip/chip.tsx | 4 - .../src/components/menu-item/interfaces.ts | 8 -- .../src/components/menu-item/menu-item.tsx | 26 ++----- .../components/src/components/menu/menu.tsx | 73 ++++++++++++------ .../components/stepper-item/stepper-item.tsx | 7 -- .../src/components/stepper/interfaces.ts | 4 - .../stepper/stepper.browser.e2e.tsx | 74 ++++++++++++++++++- .../src/components/stepper/stepper.tsx | 72 +++++++++++------- .../swatch-group/swatch-group.e2e.ts | 24 ++++++ .../components/swatch-group/swatch-group.tsx | 55 +++++++++----- .../src/components/swatch/swatch.tsx | 4 - .../src/components/tile-group/tile-group.tsx | 63 ++++++++++------ .../components/src/components/tile/tile.tsx | 12 --- 17 files changed, 359 insertions(+), 212 deletions(-) diff --git a/packages/components/src/components/card-group/card-group.tsx b/packages/components/src/components/card-group/card-group.tsx index e89da72507a..d637f318781 100644 --- a/packages/components/src/components/card-group/card-group.tsx +++ b/packages/components/src/components/card-group/card-group.tsx @@ -1,6 +1,6 @@ import { PropertyValues } from "lit"; import { createRef } from "lit/directives/ref.js"; -import { LitElement, property, createEvent, h, method, JsxNode, ToEvents } from "@arcgis/lumina"; +import { LitElement, property, createEvent, h, method, JsxNode } from "@arcgis/lumina"; import { focusElementInGroup } from "../../utils/dom"; import { Scale, SelectionMode } from "../interfaces"; import type { Card } from "../card/card"; @@ -92,10 +92,7 @@ export class CardGroup extends LitElement { constructor() { super(); - this.listen["calciteInternalCardKeyEvent"]>( - "calciteInternalCardKeyEvent", - this.calciteInternalCardKeyEventListener, - ); + this.listen("keydown", this.keyDownHandler); this.listen("calciteCardSelect", this.calciteCardSelectListener); } @@ -121,29 +118,45 @@ export class CardGroup extends LitElement { //#region Private Methods - private calciteInternalCardKeyEventListener(event: CustomEvent): void { - if (event.composedPath().includes(this.el)) { - const interactiveItems = this.items.filter((el) => !el.disabled); - switch (event.detail["key"]) { - case "ArrowRight": - focusElementInGroup(interactiveItems, event.target as Card["el"], "next", true, false); - break; - case "ArrowLeft": - focusElementInGroup( - interactiveItems, - event.target as Card["el"], - "previous", - true, - false, - ); - break; - case "Home": - focusElementInGroup(interactiveItems, event.target as Card["el"], "first", true, false); - break; - case "End": - focusElementInGroup(interactiveItems, event.target as Card["el"], "last", true, false); - break; - } + private keyDownHandler(event: KeyboardEvent): void { + if (this.disabled || !event.composedPath().includes(this.el)) { + return; + } + + const composedPath = event.composedPath(); + const card = composedPath.find( + (node): node is Card["el"] => node instanceof HTMLElement && node.matches("calcite-card"), + ); + const target = composedPath[0]; + + if ( + !card || + !this.items.includes(card) || + card.disabled || + card.selectable || + !card.shadowRoot?.contains(target as Node) + ) { + return; + } + + const interactiveItems = this.items.filter((el) => !el.disabled); + switch (event.key) { + case "ArrowRight": + focusElementInGroup(interactiveItems, card, "next", true, false); + event.preventDefault(); + break; + case "ArrowLeft": + focusElementInGroup(interactiveItems, card, "previous", true, false); + event.preventDefault(); + break; + case "Home": + focusElementInGroup(interactiveItems, card, "first", true, false); + event.preventDefault(); + break; + case "End": + focusElementInGroup(interactiveItems, card, "last", true, false); + event.preventDefault(); + break; } } diff --git a/packages/components/src/components/card/card.tsx b/packages/components/src/components/card/card.tsx index 6307b782bfa..ade643b2fb7 100644 --- a/packages/components/src/components/card/card.tsx +++ b/packages/components/src/components/card/card.tsx @@ -145,9 +145,6 @@ export class Card extends LitElement { /** Fires when the deprecated `selectable` is true, or `selectionMode` set on parent `calcite-card-group` is not `none` and the component is selected. */ calciteCardSelect = createEvent({ cancelable: false }); - /** @private */ - calciteInternalCardKeyEvent = createEvent({ cancelable: false }); - //#endregion //#region Private Methods @@ -181,16 +178,6 @@ export class Card extends LitElement { if (isActivationKey(event.key) && this.selectionMode !== "none") { this.calciteCardSelect.emit(); event.preventDefault(); - } else { - switch (event.key) { - case "ArrowRight": - case "ArrowLeft": - case "Home": - case "End": - this.calciteInternalCardKeyEvent.emit(event); - event.preventDefault(); - break; - } } } } diff --git a/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx b/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx index 3446a6dd2c6..41d745d4cb9 100644 --- a/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx +++ b/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx @@ -1,6 +1,7 @@ import { h } from "@arcgis/lumina"; -import { describe } from "vitest"; +import { describe, expect, it } from "vitest"; import { mount } from "@arcgis/lumina-compiler/testing"; +import { page, userEvent } from "vitest/browser"; import { disabled, focusable, hidden, renders, accessible } from "../../tests/commonTests/browser"; describe("accessible", () => { @@ -82,6 +83,27 @@ describe("focusable", () => { ); }); +describe("keyboard navigation", () => { + it("moves focus between chips with arrow keys", async () => { + await mount( + + + + + , + ); + + await userEvent.click(page.getBySelector("#chip-1")); + expect(document.activeElement?.id).toBe("chip-1"); + + await userEvent.keyboard("{ArrowRight}"); + expect(document.activeElement?.id).toBe("chip-2"); + + await userEvent.keyboard("{ArrowLeft}"); + expect(document.activeElement?.id).toBe("chip-1"); + }); +}); + describe("disabled", () => { disabled( () => diff --git a/packages/components/src/components/chip-group/chip-group.tsx b/packages/components/src/components/chip-group/chip-group.tsx index 8c7296586d8..2049910a8ce 100644 --- a/packages/components/src/components/chip-group/chip-group.tsx +++ b/packages/components/src/components/chip-group/chip-group.tsx @@ -104,7 +104,7 @@ export class ChipGroup extends LitElement { constructor() { super(); - this.listen("calciteInternalChipKeyEvent", this.calciteInternalChipKeyEventListener); + this.listen("keydown", this.keyDownHandler); this.listen("calciteChipClose", this.calciteChipCloseListener); this.listen("calciteChipSelect", this.calciteChipSelectListener); this.listen("calciteInternalChipSelect", this.calciteInternalChipSelectListener); @@ -125,22 +125,29 @@ export class ChipGroup extends LitElement { //#region Private Methods - private calciteInternalChipKeyEventListener(event: CustomEvent): void { - if (event.composedPath().includes(this.el)) { - const destinationFromKey: Record = { - ArrowRight: "next", - ArrowLeft: "previous", - Home: "first", - End: "last", - }; - const destination = destinationFromKey[event.detail.key]; - - if (destination) { - const interactiveItems = this.items?.filter((el) => !el.disabled); - focusElementInGroup(interactiveItems, event.detail.target, destination, true, true, true); - } + private keyDownHandler(event: KeyboardEvent): void { + const destinationFromKey: Record = { + ArrowRight: "next", + ArrowLeft: "previous", + Home: "first", + End: "last", + }; + const destination = destinationFromKey[event.key]; + + if (!destination) { + return; } - event.stopPropagation(); + + const chip = event + .composedPath() + .find((el): el is Chip["el"] => el instanceof HTMLElement && el.matches("calcite-chip")); + + if (!chip || !this.items?.includes(chip)) { + return; + } + + const interactiveItems = this.items?.filter((el) => !el.disabled); + focusElementInGroup(interactiveItems, chip, destination, true, true, true); } private calciteChipCloseListener(event: CustomEvent): void { diff --git a/packages/components/src/components/chip/chip.tsx b/packages/components/src/components/chip/chip.tsx index 7bb175c7b02..553278d62c2 100644 --- a/packages/components/src/components/chip/chip.tsx +++ b/packages/components/src/components/chip/chip.tsx @@ -166,9 +166,6 @@ export class Chip extends LitElement { /** Fires when the selected state of the component changes. */ calciteChipSelect = createEvent({ cancelable: false }); - /** @private */ - calciteInternalChipKeyEvent = createEvent({ cancelable: false }); - /** @private */ calciteInternalChipSelect = createEvent({ cancelable: false }); @@ -237,7 +234,6 @@ export class Chip extends LitElement { case "ArrowLeft": case "Home": case "End": - this.calciteInternalChipKeyEvent.emit(event); event.preventDefault(); break; } diff --git a/packages/components/src/components/menu-item/interfaces.ts b/packages/components/src/components/menu-item/interfaces.ts index 8c862aa3064..fb42dddf2ef 100644 --- a/packages/components/src/components/menu-item/interfaces.ts +++ b/packages/components/src/components/menu-item/interfaces.ts @@ -1,9 +1 @@ -import type { MenuItem } from "./menu-item"; - -export interface MenuItemCustomEvent { - event: KeyboardEvent; - children?: MenuItem["el"][]; - isSubmenuOpen?: boolean; -} - export type Layout = "horizontal" | "vertical"; diff --git a/packages/components/src/components/menu-item/menu-item.tsx b/packages/components/src/components/menu-item/menu-item.tsx index d852d73a015..d1210bbe9c4 100644 --- a/packages/components/src/components/menu-item/menu-item.tsx +++ b/packages/components/src/components/menu-item/menu-item.tsx @@ -18,7 +18,6 @@ import { useT9n } from "../../controllers/useT9n"; import type { Action } from "../action/action"; import { useSetFocus } from "../../controllers/useSetFocus"; import { CSS, SLOTS, ICONS } from "./resources"; -import { MenuItemCustomEvent } from "./interfaces"; import T9nStrings from "./assets/t9n/messages.en.json"; import { styles } from "./menu-item.scss"; @@ -144,9 +143,6 @@ export class MenuItem extends LitElement { //#region Events - /** @private */ - calciteInternalMenuItemKeyEvent = createEvent(); - /** Emits when the component is selected. */ calciteMenuItemSelect = createEvent(); @@ -219,7 +215,7 @@ export class MenuItem extends LitElement { } private async keyDownHandler(event: KeyboardEvent): Promise { - const { hasSubmenu, href, layout, open, submenuItems } = this; + const { hasSubmenu, href, layout, open } = this; const key = event.key; const targetIsDropdown = event.target === this.dropdownActionRef.value; @@ -230,6 +226,7 @@ export class MenuItem extends LitElement { if (key === " " || key === "Enter") { if (hasSubmenu && (!href || (href && targetIsDropdown))) { this.open = !open; + event.stopPropagation(); } if (!(href && targetIsDropdown) && key !== "Enter") { this.selectMenuItem(event); @@ -240,39 +237,26 @@ export class MenuItem extends LitElement { } else if (key === "Escape") { if (open) { this.open = false; + event.stopPropagation(); return; } - this.calciteInternalMenuItemKeyEvent.emit({ event }); event.preventDefault(); } else if (key === "ArrowDown" || key === "ArrowUp") { event.preventDefault(); if ((targetIsDropdown || !href) && hasSubmenu && !open && layout === "horizontal") { this.open = true; + event.stopPropagation(); return; } - this.calciteInternalMenuItemKeyEvent.emit({ - event, - children: submenuItems, - isSubmenuOpen: open && hasSubmenu, - }); } else if (key === "ArrowLeft") { event.preventDefault(); - this.calciteInternalMenuItemKeyEvent.emit({ - event, - children: submenuItems, - isSubmenuOpen: true, - }); } else if (key === "ArrowRight") { event.preventDefault(); if ((targetIsDropdown || !href) && hasSubmenu && !open && layout === "vertical") { this.open = true; + event.stopPropagation(); return; } - this.calciteInternalMenuItemKeyEvent.emit({ - event, - children: submenuItems, - isSubmenuOpen: open && hasSubmenu, - }); } } diff --git a/packages/components/src/components/menu/menu.tsx b/packages/components/src/components/menu/menu.tsx index 8edb364cd47..d4d8dbeff9a 100644 --- a/packages/components/src/components/menu/menu.tsx +++ b/packages/components/src/components/menu/menu.tsx @@ -79,7 +79,7 @@ export class Menu extends LitElement { constructor() { super(); - this.listen("calciteInternalMenuItemKeyEvent", this.calciteInternalNavMenuItemKeyEvent); + this.listen("keydown", this.calciteInternalNavMenuItemKeyEvent); } override willUpdate(changes: PropertyValues): void { @@ -101,50 +101,75 @@ export class Menu extends LitElement { this.setMenuItemLayout(this.menuItems, this.layout); } - private calciteInternalNavMenuItemKeyEvent(event: CustomEvent): void { - const target = event.target as MenuItem["el"]; - const submenuItems = event.detail.children; - const key = event.detail.event.key; - event.stopPropagation(); + private calciteInternalNavMenuItemKeyEvent(event: KeyboardEvent): void { + const target = this.getMenuItemFromEvent(event); + + if (!target) { + return; + } + + const submenuItems = this.getSubmenuItems(target); + const hasSubmenu = !!submenuItems?.length; + const key = event.key; if (key === "ArrowDown") { + event.stopPropagation(); if (target.layout === "vertical") { focusElementInGroup(this.menuItems, target, "next", false, false); - } else { - if (event.detail.isSubmenuOpen) { - submenuItems[0].setFocus(); - } + } else if (target.open && hasSubmenu) { + submenuItems?.[0]?.setFocus(); } } else if (key === "ArrowUp") { - if (this.layout === "vertical") { + event.stopPropagation(); + if (target.layout === "vertical") { focusElementInGroup(this.menuItems, target, "previous", false, false); - } else { - if (event.detail.isSubmenuOpen) { - submenuItems[submenuItems.length - 1].setFocus(); - } + } else if (target.open && hasSubmenu) { + const lastSubmenuItem = submenuItems?.[submenuItems.length - 1]; + lastSubmenuItem?.setFocus(); } } else if (key === "ArrowRight") { + event.stopPropagation(); if (this.layout === "horizontal") { focusElementInGroup(this.menuItems, target, "next", false, false); - } else { - if (event.detail.isSubmenuOpen) { - submenuItems[0].setFocus(); - } + } else if (target.open && hasSubmenu) { + submenuItems?.[0]?.setFocus(); } } else if (key === "ArrowLeft") { + event.stopPropagation(); if (this.layout === "horizontal") { focusElementInGroup(this.menuItems, target, "previous", false, false); - } else { - if (event.detail.isSubmenuOpen) { - this.focusParentElement(event.target as MenuItem["el"]); - } + } else if (target.parentElement?.tagName === "CALCITE-MENU-ITEM") { + this.focusParentElement(target); } } else if (key === "Escape") { - this.focusParentElement(event.target as MenuItem["el"]); + event.stopPropagation(); + this.focusParentElement(target); + } else { + return; } event.preventDefault(); } + private getMenuItemFromEvent(event: KeyboardEvent): MenuItem["el"] | undefined { + const target = event + .composedPath() + .find( + (node): node is MenuItem["el"] => + node instanceof HTMLElement && node.tagName === "CALCITE-MENU-ITEM", + ); + + return target && this.menuItems.includes(target) ? target : undefined; + } + + private getSubmenuItems(menuItem: MenuItem["el"]): MenuItem["el"][] | undefined { + return ( + menuItem.submenuItems ?? + (menuItem.shadowRoot + ?.querySelector('slot[name="submenu-item"]') + ?.assignedElements({ flatten: true }) as MenuItem["el"][] | undefined) + ); + } + private handleMenuSlotChange(event: Event): void { this.menuItems = slotChangeGetAssignedElements(event); this.setMenuItemLayout(this.menuItems, this.layout); diff --git a/packages/components/src/components/stepper-item/stepper-item.tsx b/packages/components/src/components/stepper-item/stepper-item.tsx index 89fede6dc66..2e0db436243 100644 --- a/packages/components/src/components/stepper-item/stepper-item.tsx +++ b/packages/components/src/components/stepper-item/stepper-item.tsx @@ -14,7 +14,6 @@ import { Scale } from "../interfaces"; import { StepperItemChangeEventDetail, StepperItemEventDetail, - StepperItemKeyEventDetail, StepperLayout, } from "../stepper/interfaces"; import { NumberingSystem, numberStringFormatter } from "../../utils/locale"; @@ -162,11 +161,6 @@ export class StepperItem extends LitElement { //#region Events - /** @private */ - calciteInternalStepperItemKeyEvent = createEvent({ - cancelable: false, - }); - /** @private */ calciteInternalStepperItemUpdate = createEvent({ cancelable: false }); @@ -265,7 +259,6 @@ export class StepperItem extends LitElement { case "ArrowRight": case "Home": case "End": - this.calciteInternalStepperItemKeyEvent.emit({ item: event }); event.preventDefault(); break; } diff --git a/packages/components/src/components/stepper/interfaces.ts b/packages/components/src/components/stepper/interfaces.ts index 83051ad73d4..35472ec1305 100644 --- a/packages/components/src/components/stepper/interfaces.ts +++ b/packages/components/src/components/stepper/interfaces.ts @@ -2,10 +2,6 @@ export interface StepperItemEventDetail { position: number; } -export interface StepperItemKeyEventDetail { - item: KeyboardEvent; -} - export interface StepperItemChangeEventDetail { position: number; } diff --git a/packages/components/src/components/stepper/stepper.browser.e2e.tsx b/packages/components/src/components/stepper/stepper.browser.e2e.tsx index 7d419c43520..ce59b024e2d 100644 --- a/packages/components/src/components/stepper/stepper.browser.e2e.tsx +++ b/packages/components/src/components/stepper/stepper.browser.e2e.tsx @@ -1,7 +1,7 @@ import { h, JsxNode } from "@arcgis/lumina"; import { describe, expect, it } from "vitest"; import { mount } from "@arcgis/lumina-compiler/testing"; -import { page } from "vitest/browser"; +import { page, userEvent } from "vitest/browser"; import { LitElement } from "@arcgis/lumina"; import { defaults, reflects, hidden, renders, t9n, themed } from "../../tests/commonTests/browser"; import { CSS as STEPPER_ITEM_CSS } from "../stepper-item/resources"; @@ -171,6 +171,78 @@ describe("inheritable props in shadow DOM", () => { }); }); +describe("keyboard navigation", () => { + function focusFirstItem(layout: Stepper["layout"]): void { + const item = document.getElementById("step-1") as HTMLElement & { shadowRoot?: ShadowRoot }; + + if (layout === "horizontal") { + (item.shadowRoot.querySelector(".stepper-item-header") as HTMLElement).focus(); + return; + } + + item.focus(); + } + + function isFocusedStepperItem(layout: Stepper["layout"], id: string): boolean { + const item = document.getElementById(id) as HTMLElement & { shadowRoot?: ShadowRoot }; + + return layout === "horizontal" + ? (item.shadowRoot?.activeElement?.classList.contains("stepper-item-header") ?? false) + : document.activeElement === item; + } + + function isFocusedStepperContentButton(): boolean { + return document.activeElement?.id === "step-1-button"; + } + + it.each([ + { + key: "ArrowRight", + layout: "horizontal" as const, + }, + { + key: "ArrowDown", + layout: "vertical" as const, + }, + ])("delegates %s keydown navigation from stepper items", async ({ key, layout }) => { + await mount( + + + + + +
Step 2 content
+ + +
Step 3 content
+ + , + ); + + await expect.element(page.getBySelector("#step-1")).toHaveAttribute("selected"); + await expect.element(page.getBySelector("#step-3")).not.toHaveAttribute("selected"); + + if (layout === "horizontal") { + (document.getElementById("step-1-button") as HTMLElement).focus(); + + await userEvent.keyboard(`{${key}}`); + + // eslint-disable-next-line vitest/no-conditional-expect -- assertion depends on test config + expect(await isFocusedStepperContentButton()).toBe(true); + } + + await focusFirstItem(layout); + await userEvent.keyboard(`{${key}}`); + + await expect.element(page.getBySelector("#step-1")).toHaveAttribute("selected"); + await expect.element(page.getBySelector("#step-2")).not.toHaveAttribute("selected"); + await expect.element(page.getBySelector("#step-3")).not.toHaveAttribute("selected"); + expect(await isFocusedStepperItem(layout, "step-3")).toBe(true); + }); +}); + describe("theme", () => { describe("horizontal-single", () => { themed( diff --git a/packages/components/src/components/stepper/stepper.tsx b/packages/components/src/components/stepper/stepper.tsx index 2229d33bca0..a1a1c225f33 100644 --- a/packages/components/src/components/stepper/stepper.tsx +++ b/packages/components/src/components/stepper/stepper.tsx @@ -1,14 +1,5 @@ import { PropertyValues } from "lit"; -import { - createEvent, - h, - JsxNode, - LitElement, - method, - property, - state, - ToEvents, -} from "@arcgis/lumina"; +import { createEvent, h, JsxNode, LitElement, method, property, state } from "@arcgis/lumina"; import { createRef } from "lit/directives/ref.js"; import { focusElementInGroup, slotChangeGetAssignedElements } from "../../utils/dom"; import { Position, Scale } from "../interfaces"; @@ -20,11 +11,7 @@ import type { StepperItem } from "../stepper-item/stepper-item"; import type { Action } from "../action/action"; import { CSS, ICONS, IDS } from "./resources"; import { StepBar } from "./functional/step-bar"; -import { - StepperItemChangeEventDetail, - StepperItemKeyEventDetail, - StepperLayout, -} from "./interfaces"; +import { StepperItemChangeEventDetail, StepperLayout } from "./interfaces"; import T9nStrings from "./assets/t9n/messages.en.json"; import { styles } from "./stepper.scss"; import { isStepperItem } from "../stepper-item/resources"; @@ -190,10 +177,7 @@ export class Stepper extends LitElement { constructor() { super(); - this.listen["calciteInternalStepperItemKeyEvent"]>( - "calciteInternalStepperItemKeyEvent", - this.calciteInternalStepperItemKeyEvent, - ); + this.listen("keydown", this.keyDownHandler); this.listen("calciteInternalStepperItemUpdate", (event: Event): void => { event.stopPropagation(); this.updateItems(); @@ -249,27 +233,59 @@ export class Stepper extends LitElement { //#region Private Methods - private calciteInternalStepperItemKeyEvent(event: CustomEvent): void { - const item = event.detail.item; - const itemToFocus = event.target as StepperItem["el"]; + private keyDownHandler(event: KeyboardEvent): void { + if (!event.composedPath().includes(this.el)) { + return; + } - switch (item.key) { + const item = this.getStepperItemFromKeyboardEvent(event); + + if (!item || item.disabled) { + return; + } + + switch (event.key) { case "ArrowDown": case "ArrowRight": - focusElementInGroup(this.focusableItems, itemToFocus, "next"); + focusElementInGroup(this.focusableItems, item, "next"); + event.preventDefault(); break; case "ArrowUp": case "ArrowLeft": - focusElementInGroup(this.focusableItems, itemToFocus, "previous"); + focusElementInGroup(this.focusableItems, item, "previous"); + event.preventDefault(); break; case "Home": - focusElementInGroup(this.focusableItems, itemToFocus, "first"); + focusElementInGroup(this.focusableItems, item, "first"); + event.preventDefault(); break; case "End": - focusElementInGroup(this.focusableItems, itemToFocus, "last"); + focusElementInGroup(this.focusableItems, item, "last"); + event.preventDefault(); break; } - event.stopPropagation(); + } + + private getStepperItemFromKeyboardEvent(event: KeyboardEvent): StepperItem["el"] | undefined { + const composedPath = event.composedPath(); + const origin = composedPath[0]; + const item = composedPath.find( + (el): el is StepperItem["el"] => el instanceof Element && isStepperItem(el), + ); + + if (!item) { + return; + } + + if (origin === item) { + return item; + } + + if (origin instanceof Node && item.shadowRoot?.contains(origin)) { + return item; + } + + return; } private updateItem(event: CustomEvent): void { diff --git a/packages/components/src/components/swatch-group/swatch-group.e2e.ts b/packages/components/src/components/swatch-group/swatch-group.e2e.ts index f947f4c3b54..2dd2fc798c6 100644 --- a/packages/components/src/components/swatch-group/swatch-group.e2e.ts +++ b/packages/components/src/components/swatch-group/swatch-group.e2e.ts @@ -326,6 +326,30 @@ describe("focus and interaction function as intended", () => { expect(await page.evaluate(() => document.activeElement!.id)).toEqual(swatch1.id); }); + it("navigation skips disabled swatches", async () => { + const page = await newE2EPage(); + await page.setContent( + html` + + + + `, + ); + + const element = await page.find("calcite-swatch-group"); + const swatch1 = await page.find("#swatch-1"); + const swatch3 = await page.find("#swatch-3"); + + await swatch1.click(); + await page.waitForChanges(); + + await page.keyboard.press("ArrowRight"); + await page.waitForChanges(); + + expect(await page.evaluate(() => document.activeElement!.id)).toEqual(swatch3.id); + expect(await element.getProperty("selectedItems")).toHaveLength(1); + }); + it("selectedItems property is correctly populated at load when property is set on swatches in DOM", async () => { const page = await newE2EPage(); await page.setContent( diff --git a/packages/components/src/components/swatch-group/swatch-group.tsx b/packages/components/src/components/swatch-group/swatch-group.tsx index 117ce4aa4a8..f812366dcde 100644 --- a/packages/components/src/components/swatch-group/swatch-group.tsx +++ b/packages/components/src/components/swatch-group/swatch-group.tsx @@ -99,7 +99,7 @@ export class SwatchGroup extends LitElement { constructor() { super(); - this.listen("calciteInternalSwatchKeyEvent", this.calciteInternalSwatchKeyEventListener); + this.listen("keydown", this.keyDownHandler); this.listen("calciteSwatchSelect", this.calciteSwatchSelectListener); this.listen("calciteInternalSwatchSelect", this.calciteInternalSwatchSelectListener); this.listen("calciteInternalSyncSelectedSwatches", this.calciteInternalSyncSelectedSwatches); @@ -115,25 +115,42 @@ export class SwatchGroup extends LitElement { //#region Private Methods - private calciteInternalSwatchKeyEventListener(event: CustomEvent): void { - if (event.composedPath().includes(this.el)) { - const interactiveItems = this.items?.filter((el) => !el.disabled); - switch (event.detail.key) { - case "ArrowRight": - focusElementInGroup(interactiveItems, event.detail.target, "next"); - break; - case "ArrowLeft": - focusElementInGroup(interactiveItems, event.detail.target, "previous"); - break; - case "Home": - focusElementInGroup(interactiveItems, event.detail.target, "first"); - break; - case "End": - focusElementInGroup(interactiveItems, event.detail.target, "last"); - break; - } + private keyDownHandler(event: KeyboardEvent): void { + const target = event + .composedPath() + .find( + (node): node is Swatch["el"] => + node instanceof HTMLElement && node.matches("calcite-swatch"), + ); + + if (!target || !this.el.contains(target)) { + return; + } + + const interactiveItems = this.items?.filter((el) => !el.disabled); + + if (!interactiveItems.includes(target)) { + return; + } + + switch (event.key) { + case "ArrowRight": + focusElementInGroup(interactiveItems, target, "next"); + event.preventDefault(); + break; + case "ArrowLeft": + focusElementInGroup(interactiveItems, target, "previous"); + event.preventDefault(); + break; + case "Home": + focusElementInGroup(interactiveItems, target, "first"); + event.preventDefault(); + break; + case "End": + focusElementInGroup(interactiveItems, target, "last"); + event.preventDefault(); + break; } - event.stopPropagation(); } private calciteSwatchSelectListener(event: CustomEvent): void { diff --git a/packages/components/src/components/swatch/swatch.tsx b/packages/components/src/components/swatch/swatch.tsx index c537e75fb94..c8ded93dbea 100644 --- a/packages/components/src/components/swatch/swatch.tsx +++ b/packages/components/src/components/swatch/swatch.tsx @@ -122,9 +122,6 @@ export class Swatch extends LitElement { //#region Events - /** @private */ - calciteInternalSwatchKeyEvent = createEvent({ cancelable: false }); - /** @private */ calciteInternalSwatchSelect = createEvent({ cancelable: false }); @@ -185,7 +182,6 @@ export class Swatch extends LitElement { case "ArrowLeft": case "Home": case "End": - this.calciteInternalSwatchKeyEvent.emit(event); event.preventDefault(); break; } diff --git a/packages/components/src/components/tile-group/tile-group.tsx b/packages/components/src/components/tile-group/tile-group.tsx index f90d274a999..5b122363f8d 100644 --- a/packages/components/src/components/tile-group/tile-group.tsx +++ b/packages/components/src/components/tile-group/tile-group.tsx @@ -105,7 +105,7 @@ export class TileGroup extends LitElement implements SelectableGroupComponent { constructor() { super(); - this.listen("calciteInternalTileKeyEvent", this.calciteInternalTileKeyEventListener); + this.listen("keydown", this.keyDownHandler); this.listen("calciteTileSelect", this.calciteTileSelectHandler); } @@ -207,27 +207,46 @@ export class TileGroup extends LitElement implements SelectableGroupComponent { this.updateSelectedItems(); } - private calciteInternalTileKeyEventListener(event: CustomEvent): void { - if (event.composedPath().includes(this.el)) { - event.preventDefault(); - event.stopPropagation(); - const interactiveItems = this.items?.filter((el) => !el.disabled); - switch (event.detail.key) { - case "ArrowDown": - case "ArrowRight": - focusElementInGroup(interactiveItems, event.detail.target, "next", true, false); - break; - case "ArrowUp": - case "ArrowLeft": - focusElementInGroup(interactiveItems, event.detail.target, "previous", true, false); - break; - case "Home": - focusElementInGroup(interactiveItems, event.detail.target, "first", true, false); - break; - case "End": - focusElementInGroup(interactiveItems, event.detail.target, "last", true, false); - break; - } + private keyDownHandler(event: KeyboardEvent): void { + const composedPath = event.composedPath(); + if (this.disabled || !composedPath.includes(this.el)) { + return; + } + + const originalTarget = composedPath[0] as Node; + const target = composedPath.find( + (node): node is Tile["el"] => node instanceof HTMLElement && node.matches("calcite-tile"), + ); + + if ( + !target || + !this.items.includes(target) || + target.disabled || + !target.shadowRoot?.contains(originalTarget) + ) { + return; + } + + const interactiveItems = this.items?.filter((el) => !el.disabled); + switch (event.key) { + case "ArrowDown": + case "ArrowRight": + event.preventDefault(); + focusElementInGroup(interactiveItems, target, "next", true, false); + break; + case "ArrowUp": + case "ArrowLeft": + event.preventDefault(); + focusElementInGroup(interactiveItems, target, "previous", true, false); + break; + case "Home": + event.preventDefault(); + focusElementInGroup(interactiveItems, target, "first", true, false); + break; + case "End": + event.preventDefault(); + focusElementInGroup(interactiveItems, target, "last", true, false); + break; } } diff --git a/packages/components/src/components/tile/tile.tsx b/packages/components/src/components/tile/tile.tsx index bd9f31d8f1b..3f324a2790e 100644 --- a/packages/components/src/components/tile/tile.tsx +++ b/packages/components/src/components/tile/tile.tsx @@ -165,9 +165,6 @@ export class Tile extends LitElement implements SelectableComponent { // #region Events - /** @private */ - calciteInternalTileKeyEvent = createEvent({ cancelable: false }); - /** Fires when the selected state of the component changes. */ calciteTileSelect = createEvent(); @@ -219,15 +216,6 @@ export class Tile extends LitElement implements SelectableComponent { this.handleSelectEvent(); event.preventDefault(); break; - case "ArrowDown": - case "ArrowLeft": - case "ArrowRight": - case "ArrowUp": - case "Home": - case "End": - this.calciteInternalTileKeyEvent.emit(event); - event.preventDefault(); - break; } } } From 3296d7e81eeb02b55c1a7b3542550a7984d5d26b Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 20 Jul 2026 21:28:42 -0700 Subject: [PATCH 2/2] tidy up --- .../src/components/card-group/card-group.tsx | 16 +--- .../chip-group/chip-group.browser.e2e.tsx | 24 +----- .../src/components/menu-item/menu-item.tsx | 12 +-- .../src/components/menu/menu.browser.e2e.tsx | 24 +++++- .../components/src/components/menu/menu.tsx | 46 ++++++----- .../stepper/stepper.browser.e2e.tsx | 77 +++++++------------ .../src/components/stepper/stepper.tsx | 24 +----- .../swatch-group/swatch-group.e2e.ts | 24 ------ .../src/components/tile-group/tile-group.tsx | 12 +-- 9 files changed, 88 insertions(+), 171 deletions(-) diff --git a/packages/components/src/components/card-group/card-group.tsx b/packages/components/src/components/card-group/card-group.tsx index d637f318781..c02b5068e23 100644 --- a/packages/components/src/components/card-group/card-group.tsx +++ b/packages/components/src/components/card-group/card-group.tsx @@ -123,19 +123,9 @@ export class CardGroup extends LitElement { return; } - const composedPath = event.composedPath(); - const card = composedPath.find( - (node): node is Card["el"] => node instanceof HTMLElement && node.matches("calcite-card"), - ); - const target = composedPath[0]; - - if ( - !card || - !this.items.includes(card) || - card.disabled || - card.selectable || - !card.shadowRoot?.contains(target as Node) - ) { + const card = this.items.find((item) => item === event.target); + + if (!card || card.disabled || card.selectable) { return; } diff --git a/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx b/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx index 41d745d4cb9..3446a6dd2c6 100644 --- a/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx +++ b/packages/components/src/components/chip-group/chip-group.browser.e2e.tsx @@ -1,7 +1,6 @@ import { h } from "@arcgis/lumina"; -import { describe, expect, it } from "vitest"; +import { describe } from "vitest"; import { mount } from "@arcgis/lumina-compiler/testing"; -import { page, userEvent } from "vitest/browser"; import { disabled, focusable, hidden, renders, accessible } from "../../tests/commonTests/browser"; describe("accessible", () => { @@ -83,27 +82,6 @@ describe("focusable", () => { ); }); -describe("keyboard navigation", () => { - it("moves focus between chips with arrow keys", async () => { - await mount( - - - - - , - ); - - await userEvent.click(page.getBySelector("#chip-1")); - expect(document.activeElement?.id).toBe("chip-1"); - - await userEvent.keyboard("{ArrowRight}"); - expect(document.activeElement?.id).toBe("chip-2"); - - await userEvent.keyboard("{ArrowLeft}"); - expect(document.activeElement?.id).toBe("chip-1"); - }); -}); - describe("disabled", () => { disabled( () => diff --git a/packages/components/src/components/menu-item/menu-item.tsx b/packages/components/src/components/menu-item/menu-item.tsx index d1210bbe9c4..999a9461b71 100644 --- a/packages/components/src/components/menu-item/menu-item.tsx +++ b/packages/components/src/components/menu-item/menu-item.tsx @@ -226,7 +226,6 @@ export class MenuItem extends LitElement { if (key === " " || key === "Enter") { if (hasSubmenu && (!href || (href && targetIsDropdown))) { this.open = !open; - event.stopPropagation(); } if (!(href && targetIsDropdown) && key !== "Enter") { this.selectMenuItem(event); @@ -237,24 +236,19 @@ export class MenuItem extends LitElement { } else if (key === "Escape") { if (open) { this.open = false; - event.stopPropagation(); + event.preventDefault(); return; } - event.preventDefault(); } else if (key === "ArrowDown" || key === "ArrowUp") { - event.preventDefault(); if ((targetIsDropdown || !href) && hasSubmenu && !open && layout === "horizontal") { this.open = true; - event.stopPropagation(); + event.preventDefault(); return; } - } else if (key === "ArrowLeft") { - event.preventDefault(); } else if (key === "ArrowRight") { - event.preventDefault(); if ((targetIsDropdown || !href) && hasSubmenu && !open && layout === "vertical") { this.open = true; - event.stopPropagation(); + event.preventDefault(); return; } } diff --git a/packages/components/src/components/menu/menu.browser.e2e.tsx b/packages/components/src/components/menu/menu.browser.e2e.tsx index 8100890db11..8965216d78c 100644 --- a/packages/components/src/components/menu/menu.browser.e2e.tsx +++ b/packages/components/src/components/menu/menu.browser.e2e.tsx @@ -1,6 +1,7 @@ import { h } from "@arcgis/lumina"; -import { describe } from "vitest"; +import { describe, expect, it } from "vitest"; import { mount } from "@arcgis/lumina-compiler/testing"; +import { userEvent } from "vitest/browser"; import { focusable, hidden, renders, t9n, accessible } from "../../tests/commonTests/browser"; describe("accessible", () => { @@ -51,6 +52,27 @@ describe("focusable", () => { ); }); +describe("keyboard navigation", () => { + it("bubbles native keydown events and only prevents handled keys", async () => { + const { el } = await mount<"calcite-menu">( + + + , + ); + const item = el.querySelector("calcite-menu-item")!; + const keydownEvents: KeyboardEvent[] = []; + + el.parentElement!.addEventListener("keydown", (event) => keydownEvents.push(event)); + await item.setFocus(); + + await userEvent.keyboard("{ArrowRight}"); + expect(keydownEvents.at(-1)?.defaultPrevented).toBe(true); + + await userEvent.keyboard("a"); + expect(keydownEvents.at(-1)?.defaultPrevented).toBe(false); + }); +}); + describe("translation support", () => { t9n(() => mount("calcite-menu")); }); diff --git a/packages/components/src/components/menu/menu.tsx b/packages/components/src/components/menu/menu.tsx index d4d8dbeff9a..b953ad9ff60 100644 --- a/packages/components/src/components/menu/menu.tsx +++ b/packages/components/src/components/menu/menu.tsx @@ -102,6 +102,10 @@ export class Menu extends LitElement { } private calciteInternalNavMenuItemKeyEvent(event: KeyboardEvent): void { + if (event.defaultPrevented) { + return; + } + const target = this.getMenuItemFromEvent(event); if (!target) { @@ -109,45 +113,50 @@ export class Menu extends LitElement { } const submenuItems = this.getSubmenuItems(target); - const hasSubmenu = !!submenuItems?.length; + const hasSubmenu = submenuItems.length > 0; const key = event.key; + let handled = false; if (key === "ArrowDown") { - event.stopPropagation(); if (target.layout === "vertical") { focusElementInGroup(this.menuItems, target, "next", false, false); + handled = true; } else if (target.open && hasSubmenu) { - submenuItems?.[0]?.setFocus(); + submenuItems[0].setFocus(); + handled = true; } } else if (key === "ArrowUp") { - event.stopPropagation(); if (target.layout === "vertical") { focusElementInGroup(this.menuItems, target, "previous", false, false); + handled = true; } else if (target.open && hasSubmenu) { - const lastSubmenuItem = submenuItems?.[submenuItems.length - 1]; - lastSubmenuItem?.setFocus(); + submenuItems[submenuItems.length - 1].setFocus(); + handled = true; } } else if (key === "ArrowRight") { - event.stopPropagation(); if (this.layout === "horizontal") { focusElementInGroup(this.menuItems, target, "next", false, false); + handled = true; } else if (target.open && hasSubmenu) { - submenuItems?.[0]?.setFocus(); + submenuItems[0].setFocus(); + handled = true; } } else if (key === "ArrowLeft") { - event.stopPropagation(); if (this.layout === "horizontal") { focusElementInGroup(this.menuItems, target, "previous", false, false); + handled = true; } else if (target.parentElement?.tagName === "CALCITE-MENU-ITEM") { this.focusParentElement(target); + handled = true; } - } else if (key === "Escape") { - event.stopPropagation(); + } else if (key === "Escape" && target.parentElement?.tagName === "CALCITE-MENU-ITEM") { this.focusParentElement(target); - } else { - return; + handled = true; + } + + if (handled) { + event.preventDefault(); } - event.preventDefault(); } private getMenuItemFromEvent(event: KeyboardEvent): MenuItem["el"] | undefined { @@ -161,12 +170,9 @@ export class Menu extends LitElement { return target && this.menuItems.includes(target) ? target : undefined; } - private getSubmenuItems(menuItem: MenuItem["el"]): MenuItem["el"][] | undefined { - return ( - menuItem.submenuItems ?? - (menuItem.shadowRoot - ?.querySelector('slot[name="submenu-item"]') - ?.assignedElements({ flatten: true }) as MenuItem["el"][] | undefined) + private getSubmenuItems(menuItem: MenuItem["el"]): MenuItem["el"][] { + return Array.from(menuItem.children).filter((child): child is MenuItem["el"] => + child.matches('calcite-menu-item[slot="submenu-item"]'), ); } diff --git a/packages/components/src/components/stepper/stepper.browser.e2e.tsx b/packages/components/src/components/stepper/stepper.browser.e2e.tsx index ce59b024e2d..03c0d62f008 100644 --- a/packages/components/src/components/stepper/stepper.browser.e2e.tsx +++ b/packages/components/src/components/stepper/stepper.browser.e2e.tsx @@ -172,40 +172,10 @@ describe("inheritable props in shadow DOM", () => { }); describe("keyboard navigation", () => { - function focusFirstItem(layout: Stepper["layout"]): void { - const item = document.getElementById("step-1") as HTMLElement & { shadowRoot?: ShadowRoot }; - - if (layout === "horizontal") { - (item.shadowRoot.querySelector(".stepper-item-header") as HTMLElement).focus(); - return; - } - - item.focus(); - } - - function isFocusedStepperItem(layout: Stepper["layout"], id: string): boolean { - const item = document.getElementById(id) as HTMLElement & { shadowRoot?: ShadowRoot }; - - return layout === "horizontal" - ? (item.shadowRoot?.activeElement?.classList.contains("stepper-item-header") ?? false) - : document.activeElement === item; - } - - function isFocusedStepperContentButton(): boolean { - return document.activeElement?.id === "step-1-button"; - } - - it.each([ - { - key: "ArrowRight", - layout: "horizontal" as const, - }, - { - key: "ArrowDown", - layout: "vertical" as const, - }, - ])("delegates %s keydown navigation from stepper items", async ({ key, layout }) => { - await mount( + async function mountStepper( + layout: Extract, + ): Promise { + await mount<"calcite-stepper">(