refactor: replace native window tabs with in-app tab bar#763
Merged
refactor: replace native window tabs with in-app tab bar#763
Conversation
…switching Native macOS window tabs used addTabbedWindow() which took 600-900ms per call, causing severe lag on Cmd+T and tab restoration. This refactor moves to a single-window-per-connection architecture with a custom SwiftUI tab bar, eliminating the N² view lifecycle cascades during tab restore.
- Fix closeInAppTab save button no-op (data loss) - Add unsaved changes check to Close Others / Close All - Fix switchSchema not persisting tabs before clearing - Fix double-tap rename gesture conflict (exclusively before) - Fix Vim :q closing entire window instead of current tab - Show dirty indicator for pending database changes - Fix draggedTabId not cleared on cancelled drag - Fix rename TextField stuck on focus loss - Register SQL files for duplicate detection - Protect preview tabs with pending changes from replacement - Fix promotePreviewTab not clearing WindowLifecycleMonitor flag - Query hasPreview from tabManager instead of stale window monitor - Remove dead code (aggregatedTabs, closeSiblingNativeWindows) - Update 15 stale "native window tab" comments across 9 files - Add debug logging for tab navigation flow
- Remove all 45 [PERF] log statements and timing infrastructure - Remove unused OSSignposter (windowPerfLog) and 5 PERF-only loggers - Fix tabbingMode = .preferred → .disallowed in windowDidBecomeKey - Remove cross-window previewWindow() lookup in sidebar double-click - Replace WindowLifecycleMonitor.setPreview() with direct subtitle update - Remove dead isFirstCoordinatorForConnection() multi-window code - Simplify ConnectionSwitcherPopover tabbingMode manipulation
SessionStateFactory.create() was called eagerly in ContentView.init, which SwiftUI invokes speculatively during body evaluation. Each call allocated 7 heavy objects (QueryTabManager, MainContentCoordinator, etc.) that were immediately discarded, causing "QueryTabManager deallocated" spam and wasted resources. Consolidated 3 duplicate creation sites into a single ensureSessionState() method, called only from reactive handlers (onChange, onReceive) after the view is committed to the hierarchy.
…ssion SwiftUI fires onDisappear transiently when the view hierarchy is reconstructed (e.g., sessionState changing from nil to a value causes if-let branches to rebuild). Without a grace period, the immediate coordinator.teardown() + disconnectSession killed the SSH tunnel while the connection was still being established, causing auto-reconnect to fail repeatedly. Fix: restore 200ms grace period with window re-registration check, and remove disconnectSession from onDisappear entirely — WindowLifecycleMonitor .handleWindowClose already handles disconnect on actual NSWindow close.
…CloseNotification SwiftUI's onDisappear fires transiently during view hierarchy reconstruction (e.g., sessionState nil→value causes if-let branches to rebuild). Using it for coordinator teardown caused race conditions where SSH tunnels were killed during auto-reconnect. Apple's recommended pattern for macOS: use NSWindow.willCloseNotification for deterministic resource cleanup, not SwiftUI view lifecycle callbacks. Changes: - WindowLifecycleMonitor: add onWindowClose closure to Entry, called in handleWindowClose before disconnect — deterministic teardown - MainContentView+Setup: pass coordinator/rightPanelState teardown closure to WindowLifecycleMonitor.register() - MainContentView: remove onDisappear teardown (grace period hack gone) - MainContentCoordinator: remove markTeardownScheduled/clearTeardownScheduled and _teardownScheduled lock (no longer needed) - Remove all [RESTORE] and [TAB-NAV] debug logging
Apple's documentation: "save data progressively and not rely solely on user actions to save important information." applicationWillTerminate does not fire on SIGKILL (Xcode Cmd+R, Force Quit, memory pressure). Now saves active connection IDs to UserDefaults immediately on connect and disconnect, so auto-reconnect works correctly after any termination. The applicationWillTerminate save is kept as a belt-and-suspenders fallback.
Reopen Closed Tab (Cmd+Shift+T): - Closed tabs stored in per-window history stack (capped at 20) - Reopened tabs get fresh RowBuffer, data re-fetched on demand MRU Tab Selection: - Track tab activation order in QueryTabManager - On tab close, select the most recently active tab (not adjacent) - Matches browser behavior (Chrome, Safari) Pinned Tabs: - Right-click → Pin/Unpin Tab - Pinned tabs show pin icon, no close button - Always at left side of tab bar, separated by divider - Survive Close Others and Close All - Persisted across sessions via isPinned in PersistedTab
Tab switch handler used await Task.yield() to debounce rapid clicks, but this deferred execution until after SwiftUI's body re-evaluation (~100-200ms). The actual handleTabChange work is only 2ms. Switched to synchronous onChange handler — tab switches are now instant.
Row data was evicted on every tab switch (when >2 tabs), then re-fetched when switching back — causing visible delays while waiting for the query. Other DB clients (Beekeeper, DataGrip, TablePlus) keep tab data in memory and only evict under memory pressure. Eviction now only happens: - When the window loses focus (didResignKeyNotification, 5s delay) - Under system memory pressure (MemoryPressureAdvisor) Also removed Task.yield() from tab switch handler — the actual work is 2ms, no debounce needed.
…itch Row data was evicted 5s after window resigned key (didResignKeyNotification), then re-fetched when switching back to the tab — causing visible delays. Other DB clients (Beekeeper, DataGrip, TablePlus) keep all tab data in memory until explicit close. Eviction now only happens under system memory pressure via MemoryPressureAdvisor — not on window resign or tab switch. Also removed [TAB-DBG] diagnostic logging.
cacheRowProvider() always called makeRowProvider() → applyDisplayFormats() even when the cached entry was still valid. This caused synchronous UserDefaults I/O (5-50ms) and format detection (1-5ms) on every tab switch, multiplied by 3 redundant onChange handlers. Now checks cache validity first — if resultVersion, metadataVersion, and sortState match, skips the expensive makeRowProvider entirely.
SwiftUI's conditional rendering (if let tab { tabContent(for: tab) })
destroyed and recreated the entire DataGridView (NSTableView) and
SourceEditor (TreeSitterClient) on every tab switch — ~200ms cost.
Replaced with ZStack + ForEach + opacity pattern: all tab views stay
alive in the hierarchy, only the active tab is visible. Matches Apple's
NSTabViewController behavior where child view controllers are kept alive
and only swapped in/out of the visible hierarchy.
Tab switch is now instant — no view destruction/recreation, no NSTableView
column rebuild, no TreeSitter language parser reinitialization.
Two fixes for tab switch performance with ZStack keep-alive: 1. Removed unconditional changeManager.reloadVersion += 1 on tab switch. With ZStack, each tab's DataGridView already has its data — the forced reload caused a redundant 200ms+ NSTableView.reloadData(). 2. Added isHandlingTabSwitch guard to handleTabsChange. Saving outgoing tab state mutates tabManager.tabs, which triggered handleTabsChange → persistence.saveNow → more cascading body re-evaluations. Tab switch reduced from ~680ms (9 body re-evals) to ~90ms (5 re-evals).
handleTabChange was mutating 5 @observable objects synchronously (filterStateManager, columnVisibilityManager, toolbarState, changeManager, tabManager.tabs), each triggering a separate SwiftUI body re-evaluation. With ZStack keep-alive, all tab views (active + hidden) re-evaluated on each mutation — 5 cascading passes blocking the visual switch. Split into two phases: - Phase 1 (sync, ~1ms): selectedRowIndices + toolbarState.isTableTab only. SwiftUI flips opacity immediately — user sees instant switch. - Phase 2 (deferred, next frame): save outgoing state + restore shared managers. Invisible to user — 16ms later, managers catch up. Also batch outgoing tab state save into single array write (1 didSet) instead of 2 separate element mutations (2 didSet calls).
The deferred Phase 2 tasks were queuing up — each keypress created a new Task that executed in order even after the user stopped pressing. Now cancels the previous tabSwitchTask before creating a new one, so only the final tab switch commits its state restoration.
The deferred Phase 2 was still restoring 5 @observable managers (filterStateManager, columnVisibilityManager, changeManager, etc.) causing 16 body re-evaluations over ~960ms after the user stops pressing Cmd+1/Cmd+2. With ZStack keep-alive, each tab's view maintains its own correct state — shared manager reconfiguration is unnecessary. Phase 2 now only saves outgoing tab state (for persistence) and checks for lazy query needs. No @observable mutations on the incoming tab.
… switch Three fixes for tab switch responsiveness: 1. Move updateWindowTitleAndFileState, syncSidebarToCurrentTab, and persistence.saveNow from synchronous handleTabSelectionChange to the deferred Phase 2 task via onTabSwitchSettled callback. These were triggering onChange(selectedTables) → handleTableSelectionChange → another full body eval chain per switch. 2. Guard .task(id: currentTab?.tableName) with isHandlingTabSwitch — during rapid Cmd+1/2/3 switching, 30+ metadata tasks queued up and all executed when the user stopped, causing ~1 second of trailing body re-evaluations. 3. handleTabSelectionChange now only calls handleTabChange (Phase 1) — all other work deferred to Phase 2 which cancels on next switch.
…switch .task(id: currentTab?.tableName) created a new SwiftUI-managed task for every tab switch. During rapid Cmd+1/2/3 spam, 28+ tasks queued up and all executed when the user stopped — each triggering loadTableMetadata. Moved metadata loading to Phase 2's onTabSwitchSettled callback, which is cancellable and only runs for the final settled tab.
Removed ALL synchronous @observable mutations from onChange(selectedTabId). The ZStack opacity flip is driven by selectedTabId binding alone — no handleTabChange Phase 1 needed. MRU tracking (lightweight array append) stays synchronous. Everything else (toolbarState, selectedRowIndices, outgoing save, title, sidebar, persist, metadata) is deferred to Phase 2 Task which coalesces rapid Cmd+1/2/3 spam via tabSwitchTask cancellation. During rapid keyboard repeat, the main thread only processes the onChange callback (~0ms) + SwiftUI body eval for opacity change. No @observable mutations means no cascading body re-evaluations.
Each tab switch triggered 6+ onChange handlers (resultColumns, inspectorTrigger, pendingChangeTrigger, EC.resultVersion, EC.metadataVersion, EC.activeResultSetId) which cascaded into 4+ extra body re-evaluations per switch cycle. Now all onChange handlers check isHandlingTabSwitch and return early during tab switching. Also onTabSwitchSettled only runs if the settled tab is still the currently selected tab — prevents stale sidebar sync from triggering onChange(selectedTables) body eval cascade. Target: reduce per-switch cycle from ~280ms to ~80ms (body eval + AppKit opacity layout only, no onChange cascades).
selectTab/selectPreviousTab/selectNextTab now check isHandlingTabSwitch and return early if a switch is still being processed. This prevents macOS keyboard repeat events (30ms interval) from queuing 20+ tab switches that continue executing after the user releases the keys. The isHandlingTabSwitch flag is set synchronously in scheduleTabSwitch and cleared in the deferred Phase 2 Task, providing a natural throttle window of ~100ms per switch.
The isHandlingTabSwitch throttle blocked ALL keyboard events during Phase 2 (~500ms), making the app feel unresponsive. Replaced with: 1. selectTab: skip only if already on target tab (Cmd+1 repeat → skip) 2. selectPreviousTab/selectNextTab: no throttle (always responsive) 3. isHandlingTabSwitch now synchronous-only (defer in scheduleTabSwitch) — true only during the onChange handler, not during Phase 2 Task
…t switching Replace SwiftUI ZStack+ForEach+opacity pattern with NSViewRepresentable (TabContentContainerView) that manages one NSHostingView per tab. Tab switching toggles isHidden instead of SwiftUI opacity, eliminating body re-evaluation cascade for all inactive tabs. Key changes: - TabContentContainerView: NSViewRepresentable with per-tab NSHostingView - Tab click via .onTapGesture (removed NSView overlay that blocked close/drag) - Cmd+W intercepted via NSEvent monitor (not window.delegate overwrite) - Phase 1 synchronous outgoing save (no state loss during rapid switching) - Shared manager restore guarded to skip unchanged values - Version-gated rootView rebuild (contentVersion includes error/executing) - Rename moved to context menu, deprecated onCommit replaced with onSubmit - teardown resumes saveCompletionContinuation to prevent Task leak - Dead code removed (evictInactiveTabs, handleTabSelectionChange, DBG logs)
- Guard background tab close against pendingChanges (data loss) - Add changeManager.hasChanges to quit-time unsaved check - Guard saveCompletionContinuation against concurrent access - Clear pendingChanges on reopened closed tab - Set isEditable on preview tab creation path - Show dirty indicator for active tab via isActiveTabDirty prop - Add persistTabs() helper to consistently exclude preview tabs
- Move closeTabsToRight to coordinator with aggregated unsaved-changes check - Enforce pinned/unpinned boundary in drag-and-drop reorder - Commit rename text on focus loss instead of discarding - Add reopenClosedTab to ShortcutAction for remappable Cmd+Shift+T - Add accessibility labels to tab bar items and close button
contentVersion used simple addition (resultVersion + metadataVersion + isExecuting?2:0), causing collisions like 0+0+2 == 1+1+0. The TabContentContainerView saw identical versions and skipped rebuilding the rootView, leaving the DataGrid showing only the "#" column. Fix: use prime multipliers (97, 31, 13) to avoid collisions. Also: - Defer query from addTableTabInApp to tab switch Phase 2 - Guard lazy query launch against stale selectedTabId - Add debug logging across query/apply/container pipeline
When a connection is already open, deeplinks (tablepro://), Handoff, and database URL schemes (mysql://, postgres://) now add content as in-app tabs via the existing coordinator instead of creating duplicate NSWindows. Falls back to openNativeTab only when no coordinator exists. Add routeToExistingWindow helper that dispatches to openTableTab or tabManager.addTab based on payload type, then brings the window to front.
- Suppress auto-reconnect when app is launched by URL/deeplink - Defer auto-reconnect to next run loop so URL handler can set flag - Add connectingURLConnectionIds guard to connectViaDeeplink - Check pendingPayloads for race with auto-reconnect window creation - Fix closeRestoredMainWindows killing deeplink window
- Remove ~35 [DEEPLINK], [TAB-NAV], [TAB-SWITCH], [QUERY], [APPLY], [CONTAINER] debug log statements added during tab bar refactor - Extract closeAllWelcomeWindows() helper (replaces 8 inline patterns) - Merge attemptAutoReconnectAll + attemptAutoReconnect into single method - Fix closeRestoredMainWindows: remove unnecessary DispatchQueue.main.async - Clean connectViaDeeplink guard chain (remove redundant variables) - Add timing comment to suppressAutoReconnect mechanism Net reduction: -178 lines
- Add Cmd+Shift+T (Reopen Closed Tab) to keyboard shortcuts - Add Close Tabs to Right, Close All, Reopen Closed Tab to tabs docs - Document MRU tab selection, reopening history (20 tabs), remappable shortcut - Expand pinned tabs: left position, divider, drag boundary enforcement - Add Rename and full context menu items - Note preview tab promotion on data changes - Update deeplinks: opens as in-app tab in existing window
The TabContentContainerView only rebuilt rootView when contentVersion changed, but several shared manager states that affect rendering were not included — toggling them had no visual effect. - Add filterStateManager.hasAppliedFilters to activeTabContentVersion - Add columnVisibilityManager.hiddenColumns hash to version - Add safeModeLevel.blocksAllWrites to version - Add showRowNumbers setting to version - Fix builtVersions init to use activeTabContentVersion - Replace NSApp.keyWindow with contentWindow in 5 confirmation dialogs - Set window subtitle for preview tabs created via "no reusable tab" path - Clear selectedRowIndices on tab switch to prevent stale selection - Remove dead didBecomeKey handler in ContentView
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
addTabbedWindow, 600-900ms per call) with a custom SwiftUIEditorTabBarfor instant tab switchingopenTableTab,newTab, FK navigation, favorites, etc.) now adds in-app tabs instead of creating newNSWindowinstancesKey changes
New components:
EditorTabBar/EditorTabBarItem— custom tab bar with drag-to-reorder, close buttons, context menus, preview tab styling, dirty indicatorsMainContentCoordinator+TabOperations— tab lifecycle operations (close, reorder, rename, duplicate)Redirected navigation (13 call sites):
MainContentCoordinator+Navigation— 5 sites (active work guard, default open, preview paths)MainContentCoordinator+SidebarActions— 3 sites (createNewTable, createView, editView)MainContentCoordinator+FKNavigation— 1 siteMainContentCoordinator+Favorites— 1 siteMainContentCoordinator— 2 sites (loadQueryIntoEditor, insertQueryFromAI)MainContentCommandActions— 1 site (openFile)Simplified infrastructure:
onDisappear200ms grace period (no more tab group merge cascades)Cmd+Tadds in-app tab via coordinator instead of creating native windowCmd+Wcloses in-app tab when >1 tab, closes window on last tabCmd+1-9,Cmd+Shift+[/]switch in-app tabswindow.tabbingMode = .disallowed(native window tabs only for connection grouping)Test plan