Skip to content

Add tab context menu open availability hook #158

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.

Sign up for GitHub

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

Merged
austinywang merged 2 commits into from
Jul 6, 2026
Merged

Add tab context menu open availability hook #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0daa688
Add tab context menu open availability hook
austinywang Jul 5, 2026
222f9a7
Keep fork conversation visible while refreshing
austinywang Jul 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Sources/Bonsplit/Internal/Views/TabBarView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ struct TabContextMenuState {
let canMoveToNewWorkspace: Bool
let canMoveToLeftPane: Bool
let canMoveToRightPane: Bool
let canForkConversation: Bool
var canForkConversation: Bool
let forkConversationDefaultAction: TabContextAction
let isZoomed: Bool
let hasSplits: Bool
Expand Down Expand Up @@ -1349,6 +1349,9 @@ struct TabBarView: View {
moveDestinationsProvider: {
controller.tabContextMoveDestinationsProvider?(TabID(id: tab.id), pane.id) ?? []
},
forkConversationOpenAvailabilityProvider: {
controller.tabContextForkConversationOpenAvailabilityProvider?(TabID(id: tab.id), pane.id)
},
onSelect: {
// Tab selection must be instant. Animating this transaction causes the pane
// content (often swapped via opacity) to crossfade, which is undesirable for
Expand Down
28 changes: 23 additions & 5 deletions Sources/Bonsplit/Internal/Views/TabItemView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ struct TabItemView: View {
let allowsClose: Bool
let contextMenuState: TabContextMenuState
let moveDestinationsProvider: () -> [TabContextMoveDestination]
let forkConversationOpenAvailabilityProvider: () -> Bool?
let onSelect: () -> Void
let onClose: (TabCloseRequestSource) -> Void
let onZoomToggle: () -> Void
Expand Down Expand Up @@ -281,7 +282,8 @@ struct TabItemView: View {
snapshot: TabContextMenuSnapshot(
tabId: tab.id,
state: contextMenuState,
moveDestinationsProvider: moveDestinationsProvider
moveDestinationsProvider: moveDestinationsProvider,
forkConversationOpenAvailabilityProvider: forkConversationOpenAvailabilityProvider
),
onContextAction: onContextAction,
onMoveDestination: onMoveDestination
Expand Down Expand Up @@ -1112,6 +1114,7 @@ struct TabContextMenuSnapshot {
let tabId: UUID
let state: TabContextMenuState
let moveDestinationsProvider: () -> [TabContextMoveDestination]
let forkConversationOpenAvailabilityProvider: () -> Bool?
}

final class TabContextMenuActionTarget: NSObject {
Expand All @@ -1137,7 +1140,10 @@ enum TabContextMenuBuilder {
snapshot: TabContextMenuSnapshot,
target: TabContextMenuActionTarget
) -> NSMenu {
let state = snapshot.state
var state = snapshot.state
let canForkConversationAtOpen = snapshot.forkConversationOpenAvailabilityProvider()
let forkConversationEnabled = canForkConversationAtOpen ?? state.canForkConversation
state.canForkConversation = state.canForkConversation || forkConversationEnabled
let menu = NSMenu()
menu.autoenablesItems = false

Expand Down Expand Up @@ -1212,11 +1218,16 @@ enum TabContextMenuBuilder {
addAction(
title: localized("tabContext.forkConversation", defaultValue: "Fork Conversation"),
action: .forkConversation,
enabled: forkConversationEnabled,
state: state,
target: target,
to: menu
)
menu.addItem(forkConversationSubmenuItem(state: state, target: target))
menu.addItem(forkConversationSubmenuItem(
state: state,
target: target,
enabled: forkConversationEnabled
))
}

menu.addItem(.separator())
Expand Down Expand Up @@ -1359,7 +1370,8 @@ enum TabContextMenuBuilder {

private static func forkConversationSubmenuItem(
state: TabContextMenuState,
target: TabContextMenuActionTarget
target: TabContextMenuActionTarget,
enabled: Bool
) -> NSMenuItem {
let item = NSMenuItem(
title: localized("tabContext.forkConversationTo", defaultValue: "Fork Conversation To"),
Expand All @@ -1375,6 +1387,7 @@ enum TabContextMenuBuilder {
addAction(
title: localized("tabContext.forkConversation.right", defaultValue: "Right Split"),
action: .forkConversationRight,
enabled: enabled,
state: state,
target: target,
to: submenu,
Expand All @@ -1383,6 +1396,7 @@ enum TabContextMenuBuilder {
addAction(
title: localized("tabContext.forkConversation.left", defaultValue: "Left Split"),
action: .forkConversationLeft,
enabled: enabled,
state: state,
target: target,
to: submenu,
Expand All @@ -1391,6 +1405,7 @@ enum TabContextMenuBuilder {
addAction(
title: localized("tabContext.forkConversation.top", defaultValue: "Top Split"),
action: .forkConversationTop,
enabled: enabled,
state: state,
target: target,
to: submenu,
Expand All @@ -1399,6 +1414,7 @@ enum TabContextMenuBuilder {
addAction(
title: localized("tabContext.forkConversation.bottom", defaultValue: "Bottom Split"),
action: .forkConversationBottom,
enabled: enabled,
state: state,
target: target,
to: submenu,
Expand All @@ -1408,6 +1424,7 @@ enum TabContextMenuBuilder {
addAction(
title: localized("tabContext.forkConversation.newTab", defaultValue: "New Tab"),
action: .forkConversationNewTab,
enabled: enabled,
state: state,
target: target,
to: submenu,
Expand All @@ -1416,14 +1433,15 @@ enum TabContextMenuBuilder {
addAction(
title: localized("tabContext.forkConversation.newWorkspace", defaultValue: "New Workspace"),
action: .forkConversationNewWorkspace,
enabled: enabled,
state: state,
target: target,
to: submenu,
stateValue: defaultAction == .forkConversationNewWorkspace ? .on : .off
)

item.submenu = submenu
item.isEnabled = true
item.isEnabled = enabled
return item
}

Expand Down
3 changes: 3 additions & 0 deletions Sources/Bonsplit/Public/BonsplitController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ public final class BonsplitController {
/// session). Return `true` to enable the item, `false` (or omit the provider) to hide it.
@ObservationIgnored public var tabContextForkConversationAvailabilityProvider: ((TabID, PaneID) -> Bool)?

/// Host-provided check evaluated only when the tab context menu is opening.
@ObservationIgnored public var tabContextForkConversationOpenAvailabilityProvider: ((TabID, PaneID) -> Bool)?
Comment on lines +85 to +86

@greptile-apps greptile-apps Bot Jul 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The doc comment for tabContextForkConversationOpenAvailabilityProvider doesn't explain what nil, true, or false return values mean, or how this interacts with the existing tabContextForkConversationAvailabilityProvider. The adjacent provider has a clear contract ("Return true to enable the item, false (or omit the provider) to hide it"); this one should be equally explicit so callers understand that nil is a no-op that defers to the build-time check.

Suggested change
/// Host-provided check evaluated only when the tab context menu is opening.
@ObservationIgnored public var tabContextForkConversationOpenAvailabilityProvider: ((TabID, PaneID) -> Bool)?
/// Host-provided check evaluated only when the tab context menu is opening (i.e. on
/// `NSMenu` open, not during SwiftUI rendering). Return `true` to enable the "Fork
/// Conversation" item, `false` to hide it, or `nil` to leave the value produced by
/// `tabContextForkConversationAvailabilityProvider` unchanged.
@ObservationIgnored public var tabContextForkConversationOpenAvailabilityProvider: ((TabID, PaneID) -> Bool)?

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


/// Host-provided default destination for the tab context menu's primary "Fork
/// Conversation" action. Return a destination-specific fork action; invalid values
/// fall back to `.forkConversationRight`.
Expand Down
51 changes: 47 additions & 4 deletions Tests/BonsplitTests/BonsplitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1673,7 +1673,8 @@ final class BonsplitTests: XCTestCase {
return [
TabContextMoveDestination(id: "workspace:abc", title: "Workspace A", isEnabled: false)
]
}
},
forkConversationOpenAvailabilityProvider: { nil }
)

let menu = TabContextMenuBuilder.makeMenu(snapshot: snapshot, target: target)
Expand Down Expand Up @@ -1720,7 +1721,8 @@ final class BonsplitTests: XCTestCase {
hasSplits: false,
shortcuts: [:]
),
moveDestinationsProvider: { [] }
moveDestinationsProvider: { [] },
forkConversationOpenAvailabilityProvider: { nil }
)

let menu = TabContextMenuBuilder.makeMenu(snapshot: snapshot, target: target)
Expand Down Expand Up @@ -1754,7 +1756,8 @@ final class BonsplitTests: XCTestCase {
hasSplits: false,
shortcuts: [:]
),
moveDestinationsProvider: { [] }
moveDestinationsProvider: { [] },
forkConversationOpenAvailabilityProvider: { nil }
)

let menu = TabContextMenuBuilder.makeMenu(snapshot: snapshot, target: target)
Expand Down Expand Up @@ -1790,7 +1793,8 @@ final class BonsplitTests: XCTestCase {
let snapshot = TabContextMenuSnapshot(
tabId: UUID(),
state: state,
moveDestinationsProvider: { [] }
moveDestinationsProvider: { [] },
forkConversationOpenAvailabilityProvider: { nil }
)

let menu = TabContextMenuBuilder.makeMenu(snapshot: snapshot, target: target)
Expand All @@ -1811,6 +1815,45 @@ final class BonsplitTests: XCTestCase {
XCTAssertEqual(selectedAction, .forkConversationNewTab)
}

@MainActor
func testTabContextMenuBuilderKeepsForkConversationVisibleWhenOpenAvailabilityIsRefreshing() throws {
let target = TabContextMenuActionTarget()
let state = TabContextMenuState(
isPinned: false,
isUnread: false,
isBrowser: false,
isAudioMuted: false,
isTerminal: true,
hasCustomTitle: false,
canCloseToLeft: true,
canCloseToRight: true,
canCloseOthers: true,
canMoveToNewWorkspace: false,
canMoveToLeftPane: false,
canMoveToRightPane: false,
canForkConversation: true,
forkConversationDefaultAction: .forkConversationLeft,
isZoomed: false,
hasSplits: false,
shortcuts: [:]
)
let snapshot = TabContextMenuSnapshot(
tabId: UUID(),
state: state,
moveDestinationsProvider: { [] },
forkConversationOpenAvailabilityProvider: { false }
)

let menu = TabContextMenuBuilder.makeMenu(snapshot: snapshot, target: target)
let forkItem = try XCTUnwrap(menu.items.first { $0.title == "Fork Conversation" })
let forkSubmenuItem = try XCTUnwrap(menu.items.first { $0.title == "Fork Conversation To" })
let destinationItems = try XCTUnwrap(forkSubmenuItem.submenu?.items.filter { !$0.isSeparatorItem })

XCTAssertFalse(forkItem.isEnabled)
XCTAssertFalse(forkSubmenuItem.isEnabled)
XCTAssertEqual(destinationItems.map(\.isEnabled), Array(repeating: false, count: 6))
}

@MainActor
func testDoubleClickingEmptyTrailingTabBarSpaceRequestsNewTerminalTab() {
let appearance = BonsplitConfiguration.Appearance()
Expand Down
Loading