From ef17a850b713835a2f2fa2be475812688731148e Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Tue, 28 Nov 2023 16:16:07 -0800 Subject: [PATCH 1/3] fix(list-item): focus on the first focusable element within the component when using arrow keys. #7974 --- .../src/components/list-item/list-item.tsx | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/calcite-components/src/components/list-item/list-item.tsx b/packages/calcite-components/src/components/list-item/list-item.tsx index 961b39793f7..6de32b098f3 100644 --- a/packages/calcite-components/src/components/list-item/list-item.tsx +++ b/packages/calcite-components/src/components/list-item/list-item.tsx @@ -12,7 +12,12 @@ import { VNode, Watch, } from "@stencil/core"; -import { getElementDir, slotChangeHasAssignedElement, toAriaBoolean } from "../../utils/dom"; +import { + focusFirstTabbable, + getElementDir, + slotChangeHasAssignedElement, + toAriaBoolean, +} from "../../utils/dom"; import { connectInteractive, disconnectInteractive, @@ -344,7 +349,7 @@ export class ListItem const focusIndex = focusMap.get(parentListEl); if (typeof focusIndex === "number") { - const cells = [actionsStartEl, contentEl, actionsEndEl].filter(Boolean); + const cells = [actionsStartEl, contentEl, actionsEndEl].filter((el) => el && !el.hidden); if (cells[focusIndex]) { this.focusCell(cells[focusIndex]); } else { @@ -737,7 +742,7 @@ export class ListItem const composedPath = event.composedPath(); const { containerEl, contentEl, actionsStartEl, actionsEndEl, open, openable } = this; - const cells = [actionsStartEl, contentEl, actionsEndEl].filter(Boolean); + const cells = [actionsStartEl, contentEl, actionsEndEl].filter((el) => el && !el.hidden); const currentIndex = cells.findIndex((cell) => composedPath.includes(cell)); if ( @@ -790,16 +795,18 @@ export class ListItem focusMap.set(parentListEl, null); } - [actionsStartEl, contentEl, actionsEndEl].filter(Boolean).forEach((tableCell, cellIndex) => { - const tabIndexAttr = "tabindex"; - if (tableCell === focusEl) { - tableCell.setAttribute(tabIndexAttr, "0"); - saveFocusIndex && focusMap.set(parentListEl, cellIndex); - } else { - tableCell.removeAttribute(tabIndexAttr); - } - }); + [actionsStartEl, contentEl, actionsEndEl] + .filter((el) => el && !el.hidden) + .forEach((tableCell, cellIndex) => { + const tabIndexAttr = "tabindex"; + if (tableCell === focusEl) { + tableCell.setAttribute(tabIndexAttr, "0"); + saveFocusIndex && focusMap.set(parentListEl, cellIndex); + } else { + tableCell.removeAttribute(tabIndexAttr); + } + }); - focusEl?.focus(); + focusFirstTabbable(focusEl); }; } From 780a54d3a66ac7219c022b67d1294f0d53a48530 Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Tue, 28 Nov 2023 17:01:20 -0800 Subject: [PATCH 2/3] cleanup --- .../src/components/list-item/list-item.tsx | 8 +++++--- packages/calcite-components/src/utils/dom.ts | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/calcite-components/src/components/list-item/list-item.tsx b/packages/calcite-components/src/components/list-item/list-item.tsx index 6de32b098f3..49afdd5ed6c 100644 --- a/packages/calcite-components/src/components/list-item/list-item.tsx +++ b/packages/calcite-components/src/components/list-item/list-item.tsx @@ -13,8 +13,8 @@ import { Watch, } from "@stencil/core"; import { - focusFirstTabbable, getElementDir, + getFirstTabbable, slotChangeHasAssignedElement, toAriaBoolean, } from "../../utils/dom"; @@ -795,18 +795,20 @@ export class ListItem focusMap.set(parentListEl, null); } + const focusedEl = getFirstTabbable(focusEl); + [actionsStartEl, contentEl, actionsEndEl] .filter((el) => el && !el.hidden) .forEach((tableCell, cellIndex) => { const tabIndexAttr = "tabindex"; if (tableCell === focusEl) { - tableCell.setAttribute(tabIndexAttr, "0"); + focusEl === focusedEl && tableCell.setAttribute(tabIndexAttr, "0"); saveFocusIndex && focusMap.set(parentListEl, cellIndex); } else { tableCell.removeAttribute(tabIndexAttr); } }); - focusFirstTabbable(focusEl); + focusedEl?.focus(); }; } diff --git a/packages/calcite-components/src/utils/dom.ts b/packages/calcite-components/src/utils/dom.ts index c6e6e987eba..d6ab2dec7e6 100644 --- a/packages/calcite-components/src/utils/dom.ts +++ b/packages/calcite-components/src/utils/dom.ts @@ -285,16 +285,26 @@ export async function focusElement(el: FocusableElement): Promise { } /** - * Helper to focus the first tabbable element. + * Helper to get the first tabbable element. * * @param {HTMLElement} element The html element containing tabbable elements. + * @returns the first tabbable element. */ -export function focusFirstTabbable(element: HTMLElement): void { +export function getFirstTabbable(element: HTMLElement): HTMLElement { if (!element) { return; } - (tabbable(element, tabbableOptions)[0] || element).focus(); + return (tabbable(element, tabbableOptions)[0] ?? element) as HTMLElement; +} + +/** + * Helper to focus the first tabbable element. + * + * @param {HTMLElement} element The html element containing tabbable elements. + */ +export function focusFirstTabbable(element: HTMLElement): void { + getFirstTabbable(element)?.focus(); } interface GetSlottedOptions { From 35f2d39c06944add97c4180f8e740288953763a4 Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Wed, 29 Nov 2023 09:30:22 -0800 Subject: [PATCH 3/3] add test --- .../src/components/list/list.e2e.ts | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/packages/calcite-components/src/components/list/list.e2e.ts b/packages/calcite-components/src/components/list/list.e2e.ts index d875c5bd247..2a581e52dfc 100755 --- a/packages/calcite-components/src/components/list/list.e2e.ts +++ b/packages/calcite-components/src/components/list/list.e2e.ts @@ -504,6 +504,70 @@ describe("calcite-list", () => { expect(await isElementFocused(page, "calcite-filter", { shadowed: true })).toBe(true); }); + + it("should navigate via ArrowRight and ArrowLeft", async () => { + const page = await newE2EPage(); + await page.setContent(html` + + + + + + + + + + `); + await page.waitForChanges(); + const list = await page.find("calcite-list"); + await list.callMethod("setFocus"); + await page.waitForChanges(); + + const one = await page.find("#one"); + expect(await one.getProperty("open")).toBe(false); + + expect(await isElementFocused(page, "#one")).toBe(true); + + await list.press("ArrowRight"); + + expect(await isElementFocused(page, "#one")).toBe(true); + expect(await one.getProperty("open")).toBe(true); + + await list.press("ArrowRight"); + + expect(await isElementFocused(page, `.${CSS.contentContainer}`, { shadowed: true })).toBe(true); + + await list.press("ArrowRight"); + + expect(await isElementFocused(page, "calcite-action")).toBe(true); + + await list.press("ArrowLeft"); + + expect(await isElementFocused(page, `.${CSS.contentContainer}`, { shadowed: true })).toBe(true); + + await list.press("ArrowLeft"); + + expect(await isElementFocused(page, "#one")).toBe(true); + expect(await one.getProperty("open")).toBe(true); + + await list.press("ArrowLeft"); + + expect(await isElementFocused(page, "#one")).toBe(true); + expect(await one.getProperty("open")).toBe(false); + }); }); describe("drag and drop", () => {