fix(mobile): keep home-page search in the hero and drop the phone command popup#473
Conversation
…mand popup On phones, mode-home pages docked the shared search composer to the bottom edge and opened the universal command popup above it, crowding the small screen. The hero-slot portal was gated to (min-width: 640px), so phones never used the in-flow hero placement that tablet+ already had. - Portal the composer into the mode-home hero slot at every viewport width, so phone home pages show the search pill mid-screen under the hero instead of a fixed bottom dock. Result and answer-thread views keep the dock. - Hide the universal command dropdown below sm for bottom-docked composers (inline placement already hid it below lg), skip its typeahead fetches at widths where nothing can display them, and stop the phone dock scrim from expanding for a popup that never shows. - Remove the now-redundant heroComposerFromTablet prop and the phone bottom-dock clearance paddings on home views that no longer have a dock. - Update the phone Playwright coverage: the popup must stay hidden on phones and mode homes must render the composer in the hero, not the bottom dock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013mCpzKWTCtBSLC6p1NsSWo
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughResponsive search composer routing, ref handling, viewport-aware command visibility, safe-area spacing, and phone UI coverage were updated. Desktop-home slots now control composer placement, while phone command interactions remain hidden when the surface is not displayable. ChangesResponsive search composer behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PhoneUser
participant MasterSearchHeader
participant UniversalSearchCommandSurface
participant SharedQueryInput
PhoneUser->>MasterSearchHeader: focus search composer
MasterSearchHeader->>SharedQueryInput: bind and focus active input
MasterSearchHeader->>UniversalSearchCommandSurface: provide composer interaction state
UniversalSearchCommandSurface->>UniversalSearchCommandSurface: evaluate placement breakpoint
UniversalSearchCommandSurface-->>PhoneUser: keep dropdown and fetching disabled when unavailable
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
With the hero composer now active on phones, navigating from a mode home into a result view briefly renders both composers; React nulls a plain shared queryInputRef when the outgoing portal composer unmounts after the dock composer has already bound it, so focus helpers (quote follow-up, "/" shortcut) silently no-oped. Bind the input through a cleanup-function ref that only clears the binding it still owns, and narrow the prop to the RefObject shape every caller already passes. Caught by ui-smoke "quote follow-up stages a composer draft from evidence quotes"; full ui-smoke suite now passes (59/59). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013mCpzKWTCtBSLC6p1NsSWo
|
@copilot resolve the merge conflicts on this branch. |
![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: a30d170434
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/clinical-dashboard/global-mockup-search-shell.tsx (1)
258-282: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winResolve the
mobileComposerReserveconflict.This unresolved conflict leaves the standalone search shell syntactically invalid. Keep one finalized reserve policy that matches the selected hero-versus-floating-composer layout.
🤖 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-mockup-search-shell.tsx` around lines 258 - 282, Resolve the conflict in the mobileComposerReserve policy within the global mockup search shell, removing all conflict markers and retaining only one valid implementation. Align the reserve value with the selected hero-versus-floating-composer layout, preserving inline standalone hero behavior while reserving the fixed bottom composer only when it is actually rendered.Source: Linters/SAST tools
🤖 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/applications-launcher-page.tsx`:
- Around line 736-741: Resolve the merge conflict in the applications launcher
JSX by removing the conflict markers and retaining the intended padding class
declaration, including its responsive safe-area behavior. Ensure the surrounding
component remains valid TypeScript/JSX and compiles successfully.
In `@src/components/clinical-dashboard/master-search-header.tsx`:
- Around line 73-81: Resolve all remaining merge conflicts in the
MasterSearchHeader module by removing every conflict marker and retaining the
mode-home hero behavior across all viewport widths. Preserve
desktopHomeComposerMediaQuery for desktop behavior and
modeHomeComposerMediaQuery as an always-matching query, and apply the same
consistent resolution at the additional conflict locations.
In `@src/components/clinical-dashboard/universal-search-command-surface.tsx`:
- Around line 317-332: Update the universal search interaction flow around
dropdownDisplayable and the component’s focus/keyboard handlers so ineligible
phone widths cannot open, navigate, or select hidden suggestions. Gate focus
handling, ArrowDown/Enter behavior, and dropdown rendering on
dropdownDisplayable, and when it becomes false close the dropdown and reset
related active/selection state so the combobox no longer reports expanded.
---
Outside diff comments:
In `@src/components/clinical-dashboard/global-mockup-search-shell.tsx`:
- Around line 258-282: Resolve the conflict in the mobileComposerReserve policy
within the global mockup search shell, removing all conflict markers and
retaining only one valid implementation. Align the reserve value with the
selected hero-versus-floating-composer layout, preserving inline standalone hero
behavior while reserving the fixed bottom composer only when it is actually
rendered.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d73c8358-af02-4eeb-894f-6ac67f564431
📒 Files selected for processing (6)
src/components/ClinicalDashboard.tsxsrc/components/applications-launcher-page.tsxsrc/components/clinical-dashboard/global-mockup-search-shell.tsxsrc/components/clinical-dashboard/master-search-header.tsxsrc/components/clinical-dashboard/universal-search-command-surface.tsxtests/ui-tools.spec.ts
# Conflicts: # docs/branch-review-ledger.md
# Conflicts: # docs/branch-review-ledger.md # src/components/clinical-dashboard/global-mockup-search-shell.tsx # tests/ui-tools.spec.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53ea5729b2
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/ui-smoke.spec.ts (1)
1909-1911: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse an exact match for the singular status
toContainText("1 source")still passes on1 sources, so this check can miss a pluralization regression. Switch both assertions to an exact text target.🤖 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/ui-smoke.spec.ts` around lines 1909 - 1911, Update both sourceStatus assertions to use an exact text match for “1 source” rather than substring matching, ensuring the singular status does not pass when the UI displays “1 sources”.
🤖 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/ui-smoke.spec.ts`:
- Around line 210-214: Update the hostname allowlist in the request filtering
logic to include the bracketed IPv6 loopback value "[::1]" alongside the
existing loopback entries, so local IPv6 requests remain allowed.
---
Outside diff comments:
In `@tests/ui-smoke.spec.ts`:
- Around line 1909-1911: Update both sourceStatus assertions to use an exact
text match for “1 source” rather than substring matching, ensuring the singular
status does not pass when the UI displays “1 sources”.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 722b3082-3217-431b-87f9-7638c0fc8972
📒 Files selected for processing (5)
docs/branch-review-ledger.mdsrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/global-mockup-search-shell.tsxtests/ui-smoke.spec.tstests/ui-tools.spec.ts
✅ Files skipped from review due to trivial changes (1)
- docs/branch-review-ledger.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/ClinicalDashboard.tsx
- tests/ui-tools.spec.ts
- src/components/clinical-dashboard/global-mockup-search-shell.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/ui-smoke.spec.ts (1)
1909-1911: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse an exact match for the singular status
toContainText("1 source")still passes on1 sources, so this check can miss a pluralization regression. Switch both assertions to an exact text target.🤖 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/ui-smoke.spec.ts` around lines 1909 - 1911, Update both sourceStatus assertions to use an exact text match for “1 source” rather than substring matching, ensuring the singular status does not pass when the UI displays “1 sources”.
🤖 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/ui-smoke.spec.ts`:
- Around line 210-214: Update the hostname allowlist in the request filtering
logic to include the bracketed IPv6 loopback value "[::1]" alongside the
existing loopback entries, so local IPv6 requests remain allowed.
---
Outside diff comments:
In `@tests/ui-smoke.spec.ts`:
- Around line 1909-1911: Update both sourceStatus assertions to use an exact
text match for “1 source” rather than substring matching, ensuring the singular
status does not pass when the UI displays “1 sources”.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 722b3082-3217-431b-87f9-7638c0fc8972
📒 Files selected for processing (5)
docs/branch-review-ledger.mdsrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/global-mockup-search-shell.tsxtests/ui-smoke.spec.tstests/ui-tools.spec.ts
✅ Files skipped from review due to trivial changes (1)
- docs/branch-review-ledger.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/ClinicalDashboard.tsx
- tests/ui-tools.spec.ts
- src/components/clinical-dashboard/global-mockup-search-shell.tsx
🛑 Comments failed to post (1)
tests/ui-smoke.spec.ts (1)
210-214: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail node <<'NODE' const hostname = new URL("http://[::1]:4173").hostname; if (hostname !== "[::1]") { throw new Error(`Unexpected hostname serialization: ${hostname}`); } console.log(hostname); NODERepository: BigSimmo/Database
Length of output: 161
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path("tests/ui-smoke.spec.ts") text = p.read_text() for i, line in enumerate(text.splitlines(), 1): if 180 <= i <= 240: print(f"{i:4d}: {line}") PYRepository: BigSimmo/Database
Length of output: 2458
Allow bracketed IPv6 loopback in the allowlist.
URL.hostnamereturns"[::1]"for IPv6 loopback, so the current"::1"entry never matches and local IPv6 app requests get blocked as external.🤖 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/ui-smoke.spec.ts` around lines 210 - 214, Update the hostname allowlist in the request filtering logic to include the bracketed IPv6 loopback value "[::1]" alongside the existing loopback entries, so local IPv6 requests remain allowed.
Findings fixed in ceccc72: exact singular source assertion and bracketed IPv6 loopback allowlist.
Summary
(min-width: 640px), so phones never got the in-flow hero placement tablet+ already had — even thoughdocs/codebase-index.mddocuments mode homes as hero-placed "on phone and tablet+ alike"./,/?mode=documents|prescribing|tools,/services,/forms,/favourites,/differentials,/applications) show the search pill mid-screen under the hero. Result and answer-thread views keep the bottom dock.smfor bottom-docked composers (inline placement already hid it belowlg), its typeahead fetches are skipped at widths where nothing can display them, and the phone dock scrim no longer expands for a popup that never shows.queryInputRefwhen the outgoing portal composer unmounted, silently breaking focus helpers (quote follow-up draft focus,/shortcut). The input now binds through a cleanup-function ref that only clears the binding it still owns.heroComposerFromTabletprop and drop the phone bottom-dock clearance paddings on home views that no longer have a dock (dashboard hubs, tools launcher, standalone shell reserve)./services,/forms,/differentials,/applications.Verification
npm run verify:pr-localDuring development, use
npm run verify:cheapas the faster iteration gate before the final PR-local preflight.npm run verify:cheap— lint, typecheck, 1438 vitest tests passnpm run verify:uiequivalent (Chromium, full spec set):ui-smoke59/59,ui-tools41/41,ui-overlap12/12,ui-universal-search5/5,ui-accessibility+ui-stress+ui-tools-collapse+ui-tools-task-directoryall pass. The one initialui-smokefailure (quote follow-up focus) was a real regression from the hero/dock transition and is fixed by the guarded ref commit; suite confirmed green on re-run.npm run verify:releasebefore release or handoff confidence claimsnpm run eval:retrieval:quality— not needed: no retrieval, ranking, selection, chunking, or scoring behavior changednpm run eval:rag— not needed: no answer generation changesnpm run check:production-readiness— not needed: no clinical workflow, privacy, env, Supabase, or governance behavior changedClinical Governance Preflight
Not applicable: this change is composer placement + popup visibility CSS/portal behavior only. It does not touch ingestion, answer generation, search/ranking logic, source rendering, document access, privacy, production env, or clinical output.
Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_013mCpzKWTCtBSLC6p1NsSWo