fix: harden Therapy Compass recovery and error accessibility#758
fix: harden Therapy Compass recovery and error accessibility#758BigSimmo wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughChangesError accessibility
Therapy Compass recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TherapyCompassShell
participant TcProvider
participant useTherapyData
TherapyCompassShell->>TcProvider: invoke retryData
TcProvider->>useTherapyData: invoke retry
useTherapyData->>useTherapyData: clear cache and reset loading state
useTherapyData->>useTherapyData: increment attempt and refetch dataset
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@copilot resolve the merge conflicts in this pull request |
…audit-findings-and-merge # Conflicts: # src/components/therapy-compass/therapy-card.tsx
Head branch was pushed to by a user without write access
Resolved. There was a single conflict in |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@src/app/global-error.tsx`:
- Line 49: Restore a visible focus indicator on the error headings by replacing
the outline removal in src/app/global-error.tsx lines 49-49 with a visible
outline style, and update the focused heading in
src/components/route-error-boundary.tsx lines 59-61 to remove focus:outline-none
and provide an equivalent visible outline. Keep the existing heading focus
behavior unchanged.
In `@src/components/therapy-compass/data/use-therapy-data.ts`:
- Around line 41-45: Update the retry and promise rejection flow around retry
and cache handling so an older request only clears cache when it still owns the
cached promise; guard the cache reset by comparing the relevant
promise/reference before assigning null, preserving newer in-flight requests.
In `@src/components/therapy-compass/therapy-compass-page.tsx`:
- Around line 47-55: Update the error/retry flow around b.retryData so focus is
preserved when retry clears b.error and unmounts the Retry button. Move focus to
a stable loading or content container after retry begins, or keep the error
panel mounted with the button disabled and updated during loading; ensure
keyboard and screen-reader users retain a logical focus position throughout
recovery.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f54bdb20-30c0-4c23-96c5-7313b30ca1b6
📒 Files selected for processing (6)
src/app/global-error.tsxsrc/components/route-error-boundary.tsxsrc/components/therapy-compass/bindings.tsxsrc/components/therapy-compass/data/use-therapy-data.tssrc/components/therapy-compass/therapy-card.tsxsrc/components/therapy-compass/therapy-compass-page.tsx
| > | ||
| <h1 style={{ margin: "0 0 0.5rem", fontSize: "1.125rem", fontWeight: 600 }}>Something went wrong</h1> | ||
| <p style={{ margin: "0 0 1.25rem", fontSize: "0.875rem", lineHeight: 1.6, color: "#5b6069" }}> | ||
| <h1 ref={headingRef} tabIndex={-1} role="alert" style={{ margin: "0 0 0.5rem", fontSize: "1.125rem", fontWeight: 600, outline: "none" }}>Something went wrong</h1> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files 'src/app/global-error.tsx' 'src/components/route-error-boundary.tsx' 'src/**/error*.tsx' 'src/**/error*.ts' 'src/**/global-error.tsx'
printf '\n== outline global-error ==\n'
ast-grep outline src/app/global-error.tsx --view expanded || true
printf '\n== outline route-error-boundary ==\n'
ast-grep outline src/components/route-error-boundary.tsx --view expanded || true
printf '\n== relevant snippets ==\n'
sed -n '1,140p' src/app/global-error.tsx
printf '\n---\n'
sed -n '1,180p' src/components/route-error-boundary.tsxRepository: BigSimmo/Database
Length of output: 8716
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for focus styles in error boundaries ==\n'
rg -n "focus:outline-none|outline:\s*\"none\"|tabIndex=\{-1\}|role=\"alert\"|headingRef" src/app src/components src -g '!**/node_modules/**'
printf '\n== search for visible focus token usage ==\n'
rg -n "var\(--focus\)|focus-visible|focus:outline|outline-offset|outline-" src/app src/components src -g '!**/node_modules/**' | sed -n '1,200p'Repository: BigSimmo/Database
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in [Path('src/app/global-error.tsx'), Path('src/components/route-error-boundary.tsx')]:
print(f"\n## {p}")
text = p.read_text()
for needle in ['outline: "none"', 'focus:outline-none', 'tabIndex={-1}', 'role="alert"']:
print(needle, text.find(needle))
PYRepository: BigSimmo/Database
Length of output: 386
Restore a visible focus style on the error headings.
Both boundaries move focus to the <h1>, but outline: "none" / focus:outline-none removes the only visible focus indicator. Add a visible outline instead.
src/app/global-error.tsx#L49src/components/route-error-boundary.tsx#L59-L61
📍 Affects 2 files
src/app/global-error.tsx#L49-L49(this comment)src/components/route-error-boundary.tsx#L59-L61
🤖 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 `@src/app/global-error.tsx` at line 49, Restore a visible focus indicator on
the error headings by replacing the outline removal in src/app/global-error.tsx
lines 49-49 with a visible outline style, and update the focused heading in
src/components/route-error-boundary.tsx lines 59-61 to remove focus:outline-none
and provide an equivalent visible outline. Keep the existing heading focus
behavior unchanged.
| const retry = useCallback(() => { | ||
| cache = null; | ||
| setState({ data: null, loading: true, error: null }); | ||
| setAttempt((value) => value + 1); | ||
| }, []); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -P '\buseTherapyData\s*\(' srcRepository: BigSimmo/Database
Length of output: 375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== use-therapy-data.ts ==\n'
cat -n src/components/therapy-compass/data/use-therapy-data.ts | sed -n '1,240p'
printf '\n== bindings.tsx (relevant range) ==\n'
cat -n src/components/therapy-compass/bindings.tsx | sed -n '180,250p'
printf '\n== all useTherapyData references ==\n'
rg -n -P '\buseTherapyData\s*\(' srcRepository: BigSimmo/Database
Length of output: 7157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/components/therapy-compass/data/use-therapy-data.ts | sed -n '1,220p'Repository: BigSimmo/Database
Length of output: 2786
Guard cache before clearing it. retry can start a new load while a previous promise is still pending, and the older rejection handler currently does cache = null unconditionally. That can wipe out the newer in-flight promise and force an unnecessary refetch.
🤖 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 `@src/components/therapy-compass/data/use-therapy-data.ts` around lines 41 -
45, Update the retry and promise rejection flow around retry and cache handling
so an older request only clears cache when it still owns the cached promise;
guard the cache reset by comparing the relevant promise/reference before
assigning null, preserving newer in-flight requests.
| {b.error ? ( | ||
| <section role="alert" aria-live="assertive" style={s(`max-width:42rem;margin:40px auto;padding:24px;border:1px solid var(--danger);border-radius:16px;background:var(--danger-soft);`)}> | ||
| <h1 style={s(`margin:0 0 8px;color:var(--text-heading);font-size:20px;`)}>Therapy Compass could not load</h1> | ||
| <p style={s(`margin:0 0 16px;color:var(--text-muted);line-height:1.5;`)}>The therapy catalogue is unavailable. No results are being shown as a substitute.</p> | ||
| <button type="button" className="tc-btn" onClick={b.retryData} style={s(`padding:10px 14px;border:0;border-radius:8px;background:var(--clinical-accent);color:var(--clinical-accent-contrast);font-weight:650;`)}> | ||
| Retry | ||
| </button> | ||
| </section> | ||
| ) : ( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve focus when Retry starts loading.
Clicking Retry clears b.error, which immediately unmounts the focused button. Focus then falls back to the document/body, so keyboard and screen-reader users lose their place during recovery. Move focus to a stable loading/content container or keep this panel mounted and disable/update the button while the retry is in progress.
🤖 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 `@src/components/therapy-compass/therapy-compass-page.tsx` around lines 47 -
55, Update the error/retry flow around b.retryData so focus is preserved when
retry clears b.error and unmounts the Retry button. Move focus to a stable
loading or content container after retry begins, or keep the error panel mounted
with the button disabled and updated during loading; ensure keyboard and
screen-reader users retain a logical focus position throughout recovery.
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #2979 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Pull request was closed
Motivation
Description
src/components/therapy-compass/data/use-therapy-data.ts) and exposed aretryDatabinding through the provider (src/components/therapy-compass/bindings.tsx).src/components/therapy-compass/therapy-compass-page.tsx).src/components/therapy-compass/therapy-card.tsx).src/components/route-error-boundary.tsx,src/app/global-error.tsx).codex/design-audit-remediation-20260717and committed the changes with messagefix: harden therapy compass recovery and error access.Testing
git diff --cached --checkwhich passed before committing the changes.npm run test:focused -- --files <paths>but the repository wrapper refused focused execution and instructed running the full suite instead, so the focused run was skipped.npm run testfor the two targeted tests but the run failed becausenode_modulesis not present (missingvitestmodule), so unit tests could not be executed in this checkout.git commitsucceeded and the worktree is clean oncodex/design-audit-remediation-20260717; pushing and merging were not performed because this checkout has no Git remote configured and remote/merge actions require an operator to configure and authorize them.Codex Task
Summary by CodeRabbit