fix(a11y): make unavailable placeholder buttons reachable by keyboard by BigSimmo · Pull Request #1778 · BigSimmo/Database

BigSimmo · 2026-08-09T07:49:43Z

Summary

The defect. The disabled-placeholder pattern put disabled on the button and attached the reason it was unavailable via aria-describedby to an sr-only span, plus a title. But disabled removes the tab stop, so a keyboard user — and a screen-reader user moving by Tab rather than by virtual cursor — could never land on the control. The explanation was written and unreachable.
Converted 25 sites across 13 components to aria-disabled="true" plus a shared inert handler (ignoreUnavailableActivation, new in ui-primitives.tsx), keeping the existing title and the aria-describedby/sr-only reason. Four of them carried disabled and aria-disabled together — belt and braces that never helped, because the native attribute still won on focus.
Left native disabled on transient states, which is correct there: a request in flight, a pager at its last page, a form action awaiting validity. Making those focusable would be a regression, not a fix. The full list is in the doc so the next pass does not "fix" them.
Styling moved with the attribute. Once the native attribute is gone, disabled: variant classes stop applying and the control becomes hoverable. controlDisabled in ui-primitives.tsx grew its aria-disabled: half, and the therapy recipes in therapy-compass/controls.ts switched hover:enabled: to hover:not-aria-disabled:enabled: so a converted control does not light up under the cursor. Verified in the built CSS, not just in review: 13 :hover:not([aria-disabled=true]) selectors and 10 [aria-disabled=true] rules are emitted by npm run build.
Contract updated with the code, rather than left saying the opposite — docs/wiring-conventions.md previously argued that native disabled was deliberately the default here (see "Deviation from a stated position" below).
New lint gate redundantDisabledPair in eslint-rules/require-button-wiring.mjs, failing on the two attributes together on any <button>. A statically-off side (disabled={false} beside aria-disabled="true") still passes, since that is a real way to spell a conditional placeholder. This settles ledger #291, which tracked the repo pinning that shape two contradictory ways.
New test tests/require-button-wiring.test.ts pins that the lint rule actually fires in both directions. A green npm run lint cannot distinguish a clean repo from a rule that matches nothing, and this rule had no test at all before.
New DOM assertion in tests/favourites-hub-unavailable-controls.dom.test.tsx: it tabs onto a converted placeholder, asserts it takes focus, asserts the accessible description is what the reader gets instead, and asserts that activating it by keyboard (Enter and Space) and by pointer does nothing. That assertion is the whole point of the change and nothing pinned it before.

Sites converted

Component	Reason carried
`calculators/search-page.tsx`	Advanced filters — coming soon
`clinical-dashboard/account-setup-dialog.tsx`	Provider sign-in unavailable
`clinical-dashboard/auth-panel.tsx`	Apple sign-in unavailable
`clinical-dashboard/evidence-panels.tsx`	Add to favourites — coming soon
`clinical-dashboard/favourites-command-library-page.tsx` ×3	Coming soon
`clinical-dashboard/favourites-hub.tsx` ×3	Sort / add / new set coming soon
`clinical-dashboard/settings-dialog.tsx`	Provider sign-in unavailable
`clinical-dashboard/visual-evidence.tsx`	Add to favourites — coming soon
`forms/form-detail-page.tsx`	No official source URL recorded for this form
`forms/forms-search-results-page.tsx` ×4	Filters / pathway coming soon
`services/service-detail-page.tsx`	No contact number listed for this service
`therapy-compass/screens/brief-screen.tsx` ×2, `screens/recommend-screen.tsx` ×2, `therapy-card.tsx` ×2	No patient sheet / handout for this record; favourite saving not available yet

therapy-compass/workspace.tsx went the other way: its retry-while-loading button carried both attributes, and as a transient state it keeps native disabled alone. It was found by the new lint rule, not by my own audit — it has neither a title nor an aria-describedby, so it sat outside the scope this change was measured against.

Deviation from a stated position — please read before approving

docs/wiring-conventions.md did not merely happen to use native disabled; it argued for it, in a section headed "Native disabled, not aria-disabled, is the default here", and tests/favourites-hub-unavailable-controls.dom.test.tsx pinned toBeDisabled() and not.toHaveAttribute("aria-disabled") together. This PR overturns that. The old argument was half right — a screen-reader user browsing with a virtual cursor genuinely can reach a disabled button and hear its description, which is what that section asserted — but it does not hold for keyboard-only users, or for screen-reader users navigating by Tab, and it was silently doing the work of justifying the pairing that #291 had already flagged as unreconciled. Five tests that pinned the old shape were updated rather than deleted, each with the reason inline.

Deliberately not converted (follow-ups, no ledger row — `docs/outstanding-issues.md` is owned by another session this cycle)

