Skip to content

Commit b87be8a

Browse files
authored
refactor(datagrid): drop DataGridIdentity + version-counter bridge (#938)
* refactor(datagrid): drop DataGridIdentity early-return + version-counter bridge (Phase D-b) Completes Phase D. The remaining metadataChanged (FK column reload) and paginationChanged (scroll-to-top) branches in updateNSView were the last consumers of DataGridIdentity. Both convert to explicit dispatch on TableViewCoordinating: - refreshForeignKeyColumns(): reloads visible FK column cells (lifted verbatim from the metadataChanged branch). Called from MainContentCoordinator+QueryHelpers after the columnEnumValues mutation that bumps metadataVersion. The other metadataVersion bump (in setupResultsForExecutedQuery) is followed immediately by setActiveTableRows, which already dispatches applyFullReplace and renders FK columns fresh — no extra dispatch needed there. - scrollToTop(): scrolls to row 0 if any rows exist. Trickier than refreshFK because pagination bumps the version BEFORE runQuery is fired; the scroll has to wait until the new data lands. Added pendingScrollToTopAfterReplace flag on MainContentCoordinator. paginateAfterConfirmation sets it; notifyFullReplaceIfActive checks and clears it after applyFullReplace fires. With both branches gone, DataGridIdentity has no consumers. Drops: - struct DataGridIdentity (entire type, 25 LOC) - var lastIdentity on TableViewCoordinator - The early-return short-circuit in updateNSView - previousIdentity capture and the metadataChanged / paginationChanged variables - metadataVersion / paginationVersion / schemaVersion props on DataGridView (no remaining readers) - The matching call-site args in MainEditorContentView.dataGridView and TableStructureView - DataGridIdentityTests entirely (-105 LOC test file) The 'initial pre-warm display cache' check (was `previousIdentity == nil || previousIdentity?.rowCount == 0`) becomes `oldRowCount == 0, rowDisplayCount > 0` — same intent, derived from the existing cachedRowCount instead of the dropped identity. Without the early-return, every SwiftUI body re-eval runs updateNSView's full body. After previous Phase D + the cachedTableRows / sort-cache / FK-routing PRs, that body is mostly cheap idempotent assignments and Equatable guards. Smoke-tested cell edit + undo, add row + undo, table switch, FK column display, pagination scroll, sort. Net -165 LOC including the deleted test file. * fix(datagrid): scope pendingScrollToTopAfterReplace per-tabId, add dispatch tests The original Bool flag could strand if pagination's runQuery failed before setActiveTableRows fired — the flag would stay set, and the next setActiveTableRows from any other code path (undo, FK navigation, multi-statement query) would unexpectedly scroll to top. Changing to Set<UUID> scopes the intent per-tab. Pagination inserts the tabId; notifyFullReplaceIfActive checks via remove() and only dispatches scrollToTop on a matching hit. A stranded entry stays scoped to that one tab and only fires when that tab next receives a full replace — narrow blast radius. Adds three regression tests: - scrollToTopFiresOnPendingFlag: setActiveTableRows on the active tab WITH the flag set dispatches scrollToTop and clears the flag - scrollToTopFlagIsScopedPerTab: a flag set for tab A doesn't fire when tab B is replaced; tab A's entry stays - scrollToTopSkippedWhenFlagAbsent: setActiveTableRows without the flag doesn't scroll
1 parent a027cdd commit b87be8a

12 files changed

Lines changed: 92 additions & 195 deletions

TablePro/Views/Main/Child/MainEditorContentView.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -504,9 +504,6 @@ struct MainEditorContentView: View {
504504
}
505505
},
506506
changeManager: currentChangeManager,
507-
schemaVersion: tab.schemaVersion,
508-
metadataVersion: tab.metadataVersion,
509-
paginationVersion: tab.paginationVersion,
510507
isEditable: isEditable,
511508
configuration: DataGridConfiguration(
512509
connectionId: connection.id,

TablePro/Views/Main/Extensions/MainContentCoordinator+Pagination.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ extension MainContentCoordinator {
9797

9898
mutate(&self.tabManager.tabs[idx].pagination)
9999
self.tabManager.tabs[idx].paginationVersion += 1
100+
self.pendingScrollToTopAfterReplace.insert(tabId)
100101
self.reloadCurrentPage()
101102
}
102103
}

TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,11 @@ extension MainContentCoordinator {
493493
return .columnsReplaced
494494
}
495495
tabManager.tabs[idx].metadataVersion += 1
496+
if let activeIdx = tabManager.selectedTabIndex,
497+
activeIdx < tabManager.tabs.count,
498+
tabManager.tabs[activeIdx].id == tabId {
499+
dataTabDelegate?.tableViewCoordinator?.refreshForeignKeyColumns()
500+
}
496501
}
497502
}
498503
}

TablePro/Views/Main/Extensions/MainContentCoordinator+TableRowsMutation.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,8 @@ extension MainContentCoordinator {
4545
idx < tabManager.tabs.count,
4646
tabManager.tabs[idx].id == tabId else { return }
4747
dataTabDelegate?.tableViewCoordinator?.applyFullReplace()
48+
if pendingScrollToTopAfterReplace.remove(tabId) != nil {
49+
dataTabDelegate?.tableViewCoordinator?.scrollToTop()
50+
}
4851
}
4952
}

TablePro/Views/Main/MainContentCoordinator.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ final class MainContentCoordinator {
147147
/// Cache for async-sorted query tab rows (large datasets sorted on background thread)
148148
@ObservationIgnored var querySortCache: [UUID: QuerySortCacheEntry] = [:]
149149

150+
@ObservationIgnored var pendingScrollToTopAfterReplace: Set<UUID> = []
151+
150152
// MARK: - Internal State
151153

152154
/// Cached column types per table for selective queries (avoids refetching schema).

TablePro/Views/Results/DataGridCoordinator.swift

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
6060

6161
@Binding var selectedRowIndices: Set<Int>
6262

63-
var lastIdentity: DataGridIdentity?
6463
private(set) var cachedRowCount: Int = 0
6564
private(set) var cachedColumnCount: Int = 0
6665
private(set) var enumOrSetColumns: Set<Int> = []
@@ -213,15 +212,13 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
213212
rebuildVisualStateCache()
214213
updateCache()
215214
tableView.insertRows(at: indices, withAnimation: .slideDown)
216-
lastIdentity = nil
217215
}
218216

219217
func applyRemovedRows(_ indices: IndexSet) {
220218
guard let tableView else { return }
221219
rebuildVisualStateCache()
222220
updateCache()
223221
tableView.removeRows(at: indices, withAnimation: .slideUp)
224-
lastIdentity = nil
225222
}
226223

227224
func applyFullReplace() {
@@ -230,7 +227,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
230227
rebuildVisualStateCache()
231228
updateCache()
232229
tableView.reloadData()
233-
lastIdentity = nil
234230
}
235231

236232
func displayRow(at displayIndex: Int) -> Row? {
@@ -422,6 +418,32 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
422418
tableView.editColumn(displayCol, row: displayRow, with: nil, select: true)
423419
}
424420

