[IAmJSD]

Picking an option from the filter chip's search list dropped keyboard focus to <body>. Screen readers were never told which option was highlighted in that search list, loading skeletons weren't marked busy, and the global filter bar's label was ignored because its div had no role. This PR fixes those items from the 2026-10-08 accessibility review. The review's other focus item, Escape in the RI Exchange column-filter popovers (UX-27), is already fixed on main by the shared popover from #1315; this PR adds regression tests for it.

Refs #729
Partial: the UX-36 "34 small inline links on Admin General" item could not be reproduced (Admin General has no <a> elements) and is not addressed; it needs confirmation of which page the review meant.

What changed

• UX-27: no source change. Main's shared lib/column-filter-popover.ts (refactor(frontend): share one column-filter popover across Plans and RI #1315) already returns focus to the filter button on Escape and leaves it alone on outside click. This PR adds 3 integration tests for the Active RIs and Reshape popovers in riexchange-active-ri-filters.test.ts.
• UX-31, frontend/src/lib/chip-select.ts: aria-activedescendant goes on whichever element holds focus while the menu is open (the search input when shown, otherwise the trigger), and the search input gets aria-controls pointing at the listbox, which now has an id. selectByValue returns focus to the trigger when focus was inside the widget.
• UX-34, frontend/src/lib/skeleton.ts: activateSkeleton sets aria-busy="true" on the container. teardownSkeleton clears it, and a per-container MutationObserver clears it on the success path, where callers replace the children directly.
• UX-36, frontend/src/index.html: role="group" on #topbar-filters.
• Tests: riexchange-active-ri-filters.test.ts, chip-select.test.ts, skeleton.test.ts, html.test.ts, a one-line update in mobile-nav.test.ts, and new tests-e2e/a11y-focus-return.spec.ts.

Why this approach

• For chip-select, the attribute moves to the focused element; switching the input to a full role="combobox" would be a bigger refactor of a shared widget.
• For aria-busy, the observer avoids editing every skeleton caller: about ten modules render success by replacing children and never call teardown, and a missed one would leave a region stuck as busy. The observer only exists while a skeleton is showing.

Screenshots

Screenshots were taken before this branch was merged with main; before = main at a14dc8f. After is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API. Each shot carries a black caption showing document.activeElement and the relevant ARIA state.

Before	After

• Account chip with 14 accounts (search input shown): type "Extra 3", ArrowDown, Enter. On main the pick applies but focus drops to <body>, aria-activedescendant sat on the unfocused chip, and #topbar-filters has no role. After, focus returns to the chip, aria-activedescendant was on the focused search input, and #topbar-filters is role="group" "Global filters". The "Failed to load dashboard: HTTP 404" banner in both shots comes from the shared mock.
• Opportunities loading (request held for 4 s): on main, the skeleton rows are aria-hidden and #recommendations-list has no aria-busy. After, it has aria-busy="true" while the skeleton shows; clearing after render is covered by jest.

Verification

Run on this branch's head e7f304df:

• Merged with main at 5fbedae; conflicts in frontend/src/riexchange.ts and frontend/src/__tests__/riexchange-active-ri-filters.test.ts resolved, see the merge commit.
• Lint: 0 errors. Typecheck and build pass.
• Jest: 3878 of 3879 passed, 1 skipped. Six of the new tests fail on main; the three UX-27 tests pass on main and guard against a regression. New tests: Escape on the Active RI and Reshape popovers and outside click leaving focus alone; search-input aria-activedescendant, Enter pick and mouse pick returning focus, short-list behavior unchanged; aria-busy set on show, kept after observers run, cleared by teardown and by a success render, re-armed on reload; #topbar-filters role="group".
• Playwright via e2e-local.sh: 204 passed, including the new a11y-focus-return.spec.ts (3 tests: Escape returns focus in the Active RI popover; the Account chip search keeps aria-activedescendant on the input and returns focus to the chip after a pick; the filter bar is a labelled group).
• git-secrets scan clean (planted key flagged); scripts/test-git-secrets-allowlist.sh: 137 pass.

Notes for review

