From ee49c2aba5e8472cc52eaebd408627a8c825df4b Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 23:22:13 +0000 Subject: [PATCH 1/4] fix(frontend): make the sidebar plain links, move focus on route change, add sub-tab keys The sidebar declared the ARIA tab pattern (role=tablist/tab, aria-selected) without its keyboard model, so screen readers announced "tab, 2 of 6" and arrow keys did nothing. It is page navigation, so it is now plain links with aria-current="page", and the top-level page containers drop their tabpanel role. Activating a sidebar link, or going back/forward, left focus on the link and announced nothing. switchTab now takes a focusContent option, set by the sidebar click handler and the popstate router, which focuses a visually hidden

page heading at the top of each page. The no-access placeholder keeps that heading. The mobile drawer no longer pulls focus back to the hamburger when a link click has already moved it. The Inventory and Admin sub-tab lists keep role=tablist, so they now get its keyboard model (lib/tablist.ts): a roving tabindex kept in sync by the sub-tab switchers, Left/Right with wrap-around, Home/End, hidden admin-only tabs skipped. Activation is manual (Enter/Space) because each selection fetches data and pushes a history entry. Refs #730 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/app.test.ts | 2 +- frontend/src/__tests__/mobile-nav.test.ts | 23 ++++ frontend/src/__tests__/route-focus.test.ts | 128 ++++++++++++++++++ .../src/__tests__/sidebar-anchors.test.ts | 8 +- frontend/src/app.ts | 23 +++- frontend/src/index.html | 46 ++++--- frontend/src/inventory.ts | 2 + frontend/src/lib/tablist.ts | 78 +++++++++++ frontend/src/navigation.ts | 39 +++++- .../tests-e2e/mobile-header-drawer.spec.ts | 2 +- frontend/tests-e2e/recommendations.spec.ts | 6 +- 11 files changed, 319 insertions(+), 38 deletions(-) create mode 100644 frontend/src/__tests__/route-focus.test.ts create mode 100644 frontend/src/lib/tablist.ts diff --git a/frontend/src/__tests__/app.test.ts b/frontend/src/__tests__/app.test.ts index b20799121..43aead925 100644 --- a/frontend/src/__tests__/app.test.ts +++ b/frontend/src/__tests__/app.test.ts @@ -243,7 +243,7 @@ describe('App Module', () => { const dashboardBtn = document.querySelector('[data-tab="dashboard"]') as HTMLButtonElement; dashboardBtn.click(); - expect(navigation.switchTab).toHaveBeenCalledWith('dashboard'); + expect(navigation.switchTab).toHaveBeenCalledWith('dashboard', { focusContent: true }); }); test('does not call switchTab if tab data attribute is missing', () => { diff --git a/frontend/src/__tests__/mobile-nav.test.ts b/frontend/src/__tests__/mobile-nav.test.ts index efb795077..0262c4f08 100644 --- a/frontend/src/__tests__/mobile-nav.test.ts +++ b/frontend/src/__tests__/mobile-nav.test.ts @@ -123,6 +123,29 @@ describe('setupMobileNav', () => { expect(document.body.classList.contains('sidebar-open')).toBe(false); }); + test('closing via a sidebar link keeps focus on the new page heading (#730)', () => { + document.body.insertAdjacentHTML('beforeend', '

Plans