differentials/differential-presentation-workflow-page.tsx — four "not available in this comparison view" placeholders plus the Compact/Detailed density pair. They qualify, but that page is scheduled for a rewrite and tests/mobile-interaction-regressions.test.ts still pins the density pair as native-only. Convert with the rewrite.
document-viewer/document-image-filmstrip.tsx — out of scope; a parallel session owns that subtree.
DocumentViewer.tsx summarize action — genuinely ambiguous: summarizeTitle mixes a persistent auth reason ("Sign in before answering from this document") with a transient loading state behind one flag. It needs splitting before it can be classified, so I left it rather than guess.
search-pins-menu.tsx "New pin" at the pin cap ("Maximum N pins reached") — ambiguous. It states a reason, but it is a capacity state the user clears by deleting a pin, which reads as transient. Left native.
calculator-mockups/search-page-mockup.tsx — design scratch, exempt from the wiring gates.

Verification

npm run verify:pr-local — 22 of 24 gates completed; test reported the pre-existing failure below, which stopped build and check:rag:fixtures being reached, so both were then run directly and passed.
npm run lint — exit 0. Re-run uncached (rm -rf node_modules/.cache/eslint) after the new rule landed: the cached run had reused a stale entry and missed workspace.tsx; the pre-push guard, which lints the pushed commit in a clean worktree, caught it. Uncached run is eslint exit=0.
npm run typecheck — exit 0.
npm run test — Test Files 546 passed (547), Tests 5878 passed | 4 skipped (5883), 1 failed.
npm run build — exit 0; built CSS greped for the new variants (13 not([aria-disabled=true]) selectors, 10 [aria-disabled=true] rules).
npm run check:rag:fixtures — Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites).
npm run format run and committed before pushing; npx prettier --check . → All matched files use Prettier code style!
npm run verify:ui — UI verification not run: this remote container has no Playwright browser provisioned, and verify:ui is a hosted-browser gate. Chromium journeys should be treated as owed for this change: the conversion alters focus order and hover behaviour on real pages. CI Production UI covers it on this PR.

The one failing test is pre-existing and unrelated. tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file exists chmods a .git directory to 000 and asserts the hook's write fails; this container runs as uid 0, where chmod does not restrict root, so the marker is created and the assertion inverts. My diff touches no file under tests/pr-handoff-stop.test.ts, .claude/hooks/, or scripts/ (git diff --stat HEAD for those paths is empty), and the test fails identically in isolation.

Note on environment: the container shipped Node 24.13.0, below the 24.15.0 floor jsdom@30 declares, so npm ci failed EBADENGINE. I installed Node 24.18.1 locally and ran every gate on it rather than passing --engine-strict=false. Open PR #1771 addresses that floor.

Risk and rollout

Risk: Low-to-moderate, and it is presentational rather than clinical. The behavioural surface is focus order and click handling on 25 controls that were already inert, plus the shared controlDisabled recipe and the therapy control recipes, which are used well beyond the converted sites. The realistic failure mode is visual — a converted control that reads as available because a disabled: variant no longer applies — which is why the built CSS was checked for the emitted aria-disabled: and hover:not-aria-disabled: rules rather than trusting review.
Rollback: git revert of the single commit restores both the markup and the contract; no data, schema, or configuration is involved.
Provider or production effects: None. No provider-backed gate was run and no live service was contacted.

Clinical Governance Preflight

Source-backed claims still require linked source verification before clinical use — no retrieval, ranking, citation, or answer-rendering code is touched; the converted controls are placeholders that perform no action, and the source/citation rows beside them (SourceActionRow, the Copy actions in evidence-panels.tsx and visual-evidence.tsx) are unchanged.
No patient-identifiable document workflow was introduced or expanded without explicit governance approval — no document-access behavior changes; no upload, download, or document-scoping path is in the diff.
Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy) — no Supabase configuration, migration, or environment value is touched.
Service-role keys and private document access remain server-only — no server code, API route, or credential handling is in the diff; every changed source file is a client component, a shared style recipe, a lint rule, or a test.
Demo/synthetic content remains clearly separated from real clinical sources — unchanged; demo-mode detection and the synthetic corpus are untouched.
Source metadata, review status, and outdated/unknown-source behavior remain conservative — unchanged. The two converted controls that sit closest to this surface (form-detail-page.tsx, service-detail-page.tsx) are precisely the "this record has no such data" case, and the conversion makes that absence more discoverable rather than less: the reason is now reachable by keyboard instead of only by pointer hover.
Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed — no decision-support behavior changed. This is an accessibility fix to controls that were already inert; no clinical output, recommendation, or ranking is affected.

Notes

ignoreUnavailableActivation calls preventDefault() and stopPropagation() deliberately: a natively disabled button fires no click at all, so nothing bubbled to a clickable ancestor. Without stopPropagation a placeholder inside a clickable row would begin activating the row — a behaviour change the conversion must not introduce.
The new lint rule checks the redundant pair on any <button>, not just type="button", since the pairing is wrong regardless of type. The existing unwired check keeps its narrower type="button" scope and its spread escape hatch untouched.
not-aria-disabled: had no prior use in this repo. I confirmed Tailwind 4.3.1 compiles it (to :not([aria-disabled="true"])) before adopting it, and the production build confirms it end to end.

