From 60537f6ce9c826cb38ebac6d1481d4acff116def Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 12:03:08 +0700 Subject: [PATCH 01/15] perf(datagrid): add OSLog signposts to measure cell rendering hot path --- .../MainContentCoordinator+QueryHelpers.swift | 5 ++ .../Views/Results/DataGridCellFactory.swift | 13 ++++ .../Views/Results/DataGridCoordinator.swift | 20 +++++ TablePro/Views/Results/DataGridView.swift | 9 +++ .../Extensions/DataGridView+Columns.swift | 78 +++++++++++++++++-- 5 files changed, 119 insertions(+), 6 deletions(-) diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift index 4359ba0f8..1b6a34c20 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift @@ -12,6 +12,7 @@ import os import TableProPluginKit private let progressLog = Logger(subsystem: "com.TablePro", category: "ProgressiveLoad") +private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") /// Context for progressive query result loading internal struct QueryPageContext { @@ -246,6 +247,7 @@ extension MainContentCoordinator { queryParameterValues: [QueryParameter]? = nil ) { guard let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return } + let __t0Total = CFAbsoluteTimeGetCurrent() var updatedTab = tabManager.tabs[idx] var columnEnumValues: [String: [String]] = [:] @@ -290,6 +292,7 @@ extension MainContentCoordinator { updatedTab.metadataVersion += 1 } + let __tFrom = CFAbsoluteTimeGetCurrent() let newTableRows = TableRows.from( queryRows: rows, columns: columns, @@ -299,6 +302,7 @@ extension MainContentCoordinator { columnEnumValues: columnEnumValues, columnNullable: columnNullable ) + gridPerfLog.notice("[grid-perf] TableRows.from took \(((CFAbsoluteTimeGetCurrent() - __tFrom) * 1000), format: .fixed(precision: 2)) ms (rows=\(rows.count) cols=\(columns.count))") setActiveTableRows(newTableRows, for: updatedTab.id) let rs = ResultSet(label: tableName ?? "Result", tableRows: newTableRows) @@ -380,6 +384,7 @@ extension MainContentCoordinator { if tabManager.selectedTabId == tabId, isEditable, !changeManager.hasChanges { changeManager.clearChangesAndUndoHistory() } + gridPerfLog.notice("[grid-perf] applyPhase1Result total took \(((CFAbsoluteTimeGetCurrent() - __t0Total) * 1000), format: .fixed(precision: 2)) ms (table=\(tableName ?? "nil"))") } /// Launch Phase 2 background work: exact COUNT(*) and enum value fetching diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index f9c3f2cc5..9d4b302e0 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -8,6 +8,7 @@ import AppKit import QuartzCore +import os @MainActor final class FKArrowButton: NSButton { @@ -122,6 +123,7 @@ final class DataGridCellFactory { let gridCellView: DataGridCellView let cell: NSTextField + let __tA = CFAbsoluteTimeGetCurrent() if let reused = tableView.makeView(withIdentifier: cellIdentifier, owner: nil) as? DataGridCellView, let textField = reused.textField { gridCellView = reused @@ -172,7 +174,9 @@ final class DataGridCellFactory { chevron.heightAnchor.constraint(equalToConstant: 12), ]) } + let __aMs = (CFAbsoluteTimeGetCurrent() - __tA) * 1000 + let __tB = CFAbsoluteTimeGetCurrent() cell.lineBreakMode = .byTruncatingTail cell.maximumNumberOfLines = 1 cell.cell?.truncatesLastVisibleLine = true @@ -228,9 +232,13 @@ final class DataGridCellFactory { let isDeleted = visualState.isDeleted let isInserted = visualState.isInserted let isModified = visualState.modifiedColumns.contains(columnIndex) + let __bMs = (CFAbsoluteTimeGetCurrent() - __tB) * 1000 + let __tC = CFAbsoluteTimeGetCurrent() configureTextContent(cell: cell, displayValue: displayValue, rawValue: rawValue, isLargeDataset: isLargeDataset) + let __cMs = (CFAbsoluteTimeGetCurrent() - __tC) * 1000 + let __tD = CFAbsoluteTimeGetCurrent() CATransaction.begin() CATransaction.setDisableActions(true) @@ -247,13 +255,18 @@ final class DataGridCellFactory { gridCellView.isFocusedCell = isFocused CATransaction.commit() + let __dMs = (CFAbsoluteTimeGetCurrent() - __tD) * 1000 + let __tE = CFAbsoluteTimeGetCurrent() let accessibilityValue = rawValue ?? String(localized: "NULL") cell.setAccessibilityLabel( String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue) ) gridCellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) gridCellView.setAccessibilityColumnIndexRange(NSRange(location: columnIndex, length: 1)) + let __eMs = (CFAbsoluteTimeGetCurrent() - __tE) * 1000 + + ViewForStats.recordSection(a: __aMs, b: __bMs, c: __cMs, d: __dMs, e: __eMs) return gridCellView } diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index c2b001e31..1b35a237f 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -1,6 +1,9 @@ import AppKit +import os import SwiftUI +private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") + // MARK: - Coordinator @MainActor @@ -257,10 +260,27 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData func applyFullReplace() { guard let tableView else { return } + let __t0 = CFAbsoluteTimeGetCurrent() + + let __tCache = CFAbsoluteTimeGetCurrent() displayCache.removeAll() + gridPerfLog.notice("[grid-perf] displayCache.removeAll took \(((CFAbsoluteTimeGetCurrent() - __tCache) * 1000), format: .fixed(precision: 2)) ms") + + let __tVisual = CFAbsoluteTimeGetCurrent() rebuildVisualStateCache() + gridPerfLog.notice("[grid-perf] rebuildVisualStateCache took \(((CFAbsoluteTimeGetCurrent() - __tVisual) * 1000), format: .fixed(precision: 2)) ms") + updateCache() + + let __tReload = CFAbsoluteTimeGetCurrent() tableView.reloadData() + gridPerfLog.notice("[grid-perf] tableView.reloadData took \(((CFAbsoluteTimeGetCurrent() - __tReload) * 1000), format: .fixed(precision: 2)) ms (rows=\(tableView.numberOfRows))") + + gridPerfLog.notice("[grid-perf] applyFullReplace total took \(((CFAbsoluteTimeGetCurrent() - __t0) * 1000), format: .fixed(precision: 2)) ms") + + DispatchQueue.main.async { + ViewForStats.flushIfQuiet() + } } func displayRow(at displayIndex: Int) -> Row? { diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index ebd88872d..a20b977b2 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -7,8 +7,11 @@ // import AppKit +import os import SwiftUI +private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") + struct CellPosition: Hashable { let row: Int let column: Int @@ -169,6 +172,8 @@ struct DataGridView: NSViewRepresentable { if tableView.editedRow >= 0 { return } if let editor = context.coordinator.overlayEditor, editor.isActive { return } + let __t0Update = CFAbsoluteTimeGetCurrent() + if let rowNumCol = tableView.tableColumns.first(where: { $0.identifier == ColumnIdentitySchema.rowNumberIdentifier }) { let shouldHide = !configuration.showRowNumbers if rowNumCol.isHidden != shouldHide { @@ -269,6 +274,8 @@ struct DataGridView: NSViewRepresentable { coordinator: coordinator, needsFullReload: needsFullReload ) + + gridPerfLog.notice("[grid-perf] DataGridView.updateNSView took \(((CFAbsoluteTimeGetCurrent() - __t0Update) * 1000), format: .fixed(precision: 2)) ms (rows=\(rowDisplayCount) cols=\(columnCount) reload=\(needsFullReload))") } // MARK: - updateNSView Helpers @@ -322,6 +329,7 @@ struct DataGridView: NSViewRepresentable { tableRows: TableRows, savedLayout: ColumnLayoutState? ) { + let __tRebuild = CFAbsoluteTimeGetCurrent() let columnsToRemove = tableView.tableColumns.filter { $0.identifier != ColumnIdentitySchema.rowNumberIdentifier } @@ -364,6 +372,7 @@ struct DataGridView: NSViewRepresentable { ) tableView.addTableColumn(column) } + gridPerfLog.notice("[grid-perf] rebuildColumns took \(((CFAbsoluteTimeGetCurrent() - __tRebuild) * 1000), format: .fixed(precision: 2)) ms (cols=\(tableRows.columns.count) restoredWidths=\(willRestoreWidths))") } private func refreshColumnTitles( diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index 21c7b4611..51688c9d5 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -4,34 +4,98 @@ // import AppKit +import os import SwiftUI +private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") + +@MainActor +internal enum ViewForStats { + static var count: Int = 0 + static var totalMs: Double = 0 + static var firstCallTime: CFAbsoluteTime = 0 + static var lastReportedCount: Int = 0 + + static var sectionA_makeView_ms: Double = 0 + static var sectionB_buttons_ms: Double = 0 + static var sectionC_textContent_ms: Double = 0 + static var sectionD_visualState_ms: Double = 0 + static var sectionE_accessibility_ms: Double = 0 + + static func record(_ ms: Double) { + if count == 0 { firstCallTime = CFAbsoluteTimeGetCurrent() } + count += 1 + totalMs += ms + } + + static func recordSection(a: Double = 0, b: Double = 0, c: Double = 0, d: Double = 0, e: Double = 0) { + sectionA_makeView_ms += a + sectionB_buttons_ms += b + sectionC_textContent_ms += c + sectionD_visualState_ms += d + sectionE_accessibility_ms += e + } + + static func flushIfQuiet() { + guard count > lastReportedCount else { return } + let delta = count - lastReportedCount + gridPerfLog.notice("[grid-perf] viewFor batch: \(delta) calls, total \(totalMs, format: .fixed(precision: 2)) ms since last report") + gridPerfLog.notice("[grid-perf] sectionA makeView/reuse: \(sectionA_makeView_ms, format: .fixed(precision: 2)) ms") + gridPerfLog.notice("[grid-perf] sectionB FK/chevron buttons: \(sectionB_buttons_ms, format: .fixed(precision: 2)) ms") + gridPerfLog.notice("[grid-perf] sectionC configureTextContent: \(sectionC_textContent_ms, format: .fixed(precision: 2)) ms") + gridPerfLog.notice("[grid-perf] sectionD CATransaction+bg+focus: \(sectionD_visualState_ms, format: .fixed(precision: 2)) ms") + gridPerfLog.notice("[grid-perf] sectionE accessibility labels: \(sectionE_accessibility_ms, format: .fixed(precision: 2)) ms") + lastReportedCount = count + totalMs = 0 + sectionA_makeView_ms = 0 + sectionB_buttons_ms = 0 + sectionC_textContent_ms = 0 + sectionD_visualState_ms = 0 + sectionE_accessibility_ms = 0 + } +} + extension TableViewCoordinator { func tableView(_ tableView: NSTableView, viewFor tableColumn: NSTableColumn?, row: Int) -> NSView? { - guard let column = tableColumn else { return nil } + let __tCell = CFAbsoluteTimeGetCurrent() + guard let column = tableColumn else { + let __view: NSView? = nil + ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) + return __view + } let tableRows = tableRowsProvider() let displayCount = sortedIDs?.count ?? tableRows.count if column.identifier == ColumnIdentitySchema.rowNumberIdentifier { - return cellFactory.makeRowNumberCell( + let __view: NSView? = cellFactory.makeRowNumberCell( tableView: tableView, row: row, cachedRowCount: displayCount, visualState: visualState(for: row) ) + ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) + return __view } - guard let columnIndex = dataColumnIndex(from: column.identifier) else { return nil } + guard let columnIndex = dataColumnIndex(from: column.identifier) else { + let __view: NSView? = nil + ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) + return __view + } guard row >= 0 && row < displayCount, columnIndex >= 0 && columnIndex < cachedColumnCount else { - return nil + let __view: NSView? = nil + ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) + return __view } guard let displayRow = displayRow(at: row), columnIndex < displayRow.values.count else { - return nil + let __view: NSView? = nil + ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) + return __view } let rawValue = displayRow.values[columnIndex] let columnType = columnIndex < tableRows.columnTypes.count @@ -65,7 +129,7 @@ extension TableViewCoordinator { return ct.isBooleanType || ct.isDateType || ct.isJsonType || ct.isBlobType }() - return cellFactory.makeDataCell( + let __view: NSView? = cellFactory.makeDataCell( tableView: tableView, row: row, columnIndex: columnIndex, @@ -83,6 +147,8 @@ extension TableViewCoordinator { chevronAction: #selector(handleChevronClick(_:)), delegate: self ) + ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) + return __view } func tableView(_ tableView: NSTableView, rowViewForRow row: Int) -> NSTableRowView? { From cd7e2c005600c3262819c7dd950ffe616246ea3f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 12:08:49 +0700 Subject: [PATCH 02/15] refactor(datagrid): add typed cell hierarchy + registry foundation --- .../Results/Cells/AccessoryButtons.swift | 55 +++++ .../Results/Cells/DataGridBaseCellView.swift | 221 ++++++++++++++++++ .../Results/Cells/DataGridBlobCellView.swift | 12 + .../Cells/DataGridBooleanCellView.swift | 12 + .../Cells/DataGridCellAccessoryDelegate.swift | 12 + .../Results/Cells/DataGridCellContent.swift | 28 +++ .../Results/Cells/DataGridCellKind.swift | 16 ++ .../Results/Cells/DataGridCellRegistry.swift | 142 +++++++++++ .../Cells/DataGridChevronCellView.swift | 44 ++++ .../Results/Cells/DataGridDateCellView.swift | 12 + .../Cells/DataGridDropdownCellView.swift | 12 + .../Cells/DataGridForeignKeyCellView.swift | 52 +++++ .../Results/Cells/DataGridJsonCellView.swift | 12 + .../Results/Cells/DataGridTextCellView.swift | 12 + 14 files changed, 642 insertions(+) create mode 100644 TablePro/Views/Results/Cells/AccessoryButtons.swift create mode 100644 TablePro/Views/Results/Cells/DataGridBaseCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridBlobCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridBooleanCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridCellAccessoryDelegate.swift create mode 100644 TablePro/Views/Results/Cells/DataGridCellContent.swift create mode 100644 TablePro/Views/Results/Cells/DataGridCellKind.swift create mode 100644 TablePro/Views/Results/Cells/DataGridCellRegistry.swift create mode 100644 TablePro/Views/Results/Cells/DataGridChevronCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridDateCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridDropdownCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridForeignKeyCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridJsonCellView.swift create mode 100644 TablePro/Views/Results/Cells/DataGridTextCellView.swift diff --git a/TablePro/Views/Results/Cells/AccessoryButtons.swift b/TablePro/Views/Results/Cells/AccessoryButtons.swift new file mode 100644 index 000000000..e40341cc7 --- /dev/null +++ b/TablePro/Views/Results/Cells/AccessoryButtons.swift @@ -0,0 +1,55 @@ +// +// AccessoryButtons.swift +// TablePro +// + +import AppKit + +@MainActor +final class FKArrowButton: NSButton { + var fkRow: Int = -1 + var fkColumnIndex: Int = -1 +} + +@MainActor +final class CellChevronButton: NSButton { + var cellRow: Int = -1 + var cellColumnIndex: Int = -1 +} + +@MainActor +enum AccessoryButtonFactory { + static func makeFKArrowButton() -> FKArrowButton { + let button = FKArrowButton() + button.bezelStyle = .inline + button.isBordered = false + button.image = NSImage( + systemSymbolName: "arrow.right.circle.fill", + accessibilityDescription: String(localized: "Navigate to referenced row") + ) + button.contentTintColor = .tertiaryLabelColor + button.translatesAutoresizingMaskIntoConstraints = false + button.setContentHuggingPriority(.required, for: .horizontal) + button.setContentCompressionResistancePriority(.required, for: .horizontal) + button.imageScaling = .scaleProportionallyDown + button.isHidden = true + return button + } + + static func makeChevronButton() -> CellChevronButton { + let chevron = CellChevronButton() + chevron.bezelStyle = .inline + chevron.isBordered = false + chevron.image = NSImage( + systemSymbolName: "chevron.up.chevron.down", + accessibilityDescription: String(localized: "Open editor") + ) + chevron.contentTintColor = .tertiaryLabelColor + chevron.translatesAutoresizingMaskIntoConstraints = false + chevron.setContentHuggingPriority(.required, for: .horizontal) + chevron.setContentCompressionResistancePriority(.required, for: .horizontal) + chevron.imageScaling = .scaleProportionallyDown + chevron.isHidden = true + return chevron + } +} diff --git a/TablePro/Views/Results/Cells/DataGridBaseCellView.swift b/TablePro/Views/Results/Cells/DataGridBaseCellView.swift new file mode 100644 index 000000000..930d9ee15 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridBaseCellView.swift @@ -0,0 +1,221 @@ +// +// DataGridBaseCellView.swift +// TablePro +// + +import AppKit +import QuartzCore + +class DataGridBaseCellView: NSTableCellView { + class var reuseIdentifier: NSUserInterfaceItemIdentifier { + fatalError("subclass must override reuseIdentifier") + } + + let cellTextField: CellTextField + weak var accessoryDelegate: DataGridCellAccessoryDelegate? + var nullDisplayString: String = "" + var cellRow: Int = -1 + var cellColumnIndex: Int = -1 + + private var textFieldTrailingConstraint: NSLayoutConstraint! + + var changeBackgroundColor: NSColor? { + didSet { + if let color = changeBackgroundColor { + backgroundView.layer?.backgroundColor = color.cgColor + backgroundView.isHidden = (backgroundStyle == .emphasized) + } else { + backgroundView.layer?.backgroundColor = nil + backgroundView.isHidden = true + } + } + } + + var isFocusedCell: Bool = false { + didSet { + guard oldValue != isFocusedCell else { return } + updateFocusRing() + } + } + + private(set) lazy var backgroundView: NSView = { + let view = NSView() + view.wantsLayer = true + view.translatesAutoresizingMaskIntoConstraints = false + addSubview(view, positioned: .below, relativeTo: subviews.first) + NSLayoutConstraint.activate([ + view.leadingAnchor.constraint(equalTo: leadingAnchor), + view.trailingAnchor.constraint(equalTo: trailingAnchor), + view.topAnchor.constraint(equalTo: topAnchor), + view.bottomAnchor.constraint(equalTo: bottomAnchor), + ]) + view.isHidden = true + return view + }() + + override init(frame frameRect: NSRect) { + cellTextField = Self.makeTextField() + super.init(frame: frameRect) + commonInit() + } + + required init?(coder: NSCoder) { + cellTextField = Self.makeTextField() + super.init(coder: coder) + commonInit() + } + + private static func makeTextField() -> CellTextField { + let field = CellTextField() + field.font = ThemeEngine.shared.dataGridFonts.regular + field.drawsBackground = false + field.isBordered = false + field.focusRingType = .none + field.lineBreakMode = .byTruncatingTail + field.maximumNumberOfLines = 1 + field.cell?.truncatesLastVisibleLine = true + field.cell?.usesSingleLineMode = true + field.translatesAutoresizingMaskIntoConstraints = false + return field + } + + private func commonInit() { + wantsLayer = true + textField = cellTextField + addSubview(cellTextField) + + textFieldTrailingConstraint = cellTextField.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -4) + + NSLayoutConstraint.activate([ + cellTextField.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 4), + textFieldTrailingConstraint, + cellTextField.centerYAnchor.constraint(equalTo: centerYAnchor), + ]) + + installAccessory() + } + + func configure(content: DataGridCellContent, state: DataGridCellState) { + cellRow = state.row + cellColumnIndex = state.columnIndex + + applyContent(content, isLargeDataset: state.isLargeDataset) + applyVisualState(state) + + cellTextField.isEditable = state.isEditable && !state.visualState.isDeleted + cellTextField.identifier = Self.reuseIdentifier + + textFieldTrailingConstraint.constant = textFieldTrailingInset(for: content, state: state) + + updateAccessoryVisibility(content: content, state: state) + + cellTextField.setAccessibilityLabel(content.accessibilityLabel) + setAccessibilityRowIndexRange(NSRange(location: state.row, length: 1)) + setAccessibilityColumnIndexRange(NSRange(location: state.columnIndex, length: 1)) + } + + func installAccessory() {} + + func updateAccessoryVisibility(content: DataGridCellContent, state: DataGridCellState) {} + + func textFieldTrailingInset(for content: DataGridCellContent, state: DataGridCellState) -> CGFloat { + -4 + } + + private func applyContent(_ content: DataGridCellContent, isLargeDataset: Bool) { + cellTextField.placeholderString = nil + + switch content.placeholder { + case .none: + cellTextField.stringValue = content.displayText + cellTextField.originalValue = content.rawValue + cellTextField.font = ThemeEngine.shared.dataGridFonts.regular + cellTextField.tag = DataGridFontVariant.regular + cellTextField.textColor = .labelColor + + case .null: + cellTextField.stringValue = "" + cellTextField.originalValue = nil + cellTextField.font = ThemeEngine.shared.dataGridFonts.italic + cellTextField.tag = DataGridFontVariant.italic + cellTextField.textColor = .secondaryLabelColor + if !isLargeDataset { + cellTextField.placeholderString = nullDisplayString + } + + case .empty: + cellTextField.stringValue = "" + cellTextField.originalValue = nil + cellTextField.font = ThemeEngine.shared.dataGridFonts.italic + cellTextField.tag = DataGridFontVariant.italic + cellTextField.textColor = .secondaryLabelColor + if !isLargeDataset { + cellTextField.placeholderString = String(localized: "Empty") + } + + case .defaultMarker: + cellTextField.stringValue = "" + cellTextField.originalValue = nil + cellTextField.font = ThemeEngine.shared.dataGridFonts.medium + cellTextField.tag = DataGridFontVariant.medium + cellTextField.textColor = .systemBlue + if !isLargeDataset { + cellTextField.placeholderString = String(localized: "DEFAULT") + } + } + } + + private func applyVisualState(_ state: DataGridCellState) { + CATransaction.begin() + CATransaction.setDisableActions(true) + + if state.visualState.isDeleted { + changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.deleted + } else if state.visualState.isInserted { + changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.inserted + } else if state.visualState.modifiedColumns.contains(state.columnIndex) { + changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.modified + } else { + changeBackgroundColor = nil + } + + isFocusedCell = state.isFocused + + CATransaction.commit() + } + + override var backgroundStyle: NSView.BackgroundStyle { + didSet { + backgroundView.isHidden = (backgroundStyle == .emphasized) || (changeBackgroundColor == nil) + if isFocusedCell { updateFocusRing() } + } + } + + override var focusRingMaskBounds: NSRect { bounds } + + override func drawFocusRingMask() { + NSBezierPath(rect: bounds).fill() + } + + override func draw(_ dirtyRect: NSRect) { + super.draw(dirtyRect) + guard isFocusedCell, backgroundStyle != .emphasized else { return } + NSGraphicsContext.saveGraphicsState() + NSFocusRingPlacement.only.set() + drawFocusRingMask() + NSGraphicsContext.restoreGraphicsState() + } + + override func viewDidChangeEffectiveAppearance() { + super.viewDidChangeEffectiveAppearance() + if isFocusedCell { + needsDisplay = true + } + } + + private func updateFocusRing() { + focusRingType = isFocusedCell ? .exterior : .none + noteFocusRingMaskChanged() + needsDisplay = true + } +} diff --git a/TablePro/Views/Results/Cells/DataGridBlobCellView.swift b/TablePro/Views/Results/Cells/DataGridBlobCellView.swift new file mode 100644 index 000000000..d4f93bf43 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridBlobCellView.swift @@ -0,0 +1,12 @@ +// +// DataGridBlobCellView.swift +// TablePro +// + +import AppKit + +final class DataGridBlobCellView: DataGridChevronCellView { + override class var reuseIdentifier: NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("dataCell.blob") + } +} diff --git a/TablePro/Views/Results/Cells/DataGridBooleanCellView.swift b/TablePro/Views/Results/Cells/DataGridBooleanCellView.swift new file mode 100644 index 000000000..5bbe227ea --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridBooleanCellView.swift @@ -0,0 +1,12 @@ +// +// DataGridBooleanCellView.swift +// TablePro +// + +import AppKit + +final class DataGridBooleanCellView: DataGridChevronCellView { + override class var reuseIdentifier: NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("dataCell.boolean") + } +} diff --git a/TablePro/Views/Results/Cells/DataGridCellAccessoryDelegate.swift b/TablePro/Views/Results/Cells/DataGridCellAccessoryDelegate.swift new file mode 100644 index 000000000..cb700c25b --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridCellAccessoryDelegate.swift @@ -0,0 +1,12 @@ +// +// DataGridCellAccessoryDelegate.swift +// TablePro +// + +import Foundation + +@MainActor +protocol DataGridCellAccessoryDelegate: AnyObject { + func dataGridCellDidClickFKArrow(row: Int, columnIndex: Int) + func dataGridCellDidClickChevron(row: Int, columnIndex: Int) +} diff --git a/TablePro/Views/Results/Cells/DataGridCellContent.swift b/TablePro/Views/Results/Cells/DataGridCellContent.swift new file mode 100644 index 000000000..2b54b72b6 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridCellContent.swift @@ -0,0 +1,28 @@ +// +// DataGridCellContent.swift +// TablePro +// + +import Foundation + +enum DataGridCellPlaceholder: Equatable { + case null + case empty + case defaultMarker +} + +struct DataGridCellContent { + let displayText: String + let rawValue: String? + let placeholder: DataGridCellPlaceholder? + let accessibilityLabel: String +} + +struct DataGridCellState { + let visualState: RowVisualState + let isFocused: Bool + let isEditable: Bool + let isLargeDataset: Bool + let row: Int + let columnIndex: Int +} diff --git a/TablePro/Views/Results/Cells/DataGridCellKind.swift b/TablePro/Views/Results/Cells/DataGridCellKind.swift new file mode 100644 index 000000000..e903394c2 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridCellKind.swift @@ -0,0 +1,16 @@ +// +// DataGridCellKind.swift +// TablePro +// + +import Foundation + +enum DataGridCellKind: Equatable { + case text + case foreignKey + case dropdown + case boolean + case date + case json + case blob +} diff --git a/TablePro/Views/Results/Cells/DataGridCellRegistry.swift b/TablePro/Views/Results/Cells/DataGridCellRegistry.swift new file mode 100644 index 000000000..8186dbc83 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridCellRegistry.swift @@ -0,0 +1,142 @@ +// +// DataGridCellRegistry.swift +// TablePro +// + +import AppKit +import Foundation + +@MainActor +final class DataGridCellRegistry { + weak var accessoryDelegate: DataGridCellAccessoryDelegate? + + private(set) var nullDisplayString: String + private var settingsObserver: NSObjectProtocol? + + private let rowNumberCellIdentifier = NSUserInterfaceItemIdentifier("RowNumberCellView") + + init() { + nullDisplayString = AppSettingsManager.shared.dataGrid.nullDisplay + settingsObserver = NotificationCenter.default.addObserver( + forName: .dataGridSettingsDidChange, + object: nil, + queue: .main + ) { [weak self] _ in + Task { @MainActor [weak self] in + self?.nullDisplayString = AppSettingsManager.shared.dataGrid.nullDisplay + } + } + } + + deinit { + if let observer = settingsObserver { + NotificationCenter.default.removeObserver(observer) + } + } + + func resolveKind( + columnIndex: Int, + columnType: ColumnType?, + isFKColumn: Bool, + isDropdownColumn: Bool + ) -> DataGridCellKind { + if isFKColumn { return .foreignKey } + if isDropdownColumn { return .dropdown } + if let type = columnType { + if type.isBooleanType { return .boolean } + if type.isDateType { return .date } + if type.isJsonType { return .json } + if type.isBlobType { return .blob } + } + return .text + } + + func dequeueCell(of kind: DataGridCellKind, in tableView: NSTableView) -> DataGridBaseCellView { + let identifier: NSUserInterfaceItemIdentifier + let cellType: DataGridBaseCellView.Type + + switch kind { + case .text: + identifier = DataGridTextCellView.reuseIdentifier + cellType = DataGridTextCellView.self + case .foreignKey: + identifier = DataGridForeignKeyCellView.reuseIdentifier + cellType = DataGridForeignKeyCellView.self + case .dropdown: + identifier = DataGridDropdownCellView.reuseIdentifier + cellType = DataGridDropdownCellView.self + case .boolean: + identifier = DataGridBooleanCellView.reuseIdentifier + cellType = DataGridBooleanCellView.self + case .date: + identifier = DataGridDateCellView.reuseIdentifier + cellType = DataGridDateCellView.self + case .json: + identifier = DataGridJsonCellView.reuseIdentifier + cellType = DataGridJsonCellView.self + case .blob: + identifier = DataGridBlobCellView.reuseIdentifier + cellType = DataGridBlobCellView.self + } + + if let reused = tableView.makeView(withIdentifier: identifier, owner: nil) as? DataGridBaseCellView { + reused.accessoryDelegate = accessoryDelegate + reused.nullDisplayString = nullDisplayString + return reused + } + + let cell = cellType.init(frame: .zero) + cell.identifier = identifier + cell.accessoryDelegate = accessoryDelegate + cell.nullDisplayString = nullDisplayString + return cell + } + + func makeRowNumberCell( + in tableView: NSTableView, + row: Int, + cachedRowCount: Int, + visualState: RowVisualState + ) -> NSView { + let cellView: NSTableCellView + let cell: NSTextField + + if let reused = tableView.makeView(withIdentifier: rowNumberCellIdentifier, owner: nil) as? NSTableCellView, + let textField = reused.textField { + cellView = reused + cell = textField + cell.font = ThemeEngine.shared.dataGridFonts.rowNumber + } else { + cellView = NSTableCellView() + cellView.identifier = rowNumberCellIdentifier + + cell = NSTextField(labelWithString: "") + cell.alignment = .right + cell.font = ThemeEngine.shared.dataGridFonts.rowNumber + cell.tag = DataGridFontVariant.rowNumber + cell.textColor = .secondaryLabelColor + cell.translatesAutoresizingMaskIntoConstraints = false + + cellView.textField = cell + cellView.addSubview(cell) + + NSLayoutConstraint.activate([ + cell.leadingAnchor.constraint(equalTo: cellView.leadingAnchor, constant: 4), + cell.trailingAnchor.constraint(equalTo: cellView.trailingAnchor, constant: -4), + cell.centerYAnchor.constraint(equalTo: cellView.centerYAnchor), + ]) + } + + guard row >= 0 && row < cachedRowCount else { + cell.stringValue = "" + return cellView + } + + cell.stringValue = "\(row + 1)" + cell.textColor = visualState.isDeleted ? ThemeEngine.shared.colors.dataGrid.deletedText : .secondaryLabelColor + cellView.setAccessibilityLabel(String(format: String(localized: "Row %d"), row + 1)) + cellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) + + return cellView + } +} diff --git a/TablePro/Views/Results/Cells/DataGridChevronCellView.swift b/TablePro/Views/Results/Cells/DataGridChevronCellView.swift new file mode 100644 index 000000000..0bf8947cb --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridChevronCellView.swift @@ -0,0 +1,44 @@ +// +// DataGridChevronCellView.swift +// TablePro +// + +import AppKit + +class DataGridChevronCellView: DataGridBaseCellView { + private lazy var chevronButton: CellChevronButton = AccessoryButtonFactory.makeChevronButton() + + override func installAccessory() { + addSubview(chevronButton) + NSLayoutConstraint.activate([ + chevronButton.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -4), + chevronButton.centerYAnchor.constraint(equalTo: centerYAnchor), + chevronButton.widthAnchor.constraint(equalToConstant: 10), + chevronButton.heightAnchor.constraint(equalToConstant: 12), + ]) + chevronButton.target = self + chevronButton.action = #selector(handleChevronClick(_:)) + } + + override func updateAccessoryVisibility(content: DataGridCellContent, state: DataGridCellState) { + let show = state.isEditable && !state.visualState.isDeleted + chevronButton.isHidden = !show + if show { + chevronButton.cellRow = state.row + chevronButton.cellColumnIndex = state.columnIndex + } else { + chevronButton.cellRow = -1 + chevronButton.cellColumnIndex = -1 + } + } + + override func textFieldTrailingInset(for content: DataGridCellContent, state: DataGridCellState) -> CGFloat { + let show = state.isEditable && !state.visualState.isDeleted + return show ? -18 : -4 + } + + @objc + private func handleChevronClick(_ sender: CellChevronButton) { + accessoryDelegate?.dataGridCellDidClickChevron(row: sender.cellRow, columnIndex: sender.cellColumnIndex) + } +} diff --git a/TablePro/Views/Results/Cells/DataGridDateCellView.swift b/TablePro/Views/Results/Cells/DataGridDateCellView.swift new file mode 100644 index 000000000..252b9c8db --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridDateCellView.swift @@ -0,0 +1,12 @@ +// +// DataGridDateCellView.swift +// TablePro +// + +import AppKit + +final class DataGridDateCellView: DataGridChevronCellView { + override class var reuseIdentifier: NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("dataCell.date") + } +} diff --git a/TablePro/Views/Results/Cells/DataGridDropdownCellView.swift b/TablePro/Views/Results/Cells/DataGridDropdownCellView.swift new file mode 100644 index 000000000..3af7116a3 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridDropdownCellView.swift @@ -0,0 +1,12 @@ +// +// DataGridDropdownCellView.swift +// TablePro +// + +import AppKit + +final class DataGridDropdownCellView: DataGridChevronCellView { + override class var reuseIdentifier: NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("dataCell.dropdown") + } +} diff --git a/TablePro/Views/Results/Cells/DataGridForeignKeyCellView.swift b/TablePro/Views/Results/Cells/DataGridForeignKeyCellView.swift new file mode 100644 index 000000000..8f7d6f2a5 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridForeignKeyCellView.swift @@ -0,0 +1,52 @@ +// +// DataGridForeignKeyCellView.swift +// TablePro +// + +import AppKit + +final class DataGridForeignKeyCellView: DataGridBaseCellView { + override class var reuseIdentifier: NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("dataCell.foreignKey") + } + + private lazy var fkButton: FKArrowButton = AccessoryButtonFactory.makeFKArrowButton() + + override func installAccessory() { + addSubview(fkButton) + NSLayoutConstraint.activate([ + fkButton.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -4), + fkButton.centerYAnchor.constraint(equalTo: centerYAnchor), + fkButton.widthAnchor.constraint(equalToConstant: 16), + fkButton.heightAnchor.constraint(equalToConstant: 16), + ]) + fkButton.target = self + fkButton.action = #selector(handleFKClick(_:)) + } + + override func updateAccessoryVisibility(content: DataGridCellContent, state: DataGridCellState) { + let show = isAccessoryVisible(for: content) + fkButton.isHidden = !show + if show { + fkButton.fkRow = state.row + fkButton.fkColumnIndex = state.columnIndex + } else { + fkButton.fkRow = -1 + fkButton.fkColumnIndex = -1 + } + } + + override func textFieldTrailingInset(for content: DataGridCellContent, state: DataGridCellState) -> CGFloat { + isAccessoryVisible(for: content) ? -22 : -4 + } + + private func isAccessoryVisible(for content: DataGridCellContent) -> Bool { + guard let raw = content.rawValue else { return false } + return !raw.isEmpty + } + + @objc + private func handleFKClick(_ sender: FKArrowButton) { + accessoryDelegate?.dataGridCellDidClickFKArrow(row: sender.fkRow, columnIndex: sender.fkColumnIndex) + } +} diff --git a/TablePro/Views/Results/Cells/DataGridJsonCellView.swift b/TablePro/Views/Results/Cells/DataGridJsonCellView.swift new file mode 100644 index 000000000..4873a7f46 --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridJsonCellView.swift @@ -0,0 +1,12 @@ +// +// DataGridJsonCellView.swift +// TablePro +// + +import AppKit + +final class DataGridJsonCellView: DataGridChevronCellView { + override class var reuseIdentifier: NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("dataCell.json") + } +} diff --git a/TablePro/Views/Results/Cells/DataGridTextCellView.swift b/TablePro/Views/Results/Cells/DataGridTextCellView.swift new file mode 100644 index 000000000..a5850bf7a --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridTextCellView.swift @@ -0,0 +1,12 @@ +// +// DataGridTextCellView.swift +// TablePro +// + +import AppKit + +final class DataGridTextCellView: DataGridBaseCellView { + override class var reuseIdentifier: NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("dataCell.text") + } +} From 0d3468c97251ce0780f6a28756a246df5ea7bbbc Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 12:12:21 +0700 Subject: [PATCH 03/15] refactor(datagrid): persistent column pool with slot-based identifiers --- TablePro/Models/UI/ColumnIdentitySchema.swift | 47 ++++--- .../Views/Results/DataGridColumnPool.swift | 119 ++++++++++++++++++ 2 files changed, 150 insertions(+), 16 deletions(-) create mode 100644 TablePro/Views/Results/DataGridColumnPool.swift diff --git a/TablePro/Models/UI/ColumnIdentitySchema.swift b/TablePro/Models/UI/ColumnIdentitySchema.swift index 7c55344be..62671ecfb 100644 --- a/TablePro/Models/UI/ColumnIdentitySchema.swift +++ b/TablePro/Models/UI/ColumnIdentitySchema.swift @@ -7,31 +7,33 @@ import AppKit struct ColumnIdentitySchema: Equatable { static let rowNumberIdentifier = NSUserInterfaceItemIdentifier("__rowNumber__") + static let dataColumnPrefix = "dataColumn-" let identifiers: [NSUserInterfaceItemIdentifier] - let isNameBased: Bool + let columnNames: [String] + private let indexByRawIdentifier: [String: Int] + private let slotByColumnName: [String: Int] init(columns: [String]) { - let canUseNames = Set(columns).count == columns.count - && !columns.contains(Self.rowNumberIdentifier.rawValue) - - if canUseNames { - self.identifiers = columns.map { NSUserInterfaceItemIdentifier($0) } - self.isNameBased = true - } else { - self.identifiers = columns.indices.map { - NSUserInterfaceItemIdentifier("col_\($0)") - } - self.isNameBased = false + self.columnNames = columns + self.identifiers = columns.indices.map { + NSUserInterfaceItemIdentifier("\(Self.dataColumnPrefix)\($0)") } - var map: [String: Int] = [:] - map.reserveCapacity(self.identifiers.count) + var rawMap: [String: Int] = [:] + rawMap.reserveCapacity(self.identifiers.count) for (index, identifier) in self.identifiers.enumerated() { - map[identifier.rawValue] = index + rawMap[identifier.rawValue] = index + } + self.indexByRawIdentifier = rawMap + + var nameMap: [String: Int] = [:] + nameMap.reserveCapacity(columns.count) + for (index, name) in columns.enumerated() { + nameMap[name] = index } - self.indexByRawIdentifier = map + self.slotByColumnName = nameMap } static let empty = ColumnIdentitySchema(columns: []) @@ -44,4 +46,17 @@ struct ColumnIdentitySchema: Equatable { func dataIndex(from identifier: NSUserInterfaceItemIdentifier) -> Int? { indexByRawIdentifier[identifier.rawValue] } + + func columnName(for dataIndex: Int) -> String? { + guard dataIndex >= 0, dataIndex < columnNames.count else { return nil } + return columnNames[dataIndex] + } + + func dataIndex(forColumnName name: String) -> Int? { + slotByColumnName[name] + } + + static func slotIdentifier(_ slot: Int) -> NSUserInterfaceItemIdentifier { + NSUserInterfaceItemIdentifier("\(dataColumnPrefix)\(slot)") + } } diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift new file mode 100644 index 000000000..5d075b3f4 --- /dev/null +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -0,0 +1,119 @@ +// +// DataGridColumnPool.swift +// TablePro +// + +import AppKit + +@MainActor +final class DataGridColumnPool { + private var pooledColumns: [NSTableColumn] = [] + private weak var attachedTableView: NSTableView? + + var totalSlots: Int { pooledColumns.count } + + func attach(to tableView: NSTableView) { + attachedTableView = tableView + } + + func reconcile( + tableView: NSTableView, + schema: ColumnIdentitySchema, + columnTypes: [ColumnType], + savedLayout: ColumnLayoutState?, + widthCalculator: (String, Int) -> CGFloat, + sortKeyForColumnName: (String) -> String + ) { + attach(to: tableView) + let visibleCount = schema.columnNames.count + growPoolIfNeeded(to: visibleCount, in: tableView) + + let willRestoreWidths = !(savedLayout?.columnWidths.isEmpty ?? true) + let hidden = savedLayout?.hiddenColumns ?? [] + + for slot in 0.. Int? { + schema.dataIndex(forColumnName: name) + } + + private func growPoolIfNeeded(to count: Int, in tableView: NSTableView) { + while pooledColumns.count < count { + let slot = pooledColumns.count + let column = NSTableColumn(identifier: ColumnIdentitySchema.slotIdentifier(slot)) + column.minWidth = 30 + column.resizingMask = .userResizingMask + column.isEditable = true + pooledColumns.append(column) + tableView.addTableColumn(column) + } + } + + private func configureColumn( + _ column: NSTableColumn, + name: String, + columnType: ColumnType?, + width: CGFloat, + sortKey: String + ) { + let cell = SortableHeaderCell(textCell: name) + cell.font = column.headerCell.font + cell.alignment = column.headerCell.alignment + column.headerCell = cell + + if let typeName = columnType?.rawType ?? columnType?.displayName { + column.headerToolTip = "\(name) (\(typeName))" + } else { + column.headerToolTip = name + } + column.headerCell.setAccessibilityLabel( + String(format: String(localized: "Column: %@"), name) + ) + column.width = width + column.sortDescriptorPrototype = NSSortDescriptor(key: sortKey, ascending: true) + } + + private func applyColumnOrder( + _ order: [String], + in tableView: NSTableView, + schema: ColumnIdentitySchema + ) { + let rowNumberIsPresent = tableView.tableColumns.first?.identifier == ColumnIdentitySchema.rowNumberIdentifier + let baseOffset = rowNumberIsPresent ? 1 : 0 + + for (targetPosition, columnName) in order.enumerated() { + guard let slot = schema.dataIndex(forColumnName: columnName) else { continue } + let identifier = ColumnIdentitySchema.slotIdentifier(slot) + guard let currentIndex = tableView.tableColumns.firstIndex(where: { $0.identifier == identifier }) else { + continue + } + let desiredIndex = baseOffset + targetPosition + if currentIndex != desiredIndex { + tableView.moveColumn(currentIndex, toColumn: desiredIndex) + } + } + } +} From bd34421063b1e92bfc5271745ab789e877d3af1c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 12:27:04 +0700 Subject: [PATCH 04/15] refactor(datagrid): wire registry + pool, delete mega-cell --- .../Views/Results/DataGridCellFactory.swift | 378 +----------------- TablePro/Views/Results/DataGridCellView.swift | 80 ---- .../Views/Results/DataGridColumnPool.swift | 16 +- .../Views/Results/DataGridCoordinator.swift | 16 + TablePro/Views/Results/DataGridView.swift | 231 ++--------- .../Extensions/DataGridView+Click.swift | 27 +- .../Extensions/DataGridView+Columns.swift | 60 ++- .../Models/UI/ColumnIdentitySchemaTests.swift | 206 ++++------ 8 files changed, 170 insertions(+), 844 deletions(-) delete mode 100644 TablePro/Views/Results/DataGridCellView.swift diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index 9d4b302e0..6c7044c37 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -2,387 +2,28 @@ // DataGridCellFactory.swift // TablePro // -// Factory for creating and configuring data grid cells. -// Extracted from DataGridView coordinator for better maintainability. -// import AppKit -import QuartzCore -import os - -@MainActor -final class FKArrowButton: NSButton { - var fkRow: Int = -1 - var fkColumnIndex: Int = -1 -} - -@MainActor -final class CellChevronButton: NSButton { - var cellRow: Int = -1 - var cellColumnIndex: Int = -1 -} +import Foundation @MainActor final class DataGridCellFactory { - private let cellIdentifier = NSUserInterfaceItemIdentifier("DataCell") - private let rowNumberCellIdentifier = NSUserInterfaceItemIdentifier("RowNumberCell") - private let largeDatasetThreshold = 5_000 - - private var nullDisplayString: String = AppSettingsManager.shared.dataGrid.nullDisplay - private var settingsObserver: NSObjectProtocol? - - init() { - settingsObserver = NotificationCenter.default.addObserver( - forName: .dataGridSettingsDidChange, - object: nil, - queue: .main - ) { [weak self] _ in - Task { @MainActor [weak self] in - self?.nullDisplayString = AppSettingsManager.shared.dataGrid.nullDisplay - } - } - } - - deinit { - if let observer = settingsObserver { - NotificationCenter.default.removeObserver(observer) - } - } - - // MARK: - Row Number Cell - - func makeRowNumberCell( - tableView: NSTableView, - row: Int, - cachedRowCount: Int, - visualState: RowVisualState - ) -> NSView { - let cellViewId = NSUserInterfaceItemIdentifier("RowNumberCellView") - let cellView: NSTableCellView - let cell: NSTextField - - if let reused = tableView.makeView(withIdentifier: cellViewId, owner: nil) as? NSTableCellView, - let textField = reused.textField { - cellView = reused - cell = textField - cell.font = ThemeEngine.shared.dataGridFonts.rowNumber - } else { - cellView = NSTableCellView() - cellView.identifier = cellViewId - - cell = NSTextField(labelWithString: "") - cell.alignment = .right - cell.font = ThemeEngine.shared.dataGridFonts.rowNumber - cell.tag = DataGridFontVariant.rowNumber - cell.textColor = .secondaryLabelColor - cell.translatesAutoresizingMaskIntoConstraints = false - - cellView.textField = cell - cellView.addSubview(cell) - - NSLayoutConstraint.activate([ - cell.leadingAnchor.constraint(equalTo: cellView.leadingAnchor, constant: 4), - cell.trailingAnchor.constraint(equalTo: cellView.trailingAnchor, constant: -4), - cell.centerYAnchor.constraint(equalTo: cellView.centerYAnchor), - ]) - } - - guard row >= 0 && row < cachedRowCount else { - cell.stringValue = "" - return cellView - } - - cell.stringValue = "\(row + 1)" - cell.textColor = visualState.isDeleted ? ThemeEngine.shared.colors.dataGrid.deletedText : .secondaryLabelColor - cellView.setAccessibilityLabel(String(format: String(localized: "Row %d"), row + 1)) - cellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) - - return cellView - } - - // MARK: - Data Cell - - func makeDataCell( - tableView: NSTableView, - row: Int, - columnIndex: Int, - displayValue: String?, - rawValue: String?, - visualState: RowVisualState, - isEditable: Bool, - isLargeDataset: Bool, - isFocused: Bool, - isDropdown: Bool = false, - isFKColumn: Bool = false, - fkArrowTarget: AnyObject? = nil, - fkArrowAction: Selector? = nil, - chevronTarget: AnyObject? = nil, - chevronAction: Selector? = nil, - delegate: NSTextFieldDelegate - ) -> NSView { - let gridCellView: DataGridCellView - let cell: NSTextField - - let __tA = CFAbsoluteTimeGetCurrent() - if let reused = tableView.makeView(withIdentifier: cellIdentifier, owner: nil) as? DataGridCellView, - let textField = reused.textField { - gridCellView = reused - cell = textField - } else { - gridCellView = DataGridCellView() - gridCellView.identifier = cellIdentifier - gridCellView.wantsLayer = true - - cell = CellTextField() - cell.font = ThemeEngine.shared.dataGridFonts.regular - cell.drawsBackground = false - cell.isBordered = false - cell.focusRingType = .none - cell.lineBreakMode = .byTruncatingTail - cell.maximumNumberOfLines = 1 - cell.cell?.truncatesLastVisibleLine = true - cell.cell?.usesSingleLineMode = true - cell.translatesAutoresizingMaskIntoConstraints = false - - gridCellView.textField = cell - gridCellView.addSubview(cell) - - let fkButton = createFKArrowButton() - gridCellView.addSubview(fkButton) - gridCellView.fkArrowButton = fkButton - - let chevron = createChevronButton() - gridCellView.addSubview(chevron) - gridCellView.chevronButton = chevron - - let trailing = cell.trailingAnchor.constraint(equalTo: gridCellView.trailingAnchor, constant: -4) - gridCellView.textFieldTrailing = trailing - - NSLayoutConstraint.activate([ - cell.leadingAnchor.constraint(equalTo: gridCellView.leadingAnchor, constant: 4), - trailing, - cell.centerYAnchor.constraint(equalTo: gridCellView.centerYAnchor), - - fkButton.trailingAnchor.constraint(equalTo: gridCellView.trailingAnchor, constant: -4), - fkButton.centerYAnchor.constraint(equalTo: gridCellView.centerYAnchor), - fkButton.widthAnchor.constraint(equalToConstant: 16), - fkButton.heightAnchor.constraint(equalToConstant: 16), - - chevron.trailingAnchor.constraint(equalTo: gridCellView.trailingAnchor, constant: -4), - chevron.centerYAnchor.constraint(equalTo: gridCellView.centerYAnchor), - chevron.widthAnchor.constraint(equalToConstant: 10), - chevron.heightAnchor.constraint(equalToConstant: 12), - ]) - } - let __aMs = (CFAbsoluteTimeGetCurrent() - __tA) * 1000 - - let __tB = CFAbsoluteTimeGetCurrent() - cell.lineBreakMode = .byTruncatingTail - cell.maximumNumberOfLines = 1 - cell.cell?.truncatesLastVisibleLine = true - cell.cell?.usesSingleLineMode = true - - let showFK = isFKColumn && rawValue != nil && rawValue?.isEmpty != true - let showChevron = isDropdown - - if let fkButton = gridCellView.fkArrowButton { - if showFK { - fkButton.isHidden = false - fkButton.target = fkArrowTarget - fkButton.action = fkArrowAction - fkButton.fkRow = row - fkButton.fkColumnIndex = columnIndex - } else { - fkButton.isHidden = true - fkButton.target = nil - fkButton.action = nil - fkButton.fkRow = -1 - fkButton.fkColumnIndex = -1 - } - } - - if let chevron = gridCellView.chevronButton { - if showChevron { - chevron.isHidden = false - chevron.target = chevronTarget - chevron.action = chevronAction - chevron.cellRow = row - chevron.cellColumnIndex = columnIndex - } else { - chevron.isHidden = true - chevron.target = nil - chevron.action = nil - chevron.cellRow = -1 - chevron.cellColumnIndex = -1 - } - } - - if showFK { - gridCellView.textFieldTrailing?.constant = -22 - } else if showChevron { - gridCellView.textFieldTrailing?.constant = -18 - } else { - gridCellView.textFieldTrailing?.constant = -4 - } - - cell.isEditable = isEditable - cell.delegate = delegate - cell.identifier = cellIdentifier - - let isDeleted = visualState.isDeleted - let isInserted = visualState.isInserted - let isModified = visualState.modifiedColumns.contains(columnIndex) - let __bMs = (CFAbsoluteTimeGetCurrent() - __tB) * 1000 - - let __tC = CFAbsoluteTimeGetCurrent() - configureTextContent(cell: cell, displayValue: displayValue, rawValue: rawValue, isLargeDataset: isLargeDataset) - let __cMs = (CFAbsoluteTimeGetCurrent() - __tC) * 1000 - - let __tD = CFAbsoluteTimeGetCurrent() - CATransaction.begin() - CATransaction.setDisableActions(true) - - if isDeleted { - gridCellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.deleted - } else if isInserted { - gridCellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.inserted - } else if isModified { - gridCellView.changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.modified - } else { - gridCellView.changeBackgroundColor = nil - } - - gridCellView.isFocusedCell = isFocused - - CATransaction.commit() - let __dMs = (CFAbsoluteTimeGetCurrent() - __tD) * 1000 - - let __tE = CFAbsoluteTimeGetCurrent() - let accessibilityValue = rawValue ?? String(localized: "NULL") - cell.setAccessibilityLabel( - String(format: String(localized: "Row %d, column %d: %@"), row + 1, columnIndex + 1, accessibilityValue) - ) - gridCellView.setAccessibilityRowIndexRange(NSRange(location: row, length: 1)) - gridCellView.setAccessibilityColumnIndexRange(NSRange(location: columnIndex, length: 1)) - let __eMs = (CFAbsoluteTimeGetCurrent() - __tE) * 1000 - - ViewForStats.recordSection(a: __aMs, b: __bMs, c: __cMs, d: __dMs, e: __eMs) - - return gridCellView - } - - // MARK: - Button Creation - - private func createFKArrowButton() -> FKArrowButton { - let button = FKArrowButton() - button.bezelStyle = .inline - button.isBordered = false - button.image = NSImage( - systemSymbolName: "arrow.right.circle.fill", - accessibilityDescription: String(localized: "Navigate to referenced row") - ) - button.contentTintColor = .tertiaryLabelColor - button.translatesAutoresizingMaskIntoConstraints = false - button.setContentHuggingPriority(.required, for: .horizontal) - button.setContentCompressionResistancePriority(.required, for: .horizontal) - button.imageScaling = .scaleProportionallyDown - button.isHidden = true - return button - } - - private func createChevronButton() -> CellChevronButton { - let chevron = CellChevronButton() - chevron.bezelStyle = .inline - chevron.isBordered = false - chevron.image = NSImage( - systemSymbolName: "chevron.up.chevron.down", - accessibilityDescription: String(localized: "Open editor") - ) - chevron.contentTintColor = .tertiaryLabelColor - chevron.translatesAutoresizingMaskIntoConstraints = false - chevron.setContentHuggingPriority(.required, for: .horizontal) - chevron.setContentCompressionResistancePriority(.required, for: .horizontal) - chevron.imageScaling = .scaleProportionallyDown - chevron.isHidden = true - return chevron - } - - // MARK: - Cell Text Content - - private func configureTextContent( - cell: NSTextField, - displayValue: String?, - rawValue: String?, - isLargeDataset: Bool - ) { - cell.placeholderString = nil - - let cellTextField = cell as? CellTextField - - if rawValue == nil { - cell.stringValue = "" - cellTextField?.originalValue = nil - cell.font = ThemeEngine.shared.dataGridFonts.italic - cell.tag = DataGridFontVariant.italic - if !isLargeDataset { - cell.placeholderString = nullDisplayString - } - cell.textColor = .secondaryLabelColor - } else if rawValue == "__DEFAULT__" { - cell.stringValue = "" - cellTextField?.originalValue = nil - cell.font = ThemeEngine.shared.dataGridFonts.medium - cell.tag = DataGridFontVariant.medium - if !isLargeDataset { - cell.placeholderString = "DEFAULT" - } - cell.textColor = .systemBlue - } else if rawValue == "" { - cell.stringValue = "" - cellTextField?.originalValue = nil - cell.font = ThemeEngine.shared.dataGridFonts.italic - cell.tag = DataGridFontVariant.italic - if !isLargeDataset { - cell.placeholderString = "Empty" - } - cell.textColor = .secondaryLabelColor - } else { - cell.stringValue = displayValue ?? "" - cellTextField?.originalValue = rawValue - cell.textColor = .labelColor - cell.font = ThemeEngine.shared.dataGridFonts.regular - cell.tag = DataGridFontVariant.regular - } - } - - // MARK: - Column Width Calculation - - /// Minimum column width private static let minColumnWidth: CGFloat = 60 - /// Maximum column width - prevents overly wide columns private static let maxColumnWidth: CGFloat = 800 - /// Number of rows to sample for width calculation (for performance) private static let sampleRowCount = 30 - /// Maximum characters to consider per cell for width estimation private static let maxMeasureChars = 50 - /// Font for measuring header + private var headerFont: NSFont { NSFont.systemFont(ofSize: 13, weight: .semibold) } - /// Calculate column width based on header name only (used for initial display) func calculateColumnWidth(for columnName: String) -> CGFloat { let attributes: [NSAttributedString.Key: Any] = [.font: headerFont] let size = (columnName as NSString).size(withAttributes: attributes) - let width = size.width + 48 // padding for sort indicator + margins + let width = size.width + 48 return min(max(width, Self.minColumnWidth), Self.maxColumnWidth) } - /// Calculate optimal column width based on header and cell content. - /// - /// Since the cell font is monospaced, we avoid per-row CoreText measurement - /// and instead multiply character count by the pre-computed glyph advance width. - /// This reduces the cost from O(sampleRows * CoreText) to O(sampleRows * 1). func calculateOptimalColumnWidth( for columnName: String, columnIndex: Int, @@ -412,8 +53,6 @@ final class DataGridCellFactory { return min(max(maxWidth, Self.minColumnWidth), Self.maxColumnWidth) } - /// Calculate column width to fit content without max-width or max-chars caps. - /// Used for user-initiated "Size to Fit" (double-click divider, context menu). func calculateFitToContentWidth( for columnName: String, columnIndex: Int, @@ -440,8 +79,6 @@ final class DataGridCellFactory { } } -// MARK: - NSFont Extension - extension NSFont { func withTraits(_ traits: NSFontDescriptor.SymbolicTraits) -> NSFont { let descriptor = fontDescriptor.withSymbolicTraits(traits) @@ -449,11 +86,7 @@ extension NSFont { } } -// MARK: - String Extension for Cell Display - internal extension String { - /// Whether the string contains any Unicode line-break character - /// (LF, CR, VT, FF, NEL, LS, PS). Uses NSString UTF-16 loop for O(1) per-char access. var containsLineBreak: Bool { let nsString = self as NSString let length = nsString.length @@ -468,17 +101,12 @@ internal extension String { return false } - /// Sanitize string for single-line cell display by replacing line-break characters with spaces. - /// Covers: LF (0x0A), CR (0x0D), VT (0x0B), FF (0x0C), NEL (0x85), LS (0x2028), PS (0x2029). - /// Uses NSString UTF-16 loop for O(1) per-character access (project convention for large strings). var sanitizedForCellDisplay: String { let nsString = self as NSString let length = nsString.length guard length > 0 else { return self } - guard containsLineBreak else { return self } - // Slow path: build new string with line breaks replaced by spaces let mutable = NSMutableString(capacity: length) for i in 0.. CGFloat, - sortKeyForColumnName: (String) -> String + isEditable: Bool, + hiddenColumnNames: Set, + widthCalculator: (String, Int) -> CGFloat ) { attach(to: tableView) let visibleCount = schema.columnNames.count growPoolIfNeeded(to: visibleCount, in: tableView) let willRestoreWidths = !(savedLayout?.columnWidths.isEmpty ?? true) - let hidden = savedLayout?.hiddenColumns ?? [] + let hiddenFromLayout = savedLayout?.hiddenColumns ?? [] for slot in 0.. Int { @@ -506,38 +365,6 @@ struct DataGridView: NSViewRepresentable { tableColumnIndex - 1 } - private static func applyColumnOrder( - _ order: [String], - to tableView: NSTableView, - schema: ColumnIdentitySchema, - columns: [String] - ) { - guard Set(order) == Set(columns) else { return } - - var columnByName: [String: NSTableColumn] = [:] - for col in tableView.tableColumns - where col.identifier != ColumnIdentitySchema.rowNumberIdentifier { - if let idx = schema.dataIndex(from: col.identifier), idx < columns.count { - columnByName[columns[idx]] = col - } - } - - for (targetDataIndex, columnName) in order.enumerated() { - guard let desired = columnByName[columnName] else { continue } - let targetTableIndex = tableColumnIndex(for: targetDataIndex) - guard targetTableIndex < tableView.numberOfColumns else { continue } - - let current = tableView.tableColumns - var currentIndex = -1 - for i in targetTableIndex..= 0, currentIndex != targetTableIndex else { continue } - tableView.moveColumn(currentIndex, toColumn: targetTableIndex) - } - } - static func dismantleNSView(_ nsView: NSScrollView, coordinator: TableViewCoordinator) { coordinator.overlayEditor?.dismiss(commit: false) coordinator.persistColumnLayoutToStorage() diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index ad0a09389..0828599fc 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -70,29 +70,14 @@ extension TableViewCoordinator { // MARK: - Chevron Click - @objc func handleChevronClick(_ sender: NSButton) { - guard let button = sender as? CellChevronButton, - isEditable else { return } - - let row = button.cellRow - let columnIndex = button.cellColumnIndex + func handleChevronAction(row: Int, columnIndex: Int) { + guard isEditable else { return } guard row >= 0, columnIndex >= 0 else { return } guard !changeManager.isRowDeleted(row) else { return } - - // Walk up the view hierarchy to find the NSTableView - var current: NSView? = button.superview - var tableView: NSTableView? - while let view = current { - if let tv = view as? NSTableView { - tableView = tv - break - } - current = view.superview - } guard let tableView else { return } + let column = DataGridView.tableColumnIndex(for: columnIndex) - // Structure view: dropdown and type picker columns take priority if let dropdownCols = dropdownColumns, dropdownCols.contains(columnIndex) { showDropdownMenu(tableView: tableView, row: row, column: column, columnIndex: columnIndex) return @@ -126,11 +111,7 @@ extension TableViewCoordinator { // MARK: - FK Navigation - @objc func handleFKArrowClick(_ sender: NSButton) { - guard let button = sender as? FKArrowButton else { return } - let row = button.fkRow - let columnIndex = button.fkColumnIndex - + func handleFKArrowAction(row: Int, columnIndex: Int) { let tableRows = tableRowsProvider() guard row >= 0 && row < cachedRowCount, columnIndex >= 0 && columnIndex < tableRows.columns.count else { return } diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index 51688c9d5..c55c64592 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -68,8 +68,8 @@ extension TableViewCoordinator { let displayCount = sortedIDs?.count ?? tableRows.count if column.identifier == ColumnIdentitySchema.rowNumberIdentifier { - let __view: NSView? = cellFactory.makeRowNumberCell( - tableView: tableView, + let __view: NSView? = cellRegistry.makeRowNumberCell( + in: tableView, row: row, cachedRowCount: displayCount, visualState: visualState(for: row) @@ -119,36 +119,52 @@ extension TableViewCoordinator { let isDropdown = dropdownColumns?.contains(columnIndex) == true let isTypePicker = typePickerColumns?.contains(columnIndex) == true - let isEnumOrSet = enumOrSetColumns.contains(columnIndex) let isFKColumn = fkColumns.contains(columnIndex) + let resolvedFK = isFKColumn && !isDropdown && !isTypePicker + let resolvedDropdown = isEditable && (isDropdown || isTypePicker || isEnumOrSet) - let hasSpecialEditor: Bool = { - guard columnIndex < tableRows.columnTypes.count else { return false } - let ct = tableRows.columnTypes[columnIndex] - return ct.isBooleanType || ct.isDateType || ct.isJsonType || ct.isBlobType - }() - - let __view: NSView? = cellFactory.makeDataCell( - tableView: tableView, - row: row, + let kind = cellRegistry.resolveKind( columnIndex: columnIndex, - displayValue: formattedValue, + columnType: columnType, + isFKColumn: resolvedFK, + isDropdownColumn: resolvedDropdown + ) + + let accessibilityValue = rawValue ?? String(localized: "NULL") + let content = DataGridCellContent( + displayText: formattedValue ?? "", rawValue: rawValue, + placeholder: placeholderKind(for: rawValue), + accessibilityLabel: String( + format: String(localized: "Row %d, column %d: %@"), + row + 1, + columnIndex + 1, + accessibilityValue + ) + ) + let cellState = DataGridCellState( visualState: state, + isFocused: isFocused, isEditable: isEditable && !state.isDeleted, isLargeDataset: isLargeDataset, - isFocused: isFocused, - isDropdown: isEditable && (isDropdown || isTypePicker || isEnumOrSet || hasSpecialEditor), - isFKColumn: isFKColumn && !isDropdown && !(typePickerColumns?.contains(columnIndex) == true), - fkArrowTarget: self, - fkArrowAction: #selector(handleFKArrowClick(_:)), - chevronTarget: self, - chevronAction: #selector(handleChevronClick(_:)), - delegate: self + row: row, + columnIndex: columnIndex ) + + let cell = cellRegistry.dequeueCell(of: kind, in: tableView) + cell.cellTextField.delegate = self + cell.configure(content: content, state: cellState) + ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) - return __view + return cell + } + + private func placeholderKind(for rawValue: String?) -> DataGridCellPlaceholder? { + guard let rawValue else { return .null } + if rawValue == "__DEFAULT__" { return .defaultMarker } + if rawValue.isEmpty { return .empty } + return nil } func tableView(_ tableView: NSTableView, rowViewForRow row: Int) -> NSTableRowView? { diff --git a/TableProTests/Models/UI/ColumnIdentitySchemaTests.swift b/TableProTests/Models/UI/ColumnIdentitySchemaTests.swift index 081d0d78a..aabef4b30 100644 --- a/TableProTests/Models/UI/ColumnIdentitySchemaTests.swift +++ b/TableProTests/Models/UI/ColumnIdentitySchemaTests.swift @@ -11,47 +11,34 @@ import Testing @Suite("ColumnIdentitySchema") @MainActor struct ColumnIdentitySchemaTests { - @Test("Unique columns produce name-based identifiers") - func nameBasedIdentifiers() { + @Test("Identifiers are slot-based regardless of column names") + func slotBasedIdentifiers() { let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) - #expect(schema.isNameBased) - #expect(schema.identifier(for: 0)?.rawValue == "id") - #expect(schema.identifier(for: 1)?.rawValue == "name") - #expect(schema.identifier(for: 2)?.rawValue == "email") + #expect(schema.identifier(for: 0)?.rawValue == "dataColumn-0") + #expect(schema.identifier(for: 1)?.rawValue == "dataColumn-1") + #expect(schema.identifier(for: 2)?.rawValue == "dataColumn-2") } - @Test("Duplicate column names fall back to positional identifiers") - func positionalFallbackForDuplicates() { + @Test("Duplicate column names produce unique slot identifiers") + func duplicateColumnNamesGetUniqueSlots() { let schema = ColumnIdentitySchema(columns: ["a", "b", "a"]) - #expect(!schema.isNameBased) - #expect(schema.identifier(for: 0)?.rawValue == "col_0") - #expect(schema.identifier(for: 1)?.rawValue == "col_1") - #expect(schema.identifier(for: 2)?.rawValue == "col_2") + #expect(schema.identifier(for: 0)?.rawValue == "dataColumn-0") + #expect(schema.identifier(for: 1)?.rawValue == "dataColumn-1") + #expect(schema.identifier(for: 2)?.rawValue == "dataColumn-2") } - @Test("Reserved row-number identifier triggers positional fallback") - func rowNumberCollisionFallback() { - let schema = ColumnIdentitySchema(columns: ["__rowNumber__", "name"]) - #expect(!schema.isNameBased) - } - - @Test("dataIndex round-trips for name-based schema") - func roundTripNameBased() { + @Test("dataIndex round-trips for slot identifier") + func roundTripDataIndex() { let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) - let identifier = NSUserInterfaceItemIdentifier("name") + let identifier = ColumnIdentitySchema.slotIdentifier(1) #expect(schema.dataIndex(from: identifier) == 1) } - @Test("dataIndex round-trips for positional schema") - func roundTripPositional() { - let schema = ColumnIdentitySchema(columns: ["a", "b", "a"]) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_2")) == 2) - } - - @Test("Out-of-range identifier returns nil") - func unknownIdentifier() { + @Test("dataIndex returns nil for unknown identifier") + func unknownIdentifierReturnsNil() { let schema = ColumnIdentitySchema(columns: ["id", "name"]) #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("missing")) == nil) + #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("dataColumn-99")) == nil) #expect(schema.identifier(for: 99) == nil) #expect(schema.identifier(for: -1) == nil) } @@ -66,136 +53,85 @@ struct ColumnIdentitySchemaTests { func emptySchema() { let schema = ColumnIdentitySchema.empty #expect(schema.identifiers.isEmpty) - #expect(schema.isNameBased) + #expect(schema.columnNames.isEmpty) #expect(schema.identifier(for: 0) == nil) } - @Test("Inserting a new column shifts position but the existing identifier still resolves") - func identifiersFollowColumnsAcrossInsert() { - let before = ColumnIdentitySchema(columns: ["id", "name", "email"]) - let after = ColumnIdentitySchema(columns: ["id", "created_at", "name", "email"]) - - let nameId = NSUserInterfaceItemIdentifier("name") - #expect(before.dataIndex(from: nameId) == 1) - #expect(after.dataIndex(from: nameId) == 2) - - let emailId = NSUserInterfaceItemIdentifier("email") - #expect(before.dataIndex(from: emailId) == 2) - #expect(after.dataIndex(from: emailId) == 3) - } - - @Test("Reordering columns reassigns indices but identifiers track the column") - func identifiersFollowColumnsAcrossReorder() { - let before = ColumnIdentitySchema(columns: ["id", "name", "email"]) - let after = ColumnIdentitySchema(columns: ["email", "id", "name"]) - - for column in ["id", "name", "email"] { - let id = NSUserInterfaceItemIdentifier(column) - let beforeIndex = before.dataIndex(from: id) - let afterIndex = after.dataIndex(from: id) - #expect(beforeIndex != nil) - #expect(afterIndex != nil) - #expect(beforeIndex != afterIndex) - } + @Test("columnName returns the name at the given slot") + func columnNameForSlot() { + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + #expect(schema.columnName(for: 0) == "id") + #expect(schema.columnName(for: 1) == "name") + #expect(schema.columnName(for: 2) == "email") + #expect(schema.columnName(for: 3) == nil) + #expect(schema.columnName(for: -1) == nil) } - @Test("Removing a column drops its identifier and keeps the others") - func identifiersDropOnColumnRemoval() { - let before = ColumnIdentitySchema(columns: ["id", "name", "email"]) - let after = ColumnIdentitySchema(columns: ["id", "email"]) - - #expect(after.dataIndex(from: NSUserInterfaceItemIdentifier("name")) == nil) - #expect(before.dataIndex(from: NSUserInterfaceItemIdentifier("email")) == 2) - #expect(after.dataIndex(from: NSUserInterfaceItemIdentifier("email")) == 1) + @Test("dataIndex(forColumnName:) returns the slot for unique names") + func dataIndexForColumnName() { + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + #expect(schema.dataIndex(forColumnName: "id") == 0) + #expect(schema.dataIndex(forColumnName: "name") == 1) + #expect(schema.dataIndex(forColumnName: "email") == 2) + #expect(schema.dataIndex(forColumnName: "missing") == nil) } - @Test("A column literally named col_0 stays name-based and resolves to its own index") - func literalColZeroColumnNameRoundTrips() { - let schema = ColumnIdentitySchema(columns: ["id", "name", "col_0"]) - #expect(schema.isNameBased == true) - - let identifier = schema.identifier(for: 2) - #expect(identifier?.rawValue == "col_0") - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_0")) == 2) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("id")) == 0) + @Test("dataIndex(forColumnName:) returns the last slot for duplicate names") + func dataIndexForDuplicateColumnNamePicksLast() { + let schema = ColumnIdentitySchema(columns: ["a", "b", "a"]) + #expect(schema.dataIndex(forColumnName: "a") == 2) + #expect(schema.dataIndex(forColumnName: "b") == 1) } - @Test("A literal col_0 column survives reordering without colliding with positional ids") - func literalColZeroSurvivesReorder() { - let before = ColumnIdentitySchema(columns: ["id", "name", "col_0"]) - let after = ColumnIdentitySchema(columns: ["col_0", "id", "name"]) - - #expect(before.isNameBased == true) - #expect(after.isNameBased == true) - - let columnId = NSUserInterfaceItemIdentifier("col_0") - #expect(before.dataIndex(from: columnId) == 2) - #expect(after.dataIndex(from: columnId) == 0) + @Test("Inserting a new column shifts subsequent slot identifiers") + func insertingColumnShiftsSlots() { + let after = ColumnIdentitySchema(columns: ["id", "created_at", "name", "email"]) + #expect(after.dataIndex(forColumnName: "name") == 2) + #expect(after.dataIndex(forColumnName: "email") == 3) } - @Test("Duplicate names ignore the duplicate column name when looking up by raw name") - func positionalSchemaIgnoresDuplicateRawName() { - let schema = ColumnIdentitySchema(columns: ["a", "b", "a"]) - #expect(!schema.isNameBased) - - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("a")) == nil) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("b")) == nil) + @Test("Reordering columns reassigns slot identifiers") + func reorderingReassignsSlots() { + let before = ColumnIdentitySchema(columns: ["id", "name", "email"]) + let after = ColumnIdentitySchema(columns: ["email", "id", "name"]) + #expect(before.dataIndex(forColumnName: "email") == 2) + #expect(after.dataIndex(forColumnName: "email") == 0) } - @Test("Positional schema only resolves col_N identifiers within range") - func positionalSchemaResolvesOnlyValidPositions() { - let schema = ColumnIdentitySchema(columns: ["a", "b", "a"]) - - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_0")) == 0) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_1")) == 1) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_2")) == 2) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_3")) == nil) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_-1")) == nil) + @Test("Removing a column drops its slot") + func removingColumnDropsSlot() { + let after = ColumnIdentitySchema(columns: ["id", "email"]) + #expect(after.dataIndex(forColumnName: "name") == nil) + #expect(after.dataIndex(forColumnName: "email") == 1) } - @Test("Name-based schema does not resolve positional identifiers like col_0") - func nameBasedSchemaDoesNotResolvePositional() { - let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) - - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_0")) == nil) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_1")) == nil) - #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_2")) == nil) + @Test("Column literally named dataColumn-0 round-trips by name lookup") + func literalDataColumnName() { + let schema = ColumnIdentitySchema(columns: ["id", "name", "dataColumn-0"]) + #expect(schema.columnName(for: 2) == "dataColumn-0") + #expect(schema.dataIndex(forColumnName: "dataColumn-0") == 2) + #expect(schema.identifier(for: 2)?.rawValue == "dataColumn-2") } - @Test("Duplicate-name positional schema disagrees with the layout key for that name") - func positionalSchemaIdentifiersDoNotMatchColumnNames() { - let schema = ColumnIdentitySchema(columns: ["id", "name", "name"]) - - #expect(!schema.isNameBased) - #expect(schema.identifier(for: 0)?.rawValue == "col_0") - #expect(schema.identifier(for: 1)?.rawValue == "col_1") - #expect(schema.identifier(for: 2)?.rawValue == "col_2") + @Test("slotIdentifier static helper produces canonical raw value") + func slotIdentifierStatic() { + #expect(ColumnIdentitySchema.slotIdentifier(0).rawValue == "dataColumn-0") + #expect(ColumnIdentitySchema.slotIdentifier(7).rawValue == "dataColumn-7") } - @Test("Single column with duplicate-trigger reserved name produces positional id") - func singleReservedNameTriggersPositional() { - let schema = ColumnIdentitySchema(columns: ["__rowNumber__"]) - - #expect(!schema.isNameBased) - #expect(schema.identifier(for: 0)?.rawValue == "col_0") + @Test("Reserved row-number column name does not collide with slot identifiers") + func reservedRowNumberNameDoesNotCollide() { + let schema = ColumnIdentitySchema(columns: ["__rowNumber__", "name"]) + #expect(schema.identifier(for: 0)?.rawValue == "dataColumn-0") #expect(schema.dataIndex(from: ColumnIdentitySchema.rowNumberIdentifier) == nil) + #expect(schema.dataIndex(forColumnName: "__rowNumber__") == 0) } - @Test("Schema with three duplicates uses positional fallback consistently") - func tripleDuplicateUsesPositionalFallback() { - let schema = ColumnIdentitySchema(columns: ["x", "x", "x"]) - - #expect(!schema.isNameBased) - for index in 0..<3 { - #expect(schema.identifier(for: index)?.rawValue == "col_\(index)") - } - } - - @Test("Empty array column input is name-based and resolves nothing") - func emptyColumnsInputIsNameBased() { + @Test("Empty array column input has no identifiers") + func emptyColumnsInput() { let schema = ColumnIdentitySchema(columns: []) - #expect(schema.isNameBased) #expect(schema.identifiers.isEmpty) #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("anything")) == nil) + #expect(schema.dataIndex(forColumnName: "anything") == nil) } } From 313d94ec5e4d7e500795eb40ab203cbeff7350a8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 12:35:40 +0700 Subject: [PATCH 05/15] fix(datagrid): set cell text-field delegate once, drop redundant per-render mutations --- TablePro/Views/Results/Cells/DataGridBaseCellView.swift | 8 +++++--- TablePro/Views/Results/Cells/DataGridCellRegistry.swift | 3 ++- TablePro/Views/Results/DataGridCoordinator.swift | 1 + .../Views/Results/Extensions/DataGridView+Columns.swift | 1 - 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/TablePro/Views/Results/Cells/DataGridBaseCellView.swift b/TablePro/Views/Results/Cells/DataGridBaseCellView.swift index 930d9ee15..3f5225b6f 100644 --- a/TablePro/Views/Results/Cells/DataGridBaseCellView.swift +++ b/TablePro/Views/Results/Cells/DataGridBaseCellView.swift @@ -53,7 +53,7 @@ class DataGridBaseCellView: NSTableCellView { return view }() - override init(frame frameRect: NSRect) { + required override init(frame frameRect: NSRect) { cellTextField = Self.makeTextField() super.init(frame: frameRect) commonInit() @@ -103,9 +103,11 @@ class DataGridBaseCellView: NSTableCellView { applyVisualState(state) cellTextField.isEditable = state.isEditable && !state.visualState.isDeleted - cellTextField.identifier = Self.reuseIdentifier - textFieldTrailingConstraint.constant = textFieldTrailingInset(for: content, state: state) + let newInset = textFieldTrailingInset(for: content, state: state) + if textFieldTrailingConstraint.constant != newInset { + textFieldTrailingConstraint.constant = newInset + } updateAccessoryVisibility(content: content, state: state) diff --git a/TablePro/Views/Results/Cells/DataGridCellRegistry.swift b/TablePro/Views/Results/Cells/DataGridCellRegistry.swift index 8186dbc83..d90f75f98 100644 --- a/TablePro/Views/Results/Cells/DataGridCellRegistry.swift +++ b/TablePro/Views/Results/Cells/DataGridCellRegistry.swift @@ -9,6 +9,7 @@ import Foundation @MainActor final class DataGridCellRegistry { weak var accessoryDelegate: DataGridCellAccessoryDelegate? + weak var textFieldDelegate: NSTextFieldDelegate? private(set) var nullDisplayString: String private var settingsObserver: NSObjectProtocol? @@ -80,7 +81,6 @@ final class DataGridCellRegistry { } if let reused = tableView.makeView(withIdentifier: identifier, owner: nil) as? DataGridBaseCellView { - reused.accessoryDelegate = accessoryDelegate reused.nullDisplayString = nullDisplayString return reused } @@ -88,6 +88,7 @@ final class DataGridCellRegistry { let cell = cellType.init(frame: .zero) cell.identifier = identifier cell.accessoryDelegate = accessoryDelegate + cell.cellTextField.delegate = textFieldDelegate cell.nullDisplayString = nullDisplayString return cell } diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 0dd0935b3..fc575c691 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -136,6 +136,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData self.cellRegistry = DataGridCellRegistry() super.init() cellRegistry.accessoryDelegate = self + cellRegistry.textFieldDelegate = self updateCache() observeThemeChanges() diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index c55c64592..dc9fba342 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -153,7 +153,6 @@ extension TableViewCoordinator { ) let cell = cellRegistry.dequeueCell(of: kind, in: tableView) - cell.cellTextField.delegate = self cell.configure(content: content, state: cellState) ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) From bc95c8603083129e5f37dd4bf945cd01f99ef37d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 12:37:39 +0700 Subject: [PATCH 06/15] fix(datagrid): filter saved column order to current schema before moveColumn --- TablePro/Views/Results/DataGridColumnPool.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift index 54a949d03..5740e15c9 100644 --- a/TablePro/Views/Results/DataGridColumnPool.swift +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -105,14 +105,16 @@ final class DataGridColumnPool { ) { let rowNumberIsPresent = tableView.tableColumns.first?.identifier == ColumnIdentitySchema.rowNumberIdentifier let baseOffset = rowNumberIsPresent ? 1 : 0 + let validOrder = order.filter { schema.dataIndex(forColumnName: $0) != nil } - for (targetPosition, columnName) in order.enumerated() { + for (targetPosition, columnName) in validOrder.enumerated() { guard let slot = schema.dataIndex(forColumnName: columnName) else { continue } let identifier = ColumnIdentitySchema.slotIdentifier(slot) guard let currentIndex = tableView.tableColumns.firstIndex(where: { $0.identifier == identifier }) else { continue } let desiredIndex = baseOffset + targetPosition + guard desiredIndex < tableView.tableColumns.count else { continue } if currentIndex != desiredIndex { tableView.moveColumn(currentIndex, toColumn: desiredIndex) } From a80f6d76daf4b2a84c6d31e150ceb24ce6160af6 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 12:42:06 +0700 Subject: [PATCH 07/15] fix(datagrid): reset slot order on reconcile, skip idempotent header rebuild --- .../Views/Results/DataGridColumnPool.swift | 58 +++++++++++++++---- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift index 5740e15c9..060e0ba3b 100644 --- a/TablePro/Views/Results/DataGridColumnPool.swift +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -52,6 +52,8 @@ final class DataGridColumnPool { } } + resetToNaturalOrder(in: tableView, visibleSlotCount: visibleCount) + if let order = savedLayout?.columnOrder, !order.isEmpty { applyColumnOrder(order, in: tableView, schema: schema) } @@ -80,22 +82,54 @@ final class DataGridColumnPool { width: CGFloat, isEditable: Bool ) { - let cell = SortableHeaderCell(textCell: name) - cell.font = column.headerCell.font - cell.alignment = column.headerCell.alignment - column.headerCell = cell + if !(column.headerCell is SortableHeaderCell) || column.headerCell.stringValue != name { + let cell = SortableHeaderCell(textCell: name) + cell.font = column.headerCell.font + cell.alignment = column.headerCell.alignment + column.headerCell = cell + } + let tooltip: String if let typeName = columnType?.rawType ?? columnType?.displayName { - column.headerToolTip = "\(name) (\(typeName))" + tooltip = "\(name) (\(typeName))" } else { - column.headerToolTip = name + tooltip = name + } + if column.headerToolTip != tooltip { + column.headerToolTip = tooltip + } + + let label = String(format: String(localized: "Column: %@"), name) + if column.headerCell.accessibilityLabel() != label { + column.headerCell.setAccessibilityLabel(label) + } + + if column.width != width { + column.width = width + } + if column.isEditable != isEditable { + column.isEditable = isEditable + } + if column.sortDescriptorPrototype?.key != name { + column.sortDescriptorPrototype = NSSortDescriptor(key: name, ascending: true) + } + } + + private func resetToNaturalOrder(in tableView: NSTableView, visibleSlotCount: Int) { + let rowNumberIsPresent = tableView.tableColumns.first?.identifier == ColumnIdentitySchema.rowNumberIdentifier + let baseOffset = rowNumberIsPresent ? 1 : 0 + + for slot in 0.. Date: Thu, 30 Apr 2026 12:53:44 +0700 Subject: [PATCH 08/15] fix(datagrid): attach columns in saved order on first growth, eliminate reorder flicker --- .../Views/Results/DataGridColumnPool.swift | 127 ++++++++++-------- 1 file changed, 73 insertions(+), 54 deletions(-) diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift index 060e0ba3b..05a14ee46 100644 --- a/TablePro/Views/Results/DataGridColumnPool.swift +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -27,7 +27,8 @@ final class DataGridColumnPool { ) { attach(to: tableView) let visibleCount = schema.columnNames.count - growPoolIfNeeded(to: visibleCount, in: tableView) + + growBackingPoolIfNeeded(to: visibleCount) let willRestoreWidths = !(savedLayout?.columnWidths.isEmpty ?? true) let hiddenFromLayout = savedLayout?.hiddenColumns ?? [] @@ -46,32 +47,90 @@ final class DataGridColumnPool { width: resolvedWidth, isEditable: isEditable ) - column.isHidden = hiddenFromLayout.contains(columnName) || hiddenColumnNames.contains(columnName) - } else { + let hidden = hiddenFromLayout.contains(columnName) || hiddenColumnNames.contains(columnName) + if column.isHidden != hidden { + column.isHidden = hidden + } + } else if !column.isHidden { column.isHidden = true } } - resetToNaturalOrder(in: tableView, visibleSlotCount: visibleCount) - - if let order = savedLayout?.columnOrder, !order.isEmpty { - applyColumnOrder(order, in: tableView, schema: schema) - } - } - - func currentSlotForColumnName(_ name: String, in schema: ColumnIdentitySchema) -> Int? { - schema.dataIndex(forColumnName: name) + let targetOrder = computeTargetOrder( + visibleCount: visibleCount, + savedOrder: savedLayout?.columnOrder, + schema: schema + ) + + attachAndOrderColumns( + in: tableView, + visibleCount: visibleCount, + targetOrder: targetOrder + ) } - private func growPoolIfNeeded(to count: Int, in tableView: NSTableView) { + private func growBackingPoolIfNeeded(to count: Int) { while pooledColumns.count < count { let slot = pooledColumns.count let column = NSTableColumn(identifier: ColumnIdentitySchema.slotIdentifier(slot)) column.minWidth = 30 column.resizingMask = .userResizingMask column.isEditable = true + column.isHidden = true pooledColumns.append(column) - tableView.addTableColumn(column) + } + } + + private func computeTargetOrder( + visibleCount: Int, + savedOrder: [String]?, + schema: ColumnIdentitySchema + ) -> [Int] { + var slots: [Int] = [] + var seen = Set() + + if let savedOrder { + for name in savedOrder { + guard let slot = schema.dataIndex(forColumnName: name), + slot < visibleCount, + !seen.contains(slot) else { continue } + slots.append(slot) + seen.insert(slot) + } + } + + for slot in 0..= visibleCount && !attachedIdentifiers.contains(pooledColumns[slot].identifier) { + tableView.addTableColumn(pooledColumns[slot]) + } + + for (targetPosition, slot) in targetOrder.enumerated() { + let identifier = ColumnIdentitySchema.slotIdentifier(slot) + guard let currentIndex = tableView.tableColumns.firstIndex(where: { $0.identifier == identifier }) else { + continue + } + let desiredIndex = baseOffset + targetPosition + guard desiredIndex < tableView.tableColumns.count else { continue } + if currentIndex != desiredIndex { + tableView.moveColumn(currentIndex, toColumn: desiredIndex) + } } } @@ -114,44 +173,4 @@ final class DataGridColumnPool { column.sortDescriptorPrototype = NSSortDescriptor(key: name, ascending: true) } } - - private func resetToNaturalOrder(in tableView: NSTableView, visibleSlotCount: Int) { - let rowNumberIsPresent = tableView.tableColumns.first?.identifier == ColumnIdentitySchema.rowNumberIdentifier - let baseOffset = rowNumberIsPresent ? 1 : 0 - - for slot in 0.. Date: Thu, 30 Apr 2026 13:02:34 +0700 Subject: [PATCH 09/15] fix(datagrid): detect column schema change via names not slot identifiers --- TablePro/Views/Results/DataGridCoordinator.swift | 5 +++++ TablePro/Views/Results/DataGridView.swift | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index fc575c691..b6564ea15 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -31,6 +31,11 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData var layoutPersister: any ColumnLayoutPersisting var onColumnLayoutDidChange: ((ColumnLayoutState) -> Void)? private(set) var identitySchema: ColumnIdentitySchema = .empty + private(set) var lastReconciledColumnNames: [String] = [] + + func markColumnsReconciled(names: [String]) { + lastReconciledColumnNames = names + } var currentSortState = SortState() func columnIdentifier(for dataIndex: Int) -> NSUserInterfaceItemIdentifier? { diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 2c42d1012..3c0b54efa 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -216,10 +216,8 @@ struct DataGridView: NSViewRepresentable { coordinator.rebuildVisualStateCache() - let currentDataColumns = tableView.tableColumns.dropFirst() - let currentColumnIds = Set(currentDataColumns.map { $0.identifier.rawValue }) - let expectedColumnIds = Set(coordinator.identitySchema.identifiers.map { $0.rawValue }) - let columnsChanged = !latestRows.columns.isEmpty && (currentColumnIds != expectedColumnIds) + let columnsChanged = !latestRows.columns.isEmpty + && coordinator.lastReconciledColumnNames != latestRows.columns let isInitialDataLoad = structureChanged && oldRowCount == 0 && !latestRows.columns.isEmpty let shouldRebuildColumns = columnsChanged || isInitialDataLoad @@ -299,6 +297,7 @@ struct DataGridView: NSViewRepresentable { ) } ) + coordinator.markColumnsReconciled(names: tableRows.columns) gridPerfLog.notice("[grid-perf] columnPool.reconcile took \(((CFAbsoluteTimeGetCurrent() - __tReconcile) * 1000), format: .fixed(precision: 2)) ms (cols=\(tableRows.columns.count))") } From 6f92de09eeec85ba55424674b5feb9095a7384dd Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 13:07:44 +0700 Subject: [PATCH 10/15] fix(datagrid): suppress moveColumn animation during reconcile to prevent visible swap --- TablePro/Views/Results/DataGridColumnPool.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift index 05a14ee46..347b4fec4 100644 --- a/TablePro/Views/Results/DataGridColumnPool.swift +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -4,6 +4,7 @@ // import AppKit +import QuartzCore @MainActor final class DataGridColumnPool { @@ -25,6 +26,16 @@ final class DataGridColumnPool { hiddenColumnNames: Set, widthCalculator: (String, Int) -> CGFloat ) { + NSAnimationContext.beginGrouping() + NSAnimationContext.current.duration = 0 + NSAnimationContext.current.allowsImplicitAnimation = false + CATransaction.begin() + CATransaction.setDisableActions(true) + defer { + CATransaction.commit() + NSAnimationContext.endGrouping() + } + attach(to: tableView) let visibleCount = schema.columnNames.count From 27e9ae8b8edd6de0a3f0bf2ac06de7511ebeef7c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 13:15:14 +0700 Subject: [PATCH 11/15] test(datagrid): add DataGridColumnPool and DataGridCellRegistry tests --- .../Cells/DataGridCellRegistryTests.swift | 291 ++++++++++++++ .../Results/DataGridColumnPoolTests.swift | 367 ++++++++++++++++++ 2 files changed, 658 insertions(+) create mode 100644 TableProTests/Views/Results/Cells/DataGridCellRegistryTests.swift create mode 100644 TableProTests/Views/Results/DataGridColumnPoolTests.swift diff --git a/TableProTests/Views/Results/Cells/DataGridCellRegistryTests.swift b/TableProTests/Views/Results/Cells/DataGridCellRegistryTests.swift new file mode 100644 index 000000000..80fe7be90 --- /dev/null +++ b/TableProTests/Views/Results/Cells/DataGridCellRegistryTests.swift @@ -0,0 +1,291 @@ +// +// DataGridCellRegistryTests.swift +// TableProTests +// + +import AppKit +import Testing + +@testable import TablePro + +@MainActor +private final class StubAccessoryDelegate: DataGridCellAccessoryDelegate { + func dataGridCellDidClickFKArrow(row: Int, columnIndex: Int) {} + func dataGridCellDidClickChevron(row: Int, columnIndex: Int) {} +} + +@Suite("DataGridCellRegistry.resolveKind") +@MainActor +struct DataGridCellRegistryResolveKindTests { + @Test("Foreign key flag wins over every other signal") + func resolveKind_returnsForeignKeyForFKColumn() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .boolean(rawType: nil), + isFKColumn: true, + isDropdownColumn: true + ) + #expect(kind == .foreignKey) + } + + @Test("Dropdown flag wins when not a foreign key") + func resolveKind_returnsDropdownForDropdownColumn() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .json(rawType: nil), + isFKColumn: false, + isDropdownColumn: true + ) + #expect(kind == .dropdown) + } + + @Test("Boolean column type resolves to boolean kind") + func resolveKind_returnsBooleanForBooleanType() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .boolean(rawType: nil), + isFKColumn: false, + isDropdownColumn: false + ) + #expect(kind == .boolean) + } + + @Test("Date, timestamp, and datetime column types all resolve to date kind") + func resolveKind_returnsDateForDateLikeTypes() { + let registry = DataGridCellRegistry() + for type: ColumnType in [.date(rawType: nil), .timestamp(rawType: nil), .datetime(rawType: nil)] { + let kind = registry.resolveKind( + columnIndex: 0, + columnType: type, + isFKColumn: false, + isDropdownColumn: false + ) + #expect(kind == .date) + } + } + + @Test("JSON column type resolves to json kind") + func resolveKind_returnsJsonForJsonType() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .json(rawType: nil), + isFKColumn: false, + isDropdownColumn: false + ) + #expect(kind == .json) + } + + @Test("Blob column type resolves to blob kind") + func resolveKind_returnsBlobForBlobType() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .blob(rawType: nil), + isFKColumn: false, + isDropdownColumn: false + ) + #expect(kind == .blob) + } + + @Test("Plain text column resolves to text kind") + func resolveKind_returnsTextForPlainTextColumn() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .text(rawType: nil), + isFKColumn: false, + isDropdownColumn: false + ) + #expect(kind == .text) + } + + @Test("Nil column type resolves to text kind") + func resolveKind_returnsTextWhenColumnTypeIsNil() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: nil, + isFKColumn: false, + isDropdownColumn: false + ) + #expect(kind == .text) + } + + @Test("FK takes priority over dropdown when both flags set") + func resolveKind_priorityForeignKeyOverDropdown() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .text(rawType: nil), + isFKColumn: true, + isDropdownColumn: true + ) + #expect(kind == .foreignKey) + } + + @Test("Dropdown takes priority over typed columns") + func resolveKind_priorityDropdownOverType() { + let registry = DataGridCellRegistry() + let kind = registry.resolveKind( + columnIndex: 0, + columnType: .blob(rawType: nil), + isFKColumn: false, + isDropdownColumn: true + ) + #expect(kind == .dropdown) + } +} + +@Suite("DataGridCellRegistry.dequeueCell") +@MainActor +struct DataGridCellRegistryDequeueTests { + private func makeTableView() -> NSTableView { + let tableView = NSTableView() + let column = NSTableColumn(identifier: ColumnIdentitySchema.slotIdentifier(0)) + tableView.addTableColumn(column) + return tableView + } + + @Test("Text kind dequeues DataGridTextCellView") + func dequeueCell_returnsTextSubclassForTextKind() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .text, in: makeTableView()) + #expect(cell is DataGridTextCellView) + #expect(cell.identifier == DataGridTextCellView.reuseIdentifier) + } + + @Test("Foreign key kind dequeues DataGridForeignKeyCellView") + func dequeueCell_returnsForeignKeySubclassForFKKind() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .foreignKey, in: makeTableView()) + #expect(cell is DataGridForeignKeyCellView) + #expect(cell.identifier == DataGridForeignKeyCellView.reuseIdentifier) + } + + @Test("Dropdown kind dequeues DataGridDropdownCellView") + func dequeueCell_returnsDropdownSubclassForDropdownKind() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .dropdown, in: makeTableView()) + #expect(cell is DataGridDropdownCellView) + #expect(cell.identifier == DataGridDropdownCellView.reuseIdentifier) + } + + @Test("Boolean kind dequeues DataGridBooleanCellView") + func dequeueCell_returnsBooleanSubclassForBooleanKind() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .boolean, in: makeTableView()) + #expect(cell is DataGridBooleanCellView) + #expect(cell.identifier == DataGridBooleanCellView.reuseIdentifier) + } + + @Test("Date kind dequeues DataGridDateCellView") + func dequeueCell_returnsDateSubclassForDateKind() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .date, in: makeTableView()) + #expect(cell is DataGridDateCellView) + #expect(cell.identifier == DataGridDateCellView.reuseIdentifier) + } + + @Test("JSON kind dequeues DataGridJsonCellView") + func dequeueCell_returnsJsonSubclassForJsonKind() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .json, in: makeTableView()) + #expect(cell is DataGridJsonCellView) + #expect(cell.identifier == DataGridJsonCellView.reuseIdentifier) + } + + @Test("Blob kind dequeues DataGridBlobCellView") + func dequeueCell_returnsBlobSubclassForBlobKind() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .blob, in: makeTableView()) + #expect(cell is DataGridBlobCellView) + #expect(cell.identifier == DataGridBlobCellView.reuseIdentifier) + } + + @Test("Reuse identifiers are distinct for every cell kind") + func reuseIdentifiers_areDistinctPerKind() { + let identifiers: [NSUserInterfaceItemIdentifier] = [ + DataGridTextCellView.reuseIdentifier, + DataGridForeignKeyCellView.reuseIdentifier, + DataGridDropdownCellView.reuseIdentifier, + DataGridBooleanCellView.reuseIdentifier, + DataGridDateCellView.reuseIdentifier, + DataGridJsonCellView.reuseIdentifier, + DataGridBlobCellView.reuseIdentifier, + ] + let unique = Set(identifiers.map(\.rawValue)) + #expect(unique.count == identifiers.count) + } + + @Test("Freshly created cell receives accessoryDelegate from registry") + func dequeueCell_propagatesAccessoryDelegateToFreshCell() { + let registry = DataGridCellRegistry() + let delegate = StubAccessoryDelegate() + registry.accessoryDelegate = delegate + + let cell = registry.dequeueCell(of: .text, in: makeTableView()) + #expect(cell.accessoryDelegate === delegate) + } + + @Test("Freshly created cell receives nullDisplayString from registry") + func dequeueCell_propagatesNullDisplayStringToFreshCell() { + let registry = DataGridCellRegistry() + let cell = registry.dequeueCell(of: .text, in: makeTableView()) + #expect(cell.nullDisplayString == registry.nullDisplayString) + } +} + +@Suite("DataGridCellRegistry.makeRowNumberCell") +@MainActor +struct DataGridCellRegistryRowNumberTests { + private func makeTableView() -> NSTableView { + let tableView = NSTableView() + let column = NSTableColumn(identifier: ColumnIdentitySchema.rowNumberIdentifier) + tableView.addTableColumn(column) + return tableView + } + + @Test("Row number cell has RowNumberCellView identifier") + func makeRowNumberCell_hasRowNumberCellViewIdentifier() { + let registry = DataGridCellRegistry() + let view = registry.makeRowNumberCell( + in: makeTableView(), + row: 0, + cachedRowCount: 5, + visualState: .empty + ) + #expect(view.identifier?.rawValue == "RowNumberCellView") + } + + @Test("Row number cell renders one-based row index") + func makeRowNumberCell_rendersOneBasedRowIndex() { + let registry = DataGridCellRegistry() + let view = registry.makeRowNumberCell( + in: makeTableView(), + row: 4, + cachedRowCount: 10, + visualState: .empty + ) + let cellView = view as? NSTableCellView + #expect(cellView != nil) + #expect(cellView?.textField?.stringValue == "5") + } + + @Test("Row number cell renders empty string when row is out of cached range") + func makeRowNumberCell_rendersEmptyWhenRowOutOfRange() { + let registry = DataGridCellRegistry() + let view = registry.makeRowNumberCell( + in: makeTableView(), + row: 99, + cachedRowCount: 5, + visualState: .empty + ) + let cellView = view as? NSTableCellView + #expect(cellView != nil) + #expect(cellView?.textField?.stringValue == "") + } +} diff --git a/TableProTests/Views/Results/DataGridColumnPoolTests.swift b/TableProTests/Views/Results/DataGridColumnPoolTests.swift new file mode 100644 index 000000000..52153058b --- /dev/null +++ b/TableProTests/Views/Results/DataGridColumnPoolTests.swift @@ -0,0 +1,367 @@ +// +// DataGridColumnPoolTests.swift +// TableProTests +// + +import AppKit +import Testing + +@testable import TablePro + +@Suite("DataGridColumnPool") +@MainActor +struct DataGridColumnPoolTests { + private func makeTableView() -> NSTableView { + let tableView = NSTableView() + let rowNumberColumn = NSTableColumn(identifier: ColumnIdentitySchema.rowNumberIdentifier) + rowNumberColumn.width = 40 + tableView.addTableColumn(rowNumberColumn) + return tableView + } + + private func makeColumnTypes(count: Int) -> [ColumnType] { + Array(repeating: ColumnType.text(rawType: nil), count: count) + } + + private func defaultWidthCalculator(name: String, slot: Int) -> CGFloat { + 100 + } + + private func dataColumns(in tableView: NSTableView) -> [NSTableColumn] { + tableView.tableColumns.filter { $0.identifier != ColumnIdentitySchema.rowNumberIdentifier } + } + + @Test("reconcile grows pool when column count exceeds capacity") + func reconcile_growsPoolWhenColumnCountExceedsCapacity() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + #expect(pool.totalSlots == 3) + #expect(dataColumns(in: tableView).count == 3) + } + + @Test("reconcile does not shrink pool when column count drops") + func reconcile_doesNotShrinkPoolWhenColumnCountDrops() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + + pool.reconcile( + tableView: tableView, + schema: ColumnIdentitySchema(columns: ["a", "b", "c", "d"]), + columnTypes: makeColumnTypes(count: 4), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + #expect(pool.totalSlots == 4) + + pool.reconcile( + tableView: tableView, + schema: ColumnIdentitySchema(columns: ["a", "b"]), + columnTypes: makeColumnTypes(count: 2), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + #expect(pool.totalSlots == 4) + let extras = dataColumns(in: tableView).filter { column in + let identifier = column.identifier.rawValue + return identifier == "dataColumn-2" || identifier == "dataColumn-3" + } + #expect(extras.count == 2) + #expect(extras.allSatisfy { $0.isHidden }) + } + + @Test("reconcile attaches columns in natural order when no saved layout") + func reconcile_attachesColumnsInNaturalOrderWithoutSavedLayout() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let identifiers = dataColumns(in: tableView).map(\.identifier.rawValue) + #expect(identifiers == ["dataColumn-0", "dataColumn-1", "dataColumn-2"]) + } + + @Test("reconcile attaches columns in saved order on first call") + func reconcile_attachesColumnsInSavedOrderOnFirstCall() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + var layout = ColumnLayoutState() + layout.columnOrder = ["email", "id", "name"] + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: layout, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let identifiers = dataColumns(in: tableView).map(\.identifier.rawValue) + #expect(identifiers == ["dataColumn-2", "dataColumn-0", "dataColumn-1"]) + } + + @Test("reconcile reorders existing columns when saved order differs from current") + func reconcile_reordersExistingColumnsWhenSavedOrderDiffersFromCurrent() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + var layout = ColumnLayoutState() + layout.columnOrder = ["email", "id", "name"] + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: layout, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let identifiers = dataColumns(in: tableView).map(\.identifier.rawValue) + #expect(identifiers == ["dataColumn-2", "dataColumn-0", "dataColumn-1"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: layout, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let afterSecond = dataColumns(in: tableView).map(\.identifier.rawValue) + #expect(afterSecond == ["dataColumn-2", "dataColumn-0", "dataColumn-1"]) + } + + @Test("reconcile reuses the same NSTableColumn instances across calls") + func reconcile_reusesSameTableColumnInstancesAcrossCalls() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let firstSnapshot = dataColumns(in: tableView) + let capturedSlot1 = firstSnapshot[1] + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let afterSnapshot = dataColumns(in: tableView) + #expect(afterSnapshot[1] === capturedSlot1) + for (before, after) in zip(firstSnapshot, afterSnapshot) { + #expect(before === after) + } + } + + @Test("reconcile honors hidden columns from saved layout") + func reconcile_honorsHiddenColumnsFromSavedLayout() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + var layout = ColumnLayoutState() + layout.hiddenColumns = ["name"] + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: layout, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let columns = dataColumns(in: tableView) + let hiddenStateByName = Dictionary(uniqueKeysWithValues: columns.map { ($0.headerCell.stringValue, $0.isHidden) }) + #expect(hiddenStateByName["id"] == false) + #expect(hiddenStateByName["name"] == true) + #expect(hiddenStateByName["email"] == false) + } + + @Test("reconcile honors hidden columns from hiddenColumnNames parameter") + func reconcile_honorsHiddenColumnsFromParameter() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: ["email"], + widthCalculator: defaultWidthCalculator + ) + + let columns = dataColumns(in: tableView) + let hiddenStateByName = Dictionary(uniqueKeysWithValues: columns.map { ($0.headerCell.stringValue, $0.isHidden) }) + #expect(hiddenStateByName["id"] == false) + #expect(hiddenStateByName["name"] == false) + #expect(hiddenStateByName["email"] == true) + } + + @Test("Slot identifiers use dataColumn-N format") + func reconcile_slotIdentifierFormatIsDataColumnN() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email", "created"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 4), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let identifiers = dataColumns(in: tableView).map(\.identifier.rawValue).sorted() + #expect(identifiers == ["dataColumn-0", "dataColumn-1", "dataColumn-2", "dataColumn-3"]) + } + + @Test("Column width comes from widthCalculator when no saved widths") + func reconcile_widthFromCalculatorWhenNoSavedWidths() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 2), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: { name, _ in name == "id" ? 50 : 200 } + ) + + let widthsByName = Dictionary(uniqueKeysWithValues: dataColumns(in: tableView).map { ($0.headerCell.stringValue, $0.width) }) + #expect(widthsByName["id"] == 50) + #expect(widthsByName["name"] == 200) + } + + @Test("Column width comes from saved layout when present") + func reconcile_widthFromSavedLayoutWhenPresent() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name"]) + + var layout = ColumnLayoutState() + layout.columnWidths = ["id": 75, "name": 250] + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 2), + savedLayout: layout, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: { _, _ in 9999 } + ) + + let widthsByName = Dictionary(uniqueKeysWithValues: dataColumns(in: tableView).map { ($0.headerCell.stringValue, $0.width) }) + #expect(widthsByName["id"] == 75) + #expect(widthsByName["name"] == 250) + } + + @Test("reconcile is idempotent for equivalent inputs") + func reconcile_isIdempotentForEquivalentInputs() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + var layout = ColumnLayoutState() + layout.columnOrder = ["name", "id", "email"] + layout.columnWidths = ["id": 60, "name": 120, "email": 180] + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: layout, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let beforeIdentifiers = tableView.tableColumns.map(\.identifier.rawValue) + let beforeWidths = tableView.tableColumns.map(\.width) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: layout, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + + let afterIdentifiers = tableView.tableColumns.map(\.identifier.rawValue) + let afterWidths = tableView.tableColumns.map(\.width) + + #expect(beforeIdentifiers == afterIdentifiers) + #expect(beforeWidths == afterWidths) + } +} From 36c04cfb351baab3d22cec48fecc6f734cc8d1b3 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 13:21:10 +0700 Subject: [PATCH 12/15] docs(datagrid): drop perf instrumentation, add Phase 2 design doc and changelog --- CHANGELOG.md | 1 + .../MainContentCoordinator+QueryHelpers.swift | 5 - .../Views/Results/DataGridCoordinator.swift | 20 --- TablePro/Views/Results/DataGridView.swift | 9 - .../Extensions/DataGridView+Columns.swift | 74 +------- docs/development/datagrid-phase2-design.md | 160 ++++++++++++++++++ 6 files changed, 166 insertions(+), 103 deletions(-) create mode 100644 docs/development/datagrid-phase2-design.md diff --git a/CHANGELOG.md b/CHANGELOG.md index db65f71cd..d30807276 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- DataGrid columns and cells refactored to use a persistent column pool and typed cell view hierarchy. CPU usage on table switch reduced significantly through proper NSTableView reuse pool retention. - Data grid column identifiers are now the column name (with positional fallback for duplicate names), so saved widths follow the column across schema changes that shift its position. Identifier resolution moved from static `DataGridView` helpers to a `ColumnIdentitySchema` value type owned by the coordinator. - `ColumnLayoutStorage` singleton replaced by a `ColumnLayoutPersisting` protocol with an injectable `FileColumnLayoutPersister` default. The coordinator depends on the protocol, not the concrete class, so tests can substitute a fake. - Column layout save/restore on table-switch (`saveColumnLayoutForTable` / `restoreColumnLayoutForTable`) folded into the data grid coordinator's lifecycle (load on column build, persist on resize/move/dismantle). The standalone `MainContentCoordinator+ColumnLayout` extension is gone; only the visibility orchestration remains. Removes the redundant `hasUserResizedColumns` flag and the external save trigger from the binding setter. diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift index 1b6a34c20..4359ba0f8 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift @@ -12,7 +12,6 @@ import os import TableProPluginKit private let progressLog = Logger(subsystem: "com.TablePro", category: "ProgressiveLoad") -private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") /// Context for progressive query result loading internal struct QueryPageContext { @@ -247,7 +246,6 @@ extension MainContentCoordinator { queryParameterValues: [QueryParameter]? = nil ) { guard let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return } - let __t0Total = CFAbsoluteTimeGetCurrent() var updatedTab = tabManager.tabs[idx] var columnEnumValues: [String: [String]] = [:] @@ -292,7 +290,6 @@ extension MainContentCoordinator { updatedTab.metadataVersion += 1 } - let __tFrom = CFAbsoluteTimeGetCurrent() let newTableRows = TableRows.from( queryRows: rows, columns: columns, @@ -302,7 +299,6 @@ extension MainContentCoordinator { columnEnumValues: columnEnumValues, columnNullable: columnNullable ) - gridPerfLog.notice("[grid-perf] TableRows.from took \(((CFAbsoluteTimeGetCurrent() - __tFrom) * 1000), format: .fixed(precision: 2)) ms (rows=\(rows.count) cols=\(columns.count))") setActiveTableRows(newTableRows, for: updatedTab.id) let rs = ResultSet(label: tableName ?? "Result", tableRows: newTableRows) @@ -384,7 +380,6 @@ extension MainContentCoordinator { if tabManager.selectedTabId == tabId, isEditable, !changeManager.hasChanges { changeManager.clearChangesAndUndoHistory() } - gridPerfLog.notice("[grid-perf] applyPhase1Result total took \(((CFAbsoluteTimeGetCurrent() - __t0Total) * 1000), format: .fixed(precision: 2)) ms (table=\(tableName ?? "nil"))") } /// Launch Phase 2 background work: exact COUNT(*) and enum value fetching diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index b6564ea15..fc6020903 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -1,9 +1,6 @@ import AppKit -import os import SwiftUI -private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") - // MARK: - Coordinator @MainActor @@ -270,27 +267,10 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData func applyFullReplace() { guard let tableView else { return } - let __t0 = CFAbsoluteTimeGetCurrent() - - let __tCache = CFAbsoluteTimeGetCurrent() displayCache.removeAll() - gridPerfLog.notice("[grid-perf] displayCache.removeAll took \(((CFAbsoluteTimeGetCurrent() - __tCache) * 1000), format: .fixed(precision: 2)) ms") - - let __tVisual = CFAbsoluteTimeGetCurrent() rebuildVisualStateCache() - gridPerfLog.notice("[grid-perf] rebuildVisualStateCache took \(((CFAbsoluteTimeGetCurrent() - __tVisual) * 1000), format: .fixed(precision: 2)) ms") - updateCache() - - let __tReload = CFAbsoluteTimeGetCurrent() tableView.reloadData() - gridPerfLog.notice("[grid-perf] tableView.reloadData took \(((CFAbsoluteTimeGetCurrent() - __tReload) * 1000), format: .fixed(precision: 2)) ms (rows=\(tableView.numberOfRows))") - - gridPerfLog.notice("[grid-perf] applyFullReplace total took \(((CFAbsoluteTimeGetCurrent() - __t0) * 1000), format: .fixed(precision: 2)) ms") - - DispatchQueue.main.async { - ViewForStats.flushIfQuiet() - } } func displayRow(at displayIndex: Int) -> Row? { diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 3c0b54efa..1e345370d 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -7,11 +7,8 @@ // import AppKit -import os import SwiftUI -private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") - struct CellPosition: Hashable { let row: Int let column: Int @@ -140,8 +137,6 @@ struct DataGridView: NSViewRepresentable { if tableView.editedRow >= 0 { return } if let editor = context.coordinator.overlayEditor, editor.isActive { return } - let __t0Update = CFAbsoluteTimeGetCurrent() - if let rowNumCol = tableView.tableColumns.first(where: { $0.identifier == ColumnIdentitySchema.rowNumberIdentifier }) { let shouldHide = !configuration.showRowNumbers if rowNumCol.isHidden != shouldHide { @@ -238,8 +233,6 @@ struct DataGridView: NSViewRepresentable { coordinator: coordinator, needsFullReload: needsFullReload ) - - gridPerfLog.notice("[grid-perf] DataGridView.updateNSView took \(((CFAbsoluteTimeGetCurrent() - __t0Update) * 1000), format: .fixed(precision: 2)) ms (rows=\(rowDisplayCount) cols=\(columnCount) reload=\(needsFullReload))") } // MARK: - updateNSView Helpers @@ -281,7 +274,6 @@ struct DataGridView: NSViewRepresentable { tableRows: TableRows, savedLayout: ColumnLayoutState? ) { - let __tReconcile = CFAbsoluteTimeGetCurrent() coordinator.columnPool.reconcile( tableView: tableView, schema: coordinator.identitySchema, @@ -298,7 +290,6 @@ struct DataGridView: NSViewRepresentable { } ) coordinator.markColumnsReconciled(names: tableRows.columns) - gridPerfLog.notice("[grid-perf] columnPool.reconcile took \(((CFAbsoluteTimeGetCurrent() - __tReconcile) * 1000), format: .fixed(precision: 2)) ms (cols=\(tableRows.columns.count))") } private func syncSortDescriptors(tableView: NSTableView, coordinator: TableViewCoordinator, columns: [String]) { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index dc9fba342..84a8f96af 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -4,98 +4,36 @@ // import AppKit -import os import SwiftUI -private let gridPerfLog = Logger(subsystem: "com.TablePro", category: "GridPerf") - -@MainActor -internal enum ViewForStats { - static var count: Int = 0 - static var totalMs: Double = 0 - static var firstCallTime: CFAbsoluteTime = 0 - static var lastReportedCount: Int = 0 - - static var sectionA_makeView_ms: Double = 0 - static var sectionB_buttons_ms: Double = 0 - static var sectionC_textContent_ms: Double = 0 - static var sectionD_visualState_ms: Double = 0 - static var sectionE_accessibility_ms: Double = 0 - - static func record(_ ms: Double) { - if count == 0 { firstCallTime = CFAbsoluteTimeGetCurrent() } - count += 1 - totalMs += ms - } - - static func recordSection(a: Double = 0, b: Double = 0, c: Double = 0, d: Double = 0, e: Double = 0) { - sectionA_makeView_ms += a - sectionB_buttons_ms += b - sectionC_textContent_ms += c - sectionD_visualState_ms += d - sectionE_accessibility_ms += e - } - - static func flushIfQuiet() { - guard count > lastReportedCount else { return } - let delta = count - lastReportedCount - gridPerfLog.notice("[grid-perf] viewFor batch: \(delta) calls, total \(totalMs, format: .fixed(precision: 2)) ms since last report") - gridPerfLog.notice("[grid-perf] sectionA makeView/reuse: \(sectionA_makeView_ms, format: .fixed(precision: 2)) ms") - gridPerfLog.notice("[grid-perf] sectionB FK/chevron buttons: \(sectionB_buttons_ms, format: .fixed(precision: 2)) ms") - gridPerfLog.notice("[grid-perf] sectionC configureTextContent: \(sectionC_textContent_ms, format: .fixed(precision: 2)) ms") - gridPerfLog.notice("[grid-perf] sectionD CATransaction+bg+focus: \(sectionD_visualState_ms, format: .fixed(precision: 2)) ms") - gridPerfLog.notice("[grid-perf] sectionE accessibility labels: \(sectionE_accessibility_ms, format: .fixed(precision: 2)) ms") - lastReportedCount = count - totalMs = 0 - sectionA_makeView_ms = 0 - sectionB_buttons_ms = 0 - sectionC_textContent_ms = 0 - sectionD_visualState_ms = 0 - sectionE_accessibility_ms = 0 - } -} - extension TableViewCoordinator { func tableView(_ tableView: NSTableView, viewFor tableColumn: NSTableColumn?, row: Int) -> NSView? { - let __tCell = CFAbsoluteTimeGetCurrent() - guard let column = tableColumn else { - let __view: NSView? = nil - ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) - return __view - } + guard let column = tableColumn else { return nil } let tableRows = tableRowsProvider() let displayCount = sortedIDs?.count ?? tableRows.count if column.identifier == ColumnIdentitySchema.rowNumberIdentifier { - let __view: NSView? = cellRegistry.makeRowNumberCell( + return cellRegistry.makeRowNumberCell( in: tableView, row: row, cachedRowCount: displayCount, visualState: visualState(for: row) ) - ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) - return __view } guard let columnIndex = dataColumnIndex(from: column.identifier) else { - let __view: NSView? = nil - ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) - return __view + return nil } guard row >= 0 && row < displayCount, columnIndex >= 0 && columnIndex < cachedColumnCount else { - let __view: NSView? = nil - ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) - return __view + return nil } guard let displayRow = displayRow(at: row), columnIndex < displayRow.values.count else { - let __view: NSView? = nil - ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) - return __view + return nil } let rawValue = displayRow.values[columnIndex] let columnType = columnIndex < tableRows.columnTypes.count @@ -154,8 +92,6 @@ extension TableViewCoordinator { let cell = cellRegistry.dequeueCell(of: kind, in: tableView) cell.configure(content: content, state: cellState) - - ViewForStats.record((CFAbsoluteTimeGetCurrent() - __tCell) * 1000) return cell } diff --git a/docs/development/datagrid-phase2-design.md b/docs/development/datagrid-phase2-design.md new file mode 100644 index 000000000..6cb7468aa --- /dev/null +++ b/docs/development/datagrid-phase2-design.md @@ -0,0 +1,160 @@ +# DataGrid Phase 2: Persistent Column Pool + Typed Cell Hierarchy + +## Problem + +Switching tables blocked the main thread for ~135 ms on a 14-column, 1000-row dataset. The CPU spike came from the cell rendering hot path. + +Two underlying causes: + +1. `rebuildColumns` removed every `NSTableColumn` and recreated them on each table switch. Removing a column also evicts its associated cell views from the AppKit reuse pool. The next `reloadData` had no warm cells to dequeue and allocated a fresh `NSTableCellView` per visible cell. OSLog signposts showed Section A (`makeView`/reuse) consuming about 87% of `viewFor` time on a cold pool. + +2. The data cell was a single `DataGridCellView` reused across every kind of column (text, foreign key, dropdown, boolean, date, JSON, blob). One `NSUserInterfaceItemIdentifier`, one constructor with 15 parameters, kind-specific state read back through `viewWithTag` integer constants. The reuse pool keyed on a single identifier could not reuse cells per-kind, and the constructor itself ran kind-checks on every call. + +The combination produced a per-cell render cost around 0.26 ms. Multiplied across ~500 visible cells on first paint, that is the visible jank on table click. + +## Approach + +### Persistent column pool + +`DataGridColumnPool` (`TablePro/Views/Results/DataGridColumnPool.swift`) holds the `NSTableColumn` instances. Identifiers are slot-based: `dataColumn-0`, `dataColumn-1`, etc. The pool grows when a wider table arrives and never removes columns. Surplus columns past the current schema are hidden, not evicted. + +`reconcile(...)` is called whenever the column schema changes: + +1. Grow the backing pool to cover the visible column count. +2. Rename headers and reset width / type / editable state on each visible slot. +3. Hide surplus slots. +4. Compute the target visual order from the saved layout, falling back to slot order. +5. Attach any not-yet-attached columns, then `moveColumn` each slot into its target position. + +Because columns are not removed, the AppKit cell reuse pool remains warm across table switches. + +### Typed cell hierarchy + +`DataGridBaseCellView` (`TablePro/Views/Results/Cells/DataGridBaseCellView.swift`) is the shared `NSTableCellView` base. It owns: + +- `cellTextField: CellTextField` plus its layout constraints +- `backgroundView` for delete / insert / modify highlight +- focus ring drawing via `drawFocusRingMask` +- value formatting branches (null, empty, default marker, regular) +- accessibility row / column index ranges + +Seven leaf subclasses, one per `DataGridCellKind`: + +- `DataGridTextCellView` +- `DataGridForeignKeyCellView` (FK arrow accessory) +- `DataGridDropdownCellView` (chevron accessory) +- `DataGridBooleanCellView` +- `DataGridDateCellView` +- `DataGridJsonCellView` +- `DataGridBlobCellView` + +Each subclass overrides `class var reuseIdentifier` so AppKit's reuse pool keys per-kind. A reused dropdown cell is always a dropdown cell. Boolean, date, JSON, and blob currently render as text and are placeholders for native widgets in a later phase. + +Accessory buttons (FK arrow, chevron) live in `AccessoryButtons.swift` as standalone `NSButton` subclasses. Subclasses that need an accessory install it once in `installAccessory()` and toggle visibility in `updateAccessoryVisibility(content:state:)`. + +### Cell registry + +`DataGridCellRegistry` (`TablePro/Views/Results/Cells/DataGridCellRegistry.swift`) replaces the 15-parameter `DataGridCellFactory.makeDataCell`. Two methods: + +- `resolveKind(columnIndex:columnType:isFKColumn:isDropdownColumn:)` returns the matching `DataGridCellKind`. +- `dequeueCell(of:in:)` calls `tableView.makeView(withIdentifier:)` against the kind's identifier and falls back to a fresh allocation only on first use. + +Construction wires the cell's `accessoryDelegate` and `cellTextField.delegate` once. Per-render mutations on those properties are gone. + +### Schema change detection + +The naive trigger for a column rebuild was "did the column identifiers change". Slot identifiers are identical across tables of the same width (`dataColumn-0` matches `dataColumn-0`), so that check missed real schema changes between tables with the same column count. + +`TableViewCoordinator.lastReconciledColumnNames` records the names that were last reconciled into the pool. `DataGridView.updateNSView` compares the current `latestRows.columns` against that list. A mismatch means the schema changed and `reconcile(...)` runs. Identical names skip the rebuild. + +### Animation suppression + +`reconcile(...)` runs `moveColumn` calls in sequence to install the saved order. Without animation suppression, the user briefly saw the natural slot order before each move animated into place, producing a visible swap on every table click. + +The reconcile body wraps the whole sequence in: + +``` +NSAnimationContext.beginGrouping() +NSAnimationContext.current.duration = 0 +NSAnimationContext.current.allowsImplicitAnimation = false +CATransaction.begin() +CATransaction.setDisableActions(true) +``` + +with matching `defer` to commit and end grouping. `moveColumn` and the header rename commit in a single visual frame. + +## Results + +Measured on a 14-column, 1000-row table click, MacBook Air M2: + +| Metric | Before | After | +| --- | --- | --- | +| Per-cell `viewFor` cost | 0.26 ms | 0.10 ms | +| Section A (`makeView` / reuse) share of `viewFor` | 87% | ~0% on warm reuse pool | +| `viewFor` total for first paint (~500 cells) | ~135 ms | ~41 ms | + +The user-visible CPU spike on table click is gone. Subsequent switches between tables of similar shape stay under 5 ms total in `reconcile` because the pool already has enough slots and only headers and order need updating. + +## Files + +### New + +`TablePro/Views/Results/Cells/`: + +- `AccessoryButtons.swift` +- `DataGridBaseCellView.swift` +- `DataGridBlobCellView.swift` +- `DataGridBooleanCellView.swift` +- `DataGridCellAccessoryDelegate.swift` +- `DataGridCellContent.swift` +- `DataGridCellKind.swift` +- `DataGridCellRegistry.swift` +- `DataGridChevronCellView.swift` +- `DataGridDateCellView.swift` +- `DataGridDropdownCellView.swift` +- `DataGridForeignKeyCellView.swift` +- `DataGridJsonCellView.swift` +- `DataGridTextCellView.swift` + +`TablePro/Views/Results/`: + +- `DataGridColumnPool.swift` + +`TableProTests/`: + +- `Views/Results/Cells/DataGridCellRegistryTests.swift` +- `Views/Results/DataGridColumnPoolTests.swift` + +### Modified + +- `TablePro/Models/UI/ColumnIdentitySchema.swift`: added slot identifier helper and column name to slot lookup. +- `TablePro/Views/Results/DataGridView.swift`: switched from `rebuildColumns` to `reconcileColumnPool`, added schema change detection by name. +- `TablePro/Views/Results/DataGridCoordinator.swift`: owns `columnPool` and `cellRegistry`, tracks `lastReconciledColumnNames`. +- `TablePro/Views/Results/Extensions/DataGridView+Columns.swift`: `viewFor` now goes through the registry, no factory call. +- `TablePro/Views/Results/Extensions/DataGridView+Click.swift`: click resolution uses slot identifiers via the schema. +- `TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift`: instrumentation added during measurement, removed at end of phase. +- `TableProTests/Models/UI/ColumnIdentitySchemaTests.swift`: covers the new lookups. + +### Deleted + +- `TablePro/Views/Results/DataGridCellView.swift`: the mega-cell. +- Most of `TablePro/Views/Results/DataGridCellFactory.swift`: kept only the column width calculator. Cell construction is gone. + +### Out of scope + +- Phase 2c: native widgets per kind. `NSButton` checkbox for boolean, `NSDatePicker` popover for date, JSON syntax view for JSON, hex viewer for blob. The kind hierarchy is in place to receive them without further architectural change. +- Phase 2d: window-level field editor that replaces the per-cell `NSTextView` overlay. Removes the per-cell editor allocation and lets editing share one editor across the grid. +- Phase 3+: interaction polish (keyboard nav, paste UX, undo grouping refinements). + +## Audit issues addressed + +From `docs/development/datagrid-audit.md`: + +- #5 partial: cell construction is decomposed but native widgets are deferred. +- #7 prepared: typed kinds make per-kind editors trivial to add. +- #19: `NSTableColumn` lifetime decoupled from schema changes. +- #20: reuse pool stays warm on table switch. +- #21: accessory buttons no longer carry stale handler state across reuse (fixed earlier in the branch and now backed by the kind hierarchy). +- #28: slot-based identifiers are stable across schema permutations. +- #40: cell construction no longer reads tag-based state. +- #50: column reorder during reconcile no longer flickers. From d25115ddc1567a9bd7d3d9418427eda4594d6d83 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 13:28:52 +0700 Subject: [PATCH 13/15] docs(datagrid): remove Phase 2 design doc --- docs/development/datagrid-phase2-design.md | 160 --------------------- 1 file changed, 160 deletions(-) delete mode 100644 docs/development/datagrid-phase2-design.md diff --git a/docs/development/datagrid-phase2-design.md b/docs/development/datagrid-phase2-design.md deleted file mode 100644 index 6cb7468aa..000000000 --- a/docs/development/datagrid-phase2-design.md +++ /dev/null @@ -1,160 +0,0 @@ -# DataGrid Phase 2: Persistent Column Pool + Typed Cell Hierarchy - -## Problem - -Switching tables blocked the main thread for ~135 ms on a 14-column, 1000-row dataset. The CPU spike came from the cell rendering hot path. - -Two underlying causes: - -1. `rebuildColumns` removed every `NSTableColumn` and recreated them on each table switch. Removing a column also evicts its associated cell views from the AppKit reuse pool. The next `reloadData` had no warm cells to dequeue and allocated a fresh `NSTableCellView` per visible cell. OSLog signposts showed Section A (`makeView`/reuse) consuming about 87% of `viewFor` time on a cold pool. - -2. The data cell was a single `DataGridCellView` reused across every kind of column (text, foreign key, dropdown, boolean, date, JSON, blob). One `NSUserInterfaceItemIdentifier`, one constructor with 15 parameters, kind-specific state read back through `viewWithTag` integer constants. The reuse pool keyed on a single identifier could not reuse cells per-kind, and the constructor itself ran kind-checks on every call. - -The combination produced a per-cell render cost around 0.26 ms. Multiplied across ~500 visible cells on first paint, that is the visible jank on table click. - -## Approach - -### Persistent column pool - -`DataGridColumnPool` (`TablePro/Views/Results/DataGridColumnPool.swift`) holds the `NSTableColumn` instances. Identifiers are slot-based: `dataColumn-0`, `dataColumn-1`, etc. The pool grows when a wider table arrives and never removes columns. Surplus columns past the current schema are hidden, not evicted. - -`reconcile(...)` is called whenever the column schema changes: - -1. Grow the backing pool to cover the visible column count. -2. Rename headers and reset width / type / editable state on each visible slot. -3. Hide surplus slots. -4. Compute the target visual order from the saved layout, falling back to slot order. -5. Attach any not-yet-attached columns, then `moveColumn` each slot into its target position. - -Because columns are not removed, the AppKit cell reuse pool remains warm across table switches. - -### Typed cell hierarchy - -`DataGridBaseCellView` (`TablePro/Views/Results/Cells/DataGridBaseCellView.swift`) is the shared `NSTableCellView` base. It owns: - -- `cellTextField: CellTextField` plus its layout constraints -- `backgroundView` for delete / insert / modify highlight -- focus ring drawing via `drawFocusRingMask` -- value formatting branches (null, empty, default marker, regular) -- accessibility row / column index ranges - -Seven leaf subclasses, one per `DataGridCellKind`: - -- `DataGridTextCellView` -- `DataGridForeignKeyCellView` (FK arrow accessory) -- `DataGridDropdownCellView` (chevron accessory) -- `DataGridBooleanCellView` -- `DataGridDateCellView` -- `DataGridJsonCellView` -- `DataGridBlobCellView` - -Each subclass overrides `class var reuseIdentifier` so AppKit's reuse pool keys per-kind. A reused dropdown cell is always a dropdown cell. Boolean, date, JSON, and blob currently render as text and are placeholders for native widgets in a later phase. - -Accessory buttons (FK arrow, chevron) live in `AccessoryButtons.swift` as standalone `NSButton` subclasses. Subclasses that need an accessory install it once in `installAccessory()` and toggle visibility in `updateAccessoryVisibility(content:state:)`. - -### Cell registry - -`DataGridCellRegistry` (`TablePro/Views/Results/Cells/DataGridCellRegistry.swift`) replaces the 15-parameter `DataGridCellFactory.makeDataCell`. Two methods: - -- `resolveKind(columnIndex:columnType:isFKColumn:isDropdownColumn:)` returns the matching `DataGridCellKind`. -- `dequeueCell(of:in:)` calls `tableView.makeView(withIdentifier:)` against the kind's identifier and falls back to a fresh allocation only on first use. - -Construction wires the cell's `accessoryDelegate` and `cellTextField.delegate` once. Per-render mutations on those properties are gone. - -### Schema change detection - -The naive trigger for a column rebuild was "did the column identifiers change". Slot identifiers are identical across tables of the same width (`dataColumn-0` matches `dataColumn-0`), so that check missed real schema changes between tables with the same column count. - -`TableViewCoordinator.lastReconciledColumnNames` records the names that were last reconciled into the pool. `DataGridView.updateNSView` compares the current `latestRows.columns` against that list. A mismatch means the schema changed and `reconcile(...)` runs. Identical names skip the rebuild. - -### Animation suppression - -`reconcile(...)` runs `moveColumn` calls in sequence to install the saved order. Without animation suppression, the user briefly saw the natural slot order before each move animated into place, producing a visible swap on every table click. - -The reconcile body wraps the whole sequence in: - -``` -NSAnimationContext.beginGrouping() -NSAnimationContext.current.duration = 0 -NSAnimationContext.current.allowsImplicitAnimation = false -CATransaction.begin() -CATransaction.setDisableActions(true) -``` - -with matching `defer` to commit and end grouping. `moveColumn` and the header rename commit in a single visual frame. - -## Results - -Measured on a 14-column, 1000-row table click, MacBook Air M2: - -| Metric | Before | After | -| --- | --- | --- | -| Per-cell `viewFor` cost | 0.26 ms | 0.10 ms | -| Section A (`makeView` / reuse) share of `viewFor` | 87% | ~0% on warm reuse pool | -| `viewFor` total for first paint (~500 cells) | ~135 ms | ~41 ms | - -The user-visible CPU spike on table click is gone. Subsequent switches between tables of similar shape stay under 5 ms total in `reconcile` because the pool already has enough slots and only headers and order need updating. - -## Files - -### New - -`TablePro/Views/Results/Cells/`: - -- `AccessoryButtons.swift` -- `DataGridBaseCellView.swift` -- `DataGridBlobCellView.swift` -- `DataGridBooleanCellView.swift` -- `DataGridCellAccessoryDelegate.swift` -- `DataGridCellContent.swift` -- `DataGridCellKind.swift` -- `DataGridCellRegistry.swift` -- `DataGridChevronCellView.swift` -- `DataGridDateCellView.swift` -- `DataGridDropdownCellView.swift` -- `DataGridForeignKeyCellView.swift` -- `DataGridJsonCellView.swift` -- `DataGridTextCellView.swift` - -`TablePro/Views/Results/`: - -- `DataGridColumnPool.swift` - -`TableProTests/`: - -- `Views/Results/Cells/DataGridCellRegistryTests.swift` -- `Views/Results/DataGridColumnPoolTests.swift` - -### Modified - -- `TablePro/Models/UI/ColumnIdentitySchema.swift`: added slot identifier helper and column name to slot lookup. -- `TablePro/Views/Results/DataGridView.swift`: switched from `rebuildColumns` to `reconcileColumnPool`, added schema change detection by name. -- `TablePro/Views/Results/DataGridCoordinator.swift`: owns `columnPool` and `cellRegistry`, tracks `lastReconciledColumnNames`. -- `TablePro/Views/Results/Extensions/DataGridView+Columns.swift`: `viewFor` now goes through the registry, no factory call. -- `TablePro/Views/Results/Extensions/DataGridView+Click.swift`: click resolution uses slot identifiers via the schema. -- `TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift`: instrumentation added during measurement, removed at end of phase. -- `TableProTests/Models/UI/ColumnIdentitySchemaTests.swift`: covers the new lookups. - -### Deleted - -- `TablePro/Views/Results/DataGridCellView.swift`: the mega-cell. -- Most of `TablePro/Views/Results/DataGridCellFactory.swift`: kept only the column width calculator. Cell construction is gone. - -### Out of scope - -- Phase 2c: native widgets per kind. `NSButton` checkbox for boolean, `NSDatePicker` popover for date, JSON syntax view for JSON, hex viewer for blob. The kind hierarchy is in place to receive them without further architectural change. -- Phase 2d: window-level field editor that replaces the per-cell `NSTextView` overlay. Removes the per-cell editor allocation and lets editing share one editor across the grid. -- Phase 3+: interaction polish (keyboard nav, paste UX, undo grouping refinements). - -## Audit issues addressed - -From `docs/development/datagrid-audit.md`: - -- #5 partial: cell construction is decomposed but native widgets are deferred. -- #7 prepared: typed kinds make per-kind editors trivial to add. -- #19: `NSTableColumn` lifetime decoupled from schema changes. -- #20: reuse pool stays warm on table switch. -- #21: accessory buttons no longer carry stale handler state across reuse (fixed earlier in the branch and now backed by the kind hierarchy). -- #28: slot-based identifiers are stable across schema permutations. -- #40: cell construction no longer reads tag-based state. -- #50: column reorder during reconcile no longer flickers. From 35d4406d6136e8e0c56122eda40f778524008b52 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 13:35:15 +0700 Subject: [PATCH 14/15] fix(datagrid): always reconcile columns when data present, drop stale layout-skip optimization --- .../Views/Results/DataGridCoordinator.swift | 5 -- TablePro/Views/Results/DataGridView.swift | 55 ++++--------------- 2 files changed, 11 insertions(+), 49 deletions(-) diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index fc6020903..723b11a15 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -28,11 +28,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData var layoutPersister: any ColumnLayoutPersisting var onColumnLayoutDidChange: ((ColumnLayoutState) -> Void)? private(set) var identitySchema: ColumnIdentitySchema = .empty - private(set) var lastReconciledColumnNames: [String] = [] - - func markColumnsReconciled(names: [String]) { - lastReconciledColumnNames = names - } var currentSortState = SortState() func columnIdentifier(for dataIndex: Int) -> NSUserInterfaceItemIdentifier? { diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 1e345370d..661c01be7 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -211,61 +211,29 @@ struct DataGridView: NSViewRepresentable { coordinator.rebuildVisualStateCache() - let columnsChanged = !latestRows.columns.isEmpty - && coordinator.lastReconciledColumnNames != latestRows.columns - - let isInitialDataLoad = structureChanged && oldRowCount == 0 && !latestRows.columns.isEmpty - let shouldRebuildColumns = columnsChanged || isInitialDataLoad - - updateColumns( - tableView: tableView, - coordinator: coordinator, - tableRows: latestRows, - columnsChanged: columnsChanged, - shouldRebuild: shouldRebuildColumns, - structureChanged: structureChanged - ) - - syncSortDescriptors(tableView: tableView, coordinator: coordinator, columns: latestRows.columns) - - reloadAndSyncSelection( - tableView: tableView, - coordinator: coordinator, - needsFullReload: needsFullReload - ) - } - - // MARK: - updateNSView Helpers - - private func updateColumns( - tableView: NSTableView, - coordinator: TableViewCoordinator, - tableRows: TableRows, - columnsChanged: Bool, - shouldRebuild: Bool, - structureChanged: Bool - ) { - if shouldRebuild { + if !latestRows.columns.isEmpty { coordinator.isRebuildingColumns = true - defer { coordinator.isRebuildingColumns = false } - let savedLayout = coordinator.savedColumnLayout(binding: columnLayout) reconcileColumnPool( tableView: tableView, coordinator: coordinator, - tableRows: tableRows, + tableRows: latestRows, savedLayout: savedLayout ) + coordinator.isRebuildingColumns = false if savedLayout == nil { coordinator.scheduleLayoutPersist() } - } else { - for column in tableView.tableColumns - where column.identifier != ColumnIdentitySchema.rowNumberIdentifier { - column.isEditable = isEditable - } } + + syncSortDescriptors(tableView: tableView, coordinator: coordinator, columns: latestRows.columns) + + reloadAndSyncSelection( + tableView: tableView, + coordinator: coordinator, + needsFullReload: needsFullReload + ) } private func reconcileColumnPool( @@ -289,7 +257,6 @@ struct DataGridView: NSViewRepresentable { ) } ) - coordinator.markColumnsReconciled(names: tableRows.columns) } private func syncSortDescriptors(tableView: NSTableView, coordinator: TableViewCoordinator, columns: [String]) { From 24145476f9411a5c2042ff52741e017c77a14416 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 30 Apr 2026 17:43:42 +0700 Subject: [PATCH 15/15] fix(datagrid): scope move animation to moveColumn calls + add pool teardown hook --- .../Views/Results/DataGridColumnPool.swift | 33 ++++++++++-------- .../Views/Results/DataGridCoordinator.swift | 1 + .../Results/DataGridColumnPoolTests.swift | 34 +++++++++++++++++++ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift index 347b4fec4..a5af5c0cd 100644 --- a/TablePro/Views/Results/DataGridColumnPool.swift +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -4,7 +4,6 @@ // import AppKit -import QuartzCore @MainActor final class DataGridColumnPool { @@ -17,6 +16,14 @@ final class DataGridColumnPool { attachedTableView = tableView } + func detachFromTableView() { + guard let tableView = attachedTableView else { return } + for column in pooledColumns where tableView.tableColumns.contains(column) { + tableView.removeTableColumn(column) + } + attachedTableView = nil + } + func reconcile( tableView: NSTableView, schema: ColumnIdentitySchema, @@ -26,16 +33,6 @@ final class DataGridColumnPool { hiddenColumnNames: Set, widthCalculator: (String, Int) -> CGFloat ) { - NSAnimationContext.beginGrouping() - NSAnimationContext.current.duration = 0 - NSAnimationContext.current.allowsImplicitAnimation = false - CATransaction.begin() - CATransaction.setDisableActions(true) - defer { - CATransaction.commit() - NSAnimationContext.endGrouping() - } - attach(to: tableView) let visibleCount = schema.columnNames.count @@ -121,17 +118,25 @@ final class DataGridColumnPool { visibleCount: Int, targetOrder: [Int] ) { - let attachedIdentifiers = Set(tableView.tableColumns.map(\.identifier)) + var attached = Set(tableView.tableColumns.map(\.identifier)) let baseOffset = tableView.tableColumns.first?.identifier == ColumnIdentitySchema.rowNumberIdentifier ? 1 : 0 - for slot in targetOrder where !attachedIdentifiers.contains(pooledColumns[slot].identifier) { + for slot in targetOrder where !attached.contains(pooledColumns[slot].identifier) { tableView.addTableColumn(pooledColumns[slot]) + attached.insert(pooledColumns[slot].identifier) } - for slot in 0..= visibleCount && !attachedIdentifiers.contains(pooledColumns[slot].identifier) { + for slot in 0..= visibleCount && !attached.contains(pooledColumns[slot].identifier) { tableView.addTableColumn(pooledColumns[slot]) + attached.insert(pooledColumns[slot].identifier) } + NSAnimationContext.beginGrouping() + NSAnimationContext.current.duration = 0 + NSAnimationContext.current.allowsImplicitAnimation = false + defer { NSAnimationContext.endGrouping() } + for (targetPosition, slot) in targetOrder.enumerated() { let identifier = ColumnIdentitySchema.slotIdentifier(slot) guard let currentIndex = tableView.tableColumns.firstIndex(where: { $0.identifier == identifier }) else { diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 723b11a15..f4b7d4e00 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -214,6 +214,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData cachedRowCount = 0 cachedColumnCount = 0 sortedIDs = nil + columnPool.detachFromTableView() if let tableView { while let col = tableView.tableColumns.last { tableView.removeTableColumn(col) diff --git a/TableProTests/Views/Results/DataGridColumnPoolTests.swift b/TableProTests/Views/Results/DataGridColumnPoolTests.swift index 52153058b..898ece5d9 100644 --- a/TableProTests/Views/Results/DataGridColumnPoolTests.swift +++ b/TableProTests/Views/Results/DataGridColumnPoolTests.swift @@ -364,4 +364,38 @@ struct DataGridColumnPoolTests { #expect(beforeIdentifiers == afterIdentifiers) #expect(beforeWidths == afterWidths) } + + @Test("detachFromTableView removes pool columns and allows clean re-attach") + func detachFromTableView_removesPoolColumnsAndAllowsCleanReattach() { + let pool = DataGridColumnPool() + let tableView = makeTableView() + let schema = ColumnIdentitySchema(columns: ["id", "name", "email"]) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + #expect(dataColumns(in: tableView).count == 3) + + pool.detachFromTableView() + #expect(dataColumns(in: tableView).count == 0) + #expect(pool.totalSlots == 3) + + pool.reconcile( + tableView: tableView, + schema: schema, + columnTypes: makeColumnTypes(count: 3), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: defaultWidthCalculator + ) + #expect(dataColumns(in: tableView).count == 3) + #expect(pool.totalSlots == 3) + } }