Repository navigation
Restore iOS scroll-to-bottom toolbar button - #6526
Closed
lawrencecchen wants to merge 11 commits into
Closed
lawrencecchen wants to merge 11 commits into
lawrencecchen wants to merge 11 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a configurable ChangesScroll to Bottom terminal flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User as User
participant SurfaceView as GhosttySurfaceView
participant Representable as GhosttySurfaceRepresentable
participant Shell as MobileShellComposite
participant Controller as TerminalController
participant Surface as TerminalSurface
User->>SurfaceView: tap scroll-to-bottom
SurfaceView->>Representable: notify delegate
Representable->>Shell: scrollTerminalToBottom(surfaceID)
Shell->>Controller: mobile.terminal.scroll_to_bottom
Controller->>Surface: mobileScrollToBottom()
Surface-->>Controller: success
Controller-->>Shell: success response
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 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 |
Contributor
Greptile SummaryRestores a configurable Scroll to Bottom button to the iOS terminal input-accessory toolbar. Tapping it jumps the local Ghostty mirror via the existing
Confidence Score: 4/5Safe to merge after adding the missing terminal.shortcut.name.scrollToBottom catalog entry; all core RPC, auth, and rendering paths look correct. The new RPC, Mac surface mutation, iOS floating button, and v3 toolbar migration are all well-structured and follow existing patterns. The only concrete defect is a missing string-catalog key (terminal.shortcut.name.scrollToBottom) used in the toolbar customization settings UI — Japanese users who open the shortcuts editor will silently see the English default rather than a translated label. Every peer key in that namespace (pageDown, pageUp, home, end, rightArrow) has both en and ja entries; this one is entirely absent. ios/cmux/Resources/Localizable.xcstrings — the shortcut name key for Scroll to Bottom is missing. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User as User (iPhone)
participant GSVIOS as GhosttySurfaceView (iOS)
participant Shell as MobileShellComposite
participant Mac as TerminalController (Mac)
participant Surface as TerminalSurface (Mac)
User->>GSVIOS: Tap ScrollToBottomButton
GSVIOS->>GSVIOS: scrollToBottom() — reset localScrollbackLineOffset
GSVIOS->>GSVIOS: ghostty_surface_binding_action("scroll_to_bottom") on outputQueue
GSVIOS->>Shell: ghosttySurfaceViewDidRequestScrollToBottom()
Shell->>Mac: RPC mobile.terminal.scroll_to_bottom
Mac->>Mac: ticketTerminalAuthorizationError check
Mac->>Surface: mobileScrollToBottom()
Surface->>Surface: ghostty_surface_binding_action("scroll_to_bottom")
Mac->>Mac: MobileTerminalRenderObserver.noteTerminalBytes()
Mac-->>Shell: Render frame (atBottom: true)
Shell->>Shell: updateTerminalScrolledUp(scrolledUp: false)
Shell-->>GSVIOS: setScrolledUp(false) via SwiftUI binding
GSVIOS->>GSVIOS: Hide ScrollToBottomButton
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User as User (iPhone)
participant GSVIOS as GhosttySurfaceView (iOS)
participant Shell as MobileShellComposite
participant Mac as TerminalController (Mac)
participant Surface as TerminalSurface (Mac)
User->>GSVIOS: Tap ScrollToBottomButton
GSVIOS->>GSVIOS: scrollToBottom() — reset localScrollbackLineOffset
GSVIOS->>GSVIOS: ghostty_surface_binding_action("scroll_to_bottom") on outputQueue
GSVIOS->>Shell: ghosttySurfaceViewDidRequestScrollToBottom()
Shell->>Mac: RPC mobile.terminal.scroll_to_bottom
Mac->>Mac: ticketTerminalAuthorizationError check
Mac->>Surface: mobileScrollToBottom()
Surface->>Surface: ghostty_surface_binding_action("scroll_to_bottom")
Mac->>Mac: MobileTerminalRenderObserver.noteTerminalBytes()
Mac-->>Shell: Render frame (atBottom: true)
Shell->>Shell: updateTerminalScrolledUp(scrolledUp: false)
Shell-->>GSVIOS: setScrolledUp(false) via SwiftUI binding
GSVIOS->>GSVIOS: Hide ScrollToBottomButton
|
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalScrollDelivery.swift (1)
91-93: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the remote workspace mapping for both scroll RPCs.
This change fixes
mobile.terminal.scroll, butscrollTerminalToBottomstill sends the localworkspaceID.rawValue. In paired-workspace mode, the absolute scroll action can therefore resolve the wrong Mac workspace or returnnot_found. Apply the sameremoteWorkspaceID(for:)mapping in the scroll-to-bottom request, preferably through one shared parameter-building path.Proposed fix
- "workspace_id": workspaceID.rawValue, + "workspace_id": remoteWorkspaceID(for: workspaceID).rawValue,🤖 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 `@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+TerminalScrollDelivery.swift around lines 91 - 93, Update scrollTerminalToBottom to use remoteWorkspaceID(for:) when constructing its RPC parameters, matching the mapping already used by the other scroll request. Prefer sharing the parameter-building path so both scroll RPCs consistently send the remote workspace identifier while preserving their existing action-specific behavior.
🤖 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.
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+TerminalScrollDelivery.swift:
- Around line 91-93: Update scrollTerminalToBottom to use
remoteWorkspaceID(for:) when constructing its RPC parameters, matching the
mapping already used by the other scroll request. Prefer sharing the
parameter-building path so both scroll RPCs consistently send the remote
workspace identifier while preserving their existing action-specific behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bdae5b9c-df5f-4b31-8eed-204c222599c0
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (11)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileTerminalRenderGrid.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/MobileTerminalRenderGridTests.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalOutputDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalScrollDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerSubmitRoutingTestSupport.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalOutputDeliveryQueueTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/ScrollToBottomButton.swift
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
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
2 participants
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.
Summary
scroll_to_bottombinding on the serial surface queue.Validation
git diff --checkpassed.ios/cmux/Resources/Localizable.xcstringsas JSON and verified both new keys haveenandjalocalizations.swift test --filter TerminalAccessoryConfigurationTestswas blocked by existing SwiftPM package platform metadata: iOS packages infer macOS 10.13 while dependencies require macOS 14.dog: https://github.com/manaflow-ai/cmux/actions/runs/27892632221dog: https://github.com/manaflow-ai/cmux/actions/runs/27892741863CMUX_PORT=3800; warmed/,/handler/sign-in, and/handler/after-sign-into final 200 responses.Dogfood Notes
dev.cmux.ios.dogon Lawrence’s iPhone.ios/scripts/reload.sh --tag dogrefuses local xcodebuild without override, and the override attempt was terminated during package resolution with exit 143.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Adds a new mobile terminal RPC and Mac viewport mutation, but it mirrors the existing scroll forward path and reuses the same authorization gates rather than changing auth or data handling.
Overview
Restores a Scroll to Bottom action on the iOS terminal input accessory. Tapping it scrolls the on-device Ghostty mirror via the
scroll_to_bottombinding (still dispatched on the serial surface queue) and notifies the host so the paired Mac viewport jumps to match.The shell sends a new
mobile.terminal.scroll_to_bottomRPC; the Mac handles it like other terminal mobile calls (workspace/surface validation, ticket auth) and invokesmobileScrollToBottom()on the real surface.GhosttySurfaceRepresentablewires the new delegate callback toscrollTerminalToBottom.The action is user-configurable (icon, labels, default placement after Page Down) with a one-time v3 toolbar migration so existing layouts gain the button without wiping hide/reorder choices. Adds EN/JA strings and tests for RPC routing, defaults, and migration.
Reviewed by Cursor Bugbot for commit f4efc11. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Restores a configurable Scroll to Bottom button in the iOS terminal toolbar and adds a floating in-terminal jump-to-bottom button that appears when the viewport is scrolled up. Tapping jumps the local mirror via Ghostty’s
scroll_to_bottomand sendsmobile.terminal.scroll_to_bottomso the Mac viewport matches.New Features
scrollToBottomtoolbar action with “arrow.down.to.line” icon; no byte output. Default placement: after Page Down. EN/JA labels and shortcut name.at_bottomonMobileTerminalRenderGridFrame; iOS tracks per-surface scrolled state to drive the button. Routed fromTerminalInputTextView→GhosttySurfaceView(local jump + redraw) →GhosttySurfaceRepresentable→scrollTerminalToBottom()→mobile.terminal.scroll_to_bottom→mobileScrollToBottom().Migration
Written for commit d4e6c0d. Summary will update on new commits.
Summary by CodeRabbit