🤖 Generated with Claude Code

https://claude.ai/code/session_018hT4qToXT5nr3hV4ENqRgS

Generated by Claude Code

The disabled-placeholder pattern put `disabled` on the button and attached the reason it was unavailable via `aria-describedby` to an `sr-only` span, plus a `title`. But `disabled` removes the tab stop, so a keyboard user — and a screen-reader user moving by Tab rather than by virtual cursor — could never land on the control. The explanation was written and unreachable; the control simply vanished for them. Convert the controls that are unavailable for a *stated* reason (feature not built yet, or this record lacks the data) to `aria-disabled="true"` plus a shared inert handler, keeping the title and the described-by reason. 24 sites across 12 components, including the four that carried `disabled` and `aria-disabled` together — belt and braces that never helped, because the native attribute still won on focus. Transiently inert controls keep native `disabled`, which is correct there: a request in flight, a pager at its last page, a form action awaiting validity. Those are listed in the wiring doc so the next pass does not "fix" them. Styling had to move with the attribute: `disabled:` variants stop applying once the native attribute is gone, and the control becomes hoverable. The `controlDisabled` recipe grew its `aria-disabled:` half, and the therapy recipes switched `hover:enabled:` to `hover:not-aria-disabled:enabled:` so a converted control does not light up under the cursor. Contract updated with the code, rather than left saying the opposite: - docs/wiring-conventions.md replaces the "native disabled is the default here" argument with the stated-reason vs transient split, and settles the unreconciled pairing tracked as ledger #291. - require-button-wiring gains `redundantDisabledPair`, failing on the two attributes together on any <button>; a statically-off side still passes. - tests/require-button-wiring.test.ts pins that the rule fires in both directions, since a green lint run cannot distinguish a clean repo from a rule that matches nothing. - favourites-hub-unavailable-controls tabs onto a converted placeholder, asserts it takes focus, asserts the description is what the reader gets, and asserts keyboard and pointer activation do nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hT4qToXT5nr3hV4ENqRgS

coderabbitai · 2026-08-09T07:49:52Z

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 28 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 94d005e6-b708-4bb6-899d-fe03bf031ec4

📥 Commits

Reviewing files that changed from the base of the PR and between 5229e30 and 958ed6c.

📒 Files selected for processing (32)

AGENTS.md
CLAUDE.md
docs/branch-review-ledger.md
docs/wiring-conventions.md
eslint-rules/require-button-wiring.mjs
scripts/check-design-system-contract.mjs
src/components/calculators/search-page.tsx
src/components/clinical-dashboard/account-setup-dialog.tsx
src/components/clinical-dashboard/auth-panel.tsx
src/components/clinical-dashboard/evidence-panels.tsx
src/components/clinical-dashboard/favourites-command-library-page.tsx
src/components/clinical-dashboard/favourites-hub.tsx
src/components/clinical-dashboard/master-search-header.tsx
src/components/clinical-dashboard/settings-dialog.tsx
src/components/clinical-dashboard/visual-evidence.tsx
src/components/forms/form-detail-page.tsx
src/components/forms/forms-search-results-page.tsx
src/components/in-page-nav/in-page-nav-classes.ts
src/components/services/service-detail-page.tsx
src/components/therapy-compass/controls.ts
src/components/therapy-compass/screens/brief-screen.tsx
src/components/therapy-compass/screens/recommend-screen.tsx
src/components/therapy-compass/therapy-card.tsx
src/components/therapy-compass/workspace.tsx
src/components/ui-primitives.tsx
tests/auth-panel-product-truth.dom.test.tsx
tests/favourites-auth-gate.dom.test.tsx
tests/favourites-hub-unavailable-controls.dom.test.tsx
tests/mobile-interaction-regressions.test.ts
tests/require-button-wiring.test.ts
tests/settings-dialog-actions.dom.test.tsx
tests/therapy-compass-responsive-contract.test.ts

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

supabase · 2026-08-09T07:50:17Z

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hT4qToXT5nr3hV4ENqRgS

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 722abdb780

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

BigSimmo · 2026-08-09T07:56:13Z

@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/disabled-button-accessibility-piclvr at starting commit fb36de3; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/disabled-button-accessibility-piclvr, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.

chatgpt-codex-connector · 2026-08-09T08:01:52Z

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: fb36de38e0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-08-09T08:03:45Z

CI triage

CI failed on this PR. Automated classification of the 3 failed job(s):

Static PR checks — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
Production UI (1) — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
PR required — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #9463 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

…esolve GitHub DIRTY/staleness) Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

BigSimmo · 2026-08-09T08:12:13Z

@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker.

If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop.

Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them.

Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head.

Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol.

Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it.

Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted.

Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval.

Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed.

One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1.

Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me.

cursor · 2026-08-09T08:12:16Z

You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor

BigSimmo · 2026-08-09T08:14:01Z