Fix accessible mobile table dialog semantics#495
Conversation
…ssibility-audit # Conflicts: # docs/branch-review-ledger.md
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughUpdated the mobile clinical table fullscreen dialog for keyboard focus trapping, Escape closure, expansion state semantics, and decorative icon accessibility. Expanded UI smoke coverage verifies these behaviors and records the review results. ChangesResponsive accessibility behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/ui-smoke.spec.ts (1)
1838-1840: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that Escape returns focus to the expand button.
The test covers dismissal and state reset but not the PR’s focus-return contract.
Proposed assertion
await page.keyboard.press("Escape"); await expect(surfaceDialog).toBeHidden(); await expect(expandButton).toHaveAttribute("aria-expanded", "false"); + await expect(expandButton).toBeFocused();As per coding guidelines, “add or update the smallest relevant test when behavior changes.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ui-smoke.spec.ts` around lines 1838 - 1840, Update the test around the Escape dismissal flow to also assert that focus returns to expandButton after surfaceDialog is hidden and aria-expanded is false. Add the smallest focused assertion using the existing expandButton locator, preserving the current dismissal and state-reset checks.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/ui-smoke.spec.ts`:
- Around line 1838-1840: Update the test around the Escape dismissal flow to
also assert that focus returns to expandButton after surfaceDialog is hidden and
aria-expanded is false. Add the smallest focused assertion using the existing
expandButton locator, preserving the current dismissal and state-reset checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68f4763d-c926-49da-aab0-444456979705
📒 Files selected for processing (3)
docs/branch-review-ledger.mdsrc/components/AccessibleTable.tsxtests/ui-smoke.spec.ts
Summary
aria-expandedon the real expand button and cover semantics/focus behavior at 390px, 768px, and 1440pxVerification
npm run verify:pr-local(not run; the narrower static, unit, and full UI gates below covered the changed surface without a production build)npm run verify:ui— 132/132 Chromium tests passednpm run verify:release(not run; includes provider-backed and release-only gates)git diff --checkpassedNo OpenAI, Supabase, hosted CI, or other provider-backed validation was run.
Clinical Governance Preflight
Notes