Skip to content

fix(datagrid): open a foreign key reference in its own tab instead of taking over the one it was followed from - #2800

Merged
datlechin merged 1 commit into
mainfrom
fix/fk-arrow-opens-new-tab
Sep 12, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/fk-arrow-opens-new-tab

Conversation

@datlechin

Copy link
Copy Markdown
Member

Clicking the arrow in a foreign key cell retargeted the tab the reader was on. The rows they were reading, the filters they had applied and the sort they had set all went with it.

Fixes #1421.

Root cause

Foreign key navigation owned a second, weaker answer to "may I take over the selected tab".

Every other way of opening a table asks isActiveTabReusable (MainContentCoordinator+Navigation.swift:394), which demands the tab be a preview, an empty query or an untouched Create Table tab, and refuses one carrying applied filters, a user sort or pinned results. navigateToFKReference asked selectedTabHoldsProtectedContent instead, which only knows about unsaved cell edits, query work and staged structure edits. A table tab the reader was browsing answers "nothing here", so the arrow retargeted it.

The openInNewTab: Bool carrying the caller's intent is the other half. It cannot say "reveal the tab that already shows this", which is why a second predicate was invented rather than the first being shared. Five call sites each passed a literal false whose meaning was "take over the tab I am looking at".

The fix

Following a reference never takes the selected tab over. A reference can only be followed from a grid, so the tab it is followed from is always one the reader is in.

  • ReferenceOpenIntent (.follow / .newTab) replaces the Bool across the delegate protocols, the arrow, the context menu, the preview popover, the row inspector and the JSON inspector.
  • ReferenceNavigationPlanner is a pure resolver for the three outcomes, testable without a window, a coordinator or a tab manager: re-filter the selected tab when it is already on the referenced table, reveal a tab already showing that reference, otherwise open a tab of its own.
  • Reveal now runs before opening, matching the order openTableTab takes with activateIfAlreadyOpen.
  • The reuse branch and replaceSelectedTabWithFKTarget are gone. With the rule above they were unreachable: every entry point needs a result grid, and a query tab that has one is already protected by holdsQueryWork.
  • A jump that leaves the tab promotes it out of preview first, so the next sidebar click cannot retarget the table the reader navigated away from.

Modifiers keep the Apple convention rather than inverting: plain click follows, Cmd-click always takes a new tab. Safari, Finder and Xcode all read that way. The reporter's ask is met by the disposition, not by the modifier.

Two pre-existing defects this would otherwise have made routine

Both live in the reveal path, which used to run only when the source tab was protected and now runs on every follow.

A tab was matched on its filter panel, not on its rows. showsOnlyFKPredicate read appliedFilters, which resolves from the editable draft: updateFilter writes each keystroke straight into filterState.filters while commit stays set from the last Apply. Fetch id = 7, type 42 into the filter row without applying, follow a reference to 42 from another tab, and that tab claimed the reference. Revealing runs no query, so the reader landed on the rows for 7 while the app reported it had found the row. TabFilterState.executedFilters now records what the rows were actually fetched with, written wherever a query is built from filters, and the match reads that. It is deliberately not persisted: a restored tab rebuilds its query on first load, which fills it.

A reveal could raise a window without showing the connection. One window hosts several connections, and selectTabAndFocusWindow only set selectedTabId and ordered the window front, so a tab torn into a window since switched to another connection came forward invisible. The sibling search also went through allActiveCoordinators(), a registry of every coordinator SwiftUI has built, which can hold one whose window is gone. Both now go through WindowManager, which is the record of what is on screen, and a reveal that cannot be made visible falls back to opening the reference instead of leaving the click doing nothing.

Two residuals, both narrower than what they replace

executedFilters is written where the query is built, not where its rows land, so pressing Apply on a tab that already has a query in flight records a filter whose result was never installed. The write belongs at the point a result is installed, and there is no single chokepoint for that today: setActiveTableRows also takes the empty buffer a retarget installs. The marker still closes the common case, an edited filter row that was never applied, and this one needs a query that is already running.

A tab created for a reference whose first load has not finished has no marker yet, so following the same reference again opens a second tab rather than revealing the first. The failure is a duplicate tab, not wrong rows.

Both want the same thing: filters recorded against the result that installed them. That is its own change in the execution path and is not attempted here.

Tested

  • ReferenceNavigationPlannerTests: every intent against every state.
  • FKNavigationTests: the source tab survives with its identity; a preview source is kept on both jumps that leave it; reveal beats opening a duplicate; an unapplied filter edit does not count as showing the reference; an unreachable reveal opens the reference; Cmd-click always takes its own tab.
  • 122 cases across FKNavigationTests, ReferenceNavigationPlannerTests, FilterTypingBeforeFirstLoadTests, MainContentCoordinatorTabSwitchTests, OpenTableTabTests and ResultPinningTests, all passing. Debug build clean, swiftlint --strict clean over TablePro, TableProTests and TableProUITests, and both docs/ checks clean.

ForeignKeyNavigationUITests drives the flow through Preview FK Reference and its Open Artist button, then asserts the strip gained a tab and the Album tab survived. It did not run here: every TableProUITests suite on this machine reports Timed out while enabling automation mode, which is an unanswered macOS UI-automation prompt rather than a test failure. It needs a run on CI or on a machine where that prompt has been answered.

No before-and-after screenshots: the change is which tab the reference lands in, which a still frame of either state does not show.

https://claude.ai/code/session_01R4D7UrumqUb2UBpPSbQHMb

@mintlify

mintlify Bot commented Sep 12, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 12, 2026, 11:52 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@datlechin
datlechin force-pushed the fix/fk-arrow-opens-new-tab branch from 1e8df61 to ac07f19 Compare September 12, 2026 11:51
@datlechin
datlechin merged commit 4ef14e4 into main Sep 12, 2026
9 checks passed
@datlechin
datlechin deleted the fix/fk-arrow-opens-new-tab branch September 12, 2026 11:53

This branch was successfully deployed

1 active deployment
staging - docs ac07f19a Deployed Sep 12, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Open foreign keys reference in new tab

1 participant