Skip to content

Commit 6a7e6ce

Browse files
datlechindatlechin
authored
Merge pull request #585 from TableProApp/refactor/unified-tab-creation
refactor: unify new tab creation — TabIntent, pendingPayloads, native tab bar fix
2 parents 3e13d10 + d69f475 commit 6a7e6ce

17 files changed

+289
-308
lines changed

‎TablePro/AppDelegate+WindowConfig.swift‎

Lines changed: 52 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,29 @@ extension AppDelegate {
6262
openWelcomeWindow()
6363
}
6464

65+
@objc func newWindowForTab(_ sender: Any?) {
66+
guard let keyWindow = NSApp.keyWindow,
67+
let connectionId = MainActor.assumeIsolated({
68+
WindowLifecycleMonitor.shared.connectionId(fromWindow: keyWindow)
69+
})
70+
else { return }
71+
72+
let payload = EditorTabPayload(
73+
connectionId: connectionId,
74+
intent: .newEmptyTab
75+
)
76+
MainActor.assumeIsolated {
77+
WindowOpener.shared.openNativeTab(payload)
78+
}
79+
}
80+
6581
@objc func connectFromDock(_ sender: NSMenuItem) {
6682
guard let connectionId = sender.representedObject as? UUID else { return }
6783
let connections = ConnectionStorage.shared.loadConnections()
6884
guard let connection = connections.first(where: { $0.id == connectionId }) else { return }
6985

70-
WindowOpener.shared.pendingConnectionId = connection.id
71-
NotificationCenter.default.post(name: .openMainWindow, object: connection.id)
86+
let payload = EditorTabPayload(connectionId: connection.id, intent: .restoreOrDefault)
87+
WindowOpener.shared.openNativeTab(payload)
7288

7389
Task { @MainActor in
7490
do {
@@ -248,65 +264,50 @@ extension AppDelegate {
248264
if isMainWindow(window) && !configuredWindows.contains(windowId) {
249265
window.tabbingMode = .preferred
250266
window.isRestorable = false
251-
let pendingId = MainActor.assumeIsolated { WindowOpener.shared.consumePendingConnectionId() }
252-
253-
// If no code opened this window (pendingId is nil), this is a
254-
// SwiftUI WindowGroup state restoration — not a window we created.
255-
// Hide it (orderOut, not close) to break the close→restore loop.
256-
// Exception: if the window is already part of a tab group, it was
257-
// attached by our addTabbedWindow call — not a restoration orphan.
258-
// Ordering it out would crash NSWindowStackController.
259-
if pendingId == nil && !isAutoReconnecting {
260-
configuredWindows.insert(windowId)
267+
configuredWindows.insert(windowId)
268+
269+
let pendingConnectionId = MainActor.assumeIsolated {
270+
WindowOpener.shared.consumeOldestPendingConnectionId()
271+
}
272+
273+
if pendingConnectionId == nil && !isAutoReconnecting {
261274
if let tabbedWindows = window.tabbedWindows, tabbedWindows.count > 1 {
262-
// Already in a tab group — leave it alone
263275
return
264276
}
265277
window.orderOut(nil)
266278
return
267279
}
268280

269-
let existingIdentifier = NSApp.windows
270-
.first { $0 !== window && isMainWindow($0) && $0.isVisible }?
271-
.tabbingIdentifier
272-
let groupAll = MainActor.assumeIsolated { AppSettingsManager.shared.tabs.groupAllConnectionTabs }
273-
let resolvedIdentifier = TabbingIdentifierResolver.resolve(
274-
pendingConnectionId: pendingId,
275-
existingIdentifier: existingIdentifier,
276-
groupAllConnections: groupAll
277-
)
278-
window.tabbingIdentifier = resolvedIdentifier
279-
configuredWindows.insert(windowId)
281+
if let connectionId = pendingConnectionId {
282+
let groupAll = MainActor.assumeIsolated { AppSettingsManager.shared.tabs.groupAllConnectionTabs }
283+
let resolvedIdentifier = WindowOpener.tabbingIdentifier(for: connectionId)
284+
window.tabbingIdentifier = resolvedIdentifier
280285

281-
if !NSWindow.allowsAutomaticWindowTabbing {
282-
NSWindow.allowsAutomaticWindowTabbing = true
283-
}
284-
285-
// Explicitly attach to existing tab group — automatic tabbing
286-
// doesn't work when tabbingIdentifier is set after window creation.
287-
let matchingWindow: NSWindow?
288-
if groupAll {
289-
// When grouping all connections, attach to any visible main window
290-
// and normalize all existing windows' tabbingIdentifiers so future
291-
// windows also match (not just the first one found).
292-
let existingMainWindows = NSApp.windows.filter {
293-
$0 !== window && isMainWindow($0) && $0.isVisible
286+
if !NSWindow.allowsAutomaticWindowTabbing {
287+
NSWindow.allowsAutomaticWindowTabbing = true
294288
}
295-
for existing in existingMainWindows {
296-
existing.tabbingIdentifier = resolvedIdentifier
289+
290+
let matchingWindow: NSWindow?
291+
if groupAll {
292+
let existingMainWindows = NSApp.windows.filter {
293+
$0 !== window && isMainWindow($0) && $0.isVisible
294+
}
295+
for existing in existingMainWindows {
296+
existing.tabbingIdentifier = resolvedIdentifier
297+
}
298+
matchingWindow = existingMainWindows.first
299+
} else {
300+
matchingWindow = NSApp.windows.first {
301+
$0 !== window && isMainWindow($0) && $0.isVisible
302+
&& $0.tabbingIdentifier == resolvedIdentifier
303+
}
297304
}
298-
matchingWindow = existingMainWindows.first
299-
} else {
300-
matchingWindow = NSApp.windows.first {
301-
$0 !== window && isMainWindow($0) && $0.isVisible
302-
&& $0.tabbingIdentifier == resolvedIdentifier
305+
if let existingWindow = matchingWindow {
306+
let targetWindow = existingWindow.tabbedWindows?.last ?? existingWindow
307+
targetWindow.addTabbedWindow(window, ordered: .above)
308+
window.makeKeyAndOrderFront(nil)
303309
}
304310
}
305-
if let existingWindow = matchingWindow {
306-
let targetWindow = existingWindow.tabbedWindows?.last ?? existingWindow
307-
targetWindow.addTabbedWindow(window, ordered: .above)
308-
window.makeKeyAndOrderFront(nil)
309-
}
310311
}
311312
}
312313

@@ -354,8 +355,8 @@ extension AppDelegate {
354355

355356
Task { @MainActor [weak self] in
356357
guard let self else { return }
357-
WindowOpener.shared.pendingConnectionId = connection.id
358-
NotificationCenter.default.post(name: .openMainWindow, object: connection.id)
358+
let payload = EditorTabPayload(connectionId: connection.id, intent: .restoreOrDefault)
359+
WindowOpener.shared.openNativeTab(payload)
359360

360361
defer { self.isAutoReconnecting = false }
361362
do {

‎TablePro/ContentView.swift‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,14 @@ struct ContentView: View {
4949
}
5050
_windowTitle = State(initialValue: defaultTitle)
5151

52-
// For Cmd+T (new tab), the session already exists. Resolve synchronously
53-
// to avoid the "Connecting..." flash while waiting for async onChange.
52+
// Resolve session synchronously to avoid "Connecting..." flash.
53+
// For payload with connectionId: look up that specific session.
54+
// For nil payload (native tab bar "+"): fall back to current session.
5455
var resolvedSession: ConnectionSession?
5556
if let connectionId = payload?.connectionId {
5657
resolvedSession = DatabaseManager.shared.activeSessions[connectionId]
58+
} else if let currentId = DatabaseManager.shared.currentSessionId {
59+
resolvedSession = DatabaseManager.shared.activeSessions[currentId]
5760
}
5861
_currentSession = State(initialValue: resolvedSession)
5962

‎TablePro/Core/Services/Infrastructure/SessionStateFactory.swift‎

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -48,54 +48,57 @@ enum SessionStateFactory {
4848
toolbarSt.databaseName = String(dbIndex)
4949
}
5050

51-
// Initialize single tab based on payload.
52-
// For isConnectionOnly (Cmd+T new tab), create a default query tab eagerly
53-
// so MainContentView doesn't flash "No tabs open" before initializeAndRestoreTabs runs.
54-
if let payload, !payload.isConnectionOnly {
55-
switch payload.tabType {
56-
case .table:
57-
if let tableName = payload.tableName {
58-
if payload.isPreview {
59-
tabMgr.addPreviewTableTab(
60-
tableName: tableName,
61-
databaseType: connection.type,
62-
databaseName: payload.databaseName ?? connection.database
63-
)
64-
} else {
65-
tabMgr.addTableTab(
66-
tableName: tableName,
67-
databaseType: connection.type,
68-
databaseName: payload.databaseName ?? connection.database
69-
)
70-
}
71-
if let index = tabMgr.selectedTabIndex {
72-
tabMgr.tabs[index].isView = payload.isView
73-
tabMgr.tabs[index].isEditable = !payload.isView
74-
tabMgr.tabs[index].schemaName = payload.schemaName
75-
if payload.showStructure {
76-
tabMgr.tabs[index].showStructure = true
51+
if let payload {
52+
switch payload.intent {
53+
case .openContent:
54+
switch payload.tabType {
55+
case .table:
56+
toolbarSt.isTableTab = true
57+
if let tableName = payload.tableName {
58+
if payload.isPreview {
59+
tabMgr.addPreviewTableTab(
60+
tableName: tableName,
61+
databaseType: connection.type,
62+
databaseName: payload.databaseName ?? connection.database
63+
)
64+
} else {
65+
tabMgr.addTableTab(
66+
tableName: tableName,
67+
databaseType: connection.type,
68+
databaseName: payload.databaseName ?? connection.database
69+
)
7770
}
78-
if let initialFilter = payload.initialFilterState {
79-
tabMgr.tabs[index].filterState = initialFilter
80-
filterMgr.restoreFromTabState(initialFilter)
71+
if let index = tabMgr.selectedTabIndex {
72+
tabMgr.tabs[index].isView = payload.isView
73+
tabMgr.tabs[index].isEditable = !payload.isView
74+
tabMgr.tabs[index].schemaName = payload.schemaName
75+
if payload.showStructure {
76+
tabMgr.tabs[index].showStructure = true
77+
}
78+
if let initialFilter = payload.initialFilterState {
79+
tabMgr.tabs[index].filterState = initialFilter
80+
filterMgr.restoreFromTabState(initialFilter)
81+
}
8182
}
83+
} else {
84+
tabMgr.addTab(databaseName: payload.databaseName ?? connection.database)
8285
}
83-
} else {
84-
tabMgr.addTab(databaseName: payload.databaseName ?? connection.database)
86+
case .query:
87+
tabMgr.addTab(
88+
initialQuery: payload.initialQuery,
89+
databaseName: payload.databaseName ?? connection.database,
90+
sourceFileURL: payload.sourceFileURL
91+
)
92+
case .createTable:
93+
tabMgr.addCreateTableTab(
94+
databaseName: payload.databaseName ?? connection.database
95+
)
8596
}
86-
case .query:
87-
tabMgr.addTab(
88-
initialQuery: payload.initialQuery,
89-
databaseName: payload.databaseName ?? connection.database,
90-
sourceFileURL: payload.sourceFileURL
91-
)
92-
case .createTable:
93-
tabMgr.addCreateTableTab(
94-
databaseName: payload.databaseName ?? connection.database
95-
)
97+
case .newEmptyTab:
98+
tabMgr.addTab(databaseName: payload.databaseName ?? connection.database)
99+
case .restoreOrDefault:
100+
break
96101
}
97-
} else if payload?.isNewTab == true {
98-
tabMgr.addTab(databaseName: payload?.databaseName ?? connection.database)
99102
}
100103

101104
let coord = MainContentCoordinator(

‎TablePro/Core/Services/Infrastructure/WindowLifecycleMonitor.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ internal final class WindowLifecycleMonitor {
119119
return entries[windowId]?.connectionId
120120
}
121121

122+
/// Returns the connectionId associated with the given NSWindow, if registered.
123+
internal func connectionId(fromWindow window: NSWindow) -> UUID? {
124+
purgeStaleEntries()
125+
return entries.values.first(where: { $0.window === window })?.connectionId
126+
}
127+
122128
/// Check if any windows are registered for a connection.
123129
internal func hasWindows(for connectionId: UUID) -> Bool {
124130
purgeStaleEntries()

‎TablePro/Core/Services/Infrastructure/WindowOpener.swift‎

Lines changed: 33 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// TablePro
44
//
55
// Bridges SwiftUI's openWindow environment action to imperative code.
6-
// Stored by ContentView on appear so MainContentCommandActions can open native tabs.
6+
// Stored on appear by ContentView, WelcomeViewModel, or ConnectionFormView.
77
//
88

99
import os
@@ -15,55 +15,49 @@ internal final class WindowOpener {
1515

1616
internal static let shared = WindowOpener()
1717

18-
/// Set by ContentView when it appears. Safe to store — OpenWindowAction is app-scoped, not view-scoped.
18+
/// Set on appear by ContentView, WelcomeViewModel, or ConnectionFormView.
19+
/// Safe to store — OpenWindowAction is app-scoped, not view-scoped.
1920
internal var openWindow: OpenWindowAction?
2021

21-
/// The connectionId for the next window about to be opened.
22-
/// Set by `openNativeTab` before calling `openWindow`, consumed by
23-
/// `AppDelegate.windowDidBecomeKey` to set the correct `tabbingIdentifier`.
24-
internal var pendingConnectionId: UUID?
22+
/// Ordered queue of pending payloads — windows requested via openNativeTab
23+
/// but not yet acknowledged by MainContentView.configureWindow.
24+
/// Ordered so consumeOldestPendingConnectionId returns the correct entry
25+
/// when multiple windows open in quick succession (e.g., tab restore).
26+
internal private(set) var pendingPayloads: [(id: UUID, connectionId: UUID)] = []
27+
28+
/// Whether any payloads are pending — used for orphan detection in windowDidBecomeKey.
29+
internal var hasPendingPayloads: Bool { !pendingPayloads.isEmpty }
2530

2631
/// Opens a new native window tab with the given payload.
27-
/// Stores the connectionId so AppDelegate can set the correct tabbingIdentifier.
32+
/// Falls back to .openMainWindow notification if openWindow is not yet available
33+
/// (cold launch from Dock menu before any SwiftUI view has appeared).
2834
internal func openNativeTab(_ payload: EditorTabPayload) {
29-
pendingConnectionId = payload.connectionId
30-
guard let openWindow else {
31-
Self.logger.warning("openNativeTab called before openWindow was set — payload dropped")
32-
return
35+
pendingPayloads.append((id: payload.id, connectionId: payload.connectionId))
36+
if let openWindow {
37+
openWindow(id: "main", value: payload)
38+
} else {
39+
Self.logger.info("openWindow not set — falling back to .openMainWindow notification")
40+
NotificationCenter.default.post(name: .openMainWindow, object: payload)
3341
}
34-
openWindow(id: "main", value: payload)
3542
}
3643

37-
/// Returns and clears the pending connectionId (consume-once pattern).
38-
internal func consumePendingConnectionId() -> UUID? {
39-
defer { pendingConnectionId = nil }
40-
return pendingConnectionId
44+
/// Called by MainContentView.configureWindow after the window is fully set up.
45+
internal func acknowledgePayload(_ id: UUID) {
46+
pendingPayloads.removeAll { $0.id == id }
47+
}
48+
49+
/// Consumes and returns the connectionId for the oldest pending payload.
50+
/// Removes the entry so subsequent calls return the next payload in order.
51+
internal func consumeOldestPendingConnectionId() -> UUID? {
52+
guard !pendingPayloads.isEmpty else { return nil }
53+
return pendingPayloads.removeFirst().connectionId
4154
}
42-
}
4355

44-
/// Pure logic for resolving the tabbingIdentifier for a new main window.
45-
/// Extracted for testability — no AppKit dependencies.
46-
internal enum TabbingIdentifierResolver {
47-
/// Resolve the tabbingIdentifier for a new main window.
48-
/// - Parameters:
49-
/// - pendingConnectionId: The connectionId from WindowOpener (if a tab was just opened)
50-
/// - existingIdentifier: The tabbingIdentifier from an existing visible main window (if any)
51-
/// - groupAllConnections: When true, all windows share one tab group regardless of connection
52-
/// - Returns: The tabbingIdentifier to assign to the new window
53-
internal static func resolve(
54-
pendingConnectionId: UUID?,
55-
existingIdentifier: String?,
56-
groupAllConnections: Bool = false
57-
) -> String {
58-
if groupAllConnections {
56+
/// Returns the tabbingIdentifier for a connection.
57+
internal static func tabbingIdentifier(for connectionId: UUID) -> String {
58+
if AppSettingsManager.shared.tabs.groupAllConnectionTabs {
5959
return "com.TablePro.main"
6060
}
61-
if let connectionId = pendingConnectionId {
62-
return "com.TablePro.main.\(connectionId.uuidString)"
63-
}
64-
if let existing = existingIdentifier {
65-
return existing
66-
}
67-
return "com.TablePro.main"
61+
return "com.TablePro.main.\(connectionId.uuidString)"
6862
}
6963
}

0 commit comments

Comments
 (0)