Repository navigation
Unify tab bar button lane layout #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 30 commits
19255c6
a6ba1db
0f608a4
be79a65
68ec77f
c1e7c7f
e73e43f
cbd73c4
4f72a3e
d0de5d1
625c370
e022718
77a6f8c
f462bf5
e625083
7aef42c
bfaad67
4730ebb
e48b4ae
9fffbdd
d212b31
a9ea9bd
5dd5b92
3db51a7
eaae2e9
bb53b34
b68e2c4
c5b2449
125625a
7c4439c
8106a1a
6dac95f
819688a
6635168
cc0f548
eeb04dd
9d082c0
90f4981
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -299,7 +299,7 @@ struct PaneContainerView<Content: View, EmptyContent: View>: View { | |
| .allowsHitTesting(!isTabDragActive) | ||
|
|
||
| Color.clear | ||
| .onDrop(of: [.tabTransfer], delegate: UnifiedPaneDropDelegate( | ||
| .onDrop(of: [.tabTransfer, .fileURL], delegate: UnifiedPaneDropDelegate( | ||
| size: size, | ||
| pane: pane, | ||
| controller: controller, | ||
|
|
@@ -408,29 +408,28 @@ struct UnifiedPaneDropDelegate: DropDelegate { | |
| // may not have propagated yet when performDrop runs. | ||
| guard let draggedTab = controller.activeDragTab ?? controller.draggingTab, | ||
| let sourcePaneId = controller.activeDragSourcePaneId ?? controller.dragSourcePaneId else { | ||
| guard let transfer = decodeTransfer(from: info), | ||
| transfer.isFromCurrentProcess else { | ||
| return false | ||
| } | ||
| let destination: BonsplitController.ExternalTabDropRequest.Destination | ||
| if zone == .center { | ||
| destination = .insert(targetPane: pane.id, targetIndex: nil) | ||
| } else if let orientation = zone.orientation { | ||
| destination = .split( | ||
| targetPane: pane.id, | ||
| orientation: orientation, | ||
| insertFirst: zone.insertsFirst | ||
| if info.hasItemsConforming(to: [.tabTransfer]) { | ||
| guard let transfer = decodeTransfer(from: info), | ||
| transfer.isFromCurrentProcess, | ||
| let destination = destination(for: zone) else { | ||
| return false | ||
| } | ||
|
|
||
| let request = BonsplitController.ExternalTabDropRequest( | ||
| tabId: TabID(id: transfer.tab.id), | ||
| sourcePaneId: PaneID(id: transfer.sourcePaneId), | ||
| destination: destination | ||
| ) | ||
| } else { | ||
| return false | ||
| let handled = bonsplitController.onExternalTabDrop?(request) ?? false | ||
| if handled { | ||
| dropLifecycle = .idle | ||
| activeDropZone = nil | ||
| } | ||
| return handled | ||
| } | ||
|
|
||
| let request = BonsplitController.ExternalTabDropRequest( | ||
| tabId: TabID(id: transfer.tab.id), | ||
| sourcePaneId: PaneID(id: transfer.sourcePaneId), | ||
| destination: destination | ||
| ) | ||
| let handled = bonsplitController.onExternalTabDrop?(request) ?? false | ||
| guard info.hasItemsConforming(to: [.fileURL]) else { return false } | ||
| let handled = performFileDrop(info: info, zone: zone) | ||
| if handled { | ||
| dropLifecycle = .idle | ||
| activeDropZone = nil | ||
|
|
@@ -509,14 +508,14 @@ struct UnifiedPaneDropDelegate: DropDelegate { | |
| #if DEBUG | ||
| dlog("pane.dropUpdated.skip pane=\(pane.id.id.uuidString.prefix(5)) reason=lifecycle_idle") | ||
| #endif | ||
| return DropProposal(operation: .move) | ||
| return DropProposal(operation: dropOperation(for: info)) | ||
| } | ||
| let zone = effectiveZone(for: info) | ||
| activeDropZone = zone | ||
| #if DEBUG | ||
| dlog("pane.dropUpdated pane=\(pane.id.id.uuidString.prefix(5)) zone=\(zone)") | ||
| #endif | ||
| return DropProposal(operation: .move) | ||
| return DropProposal(operation: dropOperation(for: info)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For hosts that only set the legacy Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 9d082c0 by making dropUpdated use the same file-drop zone acceptance as validateDrop. — Claude Code |
||
| } | ||
|
|
||
| func validateDrop(info: DropInfo) -> Bool { | ||
|
|
@@ -527,33 +526,117 @@ struct UnifiedPaneDropDelegate: DropDelegate { | |
| #endif | ||
| return false | ||
| } | ||
| // The custom UTType alone is sufficient — only Bonsplit tab drags produce it. | ||
| // Do NOT gate on draggingTab != nil: @Observable changes from createItemProvider | ||
| // may not have propagated to the drop delegate yet, causing false rejections. | ||
| let hasType = info.hasItemsConforming(to: [.tabTransfer]) | ||
| guard hasType else { return false } | ||
| let hasTabTransfer = info.hasItemsConforming(to: [.tabTransfer]) | ||
| let hasFileURL = info.hasItemsConforming(to: [.fileURL]) | ||
| guard hasTabTransfer || hasFileURL else { return false } | ||
|
|
||
| // Local drags use in-memory state and are always same-process. | ||
| if controller.activeDragTab != nil || controller.draggingTab != nil { | ||
| return true | ||
| } | ||
|
|
||
| // External drags (another Bonsplit controller) must include a payload from this process. | ||
| guard let transfer = decodeTransfer(from: info), | ||
| transfer.isFromCurrentProcess else { | ||
| return false | ||
| if hasTabTransfer { | ||
| // External drags (another Bonsplit controller) must include a payload from this process. | ||
| guard let transfer = decodeTransfer(from: info), | ||
| transfer.isFromCurrentProcess else { | ||
| return false | ||
| } | ||
| } else if hasFileURL { | ||
| guard Self.acceptsFileDrop( | ||
| zone: effectiveZone(for: info), | ||
| hasExternalFileDropHandler: bonsplitController.onExternalFileDrop != nil, | ||
| hasLegacyFileDropHandler: controller.onFileDrop != nil | ||
| ) else { | ||
| return false | ||
| } | ||
| } | ||
| #if DEBUG | ||
| let hasDrag = controller.draggingTab != nil | ||
| let hasActive = controller.activeDragTab != nil | ||
| dlog( | ||
| "pane.validateDrop pane=\(pane.id.id.uuidString.prefix(5)) " + | ||
| "allowed=\(hasType ? 1 : 0) hasDrag=\(hasDrag ? 1 : 0) hasActive=\(hasActive ? 1 : 0)" | ||
| "hasTab=\(hasTabTransfer ? 1 : 0) hasFile=\(hasFileURL ? 1 : 0) " + | ||
| "hasDrag=\(hasDrag ? 1 : 0) hasActive=\(hasActive ? 1 : 0)" | ||
| ) | ||
| #endif | ||
| return true | ||
| } | ||
|
|
||
| static func acceptsFileDrop( | ||
| zone: DropZone, | ||
| hasExternalFileDropHandler: Bool, | ||
| hasLegacyFileDropHandler: Bool | ||
| ) -> Bool { | ||
| if hasExternalFileDropHandler { | ||
| return true | ||
| } | ||
| guard hasLegacyFileDropHandler else { | ||
| return false | ||
| } | ||
| return zone == .center | ||
| } | ||
|
|
||
| private func dropOperation(for info: DropInfo) -> DropOperation { | ||
| info.hasItemsConforming(to: [.fileURL]) && !info.hasItemsConforming(to: [.tabTransfer]) | ||
| ? .copy | ||
| : .move | ||
| } | ||
|
|
||
| private func destination( | ||
| for zone: DropZone | ||
| ) -> BonsplitController.ExternalTabDropRequest.Destination? { | ||
| if zone == .center { | ||
| return .insert(targetPane: pane.id, targetIndex: nil) | ||
| } | ||
| guard let orientation = zone.orientation else { return nil } | ||
| return .split( | ||
| targetPane: pane.id, | ||
| orientation: orientation, | ||
| insertFirst: zone.insertsFirst | ||
| ) | ||
| } | ||
|
|
||
| private func performFileDrop(info: DropInfo, zone: DropZone) -> Bool { | ||
| guard let destination = destination(for: zone) else { return false } | ||
| let urls = fileURLs(from: info) | ||
| guard !urls.isEmpty else { return false } | ||
|
|
||
| if let handler = bonsplitController.onExternalFileDrop { | ||
| return handler(BonsplitController.ExternalFileDropRequest(urls: urls, destination: destination)) | ||
| } | ||
|
|
||
| if case .insert = destination { | ||
| return controller.onFileDrop?(urls, pane.id) ?? false | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| private func fileURLs(from info: DropInfo) -> [URL] { | ||
| guard info.hasItemsConforming(to: [.fileURL]) else { return [] } | ||
|
|
||
| // DropInfo exposes file URLs through asynchronous item providers, but | ||
| // DropDelegate.performDrop needs a synchronous result. AppKit publishes | ||
| // Finder/local file drags on the drag pasteboard for this callback. | ||
| return Self.fileURLs(from: NSPasteboard(name: .drag)) | ||
|
Comment on lines
+671
to
+677
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not changing this in this PR because DropDelegate.performDrop and Bonsplit's file-drop handler are synchronous. Provider-only file promises need a separate async drop API instead of blocking the main thread; the supported synchronous path remains the drag pasteboard. — Claude Code
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handled in 90f4981. The synchronous DropDelegate path now only validates file drops when the drag pasteboard has readable file URLs, matching what performDrop can actually consume. Provider-only async file promises need a separate async drop API instead of accepting a drop that this delegate cannot fulfill synchronously. |
||
| } | ||
|
|
||
| static func fileURLs(from pasteboard: NSPasteboard) -> [URL] { | ||
| let objects = pasteboard.readObjects( | ||
| forClasses: [NSURL.self], | ||
| options: [.urlReadingFileURLsOnly: true] | ||
| ) ?? [] | ||
| return objects.compactMap { object -> URL? in | ||
| if let url = object as? URL { | ||
| return url.isFileURL ? url : nil | ||
| } | ||
| if let url = object as? NSURL { | ||
| let bridged = url as URL | ||
| return bridged.isFileURL ? bridged : nil | ||
| } | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| private func decodeTransfer(from string: String) -> TabTransferData? { | ||
| guard let data = string.data(using: .utf8), | ||
| let transfer = try? JSONDecoder().decode(TabTransferData.self, from: data) else { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that this drop target accepts
.fileURL,performDropstill prioritizes the in-memory tab-drag state (activeDragTab/draggingTab) before checking the dropped item types, so any lingering local drag state causes a file drop to be handled as a tab move/split instead of a file drop. This is a behavioral regression introduced by adding file support: a stale tab-drag state can make Finder file drops unexpectedly rearrange tabs rather than invoking file handlers.Useful? React with 👍 / 👎.