fix(coordinator): activate the existing tab instead of opening a duplicate (#1613)#1623
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a93e0822
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if activateGridFocus { | ||
| requestGridFocus() |
There was a problem hiding this comment.
When activateGridFocus is true and the match is a non-selected tab in the same window, this calls requestGridFocus() immediately after changing selectedTabId. At that point SwiftUI has not rebuilt the content for the new tab yet, so dataTabDelegate can still point at the outgoing table; if its focusGrid() succeeds, pendingGridFocusOnOpen is cleared and the newly selected table never consumes it in KeyHandlingTableView.viewDidMoveToWindow(). Sidebar/quick-switcher opens of an already-open tab can therefore leave keyboard focus on the old grid instead of the activated table.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41ab3b5a80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let match = tabManager.tabs.first(where: matches) { | ||
| if tabManager.selectedTabId != match.id { | ||
| tabManager.selectedTabId = match.id | ||
| } | ||
| applyStructureMode(showStructure, toTab: match.id, in: tabManager) | ||
| if activateGridFocus { | ||
| requestGridFocus() | ||
| } | ||
| return true |
There was a problem hiding this comment.
When forceNonPreview is used by the sidebar double-click path, this new early activation path still returns for any matching tab without checking whether the match is a preview tab or promoting it. If the table is already open in an inactive preview tab, double-clicking it selects that tab but leaves isPreview == true, so it can later be replaced by another single-click and won't be persisted, contrary to the existing force-non-preview behavior.
Useful? React with 👍 / 👎.
1 participant
Fixes #1613.
Problem
With Table 1 open in one tab and Table 2 in another, clicking Table 1 in the sidebar again opened a second, duplicate Table 1 tab instead of switching to the one already open.
Repro: click Table 1, click Table 2, click Table 1 again -> duplicate Table 1 tab.
Root cause
Each native window tab is its own
NSWindow+MainContentCoordinator+tabManager.openTableTabalready had a cross-window "focus the existing tab" scan, but it was gated behind aredirectToSiblingparameter that only the quick switcher set. Sidebar clicks left it off, so a click on an already-open table fell through toWindowManager.openTaband created a duplicate. The pre-check also only looked at the selected tab, missing a match in a non-selected tab of the same window.Fix
Consolidated the duplicated "focus existing table tab" logic into one path that always runs, modeled on
NSDocumentController.openDocument(if the item is already open, bring its window forward instead of opening a new one):activateIfAlreadyOpen(...)runs first inopenTableTab. It scans the current window first (full tab list, not just the selected tab), then sibling windows for the same connection, matching on(table, database, schema). On a match it selects that tab and brings its window forward, then stops. Priority is now: activate existing -> reuse preview tab -> new tab, matching VS Code, Xcode, and DataGrip.selectTabAndFocusWindow(_:)is the shared "select tab + makeKeyAndOrderFront" primitive.TabRouter.focusExistingTableTab(deep links) now uses it too, removing a second copy of the same logic.redirectToSiblingparameter; the check is always on.This also fixes a latent bug the consolidation exposed: the old gated path focused a sibling window but never set its
selectedTabId, so a multi-tab sibling could surface the wrong tab. The shared primitive does both.Tests
openTableTabactivates a sibling window's existing tab instead of creating a new one, and does not dedupe across different connections.activateIfAlreadyOpenreturns false on no match (falls through to creation) and applies structure mode when switching to an existing tab.No docs change: behavior fix, no new shortcut, setting, or UI.