Add experimental paper layout mode for workspace panes#5014
Add experimental paper layout mode for workspace panes#5014Mohit-Bapatla wants to merge 9 commits into
Conversation
|
@Mohit-Bapatla is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a "paper" workspace layout mode with a geometry-backed PaperLayoutState, Workspace integration and lazy initialization, conditional PaperCanvas UI rendering, bonsplit-to-paper split mirroring, paper-mode focus routing, and debug commands to toggle and move the paper viewport. ChangesPaper Workspace Layout Mode
Sequence DiagramsequenceDiagram
participant User
participant WorkspaceContentView
participant Workspace
participant Bonsplit
participant PaperLayoutState
User->>WorkspaceContentView: request split (new terminal split)
WorkspaceContentView->>Workspace: newTerminalSplit(...)
Workspace->>Workspace: check layoutMode == .paper
alt paper mode
Workspace->>PaperLayoutState: ensurePaperLayoutState / focus source pane
Workspace->>Bonsplit: perform bonsplit create split
Bonsplit->>Bonsplit: create new pane/tab
Workspace->>PaperLayoutState: mirrorBonsplitSplit(sourcePane, newPane)
PaperLayoutState->>PaperLayoutState: insert PaperPane, shift downstream frames, update focusedPane & viewport
Workspace->>Workspace: adjust focus bookkeeping & call focusPanel(with forceBonsplitFocusPath)
else bonsplit mode
Workspace->>Bonsplit: perform normal split and focus
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (4 errors, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 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 SummaryThis PR adds an experimental (debug-only) "paper" layout mode to cmux workspaces, inspired by PaperWM/Niri. In paper mode, panes live on a scrollable canvas and splits create a new same-sized pane without shrinking existing ones; navigation snaps the viewport to the nearest pane in a given direction.
Confidence Score: 4/5Safe to merge for a debug/experimental feature; the one identified defect is containable but should be addressed before the paper mode path is widened beyond the current debug toggle. focusPaperPanel unconditionally calls terminalPanel.focus() after a focusPane call that may have silently no-opped — when tabId is not tracked in any PaperPane (reachable via the ensurePaperLayoutState single-pane fallback path), the viewport stays on the old pane while the terminal at panelId receives keyboard focus, so the user types into a terminal that is not displayed. Sources/Workspace.swift — focusPaperPanel focus/viewport mismatch and the focusedPane stale-id fallback. Important Files Changed
Reviews (3): Last reviewed commit: "Fix paper focus viewport handoff" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let bonsplitController: BonsplitController | ||
| @Published var layoutMode: WorkspaceLayoutMode = .bonsplit | ||
| @Published var paperLayoutState: PaperLayoutState? | ||
| private var isRunningBonsplitLifecycleForPaperSplit = false |
There was a problem hiding this comment.
isRunningBonsplitLifecycleForPaperSplit creates a transient hybrid state (layoutMode == .paper && isRunningBonsplitLifecycleForPaperSplit == true) where focusedPanelId (line 9746) silently falls through to bonsplit focus, and focusPanel (line 15266) skips the paper code path entirely. The invariant the flag names — "bonsplit lifecycle operations must not see paper focus during a paper split" — is never expressed in the type system and is unenforceable from outside the call site. If any intermediate step in the bonsplit lifecycle observes focusedPanelId or triggers a callback that calls focusPanel, it gets bonsplit-mode answers while the workspace claims to be in paper mode. The clean fix is to either pass a calledFromPaperSplitMirror: Bool parameter through the lifecycle helpers that need it, or restructure so bonsplit focus mutations during a paper split are skipped at the bonsplit layer rather than suppressed at the paper layer via global flag.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
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!
| enum WorkspaceLayoutMode: String, Codable, Sendable { | ||
| case bonsplit | ||
| case paper | ||
| } |
There was a problem hiding this comment.
PaperLayoutState, PaperPane, PaperPoint, PaperRect, and WorkspaceLayoutMode have no dependency on Workspace internals and are each independently testable value types. Adding them — along with the associated geometry helpers, mirrorBonsplitSplit, rebuildPaperLayoutStateFromBonsplitForDebug, and movePaperViewportForDebug — to Workspace.swift (which is already well over 800 lines) violates the file-package boundary rule. These types belong in a dedicated PaperLayout module or at minimum a separate Swift file, both to reduce Workspace.swift's size and to make the layout logic unit-testable without instantiating a full Workspace.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
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!
| paperState.viewportOrigin = newOrigin | ||
| objectWillChange.send() | ||
| paperLayoutState = paperState |
There was a problem hiding this comment.
objectWillChange.send() fires an extra notification with stale state before the @Published setter fires its own. This causes two SwiftUI render passes — one with the old value, one with the new — instead of one. Because paperLayoutState is @Published, the setter already sends objectWillChange before committing the new value; the explicit call before it is redundant and should be removed.
| paperState.viewportOrigin = newOrigin | |
| objectWillChange.send() | |
| paperLayoutState = paperState | |
| paperState.viewportOrigin = newOrigin | |
| paperLayoutState = paperState |
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@Sources/Workspace.swift`:
- Around line 9395-9654: The Paper layout value types (PaperLayoutState,
PaperPane, PaperPoint, PaperRect and related enum WorkspaceLayoutMode if
desired) should be extracted out of Workspace.swift into a new file named
PaperLayoutTypes.swift: cut the full declarations for PaperLayoutState,
PaperPane, PaperPoint, PaperRect (and WorkspaceLayoutMode if you want) and paste
them into the new file, keep all conformances
(Codable/Equatable/Sendable/Identifiable) and their method implementations
unchanged, add any necessary imports (e.g., Foundation/CoreGraphics) at the top
of the new file, then remove the originals from Workspace.swift and run a build
to fix any missing references or access-level issues (update any internal/public
modifiers if needed).
- Around line 10017-10019: The manual call to objectWillChange.send() is
redundant and ordered incorrectly around the `@Published` paperLayoutState
assignment; remove the explicit objectWillChange.send() (or if you must keep it,
move it to after assigning paperLayoutState) so that updating paperLayoutState =
paperState (and setting paperState.viewportOrigin = newOrigin) is what emits the
change; update the block around paperState.viewportOrigin/newOrigin and
paperLayoutState assignment accordingly to rely on the `@Published` publisher.
In `@Sources/WorkspaceContentView.swift`:
- Line 807: The view is calling paperLayoutState.paneNearestViewportOrigin()
which forwards to paneNearest(to:) and uses panes.min with originDistanceSquared
causing an O(n) scan on every SwiftUI body recompute; change paperLayoutState so
it caches the nearest pane result keyed by viewportOrigin (or stores the
nearestPane and viewportOrigin) and only recomputes when viewportOrigin or pane
frames change (invalidate on geometry updates), or move the nearest-pane
calculation out of the body into a geometry/observer update that updates a
stored property; update functions paneNearestViewportOrigin() and
paneNearest(to:) to return the cached value when valid and recompute+update the
cache when invalidated.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 23aa2967-a03a-41b1-94c3-d906ee47856f
📒 Files selected for processing (3)
Sources/Workspace.swiftSources/WorkspaceContentView.swiftSources/cmuxApp.swift
| enum WorkspaceLayoutMode: String, Codable, Sendable { | ||
| case bonsplit | ||
| case paper | ||
| } | ||
|
|
||
| struct PaperLayoutState: Codable, Equatable, Sendable { | ||
| var panes: [PaperPane] | ||
| var focusedPaneId: UUID? | ||
| var viewportOrigin: PaperPoint | ||
|
|
||
| var focusedPane: PaperPane? { | ||
| if let focusedPaneId, | ||
| let pane = panes.first(where: { $0.id == focusedPaneId }) { | ||
| return pane | ||
| } | ||
| return panes.first | ||
| } | ||
|
|
||
| static func initial(paneId: PaneID, tabId: UUID, viewportSize: CGSize) -> PaperLayoutState { | ||
| let frame = PaperRect( | ||
| x: 0, | ||
| y: 0, | ||
| width: max(900, viewportSize.width), | ||
| height: max(600, viewportSize.height) | ||
| ) | ||
| let pane = PaperPane( | ||
| id: paneId.id, | ||
| frame: frame, | ||
| tabIds: [tabId], | ||
| selectedTabId: tabId | ||
| ) | ||
|
|
||
| return PaperLayoutState( | ||
| panes: [pane], | ||
| focusedPaneId: pane.id, | ||
| viewportOrigin: PaperPoint(x: 0, y: 0) | ||
| ) | ||
| } | ||
|
|
||
| func pane(containingTabId tabId: UUID) -> PaperPane? { | ||
| panes.first { $0.tabIds.contains(tabId) } | ||
| } | ||
|
|
||
| func paneNearestViewportOrigin() -> PaperPane? { | ||
| paneNearest(to: viewportOrigin) | ||
| } | ||
|
|
||
| func paneNearest(to point: PaperPoint) -> PaperPane? { | ||
| panes.min { lhs, rhs in | ||
| let lhsDistance = lhs.frame.originDistanceSquared(to: point) | ||
| let rhsDistance = rhs.frame.originDistanceSquared(to: point) | ||
| if lhsDistance == rhsDistance { | ||
| return lhs.id.uuidString < rhs.id.uuidString | ||
| } | ||
| return lhsDistance < rhsDistance | ||
| } | ||
| } | ||
|
|
||
| func paneInDirection(dx: CGFloat, dy: CGFloat) -> PaperPane? { | ||
| guard let sourcePane = paneNearestViewportOrigin() ?? focusedPane else { return nil } | ||
| let sourceCenterX = sourcePane.frame.midX | ||
| let sourceCenterY = sourcePane.frame.midY | ||
| let epsilon: CGFloat = 0.5 | ||
|
|
||
| let candidates: [PaperPane] | ||
| if abs(dx) >= abs(dy), dx > 0 { | ||
| candidates = panes.filter { $0.id != sourcePane.id && $0.frame.midX > sourceCenterX + epsilon } | ||
| return candidates.min { | ||
| directionalScore($0, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: true) < | ||
| directionalScore($1, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: true) | ||
| } | ||
| } else if abs(dx) >= abs(dy), dx < 0 { | ||
| candidates = panes.filter { $0.id != sourcePane.id && $0.frame.midX < sourceCenterX - epsilon } | ||
| return candidates.min { | ||
| directionalScore($0, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: true) < | ||
| directionalScore($1, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: true) | ||
| } | ||
| } else if dy > 0 { | ||
| candidates = panes.filter { $0.id != sourcePane.id && $0.frame.midY > sourceCenterY + epsilon } | ||
| return candidates.min { | ||
| directionalScore($0, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: false) < | ||
| directionalScore($1, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: false) | ||
| } | ||
| } else if dy < 0 { | ||
| candidates = panes.filter { $0.id != sourcePane.id && $0.frame.midY < sourceCenterY - epsilon } | ||
| return candidates.min { | ||
| directionalScore($0, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: false) < | ||
| directionalScore($1, sourceCenterX: sourceCenterX, sourceCenterY: sourceCenterY, horizontal: false) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| private func directionalScore( | ||
| _ pane: PaperPane, | ||
| sourceCenterX: CGFloat, | ||
| sourceCenterY: CGFloat, | ||
| horizontal: Bool | ||
| ) -> CGFloat { | ||
| let primary = horizontal | ||
| ? abs(pane.frame.midX - sourceCenterX) | ||
| : abs(pane.frame.midY - sourceCenterY) | ||
| let cross = horizontal | ||
| ? abs(pane.frame.midY - sourceCenterY) | ||
| : abs(pane.frame.midX - sourceCenterX) | ||
| return (primary * 10_000) + cross | ||
| } | ||
|
|
||
| private func verticallyOverlaps(_ lhs: PaperRect, _ rhs: PaperRect) -> Bool { | ||
| lhs.minY < rhs.maxY && rhs.minY < lhs.maxY | ||
| } | ||
|
|
||
| private func horizontallyOverlaps(_ lhs: PaperRect, _ rhs: PaperRect) -> Bool { | ||
| lhs.minX < rhs.maxX && rhs.minX < lhs.maxX | ||
| } | ||
|
|
||
| mutating func focusPane(containingTabId tabId: UUID) { | ||
| guard let paneIndex = panes.firstIndex(where: { $0.tabIds.contains(tabId) }) else { return } | ||
| focusedPaneId = panes[paneIndex].id | ||
| panes[paneIndex].selectedTabId = tabId | ||
| } | ||
|
|
||
| @discardableResult | ||
| mutating func insertPaneBesideFocused( | ||
| id: UUID, | ||
| tabId: UUID, | ||
| orientation: SplitOrientation, | ||
| gap: CGFloat = 24 | ||
| ) -> PaperPane? { | ||
| guard let focusedPane = focusedPane else { return nil } | ||
| let frame: PaperRect | ||
| switch orientation { | ||
| case .horizontal: | ||
| frame = PaperRect( | ||
| x: focusedPane.frame.maxX + gap, | ||
| y: focusedPane.frame.minY, | ||
| width: focusedPane.frame.width, | ||
| height: focusedPane.frame.height | ||
| ) | ||
| case .vertical: | ||
| frame = PaperRect( | ||
| x: focusedPane.frame.minX, | ||
| y: focusedPane.frame.maxY + gap, | ||
| width: focusedPane.frame.width, | ||
| height: focusedPane.frame.height | ||
| ) | ||
| } | ||
|
|
||
| let pane = PaperPane( | ||
| id: id, | ||
| frame: frame, | ||
| tabIds: [tabId], | ||
| selectedTabId: tabId | ||
| ) | ||
| panes.append(pane) | ||
| focusedPaneId = id | ||
| viewportOrigin = PaperPoint(x: frame.minX, y: frame.minY) | ||
| return pane | ||
| } | ||
|
|
||
| @discardableResult | ||
| mutating func mirrorBonsplitSplit( | ||
| sourcePane: PaperPane, | ||
| newPaneId: PaneID, | ||
| tabId: UUID, | ||
| orientation: SplitOrientation, | ||
| gap: CGFloat = 24 | ||
| ) -> PaperPane { | ||
| let sourceFrame = sourcePane.frame | ||
| let frame: PaperRect | ||
| switch orientation { | ||
| case .horizontal: | ||
| let shift = sourceFrame.width + gap | ||
| for index in panes.indices { | ||
| guard panes[index].id != sourcePane.id, | ||
| panes[index].id != newPaneId.id, | ||
| panes[index].frame.midX > sourceFrame.midX, | ||
| verticallyOverlaps(panes[index].frame, sourceFrame) else { | ||
| continue | ||
| } | ||
| panes[index].frame.x += shift | ||
| } | ||
| frame = PaperRect( | ||
| x: sourceFrame.maxX + gap, | ||
| y: sourceFrame.minY, | ||
| width: sourceFrame.width, | ||
| height: sourceFrame.height | ||
| ) | ||
| case .vertical: | ||
| let shift = sourceFrame.height + gap | ||
| for index in panes.indices { | ||
| guard panes[index].id != sourcePane.id, | ||
| panes[index].id != newPaneId.id, | ||
| panes[index].frame.midY > sourceFrame.midY, | ||
| horizontallyOverlaps(panes[index].frame, sourceFrame) else { | ||
| continue | ||
| } | ||
| panes[index].frame.y += shift | ||
| } | ||
| frame = PaperRect( | ||
| x: sourceFrame.minX, | ||
| y: sourceFrame.maxY + gap, | ||
| width: sourceFrame.width, | ||
| height: sourceFrame.height | ||
| ) | ||
| } | ||
|
|
||
| let pane = PaperPane( | ||
| id: newPaneId.id, | ||
| frame: frame, | ||
| tabIds: [tabId], | ||
| selectedTabId: tabId | ||
| ) | ||
| if let index = panes.firstIndex(where: { $0.id == newPaneId.id }) { | ||
| panes[index] = pane | ||
| } else { | ||
| panes.append(pane) | ||
| } | ||
| focusedPaneId = pane.id | ||
| viewportOrigin = PaperPoint(x: frame.minX, y: frame.minY) | ||
| return pane | ||
| } | ||
| } | ||
|
|
||
| struct PaperPane: Codable, Equatable, Identifiable, Sendable { | ||
| var id: UUID | ||
| var frame: PaperRect | ||
| var tabIds: [UUID] | ||
| var selectedTabId: UUID? | ||
| } | ||
|
|
||
| struct PaperPoint: Codable, Equatable, Sendable { | ||
| var x: CGFloat | ||
| var y: CGFloat | ||
| } | ||
|
|
||
| struct PaperRect: Codable, Equatable, Sendable { | ||
| var x: CGFloat | ||
| var y: CGFloat | ||
| var width: CGFloat | ||
| var height: CGFloat | ||
|
|
||
| var cgRect: CGRect { | ||
| CGRect(x: x, y: y, width: width, height: height) | ||
| } | ||
|
|
||
| var minX: CGFloat { x } | ||
| var minY: CGFloat { y } | ||
| var midX: CGFloat { x + (width / 2) } | ||
| var midY: CGFloat { y + (height / 2) } | ||
| var maxX: CGFloat { x + width } | ||
| var maxY: CGFloat { y + height } | ||
|
|
||
| func originDistanceSquared(to point: PaperPoint) -> CGFloat { | ||
| let dx = x - point.x | ||
| let dy = y - point.y | ||
| return (dx * dx) + (dy * dy) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Consider extracting paper layout types to a separate file.
The PaperLayoutState, PaperPane, PaperPoint, and PaperRect types (~260 lines) are self-contained value types with no dependencies on Workspace internals. Extracting them to Sources/PaperLayoutTypes.swift would keep Workspace.swift from growing further and align with the file-length guidelines.
🤖 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 `@Sources/Workspace.swift` around lines 9395 - 9654, The Paper layout value
types (PaperLayoutState, PaperPane, PaperPoint, PaperRect and related enum
WorkspaceLayoutMode if desired) should be extracted out of Workspace.swift into
a new file named PaperLayoutTypes.swift: cut the full declarations for
PaperLayoutState, PaperPane, PaperPoint, PaperRect (and WorkspaceLayoutMode if
you want) and paste them into the new file, keep all conformances
(Codable/Equatable/Sendable/Identifiable) and their method implementations
unchanged, add any necessary imports (e.g., Foundation/CoreGraphics) at the top
of the new file, then remove the originals from Workspace.swift and run a build
to fix any missing references or access-level issues (update any internal/public
modifiers if needed).
| @ViewBuilder | ||
| private func paperCanvasView(_ paperLayoutState: PaperLayoutState, viewportSize: CGSize) -> some View { | ||
| ZStack(alignment: .topLeading) { | ||
| if let activePane = paperLayoutState.paneNearestViewportOrigin() { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify paneNearestViewportOrigin implementation for hot-path efficiency
# Check if the method caches results or is O(1)
ast-grep --pattern $'func paneNearestViewportOrigin($$$) -> $_ {
$$$
}'
# Also search for the method definition with rg for context
rg -A 20 'func paneNearestViewportOrigin' --type swiftRepository: manaflow-ai/cmux
Length of output: 1565
Avoid O(n) linear scan in paneNearestViewportOrigin() during SwiftUI body evaluation
Sources/WorkspaceContentView.swift calls paperLayoutState.paneNearestViewportOrigin() from the view body; paneNearestViewportOrigin() forwards to paneNearest(to:), which uses panes.min { ... originDistanceSquared ... }—computing distances across all panes each time (O(n) per body recompute). Cache the nearest pane keyed by viewportOrigin and invalidate only when pane frames/viewport origin change, or compute it in response to geometry updates and have the body read the precomputed result.
🤖 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 `@Sources/WorkspaceContentView.swift` at line 807, The view is calling
paperLayoutState.paneNearestViewportOrigin() which forwards to paneNearest(to:)
and uses panes.min with originDistanceSquared causing an O(n) scan on every
SwiftUI body recompute; change paperLayoutState so it caches the nearest pane
result keyed by viewportOrigin (or stores the nearestPane and viewportOrigin)
and only recomputes when viewportOrigin or pane frames change (invalidate on
geometry updates), or move the nearest-pane calculation out of the body into a
geometry/observer update that updates a stored property; update functions
paneNearestViewportOrigin() and paneNearest(to:) to return the cached value when
valid and recompute+update the cache when invalidated.
|
Updated the PR with a review-feedback pass:
I left the paper layout value types in For the |
| return panel | ||
| } | ||
|
|
||
| @discardableResult | ||
| func ensurePaperLayoutState(viewportSize: CGSize = CGSize(width: 900, height: 600)) -> PaperLayoutState? { | ||
| if let paperLayoutState { | ||
| return paperLayoutState | ||
| } | ||
|
|
||
| guard let paneId = bonsplitController.focusedPaneId ?? bonsplitController.allPaneIds.first else { | ||
| return nil | ||
| } | ||
| guard let tabId = bonsplitController.selectedTab(inPane: paneId)?.id ?? | ||
| bonsplitController.tabs(inPane: paneId).first?.id else { | ||
| return nil | ||
| } | ||
|
|
||
| let initialState = PaperLayoutState.initial( | ||
| paneId: paneId, | ||
| tabId: tabId.uuid, | ||
| viewportSize: viewportSize | ||
| ) | ||
| paperLayoutState = initialState | ||
| return initialState | ||
| } | ||
|
|
||
| private func focusPaperPanel(tabId: TabID, panelId: UUID) { | ||
| guard layoutMode == .paper else { return } | ||
| if paperLayoutState == nil { | ||
| _ = ensurePaperLayoutState() | ||
| } | ||
| paperLayoutState?.focusPane(containingTabId: tabId.uuid) | ||
|
|
||
| if let terminalPanel = terminalPanel(for: panelId) { | ||
| terminalPanel.focus() |
There was a problem hiding this comment.
focusPaperPanel leaves viewportOrigin stale, disconnecting the focused pane from the rendered pane
focusPaperPanel updates paperLayoutState.focusedPaneId and calls terminalPanel.focus(), but never advances viewportOrigin. The view renders exclusively paneNearestViewportOrigin(). Any call to focusPanel that reaches the paper branch for a panel whose pane is not currently nearest the viewport — for example from the quick-switcher, a notification-click handler, focusPanel at the workspace-tab-activation site, or a Bonsplit callback — will direct keyboard input to a terminal that is not in the SwiftUI view hierarchy. The focus() call silently fails (the view is not mounted), paperLayoutState.focusedPaneId records pane B, but the user sees pane A. movePaperViewportForDebug is the only current repair path, but nothing calls it automatically after a programmatic focus.
The fix is to move viewportOrigin to the target pane's frame origin inside focusPaperPanel, matching what insertPaneBesideFocused and mirrorBonsplitSplit already do.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Sources/Workspace.swift`:
- Around line 13609-13613: previousFocusedPanelId is being repurposed for the
paper-split handoff when shouldMirrorPaperSplit is true, causing
preserveFocusAfterNonFocusSplit(...) to restore focus to panelId (the paper
split source) instead of the pane that was actually focused; fix by introducing
and using two distinct values: keep previousFocusedPanelId = focusedPanelId (the
true prior focus) for calls like preserveFocusAfterNonFocusSplit(...) and create
a separate paperHandoffPanelId (or similar) set to shouldMirrorPaperSplit ?
panelId : focusedPanelId to use where the code needs the "paper handoff" source;
likewise compute previousHostedView separately (use terminalPanel(for:
panelId)?.hostedView for the handoff view and focusedTerminalPanel?.hostedView
for the true prior-hosted view) and pass the correct one into
focusPanel/preserveFocusAfterNonFocusSplit callers.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: e3f94f09-1429-438a-8e81-1739337c56bd
📒 Files selected for processing (1)
Sources/Workspace.swift
| private func focusPaperPanel(tabId: TabID, panelId: UUID) { | ||
| guard layoutMode == .paper else { return } | ||
| guard var state = paperLayoutState ?? ensurePaperLayoutState() else { | ||
| return | ||
| } | ||
| state.focusPane(containingTabId: tabId.uuid) | ||
| if let focusedPane = state.focusedPane { | ||
| state.viewportOrigin = PaperPoint( | ||
| x: max(0, focusedPane.frame.minX), | ||
| y: max(0, focusedPane.frame.minY) | ||
| ) | ||
| } | ||
| paperLayoutState = state | ||
|
|
||
| if let terminalPanel = terminalPanel(for: panelId) { | ||
| terminalPanel.focus() | ||
| } else if let browserPanel = panels[panelId] as? BrowserPanel { | ||
| maybeAutoFocusBrowserAddressBarOnPanelFocus(browserPanel, trigger: .standard) | ||
| } | ||
| } |
There was a problem hiding this comment.
state.focusPane(containingTabId:) is a silent no-op when tabId is not tracked in any PaperPane — it returns early without updating focusedPaneId. Because focusedPane then falls back to whatever pane was already focused, viewportOrigin advances to that same pane (or panes.first), not to the panel being focused. Yet terminalPanel.focus() is called unconditionally, so the terminal at panelId receives keyboard input even though paneNearestViewportOrigin() still renders the old pane. The concrete trigger: ensurePaperLayoutState creates a single-pane state; any call to focusPanel for a tab from a different bonsplit pane reaches focusPaperPanel, the lookup fails silently, the user sees pane A but types into pane B's terminal.
The fix is to check whether the tab was actually found before proceeding: e.g. guard state.pane(containingTabId: tabId.uuid) != nil else { return } before calling state.focusPane (and converting focusPane to return a Bool), or checking the post-focusPane focusedPaneId matches tabId's pane.
Adds an experimental Niri/PaperWM-style 2D infinite-canvas layout mode
behind the Debug menu, opt-in only.
- WorkspaceLayoutMode { bonsplit, paper }
- PaperLayoutState + PaperPane + PaperPoint + PaperRect (Codable, value types)
- Workspace gains: ensurePaperLayoutState, togglePaperLayoutModeForDebug,
rebuildPaperLayoutStateFromBonsplitForDebug, movePaperViewportForDebug,
paper-mirroring on terminal split (mirrorBonsplitSplit)
- WorkspaceContentView body dispatches by layoutMode: paper case renders
PaperCanvasWorkspaceView (one focused pane scaled to viewport),
bonsplit case keeps fork's existing top-tab + layout-tab tree
- Debug menu: Toggle Selected Workspace Paper Layout, Paper View
Left/Right/Up/Down (1200/800 increments), all gated by #if DEBUG
Source: cherry-picked from manaflow-ai/cmux PR manaflow-ai#5014 (commits
0133507..997c47e), squashed and adapted to fork's layoutTabs
structure. Fork's bonsplitController accessor (computed from active
layout tab) is API-compatible with PR's expectations, so PaperCanvas
renders unchanged.
Persistence + close pane behavior intentionally not implemented in
this drop. PR 5014 is OPEN upstream and Debug-only by design; if it
graduates we should re-pull.
1 participant
Summary
Adds an experimental/debug paper layout mode for cmux workspaces inspired by Niri/PaperWM behavior.
In paper mode:
PaperLayoutState.Testing
./scripts/reload.sh --tag mohit-paper-layout --launchNeed help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds an experimental “paper” layout for workspace panes: panes live on a scrollable canvas, and splits create a new same‑size pane without shrinking others. Also fixes focus-to-viewport handoff so focus changes and terminal splits snap the viewport reliably.
New Features
WorkspaceLayoutMode.paperwithPaperLayoutState/PaperPaneto track panes, selected tabs, and a viewport with directional snapping.Bug Fixes
Written for commit 997c47e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Chores (Debug)