Repository navigation
feat(mobile): Home's New Workspace opens the desktop's create drawer over Home (native part ships with the next app build) - #31305
Merged
Merged
Conversation
…gain The native host-list route dropped the home screen's action=newWorktree when it built the page's route, so the page opened as a plain list. The route now forwards it, and the page names it back once applied so a reload does not reopen the sheet. Reaches phones only with the next native build.
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 47.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 55 functions across 56 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: Home’s New Workspace flow now opens the desktop create drawer over Home. It is specific and related to the changeset. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The pull request description is detailed and follows the required template. It explains the user impact, implementation, rationale, testing, visual proof, compatibility considerations, review notes, a… Full details: Description check
-
Explanation
The pull request description is detailed and follows the required template. It explains the user impact, implementation, rationale, testing, visual proof, compatibility considerations, review notes, and checklist status. The Linked Issue section states "None," which does not meet the template requirement for outside contributors, but the template allows maintainers to omit needless issues; the author’s status is not provided.
-
✨ 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.
Comment @coderabbitai help to get the list of available commands.
Replaces the action=newWorktree route hand-off. The quick action picks a host (as Tasks does), then mounts the host's create sheet on Home; a create opens the new workspace's session marked as just created. The sheet controller becomes a plain visible/onClose component, and the host screen's route-action state, the action route param and its builder are deleted.
Home's cold-start seed is a pathless resume row that the name suggester cannot read, so the sheet now seeds from proven rows and is keyed by host. The host screen gets one setter for the sheet's visibility and its ref.
Jinwoo-H
changed the title
… page's create form Home pushes /new-workspace/<host>, a react-native-screens formSheet over Home whose content is MobileWebShellScreen for the page route /h/<host>/new-workspace. The page renders the shared Create Workspace form with its drawers inline, so the form and its pickers fill the native drawer. The shell's loading, download and failure states render inside the drawer. Prototype only.
…t over a transparent modal The native route is a transparentModal hosting the shell over Home; the page route renders the host screen's NewWorktreeModal on a transparent page, so the drawer is the same component. While the page loads, the shell shows a placeholder of that sheet (scrim, grabber, spinner or download bar), and the page's sheet opens in place without its slide. A host that cannot serve the page (old desktop, blocked, failed) opens the host list instead of a wall in a drawer.
…r a dimmed Home A transparent native overlay route hosts the host picker and then the page's /h/[hostId]/new-workspace route, which renders the same NewWorktreeModal as the host screen's +. The native scrim matches the drawer backdrop and the page's first drawer opens already dimmed, so Home stays dimmed throughout. Old desktops without the route fall back to the host list.
The overlay route owns a single tappable loading layer (spinner, or the download on a small card) that stays mounted from the pick until the page paints, so the fade and spinner no longer restart per shell state. The shell reports its load and shows the host list for anything it cannot open; the picker hands the pick over at once and keeps sliding away. The page's first drawer takes the dimmed backdrop once, and a gate's overlay pending view no longer stacks a second scrim over a mounted stack. Native /h/[hostId]/new-workspace redirects to the overlay. Drops the unused session-route params, the clipboard grant, and pass-through tests.
…Home While the page loads, the overlay shows one short native drawer in the shared drawer chrome with a spinner or the download, and nothing dims Home: a tap above it, a drag down or Back cancels. The page's real drawer then slides up with its own backdrop exactly as on the host screen's +. Drops the native scrim, the loading card and the already-dimmed backdrop handoff; the overlay's WebView stays invisible until the page paints, since a fresh one draws opaque.
The short loading drawer opens with the shared drawer backdrop and show duration, and a tap on that backdrop, a drag down or Back cancels. It stays in the screen's own window (a native Modal over the loading WebView stalls the page) and goes in the frame the page paints: the shell reports its load from a layout effect so the WebView shows and the drawer leaves together.
A drawer opening as its page screen mounts holds that screen's first-paint report until its open animation ends, through a hold on the existing report rather than a new bridge message. The New Workspace overlay's loading drawer now stays until the create drawer is up, so a cold open no longer shows Home undimmed between the two.
…ng drawer fades out The shell no longer imports the feature: an overlay is a shell with onOverlayLoad, which reports `unavailable` for what it cannot show, and the overlay route opens the host list for every exit. HostProtocolGate takes one transparentWhilePending flag. The loading drawer's drag handler follows a stable cancel, and on paint the drawer fades out eased in, as the shell's page cover does, because the WebView takes a frame or two to show the page after it is revealed; removing it outright left Home undimmed for those frames.
…g with it" This reverts commit be0913b.
Jinwoo-H
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
6a088d37-a32d-4db9-beab-6ef6cc442478
📒 Files selected for processing: 35
config/scripts/build-mobile-web-app-bundle.test.mjsconfig/scripts/mobile-web-app-page-route-modules.mjsconfig/scripts/mobile-web-app-session-dictation-capture.test.mjsconfig/scripts/mobile-web-page-routes.mjsconfig/scripts/verify-mobile-web-app-bundle.mjsmobile/app/_layout.tsxmobile/app/_layout.web.tsxmobile/app/h/[hostId]/new-workspace.tsxmobile/app/h/[hostId]/new-workspace.web.tsxmobile/app/h/_layout.tsxmobile/app/new-workspace.tsxmobile/src/components/HostProtocolGate.tsxmobile/src/components/MobileHomeQuickActions.test.tsmobile/src/components/MobileHomeQuickActions.tsxmobile/src/components/bottom-drawer-constants.tsmobile/src/components/mounted-bottom-drawer.tsxmobile/src/components/use-connected-host-picker.tsmobile/src/home/MobileHomeListFooter.test.tsxmobile/src/home/MobileHomeListFooter.tsxmobile/src/home/MobileHomeScreen.tsxmobile/src/mobile-web-shell/MobileWebShellScreen.tsxmobile/src/mobile-web-shell/ShellDevFacts.tsxmobile/src/mobile-web-shell/shell-overlay-load.tsmobile/src/navigation/host-stack.web.tsxmobile/src/new-workspace/NewWorkspaceHostPicker.tsxmobile/src/new-workspace/NewWorkspaceLoadingDrawer.tsxmobile/src/new-workspace/NewWorkspaceOverlay.test.tsxmobile/src/new-workspace/NewWorkspaceOverlay.tsxmobile/src/new-workspace/NewWorkspacePageScreen.test.tsxmobile/src/new-workspace/NewWorkspacePageScreen.tsxmobile/src/new-workspace/new-workspace-overlay-route.test.tsxmobile/src/new-workspace/new-workspace-overlay-route.tsmobile/src/new-workspace/open-host-list-instead.tsxmobile/src/new-workspace/overlay-page-route.tsmobile/src/new-workspace/use-host-worktree-rows.ts
💤 Files with no reviewable changes: 2
- mobile/src/home/MobileHomeListFooter.test.tsx
- mobile/src/components/MobileHomeQuickActions.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
An unreadable host catalog, or offered desktops that are all gone, left the picker invisible inside the transparent overlay, holding a screen over Home with nothing to dismiss it. The catalog read now resolves to no hosts on failure, and an empty pick list goes back to Home.
…does The page drawer opened from Home passed no host labels, so a project on an SSH host showed its run target by raw host id. Reading the SSH target summaries and display overrides moves into one function that both the host screen's metadata fetch and the page drawer call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
On the phone's home screen, tapping New Workspace used to drop you on that computer's workspace list, and you had to tap + again. Now the familiar "Create worktree" drawer opens right over the home screen. If several computers are connected, you first pick one, the same way Tasks asks.
The drawer comes from your computer (the desktop app), so it updates whenever the desktop does. Only the button, the computer picker and a short loading drawer live in the phone app. The home-screen part reaches phones with the next native app build; current phones keep today's behavior until then.
What Changed
Flow: Home → New Workspace → (picker, if more than one desktop is connected) → a short loading drawer (spinner, or the download bar after a desktop update) → the desktop's real Create worktree drawer, identical to the host screen's + → create → the new session.
Native (app build):
/new-workspace?hostIds=…, a transparent modal over Home (app/new-workspace.tsx).NewWorkspaceHostPicker), because iOS can't present one modal while another is presenting.NewWorkspaceLoadingDraweris short and deliberately not form-height. It reuses the shared drawer styles and dim, and tap-outside, drag or Back cancels. It stays mounted for the whole load and fades out when the page paints.onOverlayLoadturns the shell into an overlay. It reportsloading/painted/unavailableand keeps the WebView invisible until the page paints. When the page opens the session it replaces the modal instead of stacking on it.NewWorkspaceOverlayowns every way out. A host that can't serve the drawer (old page without the route, wall, failed, offline, blocked) closes the overlay and opens the host list (OpenHostListInstead)./h/[hostId]/new-workspaceredirects to the overlay, so a future hop there never renders a frozen native form.OTA (served by the desktop):
/h/[hostId]/new-workspace(new-workspace.web.tsx→NewWorkspacePageScreen) renders the sameNewWorktreeModalthe host screen uses, over a transparent page. Closing pops the modal; creating opens the session.Removed / simplified:
?action=newWorktreeparam and its state machine (host-route-action-state.ts) are gone. Only the session-route builder remains, now inhost-new-worktree-session-route.ts.NewWorktreeModalControlleris a plainvisible/onClosecomponent (no imperative handle), and the host screen has one setter for whether its sheet is open.HostProtocolGate: gains two props:transparentWhilePending, andblockedso a blocked host exits the overlay.host-ssh-target-labels-read.ts), so SSH projects show their host's name, not its id.Why
The owner asked for the drawer over Home, with the rule that host-independent UI is native and everything host-dependent is OTA, because native builds are rare. An earlier version of this PR drew a native copy of the drawer on Home. It worked, but that copy would only change with app builds and could drift from the host screen's. This version keeps the native part to the button, the picker and the loading drawer.
Alternatives we tried on the emulator and dropped:
Mixed versions:
actionchanges nothing, because no shipped app ever passed it to the page.Bundle: the new route re-splits chunks it shares with Tasks (tasks goes from 11 to 14 scripts per route). The page-asset ceiling projection now crosses at 25 routes instead of 27. That is a derived envelope, not measured growth: there are 91 scripts across 16 routes today. Noting the lost headroom.
Linked Issue
None. The owner reported this directly.
Visual Proof
Android emulator (API 36): a dev client loads this branch through Metro, and headless dev desktops serve the page built at this commit.
Old desktop (page without the route) falls back to the host list:
Known, accepted:
Testing
Emulator:
Tests:
NewWorkspaceOverlay.test.tsx: the loading drawer is one persistent element across load states, plus the exits.new-workspace-overlay-route.test: single host, picker pick and cancel.NewWorkspacePageScreen.test: close, and create replacing the overlay with the session.NewWorktreeModal.test.tsxforvisible/onClose.routeActionStateis gone.Checks run locally, all passing:
mobile/:tsc --noEmit,oxfmt --check ., fullvitest run --maxWorkers=4.pnpm tc, changed-code quality, React Doctor on changed lines, max-lines ratchet, anti-slop oxlint, root oxlint,build:mobile-web+ verify, mobile web app checks.Failing on main, not from this PR: CI's e2e typecheck ratchet fails on
tests/e2e/helpers/phone-feed-list.tsfor every PR that touchesmobile/, because root and mobile pin different zod versions. #31260 fixes that.Not verified: iOS (reasoned from code: the WebView is already transparent in
MobileWebShellView.swift, the modal is a transparentModal, and the picker runs inside it) and tablets.Review
Four Opus review rounds across the iterations. The last round found no blockers; its cleanups are in:
Agent skill upstream boundary
Notes
NewWorktreeModalandexecutionHostas the host screen.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)