421+
func refreshForeignKeyColumns() {
422+
guard let tableView else { return }
423+
let tableRows = tableRowsProvider()
424+
let fkColumnIndices = IndexSet(
425+
tableView.tableColumns.enumerated().compactMap { displayIndex, tableColumn in
426+
guard tableColumn.identifier.rawValue != "__rowNumber__",
427+
let modelIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier),
428+
modelIndex < tableRows.columns.count else { return nil }
429+
let columnName = tableRows.columns[modelIndex]
430+
return tableRows.columnForeignKeys[columnName] != nil ? displayIndex : nil
431+
}
432+
)
433+
guard !fkColumnIndices.isEmpty else { return }
434+
let visibleRange = tableView.rows(in: tableView.visibleRect)
435+
guard visibleRange.length > 0 else { return }
436+
let visibleRows = IndexSet(
437+
integersIn: visibleRange.location..<(visibleRange.location + visibleRange.length)
438+
)
439+
tableView.reloadData(forRowIndexes: visibleRows, columnIndexes: fkColumnIndices)
440+
}
441+
442+
func scrollToTop() {
443+
guard let tableView, tableView.numberOfRows > 0 else { return }
444+
tableView.scrollRowToVisible(0)
445+
}
446+
425447
func rebuildColumnMetadataCache(from tableRows: TableRows) {
426448
var enumSet = Set<Int>()
427449
var fkSet = Set<Int>()

TablePro/Views/Results/DataGridView.swift

Lines changed: 3 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -22,40 +22,10 @@ struct RowVisualState {
2222
static let empty = RowVisualState(isDeleted: false, isInserted: false, modifiedColumns: [])
2323
}
2424

25-
struct DataGridIdentity: Equatable {
26-
let schemaVersion: Int
27-
let metadataVersion: Int
28-
let paginationVersion: Int
29-
let rowCount: Int
30-
let columnCount: Int
31-
let isEditable: Bool
32-
let tabType: TabType?
33-
let tableName: String?
34-
let primaryKeyColumns: [String]
35-
let hiddenColumns: Set<String>
36-
37-
init(schemaVersion: Int, metadataVersion: Int, paginationVersion: Int,
38-
rowCount: Int, columnCount: Int, isEditable: Bool, configuration: DataGridConfiguration) {
39-
self.schemaVersion = schemaVersion
40-
self.metadataVersion = metadataVersion
41-
self.paginationVersion = paginationVersion
42-
self.rowCount = rowCount
43-
self.columnCount = columnCount
44-
self.isEditable = isEditable
45-
self.tabType = configuration.tabType
46-
self.tableName = configuration.tableName
47-
self.primaryKeyColumns = configuration.primaryKeyColumns
48-
self.hiddenColumns = configuration.hiddenColumns
49-
}
50-
}
51-
5225
struct DataGridView: NSViewRepresentable {
5326
var tableRowsProvider: @MainActor () -> TableRows = { TableRows() }
5427
var tableRowsMutator: @MainActor (@MainActor (inout TableRows) -> Void) -> Void = { _ in }
5528
var changeManager: AnyChangeManager
56-
var schemaVersion: Int = 0
57-
var metadataVersion: Int = 0
58-
var paginationVersion: Int = 0
5929
let isEditable: Bool
6030
var configuration: DataGridConfiguration = .init()
6131
var sortedIDs: [RowID]?
@@ -228,27 +198,6 @@ struct DataGridView: NSViewRepresentable {
228198
let rowDisplayCount = sortedIDs?.count ?? latestRows.count
229199
let columnCount = latestRows.columns.count
230200

231-
let currentIdentity = DataGridIdentity(
232-
schemaVersion: schemaVersion,
233-
metadataVersion: metadataVersion,
234-
paginationVersion: paginationVersion,
235-
rowCount: rowDisplayCount,
236-
columnCount: columnCount,
237-
isEditable: isEditable,
238-
configuration: configuration
239-
)
240-
if currentIdentity == coordinator.lastIdentity {
241-
coordinator.delegate = delegate
242-
coordinator.tableRowsProvider = tableRowsProvider
243-
coordinator.tableRowsMutator = tableRowsMutator
244-
coordinator.sortedIDs = sortedIDs
245-
coordinator.syncDisplayFormats(displayFormats)
246-
delegate?.dataGridAttach(tableViewCoordinator: coordinator)
247-
return
248-
}
249-
let previousIdentity = coordinator.lastIdentity
250-
coordinator.lastIdentity = currentIdentity
251-
252201
let settings = AppSettingsManager.shared.dataGrid
253202
if tableView.rowHeight != CGFloat(settings.rowHeight.rawValue) {
254203
tableView.rowHeight = CGFloat(settings.rowHeight.rawValue)
@@ -257,7 +206,6 @@ struct DataGridView: NSViewRepresentable {
257206
tableView.usesAlternatingRowBackgroundColors = settings.showAlternateRows
258207
}
259208

260-
let metadataChanged = previousIdentity.map { $0.metadataVersion != metadataVersion } ?? false
261209
let oldRowCount = coordinator.cachedRowCount
262210
let oldColumnCount = coordinator.cachedColumnCount
263211

@@ -267,7 +215,7 @@ struct DataGridView: NSViewRepresentable {
267215
coordinator.updateCache()
268216
coordinator.rebuildColumnMetadataCache(from: latestRows)
269217

270-
if previousIdentity == nil || previousIdentity?.rowCount == 0 {
218+
if oldRowCount == 0, rowDisplayCount > 0 {
271219
let rowH = tableView.rowHeight
272220
if rowH > 0 {
273221
let visibleRows = Int(tableView.visibleRect.height / rowH) + 5
@@ -315,15 +263,10 @@ struct DataGridView: NSViewRepresentable {
315263

316264
syncSortDescriptors(tableView: tableView, coordinator: coordinator, columns: latestRows.columns)
317265

318-
let paginationChanged = previousIdentity.map { $0.paginationVersion != paginationVersion } ?? false
319-
320266
reloadAndSyncSelection(
321267
tableView: tableView,
322268
coordinator: coordinator,
323-
tableRows: latestRows,
324-
needsFullReload: needsFullReload,
325-
metadataChanged: metadataChanged,
326-
paginationChanged: paginationChanged
269+
needsFullReload: needsFullReload
327270
)
328271
}
329272

@@ -493,36 +436,10 @@ struct DataGridView: NSViewRepresentable {
493436
private func reloadAndSyncSelection(
494437
tableView: NSTableView,
495438
coordinator: TableViewCoordinator,
496-
tableRows: TableRows,
497-
needsFullReload: Bool,
498-
metadataChanged: Bool = false,
499-
paginationChanged: Bool = false
439+
needsFullReload: Bool
500440
) {
501441
if needsFullReload {
502442
tableView.reloadData()
503-
} else if metadataChanged {
504-
let fkColumnIndices = IndexSet(
505-
tableView.tableColumns.enumerated().compactMap { displayIndex, tableColumn in
506-
guard tableColumn.identifier.rawValue != "__rowNumber__",
507-
let modelIndex = Self.dataColumnIndex(from: tableColumn.identifier),
508-
modelIndex < tableRows.columns.count else { return nil }
509-
let columnName = tableRows.columns[modelIndex]
510-
return tableRows.columnForeignKeys[columnName] != nil ? displayIndex : nil
511-
}
512-
)
513-
if !fkColumnIndices.isEmpty {
514-
let visibleRange = tableView.rows(in: tableView.visibleRect)
515-
if visibleRange.length > 0 {
516-
let visibleRows = IndexSet(
517-
integersIn: visibleRange.location..<(visibleRange.location + visibleRange.length)
518-
)
519-
tableView.reloadData(forRowIndexes: visibleRows, columnIndexes: fkColumnIndices)
520-
}
521-
}
522-
}
523-
524-
if paginationChanged && tableView.numberOfRows > 0 {
525-
tableView.scrollRowToVisible(0)
526443
}
527444

528445
let currentSelection = tableView.selectedRowIndexes

TablePro/Views/Results/TableViewCoordinating.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ protocol TableViewCoordinating: AnyObject {
99
func invalidateCachesForUndoRedo()
1010
func commitActiveCellEdit()
1111
func beginEditing(displayRow: Int, column: Int)
12+
func refreshForeignKeyColumns()
13+
func scrollToTop()
1214
}
1315

1416
extension TableViewCoordinator: TableViewCoordinating {}

TablePro/Views/Structure/TableStructureView.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,6 @@ struct TableStructureView: View {
271271
return DataGridView(
272272
tableRowsProvider: { tableRows },
273273
changeManager: wrappedChangeManager,
274-
schemaVersion: displayVersion,
275274
isEditable: canEdit,
276275
configuration: DataGridConfiguration(
277276
dropdownColumns: allDropdownColumns,

TableProTests/Views/Main/Child/DataTabGridDelegateTests.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ private final class FakeTableViewCoordinator: TableViewCoordinating {
4646
func beginEditing(displayRow: Int, column: Int) {
4747
beginEditingCalls.append((row: displayRow, column: column))
4848
}
49+
50+
var refreshFKCount: Int = 0
51+
var scrollToTopCount: Int = 0
52+
53+
func refreshForeignKeyColumns() { refreshFKCount += 1 }
54+
func scrollToTop() { scrollToTopCount += 1 }
4955
}
5056

5157
@Suite("DataTabGridDelegate row-delta forwarding")

0 commit comments

Comments
 (0)