Skip to content

fix(mobile): open page sheets above an already-open keyboard - #33760

Merged
AmoabaKelvin merged 2 commits into
mainfrom
mobile-rename-modal-keyboard
Oct 26, 2026
Merged

AmoabaKelvin merged 2 commits into
mainfrom
mobile-rename-modal-keyboard

Conversation

@AmoabaKelvin

@AmoabaKelvin AmoabaKelvin commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​36 $\color{#cf222e}{\Huge{\mathbf{−}}}$​17 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​19
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​11 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​8

ELI5

On the phone, if you're typing in a terminal and double-tap the tab to rename it, the rename box used to open hidden behind the keyboard. Now it opens on top of the keyboard, where you can see it.

What Changed

  • Before: with the terminal input focused and the keyboard up, double-tapping a terminal tab opened the Rename Terminal sheet at the bottom of the screen, behind the keyboard. The screen dimmed, the rename field had focus, but you couldn't see it.
  • After: the sheet opens sitting on the keyboard's top edge.
  • Mechanism: MountedBottomDrawer seeds a sheet's keyboard lift from the current keyboard height when it opens. That seed only ran for fill sheets. On the over-the-air page, focus moves from the terminal input into the sheet's field without the keyboard moving, so no keyboard event follows and a content-sized sheet stayed at 0. The seed now also runs for every sheet on the page (Platform.OS === 'web'), where the keyboard height comes live from the shell.

This covers every content-sized sheet that opens over an open keyboard on the page, not only Rename Terminal.

Why

Where the bug happens: the tab strip keeps the keyboard up through useKeyboardPersistingTaps, which only does anything on the page. I checked each surface on emulators/simulators:

  • Page (Android shell): reproduced, and fixed by this change (videos below).
  • Native iOS: the sheet already lifts on main, because the sheet's own Modal window gets a fresh keyboardWillShow.
  • Native Android: opening the sheet's Modal window hides the IME, and no keyboardDidHide reaches JS. Seeding there would lift the sheet over a keyboard that is gone, so native content sheets keep skipping the seed. The original reason for skipping them also still holds: Keyboard.metrics() on iOS stays stale between willHide and didHide (fix(mobile): keep name input continuous during source picker transition #10145).

I didn't build on #32027. It adds an opt-in seedKeyboard prop set only by TextInputModal. That would also seed natively, which on Android lifts the sheet over a keyboard that has already gone, and it leaves other page sheets behind the keyboard. Fixing it at the drawer's existing seed gate is smaller and covers every page sheet.

Linked Issue

Fixes #32025

Linear: STA-11744

Visual Proof

Android emulator, real over-the-air shell paired to a dev desktop serving the page. A terminal tab has the keyboard up, then I double-tap the tab.

Before (page build from main):

rename-sheet-before-android-page.webm

After (page build with this change):

rename-sheet-after-android-page.webm

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • Page, Android emulator (OTA dev client against a hidden dev desktop): repro on main, fixed with this change (videos above).

  • Native iOS 27 simulator (dev client, real TextInputModal in a temporary route): already lifts on main, unchanged.

  • Native Android emulator: on main the IME hides when the sheet's Modal opens and the sheet sits at the bottom; unchanged by this PR.

  • Page, iOS simulator: the fixed build loaded, but synthetic double-taps never opened rename in the iOS WebView, so I couldn't capture that path.

  • mounted-bottom-drawer-page-keyboard.web.test.tsx: new case where a content-sized sheet opens while the shell already reports a keyboard. It fails without the change. mounted-bottom-drawer-keyboard-seam.test.tsx still pins the native skip.

  • All 49 drawer-related mobile test files pass. check:code-quality:changed and check:react-doctor:changed are clean.

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Mobile only, and the change is in the page bundle, which ships with the desktop. An older phone shell paired to a newer desktop gets the fix. Native screens are unchanged. Nothing touches SSH, desktop platforms or folder workspaces.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@AmoabaKelvin
AmoabaKelvin marked this pull request as ready for review October 26, 2026 19:42
@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6bf155df-68e7-4a3e-a1d3-d2af6674470e
Commits

Reviewing files that changed from the base of the PR and between 7b5f50d and f8b6c7c.

Files selected for processing: 2
  • mobile/src/components/mounted-bottom-drawer-page-keyboard.web.test.tsx
  • mobile/src/components/mounted-bottom-drawer.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

On web, MountedBottomDrawer now seeds the current keyboard height for both fill-available and content-sized sheets. The web tests use a generalized sheet helper and verify that a content-sized sheet moves upward when mounted while the keyboard is already open.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to f8b6c

The change correctly lifts web content-sized sheets above an already-open keyboard without altering native behavior.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title is concise and accurately describes the main change: page sheets now open above an already-open keyboard.
Description check Passed The description is complete and follows the repository template. It explains the user impact, mechanism, rationale, linked issue, visual proof, testing, scope, and checklist status.
Linked Issues check Passed Issue [#32025] requires TextInputModal to use the current keyboard height when it opens over an already-open IME. MountedBottomDrawer now seeds currentSoftKeyboardHeight() when `Platform.OS === …
Out of Scope Changes check Passed The change is limited to MountedBottomDrawer and its page keyboard test. The broader web condition covers all page sheets that share the reported stale-keyboard state. The condition does not seed na…
Full details: Linked Issues check

Explanation

Issue [#32025] requires TextInputModal to use the current keyboard height when it opens over an already-open IME. MountedBottomDrawer now seeds currentSoftKeyboardHeight() when Platform.OS === 'web', including content-sized sheets. The test lifts a content-sized sheet over a keyboard already up, which sends no new event verifies the resulting translateY of -IME_HEIGHT. The existing fill-sheet condition remains unchanged for native behavior.

Full details: Out of Scope Changes check

Explanation

The change is limited to MountedBottomDrawer and its page keyboard test. The broader web condition covers all page sheets that share the reported stale-keyboard state. The condition does not seed native content sheets, which matches the stated issue scope and platform behavior. No unrelated product area is changed.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@AmoabaKelvin
AmoabaKelvin merged commit 33a86f3 into main Oct 26, 2026
45 checks passed
@AmoabaKelvin
AmoabaKelvin deleted the mobile-rename-modal-keyboard branch October 26, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(mobile): Rename Terminal modal is occluded by soft keyboard when double-tapping tab with IME already open

1 participant