Skip to content

fix(a11y): stop OS Reduce Motion from freezing and hiding answer progress - #2046

Merged
BigSimmo merged 18 commits into
mainfrom
claude/phone-pwa-animation-bug-suooj4
Aug 17, 2026
Merged

fix(a11y): stop OS Reduce Motion from freezing and hiding answer progress#2046
BigSimmo merged 18 commits into
mainfrom
claude/phone-pwa-animation-bug-suooj4

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

  • Fixes the reported defect: OS Reduce Motion was freezing and hiding the answer-loading panel. The panel was reported dead on a physical iPhone in both Safari and the installed PWA. Three prior PRs (Fix ECG SVG repaint on Mobile WebKit (remove paint containment; add regression test) #1974, Switch answer-activity ECG animation to opacity pulse for WebKit reliability and update tests #1989, Make answer loading pulse reliable on iPhone and PWA #1995) diagnosed it as a Mobile WebKit repaint bug and all three failed on the device. The real cause is this app's own reduced-motion CSS: with iOS Reduce Motion on, the universal suppression rule in globals.css froze every animation on the page — including the step spinner — and the ECG trace was additionally set to opacity: 0, so the only progress indicator disappeared entirely. Suppressing motion now stops the animation but leaves a legible static trace instead of deleting it.
  • Adds a tri-state Motion preference (System / Reduced / Full). Full is an explicit opt-in that overrides an OS Reduce Motion request. iOS Reduce Motion is commonly enabled to stop app-launch zoom rather than out of vestibular sensitivity, and there was previously no in-app way to get the feedback back. The default stays system, so no existing user silently gains motion, and the existing reduced opt-out still wins over an OS with no preference. The motion-reduce / motion-safe Tailwind variants are redefined via @custom-variant so the override reaches all 164 call sites without editing them.
  • Replaces the opacity pulse with a travelling sweep. The previous whole-line fade (opacity 0.2 → 1.0 over 1.8s, ease-in-out) technically ran but was too subtle on a 2px phone hairline to read as motion. It is now a continuously scrolling ECG strip — two identical copies in a 200%-wide span translated by exactly -50% for a seamless loop — driven only by transform: translateX, reusing the compositor recipe .animate-skeleton-shimmer::after already relies on in this app.
  • Closes the verification hole that let three "fixes" ship green. playwright.config.ts applies reducedMotion: "reduce" suite-wide while the one spec asserting the animation opts out to no-preference first, so the default user configuration was never exercised. The only WebKit project is Desktop Safari; there was no mobile-WebKit coverage at all. New tests/ui-phone-motion.spec.ts covers the exact reported configuration.

Verification

  • npm run verify:pr-local

Exit 0. Summary line: completed: check:runtime, check:installed-lock-parity, format:changed, lint, typecheck, test, build, eval:rag:offline, check:medication-interactions, check:medication-lexicon-report / failed: (none) / not reached: (none). Unit suite within it: Test Files 640 passed (640) / Tests 6832 passed | 4 skipped (6836).

npm run verify:cheap was used as the iteration gate and also exited 0 with the same 640/6832 result.

  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed

UI verification not run: the container has Chromium revision 1194 but this Playwright pins 1234, so check:playwright-browser-revision fails not-installed and the repo's runner refuses to start. This is the known Playwright/image drift tracked as #255, and repo policy is to delegate browser proof to CI Production UI rather than force a mismatched binary. The two directly affected specs were still executed against the available Chromium as a labelled diagnostic, not as a gate claim:

  • tests/ui-phone-motion.spec.ts3 passed (4.8s)
  • tests/answer-progress-ui-smoke.spec.ts4 passed (20.9s)

Computed style was also probed directly across all four motion states on a phone viewport, which is the evidence that confirms the diagnosis:

OS setting In-app Motion sweep animation sweep opacity step spinner
reduce system none 0.55 frozen
reduce full answer-ecg-scroll 3.2s 1 spin 1s
no-preference reduced none 0.55 frozen
no-preference system answer-ecg-scroll 3.2s 1 spin 1s

Before this change, row 1 produced opacity: 0 — the blank, frozen panel in the bug report — and row 2 was unreachable.

  • npm run verify:release before release or handoff confidence claims

Not run: no release or handoff confidence is claimed here, and it is provider-backed.

  • npm run eval:retrieval:quality (must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed

Not applicable: no retrieval, ranking, selection, chunking, or scoring code is touched. classifyPullRequestFiles returns ragRanking: false for this diff.

  • npm run eval:rag -- --limit 15 + npm run eval:quality -- --rag-only when answer generation, the synthesis prompt, or answer post-processing changed

Not applicable: answer generation is untouched. This change affects only how the progress panel renders while an answer is being produced. eval:rag:offline ran inside verify:pr-local and passed.

  • npm run check:production-readiness when clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changed

Exit 0 with 2 PASS / 5 WARN / 2 FAIL. Both FAIL entries are the documented offline provider capability gap in this container (Missing server environment variables: NEXT_PUBLIC_SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY and Missing OPENAI_API_KEY), not a regression introduced by this diff.

  • npm run check:deployment-readiness when deployment startup, hosting, or rollout behavior changed

Not applicable: no deployment startup, hosting, or rollout behavior is touched.

Risk and rollout

  • Risk: Low-to-moderate, and concentrated in the cascade rather than the feature. The reduced-motion suppression rules and the motion-reduce / motion-safe variants are now scoped by html:not([data-motion="full"]), which raises their specificity by one attribute selector across every call site. A motion-reduce: utility that previously lost a specificity tie could now win; since those utilities exist to suppress motion, winning is the intended direction, but it is a repo-wide selector change rather than a local one. The compiled output was inspected directly to confirm both @custom-variant branches emit correctly and that the --animate-answer-ecg theme token and @keyframes answer-ecg-scroll survive Tailwind's tree-shaking. The animation redesign itself is contained to one component and its CSS. No clinical content, retrieval, ranking, answer text, or document access is touched.
  • Rollback: Revert the single commit. The Motion preference is stored in the existing preferences jsonb column with no migration and no DB enum, so a revert leaves any stored "full" value to be coerced back to "system" by normalizePreferences, which is already covered by a test. Nothing else persists.
  • Provider or production effects: None / describe the explicitly authorized effect — None. No provider-backed command was run. check:production-readiness is offline and reported its FAIL entries as the expected missing-credential capability gap.

Clinical Governance Preflight

Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

This diff changes only progress-indicator presentation and a user motion preference. It does not alter answer generation, citation rendering, source governance, retrieval, or document access, so none of the clinical behaviours above move.

Notes

  • Physical-device acceptance is still owed and this PR does not close it. Playwright's WebKit is not the iOS engine, and the executed browser evidence above ran on Chromium 1194. The reported surface is a physical iPhone in Safari and as an installed PWA. Confirmation there — with Motion set to Full — is the acceptance step, and it is exactly the gap that let three previous PRs be declared fixed while the phone stayed broken.
  • tests/ui-phone-motion.spec.ts had to be registered in three places that keep duplicate copies of the production spec allow-list: playwright.config.ts (testMatch and productionSpecPattern) and scripts/playwright-pr-shards.mjs (productionSpecFilePattern plus a shard-membership row). Its shard timing is a local measurement (~5s) pending a hosted timing refresh.
  • Two existing specs were updated rather than deleted because the Motion row became a three-option segmented control: tests/settings-inert-preferences.dom.test.tsx now expects settings-row-motion, and tests/ui-smoke.spec.ts moves Motion into the existing radiogroup geometry loop and re-points its 48px switch tap-target assertion at settings-row-recent-searches-on-home, the nearest remaining boolean row.
  • Follow-ups worth capturing in /issues: the suite-wide reducedMotion: "reduce" in playwright.config.ts means no gate reflects default user configuration; there is still no mobile-WebKit or display-mode: standalone Playwright project; and --spring-tight / --spring-bouncy / --spring-gentle in globals.css are dead tokens with zero var() and zero utility usage.

Generated by Claude Code

claude added 2 commits August 17, 2026 14:24
…ress

The answer-loading panel was reported dead on a physical iPhone in both
Safari and the installed PWA. Three prior PRs (#1974, #1989, #1995)
diagnosed it as a Mobile WebKit repaint bug and all three failed on the
device. The cause was this app's own reduced-motion CSS: with iOS Reduce
Motion on, the universal suppression rule froze every animation on the
page (including the step spinner) and the ECG trace was additionally set
to `opacity: 0`, so the only progress indicator disappeared entirely.

Every gate stayed green because playwright.config.ts applies
`reducedMotion: "reduce"` suite-wide while the one spec asserting the
animation opts out to "no-preference" first, so the default user
configuration was never exercised. The only WebKit project is Desktop
Safari; there was no mobile-WebKit coverage at all.

Three changes:

1. Suppressing motion no longer deletes the indicator. The reduced-motion
   rules stop the animation but leave a legible static trace instead of
   `opacity: 0`.

2. New tri-state Motion preference (System / Reduced / Full). "Full" is an
   explicit opt-in that overrides an OS Reduce Motion request, because iOS
   Reduce Motion is commonly enabled for app-launch zoom rather than
   vestibular sensitivity and there was no in-app way to get the feedback
   back. Default stays "system", so nobody silently gains motion. The
   `motion-reduce`/`motion-safe` Tailwind variants are redefined via
   @custom-variant so the override reaches all 164 call sites.

3. The pulse becomes a travelling sweep. The previous whole-line opacity
   fade (0.2 -> 1.0 over 1.8s) technically ran but was too subtle on a
   phone hairline to read as motion. It is now a continuously scrolling
   ECG strip driven only by `transform: translateX`, reusing the
   compositor recipe `.animate-skeleton-shimmer::after` already relies on.

Adds tests/ui-phone-motion.spec.ts covering the exact reported
configuration (phone + OS Reduce Motion), which nothing covered before.

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

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

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: 15 minutes

Limit details: You’ve used all 1 included review currently available under your plan. You completed 95 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour.

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: 43978a63-647d-4639-a939-c8007f948dcb

📥 Commits

Reviewing files that changed from the base of the PR and between 66979df and 16d7cb0.

📒 Files selected for processing (21)
  • docs/branch-review-records/b3baa45bd8bd11f3109359b91166246e208af8ee8a80c77c6f19cea9b2439b80.record.md
  • docs/outstanding-issues-inbox/5cc39bc6-4cf4-4a96-9ce7-a0ee46020b90.json
  • docs/outstanding-issues-inbox/c68dae81-559c-45ad-af70-f1b0334a49c3.json
  • docs/outstanding-issues-inbox/e790f80b-efb3-4684-8195-77ff872ad014.json
  • docs/outstanding-issues-inbox/e7f18a92-046d-4fb7-98be-d6aca938657d.json
  • playwright.config.ts
  • scripts/playwright-pr-shards.mjs
  • src/app/api/account/preferences/route.ts
  • src/app/ckb-v2-tokens.css
  • src/app/globals.css
  • src/app/layout.tsx
  • src/components/clinical-dashboard/answer-status.tsx
  • src/components/clinical-dashboard/settings-dialog.tsx
  • src/components/clinical-dashboard/use-app-preferences.ts
  • src/lib/account-preferences.ts
  • tests/answer-activity-trace-css.test.ts
  • tests/answer-progress-ui-smoke.spec.ts
  • tests/app-preferences.test.ts
  • tests/settings-inert-preferences.dom.test.tsx
  • tests/ui-phone-motion.spec.ts
  • tests/ui-smoke.spec.ts

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

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@supabase

supabase Bot commented Aug 17, 2026

Copy link
Copy Markdown

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 ↗︎.

@gitguardian

gitguardian Bot commented Aug 17, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
36221231 Triggered Generic High Entropy Secret e9e501d tests/rag-adversarial-fixtures.test.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

claude and others added 4 commits August 17, 2026 15:38
Four inbox requests from the PR #2046 investigation. They travel with
their owning product PR rather than a dedicated ledger-only branch, per
the AGENTS.md PR-bundling rule.

- P2 issue: playwright.config.ts applies reducedMotion:"reduce" suite-wide,
  so no gate reflects the default user configuration. That inversion is why
  three PRs shipped green against a broken phone.
- P2 task: no mobile-WebKit or display-mode:standalone Playwright project;
  phone coverage is a narrow viewport on desktop engines, while the only
  phone-AND-standalone CSS in the repo goes untested.
- P3 task: physical iPhone acceptance still owed for this fix, in Safari
  and the installed PWA with Motion set to Full.
- P3 rec: three dead spring easing tokens in globals.css.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYndHWrYJzirxbBvt68Tmx
…uooj4' into claude/phone-pwa-animation-bug-suooj4
…uooj4' into claude/phone-pwa-animation-bug-suooj4

Copy link
Copy Markdown
Owner Author

CI status: one red check, and it is not this diff

Every required check is green — PR required success, and mergeable_state is unstable rather than blocked, which reflects a failing non-required check only.

The single failure is GitGuardian Security Checks: 1 secret uncovered, a Generic High Entropy Secret at tests/rag-adversarial-fixtures.test.ts line 80.

Why I am not fixing it

It is not in this PR's changes. That file was introduced by #2036 (f5b0932, packet S4/B0), which is already on main. This branch's commits never touch it — it reached this PR only through the main sync merge (e9e501d), which is the commit GitGuardian attributes the finding to.

It is pre-existing and was already merged past. #2036 itself completed with GitGuardian Security Checks: failure and PR required: success, and was merged to main in that state. So this check is red on main's content, and will go red on every PR that merges main forward until it is triaged at source.

It reads as a false positive. The flagged literal is the synthetic canary token CANARY-PHI-MRN-1, which exists precisely to prove the validator rejects digit-bearing tokens. The file carries a comment saying the scanner flagged that line and explaining why the literal has to stay. The repo's own secret gate, Gitleaks, passes on both this PR and #2036 — two scanners disagreeing, with the in-repo one clean. There is no credential here to revoke or rotate.

Editing #2036's test to satisfy the scanner would weaken a deliberate contract in someone else's just-merged packet, and is outside this PR's scope, so I have left it alone.

Suggested remediation (operator action)

Triage occurrence 36221231 in the GitGuardian dashboard as a false positive, or add an ignore rule for synthetic canary fixtures under scripts/fixtures/ and tests/rag-adversarial-*. That is a provider-side change I am not authorised to make. Until then this check stays red repo-wide, not just here.

Reminder on what this PR still does not prove

Physical iPhone acceptance is still owed: Safari and the installed PWA, with Settings → Motion set to Full. All browser evidence here is Chromium; Playwright's WebKit is not the iOS engine. That unverified step is exactly what let #1974, #1989 and #1995 each be declared fixed while the device stayed broken.


Generated by Claude Code

@BigSimmo
BigSimmo enabled auto-merge (squash) August 17, 2026 17:23

Copy link
Copy Markdown
Owner Author

Unit coverage failed — root cause is a pre-existing theme-timer race, not this diff

First real (non-cancelled) job failure on this PR. Every test passed; the job exits non-zero on a single unhandled async error.

⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯
Vitest caught 1 unhandled error during the test run.

ReferenceError: document is not defined
 ❯ Timeout._onTimeout src/components/clinical-dashboard/use-theme.ts:93:7
 ❯ listOnTimeout node:internal/timers:635:17

This error originated in "tests/sidebar-production.dom.test.tsx"

Test Files  648 passed (648)
Tests       6970 passed (6970)
Errors      1 error

Mechanism

applyResolvedTheme in src/components/clinical-dashboard/use-theme.ts schedules an unguarded 200 ms timer whenever the resolved theme actually changes:

document.documentElement.classList.add("theme-transitioning");
document.documentElement.classList.toggle("dark", willBeDark);
syncThemeColorMetadata(theme);
window.setTimeout(() => {
  document.documentElement.classList.remove("theme-transitioning");   // ← line 93
}, 200);

tests/sidebar-production.dom.test.tsx:114"switches Light, Dark, and Auto through the established theme owner with keyboard focus return" — drives exactly that transition. When the test file completes in under 200 ms, jsdom tears the environment down first and the pending callback throws document is not defined.

It is a genuine race, not a threshold breach: nothing about coverage percentages failed.

Why it is not attributable to this PR

  • Neither use-theme.ts nor sidebar-production.dom.test.tsx is touched by any commit on this branch; both are on main, arriving with the sidebar work (feat(sidebar): add personalised clinical navigation #2026).
  • This diff changes motion CSS, the Motion preference, and the answer-progress ECG strip. It never enters the theme-resolution path, and applyPreferenceSideEffects writes data-density/data-motion, not the dark class.
  • It surfaced here only because the three earlier CI runs on this PR were all cancelled by branch syncs before the coverage job ever ran. This is simply the first time that job completed on this PR.
  • It is timing-dependent, so it will flake for any PR — coverage instrumentation slows the run enough to widen the window. Not in tests/flake-ledger.json (which is Playwright-only and currently empty).

Suggested fix (someone owning that file)

Guard the callback, or clear the timer on teardown:

window.setTimeout(() => {
  if (typeof document === "undefined") return;
  document.documentElement.classList.remove("theme-transitioning");
}, 200);

I have not made this change: it is outside this PR's scope, and auto-merge is armed on this branch, so automation must treat it as mutation-frozen rather than push to it.


Generated by Claude Code

@BigSimmo
BigSimmo merged commit 9b79c72 into main Aug 17, 2026
25 of 26 checks passed
@BigSimmo
BigSimmo deleted the claude/phone-pwa-animation-bug-suooj4 branch August 17, 2026 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants