diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index c62b3342c..313270368 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -1219,6 +1219,49 @@ describe('Recommendations Module', () => { expect(savingsHeader?.innerHTML).toContain('active'); }); + describe('header keyboard semantics (issue #755)', () => { + test('header is a plain th with aria-sort and sibling sort and filter buttons', async () => { + await loadRecommendations(); + const savings = document.querySelector('th[data-sort="savings"]'); + expect(savings?.getAttribute('role')).toBeNull(); + expect(savings?.hasAttribute('tabindex')).toBe(false); + expect(savings?.getAttribute('aria-sort')).toBe('descending'); + expect(document.querySelector('th[data-sort="upfront_cost"]')?.hasAttribute('aria-sort')).toBe(false); + const sortBtn = savings?.querySelector('button.sort-btn'); + const filterBtn = savings?.querySelector('button.column-filter-btn'); + expect(sortBtn?.textContent).toContain('Savings'); + expect(filterBtn).not.toBeNull(); + expect(sortBtn?.contains(filterBtn ?? null)).toBe(false); + }); + + test('Enter on a column-filter button does not sort', async () => { + await loadRecommendations(); + (state.setRecommendationsSort as jest.Mock).mockClear(); + const filterBtn = document.querySelector('th[data-sort="service"] .column-filter-btn'); + filterBtn?.focus(); + const enter = new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true }); + filterBtn?.dispatchEvent(enter); + expect(enter.defaultPrevented).toBe(false); + expect(state.setRecommendationsSort).not.toHaveBeenCalled(); + // Native Enter activation of a ${filterBtn(col.key)}`; }; // issues #225 + #226: group by cell, sort groups, then render. @@ -4882,10 +4889,17 @@ function renderRecommendationsList(loadedRecs: LocalRecommendation[]): void { ensureRecommendationsTabObserver(); // Sortable column headers. Toggle ascending/descending on repeat click. + // The click listener sits on the so the whole header stays a mouse + // target; keyboard users reach it through the inner .sort-btn, whose + // native Enter/Space activation bubbles here as a click. The filter + // button stops propagation, so it never sorts (issue #755). container.querySelectorAll('th.sortable').forEach((th) => { const onActivate = (): void => { const col = th.dataset['sort']; if (!col) return; + // The re-render below replaces the header, so remember whether the + // sort button had focus and move focus to its replacement. + const hadFocus = th.contains(document.activeElement); const prev = state.getRecommendationsSort(); // Issue #480: first click on a different column uses that column's // per-config default direction (asc for text/most numerics; desc for @@ -4902,14 +4916,13 @@ function renderRecommendationsList(loadedRecs: LocalRecommendation[]): void { // at Savings desc". writeSortToUrl({ column: col as state.RecommendationsSortColumn, direction }); renderRecommendationsList(loadedRecs); + if (hadFocus) { + document.querySelector( + `#recommendations-list th[data-sort="${CSS.escape(col)}"] .sort-btn`, + )?.focus(); + } }; th.addEventListener('click', onActivate); - th.addEventListener('keydown', (e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - onActivate(); - } - }); }); // Issue #869: selection handlers are only meaningful when the session can diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 5d76ec88e..28af8417b 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -662,6 +662,25 @@ th.sortable:hover { background: var(--cudly-bg); } +/* Sort trigger inside a sortable (issue #755): looks like the header + * text, but is a real button so keyboard users can sort without the + * itself being interactive. */ +th.sortable .sort-btn { + padding: 0; + border: 0; + background: transparent; + color: inherit; + font: inherit; + text-align: inherit; + cursor: pointer; +} + +th.sortable .sort-btn:focus-visible, +th .column-filter-btn:focus-visible { + outline: 2px solid var(--cudly-primary); + outline-offset: 2px; +} + th.sortable .sort-indicator { margin-left: 0.35rem; color: var(--cudly-text-subtle); @@ -672,8 +691,14 @@ th.sortable .sort-indicator.active { color: var(--cudly-primary); } -/* Per-column filter trigger button inside . */ +/* Per-column filter trigger button inside . The 24px minimum keeps the + * glyph-sized button above the WCAG 2.5.8 target size (issue #755). */ th .column-filter-btn { + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 24px; + min-height: 24px; margin-left: 0.35rem; padding: 0 0.25rem; border: 0; @@ -682,6 +707,7 @@ th .column-filter-btn { cursor: pointer; font-size: 0.85em; line-height: 1; + vertical-align: middle; } th .column-filter-btn:hover { color: var(--cudly-primary); diff --git a/frontend/tests-e2e/recommendations-header-keyboard.spec.ts b/frontend/tests-e2e/recommendations-header-keyboard.spec.ts new file mode 100644 index 000000000..a31754e0e --- /dev/null +++ b/frontend/tests-e2e/recommendations-header-keyboard.spec.ts @@ -0,0 +1,58 @@ +import { expect, test, type Page } from '@playwright/test'; + +import { mockApi, RECS, seedAuth } from './fixtures/recs'; + +// Issue #755: Enter on a column-filter button re-sorted the table instead of +// opening the filter, because the focusable role=button handled Enter +// for the whole header. The header now holds sibling sort and filter buttons. + +async function openOpportunities(page: Page): Promise { + await mockApi(page); + await seedAuth(page); + await page.goto('/'); + await page.getByRole('tab', { name: 'Opportunities' }).click(); + await expect(page.locator('.recommendations-filter-live')).toContainText( + `Showing ${RECS.length} of ${RECS.length} recommendations`, + ); +} + +const firstRowId = (page: Page) => + page.locator('tr.recommendation-row').first().getAttribute('data-rec-id'); + +test('Enter on a column-filter button opens the filter and does not sort', async ({ page }) => { + await openOpportunities(page); + const before = await firstRowId(page); + const filterBtn = page.getByRole('button', { name: 'Filter Service' }); + await filterBtn.focus(); + await page.keyboard.press('Enter'); + + const dialog = page.getByRole('dialog', { name: 'Filter Service' }); + await expect(dialog).toBeVisible(); + expect(await firstRowId(page)).toBe(before); + await expect(page.locator('th[data-sort="savings"]')).toHaveAttribute('aria-sort', 'descending'); + await expect(page.locator('th[data-sort="service"]')).not.toHaveAttribute('aria-sort', /.*/); +}); + +test('the sort button sorts from the keyboard and keeps focus', async ({ page }) => { + await openOpportunities(page); + const sortBtn = page.locator('th[data-sort="service"] .sort-btn'); + await sortBtn.focus(); + await page.keyboard.press('Enter'); + + await expect(page.locator('th[data-sort="service"]')).toHaveAttribute('aria-sort', 'ascending'); + await expect(page.locator('th[data-sort="savings"]')).not.toHaveAttribute('aria-sort', /.*/); + await expect(page.locator('th[data-sort="service"] .sort-btn')).toBeFocused(); + await expect(page.locator('.column-filter-popover')).toHaveCount(0); +}); + +test('column headers are not nested interactive controls and filter targets are at least 24px', async ({ page }) => { + await openOpportunities(page); + await expect(page.locator('th.sortable[role="button"], th.sortable[tabindex]')).toHaveCount(0); + const boxes = await page.locator('th .column-filter-btn').evaluateAll((els) => + els.map((el) => el.getBoundingClientRect()).map((r) => ({ w: r.width, h: r.height }))); + expect(boxes.length).toBeGreaterThan(0); + for (const box of boxes) { + expect(box.w).toBeGreaterThanOrEqual(24); + expect(box.h).toBeGreaterThanOrEqual(24); + } +});