fix(viewport): implement mobile keyboard and touch target audit fixes#1140
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughAdds mobile keyboard viewport tracking and app-wide context wiring, keyboard-aware mobile composer layout, updated search focus and touch-dismissal behavior, shared focus-shadow styling, and review ledger records. ChangesMobile keyboard and composer behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MasterSearchHeader
participant MobileKeyboardProvider
participant visualViewport
participant ComposerDock
User->>MasterSearchHeader: focus search input
MasterSearchHeader->>MasterSearchHeader: resolve scroll behavior
MasterSearchHeader->>User: scroll input into view
visualViewport->>MobileKeyboardProvider: resize or focus event
MobileKeyboardProvider->>MobileKeyboardProvider: resolve keyboard height
MobileKeyboardProvider->>ComposerDock: set --keyboard-height
ComposerDock->>User: reposition dock and reserves
User->>MasterSearchHeader: downward swipe
MasterSearchHeader->>User: blur focused element
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 1f0aed90d5
ℹ️ 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".
|
@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 #5269 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f0aed90d5
ℹ️ 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".
…ort-fixes Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Route search-input focus scrolling through resolveScrollBehavior so prefers-reduced-motion is respected. Drop the npx rimraf typecheck wipe that fetched a remote package and deleted .next/types without regenerating them. Format use-mobile-keyboard.tsx for CI prettier. Addresses Codex review on PR #1140. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
@cursoragent fix merge conflict |
|
Merge conflict on #1140 is resolved. Merged PR status: MERGEABLE (was   |
…ort-fixes Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/branch-review-ledger.md`:
- Around line 733-734: Move the two PR `#1140` rows in the review-records table so
they appear before the standalone “work search chrome behaviour review” section
begins. Preserve the existing row content and table formatting, ensuring both
entries render as table rows.
In `@src/components/use-mobile-keyboard.tsx`:
- Around line 27-43: Update the resize handling around maxViewportHeight and
handleResize to reset the viewport baseline when orientation or the mobile
breakpoint changes, before calculating diff. Track the previous viewport
dimensions or breakpoint state, assign the current viewport.height as the new
baseline on transition, and ensure the orientation change cannot falsely report
the keyboard as open.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a0b8068f-7d0d-456a-a85a-badf06ccc285
📒 Files selected for processing (6)
docs/branch-review-ledger.mdsrc/app/globals.csssrc/app/layout.tsxsrc/components/clinical-dashboard/master-search-header.tsxsrc/components/ui-primitives.tsxsrc/components/use-mobile-keyboard.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c41160491e
ℹ️ 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".
Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fca3b671e1
ℹ️ 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".
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
The viewport PR intentionally adds --keyboard-height to phone composer reserves; update the source pin so Unit coverage matches.
Headless CI was treating viewport shrink as an open keyboard and inflating phone dock reserves. Only apply --keyboard-height when an input/textarea/select/contenteditable is focused, and accept identity transforms as the resting scroll-hide dock state.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/branch-review-ledger.md`:
- Around line 762-763: Update the affected branch-review ledger records,
including the entries corresponding to the additional referenced ranges, so they
do not use the future date 2026-07-25 while the review date is 2026-07-24.
Replace each date with the actual execution date, preserving the append-only
historical ordering and all other record details.
- Line 31: Update the table entry around the audit remediation record to replace
the raw pipe separators in the `node|npm|npx|tsx|vitest|playwright|bun` code
span with comma- or slash-separated wrapper names, preserving the listed names
while keeping the Markdown table valid.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: ee2dc61c-1fc9-4041-b65d-0caf7e074b7f
📒 Files selected for processing (10)
docs/branch-review-ledger.mdsrc/app/globals.csssrc/app/layout.tsxsrc/components/clinical-dashboard/mobile-composer-reserve.tssrc/components/use-mobile-keyboard.tsxtests/clinical-dashboard-merge-artifacts.test.tstests/mobile-composer-reserve.test.tstests/mobile-keyboard-viewport.test.tstests/ui-overlay-css-contract.test.tstests/ui-tools.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/layout.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3084d0465a
ℹ️ 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".
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 706188afa8
ℹ️ 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".
Defer baseline rebase while an overlay keyboard is open, restore per-width unobstructed baselines when available, and cover the rotation-with-keyboard-open path in unit tests.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/mobile-keyboard-viewport.test.ts (1)
146-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the sub-threshold counterpart for
deferredKeyboardCloseGrowth.This case clears the pending rebase at 400 vs a 398 trigger point — only 2px past the boundary, and nothing asserts the guard holds below it. A companion case would pin both sides.
💚 Suggested additional case
+ it("keeps the deferred rebase pending while the keyboard is still up", () => { + expect( + resolveMobileKeyboardBaselines({ + widthChanged: false, + hasEditableFocus: true, + keyboardWasOpen: true, + pendingOrientationRebase: true, + currentVisualHeight: 360, + currentLayoutHeight: 800, + previousVisualHeight: 350, + previousLayoutHeight: 800, + maxVisualHeight: 650, + maxLayoutHeight: 800, + deferredOccludedVisualHeight: 350, + }), + ).toEqual({ + maxVisualHeight: 650, + maxLayoutHeight: 800, + pendingOrientationRebase: true, + deferredOccludedVisualHeight: 350, + }); + });🤖 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/mobile-keyboard-viewport.test.ts` around lines 146 - 167, The orientation rebase tests lack coverage for growth below the deferredKeyboardCloseGrowth threshold. Add a companion test alongside the existing “finishes a deferred orientation rebase once the keyboard closes” case using a current height just below the 398px trigger, and assert that the pending rebase remains deferred with baseline values unchanged as appropriate.src/components/use-mobile-keyboard.tsx (1)
162-162: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
baselinesByWidthis never pruned.Every distinct rounded sub-1024px width (window dragging, pinch-zoom width changes) adds a permanent entry for the lifetime of the effect. Harmless in practice, but a small cap or eviction keeps it bounded.
♻️ Optional: bound the cache
- if (!next.isKeyboardOpen && !pendingOrientationRebase) { + if (!next.isKeyboardOpen && !pendingOrientationRebase) { + if (baselinesByWidth.size > 8 && !baselinesByWidth.has(widthKey)) { + const oldest = baselinesByWidth.keys().next().value; + if (oldest !== undefined) baselinesByWidth.delete(oldest); + } baselinesByWidth.set(widthKey, {Also applies to: 219-226
🤖 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/use-mobile-keyboard.tsx` at line 162, Bound the baselinesByWidth cache in the mobile keyboard effect so it cannot grow indefinitely as distinct widths are observed. Add a small eviction or pruning policy when entries are inserted, while preserving the existing baseline lookup and update behavior in the related width-handling logic.
🤖 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 `@src/components/use-mobile-keyboard.tsx`:
- Line 162: Bound the baselinesByWidth cache in the mobile keyboard effect so it
cannot grow indefinitely as distinct widths are observed. Add a small eviction
or pruning policy when entries are inserted, while preserving the existing
baseline lookup and update behavior in the related width-handling logic.
In `@tests/mobile-keyboard-viewport.test.ts`:
- Around line 146-167: The orientation rebase tests lack coverage for growth
below the deferredKeyboardCloseGrowth threshold. Add a companion test alongside
the existing “finishes a deferred orientation rebase once the keyboard closes”
case using a current height just below the 398px trigger, and assert that the
pending rebase remains deferred with baseline values unchanged as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a12d83ed-3dba-4718-b011-d2dc55340777
📒 Files selected for processing (4)
docs/branch-review-ledger.mdsrc/components/use-mobile-keyboard.tsxtests/mobile-keyboard-viewport.test.tstests/ui-tools.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/ui-tools.spec.ts
- docs/branch-review-ledger.md
Apply --keyboard-height transform and matching content reserve so page-owned DocumentViewer composers stay visible when visual keyboards open. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Prevent Playwright breakpoint churn from treating desktop visual height as keyboard occlusion, and settle the medication stress page-geometry assertion after the final phone resize. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Retry opener focus when sheet teardown races the composer, and poll the smoke assertions so CI does not flake on the first animation frame. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Skip the default phone max-h utility when contentClassName already sets max-h, and record the PR #1140 babysit ledger row. Co-authored-by: Cursor <cursoragent@cursor.com>
…1141) * fix(ui): implement orientation adaptive design audit recommendations * fix: remove duplicate command dropdown height cap Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * chore: note review reply permission denial GraphQL review-thread replies returned 403; resolved the fixed duplicate-height threads directly after validation. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: record PR babysit sweep Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: record Run PR behind-main sweep for #1141 * docs: ledger re-sync sweep for PR #1141 * fix(ui): collapse dock child safe-area padding * docs: record PR 1141 maintenance * fix(ui): avoid Sheet max-height racing caller caps Skip the default phone max-h utility when contentClassName already sets max-h, and record the PR #1140 babysit ledger row. Co-authored-by: Cursor <cursoragent@cursor.com> * test(ui): poll guide dialog focus trap across viewport sizes Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ui): do not steal focus from a newly opened sheet on restore retry The Sources opener retry must not reclaim focus when another dialog already became active (mobile Guide opened from the Clinical Guide menu). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ui): retry Sheet autofocus when open races a closing sibling Co-authored-by: Cursor <cursoragent@cursor.com> * test(ui): harden Sources opener focus restore assertion after Escape Co-authored-by: Cursor <cursoragent@cursor.com> * docs(review): record babysit merges for #1140 #1171 #1180 #1158 Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ui): stabilize Sources sheet autofocus on open Default UtilityDrawer sheet mode for breakpoint=all and retry sheet autofocus so the Find-a-document field keeps focus after drawer mount races. * ci: retrigger PR policy check on tip * fix(ui): stop stacked Sheet focus steal; assert Escape restore honestly Gate Sheet autofocus retries on isTopmostSheet, and remove test-side focus repair so production restore failures stay visible. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Summary
fields=index, differentials abort/debounce, universal documents typeahead soft-timeout (750ms), shared(search-app)shell to avoid composer remount, and Answer rate-limit in-memory fallback outside production./services→/dsm) syncssearchModeduring render (no stale-mode paint) even when the query string is unchanged; extracted ClinicalDashboard lazy imports to stay under the maintainability budget.RAG impact: no retrieval behaviour change — typeahead documents domain timeout and shell URL sync only; ranking formulas and full
/api/searchretrieval path unchanged.Verification
npm run verify:pr-local— focused Vitest on touched sources (362) plus api-rate-limit / search-shell / universal / route / site-map suites green;docs:check-indexOKverify:uinot required for this pass; mode-home smoke vianpm run ensurereturned HTTP 200 for/,/services,/dsm,/documents/search,/therapy-compass,/?mode=prescribing, and/api/answer/streamreturned 200 after the rate-limit fallback fixeval:retrieval:latency/ soak / live OpenAI canary — approval-gated provider work; not needed for timeout-only typeahead changeRisk and rollout
GlobalSearchShelllayouts and prior timeout/fallback behaviourClinical Governance Preflight
[REDACTED]([REDACTED])Notes
fields=indexremains for identity-only consumers (cross-mode links).