From 79e0db7c8b2c36fd787fa887b9ad7d73daee302d Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 1 May 2026 22:49:28 -0700 Subject: [PATCH 1/7] test: cover browser tab sidebar workspace drops --- cmuxTests/SidebarOrderingTests.swift | 96 ++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/cmuxTests/SidebarOrderingTests.swift b/cmuxTests/SidebarOrderingTests.swift index da58fd0761da..b18bbb6e5b4a 100644 --- a/cmuxTests/SidebarOrderingTests.swift +++ b/cmuxTests/SidebarOrderingTests.swift @@ -804,6 +804,102 @@ final class SidebarDropPlannerTests: XCTestCase { XCTAssertEqual(targetIndex, 2) } + func testWorkspaceDropCenterTargetsExistingWorkspace() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 56), + targets: targets + ) + + XCTAssertEqual(action, .existingWorkspace(second)) + } + + func testWorkspaceDropTopEdgeCreatesWorkspaceBeforeTarget() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 42), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 1, + indicator: SidebarDropIndicator(tabId: second, edge: .top) + ) + ) + } + + func testWorkspaceDropGapCreatesWorkspaceBeforeNextTarget() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 36), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 1, + indicator: SidebarDropIndicator(tabId: second, edge: .top) + ) + ) + } + + func testWorkspaceDropAfterLastRowFallsThroughToEmptyAreaDropTarget() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + XCTAssertNil( + SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 92), + targets: targets + ) + ) + } + + func testWorkspaceDropKeepsNewWorkspaceAfterPinnedRows() { + let pinnedA = UUID() + let pinnedB = UUID() + let unpinned = UUID() + let targets = workspaceDropTargets([pinnedA, pinnedB, unpinned], pinnedIds: [pinnedA, pinnedB]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 2), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 2, + indicator: SidebarDropIndicator(tabId: unpinned, edge: .top) + ) + ) + } + + private func workspaceDropTargets( + _ ids: [UUID], + pinnedIds: Set = [] + ) -> [SidebarDropPlanner.WorkspaceDropTarget] { + ids.enumerated().map { index, id in + SidebarDropPlanner.WorkspaceDropTarget( + workspaceId: id, + isPinned: pinnedIds.contains(id), + frame: CGRect(x: 0, y: CGFloat(index * 40), width: 180, height: 32) + ) + } + } } From 0e22699bfa25dd29568a831c0f2b1f8ec3479e3a Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 1 May 2026 22:49:31 -0700 Subject: [PATCH 2/7] fix: use shared workspace move path for browser tab sidebar drops --- .../AppDelegate+MoveTabToNewWorkspace.swift | 10 +- Sources/ContentView.swift | 293 +++++++++++++++++- Sources/TabManager+DetachedWorkspace.swift | 3 +- 3 files changed, 301 insertions(+), 5 deletions(-) diff --git a/Sources/AppDelegate+MoveTabToNewWorkspace.swift b/Sources/AppDelegate+MoveTabToNewWorkspace.swift index 06d5b7838815..7c37d57a0e55 100644 --- a/Sources/AppDelegate+MoveTabToNewWorkspace.swift +++ b/Sources/AppDelegate+MoveTabToNewWorkspace.swift @@ -32,7 +32,8 @@ extension AppDelegate { title: String? = nil, focus: Bool = true, focusWindow: Bool = true, - placementOverride: NewWorkspacePlacement? = nil + placementOverride: NewWorkspacePlacement? = nil, + insertionIndexOverride: Int? = nil ) -> SurfaceNewWorkspaceMoveResult? { guard let located = locateBonsplitSurface(tabId: tabId) else { return nil } return moveSurfaceToNewWorkspace( @@ -41,7 +42,8 @@ extension AppDelegate { title: title, focus: focus, focusWindow: focusWindow, - placementOverride: placementOverride + placementOverride: placementOverride, + insertionIndexOverride: insertionIndexOverride ) } @@ -52,7 +54,8 @@ extension AppDelegate { title: String? = nil, focus: Bool = true, focusWindow: Bool = true, - placementOverride: NewWorkspacePlacement? = nil + placementOverride: NewWorkspacePlacement? = nil, + insertionIndexOverride: Int? = nil ) -> SurfaceNewWorkspaceMoveResult? { guard let source = locateSurface(surfaceId: panelId), let sourceWorkspace = source.tabManager.tabs.first(where: { $0.id == source.workspaceId }), @@ -78,6 +81,7 @@ extension AppDelegate { title: destinationTitle, select: false, placementOverride: placementOverride, + insertionIndexOverride: insertionIndexOverride, focusIntent: activationIntent ) else { rollbackDetachedSurface( diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index a32da521b9ca..27dde9ec1ee5 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10106,6 +10106,25 @@ struct VerticalTabsSidebar: View { } .padding(.vertical, SidebarWorkspaceListMetrics.rowVerticalPadding) .frame(maxWidth: .infinity, alignment: .leading) + .overlayPreferenceValue(SidebarWorkspaceRowFramePreferenceKey.self) { anchors in + GeometryReader { proxy in + SidebarBonsplitTabWorkspaceDropOverlay( + tabManager: tabManager, + selectedTabIds: $selectedTabIds, + lastSidebarSelectionIndex: $lastSidebarSelectionIndex, + dropIndicator: $dropIndicator, + dragAutoScrollController: dragAutoScrollController, + targets: renderContext.tabs.compactMap { tab in + guard let anchor = anchors[tab.id] else { return nil } + return SidebarDropPlanner.WorkspaceDropTarget( + workspaceId: tab.id, + isPinned: tab.isPinned, + frame: proxy[anchor] + ) + } + ) + } + } } private func workspaceRow( @@ -10196,6 +10215,9 @@ struct VerticalTabsSidebar: View { .id(tab.id) .accessibilityIdentifier("sidebarWorkspace.\(tab.id.uuidString)") .preference(key: SidebarWorkspaceRowIdsPreferenceKey.self, value: Set([tab.id])) + .anchorPreference(key: SidebarWorkspaceRowFramePreferenceKey.self, value: .bounds) { anchor in + [tab.id: anchor] + } } private func debugShortSidebarTabId(_ id: UUID?) -> String { @@ -10212,6 +10234,14 @@ private struct SidebarWorkspaceRowIdsPreferenceKey: PreferenceKey { } } +private struct SidebarWorkspaceRowFramePreferenceKey: PreferenceKey { + static let defaultValue: [UUID: Anchor] = [:] + + static func reduce(value: inout [UUID: Anchor], nextValue: () -> [UUID: Anchor]) { + value.merge(nextValue()) { _, next in next } + } +} + enum ShortcutHintModifierPolicy { static let intentionalHoldDelay: TimeInterval = 0.30 @@ -12454,6 +12484,185 @@ private struct SidebarEmptyArea: View { } } +private struct SidebarBonsplitTabWorkspaceDropOverlay: NSViewRepresentable { + let tabManager: TabManager + @Binding var selectedTabIds: Set + @Binding var lastSidebarSelectionIndex: Int? + @Binding var dropIndicator: SidebarDropIndicator? + let dragAutoScrollController: SidebarDragAutoScrollController + let targets: [SidebarDropPlanner.WorkspaceDropTarget] + + func makeNSView(context: Context) -> SidebarBonsplitTabWorkspaceDropView { + SidebarBonsplitTabWorkspaceDropView() + } + + func updateNSView(_ nsView: SidebarBonsplitTabWorkspaceDropView, context: Context) { + nsView.targets = targets + nsView.isValidTransfer = { + guard let transfer = BonsplitTabDragPayload.currentTransfer() else { return false } + return AppDelegate.shared?.canMoveBonsplitTabToNewWorkspace(tabId: transfer.tab.id) ?? false + } + nsView.updateAutoscroll = { + dragAutoScrollController.updateFromDragLocation() + } + nsView.setDropIndicator = { indicator in + dropIndicator = indicator + } + nsView.performExistingWorkspaceMove = { workspaceId in + guard let transfer = BonsplitTabDragPayload.currentTransfer(), + let app = AppDelegate.shared else { + return false + } + if let source = app.locateBonsplitSurface(tabId: transfer.tab.id), + source.workspaceId == workspaceId { + syncSidebarSelection() + return true + } + guard app.moveBonsplitTab( + tabId: transfer.tab.id, + toWorkspace: workspaceId, + focus: true, + focusWindow: true + ) else { + return false + } + selectedTabIds = [workspaceId] + syncSidebarSelection(preferredSelectedTabId: workspaceId) + return true + } + nsView.performNewWorkspaceMove = { insertionIndex, _ in + guard let transfer = BonsplitTabDragPayload.currentTransfer(), + let app = AppDelegate.shared, + let result = app.moveBonsplitTabToNewWorkspace( + tabId: transfer.tab.id, + destinationManager: tabManager, + focus: true, + focusWindow: true, + insertionIndexOverride: insertionIndex + ) else { + return false + } + + selectedTabIds = [result.destinationWorkspaceId] + syncSidebarSelection(preferredSelectedTabId: result.destinationWorkspaceId) + return true + } + } + + private func syncSidebarSelection(preferredSelectedTabId: UUID? = nil) { + let selectedId = preferredSelectedTabId ?? tabManager.selectedTabId + if let selectedId { + lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == selectedId } + } else { + lastSidebarSelectionIndex = nil + } + } +} + +private final class SidebarBonsplitTabWorkspaceDropView: NSView { + private static let pasteboardType = NSPasteboard.PasteboardType(BonsplitTabDragPayload.typeIdentifier) + + var targets: [SidebarDropPlanner.WorkspaceDropTarget] = [] + var isValidTransfer: () -> Bool = { false } + var updateAutoscroll: () -> Void = {} + var setDropIndicator: (SidebarDropIndicator?) -> Void = { _ in } + var performExistingWorkspaceMove: (UUID) -> Bool = { _ in false } + var performNewWorkspaceMove: (Int, SidebarDropIndicator) -> Bool = { _, _ in false } + + override var isFlipped: Bool { true } + override var acceptsFirstResponder: Bool { false } + + override init(frame frameRect: NSRect) { + super.init(frame: frameRect) + registerForDraggedTypes([Self.pasteboardType]) + } + + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + override func hitTest(_ point: NSPoint) -> NSView? { + shouldCaptureHitTest() ? super.hitTest(point) : nil + } + + override func draggingEntered(_ sender: any NSDraggingInfo) -> NSDragOperation { + updateDrag(sender) + } + + override func draggingUpdated(_ sender: any NSDraggingInfo) -> NSDragOperation { + updateDrag(sender) + } + + override func draggingExited(_ sender: (any NSDraggingInfo)?) { + setDropIndicator(nil) + } + + override func prepareForDragOperation(_ sender: any NSDraggingInfo) -> Bool { + acceptsDrag(sender) && SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) != nil + } + + override func performDragOperation(_ sender: any NSDraggingInfo) -> Bool { + defer { setDropIndicator(nil) } + guard acceptsDrag(sender), + let action = SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) else { + return false + } + + let moved: Bool + switch action { + case .existingWorkspace(let workspaceId): + moved = performExistingWorkspaceMove(workspaceId) + case .newWorkspace(let insertionIndex, let indicator): + moved = performNewWorkspaceMove(insertionIndex, indicator) + } + + return moved + } + + override func concludeDragOperation(_ sender: (any NSDraggingInfo)?) { + setDropIndicator(nil) + } + + private func updateDrag(_ sender: any NSDraggingInfo) -> NSDragOperation { + guard acceptsDrag(sender) else { + setDropIndicator(nil) + return [] + } + + updateAutoscroll() + let point = localPoint(sender) + let action = SidebarDropPlanner.workspaceAction(for: point, targets: targets) + switch action { + case .newWorkspace(_, let indicator): + setDropIndicator(indicator) + case .existingWorkspace, nil: + setDropIndicator(nil) + } + + return action == nil ? [] : .move + } + + private func acceptsDrag(_ sender: any NSDraggingInfo) -> Bool { + guard sender.draggingPasteboard.types?.contains(Self.pasteboardType) == true else { return false } + return isValidTransfer() + } + + private func shouldCaptureHitTest() -> Bool { + guard BonsplitTabDragPayload.currentTransfer() != nil else { return false } + guard let eventType = NSApp.currentEvent?.type else { return true } + switch eventType { + case .leftMouseDragged, .rightMouseDragged, .otherMouseDragged, .cursorUpdate, .mouseMoved: + return true + default: + return false + } + } + + private func localPoint(_ sender: any NSDraggingInfo) -> CGPoint { + convert(sender.draggingLocation, from: nil) + } +} + enum SidebarPathFormatter { static let homeDirectoryPath: String = FileManager.default.homeDirectoryForCurrentUser.path @@ -13639,7 +13848,7 @@ private struct TabItemView: View, Equatable { } private var showsCenteredTopDropIndicator: Bool { - guard draggedTabId != nil, let indicator = dropIndicator else { return false } + guard let indicator = dropIndicator else { return false } if indicator.tabId == tab.id && indicator.edge == .top { return true } @@ -14683,6 +14892,88 @@ enum SidebarDropPlanner { return resolvedTargetIndex(from: fromIndex, insertionPosition: legalInsertionPosition, totalCount: tabIds.count) } + struct WorkspaceDropTarget: Equatable { + let workspaceId: UUID + let isPinned: Bool + let frame: CGRect + } + + enum WorkspaceDropAction: Equatable { + case newWorkspace(insertionIndex: Int, indicator: SidebarDropIndicator) + case existingWorkspace(UUID) + } + + static func workspaceAction( + for point: CGPoint, + targets: [WorkspaceDropTarget] + ) -> WorkspaceDropAction? { + guard !targets.isEmpty else { return nil } + let orderedTargets = targets.sorted { $0.frame.minY < $1.frame.minY } + if let containingTarget = orderedTargets.first(where: { $0.frame.contains(point) }) { + return workspaceAction(for: point, in: containingTarget, orderedTargets: orderedTargets) + } + + guard let beforeTarget = orderedTargets.first(where: { point.y < $0.frame.minY }) else { + return nil + } + let insertionIndex = legalNewWorkspaceInsertionIndex( + orderedTargets.firstIndex(of: beforeTarget) ?? 0, + orderedTargets: orderedTargets + ) + return .newWorkspace( + insertionIndex: insertionIndex, + indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) + ) + } + + private static func workspaceAction( + for point: CGPoint, + in target: WorkspaceDropTarget, + orderedTargets: [WorkspaceDropTarget] + ) -> WorkspaceDropAction? { + guard let targetIndex = orderedTargets.firstIndex(of: target) else { return nil } + let edgeBand = min(max(target.frame.height * 0.25, 10), target.frame.height / 2) + if point.y <= target.frame.minY + edgeBand { + let insertionIndex = legalNewWorkspaceInsertionIndex(targetIndex, orderedTargets: orderedTargets) + return .newWorkspace( + insertionIndex: insertionIndex, + indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) + ) + } + if point.y >= target.frame.maxY - edgeBand { + let insertionIndex = legalNewWorkspaceInsertionIndex(targetIndex + 1, orderedTargets: orderedTargets) + return .newWorkspace( + insertionIndex: insertionIndex, + indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) + ) + } + return .existingWorkspace(target.workspaceId) + } + + private static func legalNewWorkspaceInsertionIndex( + _ proposedInsertion: Int, + orderedTargets: [WorkspaceDropTarget] + ) -> Int { + let clamped = max(0, min(proposedInsertion, orderedTargets.count)) + let pinnedCount = orderedTargets.reduce(into: 0) { count, target in + if target.isPinned { + count += 1 + } + } + return max(clamped, pinnedCount) + } + + private static func workspaceIndicator( + forInsertionIndex insertionIndex: Int, + orderedTargets: [WorkspaceDropTarget] + ) -> SidebarDropIndicator { + let clampedInsertion = max(0, min(insertionIndex, orderedTargets.count)) + if clampedInsertion >= orderedTargets.count { + return SidebarDropIndicator(tabId: nil, edge: .bottom) + } + return SidebarDropIndicator(tabId: orderedTargets[clampedInsertion].workspaceId, edge: .top) + } + private static func indicatorForInsertionPosition(_ insertionPosition: Int, tabIds: [UUID]) -> SidebarDropIndicator { let clampedInsertion = max(0, min(insertionPosition, tabIds.count)) if clampedInsertion >= tabIds.count { diff --git a/Sources/TabManager+DetachedWorkspace.swift b/Sources/TabManager+DetachedWorkspace.swift index 4ea6f2144b61..61a982a519ec 100644 --- a/Sources/TabManager+DetachedWorkspace.swift +++ b/Sources/TabManager+DetachedWorkspace.swift @@ -26,6 +26,7 @@ extension TabManager { title: String? = nil, select: Bool = true, placementOverride: NewWorkspacePlacement? = nil, + insertionIndexOverride: Int? = nil, focusIntent: PanelFocusIntent? = nil ) -> Workspace? { let sourceWorkspace = selectedWorkspace @@ -51,7 +52,7 @@ extension TabManager { let inheritedConfig = workspaceCreationConfigTemplate( inheritedTerminalFontPoints: snapshot.inheritedTerminalFontPoints ) - let insertIndex = newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) + let insertIndex = insertionIndexOverride ?? newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) let ordinal = Self.nextPortOrdinal Self.nextPortOrdinal += 1 let newWorkspace = Workspace( From f97fb7bd7c3e8673f13fd40fa0e805cc4d306b0b Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 1 May 2026 23:01:01 -0700 Subject: [PATCH 3/7] chore: split sidebar drop planner files --- .github/swift-file-length-budget.tsv | 6 +- GhosttyTabs.xcodeproj/project.pbxproj | 12 + Sources/ContentView.swift | 403 +----------------- ...debarBonsplitTabWorkspaceDropOverlay.swift | 179 ++++++++ Sources/Sidebar/SidebarDropPlanner.swift | 222 ++++++++++ cmuxTests/SidebarOrderingTests.swift | 96 ----- .../SidebarWorkspaceDropPlannerTests.swift | 107 +++++ 7 files changed, 526 insertions(+), 499 deletions(-) create mode 100644 Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift create mode 100644 Sources/Sidebar/SidebarDropPlanner.swift create mode 100644 cmuxTests/SidebarWorkspaceDropPlannerTests.swift diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 7cbd9361e913..735ff39817bd 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -4,7 +4,7 @@ 20530 CLI/cmux.swift 17229 Sources/TerminalController.swift 16020 Sources/ContentView.swift -14471 Sources/AppDelegate.swift +14475 Sources/AppDelegate.swift 13932 Sources/Workspace.swift 13409 Sources/GhosttyTerminalView.swift 10597 Sources/Panels/BrowserPanel.swift @@ -21,7 +21,7 @@ 3840 Sources/Feed/FeedPanelView.swift 3588 cmuxTests/BrowserConfigTests.swift 3145 cmuxTests/BrowserPanelTests.swift -2922 Sources/CmuxConfig.swift +2938 Sources/CmuxConfig.swift 2863 cmuxTests/WindowAndDragTests.swift 2609 Sources/SessionIndexView.swift 2491 Sources/Panels/CmuxWebView.swift @@ -33,7 +33,7 @@ 2026 cmuxTests/CJKIMEInputTests.swift 1949 Sources/Panels/BrowserWebAuthnSupport.swift 1879 Sources/FileExplorerView.swift -1846 cmuxTests/CmuxConfigTests.swift +1899 cmuxTests/CmuxConfigTests.swift 1826 Sources/SessionIndexStore.swift 1784 cmuxTests/ShortcutAndCommandPaletteTests.swift 1684 Sources/KeyboardShortcutSettingsFileStore.swift diff --git a/GhosttyTabs.xcodeproj/project.pbxproj b/GhosttyTabs.xcodeproj/project.pbxproj index 5e27c69d6dce..f7e9d89f716f 100644 --- a/GhosttyTabs.xcodeproj/project.pbxproj +++ b/GhosttyTabs.xcodeproj/project.pbxproj @@ -38,6 +38,9 @@ D0B1000CA1B2C3D4E5F60001 /* CmuxWebViewDragRoutingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0B1000DA1B2C3D4E5F60001 /* CmuxWebViewDragRoutingTests.swift */; }; D0B1000EA1B2C3D4E5F60001 /* BrowserPaneDropRoutingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0B1000FA1B2C3D4E5F60001 /* BrowserPaneDropRoutingTests.swift */; }; D0B10010A1B2C3D4E5F60001 /* BonsplitTabBarDebug.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0B10011A1B2C3D4E5F60001 /* BonsplitTabBarDebug.swift */; }; + D7AB34300000000000000001 /* SidebarDropPlanner.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000002 /* SidebarDropPlanner.swift */; }; + D7AB34300000000000000003 /* SidebarBonsplitTabWorkspaceDropOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */; }; + D7AB34300000000000000005 /* SidebarWorkspaceDropPlannerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */; }; 5EDB6027B346C46521A93C74 /* CMUXAuthCore in Frameworks */ = {isa = PBXBuildFile; productRef = 29813FE5A6CBC1019289A251 /* CMUXAuthCore */; }; AA11BB22CC33DD44EE550001 /* CMUXWorkstream in Frameworks */ = {isa = PBXBuildFile; productRef = AA11BB22CC33DD44EE550002 /* CMUXWorkstream */; }; FEED0000000000000000F002 /* FeedCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = FEED0000000000000000F001 /* FeedCoordinator.swift */; }; @@ -374,6 +377,7 @@ 43430FA5929121E2EAAB3091 /* AuthEnvironment.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AuthEnvironment.swift; sourceTree = ""; }; 491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketSecurityTests.swift; sourceTree = ""; }; 51D800000000000000000002 /* SidebarIdentifierFormattingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarIdentifierFormattingTests.swift; sourceTree = ""; }; + D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarWorkspaceDropPlannerTests.swift; sourceTree = ""; }; 9C1BEA3D2E6F49709A71C021 /* TerminalControllerSocketWriteTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketWriteTests.swift; sourceTree = ""; }; 58C7B1B978620BE162CC057E /* BrowserPanelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserPanelTests.swift; sourceTree = ""; }; 5B1EA8948C5F126FE63CFB4E /* AuthSettingsStore.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AuthSettingsStore.swift; sourceTree = ""; }; @@ -403,6 +407,8 @@ A5001012 /* ContentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContentView.swift; sourceTree = ""; }; C3408A000000000000000002 /* ContentView+RightSidebarCommandPalette.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ContentView+RightSidebarCommandPalette.swift"; sourceTree = ""; }; D7AB00000000000000000004 /* ContentView+MoveTabToNewWorkspace.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ContentView+MoveTabToNewWorkspace.swift"; sourceTree = ""; }; + D7AB34300000000000000002 /* SidebarDropPlanner.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarDropPlanner.swift; sourceTree = ""; }; + D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift; sourceTree = ""; }; D0C0D0C0D0C0D0C0D0C0D002 /* DockPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DockPanelView.swift; sourceTree = ""; }; D0C0D0C0D0C0D0C0D0C0D004 /* DockEmptyView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DockEmptyView.swift; sourceTree = ""; }; C0DE32470000000000000002 /* ContentViewIdentifierCopyCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContentViewIdentifierCopyCommands.swift; sourceTree = ""; }; @@ -778,6 +784,8 @@ A5001012 /* ContentView.swift */, C3408A000000000000000002 /* ContentView+RightSidebarCommandPalette.swift */, D7AB00000000000000000004 /* ContentView+MoveTabToNewWorkspace.swift */, + D7AB34300000000000000002 /* SidebarDropPlanner.swift */, + D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */, C0DE32470000000000000002 /* ContentViewIdentifierCopyCommands.swift */, 9960CC3992F3C44489E7627C /* WorkspaceRuntimeSettings.swift */, 243632BA1DBBA36FD46E0610 /* SidebarAppearanceSupport.swift */, @@ -999,6 +1007,7 @@ 1D301919B10F22B8708E8883 /* WorkspaceManualUnreadTests.swift */, EE0171AF1F49F7547191CEE5 /* SidebarWidthPolicyTests.swift */, 51D800000000000000000002 /* SidebarIdentifierFormattingTests.swift */, + D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */, 491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */, 9C1BEA3D2E6F49709A71C021 /* TerminalControllerSocketWriteTests.swift */, A5A5A504A1B2C3D4E5F60718 /* TerminalNotificationQueueTests.swift */, @@ -1258,6 +1267,8 @@ A5001002 /* ContentView.swift in Sources */, C3408A000000000000000001 /* ContentView+RightSidebarCommandPalette.swift in Sources */, D7AB00000000000000000003 /* ContentView+MoveTabToNewWorkspace.swift in Sources */, + D7AB34300000000000000001 /* SidebarDropPlanner.swift in Sources */, + D7AB34300000000000000003 /* SidebarBonsplitTabWorkspaceDropOverlay.swift in Sources */, C0DE32470000000000000001 /* ContentViewIdentifierCopyCommands.swift in Sources */, 619D509BC9B1EA946CBD6A8A /* WorkspaceRuntimeSettings.swift in Sources */, 211A75777F433ED8BA5AE208 /* SidebarAppearanceSupport.swift in Sources */, @@ -1514,6 +1525,7 @@ 0F2C25F9170130F8DC09DD1B /* WorkspaceManualUnreadTests.swift in Sources */, CA39C0304FE351A21C372429 /* SidebarWidthPolicyTests.swift in Sources */, 51D800000000000000000001 /* SidebarIdentifierFormattingTests.swift in Sources */, + D7AB34300000000000000005 /* SidebarWorkspaceDropPlannerTests.swift in Sources */, 8C4BBF2DEF6DF93F395A9EE7 /* TerminalControllerSocketSecurityTests.swift in Sources */, 9C1BEA3D2E6F49709A71C020 /* TerminalControllerSocketWriteTests.swift in Sources */, A5A5A503A1B2C3D4E5F60718 /* TerminalNotificationQueueTests.swift in Sources */, diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 27dde9ec1ee5..645358ee7ab7 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10113,7 +10113,9 @@ struct VerticalTabsSidebar: View { selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, dropIndicator: $dropIndicator, - dragAutoScrollController: dragAutoScrollController, + updateAutoscroll: { + dragAutoScrollController.updateFromDragLocation() + }, targets: renderContext.tabs.compactMap { tab in guard let anchor = anchors[tab.id] else { return nil } return SidebarDropPlanner.WorkspaceDropTarget( @@ -12484,185 +12486,6 @@ private struct SidebarEmptyArea: View { } } -private struct SidebarBonsplitTabWorkspaceDropOverlay: NSViewRepresentable { - let tabManager: TabManager - @Binding var selectedTabIds: Set - @Binding var lastSidebarSelectionIndex: Int? - @Binding var dropIndicator: SidebarDropIndicator? - let dragAutoScrollController: SidebarDragAutoScrollController - let targets: [SidebarDropPlanner.WorkspaceDropTarget] - - func makeNSView(context: Context) -> SidebarBonsplitTabWorkspaceDropView { - SidebarBonsplitTabWorkspaceDropView() - } - - func updateNSView(_ nsView: SidebarBonsplitTabWorkspaceDropView, context: Context) { - nsView.targets = targets - nsView.isValidTransfer = { - guard let transfer = BonsplitTabDragPayload.currentTransfer() else { return false } - return AppDelegate.shared?.canMoveBonsplitTabToNewWorkspace(tabId: transfer.tab.id) ?? false - } - nsView.updateAutoscroll = { - dragAutoScrollController.updateFromDragLocation() - } - nsView.setDropIndicator = { indicator in - dropIndicator = indicator - } - nsView.performExistingWorkspaceMove = { workspaceId in - guard let transfer = BonsplitTabDragPayload.currentTransfer(), - let app = AppDelegate.shared else { - return false - } - if let source = app.locateBonsplitSurface(tabId: transfer.tab.id), - source.workspaceId == workspaceId { - syncSidebarSelection() - return true - } - guard app.moveBonsplitTab( - tabId: transfer.tab.id, - toWorkspace: workspaceId, - focus: true, - focusWindow: true - ) else { - return false - } - selectedTabIds = [workspaceId] - syncSidebarSelection(preferredSelectedTabId: workspaceId) - return true - } - nsView.performNewWorkspaceMove = { insertionIndex, _ in - guard let transfer = BonsplitTabDragPayload.currentTransfer(), - let app = AppDelegate.shared, - let result = app.moveBonsplitTabToNewWorkspace( - tabId: transfer.tab.id, - destinationManager: tabManager, - focus: true, - focusWindow: true, - insertionIndexOverride: insertionIndex - ) else { - return false - } - - selectedTabIds = [result.destinationWorkspaceId] - syncSidebarSelection(preferredSelectedTabId: result.destinationWorkspaceId) - return true - } - } - - private func syncSidebarSelection(preferredSelectedTabId: UUID? = nil) { - let selectedId = preferredSelectedTabId ?? tabManager.selectedTabId - if let selectedId { - lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == selectedId } - } else { - lastSidebarSelectionIndex = nil - } - } -} - -private final class SidebarBonsplitTabWorkspaceDropView: NSView { - private static let pasteboardType = NSPasteboard.PasteboardType(BonsplitTabDragPayload.typeIdentifier) - - var targets: [SidebarDropPlanner.WorkspaceDropTarget] = [] - var isValidTransfer: () -> Bool = { false } - var updateAutoscroll: () -> Void = {} - var setDropIndicator: (SidebarDropIndicator?) -> Void = { _ in } - var performExistingWorkspaceMove: (UUID) -> Bool = { _ in false } - var performNewWorkspaceMove: (Int, SidebarDropIndicator) -> Bool = { _, _ in false } - - override var isFlipped: Bool { true } - override var acceptsFirstResponder: Bool { false } - - override init(frame frameRect: NSRect) { - super.init(frame: frameRect) - registerForDraggedTypes([Self.pasteboardType]) - } - - required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") - } - - override func hitTest(_ point: NSPoint) -> NSView? { - shouldCaptureHitTest() ? super.hitTest(point) : nil - } - - override func draggingEntered(_ sender: any NSDraggingInfo) -> NSDragOperation { - updateDrag(sender) - } - - override func draggingUpdated(_ sender: any NSDraggingInfo) -> NSDragOperation { - updateDrag(sender) - } - - override func draggingExited(_ sender: (any NSDraggingInfo)?) { - setDropIndicator(nil) - } - - override func prepareForDragOperation(_ sender: any NSDraggingInfo) -> Bool { - acceptsDrag(sender) && SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) != nil - } - - override func performDragOperation(_ sender: any NSDraggingInfo) -> Bool { - defer { setDropIndicator(nil) } - guard acceptsDrag(sender), - let action = SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) else { - return false - } - - let moved: Bool - switch action { - case .existingWorkspace(let workspaceId): - moved = performExistingWorkspaceMove(workspaceId) - case .newWorkspace(let insertionIndex, let indicator): - moved = performNewWorkspaceMove(insertionIndex, indicator) - } - - return moved - } - - override func concludeDragOperation(_ sender: (any NSDraggingInfo)?) { - setDropIndicator(nil) - } - - private func updateDrag(_ sender: any NSDraggingInfo) -> NSDragOperation { - guard acceptsDrag(sender) else { - setDropIndicator(nil) - return [] - } - - updateAutoscroll() - let point = localPoint(sender) - let action = SidebarDropPlanner.workspaceAction(for: point, targets: targets) - switch action { - case .newWorkspace(_, let indicator): - setDropIndicator(indicator) - case .existingWorkspace, nil: - setDropIndicator(nil) - } - - return action == nil ? [] : .move - } - - private func acceptsDrag(_ sender: any NSDraggingInfo) -> Bool { - guard sender.draggingPasteboard.types?.contains(Self.pasteboardType) == true else { return false } - return isValidTransfer() - } - - private func shouldCaptureHitTest() -> Bool { - guard BonsplitTabDragPayload.currentTransfer() != nil else { return false } - guard let eventType = NSApp.currentEvent?.type else { return true } - switch eventType { - case .leftMouseDragged, .rightMouseDragged, .otherMouseDragged, .cursorUpdate, .mouseMoved: - return true - default: - return false - } - } - - private func localPoint(_ sender: any NSDraggingInfo) -> CGPoint { - convert(sender.draggingLocation, from: nil) - } -} - enum SidebarPathFormatter { static let homeDirectoryPath: String = FileManager.default.homeDirectoryForCurrentUser.path @@ -14810,226 +14633,6 @@ private struct SidebarMetadataMarkdownBlockRow: View { } } -enum SidebarDropEdge: Equatable { - case top - case bottom -} - -struct SidebarDropIndicator: Equatable { - let tabId: UUID? - let edge: SidebarDropEdge -} - -enum SidebarDropPlanner { - static func indicator( - draggedTabId: UUID?, - targetTabId: UUID?, - tabIds: [UUID], - pinnedTabIds: Set, - pointerY: CGFloat? = nil, - targetHeight: CGFloat? = nil - ) -> SidebarDropIndicator? { - guard tabIds.count > 1, let draggedTabId else { return nil } - guard let fromIndex = tabIds.firstIndex(of: draggedTabId) else { return nil } - - let insertionPosition: Int - if let targetTabId { - guard let targetTabIndex = tabIds.firstIndex(of: targetTabId) else { return nil } - let edge: SidebarDropEdge - if let pointerY, let targetHeight { - edge = edgeForPointer(locationY: pointerY, targetHeight: targetHeight) - } else { - edge = preferredEdge(fromIndex: fromIndex, targetTabId: targetTabId, tabIds: tabIds) - } - insertionPosition = (edge == .bottom) ? targetTabIndex + 1 : targetTabIndex - } else { - insertionPosition = tabIds.count - } - - let legalInsertionPosition = legalInsertionPosition( - draggedTabId: draggedTabId, - proposedInsertionPosition: insertionPosition, - tabIds: tabIds, - pinnedTabIds: pinnedTabIds - ) - let legalTargetIndex = resolvedTargetIndex( - from: fromIndex, - insertionPosition: legalInsertionPosition, - totalCount: tabIds.count - ) - guard legalTargetIndex != fromIndex else { return nil } - return indicatorForInsertionPosition(legalInsertionPosition, tabIds: tabIds) - } - - static func targetIndex( - draggedTabId: UUID, - targetTabId: UUID?, - indicator: SidebarDropIndicator?, - tabIds: [UUID], - pinnedTabIds: Set - ) -> Int? { - guard let fromIndex = tabIds.firstIndex(of: draggedTabId) else { return nil } - - let insertionPosition: Int - if let indicator, let indicatorInsertion = insertionPositionForIndicator(indicator, tabIds: tabIds) { - insertionPosition = indicatorInsertion - } else if let targetTabId { - guard let targetTabIndex = tabIds.firstIndex(of: targetTabId) else { return nil } - let edge = (indicator?.tabId == targetTabId) - ? (indicator?.edge ?? preferredEdge(fromIndex: fromIndex, targetTabId: targetTabId, tabIds: tabIds)) - : preferredEdge(fromIndex: fromIndex, targetTabId: targetTabId, tabIds: tabIds) - insertionPosition = (edge == .bottom) ? targetTabIndex + 1 : targetTabIndex - } else { - insertionPosition = tabIds.count - } - - let legalInsertionPosition = legalInsertionPosition( - draggedTabId: draggedTabId, - proposedInsertionPosition: insertionPosition, - tabIds: tabIds, - pinnedTabIds: pinnedTabIds - ) - return resolvedTargetIndex(from: fromIndex, insertionPosition: legalInsertionPosition, totalCount: tabIds.count) - } - - struct WorkspaceDropTarget: Equatable { - let workspaceId: UUID - let isPinned: Bool - let frame: CGRect - } - - enum WorkspaceDropAction: Equatable { - case newWorkspace(insertionIndex: Int, indicator: SidebarDropIndicator) - case existingWorkspace(UUID) - } - - static func workspaceAction( - for point: CGPoint, - targets: [WorkspaceDropTarget] - ) -> WorkspaceDropAction? { - guard !targets.isEmpty else { return nil } - let orderedTargets = targets.sorted { $0.frame.minY < $1.frame.minY } - if let containingTarget = orderedTargets.first(where: { $0.frame.contains(point) }) { - return workspaceAction(for: point, in: containingTarget, orderedTargets: orderedTargets) - } - - guard let beforeTarget = orderedTargets.first(where: { point.y < $0.frame.minY }) else { - return nil - } - let insertionIndex = legalNewWorkspaceInsertionIndex( - orderedTargets.firstIndex(of: beforeTarget) ?? 0, - orderedTargets: orderedTargets - ) - return .newWorkspace( - insertionIndex: insertionIndex, - indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) - ) - } - - private static func workspaceAction( - for point: CGPoint, - in target: WorkspaceDropTarget, - orderedTargets: [WorkspaceDropTarget] - ) -> WorkspaceDropAction? { - guard let targetIndex = orderedTargets.firstIndex(of: target) else { return nil } - let edgeBand = min(max(target.frame.height * 0.25, 10), target.frame.height / 2) - if point.y <= target.frame.minY + edgeBand { - let insertionIndex = legalNewWorkspaceInsertionIndex(targetIndex, orderedTargets: orderedTargets) - return .newWorkspace( - insertionIndex: insertionIndex, - indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) - ) - } - if point.y >= target.frame.maxY - edgeBand { - let insertionIndex = legalNewWorkspaceInsertionIndex(targetIndex + 1, orderedTargets: orderedTargets) - return .newWorkspace( - insertionIndex: insertionIndex, - indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) - ) - } - return .existingWorkspace(target.workspaceId) - } - - private static func legalNewWorkspaceInsertionIndex( - _ proposedInsertion: Int, - orderedTargets: [WorkspaceDropTarget] - ) -> Int { - let clamped = max(0, min(proposedInsertion, orderedTargets.count)) - let pinnedCount = orderedTargets.reduce(into: 0) { count, target in - if target.isPinned { - count += 1 - } - } - return max(clamped, pinnedCount) - } - - private static func workspaceIndicator( - forInsertionIndex insertionIndex: Int, - orderedTargets: [WorkspaceDropTarget] - ) -> SidebarDropIndicator { - let clampedInsertion = max(0, min(insertionIndex, orderedTargets.count)) - if clampedInsertion >= orderedTargets.count { - return SidebarDropIndicator(tabId: nil, edge: .bottom) - } - return SidebarDropIndicator(tabId: orderedTargets[clampedInsertion].workspaceId, edge: .top) - } - - private static func indicatorForInsertionPosition(_ insertionPosition: Int, tabIds: [UUID]) -> SidebarDropIndicator { - let clampedInsertion = max(0, min(insertionPosition, tabIds.count)) - if clampedInsertion >= tabIds.count { - return SidebarDropIndicator(tabId: nil, edge: .bottom) - } - return SidebarDropIndicator(tabId: tabIds[clampedInsertion], edge: .top) - } - - private static func insertionPositionForIndicator(_ indicator: SidebarDropIndicator, tabIds: [UUID]) -> Int? { - if let tabId = indicator.tabId { - guard let targetTabIndex = tabIds.firstIndex(of: tabId) else { return nil } - return indicator.edge == .bottom ? targetTabIndex + 1 : targetTabIndex - } - return tabIds.count - } - - private static func preferredEdge(fromIndex: Int, targetTabId: UUID, tabIds: [UUID]) -> SidebarDropEdge { - guard let targetIndex = tabIds.firstIndex(of: targetTabId) else { return .top } - return fromIndex < targetIndex ? .bottom : .top - } - - private static func legalInsertionPosition( - draggedTabId: UUID, - proposedInsertionPosition: Int, - tabIds: [UUID], - pinnedTabIds: Set - ) -> Int { - let clampedInsertion = max(0, min(proposedInsertionPosition, tabIds.count)) - guard !pinnedTabIds.isEmpty else { return clampedInsertion } - - let pinnedCount = tabIds.reduce(into: 0) { count, tabId in - if pinnedTabIds.contains(tabId) { - count += 1 - } - } - guard pinnedCount > 0 else { return clampedInsertion } - - if pinnedTabIds.contains(draggedTabId) { - return min(clampedInsertion, pinnedCount) - } - return max(clampedInsertion, pinnedCount) - } - - static func edgeForPointer(locationY: CGFloat, targetHeight: CGFloat) -> SidebarDropEdge { - guard targetHeight > 0 else { return .top } - let clampedY = min(max(locationY, 0), targetHeight) - return clampedY < (targetHeight / 2) ? .top : .bottom - } - - private static func resolvedTargetIndex(from sourceIndex: Int, insertionPosition: Int, totalCount: Int) -> Int { - let clampedInsertion = max(0, min(insertionPosition, totalCount)) - let adjusted = clampedInsertion > sourceIndex ? clampedInsertion - 1 : clampedInsertion - return max(0, min(adjusted, max(0, totalCount - 1))) - } -} - enum SidebarAutoScrollDirection: Equatable { case up case down diff --git a/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift b/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift new file mode 100644 index 000000000000..496294809c0e --- /dev/null +++ b/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift @@ -0,0 +1,179 @@ +import AppKit +import SwiftUI + +struct SidebarBonsplitTabWorkspaceDropOverlay: NSViewRepresentable { + let tabManager: TabManager + @Binding var selectedTabIds: Set + @Binding var lastSidebarSelectionIndex: Int? + @Binding var dropIndicator: SidebarDropIndicator? + let updateAutoscroll: () -> Void + let targets: [SidebarDropPlanner.WorkspaceDropTarget] + + func makeNSView(context: Context) -> SidebarBonsplitTabWorkspaceDropView { + SidebarBonsplitTabWorkspaceDropView() + } + + func updateNSView(_ nsView: SidebarBonsplitTabWorkspaceDropView, context: Context) { + nsView.targets = targets + nsView.isValidTransfer = { + guard let transfer = BonsplitTabDragPayload.currentTransfer() else { return false } + return AppDelegate.shared?.canMoveBonsplitTabToNewWorkspace(tabId: transfer.tab.id) ?? false + } + nsView.updateAutoscroll = updateAutoscroll + nsView.setDropIndicator = { indicator in + dropIndicator = indicator + } + nsView.performExistingWorkspaceMove = { workspaceId in + guard let transfer = BonsplitTabDragPayload.currentTransfer(), + let app = AppDelegate.shared else { + return false + } + if let source = app.locateBonsplitSurface(tabId: transfer.tab.id), + source.workspaceId == workspaceId { + syncSidebarSelection() + return true + } + guard app.moveBonsplitTab( + tabId: transfer.tab.id, + toWorkspace: workspaceId, + focus: true, + focusWindow: true + ) else { + return false + } + selectedTabIds = [workspaceId] + syncSidebarSelection(preferredSelectedTabId: workspaceId) + return true + } + nsView.performNewWorkspaceMove = { insertionIndex, _ in + guard let transfer = BonsplitTabDragPayload.currentTransfer(), + let app = AppDelegate.shared, + let result = app.moveBonsplitTabToNewWorkspace( + tabId: transfer.tab.id, + destinationManager: tabManager, + focus: true, + focusWindow: true, + insertionIndexOverride: insertionIndex + ) else { + return false + } + + selectedTabIds = [result.destinationWorkspaceId] + syncSidebarSelection(preferredSelectedTabId: result.destinationWorkspaceId) + return true + } + } + + private func syncSidebarSelection(preferredSelectedTabId: UUID? = nil) { + let selectedId = preferredSelectedTabId ?? tabManager.selectedTabId + if let selectedId { + lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == selectedId } + } else { + lastSidebarSelectionIndex = nil + } + } +} + +final class SidebarBonsplitTabWorkspaceDropView: NSView { + private static let pasteboardType = NSPasteboard.PasteboardType(BonsplitTabDragPayload.typeIdentifier) + + var targets: [SidebarDropPlanner.WorkspaceDropTarget] = [] + var isValidTransfer: () -> Bool = { false } + var updateAutoscroll: () -> Void = {} + var setDropIndicator: (SidebarDropIndicator?) -> Void = { _ in } + var performExistingWorkspaceMove: (UUID) -> Bool = { _ in false } + var performNewWorkspaceMove: (Int, SidebarDropIndicator) -> Bool = { _, _ in false } + + override var isFlipped: Bool { true } + override var acceptsFirstResponder: Bool { false } + + override init(frame frameRect: NSRect) { + super.init(frame: frameRect) + registerForDraggedTypes([Self.pasteboardType]) + } + + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + override func hitTest(_ point: NSPoint) -> NSView? { + shouldCaptureHitTest() ? super.hitTest(point) : nil + } + + override func draggingEntered(_ sender: any NSDraggingInfo) -> NSDragOperation { + updateDrag(sender) + } + + override func draggingUpdated(_ sender: any NSDraggingInfo) -> NSDragOperation { + updateDrag(sender) + } + + override func draggingExited(_ sender: (any NSDraggingInfo)?) { + setDropIndicator(nil) + } + + override func prepareForDragOperation(_ sender: any NSDraggingInfo) -> Bool { + acceptsDrag(sender) && SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) != nil + } + + override func performDragOperation(_ sender: any NSDraggingInfo) -> Bool { + defer { setDropIndicator(nil) } + guard acceptsDrag(sender), + let action = SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) else { + return false + } + + let moved: Bool + switch action { + case .existingWorkspace(let workspaceId): + moved = performExistingWorkspaceMove(workspaceId) + case .newWorkspace(let insertionIndex, let indicator): + moved = performNewWorkspaceMove(insertionIndex, indicator) + } + + return moved + } + + override func concludeDragOperation(_ sender: (any NSDraggingInfo)?) { + setDropIndicator(nil) + } + + private func updateDrag(_ sender: any NSDraggingInfo) -> NSDragOperation { + guard acceptsDrag(sender) else { + setDropIndicator(nil) + return [] + } + + updateAutoscroll() + let point = localPoint(sender) + let action = SidebarDropPlanner.workspaceAction(for: point, targets: targets) + switch action { + case .newWorkspace(_, let indicator): + setDropIndicator(indicator) + case .existingWorkspace, nil: + setDropIndicator(nil) + } + + return action == nil ? [] : .move + } + + private func acceptsDrag(_ sender: any NSDraggingInfo) -> Bool { + guard sender.draggingPasteboard.types?.contains(Self.pasteboardType) == true else { return false } + return isValidTransfer() + } + + private func shouldCaptureHitTest() -> Bool { + guard BonsplitTabDragPayload.currentTransfer() != nil else { return false } + guard let eventType = NSApp.currentEvent?.type else { return true } + switch eventType { + case .leftMouseDragged, .rightMouseDragged, .otherMouseDragged, .cursorUpdate, .mouseMoved: + return true + default: + return false + } + } + + private func localPoint(_ sender: any NSDraggingInfo) -> CGPoint { + convert(sender.draggingLocation, from: nil) + } +} diff --git a/Sources/Sidebar/SidebarDropPlanner.swift b/Sources/Sidebar/SidebarDropPlanner.swift new file mode 100644 index 000000000000..237e1dc53133 --- /dev/null +++ b/Sources/Sidebar/SidebarDropPlanner.swift @@ -0,0 +1,222 @@ +import CoreGraphics +import Foundation + +enum SidebarDropEdge: Equatable { + case top + case bottom +} + +struct SidebarDropIndicator: Equatable { + let tabId: UUID? + let edge: SidebarDropEdge +} + +enum SidebarDropPlanner { + static func indicator( + draggedTabId: UUID?, + targetTabId: UUID?, + tabIds: [UUID], + pinnedTabIds: Set, + pointerY: CGFloat? = nil, + targetHeight: CGFloat? = nil + ) -> SidebarDropIndicator? { + guard tabIds.count > 1, let draggedTabId else { return nil } + guard let fromIndex = tabIds.firstIndex(of: draggedTabId) else { return nil } + + let insertionPosition: Int + if let targetTabId { + guard let targetTabIndex = tabIds.firstIndex(of: targetTabId) else { return nil } + let edge: SidebarDropEdge + if let pointerY, let targetHeight { + edge = edgeForPointer(locationY: pointerY, targetHeight: targetHeight) + } else { + edge = preferredEdge(fromIndex: fromIndex, targetTabId: targetTabId, tabIds: tabIds) + } + insertionPosition = (edge == .bottom) ? targetTabIndex + 1 : targetTabIndex + } else { + insertionPosition = tabIds.count + } + + let legalInsertionPosition = legalInsertionPosition( + draggedTabId: draggedTabId, + proposedInsertionPosition: insertionPosition, + tabIds: tabIds, + pinnedTabIds: pinnedTabIds + ) + let legalTargetIndex = resolvedTargetIndex( + from: fromIndex, + insertionPosition: legalInsertionPosition, + totalCount: tabIds.count + ) + guard legalTargetIndex != fromIndex else { return nil } + return indicatorForInsertionPosition(legalInsertionPosition, tabIds: tabIds) + } + + static func targetIndex( + draggedTabId: UUID, + targetTabId: UUID?, + indicator: SidebarDropIndicator?, + tabIds: [UUID], + pinnedTabIds: Set + ) -> Int? { + guard let fromIndex = tabIds.firstIndex(of: draggedTabId) else { return nil } + + let insertionPosition: Int + if let indicator, let indicatorInsertion = insertionPositionForIndicator(indicator, tabIds: tabIds) { + insertionPosition = indicatorInsertion + } else if let targetTabId { + guard let targetTabIndex = tabIds.firstIndex(of: targetTabId) else { return nil } + let edge = (indicator?.tabId == targetTabId) + ? (indicator?.edge ?? preferredEdge(fromIndex: fromIndex, targetTabId: targetTabId, tabIds: tabIds)) + : preferredEdge(fromIndex: fromIndex, targetTabId: targetTabId, tabIds: tabIds) + insertionPosition = (edge == .bottom) ? targetTabIndex + 1 : targetTabIndex + } else { + insertionPosition = tabIds.count + } + + let legalInsertionPosition = legalInsertionPosition( + draggedTabId: draggedTabId, + proposedInsertionPosition: insertionPosition, + tabIds: tabIds, + pinnedTabIds: pinnedTabIds + ) + return resolvedTargetIndex(from: fromIndex, insertionPosition: legalInsertionPosition, totalCount: tabIds.count) + } + + struct WorkspaceDropTarget: Equatable { + let workspaceId: UUID + let isPinned: Bool + let frame: CGRect + } + + enum WorkspaceDropAction: Equatable { + case newWorkspace(insertionIndex: Int, indicator: SidebarDropIndicator) + case existingWorkspace(UUID) + } + + static func workspaceAction( + for point: CGPoint, + targets: [WorkspaceDropTarget] + ) -> WorkspaceDropAction? { + guard !targets.isEmpty else { return nil } + let orderedTargets = targets.sorted { $0.frame.minY < $1.frame.minY } + if let containingTarget = orderedTargets.first(where: { $0.frame.contains(point) }) { + return workspaceAction(for: point, in: containingTarget, orderedTargets: orderedTargets) + } + + guard let beforeTarget = orderedTargets.first(where: { point.y < $0.frame.minY }) else { + return nil + } + let insertionIndex = legalNewWorkspaceInsertionIndex( + orderedTargets.firstIndex(of: beforeTarget) ?? 0, + orderedTargets: orderedTargets + ) + return .newWorkspace( + insertionIndex: insertionIndex, + indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) + ) + } + + private static func workspaceAction( + for point: CGPoint, + in target: WorkspaceDropTarget, + orderedTargets: [WorkspaceDropTarget] + ) -> WorkspaceDropAction? { + guard let targetIndex = orderedTargets.firstIndex(of: target) else { return nil } + let edgeBand = min(max(target.frame.height * 0.25, 10), target.frame.height / 2) + if point.y <= target.frame.minY + edgeBand { + let insertionIndex = legalNewWorkspaceInsertionIndex(targetIndex, orderedTargets: orderedTargets) + return .newWorkspace( + insertionIndex: insertionIndex, + indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) + ) + } + if point.y >= target.frame.maxY - edgeBand { + let insertionIndex = legalNewWorkspaceInsertionIndex(targetIndex + 1, orderedTargets: orderedTargets) + return .newWorkspace( + insertionIndex: insertionIndex, + indicator: workspaceIndicator(forInsertionIndex: insertionIndex, orderedTargets: orderedTargets) + ) + } + return .existingWorkspace(target.workspaceId) + } + + private static func legalNewWorkspaceInsertionIndex( + _ proposedInsertion: Int, + orderedTargets: [WorkspaceDropTarget] + ) -> Int { + let clamped = max(0, min(proposedInsertion, orderedTargets.count)) + let pinnedCount = orderedTargets.reduce(into: 0) { count, target in + if target.isPinned { + count += 1 + } + } + return max(clamped, pinnedCount) + } + + private static func workspaceIndicator( + forInsertionIndex insertionIndex: Int, + orderedTargets: [WorkspaceDropTarget] + ) -> SidebarDropIndicator { + let clampedInsertion = max(0, min(insertionIndex, orderedTargets.count)) + if clampedInsertion >= orderedTargets.count { + return SidebarDropIndicator(tabId: nil, edge: .bottom) + } + return SidebarDropIndicator(tabId: orderedTargets[clampedInsertion].workspaceId, edge: .top) + } + + private static func indicatorForInsertionPosition(_ insertionPosition: Int, tabIds: [UUID]) -> SidebarDropIndicator { + let clampedInsertion = max(0, min(insertionPosition, tabIds.count)) + if clampedInsertion >= tabIds.count { + return SidebarDropIndicator(tabId: nil, edge: .bottom) + } + return SidebarDropIndicator(tabId: tabIds[clampedInsertion], edge: .top) + } + + private static func insertionPositionForIndicator(_ indicator: SidebarDropIndicator, tabIds: [UUID]) -> Int? { + if let tabId = indicator.tabId { + guard let targetTabIndex = tabIds.firstIndex(of: tabId) else { return nil } + return indicator.edge == .bottom ? targetTabIndex + 1 : targetTabIndex + } + return tabIds.count + } + + private static func preferredEdge(fromIndex: Int, targetTabId: UUID, tabIds: [UUID]) -> SidebarDropEdge { + guard let targetIndex = tabIds.firstIndex(of: targetTabId) else { return .top } + return fromIndex < targetIndex ? .bottom : .top + } + + private static func legalInsertionPosition( + draggedTabId: UUID, + proposedInsertionPosition: Int, + tabIds: [UUID], + pinnedTabIds: Set + ) -> Int { + let clampedInsertion = max(0, min(proposedInsertionPosition, tabIds.count)) + guard !pinnedTabIds.isEmpty else { return clampedInsertion } + + let pinnedCount = tabIds.reduce(into: 0) { count, tabId in + if pinnedTabIds.contains(tabId) { + count += 1 + } + } + guard pinnedCount > 0 else { return clampedInsertion } + + if pinnedTabIds.contains(draggedTabId) { + return min(clampedInsertion, pinnedCount) + } + return max(clampedInsertion, pinnedCount) + } + + static func edgeForPointer(locationY: CGFloat, targetHeight: CGFloat) -> SidebarDropEdge { + guard targetHeight > 0 else { return .top } + let clampedY = min(max(locationY, 0), targetHeight) + return clampedY < (targetHeight / 2) ? .top : .bottom + } + + private static func resolvedTargetIndex(from sourceIndex: Int, insertionPosition: Int, totalCount: Int) -> Int { + let clampedInsertion = max(0, min(insertionPosition, totalCount)) + let adjusted = clampedInsertion > sourceIndex ? clampedInsertion - 1 : clampedInsertion + return max(0, min(adjusted, max(0, totalCount - 1))) + } +} diff --git a/cmuxTests/SidebarOrderingTests.swift b/cmuxTests/SidebarOrderingTests.swift index b18bbb6e5b4a..da58fd0761da 100644 --- a/cmuxTests/SidebarOrderingTests.swift +++ b/cmuxTests/SidebarOrderingTests.swift @@ -804,102 +804,6 @@ final class SidebarDropPlannerTests: XCTestCase { XCTAssertEqual(targetIndex, 2) } - func testWorkspaceDropCenterTargetsExistingWorkspace() { - let first = UUID() - let second = UUID() - let targets = workspaceDropTargets([first, second]) - - let action = SidebarDropPlanner.workspaceAction( - for: CGPoint(x: 12, y: 56), - targets: targets - ) - - XCTAssertEqual(action, .existingWorkspace(second)) - } - - func testWorkspaceDropTopEdgeCreatesWorkspaceBeforeTarget() { - let first = UUID() - let second = UUID() - let targets = workspaceDropTargets([first, second]) - - let action = SidebarDropPlanner.workspaceAction( - for: CGPoint(x: 12, y: 42), - targets: targets - ) - - XCTAssertEqual( - action, - .newWorkspace( - insertionIndex: 1, - indicator: SidebarDropIndicator(tabId: second, edge: .top) - ) - ) - } - - func testWorkspaceDropGapCreatesWorkspaceBeforeNextTarget() { - let first = UUID() - let second = UUID() - let targets = workspaceDropTargets([first, second]) - - let action = SidebarDropPlanner.workspaceAction( - for: CGPoint(x: 12, y: 36), - targets: targets - ) - - XCTAssertEqual( - action, - .newWorkspace( - insertionIndex: 1, - indicator: SidebarDropIndicator(tabId: second, edge: .top) - ) - ) - } - - func testWorkspaceDropAfterLastRowFallsThroughToEmptyAreaDropTarget() { - let first = UUID() - let second = UUID() - let targets = workspaceDropTargets([first, second]) - - XCTAssertNil( - SidebarDropPlanner.workspaceAction( - for: CGPoint(x: 12, y: 92), - targets: targets - ) - ) - } - - func testWorkspaceDropKeepsNewWorkspaceAfterPinnedRows() { - let pinnedA = UUID() - let pinnedB = UUID() - let unpinned = UUID() - let targets = workspaceDropTargets([pinnedA, pinnedB, unpinned], pinnedIds: [pinnedA, pinnedB]) - - let action = SidebarDropPlanner.workspaceAction( - for: CGPoint(x: 12, y: 2), - targets: targets - ) - - XCTAssertEqual( - action, - .newWorkspace( - insertionIndex: 2, - indicator: SidebarDropIndicator(tabId: unpinned, edge: .top) - ) - ) - } - - private func workspaceDropTargets( - _ ids: [UUID], - pinnedIds: Set = [] - ) -> [SidebarDropPlanner.WorkspaceDropTarget] { - ids.enumerated().map { index, id in - SidebarDropPlanner.WorkspaceDropTarget( - workspaceId: id, - isPinned: pinnedIds.contains(id), - frame: CGRect(x: 0, y: CGFloat(index * 40), width: 180, height: 32) - ) - } - } } diff --git a/cmuxTests/SidebarWorkspaceDropPlannerTests.swift b/cmuxTests/SidebarWorkspaceDropPlannerTests.swift new file mode 100644 index 000000000000..64d29ce0070f --- /dev/null +++ b/cmuxTests/SidebarWorkspaceDropPlannerTests.swift @@ -0,0 +1,107 @@ +import CoreGraphics +import XCTest + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +final class SidebarWorkspaceDropPlannerTests: XCTestCase { + func testWorkspaceDropCenterTargetsExistingWorkspace() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 56), + targets: targets + ) + + XCTAssertEqual(action, .existingWorkspace(second)) + } + + func testWorkspaceDropTopEdgeCreatesWorkspaceBeforeTarget() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 42), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 1, + indicator: SidebarDropIndicator(tabId: second, edge: .top) + ) + ) + } + + func testWorkspaceDropGapCreatesWorkspaceBeforeNextTarget() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 36), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 1, + indicator: SidebarDropIndicator(tabId: second, edge: .top) + ) + ) + } + + func testWorkspaceDropAfterLastRowFallsThroughToEmptyAreaDropTarget() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + XCTAssertNil( + SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 92), + targets: targets + ) + ) + } + + func testWorkspaceDropKeepsNewWorkspaceAfterPinnedRows() { + let pinnedA = UUID() + let pinnedB = UUID() + let unpinned = UUID() + let targets = workspaceDropTargets([pinnedA, pinnedB, unpinned], pinnedIds: [pinnedA, pinnedB]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 2), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 2, + indicator: SidebarDropIndicator(tabId: unpinned, edge: .top) + ) + ) + } + + private func workspaceDropTargets( + _ ids: [UUID], + pinnedIds: Set = [] + ) -> [SidebarDropPlanner.WorkspaceDropTarget] { + ids.enumerated().map { index, id in + SidebarDropPlanner.WorkspaceDropTarget( + workspaceId: id, + isPinned: pinnedIds.contains(id), + frame: CGRect(x: 0, y: CGFloat(index * 40), width: 180, height: 32) + ) + } + } +} From 98ea55e9e22fb3114ece40185c9f2d2a0d8a5425 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 1 May 2026 23:05:32 -0700 Subject: [PATCH 4/7] fix: harden sidebar workspace drop insertion --- Sources/ContentView.swift | 37 +++++++++++++++- ...debarBonsplitTabWorkspaceDropOverlay.swift | 44 +++++-------------- Sources/TabManager+DetachedWorkspace.swift | 30 ++++++++++++- .../SidebarWorkspaceDropPlannerTests.swift | 19 ++++++++ cmuxTests/WorkspaceUnitTests.swift | 24 ++++++++++ 5 files changed, 118 insertions(+), 36 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 645358ee7ab7..ed44631b2f01 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10109,7 +10109,42 @@ struct VerticalTabsSidebar: View { .overlayPreferenceValue(SidebarWorkspaceRowFramePreferenceKey.self) { anchors in GeometryReader { proxy in SidebarBonsplitTabWorkspaceDropOverlay( - tabManager: tabManager, + currentSelectedTabId: { + tabManager.selectedTabId + }, + sidebarIndexForTabId: { workspaceId in + tabManager.tabs.firstIndex { $0.id == workspaceId } + }, + moveToExistingWorkspace: { workspaceId in + guard let transfer = BonsplitTabDragPayload.currentTransfer(), + let app = AppDelegate.shared else { + return false + } + if let source = app.locateBonsplitSurface(tabId: transfer.tab.id), + source.workspaceId == workspaceId { + return true + } + return app.moveBonsplitTab( + tabId: transfer.tab.id, + toWorkspace: workspaceId, + focus: true, + focusWindow: true + ) + }, + moveToNewWorkspace: { insertionIndex in + guard let transfer = BonsplitTabDragPayload.currentTransfer(), + let app = AppDelegate.shared, + let result = app.moveBonsplitTabToNewWorkspace( + tabId: transfer.tab.id, + destinationManager: tabManager, + focus: true, + focusWindow: true, + insertionIndexOverride: insertionIndex + ) else { + return nil + } + return result.destinationWorkspaceId + }, selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, dropIndicator: $dropIndicator, diff --git a/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift b/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift index 496294809c0e..0184a6d324a2 100644 --- a/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift +++ b/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift @@ -2,7 +2,10 @@ import AppKit import SwiftUI struct SidebarBonsplitTabWorkspaceDropOverlay: NSViewRepresentable { - let tabManager: TabManager + let currentSelectedTabId: () -> UUID? + let sidebarIndexForTabId: (UUID) -> Int? + let moveToExistingWorkspace: (UUID) -> Bool + let moveToNewWorkspace: (Int) -> UUID? @Binding var selectedTabIds: Set @Binding var lastSidebarSelectionIndex: Int? @Binding var dropIndicator: SidebarDropIndicator? @@ -24,50 +27,23 @@ struct SidebarBonsplitTabWorkspaceDropOverlay: NSViewRepresentable { dropIndicator = indicator } nsView.performExistingWorkspaceMove = { workspaceId in - guard let transfer = BonsplitTabDragPayload.currentTransfer(), - let app = AppDelegate.shared else { - return false - } - if let source = app.locateBonsplitSurface(tabId: transfer.tab.id), - source.workspaceId == workspaceId { - syncSidebarSelection() - return true - } - guard app.moveBonsplitTab( - tabId: transfer.tab.id, - toWorkspace: workspaceId, - focus: true, - focusWindow: true - ) else { - return false - } + guard moveToExistingWorkspace(workspaceId) else { return false } selectedTabIds = [workspaceId] syncSidebarSelection(preferredSelectedTabId: workspaceId) return true } nsView.performNewWorkspaceMove = { insertionIndex, _ in - guard let transfer = BonsplitTabDragPayload.currentTransfer(), - let app = AppDelegate.shared, - let result = app.moveBonsplitTabToNewWorkspace( - tabId: transfer.tab.id, - destinationManager: tabManager, - focus: true, - focusWindow: true, - insertionIndexOverride: insertionIndex - ) else { - return false - } - - selectedTabIds = [result.destinationWorkspaceId] - syncSidebarSelection(preferredSelectedTabId: result.destinationWorkspaceId) + guard let destinationWorkspaceId = moveToNewWorkspace(insertionIndex) else { return false } + selectedTabIds = [destinationWorkspaceId] + syncSidebarSelection(preferredSelectedTabId: destinationWorkspaceId) return true } } private func syncSidebarSelection(preferredSelectedTabId: UUID? = nil) { - let selectedId = preferredSelectedTabId ?? tabManager.selectedTabId + let selectedId = preferredSelectedTabId ?? currentSelectedTabId() if let selectedId { - lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == selectedId } + lastSidebarSelectionIndex = sidebarIndexForTabId(selectedId) } else { lastSidebarSelectionIndex = nil } diff --git a/Sources/TabManager+DetachedWorkspace.swift b/Sources/TabManager+DetachedWorkspace.swift index 61a982a519ec..01e2a194a539 100644 --- a/Sources/TabManager+DetachedWorkspace.swift +++ b/Sources/TabManager+DetachedWorkspace.swift @@ -52,7 +52,11 @@ extension TabManager { let inheritedConfig = workspaceCreationConfigTemplate( inheritedTerminalFontPoints: snapshot.inheritedTerminalFontPoints ) - let insertIndex = insertionIndexOverride ?? newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) + let insertIndex = detachedWorkspaceInsertIndex( + insertionIndexOverride: insertionIndexOverride, + snapshot: snapshot, + placementOverride: placementOverride + ) let ordinal = Self.nextPortOrdinal Self.nextPortOrdinal += 1 let newWorkspace = Workspace( @@ -101,4 +105,28 @@ extension TabManager { return newWorkspace } } + + private func detachedWorkspaceInsertIndex( + insertionIndexOverride: Int?, + snapshot: WorkspaceCreationSnapshot, + placementOverride: NewWorkspacePlacement? + ) -> Int { + guard let insertionIndexOverride else { + return newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) + } + return Self.clampedDetachedWorkspaceInsertIndex(insertionIndexOverride, tabs: snapshot.tabs) + } + + private static func clampedDetachedWorkspaceInsertIndex( + _ proposedInsertion: Int, + tabs: [WorkspaceCreationTabSnapshot] + ) -> Int { + let clampedInsertion = max(0, min(proposedInsertion, tabs.count)) + let pinnedCount = tabs.reduce(into: 0) { count, tab in + if tab.isPinned { + count += 1 + } + } + return max(clampedInsertion, pinnedCount) + } } diff --git a/cmuxTests/SidebarWorkspaceDropPlannerTests.swift b/cmuxTests/SidebarWorkspaceDropPlannerTests.swift index 64d29ce0070f..33976ea48be1 100644 --- a/cmuxTests/SidebarWorkspaceDropPlannerTests.swift +++ b/cmuxTests/SidebarWorkspaceDropPlannerTests.swift @@ -40,6 +40,25 @@ final class SidebarWorkspaceDropPlannerTests: XCTestCase { ) } + func testWorkspaceDropBottomEdgeCreatesWorkspaceAfterTarget() { + let first = UUID() + let second = UUID() + let targets = workspaceDropTargets([first, second]) + + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 65), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 2, + indicator: SidebarDropIndicator(tabId: nil, edge: .bottom) + ) + ) + } + func testWorkspaceDropGapCreatesWorkspaceBeforeNextTarget() { let first = UUID() let second = UUID() diff --git a/cmuxTests/WorkspaceUnitTests.swift b/cmuxTests/WorkspaceUnitTests.swift index b7753a9da815..849701e6cab4 100644 --- a/cmuxTests/WorkspaceUnitTests.swift +++ b/cmuxTests/WorkspaceUnitTests.swift @@ -2824,6 +2824,30 @@ final class WorkspaceReorderTests: XCTestCase { XCTAssertTrue(manager.reorderWorkspace(tabId: firstPinned.id, toIndex: 999)) XCTAssertEqual(manager.tabs.map(\.id), [secondPinned.id, firstPinned.id, unpinned.id]) } + + @MainActor + func testDetachedWorkspaceInsertionOverrideClampsAfterPinnedSegment() { + let manager = TabManager() + let firstPinned = manager.tabs[0] + manager.setPinned(firstPinned, pinned: true) + let secondPinned = manager.addWorkspace() + manager.setPinned(secondPinned, pinned: true) + let source = manager.addWorkspace() + manager.selectWorkspace(source) + + guard let panelId = source.focusedPanelId, + let detached = source.detachSurface(panelId: panelId), + let inserted = manager.addWorkspace( + fromDetachedSurface: detached, + insertionIndexOverride: 0 + ) else { + XCTFail("Expected detached workspace insertion to succeed") + return + } + + XCTAssertEqual(manager.tabs.map(\.id), [firstPinned.id, secondPinned.id, inserted.id, source.id]) + XCTAssertFalse(inserted.isPinned) + } } @MainActor From e3945473d32dc9ad4af21d892375166315b78737 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 1 May 2026 23:22:42 -0700 Subject: [PATCH 5/7] fix: validate sidebar drops by planned action --- .../AppDelegate+MoveTabToNewWorkspace.swift | 11 ++ ...debarBonsplitTabWorkspaceDropOverlay.swift | 118 +++++++++++++++--- Sources/TabManager+DetachedWorkspace.swift | 35 ++++-- 3 files changed, 139 insertions(+), 25 deletions(-) diff --git a/Sources/AppDelegate+MoveTabToNewWorkspace.swift b/Sources/AppDelegate+MoveTabToNewWorkspace.swift index 7c37d57a0e55..9bacacc4e455 100644 --- a/Sources/AppDelegate+MoveTabToNewWorkspace.swift +++ b/Sources/AppDelegate+MoveTabToNewWorkspace.swift @@ -25,6 +25,17 @@ extension AppDelegate { return canMoveSurfaceToNewWorkspace(panelId: located.panelId) } + func canMoveBonsplitTab(tabId: UUID, toWorkspace targetWorkspaceId: UUID) -> Bool { + guard let located = locateBonsplitSurface(tabId: tabId), + let sourceWorkspace = located.tabManager.tabs.first(where: { $0.id == located.workspaceId }), + sourceWorkspace.panels[located.panelId] != nil, + let destinationManager = tabManagerFor(tabId: targetWorkspaceId), + destinationManager.tabs.contains(where: { $0.id == targetWorkspaceId }) else { + return false + } + return true + } + @discardableResult func moveBonsplitTabToNewWorkspace( tabId: UUID, diff --git a/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift b/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift index 0184a6d324a2..0cc107f909ad 100644 --- a/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift +++ b/Sources/Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift @@ -1,4 +1,5 @@ import AppKit +import Bonsplit import SwiftUI struct SidebarBonsplitTabWorkspaceDropOverlay: NSViewRepresentable { @@ -18,9 +19,24 @@ struct SidebarBonsplitTabWorkspaceDropOverlay: NSViewRepresentable { func updateNSView(_ nsView: SidebarBonsplitTabWorkspaceDropView, context: Context) { nsView.targets = targets - nsView.isValidTransfer = { - guard let transfer = BonsplitTabDragPayload.currentTransfer() else { return false } - return AppDelegate.shared?.canMoveBonsplitTabToNewWorkspace(tabId: transfer.tab.id) ?? false + nsView.hasValidTransfer = { + BonsplitTabDragPayload.currentTransfer() != nil + } + nsView.canPerformAction = { action in + guard let transfer = BonsplitTabDragPayload.currentTransfer(), + let app = AppDelegate.shared else { + return false + } + switch action { + case .existingWorkspace(let workspaceId): + if let source = app.locateBonsplitSurface(tabId: transfer.tab.id), + source.workspaceId == workspaceId { + return true + } + return app.canMoveBonsplitTab(tabId: transfer.tab.id, toWorkspace: workspaceId) + case .newWorkspace: + return app.canMoveBonsplitTabToNewWorkspace(tabId: transfer.tab.id) + } } nsView.updateAutoscroll = updateAutoscroll nsView.setDropIndicator = { indicator in @@ -54,7 +70,8 @@ final class SidebarBonsplitTabWorkspaceDropView: NSView { private static let pasteboardType = NSPasteboard.PasteboardType(BonsplitTabDragPayload.typeIdentifier) var targets: [SidebarDropPlanner.WorkspaceDropTarget] = [] - var isValidTransfer: () -> Bool = { false } + var hasValidTransfer: () -> Bool = { false } + var canPerformAction: (SidebarDropPlanner.WorkspaceDropAction) -> Bool = { _ in false } var updateAutoscroll: () -> Void = {} var setDropIndicator: (SidebarDropIndicator?) -> Void = { _ in } var performExistingWorkspaceMove: (UUID) -> Bool = { _ in false } @@ -77,25 +94,42 @@ final class SidebarBonsplitTabWorkspaceDropView: NSView { } override func draggingEntered(_ sender: any NSDraggingInfo) -> NSDragOperation { - updateDrag(sender) + updateDrag(sender, phase: "entered") } override func draggingUpdated(_ sender: any NSDraggingInfo) -> NSDragOperation { - updateDrag(sender) + updateDrag(sender, phase: "updated") } override func draggingExited(_ sender: (any NSDraggingInfo)?) { +#if DEBUG + dlog("sidebar.workspaceDropOverlay.exited clear=1") +#endif setDropIndicator(nil) } override func prepareForDragOperation(_ sender: any NSDraggingInfo) -> Bool { - acceptsDrag(sender) && SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) != nil + let action = action(for: sender) + let accepted = acceptsDrag(sender, action: action) +#if DEBUG + dlog( + "sidebar.workspaceDropOverlay.prepare accepted=\(accepted ? 1 : 0) " + + "action=\(debugActionDescription(action))" + ) +#endif + return accepted } override func performDragOperation(_ sender: any NSDraggingInfo) -> Bool { defer { setDropIndicator(nil) } - guard acceptsDrag(sender), - let action = SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) else { + let action = action(for: sender) + guard acceptsDrag(sender, action: action), let action else { +#if DEBUG + dlog( + "sidebar.workspaceDropOverlay.perform moved=0 reason=notAccepted " + + "action=\(debugActionDescription(action))" + ) +#endif return false } @@ -107,35 +141,63 @@ final class SidebarBonsplitTabWorkspaceDropView: NSView { moved = performNewWorkspaceMove(insertionIndex, indicator) } +#if DEBUG + dlog( + "sidebar.workspaceDropOverlay.perform moved=\(moved ? 1 : 0) " + + "action=\(debugActionDescription(action))" + ) +#endif return moved } override func concludeDragOperation(_ sender: (any NSDraggingInfo)?) { +#if DEBUG + dlog("sidebar.workspaceDropOverlay.concluded clear=1") +#endif setDropIndicator(nil) } - private func updateDrag(_ sender: any NSDraggingInfo) -> NSDragOperation { - guard acceptsDrag(sender) else { + private func updateDrag(_ sender: any NSDraggingInfo, phase: String) -> NSDragOperation { + let action = action(for: sender) + guard acceptsDrag(sender, action: action), let action else { setDropIndicator(nil) +#if DEBUG + dlog( + "sidebar.workspaceDropOverlay.\(phase) accepted=0 clear=1 " + + "action=\(debugActionDescription(action))" + ) +#endif return [] } updateAutoscroll() - let point = localPoint(sender) - let action = SidebarDropPlanner.workspaceAction(for: point, targets: targets) switch action { case .newWorkspace(_, let indicator): setDropIndicator(indicator) - case .existingWorkspace, nil: + case .existingWorkspace: setDropIndicator(nil) } - return action == nil ? [] : .move +#if DEBUG + dlog( + "sidebar.workspaceDropOverlay.\(phase) accepted=1 " + + "action=\(debugActionDescription(action))" + ) +#endif + return .move } - private func acceptsDrag(_ sender: any NSDraggingInfo) -> Bool { + private func acceptsDrag( + _ sender: any NSDraggingInfo, + action: SidebarDropPlanner.WorkspaceDropAction? + ) -> Bool { guard sender.draggingPasteboard.types?.contains(Self.pasteboardType) == true else { return false } - return isValidTransfer() + guard hasValidTransfer(), let action else { return false } + return canPerformAction(action) + } + + private func action(for sender: any NSDraggingInfo) -> SidebarDropPlanner.WorkspaceDropAction? { + SidebarDropPlanner.workspaceAction(for: localPoint(sender), targets: targets) } private func shouldCaptureHitTest() -> Bool { @@ -152,4 +214,26 @@ final class SidebarBonsplitTabWorkspaceDropView: NSView { private func localPoint(_ sender: any NSDraggingInfo) -> CGPoint { convert(sender.draggingLocation, from: nil) } + +#if DEBUG + private func debugActionDescription(_ action: SidebarDropPlanner.WorkspaceDropAction?) -> String { + guard let action else { return "nil" } + switch action { + case .existingWorkspace(let workspaceId): + return "existing:\(debugShortId(workspaceId))" + case .newWorkspace(let insertionIndex, let indicator): + return "new:index=\(insertionIndex),indicator=\(debugIndicatorDescription(indicator))" + } + } + + private func debugIndicatorDescription(_ indicator: SidebarDropIndicator) -> String { + let target = indicator.tabId.map(debugShortId) ?? "end" + let edge = indicator.edge == .top ? "top" : "bottom" + return "\(target):\(edge)" + } + + private func debugShortId(_ id: UUID) -> String { + String(id.uuidString.prefix(5)) + } +#endif } diff --git a/Sources/TabManager+DetachedWorkspace.swift b/Sources/TabManager+DetachedWorkspace.swift index 01e2a194a539..63a425806746 100644 --- a/Sources/TabManager+DetachedWorkspace.swift +++ b/Sources/TabManager+DetachedWorkspace.swift @@ -52,7 +52,7 @@ extension TabManager { let inheritedConfig = workspaceCreationConfigTemplate( inheritedTerminalFontPoints: snapshot.inheritedTerminalFontPoints ) - let insertIndex = detachedWorkspaceInsertIndex( + let plannedInsertIndex = detachedWorkspaceInsertIndex( insertionIndexOverride: insertionIndexOverride, snapshot: snapshot, placementOverride: placementOverride @@ -76,11 +76,8 @@ extension TabManager { wireClosedBrowserTracking(for: newWorkspace) var updatedTabs = tabs - if insertIndex >= 0 && insertIndex <= updatedTabs.count { - updatedTabs.insert(newWorkspace, at: insertIndex) - } else { - updatedTabs.append(newWorkspace) - } + let insertIndex = Self.clampedDetachedWorkspaceInsertIndex(plannedInsertIndex, workspaces: updatedTabs) + updatedTabs.insert(newWorkspace, at: insertIndex) tabs = updatedTabs if select { @@ -121,12 +118,34 @@ extension TabManager { _ proposedInsertion: Int, tabs: [WorkspaceCreationTabSnapshot] ) -> Int { - let clampedInsertion = max(0, min(proposedInsertion, tabs.count)) let pinnedCount = tabs.reduce(into: 0) { count, tab in if tab.isPinned { count += 1 } } - return max(clampedInsertion, pinnedCount) + return clampedDetachedWorkspaceInsertIndex(proposedInsertion, totalCount: tabs.count, pinnedCount: pinnedCount) + } + + private static func clampedDetachedWorkspaceInsertIndex( + _ proposedInsertion: Int, + workspaces: [Workspace] + ) -> Int { + let pinnedCount = workspaces.prefix { $0.isPinned }.count + return clampedDetachedWorkspaceInsertIndex( + proposedInsertion, + totalCount: workspaces.count, + pinnedCount: pinnedCount + ) + } + + private static func clampedDetachedWorkspaceInsertIndex( + _ proposedInsertion: Int, + totalCount: Int, + pinnedCount: Int + ) -> Int { + let clampedCount = max(0, totalCount) + let clampedPinnedCount = max(0, min(pinnedCount, clampedCount)) + let clampedInsertion = max(0, min(proposedInsertion, clampedCount)) + return max(clampedInsertion, clampedPinnedCount) } } From 7378247b635a459e5545f1de2e930f54f8afdcfc Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Sat, 2 May 2026 02:34:37 -0700 Subject: [PATCH 6/7] fix: keep unmounted workspace portals hidden --- GhosttyTabs.xcodeproj/project.pbxproj | 8 + Sources/ContentView.swift | 34 ++-- Sources/GhosttyTerminalView.swift | 9 +- Sources/Workspace.swift | 151 ++++++------------ Sources/WorkspaceSurfaceConfig.swift | 107 +++++++++++++ ...ttyTerminalViewVisibilityPolicyTests.swift | 60 +++++++ cmuxTests/TerminalAndGhosttyTests.swift | 49 ------ cmuxTests/WorkspaceUnitTests.swift | 17 ++ 8 files changed, 254 insertions(+), 181 deletions(-) create mode 100644 Sources/WorkspaceSurfaceConfig.swift create mode 100644 cmuxTests/GhosttyTerminalViewVisibilityPolicyTests.swift diff --git a/GhosttyTabs.xcodeproj/project.pbxproj b/GhosttyTabs.xcodeproj/project.pbxproj index f7e9d89f716f..fc6979965b73 100644 --- a/GhosttyTabs.xcodeproj/project.pbxproj +++ b/GhosttyTabs.xcodeproj/project.pbxproj @@ -41,6 +41,8 @@ D7AB34300000000000000001 /* SidebarDropPlanner.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000002 /* SidebarDropPlanner.swift */; }; D7AB34300000000000000003 /* SidebarBonsplitTabWorkspaceDropOverlay.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */; }; D7AB34300000000000000005 /* SidebarWorkspaceDropPlannerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */; }; + D7AB34400000000000000001 /* WorkspaceSurfaceConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34400000000000000002 /* WorkspaceSurfaceConfig.swift */; }; + D7AB34400000000000000003 /* GhosttyTerminalViewVisibilityPolicyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB34400000000000000004 /* GhosttyTerminalViewVisibilityPolicyTests.swift */; }; 5EDB6027B346C46521A93C74 /* CMUXAuthCore in Frameworks */ = {isa = PBXBuildFile; productRef = 29813FE5A6CBC1019289A251 /* CMUXAuthCore */; }; AA11BB22CC33DD44EE550001 /* CMUXWorkstream in Frameworks */ = {isa = PBXBuildFile; productRef = AA11BB22CC33DD44EE550002 /* CMUXWorkstream */; }; FEED0000000000000000F002 /* FeedCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = FEED0000000000000000F001 /* FeedCoordinator.swift */; }; @@ -378,6 +380,7 @@ 491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketSecurityTests.swift; sourceTree = ""; }; 51D800000000000000000002 /* SidebarIdentifierFormattingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarIdentifierFormattingTests.swift; sourceTree = ""; }; D7AB34300000000000000006 /* SidebarWorkspaceDropPlannerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarWorkspaceDropPlannerTests.swift; sourceTree = ""; }; + D7AB34400000000000000004 /* GhosttyTerminalViewVisibilityPolicyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GhosttyTerminalViewVisibilityPolicyTests.swift; sourceTree = ""; }; 9C1BEA3D2E6F49709A71C021 /* TerminalControllerSocketWriteTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketWriteTests.swift; sourceTree = ""; }; 58C7B1B978620BE162CC057E /* BrowserPanelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserPanelTests.swift; sourceTree = ""; }; 5B1EA8948C5F126FE63CFB4E /* AuthSettingsStore.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AuthSettingsStore.swift; sourceTree = ""; }; @@ -409,6 +412,7 @@ D7AB00000000000000000004 /* ContentView+MoveTabToNewWorkspace.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ContentView+MoveTabToNewWorkspace.swift"; sourceTree = ""; }; D7AB34300000000000000002 /* SidebarDropPlanner.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarDropPlanner.swift; sourceTree = ""; }; D7AB34300000000000000004 /* SidebarBonsplitTabWorkspaceDropOverlay.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarBonsplitTabWorkspaceDropOverlay.swift; sourceTree = ""; }; + D7AB34400000000000000002 /* WorkspaceSurfaceConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceSurfaceConfig.swift; sourceTree = ""; }; D0C0D0C0D0C0D0C0D0C0D002 /* DockPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DockPanelView.swift; sourceTree = ""; }; D0C0D0C0D0C0D0C0D0C0D004 /* DockEmptyView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DockEmptyView.swift; sourceTree = ""; }; C0DE32470000000000000002 /* ContentViewIdentifierCopyCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContentViewIdentifierCopyCommands.swift; sourceTree = ""; }; @@ -822,6 +826,7 @@ A5001520 /* PostHogAnalytics.swift */, A5001416 /* Workspace.swift */, D7AB00000000000000000016 /* Workspace+DetachedSurfaceTransfer.swift */, + D7AB34400000000000000002 /* WorkspaceSurfaceConfig.swift */, 1A1B2C3D4E5F607180000002 /* WorkspacePromptSubmit.swift */, D0B10005A1B2C3D4E5F60001 /* WorkspacePortalPaneDrop.swift */, C0DE32470000000000000004 /* WorkspaceSurfaceIdentifierClipboardText.swift */, @@ -984,6 +989,7 @@ D0B1000DA1B2C3D4E5F60001 /* CmuxWebViewDragRoutingTests.swift */, D0B1000FA1B2C3D4E5F60001 /* BrowserPaneDropRoutingTests.swift */, 02FC74F2C27127CC565B3E8C /* TerminalAndGhosttyTests.swift */, + D7AB34400000000000000004 /* GhosttyTerminalViewVisibilityPolicyTests.swift */, D7AB00000000000000000012 /* WorkspaceAdjacentPaneMoveTests.swift */, D0B10009A1B2C3D4E5F60001 /* PortalTabDragRoutingTests.swift */, 71F8ED91A4B55D34BE6A0668 /* WorkspaceUnitTests.swift */, @@ -1305,6 +1311,7 @@ A5001521 /* PostHogAnalytics.swift in Sources */, A5001406 /* Workspace.swift in Sources */, D7AB00000000000000000015 /* Workspace+DetachedSurfaceTransfer.swift in Sources */, + D7AB34400000000000000001 /* WorkspaceSurfaceConfig.swift in Sources */, 1A1B2C3D4E5F607180000001 /* WorkspacePromptSubmit.swift in Sources */, D0B10004A1B2C3D4E5F60001 /* WorkspacePortalPaneDrop.swift in Sources */, C0DE32470000000000000003 /* WorkspaceSurfaceIdentifierClipboardText.swift in Sources */, @@ -1501,6 +1508,7 @@ D0B1000CA1B2C3D4E5F60001 /* CmuxWebViewDragRoutingTests.swift in Sources */, D0B1000EA1B2C3D4E5F60001 /* BrowserPaneDropRoutingTests.swift in Sources */, 46F6AC15863EC84DCD3770A2 /* TerminalAndGhosttyTests.swift in Sources */, + D7AB34400000000000000003 /* GhosttyTerminalViewVisibilityPolicyTests.swift in Sources */, D7AB00000000000000000011 /* WorkspaceAdjacentPaneMoveTests.swift in Sources */, D0B10008A1B2C3D4E5F60001 /* PortalTabDragRoutingTests.swift in Sources */, 6B524A0BA34FD46A771335AB /* WorkspaceUnitTests.swift in Sources */, diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index ed44631b2f01..7973b67e3182 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -3859,11 +3859,13 @@ struct ContentView: View { maxMounted: maxMounted ) let removedIds = previousMountedIds.filter { !mountedWorkspaceIds.contains($0) } - hidePortalViewsForUnmountedWorkspaces( - removedIds, - tabs: currentTabs, - selectedId: effectiveSelectedId - ) + let mountedIdSet = Set(mountedWorkspaceIds) + for workspace in currentTabs { + workspace.setPortalRenderingEnabled( + mountedIdSet.contains(workspace.id), + reason: "workspaceMount" + ) + } #if DEBUG if mountedWorkspaceIds != previousMountedIds { let added = mountedWorkspaceIds.filter { !previousMountedIds.contains($0) } @@ -3885,19 +3887,6 @@ struct ContentView: View { #endif } - private func hidePortalViewsForUnmountedWorkspaces( - _ workspaceIds: [UUID], - tabs: [Workspace], - selectedId: UUID? - ) { - guard !workspaceIds.isEmpty else { return } - let unmountedIds = Set(workspaceIds) - for workspace in tabs where unmountedIds.contains(workspace.id) && workspace.id != selectedId { - workspace.hideAllTerminalPortalViews() - workspace.hideAllBrowserPortalViews() - } - } - private enum BackgroundWorkspacePrimeState { case pending case completed(reason: String) @@ -4379,14 +4368,13 @@ struct ContentView: View { workspaceHandoffFallbackTask = nil let retiring = retiringWorkspaceId - // Hide portal-hosted views for the retiring workspace BEFORE clearing + // Disable portal rendering for the retiring workspace BEFORE clearing // retiringWorkspaceId. Once cleared, reconcileMountedWorkspaceIds unmounts // the workspace — but dismantleNSView intentionally doesn't hide portal views - // during transient rebuilds. Hiding here prevents stale terminal/browser - // portals from covering the newly selected workspace. + // during transient rebuilds. Disabling here also cancels stale layout follow-up + // loops that could re-show an old terminal above the newly selected workspace. if let retiring, let workspace = tabManager.tabs.first(where: { $0.id == retiring }) { - workspace.hideAllTerminalPortalViews() - workspace.hideAllBrowserPortalViews() + workspace.setPortalRenderingEnabled(false, reason: "workspaceHandoff") } retiringWorkspaceId = nil diff --git a/Sources/GhosttyTerminalView.swift b/Sources/GhosttyTerminalView.swift index e97fbdfd7046..830ad5373ad6 100644 --- a/Sources/GhosttyTerminalView.swift +++ b/Sources/GhosttyTerminalView.swift @@ -13043,14 +13043,12 @@ struct GhosttyTerminalView: NSViewRepresentable { weak var hostedView: GhosttySurfaceScrollView? } - func makeCoordinator() -> Coordinator { - Coordinator() - } + func makeCoordinator() -> Coordinator { Coordinator() } static func shouldApplyImmediateHostedStateUpdate( - hostedViewHasSuperview: Bool, - isBoundToCurrentHost: Bool + desiredVisibleInUI: Bool, hostedViewHasSuperview: Bool, isBoundToCurrentHost: Bool ) -> Bool { + if !desiredVisibleInUI { return true } // If this update originates from a stale/replaced host while the hosted view is // already attached elsewhere, do not mutate visibility/active state here. if isBoundToCurrentHost { return true } @@ -13337,6 +13335,7 @@ struct GhosttyTerminalView: NSViewRepresentable { TerminalWindowPortalRegistry.isHostedView(hostedView, boundTo: host) } ?? true let shouldApplyImmediateHostedState = hostOwnsPortalNow && Self.shouldApplyImmediateHostedStateUpdate( + desiredVisibleInUI: isVisibleInUI, hostedViewHasSuperview: hostedView.superview != nil, isBoundToCurrentHost: isBoundToCurrentHost ) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index c259187096c5..6f4b41c249d6 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -23,40 +23,6 @@ private func debugWorkspaceDescriptionPreview(_ text: String?, limit: Int = 120) } #endif -struct CmuxSurfaceConfigTemplate { - var fontSize: Float32 = 0 - var workingDirectory: String? - var command: String? - var environmentVariables: [String: String] = [:] - var initialInput: String? - var waitAfterCommand: Bool = false - - init() {} - - init(cConfig: ghostty_surface_config_s) { - fontSize = cConfig.font_size - if let workingDirectory = cConfig.working_directory { - self.workingDirectory = String(cString: workingDirectory, encoding: .utf8) - } - if let command = cConfig.command { - self.command = String(cString: command, encoding: .utf8) - } - if let initialInput = cConfig.initial_input { - self.initialInput = String(cString: initialInput, encoding: .utf8) - } - if cConfig.env_var_count > 0, let envVars = cConfig.env_vars { - for index in 0.. String { - switch context { - case GHOSTTY_SURFACE_CONTEXT_WINDOW: - return "window" - case GHOSTTY_SURFACE_CONTEXT_TAB: - return "tab" - case GHOSTTY_SURFACE_CONTEXT_SPLIT: - return "split" - default: - return "unknown(\(context))" - } -} - -private func cmuxPointerAppearsLive(_ pointer: UnsafeMutableRawPointer?) -> Bool { - guard let pointer, - malloc_zone_from_ptr(pointer) != nil else { - return false - } - return malloc_size(pointer) > 0 -} - -func cmuxSurfacePointerAppearsLive(_ surface: ghostty_surface_t) -> Bool { - // Best-effort check: reject pointers that no longer belong to an active - // malloc zone allocation. A Swift wrapper around `ghostty_surface_t` can - // remain non-nil after the backing native surface has already been freed. - cmuxPointerAppearsLive(surface) -} - -func cmuxCurrentSurfaceFontSizePoints(_ surface: ghostty_surface_t) -> Float? { - guard cmuxSurfacePointerAppearsLive(surface) else { - return nil - } - - guard let quicklookFont = ghostty_surface_quicklook_font(surface) else { - return nil - } - - let ctFont = Unmanaged.fromOpaque(quicklookFont).takeUnretainedValue() - let points = Float(CTFontGetSize(ctFont)) - guard points > 0 else { return nil } - return points -} - -func cmuxInheritedSurfaceConfig( - sourceSurface: ghostty_surface_t, - context: ghostty_surface_context_e -) -> CmuxSurfaceConfigTemplate { - let inherited = ghostty_surface_inherited_config(sourceSurface, context) - var config = CmuxSurfaceConfigTemplate(cConfig: inherited) - - // Make runtime zoom inheritance explicit, even when Ghostty's - // inherit-font-size config is disabled. - let runtimePoints = cmuxCurrentSurfaceFontSizePoints(sourceSurface) - if let points = runtimePoints { - config.fontSize = points - } - -#if DEBUG - let inheritedText = String(format: "%.2f", inherited.font_size) - let runtimeText = runtimePoints.map { String(format: "%.2f", $0) } ?? "nil" - let finalText = String(format: "%.2f", config.fontSize) - cmuxDebugLog( - "zoom.inherit context=\(cmuxSurfaceContextName(context)) " + - "inherited=\(inheritedText) runtime=\(runtimeText) final=\(finalText)" - ) -#endif - - return config -} - struct SidebarStatusEntry: Equatable { let key: String let value: String @@ -8054,6 +7950,7 @@ final class Workspace: Identifiable, ObservableObject { private var layoutFollowUpAttemptScheduled = false private var layoutFollowUpAttemptVersion: Int = 0 private var layoutFollowUpStalledAttemptCount = 0 + private var portalRenderingEnabled = true private var isAttemptingLayoutFollowUp = false private var isNormalizingPinnedTabOrder = false private var pendingNonFocusSplitFocusReassert: PendingNonFocusSplitFocusReassert? @@ -10608,6 +10505,8 @@ final class Workspace: Identifiable, ObservableObject { // Hide portal-hosted content up front so a workspace being torn down // cannot keep drawing above the next selected/restored workspace while // panel close work is still unwinding. + portalRenderingEnabled = false + clearLayoutFollowUp() hideAllTerminalPortalViews() hideAllBrowserPortalViews() let panelEntries = Array(panels) @@ -11625,6 +11524,23 @@ final class Workspace: Identifiable, ObservableObject { } } + func setPortalRenderingEnabled(_ enabled: Bool, reason: String) { + let changed = portalRenderingEnabled != enabled + portalRenderingEnabled = enabled + if enabled { + if changed { + beginEventDrivenLayoutFollowUp( + reason: reason, + includeGeometry: true + ) + } + } else { + clearLayoutFollowUp() + hideAllTerminalPortalViews() + hideAllBrowserPortalViews() + } + } + // MARK: - Utility /// Writes a small shell wrapper that prints a banner ("remote ssh ended — target X"), @@ -11740,6 +11656,7 @@ final class Workspace: Identifiable, ObservableObject { } private func reconcileFocusState() { + guard portalRenderingEnabled else { return } guard !isReconcilingFocusState else { return } isReconcilingFocusState = true defer { isReconcilingFocusState = false } @@ -11796,6 +11713,7 @@ final class Workspace: Identifiable, ObservableObject { /// Reconcile focus/first-responder convergence. /// Coalesce to the next main-queue turn so bonsplit selection/pane mutations settle first. private func scheduleFocusReconcile() { + guard portalRenderingEnabled else { return } #if DEBUG if isDetachingCloseTransaction { debugFocusReconcileScheduledDuringDetachCount += 1 @@ -11805,6 +11723,10 @@ final class Workspace: Identifiable, ObservableObject { focusReconcileScheduled = true DispatchQueue.main.async { [weak self] in guard let self else { return } + guard self.portalRenderingEnabled else { + self.focusReconcileScheduled = false + return + } self.focusReconcileScheduled = false self.reconcileFocusState() } @@ -11817,6 +11739,7 @@ final class Workspace: Identifiable, ObservableObject { terminalFocusPanelId: UUID? = nil, includeGeometry: Bool = false ) { + guard portalRenderingEnabled else { return } layoutFollowUpReason = reason if let browserPanelId { layoutFollowUpBrowserPanelId = browserPanelId @@ -11941,6 +11864,7 @@ final class Workspace: Identifiable, ObservableObject { } private func scheduleLayoutFollowUpAttempt() { + guard portalRenderingEnabled else { return } guard layoutFollowUpTimeoutWorkItem != nil else { return } guard !layoutFollowUpAttemptScheduled else { return } @@ -11950,6 +11874,11 @@ final class Workspace: Identifiable, ObservableObject { DispatchQueue.main.asyncAfter(deadline: .now() + delay) { [weak self] in guard let self else { return } guard self.layoutFollowUpAttemptVersion == version else { return } + guard self.portalRenderingEnabled else { + self.layoutFollowUpAttemptScheduled = false + self.clearLayoutFollowUp() + return + } self.layoutFollowUpAttemptScheduled = false self.attemptEventDrivenLayoutFollowUp() } @@ -12015,6 +11944,12 @@ final class Workspace: Identifiable, ObservableObject { private func attemptEventDrivenLayoutFollowUp() { guard layoutFollowUpTimeoutWorkItem != nil, !isAttemptingLayoutFollowUp else { return } + guard portalRenderingEnabled else { + clearLayoutFollowUp() + hideAllTerminalPortalViews() + hideAllBrowserPortalViews() + return + } isAttemptingLayoutFollowUp = true defer { isAttemptingLayoutFollowUp = false } @@ -12165,6 +12100,7 @@ final class Workspace: Identifiable, ObservableObject { } private func renderedVisiblePanelIdsForCurrentLayout() -> Set { + guard portalRenderingEnabled else { return [] } let renderedPaneIds = bonsplitController.zoomedPaneId.map { [$0] } ?? bonsplitController.allPaneIds var visiblePanelIds: Set = [] @@ -12234,6 +12170,13 @@ final class Workspace: Identifiable, ObservableObject { return false } +#if DEBUG + @discardableResult + func debugReconcileTerminalPortalVisibilityForTesting() -> Bool { + reconcileTerminalPortalVisibilityForCurrentRenderedLayout() + } +#endif + @discardableResult private func reconcileBrowserPortalVisibilityForCurrentRenderedLayout(reason: String) -> Bool { let visiblePanelIds = renderedVisiblePanelIdsForCurrentLayout() diff --git a/Sources/WorkspaceSurfaceConfig.swift b/Sources/WorkspaceSurfaceConfig.swift new file mode 100644 index 000000000000..9ed9d9c1d3d5 --- /dev/null +++ b/Sources/WorkspaceSurfaceConfig.swift @@ -0,0 +1,107 @@ +import CoreText +import Darwin +import Foundation + +struct CmuxSurfaceConfigTemplate { + var fontSize: Float32 = 0 + var workingDirectory: String? + var command: String? + var environmentVariables: [String: String] = [:] + var initialInput: String? + var waitAfterCommand: Bool = false + + init() {} + + init(cConfig: ghostty_surface_config_s) { + fontSize = cConfig.font_size + if let workingDirectory = cConfig.working_directory { + self.workingDirectory = String(cString: workingDirectory, encoding: .utf8) + } + if let command = cConfig.command { + self.command = String(cString: command, encoding: .utf8) + } + if let initialInput = cConfig.initial_input { + self.initialInput = String(cString: initialInput, encoding: .utf8) + } + if cConfig.env_var_count > 0, let envVars = cConfig.env_vars { + for index in 0.. String { + switch context { + case GHOSTTY_SURFACE_CONTEXT_WINDOW: + return "window" + case GHOSTTY_SURFACE_CONTEXT_TAB: + return "tab" + case GHOSTTY_SURFACE_CONTEXT_SPLIT: + return "split" + default: + return "unknown(\(context))" + } +} + +private func cmuxPointerAppearsLive(_ pointer: UnsafeMutableRawPointer?) -> Bool { + guard let pointer, + malloc_zone_from_ptr(pointer) != nil else { + return false + } + return malloc_size(pointer) > 0 +} + +func cmuxSurfacePointerAppearsLive(_ surface: ghostty_surface_t) -> Bool { + // Best-effort check: reject pointers that no longer belong to an active + // malloc zone allocation. A Swift wrapper around `ghostty_surface_t` can + // remain non-nil after the backing native surface has already been freed. + cmuxPointerAppearsLive(surface) +} + +func cmuxCurrentSurfaceFontSizePoints(_ surface: ghostty_surface_t) -> Float? { + guard cmuxSurfacePointerAppearsLive(surface) else { + return nil + } + + guard let quicklookFont = ghostty_surface_quicklook_font(surface) else { + return nil + } + + let ctFont = Unmanaged.fromOpaque(quicklookFont).takeUnretainedValue() + let points = Float(CTFontGetSize(ctFont)) + guard points > 0 else { return nil } + return points +} + +func cmuxInheritedSurfaceConfig( + sourceSurface: ghostty_surface_t, + context: ghostty_surface_context_e +) -> CmuxSurfaceConfigTemplate { + let inherited = ghostty_surface_inherited_config(sourceSurface, context) + var config = CmuxSurfaceConfigTemplate(cConfig: inherited) + + // Make runtime zoom inheritance explicit, even when Ghostty's + // inherit-font-size config is disabled. + let runtimePoints = cmuxCurrentSurfaceFontSizePoints(sourceSurface) + if let points = runtimePoints { + config.fontSize = points + } + +#if DEBUG + let inheritedText = String(format: "%.2f", inherited.font_size) + let runtimeText = runtimePoints.map { String(format: "%.2f", $0) } ?? "nil" + let finalText = String(format: "%.2f", config.fontSize) + cmuxDebugLog( + "zoom.inherit context=\(cmuxSurfaceContextName(context)) " + + "inherited=\(inheritedText) runtime=\(runtimeText) final=\(finalText)" + ) +#endif + + return config +} diff --git a/cmuxTests/GhosttyTerminalViewVisibilityPolicyTests.swift b/cmuxTests/GhosttyTerminalViewVisibilityPolicyTests.swift new file mode 100644 index 000000000000..2f9a6d0858e9 --- /dev/null +++ b/cmuxTests/GhosttyTerminalViewVisibilityPolicyTests.swift @@ -0,0 +1,60 @@ +import XCTest + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +final class GhosttyTerminalViewVisibilityPolicyTests: XCTestCase { + func testImmediateStateUpdateAllowedWhenDesiredStateIsHidden() { + XCTAssertTrue( + GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( + desiredVisibleInUI: false, + hostedViewHasSuperview: true, + isBoundToCurrentHost: false + ) + ) + } + + func testImmediateStateUpdateAllowedWhenBoundToCurrentHost() { + XCTAssertTrue( + GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( + desiredVisibleInUI: true, + hostedViewHasSuperview: true, + isBoundToCurrentHost: true + ) + ) + } + + func testImmediateStateUpdateSkippedForStaleHostBoundElsewhere() { + XCTAssertFalse( + GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( + desiredVisibleInUI: true, + hostedViewHasSuperview: true, + isBoundToCurrentHost: false + ) + ) + } + + func testImmediateStateUpdateAllowedWhenUnboundAndNotAttachedAnywhere() { + XCTAssertTrue( + GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( + desiredVisibleInUI: true, + hostedViewHasSuperview: false, + isBoundToCurrentHost: false + ) + ) + } + + func testInteractiveGeometryResizeUsesImmediatePortalSyncDecision() { + XCTAssertTrue( + GhosttyTerminalView.shouldSynchronizePortalGeometryImmediately( + hostInLiveResize: false, + windowInLiveResize: false, + interactiveGeometryResizeActive: true + ), + "Interactive resize should use the immediate portal sync path" + ) + } +} diff --git a/cmuxTests/TerminalAndGhosttyTests.swift b/cmuxTests/TerminalAndGhosttyTests.swift index d6eab4ee820b..c143a41971ae 100644 --- a/cmuxTests/TerminalAndGhosttyTests.swift +++ b/cmuxTests/TerminalAndGhosttyTests.swift @@ -4246,55 +4246,6 @@ final class TerminalControllerSocketTextChunkTests: XCTestCase { } -final class GhosttyTerminalViewVisibilityPolicyTests: XCTestCase { - func testImmediateStateUpdateAllowedWhenHostNotInWindow() { - XCTAssertTrue( - GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostedViewHasSuperview: true, - isBoundToCurrentHost: false - ) - ) - } - - func testImmediateStateUpdateAllowedWhenBoundToCurrentHost() { - XCTAssertTrue( - GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostedViewHasSuperview: true, - isBoundToCurrentHost: true - ) - ) - } - - func testImmediateStateUpdateSkippedForStaleHostBoundElsewhere() { - XCTAssertFalse( - GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostedViewHasSuperview: true, - isBoundToCurrentHost: false - ) - ) - } - - func testImmediateStateUpdateAllowedWhenUnboundAndNotAttachedAnywhere() { - XCTAssertTrue( - GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostedViewHasSuperview: false, - isBoundToCurrentHost: false - ) - ) - } - - func testInteractiveGeometryResizeUsesImmediatePortalSyncDecision() { - XCTAssertTrue( - GhosttyTerminalView.shouldSynchronizePortalGeometryImmediately( - hostInLiveResize: false, - windowInLiveResize: false, - interactiveGeometryResizeActive: true - ), - "Interactive resize should use the immediate portal sync path" - ) - } -} - final class GhosttyModifierFlagsChangedActionTests: XCTestCase { func testLeftShiftPressReturnsPress() { XCTAssertEqual( diff --git a/cmuxTests/WorkspaceUnitTests.swift b/cmuxTests/WorkspaceUnitTests.swift index 849701e6cab4..959c98a438b2 100644 --- a/cmuxTests/WorkspaceUnitTests.swift +++ b/cmuxTests/WorkspaceUnitTests.swift @@ -2938,6 +2938,23 @@ final class WorkspaceTeardownTests: XCTestCase { XCTAssertTrue(workspace.pinnedPanelIds.isEmpty) XCTAssertTrue(workspace.manualUnreadPanelIds.isEmpty) } + + func testDisabledPortalRenderingDoesNotRestoreTerminalVisibility() throws { +#if DEBUG + let workspace = Workspace() + let panelId = try XCTUnwrap(workspace.focusedPanelId) + let terminalPanel = try XCTUnwrap(workspace.terminalPanel(for: panelId)) + + terminalPanel.hostedView.setVisibleInUI(true) + workspace.setPortalRenderingEnabled(false, reason: "test") + XCTAssertFalse(terminalPanel.hostedView.debugPortalVisibleInUI) + + workspace.debugReconcileTerminalPortalVisibilityForTesting() + XCTAssertFalse(terminalPanel.hostedView.debugPortalVisibleInUI) +#else + throw XCTSkip("Debug-only regression test") +#endif + } } From 78e98dc98b9f7b1aa01ef6bb202214fc6f249602 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Sat, 2 May 2026 02:37:58 -0700 Subject: [PATCH 7/7] fix: accept workspace drops below last row --- Sources/Sidebar/SidebarDropPlanner.swift | 9 ++++++--- cmuxTests/SidebarWorkspaceDropPlannerTests.swift | 16 +++++++++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/Sources/Sidebar/SidebarDropPlanner.swift b/Sources/Sidebar/SidebarDropPlanner.swift index 237e1dc53133..20839c9f9409 100644 --- a/Sources/Sidebar/SidebarDropPlanner.swift +++ b/Sources/Sidebar/SidebarDropPlanner.swift @@ -104,11 +104,14 @@ enum SidebarDropPlanner { return workspaceAction(for: point, in: containingTarget, orderedTargets: orderedTargets) } - guard let beforeTarget = orderedTargets.first(where: { point.y < $0.frame.minY }) else { - return nil + let proposedInsertion: Int + if let beforeTarget = orderedTargets.first(where: { point.y < $0.frame.minY }) { + proposedInsertion = orderedTargets.firstIndex(of: beforeTarget) ?? 0 + } else { + proposedInsertion = orderedTargets.count } let insertionIndex = legalNewWorkspaceInsertionIndex( - orderedTargets.firstIndex(of: beforeTarget) ?? 0, + proposedInsertion, orderedTargets: orderedTargets ) return .newWorkspace( diff --git a/cmuxTests/SidebarWorkspaceDropPlannerTests.swift b/cmuxTests/SidebarWorkspaceDropPlannerTests.swift index 33976ea48be1..819eedddf20c 100644 --- a/cmuxTests/SidebarWorkspaceDropPlannerTests.swift +++ b/cmuxTests/SidebarWorkspaceDropPlannerTests.swift @@ -78,15 +78,21 @@ final class SidebarWorkspaceDropPlannerTests: XCTestCase { ) } - func testWorkspaceDropAfterLastRowFallsThroughToEmptyAreaDropTarget() { + func testWorkspaceDropAfterLastRowCreatesWorkspaceAtEnd() { let first = UUID() let second = UUID() let targets = workspaceDropTargets([first, second]) - XCTAssertNil( - SidebarDropPlanner.workspaceAction( - for: CGPoint(x: 12, y: 92), - targets: targets + let action = SidebarDropPlanner.workspaceAction( + for: CGPoint(x: 12, y: 92), + targets: targets + ) + + XCTAssertEqual( + action, + .newWorkspace( + insertionIndex: 2, + indicator: SidebarDropIndicator(tabId: nil, edge: .bottom) ) ) }