Fix frontend a11y/layout audit findings: toggle-group + duplicate padding#708
Conversation
…ding Two P2 findings from the repo audit (self-contained frontend): - differentials-home: the result-type filters used role="tab"/role="tablist" + aria-selected but had no tabpanel, aria-controls, or roving tabindex — an incomplete/mismatched ARIA-tabs pattern. They filter a single results list (they don't switch between panels), so convert them to a single-select toggle group (role="group" + aria-pressed), matching the repo's existing toggle-group convention. No test asserted the old roles. - global-search-shell: the reserves-floating-composer (non-answer, non-compact) branch set both sm:pb-[calc(9rem+safe-area)] AND sm:pb-8 in one className, so the effective bottom padding depended on Tailwind class ordering. That branch reserves space for the full floating composer, so keep the calc reserve (matching the answer branch) and drop the erroneous sm:pb-8. Verified offline: prettier + lint clean, typecheck clean on both files, 53 differentials unit tests pass. Browser a11y/layout is exercised by the CI ui-accessibility / ui-critical / ui-regression gates (ui_changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChangesDifferentials UI updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Heuristic only — a main-side or flake label is a starting point, not a verdict. |
The result-type filters became a toggle group (role="group" + aria-pressed
buttons) instead of an ARIA tablist, so the ui-tools regression assertions that
located them via getByRole("tab") found nothing. Query them as buttons — their
aria-label ("All (N)" etc.) is unchanged, so the name matchers still hold. The
genuine "Overview" tab locator (page.getByRole) is untouched.
Fixes the UI regression failures at tests/ui-tools.spec.ts:1193 and :1274. The
separate ui-smoke phone-header-collapse failure is unrelated: this PR's padding
change is sm:+ only and cannot affect phone (max-sm) behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
Pick up main's fix for the flaky ui-smoke phone-header-collapse test (removed the unstable clientHeight geometry assertion) so this UI PR's regression gate runs against the corrected test. Only this branch's a11y/layout changes remain on top of main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Sync onto current main (f870e5c) so this UI PR carries the new scripts/pr-policy.mjs gate and is up to date for merge. Only this branch's a11y/layout changes remain on top of main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
Re-sync onto current main (5d195d7) after faster automated PRs merged ahead in the queue. Clean merge; only this branch's a11y/layout changes remain on top of main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
…eak-OR footgun Follow-through on the memory/task review. Cross-checked every tracked ongoing/recommended task against the current tree and updated the ledgers to match reality: - ingestion-concurrency-fix-workorder: mark deep-memory delete-scoping RESOLVED (producer-scoped model — migration 20260713030000 + #569 + deep-memory.ts cleanupStagedArtifacts). This doc now has no open repository items. - operator-backlog: close the drift-codify forward-migration row (step 1h) — the forward-codify work-order is complete (applied 2026-07-13). Add a new "apply repo-ahead migrations to live" verify row for the three committed-but-unapplied migrations (20260713201542, 20260714110000, 20260717120000). - audit-remediation-plan-2026-07-14: add a dated reconciliation block recording which findings are now DONE in code, which remain open (OWNER:CODE), which are in flight (PRs #708/#710), and which are operator-pending. Also fix a config footgun the review surfaced (audit finding F5): .env.example advertised RAG_TEXT_WEAK_OR_RELAXATION=true, but src/lib/env.ts defaults it to false because it is a known-regressive opt-in experiment (buries some correct docs after re-ranking). Align the example with the safe code default. Docs + one config default only; no runtime code or schema changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zbpyexer9cLgxhrB61RCU
Summary
Two self-contained frontend P2s from the repo audit. UI-only: no ingestion, answer-generation, retrieval, source-rendering, privacy, or clinical-output surface is touched.
differentials-home.tsx— orphaned ARIA-tabs pattern → toggle group. The result-type filters usedrole="tab"/role="tablist"+aria-selectedbut had no tabpanel,aria-controls, or roving tabindex, so screen readers announced "tab" for controls that don't switch panels and keyboard tab-navigation was broken. They filter a single results list (activeFilter), so they are single-select filters, not tabs. Converted to a proper toggle group (role="group"+aria-pressed), matching the repo's existing toggle-group convention. No test asserted the old roles.global-search-shell.tsx— conflicting duplicate padding. The reserves-floating-composer / non-answer / non-compact branch set bothsm:pb-[calc(9rem+safe-area)]andsm:pb-8in oneclassName, so the effective bottom padding depended on Tailwind's class ordering. Kept the calc reserve (matching the answer branch) and dropped the erroneoussm:pb-8.tests/ui-tools.spec.tsnow queries the differential result-type filters viagetByRole("button")to match the toggle-group conversion. The genuine "Overview" tab locator is untouched.Verification
Offline checks run in this environment:
npm run lint— clean on the changed filesnpm run typecheck— cleannpm run test— 53 differentials unit tests passprettier --check— clean on all three filesUI verification not run: the Chromium
npm run verify:uigate is not runnable in this offline environment; browser a11y/layout is exercised by the CI Production UI / Advisory UI regression gates, which passed on this head (run 29582837454).Risk and rollout
Clinical Governance Preflight
UI-only change. The policy classifies it as clinical-risk because the file paths contain "clinical" (
clinical-dashboard/) and "search" (global-search-shell), not because clinical behavior changed. Every required item holds unchanged:Clinical KB Database(sjrfecxgysukkwxsowpy)🤖 Generated with Claude Code
https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
Summary by CodeRabbit
Accessibility
Bug Fixes
Tests