[IAmJSD]

In the Opportunities table, pressing Enter on a focused column-filter button re-sorted the table, never opened the filter, and dropped focus to the page body, so keyboard users could not filter. The header was a focusable role=button <th> wrapping the filter button, with no aria-sort. This PR makes each header a plain column header with aria-sort and separate sort and filter buttons.

Closes #755

What changed

• frontend/src/recommendations.ts: buildListMarkup renders each sortable header as <th class="sortable" data-sort=... aria-sort=ascending|descending> (aria-sort on the active column only), containing a <button class="sort-btn"> with the label and sort indicator, and the existing filter button as a sibling. role=button, tabindex and aria-label are removed from the <th>.
• The <th> click listener stays (the whole header is still a mouse target, and the sort button's native Enter/Space arrives as a click); the custom keydown handler that sorted on any bubbled Enter/Space is removed. After a sort, focus moves to the re-rendered sort button for the same column.
• frontend/src/styles/components.css: a reset style for .sort-btn so it looks like the old header text, :focus-visible rings for the sort and filter buttons, and a 24px minimum width/height for th .column-filter-btn (WCAG 2.5.8).
• Tests: new "header keyboard semantics (issue fix(frontend): Enter on a column-filter button sorts instead of opening the filter #755)" cases in recommendations.test.ts and a new tests-e2e/recommendations-header-keyboard.spec.ts.

Commit: 270fb90e.

Why this approach

This follows the issue's suggested fix (a real <th> with aria-sort, sort and filter buttons as siblings) rather than the smallest patch, an e.target !== th guard in the keydown handler, which would leave the nested-interactive markup and the missing aria-sort. The <th> click listener and the th.sortable/data-sort hooks are kept, so existing tests and mouse behavior are unchanged. The sort button's accessible name is the column label (e.g. "Service") and aria-sort carries the direction, so screen readers announce "Service, column header, sorted ascending, button".

Screenshots

Before is main (a14dc8f2), after is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API.

Before	After
With the Service filter button focused, press Enter: no popover opens; the table re-sorts by Service (Azure rows first) and focus is lost.	The same Enter opens the "Filter Service" popover with focus on (All); the table keeps its default Monthly Savings sort.
Keyboard sorting by Service works, but after the re-render focus is lost (no focus ring anywhere).	Enter on the Service sort button sorts, and focus stays on the re-rendered sort button (ring around "Service ▲").

Verification

Run on this branch's head 270fb90e on top of main a14dc8f2, from frontend/:

• npm run lint: 0 errors (142 warnings already on main). Typecheck and build pass.
• Jest: 109 suites, 3489 passed, 1 skipped. The 3 new tests also pass with --randomize. The whole recommendations.test.ts file fails 2 tests under --randomize on main as well (see test(frontend): recommendations.test.ts fails 6 tests under random ordering (order-dependent state) #803), not caused by this change.
• Playwright (e2e-local.sh): 160/160 passed, including the new recommendations-header-keyboard.spec.ts: Enter on Filter Service opens the dialog without re-sorting; keyboard sort sets aria-sort and keeps focus on the sort button; no th has role=button or tabindex; every filter button is at least 24x24.
• Secrets scan clean (planted key flagged); scripts/test-git-secrets-allowlist.sh: 137 PASS.

Notes for review

• The filter glyph button now has a 24x24 minimum box, so the header row is about 8px taller and some two-word headers (e.g. Resource Type) wrap the glyph onto its own line. The same rule also enlarges the filter buttons in the Plans and RI Exchange tables, which share th .column-filter-btn.
• The sort and filter buttons get a 2px primary-color :focus-visible outline.
• The sort control's accessible name changed from "Sort by X" to the bare label, with aria-sort on the th. Clicking anywhere in the header still sorts.
• axe was not run on this branch; the nested-interactive and target-size fixes are covered by the e2e assertions instead.

Overlaps

• fix(frontend): three money formatters, mixed placeholders, left-aligned money columns #739 edits the same colHeader return line in recommendations.ts (it adds a num class to th.sortable): textual conflict, resolve by keeping both (class="sortable${numClass}" plus this markup).
• chore(frontend): add typography foundation (line-height, heading scale, font inherit) #741 edits th .column-filter-btn.active a few lines below the CSS changed here: small context conflict.
• refactor(frontend): consolidate column-filter popover plumbing into shared lib #18 (popover consolidation) also touches this area.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 61be5b6
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3a11e81d-109b-4fcf-a5e7-98d184f2ff34

Commits

Reviewing files that changed from the base of the PR and between 61be5b6 and 270fb90.

Files selected for processing: 4

• frontend/src/__tests__/recommendations.test.ts
• frontend/src/recommendations.ts
• frontend/src/styles/components.css
• frontend/tests-e2e/recommendations-header-keyboard.spec.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE 270fb90e74172468d03fb5fe04564b885dff5362

Blocking:

• none
  Non-blocking:
• frontend/src/recommendations.ts:3249: the sort control's accessible name dropped "Sort by". With aria-sort only on the active column, an unsorted header's button reads as just "Service, button", which gives no hint that it sorts. Consider aria-describedby or visually hidden "sort" text.
• The 24px minimum on th .column-filter-btn also enlarges the Plans and RI Exchange headers. This is acknowledged in the PR, and fix(frontend): three money formatters, mixed placeholders, left-aligned money columns #739 will have a textual conflict on the same colHeader line.
  Attacked: Checked that the filter button's click handler still stops propagation, so the th click listener cannot sort from it, and that removing the th keydown handler leaves native button Enter/Space as the only keyboard path. Checked that the outside-click handler still ignores .column-filter-btn. Checked that sortIndicator glyphs are aria-hidden, so the button name is the label. Checked that the focus-restore selector uses CSS.escape on the column key, that there are no new innerHTML interpolations of untrusted data (lbl comes from static column labels), and that no other module depends on th[role=button] or the old aria-label. The new jest Enter test fails on main as claimed.

[IAmJSD]

Local verification at 270fb90e merged onto current main (d4342031; merge-tree 1c603e99; Linux, Node, headless Chromium):

• This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
• The combined tree of every UI PR in this merge wave (tree 89dd34d8, main plus 46 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 3914 passed, 3915 total), and the full Playwright suite (208 passed).
• CI is green on 270fb90e (CI Success, Run pre-commit hooks).
• The independent review above approves this exact head.
• Nothing was applied to any cloud.

[IAmJSD]

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at 270fb90e.