Skip to content

Commit 0631570

Browse files
authored
refactor(datagrid): persistent column pool + typed cell hierarchy (#951)
* perf(datagrid): add OSLog signposts to measure cell rendering hot path * refactor(datagrid): add typed cell hierarchy + registry foundation * refactor(datagrid): persistent column pool with slot-based identifiers * refactor(datagrid): wire registry + pool, delete mega-cell * fix(datagrid): set cell text-field delegate once, drop redundant per-render mutations * fix(datagrid): filter saved column order to current schema before moveColumn * fix(datagrid): reset slot order on reconcile, skip idempotent header rebuild * fix(datagrid): attach columns in saved order on first growth, eliminate reorder flicker * fix(datagrid): detect column schema change via names not slot identifiers * fix(datagrid): suppress moveColumn animation during reconcile to prevent visible swap * test(datagrid): add DataGridColumnPool and DataGridCellRegistry tests * docs(datagrid): drop perf instrumentation, add Phase 2 design doc and changelog * docs(datagrid): remove Phase 2 design doc * fix(datagrid): always reconcile columns when data present, drop stale layout-skip optimization * fix(datagrid): scope move animation to moveColumn calls + add pool teardown hook
1 parent 6c69fab commit 0631570

26 files changed

Lines changed: 1731 additions & 880 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2424

2525
### Changed
2626

27+
- 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.
2728
- 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.
2829
- `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.
2930
- 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.

TablePro/Models/UI/ColumnIdentitySchema.swift

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,31 +7,33 @@ import AppKit
77

88
struct ColumnIdentitySchema: Equatable {
99
static let rowNumberIdentifier = NSUserInterfaceItemIdentifier("__rowNumber__")
10+
static let dataColumnPrefix = "dataColumn-"
1011

1112
let identifiers: [NSUserInterfaceItemIdentifier]
12-
let isNameBased: Bool
13+
let columnNames: [String]
14+
1315
private let indexByRawIdentifier: [String: Int]
16+
private let slotByColumnName: [String: Int]
1417

1518
init(columns: [String]) {
16-
let canUseNames = Set(columns).count == columns.count
17-
&& !columns.contains(Self.rowNumberIdentifier.rawValue)
18-
19-
if canUseNames {
20-
self.identifiers = columns.map { NSUserInterfaceItemIdentifier($0) }
21-
self.isNameBased = true
22-
} else {
23-
self.identifiers = columns.indices.map {
24-
NSUserInterfaceItemIdentifier("col_\($0)")
25-
}
26-
self.isNameBased = false
19+
self.columnNames = columns
20+
self.identifiers = columns.indices.map {
21+
NSUserInterfaceItemIdentifier("\(Self.dataColumnPrefix)\($0)")
2722
}
2823

29-
var map: [String: Int] = [:]
30-
map.reserveCapacity(self.identifiers.count)
24+
var rawMap: [String: Int] = [:]
25+
rawMap.reserveCapacity(self.identifiers.count)
3126
for (index, identifier) in self.identifiers.enumerated() {
32-
map[identifier.rawValue] = index
27+
rawMap[identifier.rawValue] = index
28+
}
29+
self.indexByRawIdentifier = rawMap
30+
31+
var nameMap: [String: Int] = [:]
32+
nameMap.reserveCapacity(columns.count)
33+
for (index, name) in columns.enumerated() {
34+
nameMap[name] = index
3335
}
34-
self.indexByRawIdentifier = map
36+
self.slotByColumnName = nameMap
3537
}
3638

3739
static let empty = ColumnIdentitySchema(columns: [])
@@ -44,4 +46,17 @@ struct ColumnIdentitySchema: Equatable {
4446
func dataIndex(from identifier: NSUserInterfaceItemIdentifier) -> Int? {
4547
indexByRawIdentifier[identifier.rawValue]
4648
}
49+
50+
func columnName(for dataIndex: Int) -> String? {
51+
guard dataIndex >= 0, dataIndex < columnNames.count else { return nil }
52+
return columnNames[dataIndex]
53+
}
54+
55+
func dataIndex(forColumnName name: String) -> Int? {
56+
slotByColumnName[name]
57+
}
58+
59+
static func slotIdentifier(_ slot: Int) -> NSUserInterfaceItemIdentifier {
60+
NSUserInterfaceItemIdentifier("\(dataColumnPrefix)\(slot)")
61+
}
4762
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//
2+
// AccessoryButtons.swift
3+
// TablePro
4+
//
5+
6+
import AppKit
7+
8+
@MainActor
9+
final class FKArrowButton: NSButton {
10+
var fkRow: Int = -1
11+
var fkColumnIndex: Int = -1
12+
}
13+
14+
@MainActor
15+
final class CellChevronButton: NSButton {
16+
var cellRow: Int = -1
17+
var cellColumnIndex: Int = -1
18+
}
19+
20+
@MainActor
21+
enum AccessoryButtonFactory {
22+
static func makeFKArrowButton() -> FKArrowButton {
23+
let button = FKArrowButton()
24+
button.bezelStyle = .inline
25+
button.isBordered = false
26+
button.image = NSImage(
27+
systemSymbolName: "arrow.right.circle.fill",
28+
accessibilityDescription: String(localized: "Navigate to referenced row")
29+
)
30+
button.contentTintColor = .tertiaryLabelColor
31+
button.translatesAutoresizingMaskIntoConstraints = false
32+
button.setContentHuggingPriority(.required, for: .horizontal)
33+
button.setContentCompressionResistancePriority(.required, for: .horizontal)
34+
button.imageScaling = .scaleProportionallyDown
35+
button.isHidden = true
36+
return button
37+
}
38+
39+
static func makeChevronButton() -> CellChevronButton {
40+
let chevron = CellChevronButton()
41+
chevron.bezelStyle = .inline
42+
chevron.isBordered = false
43+
chevron.image = NSImage(
44+
systemSymbolName: "chevron.up.chevron.down",
45+
accessibilityDescription: String(localized: "Open editor")
46+
)
47+
chevron.contentTintColor = .tertiaryLabelColor
48+
chevron.translatesAutoresizingMaskIntoConstraints = false
49+
chevron.setContentHuggingPriority(.required, for: .horizontal)
50+
chevron.setContentCompressionResistancePriority(.required, for: .horizontal)
51+
chevron.imageScaling = .scaleProportionallyDown
52+
chevron.isHidden = true
53+
return chevron
54+
}
55+
}
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
//
2+
// DataGridBaseCellView.swift
3+
// TablePro
4+
//
5+
6+
import AppKit
7+
import QuartzCore
8+
9+
class DataGridBaseCellView: NSTableCellView {
10+
class var reuseIdentifier: NSUserInterfaceItemIdentifier {
11+
fatalError("subclass must override reuseIdentifier")
12+
}
13+
14+
let cellTextField: CellTextField
15+
weak var accessoryDelegate: DataGridCellAccessoryDelegate?
16+
var nullDisplayString: String = ""
17+
var cellRow: Int = -1
18+
var cellColumnIndex: Int = -1
19+
20+
private var textFieldTrailingConstraint: NSLayoutConstraint!
21+
22+
var changeBackgroundColor: NSColor? {
23+
didSet {
24+
if let color = changeBackgroundColor {
25+
backgroundView.layer?.backgroundColor = color.cgColor
26+
backgroundView.isHidden = (backgroundStyle == .emphasized)
27+
} else {
28+
backgroundView.layer?.backgroundColor = nil
29+
backgroundView.isHidden = true
30+
}
31+
}
32+
}
33+
34+
var isFocusedCell: Bool = false {
35+
didSet {
36+
guard oldValue != isFocusedCell else { return }
37+
updateFocusRing()
38+
}
39+
}
40+
41+
private(set) lazy var backgroundView: NSView = {
42+
let view = NSView()
43+
view.wantsLayer = true
44+
view.translatesAutoresizingMaskIntoConstraints = false
45+
addSubview(view, positioned: .below, relativeTo: subviews.first)
46+
NSLayoutConstraint.activate([
47+
view.leadingAnchor.constraint(equalTo: leadingAnchor),
48+
view.trailingAnchor.constraint(equalTo: trailingAnchor),
49+
view.topAnchor.constraint(equalTo: topAnchor),
50+
view.bottomAnchor.constraint(equalTo: bottomAnchor),
51+
])
52+
view.isHidden = true
53+
return view
54+
}()
55+
56+
required override init(frame frameRect: NSRect) {
57+
cellTextField = Self.makeTextField()
58+
super.init(frame: frameRect)
59+
commonInit()
60+
}
61+
62+
required init?(coder: NSCoder) {
63+
cellTextField = Self.makeTextField()
64+
super.init(coder: coder)
65+
commonInit()
66+
}
67+
68+
private static func makeTextField() -> CellTextField {
69+
let field = CellTextField()
70+
field.font = ThemeEngine.shared.dataGridFonts.regular
71+
field.drawsBackground = false
72+
field.isBordered = false
73+
field.focusRingType = .none
74+
field.lineBreakMode = .byTruncatingTail
75+
field.maximumNumberOfLines = 1
76+
field.cell?.truncatesLastVisibleLine = true
77+
field.cell?.usesSingleLineMode = true
78+
field.translatesAutoresizingMaskIntoConstraints = false
79+
return field
80+
}
81+
82+
private func commonInit() {
83+
wantsLayer = true
84+
textField = cellTextField
85+
addSubview(cellTextField)
86+
87+
textFieldTrailingConstraint = cellTextField.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -4)
88+
89+
NSLayoutConstraint.activate([
90+
cellTextField.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 4),
91+
textFieldTrailingConstraint,
92+
cellTextField.centerYAnchor.constraint(equalTo: centerYAnchor),
93+
])
94+
95+
installAccessory()
96+
}
97+
98+
func configure(content: DataGridCellContent, state: DataGridCellState) {
99+
cellRow = state.row
100+
cellColumnIndex = state.columnIndex
101+
102+
applyContent(content, isLargeDataset: state.isLargeDataset)
103+
applyVisualState(state)
104+
105+
cellTextField.isEditable = state.isEditable && !state.visualState.isDeleted
106+
107+
let newInset = textFieldTrailingInset(for: content, state: state)
108+
if textFieldTrailingConstraint.constant != newInset {
109+
textFieldTrailingConstraint.constant = newInset
110+
}
111+
112+
updateAccessoryVisibility(content: content, state: state)
113+
114+
cellTextField.setAccessibilityLabel(content.accessibilityLabel)
115+
setAccessibilityRowIndexRange(NSRange(location: state.row, length: 1))
116+
setAccessibilityColumnIndexRange(NSRange(location: state.columnIndex, length: 1))
117+
}
118+
119+
func installAccessory() {}
120+
121+
func updateAccessoryVisibility(content: DataGridCellContent, state: DataGridCellState) {}
122+
123+
func textFieldTrailingInset(for content: DataGridCellContent, state: DataGridCellState) -> CGFloat {
124+
-4
125+
}
126+
127+
private func applyContent(_ content: DataGridCellContent, isLargeDataset: Bool) {
128+
cellTextField.placeholderString = nil
129+
130+
switch content.placeholder {
131+
case .none:
132+
cellTextField.stringValue = content.displayText
133+
cellTextField.originalValue = content.rawValue
134+
cellTextField.font = ThemeEngine.shared.dataGridFonts.regular
135+
cellTextField.tag = DataGridFontVariant.regular
136+
cellTextField.textColor = .labelColor
137+
138+
case .null:
139+
cellTextField.stringValue = ""
140+
cellTextField.originalValue = nil
141+
cellTextField.font = ThemeEngine.shared.dataGridFonts.italic
142+
cellTextField.tag = DataGridFontVariant.italic
143+
cellTextField.textColor = .secondaryLabelColor
144+
if !isLargeDataset {
145+
cellTextField.placeholderString = nullDisplayString
146+
}
147+
148+
case .empty:
149+
cellTextField.stringValue = ""
150+
cellTextField.originalValue = nil
151+
cellTextField.font = ThemeEngine.shared.dataGridFonts.italic
152+
cellTextField.tag = DataGridFontVariant.italic
153+
cellTextField.textColor = .secondaryLabelColor
154+
if !isLargeDataset {
155+
cellTextField.placeholderString = String(localized: "Empty")
156+
}
157+
158+
case .defaultMarker:
159+
cellTextField.stringValue = ""
160+
cellTextField.originalValue = nil
161+
cellTextField.font = ThemeEngine.shared.dataGridFonts.medium
162+
cellTextField.tag = DataGridFontVariant.medium
163+
cellTextField.textColor = .systemBlue
164+
if !isLargeDataset {
165+
cellTextField.placeholderString = String(localized: "DEFAULT")
166+
}
167+
}
168+
}
169+
170+
private func applyVisualState(_ state: DataGridCellState) {
171+
CATransaction.begin()
172+
CATransaction.setDisableActions(true)
173+
174+
if state.visualState.isDeleted {
175+
changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.deleted
176+
} else if state.visualState.isInserted {
177+
changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.inserted
178+
} else if state.visualState.modifiedColumns.contains(state.columnIndex) {
179+
changeBackgroundColor = ThemeEngine.shared.colors.dataGrid.modified
180+
} else {
181+
changeBackgroundColor = nil
182+
}
183+
184+
isFocusedCell = state.isFocused
185+
186+
CATransaction.commit()
187+
}
188+
189+
override var backgroundStyle: NSView.BackgroundStyle {
190+
didSet {
191+
backgroundView.isHidden = (backgroundStyle == .emphasized) || (changeBackgroundColor == nil)
192+
if isFocusedCell { updateFocusRing() }
193+
}
194+
}
195+
196+
override var focusRingMaskBounds: NSRect { bounds }
197+
198+
override func drawFocusRingMask() {
199+
NSBezierPath(rect: bounds).fill()
200+
}
201+
202+
override func draw(_ dirtyRect: NSRect) {
203+
super.draw(dirtyRect)
204+
guard isFocusedCell, backgroundStyle != .emphasized else { return }
205+
NSGraphicsContext.saveGraphicsState()
206+
NSFocusRingPlacement.only.set()
207+
drawFocusRingMask()
208+
NSGraphicsContext.restoreGraphicsState()
209+
}
210+
211+
override func viewDidChangeEffectiveAppearance() {
212+
super.viewDidChangeEffectiveAppearance()
213+
if isFocusedCell {
214+
needsDisplay = true
215+
}
216+
}
217+
218+
private func updateFocusRing() {
219+
focusRingType = isFocusedCell ? .exterior : .none
220+
noteFocusRingMaskChanged()
221+
needsDisplay = true
222+
}
223+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//
2+
// DataGridBlobCellView.swift
3+
// TablePro
4+
//
5+
6+
import AppKit
7+
8+
final class DataGridBlobCellView: DataGridChevronCellView {
9+
override class var reuseIdentifier: NSUserInterfaceItemIdentifier {
10+
NSUserInterfaceItemIdentifier("dataCell.blob")
11+
}
12+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//
2+
// DataGridBooleanCellView.swift
3+
// TablePro
4+
//
5+
6+
import AppKit
7+
8+
final class DataGridBooleanCellView: DataGridChevronCellView {
9+
override class var reuseIdentifier: NSUserInterfaceItemIdentifier {
10+
NSUserInterfaceItemIdentifier("dataCell.boolean")
11+
}
12+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//
2+
// DataGridCellAccessoryDelegate.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
8+
@MainActor
9+
protocol DataGridCellAccessoryDelegate: AnyObject {
10+
func dataGridCellDidClickFKArrow(row: Int, columnIndex: Int)
11+
func dataGridCellDidClickChevron(row: Int, columnIndex: Int)
12+
}

0 commit comments

Comments
 (0)