Fix Safety Plan phone header safe-area layout - #1711
Conversation
Own the OS top inset on the standalone tool header and restack the phone chrome so the back control sits below the status bar. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 46 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe changes update search-surface dismissal and focus handling. They also restructure the safety-plan header for safe-area-aware responsive layout and add regression coverage for both behaviors. ChangesSearch surface behavior
Safety-plan header layout
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
37f958d to
65d165b
Compare
|
@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. |
|
PR #1711 — tip Worked this PR only (Safety Plan phone safe-area). No product code fix this pass. Snapshot
Review
Verification
Ledger / comment
Merge left to you once post-sync   |
Sync behind-but-clean tip with origin/main (#1712 design-system COMPONENTS §0.4). Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
PR #1711 review-and-fix summaryTip: Snapshot
Findings
Sync
Local gates (decisive lines)
Ledger
Residual risk
Merge left to you. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #8987 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Focus restore onto the composer + trigger was reopening the documents actions panel via onFocusCapture, covering Browse library and failing the deferred-request @critical journey after the Button ref sync. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/patient-safety-plan.tsx (1)
603-603: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove safe-area header spacing to named theme tokens.
The component and its test duplicate raw spacing values. Define named Tailwind 4
@themetokens insrc/app/globals.css. Use those tokens in themax()expressions. Update the DOM assertions to match the token-based classes.
src/components/patient-safety-plan.tsx#L603-L603: replace0.75remand1.25remwith named safe-area header spacing tokens.tests/patient-safety-plan.dom.test.tsx#L35-L36: assert the token-based class names instead of raw spacing values.Proposed fix
+/* src/app/globals.css */ +@theme { + --safety-plan-header-top-pad-phone: 0.75rem; + --safety-plan-header-top-pad-desktop: 1.25rem; +}- className="... pt-[max(0.75rem,var(--safe-area-top))] sm:pt-[max(1.25rem,var(--safe-area-top))]" + className="... pt-[max(var(--safety-plan-header-top-pad-phone),var(--safe-area-top))] sm:pt-[max(var(--safety-plan-header-top-pad-desktop),var(--safe-area-top))]"- expect(className).toContain("pt-[max(0.75rem,var(--safe-area-top))]"); - expect(className).toContain("sm:pt-[max(1.25rem,var(--safe-area-top))]"); + expect(className).toContain("pt-[max(var(--safety-plan-header-top-pad-phone),var(--safe-area-top))]"); + expect(className).toContain("sm:pt-[max(var(--safety-plan-header-top-pad-desktop),var(--safe-area-top))]");Confirm Tailwind emits both token references in the final stylesheet. As per coding guidelines, use Tailwind 4
@themetokens rather than hardcoded design values.🤖 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/patient-safety-plan.tsx` at line 603, Define named safe-area header spacing tokens in src/app/globals.css using Tailwind 4 `@theme`, then update the className in src/components/patient-safety-plan.tsx at lines 603-603 to use those tokens in both max() expressions. Update the DOM assertions in tests/patient-safety-plan.dom.test.tsx at lines 35-36 to expect the token-based classes, and verify Tailwind emits both token references in the final stylesheet.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.
Inline comments:
In `@tests/patient-safety-plan.dom.test.tsx`:
- Line 37: Update the className assertion in the patient safety plan regression
test to reject both unprefixed and variant-prefixed vertical padding utilities,
including classes such as sm:py-*. Preserve acceptance of unrelated utilities
and keep the check scoped to py-* axis classes.
---
Nitpick comments:
In `@src/components/patient-safety-plan.tsx`:
- Line 603: Define named safe-area header spacing tokens in src/app/globals.css
using Tailwind 4 `@theme`, then update the className in
src/components/patient-safety-plan.tsx at lines 603-603 to use those tokens in
both max() expressions. Update the DOM assertions in
tests/patient-safety-plan.dom.test.tsx at lines 35-36 to expect the token-based
classes, and verify Tailwind emits both token references in the final
stylesheet.
🪄 Autofix
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
Run ID: c96e0ae4-2850-4d0a-8841-26d799dd4c88
📒 Files selected for processing (5)
src/components/clinical-dashboard/master-search-header.tsxsrc/components/clinical-dashboard/universal-search-command-surface.tsxsrc/components/patient-safety-plan.tsxtests/patient-safety-plan.dom.test.tsxtests/ui-smoke.spec.ts
|
@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. |
Match the standalone shell contract so breakpoint-prefixed py utilities cannot bypass the safe-area top-pad regression check. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Review outcome for the post-sync tip after the sm:py guard fix and late main sync (#1720). Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
PR #1711 review-and-fix summaryTip: Snapshot
Review
Fixed
Dispositioned (inline reply failed — 403)
Local gates (decisive lines)
Not run
Residual risks
Merge left to you. |
|
@cursoragent Unblock the current open PR. 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; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix. Snapshot the GitHub head: tip, 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, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop. Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels 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. Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push. Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress. |
|
Taking a look! |
|
@cursoragent Unblock the current open PR. 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; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix. Snapshot the GitHub head: tip, 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, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop. Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels 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. Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push. Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress. |
|
Taking a look! |
There was a problem hiding this comment.
Pull request overview
Updates the standalone Safety Plan tool’s phone header to correctly respect the OS top safe-area inset (since it’s outside the search shell), and tightens universal search focus/close behavior so closing Scope or mode surfaces can’t unintentionally reopen the command dropdown overlay on mode homes.
Changes:
- Adjust
/safety-planheader layout to ownpt-[max(...,var(--safe-area-top))]and restack phone chrome (back row → shield/title), preserving the existinglg+layout vialg:contents. - Add focused DOM test coverage to assert the safe-area
pt-*contract and guard against introducingpy-*on the header. - Prevent
UniversalSearchCommandSurfacefrom opening the command dropdown on focus restore unless focus lands on the actual search input; additionally ensure mode-surface close dismisses the command dropdown, with smoke coverage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/components/patient-safety-plan.tsx |
Makes the Safety Plan header own top safe-area padding and adjusts mobile vs lg+ layout stacking. |
tests/patient-safety-plan.dom.test.tsx |
Adds a DOM assertion for the safe-area pt-* header contract and guards against py-* regression. |
src/components/clinical-dashboard/universal-search-command-surface.tsx |
Restricts focus-capture dropdown opening to the actual global search input (avoids overlay reopening on focus restore). |
src/components/clinical-dashboard/master-search-header.tsx |
Ensures closing mode surfaces also closes the command dropdown (in addition to scope/action menu). |
tests/ui-smoke.spec.ts |
Adds UI smoke assertions that scope close and mode switching do not leave/reopen the suggestions listbox/popover. |
docs/branch-review-ledger.md |
Updates the review ledger entries related to the work. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
/safety-plansits outside the search shell and never padded for the OS top inset, so the tool header collided with the phone status bar.pt-[max(...var(--safe-area-top))](same contract as/privacyand colour-coding) and restacks the phone chrome: back on its own tap row, then shield + title.lg+keeps the existing two-column header vialg:contents.--safe-area-topand does not use axispy-*.+trigger no longer reopens the command listbox viaonFocusCapture(that overlay covered Browse library and failed Production UI critical). Mode-menu open also clears the command dropdown.RAG impact: no retrieval behaviour change — UI-only standalone tool chrome spacing plus composer focus/dismiss behaviour; no retrieval, ranking, or answer surfaces touched.
Verification
npm run test—tests/patient-safety-plan.dom.test.tsx+tests/patient-safety-plan-privacy.dom.test.tsx— 14 passednpm run ensure→ http://localhost:4461 (project identity confirmed)/safety-plan— back on own row below top pad; title block below; no status-bar collision under the shared safe-area tokendashboard defers source and administration requests until their surfaces open @critical—1 passed (3.1s)npm run verify:uideferred — change is standalone tool-header spacing outside the search shell plus a narrow composer focus/dismiss fix; phone viewport proof of/safety-planalready covered the original chrome; the failing Production UI critical journey is covered by the focused Playwright proof aboveRisk and rollout
cursor/safety-plan-phone-safe-area-624a.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Summary by CodeRabbit
Bug Fixes
UI Improvements
Tests