Skip to content

Commit 1e8df61

Browse files
committed
fix(datagrid): open a foreign key reference in its own tab instead of taking over the one it was followed from
Claude-Session: https://claude.ai/code/session_01R4D7UrumqUb2UBpPSbQHMb
1 parent d0c7b2d commit 1e8df61

19 files changed

Lines changed: 710 additions & 292 deletions

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5151

5252
### Fixed
5353

54+
- Foreign key arrow replacing the tab you were reading. (#1421)
55+
- Applied filter left in the panel after cancelling the unsaved-changes alert on a foreign key jump.
5456
- Select All painting the whole column header row as selected, and leaving a cell cursor on the first cell.
5557
- Column header shown as selected after a cell drag reached the first and last row of the page.
5658
- No outline around a swept cell block whose rows reached both ends of the page.

TablePro/Core/Coordinators/FilterCoordinator.swift

Lines changed: 61 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,35 +26,63 @@ final class FilterCoordinator {
2626
let capturedLogicMode = logicMode
2727
parent.confirmDiscardChangesIfNeeded(action: .filter) { [weak self] confirmed in
2828
guard let self, confirmed else { return }
29-
guard capturedTabIndex < parent.tabManager.tabs.count else { return }
29+
commitFilters(
30+
capturedFilters,
31+
logicMode: capturedLogicMode,
32+
tabIndex: capturedTabIndex,
33+
tableName: capturedTableName
34+
)
35+
}
36+
}
3037

31-
if let capturedLogicMode {
32-
parent.tabManager.mutate(at: capturedTabIndex) {
33-
$0.filterState.filterLogicMode = capturedLogicMode
34-
$0.filterState.isVisible = true
35-
}
38+
/// Writes the one predicate a reference jump carries and re-queries for it.
39+
///
40+
/// The caller has already taken the discard guard, because it also records the view the tab is
41+
/// leaving and both have to land on the same side of a refusal.
42+
func commitReferenceFilter(_ filter: TableFilter) {
43+
guard let (tab, tabIndex) = parent.tabManager.selectedTabAndIndex,
44+
let tableName = tab.tableContext.tableName else { return }
45+
setFKFilter(filter)
46+
commitFilters([filter], logicMode: nil, tabIndex: tabIndex, tableName: tableName)
47+
}
48+
49+
private func commitFilters(
50+
_ filters: [TableFilter],
51+
logicMode: FilterLogicMode?,
52+
tabIndex: Int,
53+
tableName: String
54+
) {
55+
guard tabIndex < parent.tabManager.tabs.count else { return }
56+
57+
if let logicMode {
58+
parent.tabManager.mutate(at: tabIndex) {
59+
$0.filterState.filterLogicMode = logicMode
60+
$0.filterState.isVisible = true
3661
}
37-
parent.tabManager.mutate(at: capturedTabIndex) { $0.pagination.reset() }
62+
}
63+
parent.tabManager.mutate(at: tabIndex) { $0.pagination.reset() }
3864

39-
let tab = parent.tabManager.tabs[capturedTabIndex]
40-
let queryColumns = parent.queryColumns(for: tab)
41-
let newQuery = parent.queryBuilder.buildFilteredQuery(
42-
tableName: capturedTableName,
43-
schemaName: tab.tableContext.schemaName,
44-
filters: capturedFilters,
45-
logicMode: tab.filterState.filterLogicMode,
46-
sortState: tab.sortState,
47-
columns: queryColumns.columns,
48-
columnTypes: queryColumns.columnTypes,
49-
selectColumns: parent.selectColumns(for: tab),
50-
limit: tab.pagination.pageSize,
51-
offset: tab.pagination.currentOffset
52-
)
65+
let tab = parent.tabManager.tabs[tabIndex]
66+
let queryColumns = parent.queryColumns(for: tab)
67+
let newQuery = parent.queryBuilder.buildFilteredQuery(
68+
tableName: tableName,
69+
schemaName: tab.tableContext.schemaName,
70+
filters: filters,
71+
logicMode: tab.filterState.filterLogicMode,
72+
sortState: tab.sortState,
73+
columns: queryColumns.columns,
74+
columnTypes: queryColumns.columnTypes,
75+
selectColumns: parent.selectColumns(for: tab),
76+
limit: tab.pagination.pageSize,
77+
offset: tab.pagination.currentOffset
78+
)
5379

54-
parent.tabManager.mutate(at: capturedTabIndex) { $0.content.query = newQuery }
55-
saveLastFilters(for: capturedTableName)
56-
parent.runQuery()
80+
parent.tabManager.mutate(at: tabIndex) {
81+
$0.content.query = newQuery
82+
$0.filterState.executedFilters = filters
5783
}
84+
saveLastFilters(for: tableName)
85+
parent.runQuery()
5886
}
5987

6088
func clearFiltersAndReload() {
@@ -81,7 +109,10 @@ final class FilterCoordinator {
81109
offset: tab.pagination.currentOffset
82110
)
83111

84-
parent.tabManager.mutate(at: capturedTabIndex) { $0.content.query = newQuery }
112+
parent.tabManager.mutate(at: capturedTabIndex) {
113+
$0.content.query = newQuery
114+
$0.filterState.executedFilters = []
115+
}
85116
clearLastFilters(for: capturedTableName)
86117
parent.runQuery()
87118
}
@@ -216,7 +247,11 @@ final class FilterCoordinator {
216247
)
217248
}
218249

219-
parent.tabManager.mutate(at: tabIndex) { $0.content.query = newQuery }
250+
let executed = hasFilters ? tab.filterState.appliedFilters : []
251+
parent.tabManager.mutate(at: tabIndex) {
252+
$0.content.query = newQuery
253+
$0.filterState.executedFilters = executed
254+
}
220255
}
221256

