Fix mobile edge-to-edge content when chrome hides#900
Conversation
📝 WalkthroughWalkthroughThe mobile search shell preserves its existing composer reserve calculation and applies smaller safe-area-based bottom spacing when mobile chrome is hidden during phone scrolling. ChangesMobile Composer Reserve
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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. |
![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
🤖 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/components/clinical-dashboard/global-search-shell.tsx`:
- Around line 280-282: Update the mobileComposerReserve calculation in the
global search shell to use the effective composer visibility state rather than
phoneScrollHide.hidden alone. Reuse or expose the state from MasterSearchHeader
so open menus and focused controls retain visibleMobileComposerReserve, and add
a regression test covering chrome reopening or focus after scroll-hide.
🪄 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: d9ad0d9a-ffca-4307-88c8-57e6094b0ade
📒 Files selected for processing (1)
src/components/clinical-dashboard/global-search-shell.tsx
| const mobileComposerReserve = phoneScrollHide.hidden | ||
| ? "max(0.75rem, env(safe-area-inset-bottom))" | ||
| : visibleMobileComposerReserve; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Base the reserve on effective composer visibility.
phoneScrollHide.hidden can remain true while MasterSearchHeader keeps the header/composer visible for an open menu or focused control (master-search-header.tsx lines 308-330). This drops the main content reserve to 0.75rem while visible chrome can still overlay the bottom of #main-content. Use the same effective hidden state, or expose that state from MasterSearchHeader, and add a regression test for reopening/focusing chrome after scroll-hide.
🤖 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/clinical-dashboard/global-search-shell.tsx` around lines 280 -
282, Update the mobileComposerReserve calculation in the global search shell to
use the effective composer visibility state rather than phoneScrollHide.hidden
alone. Reuse or expose the state from MasterSearchHeader so open menus and
focused controls retain visibleMobileComposerReserve, and add a regression test
covering chrome reopening or focus after scroll-hide.
There was a problem hiding this comment.
Summary
- Added
mobileBottomComposerHiddenstate in the global shell and now shrink--mobile-composer-reserveonly when the bottom composer reports that it is effectively hidden, preserving the full reserve while menus, focus, or persistent add-ons keep the dock visible. src/components/clinical-dashboard/global-search-shell.tsxL162-L164 src/components/clinical-dashboard/global-search-shell.tsxL275-L282 - Added an
onMobileBottomComposerHiddenChangecallback fromMasterSearchHeader, reporting the existing effective hidden state that already accounts for phone layout, add-ons, open menus/dropdowns/sheets, and composer focus. src/components/clinical-dashboard/master-search-header.tsxL240-L243 src/components/clinical-dashboard/master-search-header.tsxL327-L341 - Wired the global shell to receive that effective bottom-dock hidden state from
MasterSearchHeader. src/components/clinical-dashboard/global-search-shell.tsxL579-L588 - Updated merge-artifact guard coverage to assert the reserve depends on the effective hidden-state callback and that persistent mobile add-ons still disable bottom-dock scroll hiding. tests/clinical-dashboard-merge-artifacts.test.tsL89-L102
- Committed the fix on the current branch:
2882294 fix(ui): reserve space until mobile dock hides. - Created PR metadata with the
make_prtool: “Fix mobile reserve until bottom dock actually hides”.
Testing
- ✅
git diff --check - ❌
npm run test:focused -- --files src/components/clinical-dashboard/global-search-shell.tsx,src/components/clinical-dashboard/master-search-header.tsx,tests/clinical-dashboard-merge-artifacts.test.ts— focused runner rejected direct test/config paths and instructed running the full unit suite. ⚠️ npm run test— blocked by environment setup: missingnode_modules/vitest/vitest.mjs; local runtime reportedNode.js v20.20.2.
There was a problem hiding this comment.
Skipped: comment is from another GitHub bot.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29f09f473e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const mobileComposerReserve = phoneScrollHide.hidden | ||
| ? "max(0.75rem, env(safe-area-inset-bottom))" | ||
| : visibleMobileComposerReserve; |
There was a problem hiding this comment.
On phone differential search results with the compare add-on active, MasterSearchHeader intentionally keeps the bottom dock visible because mobileBottomSearchAddonSlotId disables bottom hide-on-scroll, but this new reserve switch still shrinks #main-content padding whenever only the header hides. After a downward scroll on /differentials results, the visible compare/dock can overlap the final cards; the existing geometry check in tests/ui-tools.spec.ts that scrolls results and asserts lastResultBottom <= dockTop would catch this. Gate the reserve reduction on the same condition that actually hides the bottom composer, not just phoneScrollHide.hidden.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Updated the shell to keep the full visible mobile composer reserve unless
MasterSearchHeaderreports that the bottom composer itself is effectively hidden, instead of relying on rawphoneScrollHide.hidden. src/components/clinical-dashboard/global-search-shell.tsxL274-L282 - Added an
onBottomComposerHiddenChangecallback fromMasterSearchHeaderso the shell can track the same effective bottom-dock visibility logic that already accounts for focus, open menus/sheets, command dropdowns, and differential compare add-ons. src/components/clinical-dashboard/master-search-header.tsxL186-L187 src/components/clinical-dashboard/master-search-header.tsxL326-L340 - Wired the shell to receive the effective bottom-composer hidden state from
MasterSearchHeader. src/components/clinical-dashboard/global-search-shell.tsxL585-L589 - Added a focused mobile regression assertion that after scroll-hide, focusing the bottom dock restores visible dock state and enough
#main-contentbottom reserve to clear the fixed composer. tests/ui-tools.spec.tsL1074-L1080 - Committed the change on the current branch with commit
50318c2(fix(ui): reserve space for visible mobile composer). - Created PR metadata via the
make_prtool with title:Fix mobile composer reserve when dock remains visible.
Testing
- ✅
npm run workflow:flightplan -- --write-evidence --files src/components/clinical-dashboard/global-search-shell.tsx,src/components/clinical-dashboard/master-search-header.tsx,tests/ui-tools.spec.ts - ✅
git diff --check ⚠️ npm run test:focused -- --files src/components/clinical-dashboard/global-search-shell.tsx,src/components/clinical-dashboard/master-search-header.tsx,tests/ui-tools.spec.ts— helper failed closed because the changed file list includestests/ui-tools.spec.tsand instructed runningnpm run test.⚠️ npm run test— blocked by missing local dependencynode_modules/vitest/vitest.mjsunder Node20.20.2.⚠️ npm run ensure— blocked because the project requires Node24.x, but the local runtime is Node20.20.2.⚠️ npm run typecheck— blocked by missing local dependencynode_modules/typescript/bin/tscunder Node20.20.2.
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #3471 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Motivation
Description
src/components/clinical-dashboard/global-search-shell.tsxto computevisibleMobileComposerReserveand to setmobileComposerReservetomax(0.75rem, env(safe-area-inset-bottom))whenphoneScrollHide.hiddenis true so the large bottom clearance is released while keeping the original reserve when chrome is visible.Testing
git diff --checkwhich completed successfully with no reported issues.work(561498f) and verified the worktree is clean after the commit.npm run ensurebut it was blocked because the local runtime is Node20.20.2while the project requires Node24.xso the local server could not be started.npm run test:focused -- --files src/components/clinical-dashboard/global-search-shell.tsxbut it failed due to missing test dependencies (node_modules/vitest/vitest.mjs) and the Node version mismatch, so focused automated tests could not be executed.Codex Task
Summary by CodeRabbit