Repository navigation
iOS: make terminal picker a native Menu (tap + press-drag-release) - #5618
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughWorkspaceDetailView's terminal picker migrates from a custom Button+popover to SwiftUI's native Menu component. The Menu displays sectioned options for available terminals, new workspace/terminal creation, and debug tooling, with selection routed through action handlers that no longer manage picker presentation state. ChangesTerminal Picker Menu Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (17 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 |
Greptile SummaryReplaces the iOS terminal picker from a
Confidence Score: 5/5Safe to merge; this is a well-scoped SwiftUI refactor that removes state and complexity without changing any observable selection logic. The change is narrowly confined to toolbar presentation mechanics. All selection paths, keyboard-dismiss behavior, accessibility identifiers, and localization strings are preserved verbatim. The only deliberate UX delta (keyboard stays up while the menu is open) is clearly documented in both the PR description and an inline code comment. No new state, no new concurrency, no new layout logic was introduced. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Old["Old: Button + .popover (two hit-test sessions)"]
A1[User taps terminal button] --> B1[dismissTerminalKeyboardForChrome\ncalled on button tap]
B1 --> C1[isTerminalPickerPresented = true\npopover presented]
C1 --> D1[User taps an item in popover]
D1 --> E1[selectTerminalFromPicker\nor createWorkspace/Terminal]
E1 --> F1[isTerminalPickerPresented = false\nkeyboard dismissed again]
end
subgraph New["New: Native SwiftUI Menu (single gesture)"]
A2[User taps OR press-and-holds\nterminal button] --> B2[UIMenu opens\nkeyboard stays visible]
B2 --> C2{Gesture type}
C2 -->|Tap to open then tap| D2[Tap menu item]
C2 -->|Press-drag-release| E2[Drag to item, release]
D2 --> F2[selectTerminalFromPicker\nor createWorkspace/Terminal]
E2 --> F2
F2 --> G2[dismissTerminalKeyboardForChrome\ncalled on selection]
end
Reviews (3): Last reviewed commit: "Dedupe identical New Workspace helper in..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private func createWorkspaceFromTerminalPicker() { | ||
| dismissTerminalKeyboardForChrome() | ||
| isTerminalPickerPresented = false | ||
| createWorkspace() | ||
| } |
There was a problem hiding this comment.
createWorkspaceFromTerminalPicker is now byte-for-byte identical to createWorkspaceFromToolbar after the isTerminalPickerPresented = false removal. The only functional difference between the two was that presentation state mutation, which is gone. Having two identically-bodied private helpers makes it easy for them to drift unintentionally. One could be removed and the menu item's Button(action:) could point at createWorkspaceFromToolbar instead.
| private func createWorkspaceFromTerminalPicker() { | |
| dismissTerminalKeyboardForChrome() | |
| isTerminalPickerPresented = false | |
| createWorkspace() | |
| } | |
| // Merged into createWorkspaceFromToolbar — both paths are identical after | |
| // the isTerminalPickerPresented = false removal. |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Done in 99c3d85 — pointed the menu's New Workspace action at createWorkspaceFromToolbar and deleted the duplicate createWorkspaceFromTerminalPicker.
iOS CI flake analysis (not caused by this change)The The Menu itself works. Two of the three UI tests that drive the picker passed on both simulators across these runs: The red came from two flake families:
Isolation proves it is ambient, not causal: running Re-running the full |
The top-right terminal picker on the mobile workspace detail screen used a Button toggling a custom .popover. That is two separate hit-test sessions (tap to present, then tap an item), so it never supported the standard iOS menu gesture: a single press-and-drag from the button onto an item with a release-to-select. Replace it with a native SwiftUI Menu, which renders as a UIMenu on iOS and gets tap-to-open plus press-drag-release-to-select for free. The terminal list, selected checkmark, New Workspace / New Terminal actions, and the DEBUG-only entries are preserved as Menu sections; selection still routes through selectTerminalFromChrome. The only behavior change is that the menu opens over a live keyboard instead of dismissing it first (Menu has no will-open hook); the keyboard still clears on selection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After dropping the popover presentation flag, createWorkspaceFromTerminalPicker became byte-for-byte identical to createWorkspaceFromToolbar. Point the menu's New Workspace action at createWorkspaceFromToolbar and remove the duplicate so the two paths cannot drift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
99c3d85 to
76240aa
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 76240aa. Configure here.
| // DEV-only debug tooling; not shipped, so not localized. | ||
| Label("Send to agent", systemImage: "paperplane") | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| .contentShape(Rectangle()) |
There was a problem hiding this comment.
Feedback sheet omits keyboard dismiss
Low Severity
openFeedbackComposerFromMenu presents the feedback sheet without calling dismissTerminalKeyboardForChrome. Other menu actions dismiss the terminal keyboard on selection, but this DEBUG path no longer inherits dismissal from opening the old popover, so the Ghostty input and keyboard geometry can stay active under the sheet.
Reviewed by Cursor Bugbot for commit 76240aa. Configure here.
iOS CI failure attribution (updated, precise)The The Menu works. All three picker UI tests reach and tap the Two distinct failure modes, neither caused by a picker view change:
I rebased onto current A base-SHA-no-diff control run is queued to convert the pairing asymmetry from "unexplained" to "confirmed environment"; it is waiting on the saturated runner pool. |
Root cause confirmed: CI infra flake, not this diffA control run of the full iOS suite on the branch's base SHA with ZERO diff ( The iphone job log shows the actual root cause:
This is a CI infrastructure flake (504 on the GhosttyKit fetch) plus a broken from-source GhosttyKit fallback on the runner. It is independent of this diff and reproduces with no diff at all. Recommendation: re-run |
…, richer desync trace Item 1 (core): the docked accessory toolbar now stays pinned to the keyboard top through BOTH a terminal resize and a composer field-grow. - Resize/letterbox (composer closed): the docked bar's button row was top-pinned inside its container, so a letterboxed terminal (render bottom above the reserved top) grew the container upward and the buttons rode UP off the keyboard. Bottom-pin the button row to the container's bottom (minus the new bottom padding) in TerminalInputTextView so the controls hug the keyboard top no matter how tall the container grows; only the pure-background slack above them grows. dockedToolbarFrame comment updated to match. - Composer-grow (composer open): round 5 put the field + toolbar in ONE VStack inside ONE bottom safeAreaInset, so a keystroke reflowed the whole stack as a unit and the toolbar drifted. Re-approach: split into TWO stacked bottom insets in WorkspaceDetailView — field inset applied first (inner), toolbar inset applied second (outer, pinned at keyboard top). A field-grow now changes only the field inset's height (pushing the terminal up) and structurally cannot move the constant-height toolbar inset. TerminalComposerView is now field-only; the toolbar host is a sibling inset. Item 2: add dockedBottomPadding (8pt) below the control row so the bar is not flush-tight at its bottom; the top stays snug to the terminal's last row. Folded into dockedButtonRowHeight (now nub 28 + pad 8 = 36) so the grid reserve, the surface frame, and the composer host frame all stay in agreement. Item 3: spawn the floating dogfood pill ~30px lower (default accumulatedOffset height 64 -> 94) so it clears the top-left connection pill / nav chrome on launch. Item 4: fold in #5618 — the top-right terminal picker is now a native SwiftUI Menu (tap + press-drag-release) instead of a Button+popover (applied the picker-menu branch's two WorkspaceDetailView commits directly, not a 43-file merge). Item 5 (diagnostics only, no fix): make the composer-up/keyboard-down desync trace conclusive. Code 24 now mirrors code 23 (adds FR-owner identity b + keyboardHeight ms, not just proxyIsFR). Added a keyboardWillHide record (while composerActive) so the swipe/hardware-keyboard/backgrounding dismiss paths — which never hit the dismiss button — still log the desync onset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
What
The top-right terminal picker on the iOS workspace detail screen (the
terminalbutton that lists all terminals in the workspace) used aButtontoggling a custom.popover. AButton+.popoveris two separate hit-test sessions: you tap the button to present the popover, then tap an item. It never supported the standard iOS menu gesture, where you press-and-drag from the button straight onto an item and release to select it.This replaces it with a native SwiftUI
Menu. On iOS,Menurenders as aUIMenu, which gives both interaction modes for free: single tap to open then tap an item, and press-drag-release-to-select, exactly like any native nav-bar menu.Behavior preserved
selectTerminalFromChrome(keyboard suppressed on attach, same as before).Sections.MobileTerminalDropdown,MobileTerminalMenuItem-*,MobileNewTerminalMenuItem,MobileNewWorkspaceMenuItem) are kept on the same buttons.Only behavior change
Menuhas no will-open hook, so the keyboard is no longer dismissed when the menu opens (the menu floats over the live keyboard like any nav-bar menu). The keyboard still clears on selection, since the action handlers calldismissTerminalKeyboardForChrome().Verification
cmux-iosworkspace scheme (arm64 simulator, build-only, no device, no test, no launch): BUILD SUCCEEDED.testTerminalDropdownSwitchesToAlternateScreenSnapshot,testWorkspaceToolbarCreatesWorkspaceAndTerminal,testTUITerminalUsesAvailableViewportAndResizes) run on theiOS simulator testsworkflow, which gates this PR.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Localized SwiftUI toolbar change on iOS with preserved accessibility IDs and selection logic; minor keyboard-on-open behavior change only.
Overview
Replaces the workspace toolbar terminal picker from a
Button+ custom.popoverto a native SwiftUIMenu(iOSUIMenu), so users get standard tap-to-open and press-drag-release selection instead of two separate tap sessions.The menu content is reorganized into
Sections (terminals, new workspace/terminal actions, DEBUG tools) with less custom padding/layout; accessibility identifiers and selection viaselectTerminalFromPicker/selectTerminalFromChromeare unchanged. New Workspace from the menu now usescreateWorkspaceFromToolbardirectly (duplicate picker-only helper removed).Only UX delta: the keyboard is no longer dismissed when the menu opens (
Menuhas no will-open hook); it still dismisses when an item is chosen because actions calldismissTerminalKeyboardForChrome().Reviewed by Cursor Bugbot for commit 99c3d85. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Replaced the terminal picker popover with a native SwiftUI
Menuin the iOS workspace toolbar, enabling tap and press-drag-release selection and preserving existing items/accessibility IDs. The "New Workspace" action now usescreateWorkspaceFromToolbar; the keyboard stays up on open and still dismisses on selection.Written for commit 76240aa. Summary will update on new commits.
Summary by CodeRabbit
UI/UX Improvements
Bug Fixes
New Features