• Done: UX-31, the aria-busy half of UX-34, and the role="group" half of UX-36. UX-27 is done on main via refactor(frontend): share one column-filter popover across Plans and RI #1315 and only gets tests here.
• Already fixed on main: the UX-34 ladder canvas Tab stop. fix(ladder): timeline editor 409 reload, focus restore and canvas tab stop #688 (e5ac75d2, after the reviewed commit b50ca383) removed tabindex="0" from the canvas (frontend/src/ladder-timeline.ts:36).
• Not done: the UX-36 small-target links. A Playwright probe of every <a> on all four Admin sub-tabs, plus Home, Purchases and RI Exchange, found no link under 24px on a14dc8f2. Admin General has no <a> elements; the only sub-24px targets there are three native 13px provider checkboxes inside their text labels. The review's screenshot (usab-admin-general.png) is only in a local archive, so it is unclear whether the item still applies or which page it meant.
• The aria-busy MutationObserver is a small runtime mechanism; explicit clears in each render path are the alternative.
• In the short-list chip-select case (no search input), aria-activedescendant stays on the trigger as before.

Overlaps

Textual conflict with the open branch for #186 (lib/skeleton.ts and __tests__/skeleton.test.ts, checked with git merge-tree). Open branches touching the same files on other lines: #730 and #215 (index.html, mobile-nav.test.ts); #190, #676, #727, #728, #739, #749, #751, #756 (index.html).

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 9 billable files and costs up to $2.25.}

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: 5fbedae
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5a423627-fa0d-4dd3-8970-0530a1fdeaa4

Commits

Reviewing files that changed from the base of the PR and between 5fbedae and e7f304d.

Files selected for processing: 9

• frontend/src/__tests__/chip-select.test.ts
• frontend/src/__tests__/html.test.ts
• frontend/src/__tests__/mobile-nav.test.ts
• frontend/src/__tests__/riexchange-active-ri-filters.test.ts
• frontend/src/__tests__/skeleton.test.ts
• frontend/src/index.html
• frontend/src/lib/chip-select.ts
• frontend/src/lib/skeleton.ts
• frontend/tests-e2e/a11y-focus-return.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 e7f304df5daa4ef658c1a6167456b059342e6c8a

Blocking:

• none
  Non-blocking:
• frontend/src/lib/chip-select.ts:162-164,195-202: aria-activedescendant now sits on an input type=search that has no role="combobox" and no aria-expanded. ARIA allows that, but NVDA and JAWS often don't announce activedescendant changes on a plain textbox. The jest test (chip-select.test.ts:357-368) and the e2e test (a11y-focus-return.spec.ts:62-86) only check where the attribute is, so nobody has confirmed with a screen reader that UX-31 is fixed. The body already names the combobox refactor as a follow-up.
• frontend/src/tests/riexchange-active-ri-filters.test.ts:269-326: the three UX-27 tests now cover main's shared lib/column-filter-popover.ts (refactor(frontend): share one column-filter popover across Plans and RI #1315) and repeat column-filter-popover.test.ts:110 at integration level. Their comments ("Escape used to drop focus to ", "the way the History popover does") describe the code before refactor(frontend): share one column-filter popover across Plans and RI #1315. The outside-click test is still meaningful.
• Jest prints "A worker process has failed to exit gracefully" for the touched suites. Probably pre-existing; not traced.
  Attacked: In merge e7f304d, riexchange.ts is identical to main's parent 5fbedae. The branch's restoreFocus edits were dropped correctly, because main's shared lib already calls preventDefault on Escape, restores trigger focus and leaves focus alone on outside click. The test file keeps main's re-anchor/reshape tests and the afterEach close, and adds the branch's 3 tests. diff merge^2..merge is exactly the PR's 9 files, and merge-tree against current main is clean.
  Fresh review: chip-select focus order, activedescendant cleanup, mousedown preventDefault, no listener leaks, unique listbox ids. The skeleton MutationObserver arms only in activateSkeleton and disconnects on clear, and none of the 10 caller modules can leave aria-busy stuck. No innerHTML or inline styles. Scratch jest: 197/197 passed.

[IAmJSD]

Local verification at e7f304df merged onto current main (f5cb7ae7; merge-tree 22beaee7; 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 dc090cc2, main plus 17 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4097 passed, 4098 total), and the full Playwright suite (232 passed).
• CI is green on e7f304df (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 e7f304df.