Skip to content

refactor(datagrid): drop DataGridIdentity + version-counter bridge #938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
datlechin merged 2 commits into from
Apr 29, 2026
Merged

refactor(datagrid): drop DataGridIdentity + version-counter bridge #938

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e59834
refactor(datagrid): drop DataGridIdentity early-return + version-coun…
datlechin Apr 29, 2026
0af2e8f
fix(datagrid): scope pendingScrollToTopAfterReplace per-tabId, add di…
datlechin Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions TablePro/Views/Main/Child/MainEditorContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,6 @@ struct MainEditorContentView: View {
}
},
changeManager: currentChangeManager,
schemaVersion: tab.schemaVersion,
metadataVersion: tab.metadataVersion,
paginationVersion: tab.paginationVersion,
isEditable: isEditable,
configuration: DataGridConfiguration(
connectionId: connection.id,
Expand Down
1 change: 1 addition & 0 deletions TablePro/Views/Main/Extensions/MainContentCoordinator+Pagination.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ extension MainContentCoordinator {

mutate(&self.tabManager.tabs[idx].pagination)
self.tabManager.tabs[idx].paginationVersion += 1
self.pendingScrollToTopAfterReplace = true
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Track pending pagination scroll per tab

pendingScrollToTopAfterReplace is a single global flag set during pagination, but it is consumed only when notifyFullReplaceIfActive runs for whichever tab is currently selected. If the user paginates tab A and switches to tab B before A’s query result applies, tab A’s setActiveTableRows path returns early (tabManager.tabs[idx].id == tabId guard fails), so A never gets its scroll-to-top; later, an unrelated full replace on tab B can consume the flag and scroll B unexpectedly. This is a real behavior regression from the old paginationVersion-driven logic for cross-tab timing.

Useful? React with 👍 / 👎.

self.reloadCurrentPage()
}
}
Expand Down
5 changes: 5 additions & 0 deletions TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,11 @@ extension MainContentCoordinator {
return .columnsReplaced
}
tabManager.tabs[idx].metadataVersion += 1
if let activeIdx = tabManager.selectedTabIndex,
activeIdx < tabManager.tabs.count,
tabManager.tabs[activeIdx].id == tabId {
dataTabDelegate?.tableViewCoordinator?.refreshForeignKeyColumns()
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions TablePro/Views/Main/Extensions/MainContentCoordinator+TableRowsMutation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,9 @@ extension MainContentCoordinator {
idx < tabManager.tabs.count,
tabManager.tabs[idx].id == tabId else { return }
dataTabDelegate?.tableViewCoordinator?.applyFullReplace()
if pendingScrollToTopAfterReplace {
pendingScrollToTopAfterReplace = false
dataTabDelegate?.tableViewCoordinator?.scrollToTop()
}
}
}
2 changes: 2 additions & 0 deletions TablePro/Views/Main/MainContentCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ final class MainContentCoordinator {
/// Cache for async-sorted query tab rows (large datasets sorted on background thread)
@ObservationIgnored var querySortCache: [UUID: QuerySortCacheEntry] = [:]

@ObservationIgnored var pendingScrollToTopAfterReplace: Bool = false

// MARK: - Internal State

/// Cached column types per table for selective queries (avoids refetching schema).
Expand Down
30 changes: 26 additions & 4 deletions TablePro/Views/Results/DataGridCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData

@Binding var selectedRowIndices: Set<Int>

var lastIdentity: DataGridIdentity?
private(set) var cachedRowCount: Int = 0
private(set) var cachedColumnCount: Int = 0
private(set) var enumOrSetColumns: Set<Int> = []
Expand Down Expand Up @@ -213,15 +212,13 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
rebuildVisualStateCache()
updateCache()
tableView.insertRows(at: indices, withAnimation: .slideDown)
lastIdentity = nil
}

func applyRemovedRows(_ indices: IndexSet) {
guard let tableView else { return }
rebuildVisualStateCache()
updateCache()
tableView.removeRows(at: indices, withAnimation: .slideUp)
lastIdentity = nil
}

func applyFullReplace() {
Expand All @@ -230,7 +227,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
rebuildVisualStateCache()
updateCache()
tableView.reloadData()
lastIdentity = nil
}

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