'); + const pageHeading = document.getElementById('page-h') as HTMLElement; + const link = sidebar().querySelectorAll('.tab-btn')[1]!; + // Stands in for app.ts's tab handler, registered before the drawer's: + // switchTab(..., { focusContent: true }) has moved focus by now. + link.addEventListener('click', () => pageHeading.focus()); + setupMobileNav(); + hamburger().click(); // open + link.dispatchEvent(new MouseEvent('click', { button: 0, bubbles: true })); + expect(document.body.classList.contains('sidebar-open')).toBe(false); + expect(document.activeElement).toBe(pageHeading); + }); + + test('closing via a sidebar link rescues focus left inside the drawer', () => { + setupMobileNav(); + hamburger().click(); // open + const link = sidebar().querySelector('.tab-btn')!; + link.focus(); + link.dispatchEvent(new MouseEvent('click', { button: 0, bubbles: true })); + expect(document.activeElement).toBe(hamburger()); + }); + test('modifier-key click on sidebar link does not close the drawer', () => { setupMobileNav(); hamburger().click(); // open diff --git a/frontend/src/__tests__/route-focus.test.ts b/frontend/src/__tests__/route-focus.test.ts new file mode 100644 index 000000000..63bc02894 --- /dev/null +++ b/frontend/src/__tests__/route-focus.test.ts @@ -0,0 +1,128 @@ +/** + * Route-change focus and sidebar aria-current (issue #730). + * + * After a user-driven page change, focus moves to the new page's heading + * so a screen reader announces the page and keyboard users continue from + * its top instead of staying on the sidebar link. + */ +import { switchTab, switchSettingsSubTab, initRouter } from '../navigation'; + +jest.mock('../dashboard', () => ({ loadDashboard: jest.fn().mockResolvedValue(undefined) })); +jest.mock('../recommendations', () => ({ loadRecommendations: jest.fn().mockResolvedValue(undefined) })); +jest.mock('../plans', () => ({ loadPlans: jest.fn().mockResolvedValue(undefined) })); +jest.mock('../history', () => ({ + initHistoryDateRange: jest.fn(), + loadHistory: jest.fn().mockResolvedValue(undefined), +})); +jest.mock('../settings', () => ({ + loadGlobalSettings: jest.fn().mockResolvedValue(undefined), + isUnsavedChanges: jest.fn().mockReturnValue(false), + loadAccountsTab: jest.fn().mockResolvedValue(undefined), +})); +jest.mock('../users', () => ({ loadUsers: jest.fn() })); +jest.mock('../apikeys', () => ({ loadApiKeys: jest.fn() })); +jest.mock('../modules/savings-history', () => ({ loadSavingsHistory: jest.fn() })); +jest.mock('../riexchange', () => ({ + loadRIExchange: jest.fn().mockResolvedValue(undefined), + loadAutomationSettings: jest.fn().mockResolvedValue(undefined), +})); +jest.mock('../auth', () => ({ isAdmin: jest.fn().mockReturnValue(true) })); +jest.mock('../permissions', () => ({ canAccess: jest.fn().mockReturnValue(true) })); +jest.mock('../inventory', () => ({ + DEFAULT_INVENTORY_SUB_SECTION: 'active-commitments', + isValidInventorySubSection: (n: string): boolean => ['active-commitments', 'coverage', 'ri-exchange'].includes(n), + switchInventorySubSection: jest.fn((n: string) => n), + loadInventory: jest.fn(), +})); + +import { canAccess } from '../permissions'; + +const TABS = ['home', 'opportunities', 'plans', 'purchases', 'inventory', 'admin']; + +function buildDOM(): void { + document.body.innerHTML = ` + + ${TABS.map(t => ` +
+

${t} page

+
+
`).join('')} + `; + const admin = document.getElementById('admin-tab')!; + admin.insertAdjacentHTML('beforeend', ` +
+ + +
+
+ `); +} + +const heading = (tab: string): HTMLElement => + document.querySelector(`#${tab}-tab > .page-heading`)!; + +describe('route-change focus (issue #730)', () => { + beforeEach(() => { + buildDOM(); + (canAccess as jest.Mock).mockReturnValue(true); + switchTab('home', { push: false }); + }); + + test('sidebar marks only the current page with aria-current="page"', () => { + switchTab('plans'); + const current = document.querySelectorAll('.tab-btn[aria-current="page"]'); + expect(current.length).toBe(1); + expect((current[0] as HTMLElement).dataset['tab']).toBe('plans'); + expect(document.querySelector('.tab-btn[aria-selected]')).toBeNull(); + }); + + test('focusContent moves focus from the sidebar link to the new page heading', () => { + const link = document.getElementById('opportunities-tab-btn')!; + link.focus(); + switchTab('opportunities', { focusContent: true }); + expect(document.activeElement).toBe(heading('opportunities')); + }); + + test('programmatic switches leave focus where it is', () => { + const inner = document.querySelector('#home-tab .inner')!; + inner.focus(); + switchTab('plans'); + expect(document.activeElement).toBe(inner); + }); + + test('re-activating the current page still moves focus to its heading', () => { + switchTab('plans', { focusContent: true }); + document.getElementById('plans-tab-btn')!.focus(); + switchTab('plans', { focusContent: true }); + expect(document.activeElement).toBe(heading('plans')); + }); + + test('the no-access placeholder keeps the heading as the focus target', () => { + (canAccess as jest.Mock).mockReturnValue(false); + switchTab('purchases', { focusContent: true }); + const page = document.getElementById('purchases-tab')!; + expect(page.firstElementChild).toBe(heading('purchases')); + expect(page.querySelector('.empty-state')).not.toBeNull(); + expect(page.querySelector('.inner')).toBeNull(); + expect(document.activeElement).toBe(heading('purchases')); + }); + + test('browser back/forward moves focus to the restored page heading', () => { + initRouter(); + window.history.replaceState(null, '', '/plans'); + window.dispatchEvent(new PopStateEvent('popstate', { state: null })); + expect(document.getElementById('plans-tab')?.classList.contains('active')).toBe(true); + expect(document.activeElement).toBe(heading('plans')); + }); + + test('switching Admin sub-tabs moves the roving tab stop with the selection', () => { + switchTab('admin', { push: false }); + switchSettingsSubTab('purchasing'); + const [general, purchasing] = Array.from(document.querySelectorAll('.settings-tabs [role="tab"]')); + expect(purchasing!.getAttribute('aria-selected')).toBe('true'); + expect(purchasing!.getAttribute('tabindex')).toBe('0'); + expect(general!.getAttribute('tabindex')).toBe('-1'); + }); +}); diff --git a/frontend/src/__tests__/sidebar-anchors.test.ts b/frontend/src/__tests__/sidebar-anchors.test.ts index 506b7c3e9..d1b2cd720 100644 --- a/frontend/src/__tests__/sidebar-anchors.test.ts +++ b/frontend/src/__tests__/sidebar-anchors.test.ts @@ -88,14 +88,15 @@ describe('Sidebar nav - issue #462 anchor markup', () => { }); test.each(EXPECTED_NAV)( - '$tab is rendered as anchor at $href with role=tab', + '$tab is rendered as a plain link at $href (no ARIA tab role, issue #730)', ({ tab, href }) => { document.body.innerHTML = html; const el = document.querySelector(`.app-sidebar-nav [data-tab="${tab}"]`); expect(el).toBeTruthy(); expect(el?.tagName).toBe('A'); expect((el as HTMLAnchorElement).getAttribute('href')).toBe(href); - expect(el?.getAttribute('role')).toBe('tab'); + expect(el?.hasAttribute('role')).toBe(false); + expect(el?.hasAttribute('aria-selected')).toBe(false); }, ); @@ -119,7 +120,6 @@ describe('Sidebar nav click handling - issue #462', () => { a.className = 'tab-btn'; a.setAttribute('href', '/opportunities'); a.setAttribute('data-tab', 'opportunities'); - a.setAttribute('role', 'tab'); a.textContent = 'Opportunities'; document.body.appendChild(a); setupEventListeners(); @@ -134,7 +134,7 @@ describe('Sidebar nav click handling - issue #462', () => { }); link.dispatchEvent(evt); expect(evt.defaultPrevented).toBe(true); - expect(switchTab).toHaveBeenCalledWith('opportunities'); + expect(switchTab).toHaveBeenCalledWith('opportunities', { focusContent: true }); }); test('Ctrl-click falls through to the browser (no preventDefault, no switchTab)', () => { diff --git a/frontend/src/app.ts b/frontend/src/app.ts index c73b9658e..7250fcf52 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -22,6 +22,7 @@ import { buildCommitmentSummary, commitmentTotals, executeButtonLabel, executeCo import { handlePurchaseDeeplink } from './purchases-deeplink'; import { handleArcheraDeeplink, openArcheraOfferModal } from './archera'; import { closeModal } from './modal'; +import { wireTablistKeyboard } from './lib/tablist'; /** * describeDuplicateSubmit words the outcome of a submit the backend collapsed @@ -172,11 +173,17 @@ export function setupEventListeners(): void { const tab = btn.dataset['tab']; if (tab) { e.preventDefault(); - switchTab(tab); + switchTab(tab, { focusContent: true }); } }); }); + // In-page sub-tab lists (Inventory & Coverage, Admin): arrow/Home/End + // focus movement and a roving tabindex, the keyboard model their + // role="tablist" promises (issue #730). + document.querySelectorAll('.inventory-subnav[role="tablist"], .settings-tabs[role="tablist"]') + .forEach(wireTablistKeyboard); + // Settings sub-tab switching document.querySelectorAll('.sub-tab-btn').forEach(btn => { btn.addEventListener('click', () => { @@ -867,13 +874,21 @@ export function setupMobileNav(): void { firstFocusable?.focus(); } - function closeDrawer(): void { + /** + * `returnFocus: 'if-inside'` returns focus to the hamburger only when it is + * still inside the drawer. A sidebar link click has by then moved it to the + * new page's heading (issue #730), or a dirty-guard dialog holds it, and + * pulling it back to the hamburger would undo either. + */ + function closeDrawer(returnFocus: 'always' | 'if-inside' = 'always'): void { + const focusInside = sidebar!.contains(document.activeElement); document.body.classList.remove('sidebar-open'); hamburger!.setAttribute('aria-expanded', 'false'); sidebar!.setAttribute('aria-hidden', 'true'); + // Before inert lands, which does not blur focus in Chromium. + if (returnFocus === 'always' || focusInside) hamburger!.focus(); sidebar!.setAttribute('inert', ''); if (overlay) overlay.setAttribute('aria-hidden', 'true'); - hamburger!.focus(); } /** @@ -976,7 +991,7 @@ export function setupMobileNav(): void { return; } if (document.body.classList.contains('sidebar-open')) { - closeDrawer(); + closeDrawer('if-inside'); } }); }); diff --git a/frontend/src/index.html b/frontend/src/index.html index ebbbdd24c..57a05566f 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -56,33 +56,33 @@

CUDly

-