Fix iOS Safari content below hidden search composer#922
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMobile composer visibility now drives safe-area-aware bottom reserves for dashboard, search, and document content. Tests simulate Safari safe-area behavior and verify padding and geometry while composers hide and reappear. ChangesMobile composer reserve synchronization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…idden-composer-edge
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/clinical-dashboard-merge-artifacts.test.ts (1)
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider guarding that
--safe-area-bottomis actually defined inglobals.css.This suite already loads
globalStylesSource(line 33, unchanged) but only asserts that consumers referencevar(--safe-area-bottom), never that it's defined. Given the whole PR hinges on that variable resolving to a real value in production (see the request-verification note onClinicalDashboard.tsxlines 3253-3266), a cheap guard here (e.g.expect(globalStylesSource).toContain("--safe-area-bottom")) would catch a future accidental removal of the definition.✅ Proposed guard addition
expect(documentViewerSource).toContain('"max-sm:pb-[calc(9rem+var(--safe-area-bottom))]"'); + expect(globalStylesSource).toContain("--safe-area-bottom");Also applies to: 90-101
🤖 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 `@tests/clinical-dashboard-merge-artifacts.test.ts` at line 32, Update the assertions in the clinical dashboard merge-artifacts test to verify that globalStylesSource defines --safe-area-bottom, not only that consumers reference it. Add the guard alongside the existing DocumentViewer.tsx and ClinicalDashboard.tsx variable-reference checks, preserving the current test structure.
🤖 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.
Nitpick comments:
In `@tests/clinical-dashboard-merge-artifacts.test.ts`:
- Line 32: Update the assertions in the clinical dashboard merge-artifacts test
to verify that globalStylesSource defines --safe-area-bottom, not only that
consumers reference it. Add the guard alongside the existing DocumentViewer.tsx
and ClinicalDashboard.tsx variable-reference checks, preserving the current test
structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 87ed1b50-3e78-4654-bb4b-a8d6c364d631
📒 Files selected for processing (7)
src/components/ClinicalDashboard.tsxsrc/components/DocumentViewer.tsxsrc/components/clinical-dashboard/global-search-shell.tsxsrc/components/clinical-dashboard/master-search-header.tsxtests/clinical-dashboard-merge-artifacts.test.tstests/ui-smoke.spec.tstests/ui-tools.spec.ts
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #3573 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
@cursoragent fix comment and resolve without regression. Also fix failing CI to ensure this can merge |
|
Fixed the review comment and the CI failure on PR #922. Review comment (CodeRabbit)Added the suggested guard in CI failure (
|
- Restore the phone answer dock to the viewport in openScopeControl before clicking, preventing CI timeouts when prior sheet interactions leave the composer translated off-screen. - Assert globals.css defines --safe-area-bottom in merge-artifact guards per review feedback. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Addressed the CodeRabbit nitpick and the Production UI failure:
Local verification
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/clinical-dashboard-merge-artifacts.test.ts`:
- Line 102: Strengthen the assertion for globalStylesSource in the relevant test
so it verifies the --safe-area-bottom custom-property declaration rather than
merely token usage. Match the property name followed by optional whitespace and
a colon, or assert its expected env(safe-area-inset-bottom) value.
🪄 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: ad5e10e3-2fbc-401c-b31d-4c4f49d84954
📒 Files selected for processing (2)
tests/clinical-dashboard-merge-artifacts.test.tstests/ui-smoke.spec.ts
Match the custom-property definition (--safe-area-bottom:) rather than any token reference so the guard catches accidental removal of the declaration. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Resolved the CodeRabbit follow-up on
Local check: |


Summary
Verification
git diff --check origin/main...HEADnpm run verify:pr-localin the isolated worktree — not completed because the repository-wide heavy-command lock remained held by another active worktree's Playwright suite; required PR checks are running on this branchnpm run verify:uisuite was blocked by the repository-wide heavy-command lock; the affected Chromium and WebKit scenarios passed in targeted runs before isolation.Risk and rollout
7970ae800to restore the prior fixed reserve behavior.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
mainstill retainedmax(0.75rem, env(safe-area-inset-bottom))for the hidden state. On iOS Safari that inset includes the translucent bottom browser toolbar, recreating the toolbar-sized blank band shown in the report.Summary by CodeRabbit
Bug Fixes
Tests