func refreshForeignKeyColumns() {
guard let tableView else { return }
let tableRows = tableRowsProvider()
let fkColumnIndices = IndexSet(
tableView.tableColumns.enumerated().compactMap { displayIndex, tableColumn in
guard tableColumn.identifier.rawValue != "__rowNumber__",
let modelIndex = DataGridView.dataColumnIndex(from: tableColumn.identifier),
modelIndex < tableRows.columns.count else { return nil }
let columnName = tableRows.columns[modelIndex]
return tableRows.columnForeignKeys[columnName] != nil ? displayIndex : nil
}
)
guard !fkColumnIndices.isEmpty else { return }
let visibleRange = tableView.rows(in: tableView.visibleRect)
guard visibleRange.length > 0 else { return }
let visibleRows = IndexSet(
integersIn: visibleRange.location..<(visibleRange.location + visibleRange.length)
)
tableView.reloadData(forRowIndexes: visibleRows, columnIndexes: fkColumnIndices)
}

func scrollToTop() {
guard let tableView, tableView.numberOfRows > 0 else { return }
tableView.scrollRowToVisible(0)
}

func rebuildColumnMetadataCache(from tableRows: TableRows) {
var enumSet = Set<Int>()
var fkSet = Set<Int>()
Expand Down
89 changes: 3 additions & 86 deletions TablePro/Views/Results/DataGridView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,40 +22,10 @@ struct RowVisualState {
static let empty = RowVisualState(isDeleted: false, isInserted: false, modifiedColumns: [])
}

struct DataGridIdentity: Equatable {
let schemaVersion: Int
let metadataVersion: Int
let paginationVersion: Int
let rowCount: Int
let columnCount: Int
let isEditable: Bool
let tabType: TabType?
let tableName: String?
let primaryKeyColumns: [String]
let hiddenColumns: Set<String>

init(schemaVersion: Int, metadataVersion: Int, paginationVersion: Int,
rowCount: Int, columnCount: Int, isEditable: Bool, configuration: DataGridConfiguration) {
self.schemaVersion = schemaVersion
self.metadataVersion = metadataVersion
self.paginationVersion = paginationVersion
self.rowCount = rowCount
self.columnCount = columnCount
self.isEditable = isEditable
self.tabType = configuration.tabType
self.tableName = configuration.tableName
self.primaryKeyColumns = configuration.primaryKeyColumns
self.hiddenColumns = configuration.hiddenColumns
}
}

struct DataGridView: NSViewRepresentable {
var tableRowsProvider: @MainActor () -> TableRows = { TableRows() }
var tableRowsMutator: @MainActor (@MainActor (inout TableRows) -> Void) -> Void = { _ in }
var changeManager: AnyChangeManager
var schemaVersion: Int = 0
var metadataVersion: Int = 0
var paginationVersion: Int = 0
let isEditable: Bool
var configuration: DataGridConfiguration = .init()
var sortedIDs: [RowID]?
Expand Down Expand Up @@ -228,27 +198,6 @@ struct DataGridView: NSViewRepresentable {
let rowDisplayCount = sortedIDs?.count ?? latestRows.count
let columnCount = latestRows.columns.count

let currentIdentity = DataGridIdentity(
schemaVersion: schemaVersion,
metadataVersion: metadataVersion,
paginationVersion: paginationVersion,
rowCount: rowDisplayCount,
columnCount: columnCount,
isEditable: isEditable,
configuration: configuration
)
if currentIdentity == coordinator.lastIdentity {
coordinator.delegate = delegate
coordinator.tableRowsProvider = tableRowsProvider
coordinator.tableRowsMutator = tableRowsMutator
coordinator.sortedIDs = sortedIDs
coordinator.syncDisplayFormats(displayFormats)
delegate?.dataGridAttach(tableViewCoordinator: coordinator)
return
}
let previousIdentity = coordinator.lastIdentity
coordinator.lastIdentity = currentIdentity

let settings = AppSettingsManager.shared.dataGrid
if tableView.rowHeight != CGFloat(settings.rowHeight.rawValue) {
tableView.rowHeight = CGFloat(settings.rowHeight.rawValue)
Expand All @@ -257,7 +206,6 @@ struct DataGridView: NSViewRepresentable {
tableView.usesAlternatingRowBackgroundColors = settings.showAlternateRows
}

let metadataChanged = previousIdentity.map { $0.metadataVersion != metadataVersion } ?? false
let oldRowCount = coordinator.cachedRowCount
let oldColumnCount = coordinator.cachedColumnCount

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

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

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

let paginationChanged = previousIdentity.map { $0.paginationVersion != paginationVersion } ?? false

reloadAndSyncSelection(
tableView: tableView,
coordinator: coordinator,
tableRows: latestRows,
needsFullReload: needsFullReload,
metadataChanged: metadataChanged,
paginationChanged: paginationChanged
needsFullReload: needsFullReload
)
}

Expand Down Expand Up @@ -493,36 +436,10 @@ struct DataGridView: NSViewRepresentable {
private func reloadAndSyncSelection(
tableView: NSTableView,
coordinator: TableViewCoordinator,
tableRows: TableRows,
needsFullReload: Bool,
metadataChanged: Bool = false,
paginationChanged: Bool = false
needsFullReload: Bool
) {
if needsFullReload {
tableView.reloadData()
} else if metadataChanged {
let fkColumnIndices = IndexSet(
tableView.tableColumns.enumerated().compactMap { displayIndex, tableColumn in
guard tableColumn.identifier.rawValue != "__rowNumber__",
let modelIndex = Self.dataColumnIndex(from: tableColumn.identifier),
modelIndex < tableRows.columns.count else { return nil }
let columnName = tableRows.columns[modelIndex]
return tableRows.columnForeignKeys[columnName] != nil ? displayIndex : nil
}
)
if !fkColumnIndices.isEmpty {
let visibleRange = tableView.rows(in: tableView.visibleRect)
if visibleRange.length > 0 {
let visibleRows = IndexSet(
integersIn: visibleRange.location..<(visibleRange.location + visibleRange.length)
)
tableView.reloadData(forRowIndexes: visibleRows, columnIndexes: fkColumnIndices)
}
}
}

if paginationChanged && tableView.numberOfRows > 0 {
tableView.scrollRowToVisible(0)
}

let currentSelection = tableView.selectedRowIndexes
Expand Down
2 changes: 2 additions & 0 deletions TablePro/Views/Results/TableViewCoordinating.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ protocol TableViewCoordinating: AnyObject {
func invalidateCachesForUndoRedo()
func commitActiveCellEdit()
func beginEditing(displayRow: Int, column: Int)
func refreshForeignKeyColumns()
func scrollToTop()
}

extension TableViewCoordinator: TableViewCoordinating {}
1 change: 0 additions & 1 deletion TablePro/Views/Structure/TableStructureView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,6 @@ struct TableStructureView: View {
return DataGridView(
tableRowsProvider: { tableRows },
changeManager: wrappedChangeManager,
schemaVersion: displayVersion,
isEditable: canEdit,
configuration: DataGridConfiguration(
dropdownColumns: allDropdownColumns,
Expand Down
6 changes: 6 additions & 0 deletions TableProTests/Views/Main/Child/DataTabGridDelegateTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ private final class FakeTableViewCoordinator: TableViewCoordinating {
func beginEditing(displayRow: Int, column: Int) {
beginEditingCalls.append((row: displayRow, column: column))
}

var refreshFKCount: Int = 0
var scrollToTopCount: Int = 0

func refreshForeignKeyColumns() { refreshFKCount += 1 }
func scrollToTop() { scrollToTopCount += 1 }
}

@Suite("DataTabGridDelegate row-delta forwarding")
Expand Down
5 changes: 5 additions & 0 deletions TableProTests/Views/Main/TableRowsMutationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ private final class FakeTableViewCoordinator: TableViewCoordinating {
func beginEditing(displayRow: Int, column: Int) {
beginEditingCalls.append((row: displayRow, column: column))
}

var refreshFKCount = 0
var scrollToTopCount = 0
func refreshForeignKeyColumns() { refreshFKCount += 1 }
func scrollToTop() { scrollToTopCount += 1 }
}

@Suite("setActiveTableRows dispatch")
Expand Down
101 changes: 0 additions & 101 deletions TableProTests/Views/Results/DataGridIdentityTests.swift
View file
Open in desktop

This file was deleted.

Loading