From 560ce193422a23c7e48e0d3356a345f450a6dadb Mon Sep 17 00:00:00 2001 From: John Yanarella Date: Wed, 2 Sep 2026 16:21:30 -0700 Subject: [PATCH] fix(core): restore tree keyboard navigation Signed-off-by: John Yanarella --- projects/core/src/index.test.lighthouse.ts | 4 +- .../keynav-list.controller.test.ts | 23 ++++++ .../controllers/keynav-list.controller.ts | 25 +++++-- .../core/src/menu/menu.test.lighthouse.ts | 9 ++- .../pagination/pagination.test.lighthouse.ts | 9 ++- projects/core/src/tree/tree-node.test.ts | 70 +++++++++++++++++++ projects/core/src/tree/tree-node.ts | 14 +++- .../core/src/tree/tree.test.lighthouse.ts | 2 +- projects/core/src/tree/tree.test.ts | 23 ++++++ 9 files changed, 162 insertions(+), 17 deletions(-) diff --git a/projects/core/src/index.test.lighthouse.ts b/projects/core/src/index.test.lighthouse.ts index 84a886dc10..70de06d19d 100644 --- a/projects/core/src/index.test.lighthouse.ts +++ b/projects/core/src/index.test.lighthouse.ts @@ -18,7 +18,7 @@ describe('lighthouse report', () => { expect(report.scores.performance).toBe(100); expect(report.scores.accessibility).toBe(100); expect(report.scores.bestPractices).toBe(100); - expect(report.payload.javascript.requests['index.js'].kb).toBeLessThan(109.02); + expect(report.payload.javascript.requests['index.js'].kb).toBeLessThan(109.05); // if sudden drop in size, check vite bundle config and bundle demo to ensure side effects are properly preserved expect(report.payload.javascript.requests['index.js'].kb).toBeGreaterThan(100); @@ -107,7 +107,7 @@ describe('lighthouse report', () => { expect(report.scores.accessibility).toBe(100); expect(report.scores.bestPractices).toBe(100); expect(report.payload.javascript.requests[Object.keys(report.payload.javascript.requests)[0]].kb).toBeLessThan( - 89.5 + 89.56 ); }); }); diff --git a/projects/core/src/internal/controllers/keynav-list.controller.test.ts b/projects/core/src/internal/controllers/keynav-list.controller.test.ts index d6399d5d2e..0fdc9f44e6 100644 --- a/projects/core/src/internal/controllers/keynav-list.controller.test.ts +++ b/projects/core/src/internal/controllers/keynav-list.controller.test.ts @@ -85,6 +85,18 @@ describe('keynav-list.controller', () => { expect(element.keynavListConfig.items[2].tabIndex).toBe(0); }); + it('should activate an item when a non-focusable descendant is clicked', async () => { + const item = element.keynavListConfig.items[2]!; + const nonFocusableLabel = document.createElement('span'); + item.append(nonFocusableLabel); + nonFocusableLabel.dispatchEvent(new PointerEvent('pointerup', { bubbles: true, composed: true })); + await elementIsStable(element); + + expect(element.keynavListConfig.items[0].tabIndex).toBe(-1); + expect(item.tabIndex).toBe(0); + expect(item.matches(':focus')).toBe(true); + }); + it('should not duplicate listeners after reconnect', async () => { const listener = vi.fn(); element.addEventListener('nve-key-change', listener); @@ -522,4 +534,15 @@ describe('nested interactive keynav-list.controller', () => { expect(element.keynavListConfig.items[2].tabIndex).toBe(-1); expect(element.keynavListConfig.items[3].tabIndex).toBe(-1); }); + + it('should not activate a node when a nested interactive element is clicked', async () => { + const button = element.keynavListConfig.items[1]!.querySelector('button')!; + button.focus(); + button.dispatchEvent(new PointerEvent('pointerup', { bubbles: true, composed: true })); + await elementIsStable(element); + + expect(button.matches(':focus')).toBe(true); + expect(element.keynavListConfig.items[0].tabIndex).toBe(0); + expect(element.keynavListConfig.items[1].tabIndex).toBe(-1); + }); }); diff --git a/projects/core/src/internal/controllers/keynav-list.controller.ts b/projects/core/src/internal/controllers/keynav-list.controller.ts index 88e08a4501..a20425a9f4 100644 --- a/projects/core/src/internal/controllers/keynav-list.controller.ts +++ b/projects/core/src/internal/controllers/keynav-list.controller.ts @@ -3,7 +3,7 @@ import type { ReactiveController, ReactiveElement } from 'lit'; import type { LegacyDecoratorTarget } from '../types/index.js'; -import { focusElement, initializeKeyListItems, setActiveKeyListItem } from '../utils/focus.js'; +import { focusElement, initializeKeyListItems, isFocusable, setActiveKeyListItem } from '../utils/focus.js'; import { KeynavCode, validKeyNavigationCode } from '../utils/dom.js'; export interface KeynavListConfig { @@ -74,7 +74,7 @@ export class KeyNavigationListController i === focusedElement) ? focusedElement : null; + #getKeyboardActiveItem(e: KeyboardEvent, items: HTMLElement[]) { + const target = e.composedPath()[0]; + return target instanceof HTMLElement && items.includes(target) ? target : null; + } + + #getPointerActiveItem(e: PointerEvent, items: HTMLElement[]) { + const path = e.composedPath(); + const itemIndex = path.findIndex(item => item instanceof HTMLElement && items.includes(item)); + const item = path[itemIndex]; + + if (!(item instanceof HTMLElement)) return null; + + const hasFocusableDescendant = path + .slice(0, itemIndex) + .some(descendant => descendant instanceof Element && isFocusable(descendant)); + return hasFocusableDescendant ? null : item; } #setActiveItem(e: KeyboardEvent | PointerEvent, activeItem: HTMLElement, previousItem?: HTMLElement) { diff --git a/projects/core/src/menu/menu.test.lighthouse.ts b/projects/core/src/menu/menu.test.lighthouse.ts index e3677aa32a..0a50e54f72 100644 --- a/projects/core/src/menu/menu.test.lighthouse.ts +++ b/projects/core/src/menu/menu.test.lighthouse.ts @@ -6,7 +6,9 @@ import { lighthouseRunner } from '@internals/vite'; describe('menu lighthouse report', () => { test('menu should meet lighthouse benchmarks', async () => { - const report = await lighthouseRunner.getReport('nve-menu', /* html */` + const report = await lighthouseRunner.getReport( + 'nve-menu', + /* html */ ` item 1 item 2 @@ -16,11 +18,12 @@ describe('menu lighthouse report', () => { - `); + ` + ); expect(report.scores.performance).toBe(100); expect(report.scores.accessibility).toBe(100); expect(report.scores.bestPractices).toBe(100); - expect(report.payload.javascript.kb).toBeLessThan(16.3); + expect(report.payload.javascript.kb).toBeLessThan(16.33); }); }); diff --git a/projects/core/src/pagination/pagination.test.lighthouse.ts b/projects/core/src/pagination/pagination.test.lighthouse.ts index 460c404903..609eb990ad 100644 --- a/projects/core/src/pagination/pagination.test.lighthouse.ts +++ b/projects/core/src/pagination/pagination.test.lighthouse.ts @@ -6,16 +6,19 @@ import { lighthouseRunner } from '@internals/vite'; describe('pagination lighthouse report', () => { test('pagination should meet lighthouse benchmarks', async () => { - const report = await lighthouseRunner.getReport('nve-pagination', /* html */` + const report = await lighthouseRunner.getReport( + 'nve-pagination', + /* html */ ` - `); + ` + ); expect(report.scores.performance).toBe(100); expect(report.scores.accessibility).toBe(100); expect(report.scores.bestPractices).toBe(100); - expect(report.payload.javascript.kb).toBeLessThan(38.9); + expect(report.payload.javascript.kb).toBeLessThan(38.96); }); }); diff --git a/projects/core/src/tree/tree-node.test.ts b/projects/core/src/tree/tree-node.test.ts index c15f2712ac..9e032553a6 100644 --- a/projects/core/src/tree/tree-node.test.ts +++ b/projects/core/src/tree/tree-node.test.ts @@ -312,6 +312,76 @@ describe(TreeNode.metadata.tag, () => { expect(element.selected).toBe(false); }); + it('should focus the node header when its label is clicked', () => { + const nodeHeader = element.shadowRoot!.querySelector('[part="_node-header"]')!; + const nodeTitle = element.shadowRoot!.querySelector('.node-title')!; + + expect(nodeTitle.tabIndex).toBe(-1); + + nodeTitle.dispatchEvent(new PointerEvent('pointerup', { bubbles: true, composed: true })); + + expect(nodeHeader.matches(':focus')).toBe(true); + }); + + it('should preserve focus on an interactive node label descendant', async () => { + const anchor = document.createElement('a'); + anchor.href = '#'; + element.appendChild(anchor); + await elementIsStable(element); + + const nodeHeader = element.shadowRoot!.querySelector('[part="_node-header"]')!; + anchor.focus(); + anchor.dispatchEvent(new PointerEvent('pointerup', { bubbles: true, composed: true })); + + expect(anchor.matches(':focus')).toBe(true); + expect(nodeHeader.matches(':focus')).toBe(false); + }); + + it('should prevent Space from scrolling while toggling tree selection', async () => { + element.selectable = 'single'; + element.behaviorSelect = true; + await elementIsStable(element); + + const nodeHeader = element.shadowRoot!.querySelector('[part="_node-header"]')!; + const keydown = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, code: 'Space', composed: true }); + const keydownHandler = vi.fn(); + tree.addEventListener('keydown', keydownHandler); + + nodeHeader.dispatchEvent(keydown); + nodeHeader.dispatchEvent(new KeyboardEvent('keyup', { bubbles: true, code: 'Space', composed: true })); + await elementIsStable(element); + + expect(keydown.defaultPrevented).toBe(true); + expect(keydownHandler).toHaveBeenCalledOnce(); + expect(element.selected).toBe(true); + }); + + it('should prevent expansion arrow keys from scrolling with application-managed expansion', async () => { + element.expandable = true; + await elementIsStable(element); + + const nodeHeader = element.shadowRoot!.querySelector('[part="_node-header"]')!; + const keydownHandler = vi.fn(); + const openHandler = vi.fn(); + const closeHandler = vi.fn(); + tree.addEventListener('keydown', keydownHandler); + element.addEventListener('open', openHandler); + element.addEventListener('close', closeHandler); + + for (const code of ['ArrowLeft', 'ArrowRight']) { + const keydown = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, code, composed: true }); + nodeHeader.dispatchEvent(keydown); + nodeHeader.dispatchEvent(new KeyboardEvent('keyup', { bubbles: true, code, composed: true })); + + expect(keydown.defaultPrevented).toBe(true); + } + + expect(keydownHandler).toHaveBeenCalledTimes(2); + expect(closeHandler).toHaveBeenCalledOnce(); + expect(openHandler).toHaveBeenCalledOnce(); + expect(element.expanded).toBe(false); + }); + it('should expand node if node header is clicked with behavior-expand and no interactive elements', async () => { expect(nestedNodeElement.expanded).toBe(false); diff --git a/projects/core/src/tree/tree-node.ts b/projects/core/src/tree/tree-node.ts index f382627344..15c56ccdc5 100644 --- a/projects/core/src/tree/tree-node.ts +++ b/projects/core/src/tree/tree-node.ts @@ -167,7 +167,7 @@ export class TreeNode extends LitElement { : nothing }
- +
@@ -180,12 +180,14 @@ export class TreeNode extends LitElement { super.connectedCallback(); attachInternals(this); this._internals.role = 'treeitem'; + this.addEventListener('keydown', this.#onKeydown); this.addEventListener('keyup', this.#onKeyup); this.#nodeUpdate(); } disconnectedCallback() { super.disconnectedCallback(); + this.removeEventListener('keydown', this.#onKeydown); this.removeEventListener('keyup', this.#onKeyup); } @@ -208,6 +210,15 @@ export class TreeNode extends LitElement { this.#isExpandable ? this._internals.states.add('is-expandable') : this._internals.states.delete('is-expandable'); } + #onKeydown = (e: KeyboardEvent) => { + const isSelectionKey = this.selectable && e.code === 'Space'; + const isExpansionKey = this.#isExpandable && (e.code === 'ArrowLeft' || e.code === 'ArrowRight'); + + if (e.target === this && (isSelectionKey || isExpansionKey)) { + e.preventDefault(); + } + }; + #onKeyup = (e: KeyboardEvent) => { if (this.#isExpandable && e.code === 'ArrowLeft' && e.target === this) { this.close(); @@ -218,7 +229,6 @@ export class TreeNode extends LitElement { } if (e.code === 'Space' && e.target === this && this.selectable) { - e.preventDefault(); this.#toggleSelection(); } }; diff --git a/projects/core/src/tree/tree.test.lighthouse.ts b/projects/core/src/tree/tree.test.lighthouse.ts index 3f4f2172d2..625c2fa3e1 100644 --- a/projects/core/src/tree/tree.test.lighthouse.ts +++ b/projects/core/src/tree/tree.test.lighthouse.ts @@ -30,6 +30,6 @@ describe('tree lighthouse report', () => { expect(report.scores.performance).toBe(100); expect(report.scores.accessibility).toBe(100); expect(report.scores.bestPractices).toBe(100); - expect(report.payload.javascript.kb).toBeLessThan(30.41); + expect(report.payload.javascript.kb).toBeLessThan(30.42); }); }); diff --git a/projects/core/src/tree/tree.test.ts b/projects/core/src/tree/tree.test.ts index 70e99891f2..16346df487 100644 --- a/projects/core/src/tree/tree.test.ts +++ b/projects/core/src/tree/tree.test.ts @@ -280,6 +280,29 @@ describe(`${Tree.metadata.tag} - collapsed nodes`, () => { expect(nodes[11].matches(':focus')).toBe(false); }); + it('should move focus with ArrowDown after a node label is clicked', async () => { + const currentNode = element.nodes[1]!; + const currentHeader = currentNode.shadowRoot!.querySelector('[part="_node-header"]')!; + const currentLabel = currentNode.shadowRoot!.querySelector('.node-title')!; + const nextHeader = element.nodes[2]!.shadowRoot!.querySelector('[part="_node-header"]')!; + let eventDetail: Record | undefined; + + element.addEventListener('nve-key-change', ((e: CustomEvent) => { + eventDetail = e.detail; + }) as EventListener); + + currentLabel.dispatchEvent(new PointerEvent('pointerup', { bubbles: true, composed: true })); + expect(currentHeader.matches(':focus')).toBe(true); + expect(element.nodes[0]!.shadowRoot!.querySelector('[part="_node-header"]')!.tabIndex).toBe(-1); + expect(currentHeader.tabIndex).toBe(0); + + currentHeader.dispatchEvent(new KeyboardEvent('keydown', { code: 'ArrowDown', bubbles: true, composed: true })); + await elementIsStable(element); + + expect(nextHeader.matches(':focus')).toBe(true); + expect(eventDetail).toMatchObject({ activeItem: nextHeader, code: 'ArrowDown' }); + }); + it('should keynav from a single level node to a expanded multi level node', async () => { await elementIsStable(element); nodes[1].focus();