feat(ui): phone Mode menu opens as a bottom sheet#935
Conversation
On viewports ≤639px the header Mode picker now uses the shared Sheet bottom sheet so the full mode list is scrollable with backdrop dismiss and focus restore. Desktop keeps the anchored absolute dropdown and existing keyboard/blur contracts. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Drop the tautological count check in the phone Mode sheet smoke test. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR adds a responsive phone Sheet mode menu, hardens Sheet dismissal and focus trapping, improves PDF extractor process termination and Python resolution tests, and adds policy, review-ledger, and governance workflow validation updates. ChangesMode menu interaction
PDF extractor process handling
PR policy synchronization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #3763 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
- Resolve python/python3 for PDF extraction when PYTHON_BIN is unset - Sheet: dismiss backdrop on click; exclude tabindex=-1 from Tab trap - Mode menu: dialog aria-haspopup on phone, close on breakpoint flip, host Sheet outside the header grid, clear scope sheet from + menu Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Always process-group kill on deadline even if the leader already exited, await close, and poll briefly in the budget test under suite load. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Prevent accidental dismiss when a press begins on the panel and ends on the backdrop by tracking pointerdown origin before honoring the click. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Satisfy CI Static PR format check on the touched sources/tests. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Append the completed PR #935 review outcome and local verification evidence. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Replace the stale #932 PR_POLICY_BODY template, keep the Supabase identity checklist line commit-safe with an allowlist pragma, and teach PR policy to accept trailing allowlist markers on checked items. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Resolve branch-review-ledger append conflict by keeping both review rows. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Use a PR_POLICY_BODY marker so CI injects checked Clinical Governance items from trusted policy metadata, avoiding pragma/secret-scan clashes with pull_request_target PR policy. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Keep PR_POLICY_BODY for CI body sync. Adopt main's python-bin helper and retain process-tree kill hardening in the PDF extractor. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Keep pace with main closeout docs so the PR is not behind for merge. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Record the post-merge tip and local revalidation evidence for merge readiness. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/ci.yml:
- Around line 109-123: Prevent PR-head content from self-attesting governance
completion: in .github/workflows/ci.yml lines 109-123, remove the
template-controlled forceChecked logic and ensure required items are checked
only through the existing trusted attestation source; in PR_POLICY_BODY.md lines
23-28, remove the GOVERNANCE_ALL_CHECKED marker or gate it through that same
trusted approval path.
In `@scripts/pr-policy.mjs`:
- Around line 106-109: Update the regular expression in the checklist-matching
logic to use line-local whitespace instead of \s* before the optional trailing
annotation, and ensure the # or <!-- suffix cannot consume newline characters.
Preserve matching for annotations that remain on the same line while rejecting
annotations that begin on a separate line.
In `@src/components/clinical-dashboard/master-search-header.tsx`:
- Around line 324-328: Replace the window.matchMedia initializer for
usesPhoneSearchLayout with a hydration-safe media-query hook that provides a
stable server snapshot and client subscription. Preserve the existing
phone-layout breakpoint and the breakpoint-close effect, using the hook’s value
to keep the desktop menu and Sheet synchronized after hydration.
In `@tests/pdf-extraction-budget.test.ts`:
- Around line 58-60: Update the test covering non-Windows Python resolution to
exercise the actual default-parameter path in resolvePythonBin: delete or
otherwise temporarily unset process.env.PYTHON_BIN, restore its original value
after the assertion, and call resolvePythonBin() without an argument. Preserve
the existing platform-specific expectation.
🪄 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: 1daac8fa-2823-45c4-b1f3-7454847158ca
📒 Files selected for processing (11)
.github/workflows/ci.ymlPR_POLICY_BODY.mddocs/branch-review-ledger.mdscripts/pr-policy.mjssrc/components/clinical-dashboard/master-search-header.tsxsrc/components/ui/sheet.tsxsrc/lib/extractors/document.tstests/audit-navigation-auth-regressions.test.tstests/pdf-extraction-budget.test.tstests/ui-overlay-css-contract.test.tstests/ui-smoke.spec.ts
Incorporate mobile header new-chat inset fix before Production UI re-run. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Initialize usesPhoneSearchLayout to false so SSR and the first client paint match, then sync from matchMedia after mount and on Mode open. Stops React #418 pageerrors on phone route-coverage journeys. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Capture the React #418 fix evidence and local Playwright revalidation. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Keep Sync PR policy body accurate for merge-ready evidence. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Record merge-ready hosted checks on 792142c; human approval still required. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Remove PR-head GOVERNANCE_ALL_CHECKED self-attest, restore strict checklist matching, harden the sync workflow guard, and exercise the real unset PYTHON_BIN path in resolvePythonBin tests. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Note closed review threads and local verification before merge. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…point closeout (#961) * polish(pwa): native install sheet on phones with app identity and iOS steps Install prompt and iOS hint present as a bottom sheet on phones: full bleed to the viewport edges, flush bottom over the composer zone, rounded top, and a static grip bar matching the Mode-menu sheet language (#935). The real app icon (/icons/icon-192) replaces the generic download tile beside the name and tagline; the install card gains a Free/no-store meta row and the iOS hint gains two accent step chips (Share -> Add to Home Screen) as aria-hidden visual reinforcement of the unchanged accessible sentence. Stack bottom offset extracted to --pwa-notice-bottom-gap (standalone override included) so the sheet's flush-bottom math tracks any future re-anchoring. >=640px keeps the #905 card/toast placements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 * docs(ledger): record PWA-window review and credentialed checkpoint closeout Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
max-width: 639px), the header Mode picker opens as the shared bottomSheetso the full mode list is scrollable with backdrop dismiss, Escape, focus trap, and return-focus to the Mode button.sm:+) keeps the existing anchored absolute dropdown, keyboard navigation, blur-leave dismiss, and outside-click dismiss viauseDismissableLayer.Sheetbackdrop dismiss (gesture must start on the dimmed area; no click-through) and Tab-trap handling for rovingtabindex="-1"menus.python-binfrom main.matchMediaafter mount / on open (avoids React Fix differential badge design for mobile search results #418 on phone route coverage).Verification
npm run check:production-readiness:ci— READY (no blocking failures)verify:ui/verify:releasenot required beyond the focused Mode + route-coverage proofs aboveRisk and rollout
Clinical Governance Preflight
Touched extractors/document.ts (fail-closed process-tree cleanup only). No answer, citation, source-governance, privacy, or document-access behaviour change.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
phoneSearchLayoutMediaQuery(max-width: 639px).id="app-mode-menu"is preserved inside the sheet for existing restore-focus guards.Summary by CodeRabbit
New Features
Bug Fixes