222257
// MARK: - Filter State

TablePro/Models/Database/TableFilter.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,15 @@ struct TabFilterState: Equatable, Hashable, Codable {
261261
var keyPattern: String
262262
var keyTypeScope: String?
263263

264+
/// The filters the rows on screen were actually fetched with.
265+
///
266+
/// `filters` is the panel's editable draft, and `appliedFilters` resolves from it, so both
267+
/// describe a query that has not run the moment a reader types into a filter row without
268+
/// pressing Apply. Anything asking what a tab is *showing*, rather than what its panel says,
269+
/// has to read this. It is written wherever a query is built from the filters and is
270+
/// deliberately not persisted: a restored tab rebuilds its query on first load, which fills it.
271+
var executedFilters: [TableFilter] = []
272+
264273
init(isVisible: Bool = false) {
265274
self.filters = []
266275
self.commit = nil
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import Foundation
2+
3+
/// What the reader asked for when they followed a foreign key.
4+
///
5+
/// A Bool cannot say this. The decision has three destinations, not two, and `false` meaning
6+
/// "take over the tab I am looking at" read backwards at every call site.
7+
internal enum ReferenceOpenIntent: Equatable, Sendable {
8+
/// The ordinary gesture: a click on the cell's arrow, the row inspector's link, the preview
9+
/// popover's button, the menu's plain item.
10+
case follow
11+
/// Command-click, and the menu item that spells it out. Always its own tab, even when one is
12+
/// already open on the same reference.
13+
case newTab
14+
}
15+
16+
/// The one thing a reference jump does with the window's tabs.
17+
///
18+
/// No outcome re-points the selected tab at a different table. A reference can only be followed
19+
/// from a grid, so the tab it is followed from is always one the reader is reading, and retargeting
20+
/// it left them nowhere to go back to, which is the whole defect. Re-filtering a tab that is
21+
/// already on the referenced table is not that: it stays in the table they are in, and Back undoes
22+
/// it.
23+
internal enum ReferenceNavigationPlan: Equatable, Sendable {
24+
/// The selected tab is already on the referenced table, so only its filter changes.
25+
case refilterSelectedTab
26+
/// Another tab is already showing exactly this reference; bring it forward rather than
27+
/// building a second one beside it.
28+
case revealExistingTab
29+
/// A tab of its own, leaving whatever the reader was looking at where it was.
30+
case openNewTab
31+
}
32+
33+
/// What the window looks like from the jump's point of view.
34+
internal struct ReferenceNavigationContext: Equatable, Sendable {
35+
let intent: ReferenceOpenIntent
36+
/// The selected tab is browsing the referenced table already, whatever it is filtered to.
37+
let selectedTabShowsTarget: Bool
38+
/// Re-filtering that tab in place is safe. Staged structure edits say it is not: the discard
39+
/// alert clears cell changes and nothing else, so re-querying under them would promise
40+
/// something this path cannot keep.
41+
let selectedTabAcceptsRefilter: Bool
42+
/// Some tab, in this window or a sibling on the same connection, is already showing exactly
43+
/// this reference.
44+
let anotherTabShowsReference: Bool
45+
}
46+
47+
/// Resolves a reference jump to exactly one outcome.
48+
///
49+
/// Pure, so the choice can be tested without a window, a coordinator or a tab manager. It used to
50+
/// be an open-coded chain of guards inside `navigateToFKReference`, which is how it came to take
51+
/// over a tab that every other way of opening a table would have left alone.
52+
internal enum ReferenceNavigationPlanner {
53+
static func plan(for context: ReferenceNavigationContext) -> ReferenceNavigationPlan {
54+
guard context.intent == .follow else { return .openNewTab }
55+
56+
if context.selectedTabShowsTarget, context.selectedTabAcceptsRefilter {
57+
return .refilterSelectedTab
58+
}
59+
/// Landing on the tab that already answers the question beats building a second one beside
60+
/// it, which is also the order `openTableTab` takes with `activateIfAlreadyOpen`.
61+
if context.anotherTabShowsReference { return .revealExistingTab }
62+
return .openNewTab
63+
}
64+
}

TablePro/Views/Main/Child/DataTabGridDelegate.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ final class DataTabGridDelegate: DataGridViewDelegate {
9292
coordinator?.canClearActiveQueryResults ?? false
9393
}
9494

95-
func dataGridNavigateFK(value: String, fkInfo: ForeignKeyInfo, openInNewTab: Bool) {
96-
coordinator?.navigateToFKReference(value: value, fkInfo: fkInfo, openInNewTab: openInNewTab)
95+
func dataGridNavigateFK(value: String, fkInfo: ForeignKeyInfo, intent: ReferenceOpenIntent) {
96+
coordinator?.navigateToFKReference(value: value, fkInfo: fkInfo, intent: intent)
9797
}
9898

9999
/// The panel reads the selection, not a row this is told about.

0 commit comments

Comments
 (0)