Codex/mobile search phone fix 20260717#700
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe universal search command dropdown now uses placement-specific desktop pointer media queries, synchronizes displayability state, and suppresses fetching, keyboard behavior, opening, and rendering on unsupported viewports. Unit and phone UI tests cover the media queries and hidden dropdown behavior. ChangesResponsive dropdown gating
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UniversalSearchCommandSurface
participant matchMedia
participant useUniversalSearch
participant CommandDropdown
UniversalSearchCommandSurface->>matchMedia: Evaluate placement media query
matchMedia-->>UniversalSearchCommandSurface: Return dropdownDisplayable
UniversalSearchCommandSurface->>useUniversalSearch: Enable when open and displayable
UniversalSearchCommandSurface->>CommandDropdown: Render when displayable
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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/universal-search-command-surface.tsx`:
- Around line 383-406: The dropdownDisplayable state initializer must not access
window.matchMedia. Initialize it deterministically without browser APIs, then
let the existing effect’s sync function read matchMedia and update the state,
while guarding environments where matchMedia is unavailable before using it.
🪄 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: 1e1204d0-fe96-421c-9fd2-afbf3e20eb6b
📒 Files selected for processing (4)
src/components/clinical-dashboard/universal-search-command-surface.tsxsrc/lib/search-command-surface.tstests/search-command-surface.test.tstests/ui-tools.spec.ts
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Heuristic only — a main-side or flake label is a starting point, not a verdict. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/ui-universal-search.spec.ts (1)
193-201: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the touch-pointer branch, not only the narrow-width branch.
At 390px, this test passes solely because the viewport is below the dropdown breakpoint. It would still pass if the new
(hover: hover) and (pointer: fine)gating were removed, so wide or landscape touch phones remain unprotected. Add a mobile Playwright context (hasTouch: true) with a viewport above the relevant breakpoint, or add that as a separate case.🤖 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-universal-search.spec.ts` around lines 193 - 201, Update the “keeps cross-mode typeahead hidden on a phone viewport” test to exercise the touch-pointer condition, not just narrow width: use a mobile Playwright context with hasTouch enabled and a viewport wider than the dropdown breakpoint, or add a separate case. Preserve the existing assertions that the dropdown and cross-mode results remain hidden.
🤖 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/ui-universal-search.spec.ts`:
- Around line 193-201: Update the “keeps cross-mode typeahead hidden on a phone
viewport” test to exercise the touch-pointer condition, not just narrow width:
use a mobile Playwright context with hasTouch enabled and a viewport wider than
the dropdown breakpoint, or add a separate case. Preserve the existing
assertions that the dropdown and cross-mode results remain hidden.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c3a2c204-c533-4d41-87be-1970d73aa772
📒 Files selected for processing (1)
tests/ui-universal-search.spec.ts
|
Superseded by merged PR #717. The exact five-file scope from this branch is now present on |
Summary
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:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsFor retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes,
verify:pr-localrunseval:rag:offlineautomatically. Run the offline command directly during iteration before spending a live eval.npm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-onlywhen answer generation, the synthesis prompt, or answer post-processing changed (grounded-supported must not drop; citation-failure 0)npm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedClinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
New Features
Bug Fixes
Tests