From d6eacafee1db0798377721ed4510de9af8088d34 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 22 Aug 2026 20:18:17 +0700 Subject: [PATCH 1/7] fix(datagrid): keep the column window with the viewport on a wide result (#2381) --- CHANGELOG.md | 8 + .../MainContentCoordinator+FKNavigation.swift | 2 +- .../Views/Results/ColumnWindowResolver.swift | 20 ++ .../Views/Results/DataGridColumnPool.swift | 92 +++++- .../Views/Results/DataGridCoordinator.swift | 52 +++- TablePro/Views/Results/DataGridRowView.swift | 2 +- TablePro/Views/Results/DataGridView.swift | 6 - .../Extensions/DataGridView+Editing.swift | 53 ++-- .../Extensions/DataGridView+Selection.swift | 4 +- .../Extensions/DataGridView+Sort.swift | 5 + .../Views/Results/KeyHandlingTableView.swift | 74 ++--- .../Results/ColumnWindowResolverTests.swift | 35 +++ .../Results/DataGridColumnPoolTests.swift | 278 +++++++++++++++++- .../Results/DataGridRowViewCopyTests.swift | 26 +- .../FocusedColumnResolutionTests.swift | 89 +++++- 15 files changed, 643 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08b8665e6..d690c45eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Flickering columns, blank columns, and an unpainted gap while scrolling a result with about 100 columns sideways. (#2381) +- Find, arrow keys, and the inline editor unable to reach a column scrolled off the side of a wide result. +- Return opening no editor on a row selected with the arrow keys. +- Tab out of a row's last cell and Shift+Tab out of its first doing nothing. +- Size All Columns to Fit leaving the far columns of a wide result unreachable. + ## [0.67.1] - 2026-08-22 ### Added diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+FKNavigation.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+FKNavigation.swift index 600d6b941..f3ca8df6d 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+FKNavigation.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+FKNavigation.swift @@ -117,7 +117,7 @@ extension MainContentCoordinator { guard let tableView = NSApp.keyWindow?.firstResponder as? KeyHandlingTableView, let coordinator = tableView.coordinator, tableView.selectedRow >= 0, - DataGridView.isDataTableColumn(tableView.focusedColumn), + tableView.presentsDataColumn(at: tableView.focusedColumn), let columnIndex = DataGridView.dataColumnIndex( for: tableView.focusedColumn, in: tableView, diff --git a/TablePro/Views/Results/ColumnWindowResolver.swift b/TablePro/Views/Results/ColumnWindowResolver.swift index 5cc3f58e8..29494f041 100644 --- a/TablePro/Views/Results/ColumnWindowResolver.swift +++ b/TablePro/Views/Results/ColumnWindowResolver.swift @@ -68,6 +68,26 @@ internal enum ColumnWindowResolver { return window(for: desired, columnWidths: columnWidths) } + /// A window over `index`, for a caller that needs a column's frame before the viewport has + /// reached it. + /// + /// Re-centres rather than stretching the mounted range out to reach the target. Spanning from + /// the current range to a far column mounts every column in between, which is the whole cost + /// the window exists to avoid: measured at 848ms and 3,081 cell views for one Find match 90 + /// columns away, and 4.8s at 500 columns. The caller scrolls in the same turn, so the columns + /// this drops were never drawn again anyway. + /// + /// - Returns: `nil` when the range already covers `index` and there is nothing to mount. + internal static func window( + containing index: Int, + columnWidths: [CGFloat], + current: Range? + ) -> Window? { + guard columnWidths.indices.contains(index) else { return nil } + if let current, current.contains(index) { return nil } + return window(for: padded(index..<(index + 1), count: columnWidths.count), columnWidths: columnWidths) + } + /// The columns the viewport actually intersects. Always at least one column, so a viewport /// narrower than a single column still mounts the one under it. private static func visibleRange( diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift index e506f56a6..8b517b9c9 100644 --- a/TablePro/Views/Results/DataGridColumnPool.swift +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -154,9 +154,7 @@ final class DataGridColumnPool { /// Hiding the far ones is measurably close to free; the spacers keep the document width and /// therefore the scroll extent identical to mounting everything. func applyColumnWindow(in tableView: NSTableView) { - let candidates = tableView.tableColumns.filter { - activeIdentifiers.contains($0.identifier) && !userHiddenIdentifiers.contains($0.identifier) - } + let candidates = presentedColumns(in: tableView) guard !candidates.isEmpty else { hideSpacers() return @@ -168,36 +166,106 @@ final class DataGridColumnPool { return } - // The document starts at the row-number column, but the resolver measures from the first - // data column, so the viewport has to be rebased before it can index into these widths. - let leadingChrome = tableView.tableColumns - .prefix { $0.identifier != candidates[0].identifier } - .filter { !$0.isHidden } - .reduce(0) { $0 + $1.width + tableView.intercellSpacing.width } - // A mounted column contributes its width plus one intercell gap; a hidden one contributes // nothing. A spacer standing in for N columns has to carry their gaps too, or the document // ends up short and the last columns cannot be reached. let spacing = tableView.intercellSpacing.width let window = ColumnWindowResolver.resolve( columnWidths: candidates.map { $0.width + spacing }, - viewportMinX: viewport.minX - leadingChrome, + viewportMinX: viewport.minX - leadingChromeWidth(in: tableView, before: candidates[0]), viewportWidth: viewport.width, current: windowedRange ) guard window.range != windowedRange else { return } - windowedRange = window.range + mount(window, over: candidates, in: tableView) + } + + /// Mounts a column the window left out, so anything that reads its frame gets a real rect. + /// + /// `rect(ofColumn:)` and `frameOfCell(atColumn:row:)` are both empty for a hidden column, so + /// `scrollColumnToVisible` scrolls to the document origin instead of the column, and the inline + /// editor's own empty-frame guard makes it open nothing at all (#2381). + /// - Returns: whether the window had to widen, so the caller can drop it and let the next + /// resolve pick a tight one instead of leaving the widened range mounted. + @discardableResult + func mountColumn(_ column: NSTableColumn, in tableView: NSTableView) -> Bool { + guard presentsColumn(column) else { return false } + let candidates = presentedColumns(in: tableView) + guard let position = candidates.firstIndex(of: column) else { return false } + + let spacing = tableView.intercellSpacing.width + guard let window = ColumnWindowResolver.window( + containing: position, + columnWidths: candidates.map { $0.width + spacing }, + current: windowedRange + ) else { return false } + mount(window, over: candidates, in: tableView) + return true + } + + /// The first and last columns the result presents, in display order. The pool owns these + /// because the spacers are attached columns too and one of them sits immediately before the + /// first data column, so no fixed position can name either end. + func firstPresentedColumnIndex(in tableView: NSTableView) -> Int? { + tableView.tableColumns.firstIndex { presentsColumn($0) } + } + + func lastPresentedColumnIndex(in tableView: NSTableView) -> Int? { + tableView.tableColumns.lastIndex { presentsColumn($0) } + } + func nextPresentedColumnIndex(after index: Int, in tableView: NSTableView) -> Int? { + let start = max(0, index + 1) + guard start < tableView.tableColumns.count else { return nil } + return tableView.tableColumns[start...].firstIndex { presentsColumn($0) } + } + + func previousPresentedColumnIndex(before index: Int, in tableView: NSTableView) -> Int? { + let end = min(max(0, index), tableView.tableColumns.count) + guard end > 0 else { return nil } + return tableView.tableColumns[.. Bool { + guard index >= 0, index < tableView.tableColumns.count else { return false } + return presentsColumn(tableView.tableColumns[index]) + } + + private func presentedColumns(in tableView: NSTableView) -> [NSTableColumn] { + tableView.tableColumns.filter { presentsColumn($0) } + } + + private func mount( + _ window: ColumnWindowResolver.Window, + over candidates: [NSTableColumn], + in tableView: NSTableView + ) { + windowedRange = window.range for (index, column) in candidates.enumerated() { let mounted = window.range.contains(index) if column.isHidden == mounted { column.isHidden = !mounted } } + let spacing = tableView.intercellSpacing.width applySpacer(leadingSpacer, width: spacerWidth(window.leadingWidth, spacing: spacing), in: tableView) applySpacer(trailingSpacer, width: spacerWidth(window.trailingWidth, spacing: spacing), in: tableView) } + /// The document starts at the row-number column, but the resolver measures from the first data + /// column, so the viewport has to be rebased before it can index into those widths. + /// + /// Only chrome counts. The leading spacer sits ahead of the first data column and holds exactly + /// the width of the columns the window left out, which the resolver's own model already carries, + /// so counting it here subtracts that width twice and walks the window left while the reader + /// scrolls right, until it parks off screen and the grid paints nothing (#2381). + private func leadingChromeWidth(in tableView: NSTableView, before firstColumn: NSTableColumn) -> CGFloat { + tableView.tableColumns + .prefix { $0.identifier != firstColumn.identifier } + .filter { !$0.isHidden && !ColumnIdentitySchema.isSpacer($0.identifier) } + .reduce(0) { $0 + $1.width + tableView.intercellSpacing.width } + } + /// The resolver works in per-column slots that already include one gap each. A spacer is a /// single column, so it keeps one gap of its own and absorbs the rest as width. private func spacerWidth(_ slotWidth: CGFloat, spacing: CGFloat) -> CGFloat { diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 740a09453..32f83be5c 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -132,6 +132,52 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData columnPool.presentsColumn(column) } + /// Whether this position in `tableColumns` holds one of the columns the result presents. + /// + /// The row-number column and the window's two spacers are attached columns as well, and one + /// spacer sits immediately before the first data column, so no fixed position answers this. + func presentsColumn(atTableColumnIndex index: Int) -> Bool { + guard let tableView else { return false } + return columnPool.presentsColumn(atTableColumnIndex: index, in: tableView) + } + + func firstPresentedColumnIndex() -> Int? { + guard let tableView else { return nil } + return columnPool.firstPresentedColumnIndex(in: tableView) + } + + func lastPresentedColumnIndex() -> Int? { + guard let tableView else { return nil } + return columnPool.lastPresentedColumnIndex(in: tableView) + } + + func nextPresentedColumnIndex(after index: Int) -> Int? { + guard let tableView else { return nil } + return columnPool.nextPresentedColumnIndex(after: index, in: tableView) + } + + func previousPresentedColumnIndex(before index: Int) -> Int? { + guard let tableView else { return nil } + return columnPool.previousPresentedColumnIndex(before: index, in: tableView) + } + + /// The single way to reach a column, for Find, cell navigation and the inline editor alike. + /// + /// A column the window left out has no frame at all, so `scrollColumnToVisible` scrolls to the + /// document origin instead of the column and the editor's own empty-frame guard opens nothing. + /// Mounting first gives it one. Only a mount that had to widen the window drops it afterwards, + /// so stepping column by column keeps the resolver's hysteresis instead of re-windowing on + /// every keystroke. + func scrollColumnToVisible(tableColumnIndex index: Int) { + guard let tableView, index >= 0, index < tableView.numberOfColumns else { return } + let widened = columnPool.mountColumn(tableView.tableColumns[index], in: tableView) + tableView.scrollColumnToVisible(index) + if widened { + columnPool.invalidateColumnWindow() + } + updateColumnWindow() + } + /// The columns the user is looking at, which is every presented column and not merely the /// mounted ones. Copy, find and size-all all read this, so narrowing it to the window would /// silently drop the columns off screen from a copied row or a search. @@ -1080,9 +1126,8 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData guard let match, match.displayRow >= 0, match.displayRow < tableView.numberOfRows else { return } tableView.scrollRowToVisible(match.displayRow) - if let displayColumn = tableColumnIndex(for: match.columnIndex), - displayColumn < tableView.numberOfColumns { - tableView.scrollColumnToVisible(displayColumn) + if let displayColumn = tableColumnIndex(for: match.columnIndex) { + scrollColumnToVisible(tableColumnIndex: displayColumn) } tableView.selectRowIndexes(IndexSet(integer: match.displayRow), byExtendingSelection: false) } @@ -1094,6 +1139,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData guard displayRow >= 0, displayRow < tableView.numberOfRows else { return } tableView.scrollRowToVisible(displayRow) tableView.selectRowIndexes(IndexSet(integer: displayRow), byExtendingSelection: false) + scrollColumnToVisible(tableColumnIndex: displayCol) beginCellEdit(row: displayRow, tableColumnIndex: displayCol) } diff --git a/TablePro/Views/Results/DataGridRowView.swift b/TablePro/Views/Results/DataGridRowView.swift index e331360b0..3e2ce0c00 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -462,7 +462,7 @@ class DataGridRowView: NSTableRowView { private func focusedDataColumnIndex(in coordinator: TableViewCoordinator) -> Int? { guard let tableView = coordinator.tableView as? KeyHandlingTableView, tableView.focusedRow == rowIndex, - DataGridView.isDataTableColumn(tableView.focusedColumn) else { return nil } + tableView.presentsDataColumn(at: tableView.focusedColumn) else { return nil } return DataGridView.dataColumnIndex( for: tableView.focusedColumn, in: tableView, diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index b5a12a3fb..5d5e4528c 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -427,12 +427,6 @@ struct DataGridView: NSViewRepresentable { tableView.selectionOverlay = overlay } - static let firstDataTableColumnIndex: Int = 1 - - static func isDataTableColumn(_ tableColumnIndex: Int) -> Bool { - tableColumnIndex >= firstDataTableColumnIndex - } - static func dataColumnIndex( for tableColumnIndex: Int, in tableView: NSTableView, diff --git a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift index d69bd27ae..2e40cca0a 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift @@ -117,32 +117,14 @@ extension TableViewCoordinator { } func handleOverlayTabNavigation(row: Int, column: Int, forward: Bool) { - guard let tableView = tableView else { return } - - var nextColumn = forward ? column + 1 : column - 1 - var nextRow = row - - if forward { - if nextColumn >= tableView.numberOfColumns { - nextColumn = DataGridView.firstDataTableColumnIndex - nextRow += 1 - } - if nextRow >= tableView.numberOfRows { - nextRow = tableView.numberOfRows - 1 - nextColumn = tableView.numberOfColumns - 1 - } - } else { - if !DataGridView.isDataTableColumn(nextColumn) { - nextColumn = tableView.numberOfColumns - 1 - nextRow -= 1 - } - if nextRow < 0 { - nextRow = 0 - nextColumn = DataGridView.firstDataTableColumnIndex - } - } + guard let tableView = tableView, + let target = tabNavigationTarget(from: (row, column), forward: forward, in: tableView) + else { return } + let nextRow = target.row + let nextColumn = target.column tableView.selectRowIndexes(IndexSet(integer: nextRow), byExtendingSelection: false) + scrollColumnToVisible(tableColumnIndex: nextColumn) guard let nextColumnIndex = DataGridView.dataColumnIndex( for: nextColumn, @@ -161,4 +143,27 @@ extension TableViewCoordinator { value: value ) } + + /// Tab walks the presented columns and wraps onto the next row's first, Shift+Tab onto the + /// previous row's last. Both ends are resolved rather than assumed: the window's spacers and + /// the pool's surplus slots are attached columns too, so neither end of `tableColumns` holds a + /// data column and a fixed position lands on a spacer that swallows the keystroke. + private func tabNavigationTarget( + from cell: (row: Int, column: Int), + forward: Bool, + in tableView: NSTableView + ) -> (row: Int, column: Int)? { + if forward { + if let next = nextPresentedColumnIndex(after: cell.column) { + return (cell.row, next) + } + guard cell.row + 1 < tableView.numberOfRows, let first = firstPresentedColumnIndex() else { return nil } + return (cell.row + 1, first) + } + if let previous = previousPresentedColumnIndex(before: cell.column) { + return (cell.row, previous) + } + guard cell.row > 0, let last = lastPresentedColumnIndex() else { return nil } + return (cell.row - 1, last) + } } diff --git a/TablePro/Views/Results/Extensions/DataGridView+Selection.swift b/TablePro/Views/Results/Extensions/DataGridView+Selection.swift index d771788c1..e8bab696c 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Selection.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Selection.swift @@ -102,7 +102,9 @@ extension TableViewCoordinator { return (-1, -1) } - let column = existingFocusedColumn >= 1 ? existingFocusedColumn : 1 + let column = presentsColumn(atTableColumnIndex: existingFocusedColumn) + ? existingFocusedColumn + : (firstPresentedColumnIndex() ?? -1) let added = current.subtracting(previous) if let tip = added.max() { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift index d886e5fc2..ca81bbe46 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift @@ -371,6 +371,11 @@ extension TableViewCoordinator { fittedColumnCount: fittedColumns.count ) } + // Sizing reaches the columns the window unmounted as well, and the spacers stand in for + // those at the width they had before, so the document is short by the whole delta until + // the window is resolved again. + columnPool.invalidateColumnWindow() + updateColumnWindow() scheduleLayoutPersist() } diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index 37aebb9bf..81cae8026 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -214,7 +214,7 @@ final class KeyHandlingTableView: NSTableView { } private func clampDataColumn(_ value: Int, schema: ColumnIdentitySchema) -> Int { - let firstData = DataGridView.firstDataTableColumnIndex + let firstData = firstVisibleDataColumn() let candidate = value < firstData ? firstData : value guard candidate >= 0, candidate < numberOfColumns else { return -1 } return DataGridView.dataColumnIndex(for: candidate, in: self, schema: schema) ?? -1 @@ -258,7 +258,7 @@ final class KeyHandlingTableView: NSTableView { private func focusedDataCell() -> (row: Int, columnIndex: Int)? { guard selectedRowIndexes.count == 1, focusedRow >= 0, - DataGridView.isDataTableColumn(focusedColumn), + presentsDataColumn(at: focusedColumn), let schema = coordinator?.identitySchema, let dataColumn = DataGridView.dataColumnIndex(for: focusedColumn, in: self, schema: schema) else { return nil @@ -279,7 +279,7 @@ final class KeyHandlingTableView: NSTableView { /// requires a single selected row: a paste anchors on the focused cell alone. private func pasteAnchorCell() -> (row: Int, column: Int)? { guard focusedRow >= 0, - DataGridView.isDataTableColumn(focusedColumn), + presentsDataColumn(at: focusedColumn), let schema = coordinator?.identitySchema, let dataCol = DataGridView.dataColumnIndex(for: focusedColumn, in: self, schema: schema) else { return nil @@ -312,7 +312,7 @@ final class KeyHandlingTableView: NSTableView { case #selector(paste(_:)): return canPaste case #selector(insertNewline(_:)): - return selectedRow >= 0 && DataGridView.isDataTableColumn(focusedColumn) + return selectedRow >= 0 && presentsDataColumn(at: focusedColumn) case #selector(selectAll(_:)): return numberOfRows > 0 default: @@ -358,7 +358,7 @@ final class KeyHandlingTableView: NSTableView { !fkCombo.isCleared, fkCombo.matches(event), selectedRow >= 0, - DataGridView.isDataTableColumn(focusedColumn), + presentsDataColumn(at: focusedColumn), let schema = coordinator?.identitySchema, let columnIndex = DataGridView.dataColumnIndex(for: focusedColumn, in: self, schema: schema) { coordinator?.toggleForeignKeyPreview( @@ -418,12 +418,16 @@ final class KeyHandlingTableView: NSTableView { @objc override func insertNewline(_ sender: Any?) { let row = selectedRow guard row >= 0, - DataGridView.isDataTableColumn(focusedColumn), + presentsDataColumn(at: focusedColumn), let schema = coordinator?.identitySchema, let columnIndex = DataGridView.dataColumnIndex(for: focusedColumn, in: self, schema: schema), let coordinator else { return } + // The cell cursor can sit on a column the window left out, and a cell with no view behind + // it opens nothing at all. Reaching it first also puts it on screen, where the editor the + // keystroke is about to open belongs. + coordinator.scrollColumnToVisible(tableColumnIndex: focusedColumn) coordinator.handleCellInteraction(row: row, tableColumn: focusedColumn, columnIndex: columnIndex, tableView: self) } @@ -445,64 +449,46 @@ final class KeyHandlingTableView: NSTableView { let target = focusedColumn < 0 ? lastVisibleDataColumn() : previousVisibleDataColumn(before: focusedColumn) - guard DataGridView.isDataTableColumn(target) else { return } + guard presentsDataColumn(at: target) else { return } focusedColumn = target coordinator?.dismissFKPreviewOnColumnChange() - if currentRow >= 0 { scrollColumnToVisible(target) } + if currentRow >= 0 { coordinator?.scrollColumnToVisible(tableColumnIndex: target) } } private func handleRightArrow(currentRow: Int) { - let target = DataGridView.isDataTableColumn(focusedColumn) + let target = presentsDataColumn(at: focusedColumn) ? nextVisibleDataColumn(after: focusedColumn) : firstVisibleDataColumn() - guard DataGridView.isDataTableColumn(target) else { return } + guard presentsDataColumn(at: target) else { return } focusedColumn = target coordinator?.dismissFKPreviewOnColumnChange() - if currentRow >= 0 { scrollColumnToVisible(target) } + if currentRow >= 0 { coordinator?.scrollColumnToVisible(tableColumnIndex: target) } } private func firstVisibleDataColumn() -> Int { - for index in DataGridView.firstDataTableColumnIndex.. Int { - for index in stride( - from: numberOfColumns - 1, - through: DataGridView.firstDataTableColumnIndex, - by: -1 - ) where isVisibleDataColumn(at: index) { - return index - } - return -1 + coordinator?.lastPresentedColumnIndex() ?? -1 } private func nextVisibleDataColumn(after current: Int) -> Int { - guard current + 1 < numberOfColumns else { return -1 } - for index in (current + 1).. Int { - guard current > DataGridView.firstDataTableColumnIndex else { return -1 } - for index in stride( - from: current - 1, - through: DataGridView.firstDataTableColumnIndex, - by: -1 - ) where isVisibleDataColumn(at: index) { - return index - } - return -1 + coordinator?.previousPresentedColumnIndex(before: current) ?? -1 } - private func isVisibleDataColumn(at index: Int) -> Bool { + /// Whether this position in `tableColumns` holds one of the columns the result presents. + /// + /// The row-number column and the window's two spacers are attached columns as well, and one + /// spacer sits immediately before the first data column, so no fixed position answers this. + func presentsDataColumn(at index: Int) -> Bool { guard index >= 0, index < numberOfColumns else { return false } - let column = tableColumns[index] - return coordinator?.presentsColumn(column) ?? !column.isHidden + guard let coordinator else { return !tableColumns[index].isHidden } + return coordinator.presentsColumn(atTableColumnIndex: index) } /// `NSResponder` declares these two but does not implement them, so calling `super` raises @@ -516,7 +502,7 @@ final class KeyHandlingTableView: NSTableView { /// element itself stays the table: `NSTableView`'s own focused-element resolution already /// walks to the cell, and overriding it in Swift is not available on this type. internal func postCellCursorMoved() { - guard selectedRow >= 0, DataGridView.isDataTableColumn(focusedColumn) else { return } + guard selectedRow >= 0, presentsDataColumn(at: focusedColumn) else { return } guard let cell = view(atColumn: focusedColumn, row: selectedRow, makeIfNecessary: false) else { return } NSAccessibility.post(element: cell, notification: .focusedUIElementChanged) } @@ -551,7 +537,7 @@ final class KeyHandlingTableView: NSTableView { private func moveFocusToNextCell() -> Bool { let row = selectedRow - guard row >= 0, DataGridView.isDataTableColumn(focusedColumn) else { return false } + guard row >= 0, presentsDataColumn(at: focusedColumn) else { return false } var nextColumn = nextVisibleDataColumn(after: focusedColumn) var nextRow = row @@ -567,7 +553,7 @@ final class KeyHandlingTableView: NSTableView { private func moveFocusToPreviousCell() -> Bool { let row = selectedRow - guard row >= 0, DataGridView.isDataTableColumn(focusedColumn) else { return false } + guard row >= 0, presentsDataColumn(at: focusedColumn) else { return false } var previousColumn = previousVisibleDataColumn(before: focusedColumn) var previousRow = row @@ -586,7 +572,7 @@ final class KeyHandlingTableView: NSTableView { focusedRow = row focusedColumn = column scrollRowToVisible(row) - scrollColumnToVisible(column) + coordinator?.scrollColumnToVisible(tableColumnIndex: column) postCellCursorMoved() } diff --git a/TableProTests/Views/Results/ColumnWindowResolverTests.swift b/TableProTests/Views/Results/ColumnWindowResolverTests.swift index 5e8fdc1d1..792f09121 100644 --- a/TableProTests/Views/Results/ColumnWindowResolverTests.swift +++ b/TableProTests/Views/Results/ColumnWindowResolverTests.swift @@ -20,6 +20,41 @@ struct ColumnWindowResolverTests { return window.leadingWidth + mounted + window.trailingWidth } + @Test("A range that already covers the column needs no widening") + func containingIsNilWhenAlreadyMounted() { + #expect(ColumnWindowResolver.window(containing: 20, columnWidths: wide, current: 10..<40) == nil) + } + + @Test("An out-of-bounds column widens nothing") + func containingIsNilOutOfBounds() { + #expect(ColumnWindowResolver.window(containing: 500, columnWidths: wide, current: nil) == nil) + } + + /// Stretching the mounted range out to reach a far column would mount every column in between, + /// which is the cost the window exists to avoid. The caller scrolls in the same turn, so the + /// columns it leaves behind are never drawn again. + @Test("Reaching a far column mounts a window around it, not everything in between") + func containingRecentresRatherThanStretches() throws { + let window = try #require( + ColumnWindowResolver.window(containing: 400, columnWidths: wide, current: 0..<30) + ) + + #expect(window.range.contains(400)) + #expect(window.range.count <= ColumnWindowResolver.overscan * 2 + 1) + #expect(totalWidth(window, widths: wide) == wide.reduce(0, +)) + } + + @Test("Reaching a column with no window yet mounts a window around it") + func containingFromNoWindow() throws { + let window = try #require( + ColumnWindowResolver.window(containing: 250, columnWidths: wide, current: nil) + ) + + #expect(window.range.contains(250)) + #expect(window.range.count < 50) + #expect(totalWidth(window, widths: wide) == wide.reduce(0, +)) + } + @Test("No columns resolves to an empty window") func emptyColumns() { let window = ColumnWindowResolver.resolve( diff --git a/TableProTests/Views/Results/DataGridColumnPoolTests.swift b/TableProTests/Views/Results/DataGridColumnPoolTests.swift index b6918b216..3e1a9ab8c 100644 --- a/TableProTests/Views/Results/DataGridColumnPoolTests.swift +++ b/TableProTests/Views/Results/DataGridColumnPoolTests.swift @@ -15,8 +15,12 @@ struct DataGridColumnPoolTests { private func makeTableView() -> NSTableView { let tableView = NSTableView() // Mirrors DataGridView. The default style redistributes column widths on resize, which - // would silently rewrite the widths these tests assert on. + // would silently rewrite the widths these tests assert on, and the default style and + // intercell spacing put the columns at different document positions than the grid's, which + // is the geometry the window is resolved against. tableView.columnAutoresizingStyle = .noColumnAutoresizing + tableView.style = .plain + tableView.intercellSpacing = NSSize(width: 1, height: 0) let rowNumberColumn = NSTableColumn(identifier: ColumnIdentitySchema.rowNumberIdentifier) rowNumberColumn.width = 40 tableView.addTableColumn(rowNumberColumn) @@ -809,4 +813,276 @@ struct DataGridColumnPoolTests { let hasSpacer = tableView.tableColumns.contains(where: { ColumnIdentitySchema.isSpacer($0.identifier) }) #expect(!hasSpacer) } + + // MARK: - Window geometry while scrolling + + private func scroll(_ scrollView: NSScrollView, to offsetX: CGFloat, tableView: NSTableView) { + scrollView.contentView.scroll(to: NSPoint(x: offsetX, y: scrollView.contentView.bounds.origin.y)) + scrollView.reflectScrolledClipView(scrollView.contentView) + tableView.layoutSubtreeIfNeeded() + } + + private func documentWidth(of tableView: NSTableView) -> CGFloat { + tableView.layoutSubtreeIfNeeded() + return tableView.frame.width + } + + /// How much of the viewport, past the row-number column, no mounted data column paints into. + /// + /// Measured through `rect(ofColumn:)`, which is `NSTableView`'s own answer for where a column + /// sits and is `NSZeroRect` for an unmounted one. Asking the resolver instead would only prove + /// the resolver agrees with itself, which is exactly how #2381 shipped. + private func unpaintedViewportWidth(in scrollView: NSScrollView, tableView: NSTableView) -> CGFloat { + let viewport = scrollView.contentView.bounds + let rowNumber = tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) + let contentStart = rowNumber >= 0 + ? max(viewport.minX, tableView.rect(ofColumn: rowNumber).maxX) + : viewport.minX + + var painted: CGFloat = 0 + for column in dataColumns(in: tableView) where !column.isHidden { + let index = tableView.column(withIdentifier: column.identifier) + guard index >= 0 else { continue } + let rect = tableView.rect(ofColumn: index) + painted += max(0, min(rect.maxX, viewport.maxX) - max(rect.minX, contentStart)) + } + return max(0, (viewport.maxX - contentStart) - painted) + } + + private func mountedIdentifiers(in tableView: NSTableView) -> Set { + Set(dataColumns(in: tableView).filter { !$0.isHidden }.map(\.identifier)) + } + + private func scrollOffsets(in scrollView: NSScrollView, tableView: NSTableView) -> [CGFloat] { + let maximum = documentWidth(of: tableView) - scrollView.contentView.bounds.width + guard maximum > 0 else { return [0] } + let forward = Array(stride(from: 0, through: maximum, by: 150)) + [maximum] + return forward + forward.reversed() + } + + /// The reported bug. The window is resolved against a model of the whole column run, so the + /// viewport has to be rebased into that model before it can pick a range. Counting the leading + /// spacer as chrome subtracted the columns it stands in for a second time, and the window + /// walked left while the reader scrolled right until it painted nothing at all. + @Test("The mounted columns cover the viewport at every horizontal scroll offset") + func windowCoversTheViewportWhileScrolling() { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + + var worstGap: CGFloat = 0 + for offset in scrollOffsets(in: scrollView, tableView: tableView) { + scroll(scrollView, to: offset, tableView: tableView) + pool.applyColumnWindow(in: tableView) + tableView.layoutSubtreeIfNeeded() + worstGap = max(worstGap, unpaintedViewportWidth(in: scrollView, tableView: tableView)) + } + + #expect(worstGap == 0) + } + + /// A window that alternates between two ranges re-mounts columns on every scroll event, which + /// is what the reader sees as flicker. + @Test("Resolving again at the same scroll offset settles rather than alternating") + func windowSettlesAtOneOffset() { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + + scroll(scrollView, to: documentWidth(of: tableView) / 2, tableView: tableView) + pool.applyColumnWindow(in: tableView) + tableView.layoutSubtreeIfNeeded() + let settled = mountedIdentifiers(in: tableView) + + for _ in 0..<5 { + pool.applyColumnWindow(in: tableView) + tableView.layoutSubtreeIfNeeded() + } + + #expect(mountedIdentifiers(in: tableView) == settled) + } + + /// The spacers exist to keep the scroll extent, so no window position may change it. + @Test("The document keeps its width at every window position") + func documentWidthSurvivesEveryWindowPosition() { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + let expected = documentWidth(of: tableView) + + for offset in scrollOffsets(in: scrollView, tableView: tableView) { + scroll(scrollView, to: offset, tableView: tableView) + pool.applyColumnWindow(in: tableView) + #expect(documentWidth(of: tableView) == expected) + } + } + + @Test("Scrolled to the end, the last column is mounted") + func lastColumnIsMountedAtTheEnd() throws { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + + scroll( + scrollView, + to: documentWidth(of: tableView) - scrollView.contentView.bounds.width, + tableView: tableView + ) + pool.applyColumnWindow(in: tableView) + + let last = try #require(dataColumns(in: tableView).last) + #expect(!last.isHidden) + } + + // MARK: - Reaching a column the window left out + + /// `rect(ofColumn:)` and `frameOfCell(atColumn:row:)` are both empty for an unmounted column, + /// so Find scrolled to the document origin instead of the match and the inline editor opened + /// nothing at all. + @Test("A column the window left out can be mounted on demand") + func mountColumnReachesAnUnmountedColumn() throws { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + scroll(scrollView, to: 0, tableView: tableView) + pool.applyColumnWindow(in: tableView) + + let last = try #require(dataColumns(in: tableView).last) + #expect(last.isHidden) + + pool.mountColumn(last, in: tableView) + tableView.layoutSubtreeIfNeeded() + + #expect(!last.isHidden) + #expect(tableView.rect(ofColumn: tableView.column(withIdentifier: last.identifier)).width > 0) + } + + /// Stretching the window out to reach a far column mounts every column in between, which is the + /// cost the window exists to avoid: measured at 848ms and 3,081 cell views for one match 90 + /// columns away, and 4.8s at 500 columns. + @Test("Mounting a far column does not mount everything in between") + func mountColumnStaysBounded() throws { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + scroll(scrollView, to: 0, tableView: tableView) + pool.applyColumnWindow(in: tableView) + let mountedBefore = mountedIdentifiers(in: tableView).count + + let last = try #require(dataColumns(in: tableView).last) + pool.mountColumn(last, in: tableView) + tableView.layoutSubtreeIfNeeded() + + #expect(!last.isHidden) + #expect(mountedIdentifiers(in: tableView).count <= mountedBefore) + } + + @Test("Mounting a far column keeps the document width") + func mountColumnKeepsTheDocumentWidth() throws { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + scroll(scrollView, to: 0, tableView: tableView) + pool.applyColumnWindow(in: tableView) + let expected = documentWidth(of: tableView) + + let last = try #require(dataColumns(in: tableView).last) + pool.mountColumn(last, in: tableView) + + #expect(documentWidth(of: tableView) == expected) + } + + @Test("A column the user hid is never mounted on demand") + func mountColumnRefusesAUserHiddenColumn() throws { + let pool = DataGridColumnPool() + let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100, hidden: ["c99"]) + + let hidden = try #require(dataColumns(in: tableView).last) + pool.mountColumn(hidden, in: tableView) + + #expect(hidden.isHidden) + } + + // MARK: - Naming the ends of the data run + + /// The window's spacers are attached columns too, and the leading one sits immediately before + /// the first data column, so a fixed position names a spacer rather than data. + @Test("The first and last presented columns are data columns, not spacers") + func presentedEndsSkipTheSpacers() throws { + let pool = DataGridColumnPool() + let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + + let first = try #require(pool.firstPresentedColumnIndex(in: tableView)) + let last = try #require(pool.lastPresentedColumnIndex(in: tableView)) + + #expect(!ColumnIdentitySchema.isSpacer(tableView.tableColumns[first].identifier)) + #expect(!ColumnIdentitySchema.isSpacer(tableView.tableColumns[last].identifier)) + #expect(tableView.tableColumns[first].identifier == dataColumns(in: tableView).first?.identifier) + #expect(tableView.tableColumns[last].identifier == dataColumns(in: tableView).last?.identifier) + } + + @Test("Walking forward and back from an end stays inside the data run") + func presentedNeighboursStayInsideTheDataRun() throws { + let pool = DataGridColumnPool() + let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 20) + + let first = try #require(pool.firstPresentedColumnIndex(in: tableView)) + let last = try #require(pool.lastPresentedColumnIndex(in: tableView)) + + #expect(pool.previousPresentedColumnIndex(before: first, in: tableView) == nil) + #expect(pool.nextPresentedColumnIndex(after: last, in: tableView) == nil) + #expect(pool.nextPresentedColumnIndex(after: first, in: tableView) != nil) + #expect(pool.previousPresentedColumnIndex(before: last, in: tableView) != nil) + } + + /// The create-table grid opens with no columns at all, where every attached column is chrome. + @Test("A result with no columns presents no column at either end") + func emptyResultHasNoPresentedEnds() { + let pool = DataGridColumnPool() + let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + + reconcileWide(pool, tableView: tableView, count: 0) + + #expect(pool.firstPresentedColumnIndex(in: tableView) == nil) + #expect(pool.lastPresentedColumnIndex(in: tableView) == nil) + } + + @Test("A single-column result presents that column at both ends") + func singleColumnResultHasOneEnd() { + let pool = DataGridColumnPool() + let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + + reconcileWide(pool, tableView: tableView, count: 1) + + #expect(pool.firstPresentedColumnIndex(in: tableView) == pool.lastPresentedColumnIndex(in: tableView)) + #expect(pool.firstPresentedColumnIndex(in: tableView) != nil) + } + + /// Size All Columns to Fit reaches the columns the window unmounted as well, so the spacers + /// stand in at the width those columns used to have and the document ends up short. + @Test("Resizing unmounted columns restores the full document width") + func widthChangeOutsideTheWindowRestoresDocumentWidth() { + let pool = DataGridColumnPool() + let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) + reconcileWide(pool, tableView: tableView, count: 100) + scroll(scrollView, to: 0, tableView: tableView) + pool.applyColumnWindow(in: tableView) + + for column in dataColumns(in: tableView) { + column.width = 300 + } + pool.invalidateColumnWindow() + pool.applyColumnWindow(in: tableView) + + let gap = tableView.intercellSpacing.width + let everyColumnSlot = dataColumns(in: tableView).reduce(0) { $0 + $1.width + gap } + let occupied = tableView.tableColumns + .filter { !$0.isHidden && $0.identifier != ColumnIdentitySchema.rowNumberIdentifier } + .reduce(0) { $0 + $1.width + gap } + + #expect(occupied == everyColumnSlot) + } } diff --git a/TableProTests/Views/Results/DataGridRowViewCopyTests.swift b/TableProTests/Views/Results/DataGridRowViewCopyTests.swift index 1b9b5a17b..f2eac8664 100644 --- a/TableProTests/Views/Results/DataGridRowViewCopyTests.swift +++ b/TableProTests/Views/Results/DataGridRowViewCopyTests.swift @@ -66,17 +66,33 @@ struct DataGridRowViewCopyTests { return coordinator } + /// Built through the column pool rather than by attaching columns by hand, so the positions + /// these tests focus are the ones the grid really has: the pool also attaches the window's two + /// spacers, one of them ahead of the first data column. private func makeTableView(for coordinator: TableViewCoordinator) -> KeyHandlingTableView { let tableView = KeyHandlingTableView() tableView.coordinator = coordinator tableView.addTableColumn(DataGridView.makeRowNumberColumn()) - for identifier in coordinator.identitySchema.identifiers { - tableView.addTableColumn(NSTableColumn(identifier: identifier)) - } coordinator.tableView = tableView + coordinator.columnPool.reconcile( + tableView: tableView, + schema: coordinator.identitySchema, + columnTypes: [], + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: { _, _ in 100 } + ) return tableView } + private func tableColumnIndex(of dataIndex: Int, in tableView: KeyHandlingTableView) -> Int { + guard let coordinator = tableView.coordinator, + let identifier = coordinator.identitySchema.identifier(for: dataIndex) + else { return -1 } + return tableView.column(withIdentifier: identifier) + } + private func invokeCopy( on rowView: DataGridRowView, target: DataGridRowView.CopyContextTarget = .unresolved @@ -243,7 +259,7 @@ struct DataGridRowViewCopyTests { ) let tableView = makeTableView(for: coordinator) tableView.focusedRow = 0 - tableView.focusedColumn = 2 + tableView.focusedColumn = tableColumnIndex(of: 1, in: tableView) let rowView = DataGridRowView() rowView.coordinator = coordinator @@ -266,7 +282,7 @@ struct DataGridRowViewCopyTests { ) let tableView = makeTableView(for: coordinator) tableView.focusedRow = 0 - tableView.focusedColumn = 2 + tableView.focusedColumn = tableColumnIndex(of: 1, in: tableView) let rowView = DataGridRowView() rowView.coordinator = coordinator diff --git a/TableProTests/Views/Results/FocusedColumnResolutionTests.swift b/TableProTests/Views/Results/FocusedColumnResolutionTests.swift index 2cb05165e..7946ce292 100644 --- a/TableProTests/Views/Results/FocusedColumnResolutionTests.swift +++ b/TableProTests/Views/Results/FocusedColumnResolutionTests.swift @@ -4,11 +4,19 @@ // import AppKit +import SwiftUI import TableProPluginKit import Testing @testable import TablePro +@MainActor +private final class FocusedColumnLayoutPersister: ColumnLayoutPersisting { + func load(for key: ColumnLayoutTableKey) -> ColumnLayoutState? { nil } + func save(_ layout: ColumnLayoutState, for key: ColumnLayoutTableKey) {} + func clear(for key: ColumnLayoutTableKey) {} +} + /// `focusedColumn` is a position in `tableView.tableColumns`, which carries the row-number column /// and a hidden spacer ahead of the data and which the reader can reorder. Preview FK Reference /// used to turn it into a data index by subtracting 1, so the menu command previewed the wrong @@ -36,6 +44,43 @@ struct FocusedColumnResolutionTests { return (tableView, schema) } + private func makeCoordinator(columns: [String]) -> TableViewCoordinator { + let coordinator = TableViewCoordinator( + changeManager: AnyChangeManager(DataChangeManager()), + isEditable: true, + selectedRowIndices: .constant([]), + delegate: nil, + layoutPersister: FocusedColumnLayoutPersister() + ) + let rows = columns.map { PluginCellValue.text($0) } + let tableRows = TableRows.from( + queryRows: [rows], + columns: columns, + columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: columns.count) + ) + coordinator.tableRowsProvider = { tableRows } + coordinator.rebuildColumnMetadataCache(from: tableRows) + coordinator.updateCache() + + let tableView = KeyHandlingTableView() + tableView.coordinator = coordinator + tableView.dataSource = coordinator + tableView.delegate = coordinator + tableView.addTableColumn(DataGridView.makeRowNumberColumn()) + coordinator.tableView = tableView + coordinator.columnPool.reconcile( + tableView: tableView, + schema: coordinator.identitySchema, + columnTypes: [], + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: { _, _ in 100 } + ) + tableView.reloadData() + return coordinator + } + private func tableColumnIndex(of name: String, in grid: (tableView: NSTableView, schema: ColumnIdentitySchema)) -> Int? { guard let identifier = grid.schema.identifier(for: grid.schema.dataIndex(forColumnName: name) ?? -1) else { return nil @@ -44,6 +89,32 @@ struct FocusedColumnResolutionTests { return index >= 0 ? index : nil } + /// Moving the selection with the keyboard leaves no cell cursor behind, so the grid seeds one + /// from the selection change. Seeding it with a fixed position lands on the window's leading + /// spacer, and every command that reads the cursor then resolves it to no column at all: Return + /// opens no editor while the menu item still validates as enabled (#2381). + @Test("A selection with no cell cursor seeds one on a data column, not a spacer") + func selectionSeedsTheCursorOnADataColumn() throws { + let coordinator = makeCoordinator(columns: ["id", "name"]) + let tableView = try #require(coordinator.tableView as? KeyHandlingTableView) + tableView.focusedRow = -1 + tableView.focusedColumn = -1 + + tableView.selectRowIndexes(IndexSet(integer: 0), byExtendingSelection: false) + coordinator.tableViewSelectionDidChange( + Notification(name: NSTableView.selectionDidChangeNotification, object: tableView) + ) + + #expect(coordinator.presentsColumn(atTableColumnIndex: tableView.focusedColumn)) + #expect( + DataGridView.dataColumnIndex( + for: tableView.focusedColumn, + in: tableView, + schema: coordinator.identitySchema + ) == 0 + ) + } + @Test("A data column does not sit one place after its data index") func dataColumnsAreNotOffsetByOne() throws { let grid = makeGrid(columns: ["id", "name", "customer_id"]) @@ -70,18 +141,26 @@ struct FocusedColumnResolutionTests { func reorderingKeepsTheMapping() throws { let grid = makeGrid(columns: ["id", "name", "customer_id"]) let before = try #require(tableColumnIndex(of: "customer_id", in: grid)) + let firstData = try #require(tableColumnIndex(of: "id", in: grid)) - grid.tableView.moveColumn(before, toColumn: DataGridView.firstDataTableColumnIndex) + grid.tableView.moveColumn(before, toColumn: firstData) let after = try #require(tableColumnIndex(of: "customer_id", in: grid)) #expect(after != before) #expect(DataGridView.dataColumnIndex(for: after, in: grid.tableView, schema: grid.schema) == 2) } - @Test("The row-number column is not a data column") - func rowNumberColumnIsNotData() { + /// The row-number column and the window's two spacers all sit in `tableColumns`, and one spacer + /// sits ahead of the first data column, so a fixed position names chrome rather than data. + @Test("Neither the row-number column nor the window's spacers are data columns") + func chromeColumnsAreNotData() { let grid = makeGrid(columns: ["id", "name"]) - #expect(DataGridView.isDataTableColumn(0) == false) - #expect(DataGridView.dataColumnIndex(for: 0, in: grid.tableView, schema: grid.schema) == nil) + + let chrome = grid.tableView.tableColumns.indices.filter { + DataGridView.dataColumnIndex(for: $0, in: grid.tableView, schema: grid.schema) == nil + } + + #expect(chrome.contains(0)) + #expect(chrome.count == 3) } } From 9f09dcbd41058ddc3084abab7163b68dcf552cdf Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 22 Aug 2026 20:55:38 +0700 Subject: [PATCH 2/7] fix(datagrid): classify a clicked column by what the result presents --- .../Views/Results/KeyHandlingTableView.swift | 3 +-- .../FocusedColumnResolutionTests.swift | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index 81cae8026..e48dad970 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -127,8 +127,7 @@ final class KeyHandlingTableView: NSTableView { return } - let column = tableColumns[clickedColumn] - let isDataColumn = column.identifier != ColumnIdentitySchema.rowNumberIdentifier + let isDataColumn = presentsDataColumn(at: clickedColumn) let modifiers = event.modifierFlags.intersection(.deviceIndependentFlagsMask) if event.clickCount >= 2 { diff --git a/TableProTests/Views/Results/FocusedColumnResolutionTests.swift b/TableProTests/Views/Results/FocusedColumnResolutionTests.swift index 7946ce292..73edc7b86 100644 --- a/TableProTests/Views/Results/FocusedColumnResolutionTests.swift +++ b/TableProTests/Views/Results/FocusedColumnResolutionTests.swift @@ -115,6 +115,33 @@ struct FocusedColumnResolutionTests { ) } + /// The whole keystroke, not just the seed: with no cell cursor, a selection change seeds one and + /// Return has to open the editor on it. Seeded onto a spacer, every step past the seed resolved + /// to no column and the keystroke was swallowed (#2381). + @Test("Return opens the editor on the column a keyboard selection seeded") + func returnOpensTheEditorOnTheSeededColumn() throws { + let coordinator = makeCoordinator(columns: ["id", "name"]) + let tableView = try #require(coordinator.tableView as? KeyHandlingTableView) + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 600, height: 400), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + window.contentView = tableView + tableView.layoutSubtreeIfNeeded() + tableView.focusedRow = -1 + tableView.focusedColumn = -1 + + tableView.selectRowIndexes(IndexSet(integer: 0), byExtendingSelection: false) + coordinator.tableViewSelectionDidChange( + Notification(name: NSTableView.selectionDidChangeNotification, object: tableView) + ) + tableView.insertNewline(nil) + + #expect(coordinator.overlayEditor != nil) + } + @Test("A data column does not sit one place after its data index") func dataColumnsAreNotOffsetByOne() throws { let grid = makeGrid(columns: ["id", "name", "customer_id"]) From d6b04847aa792a17b4f545336a10b664b9a8dffd Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 22 Aug 2026 20:55:42 +0700 Subject: [PATCH 3/7] docs(claude-md): record the data grid column window invariants --- CLAUDE.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 084b18cb2..4341eec86 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -208,6 +208,10 @@ To ship one: add the record type or field in CloudKit Console (or `xcrun cktool **The data grid header owns all of its own chrome, so nothing may ask AppKit to paint any of it**: `NSTableHeaderCell` and `NSTableHeaderView` both paint a fixed 28pt band that they centre vertically in whatever frame they are given, a 16pt column divider on `midY` and a 1pt rule at `midY + 13`. The data grid grows its header to 42pt for a column comment, so that band lands mid-cell: the rule crosses the comment's descenders and sits 8pt above the real bottom edge. `SortableHeaderChrome` is therefore the single owner of header geometry and colours, `SortableHeaderCell.draw(withFrame:in:)` never calls `super`, and `SortableHeaderView.draw(_:)` fills the background and rules the bottom edge itself. The trap is that the header view paints a second copy of that same band for `NSTableView.highlightedTableColumn`, driven by *state* rather than by a drawing call, so no cell override can reach it: setting it gives the sorted column a stray divider and a rule no other column has. TablePro already draws the sorted-column affordance itself (bold title, chevron, priority number, with `drawSortIndicator` overridden to nothing), so `highlightedTableColumn` is a redundant second channel and must stay unset. All sorted-column presentation goes through `SortableHeaderView.applySortState(_:schema:)`, which publishes the order natively through `tableView.sortDescriptors` (for accessibility; it paints nothing) and updates the cells. `SortableHeaderRenderingTests` rasterises the header and guards this. This shipped as a rule through the comment line and a stray divider on the sorted column (#2017). +**The data grid's column window measures the viewport against the whole column run, so its rebase counts chrome only**: `ColumnWindowResolver` is pure and models every column from the first data column onward, and `DataGridColumnPool.applyColumnWindow` has to move the live viewport into that space before it can pick a range. Only chrome may be subtracted there. The leading spacer sits ahead of the first data column and holds exactly the width of the columns the window left out, which the resolver's model already carries, so counting it subtracts that width twice: the window walks left while the reader scrolls right, alternates between two ranges, and finally parks off screen, where the grid paints nothing until the table is reopened. That shipped as #2381, and a frame-by-frame capture of one scroll sweep across a 100-column table shows it plainly: the worst frame left 100% of the grid unpainted before the fix and 40pt, one column gutter, after it. Two measured AppKit facts hold the design up. A visible column occupies exactly `width + intercellSpacing.width` and a hidden one occupies nothing, spacing included, which is what lets the spacers keep the document width and therefore the scroll extent. And `rect(ofColumn:)` and `frameOfCell(atColumn:row:)` are both empty for a hidden column, so anything that reads a column's frame has to mount it first through `TableViewCoordinator.scrollColumnToVisible(tableColumnIndex:)`, which re-centres a bounded window instead of stretching the mounted range out to the target, because stretching mounts every column in between (848ms and 3,081 cell views for a match 90 columns away, 4.8s at 500). A test here measures through `rect(ofColumn:)` and never through the resolver, or it only proves the resolver agrees with itself. + +**No fixed position in `tableColumns` names a data column**: the attached order is `[__rowNumber__, __leadingSpacer__, data columns, surplus pool slots, __trailingSpacer__]`, so `presentsColumn` is the question to ask, with `firstPresentedColumnIndex` and its neighbours beside it on `DataGridColumnPool`. `DataGridView.firstDataTableColumnIndex` was a hardcoded `1` that the leading spacer took over when windowing landed, and `isDataTableColumn` accepted the trailing spacer at the other end. The cell cursor was seeded onto a spacer whenever the selection moved without a click, so Down then Return did nothing on any table while the Edit menu item still validated as enabled, Tab out of a row's last cell and Shift+Tab out of its first were swallowed, and `scrollColumnToVisible` on a column the window had unmounted scrolled to the document origin instead of the column (#2381). + **Decoding a MongoDB binary UUID is a per-column decision, and the column's type name is load-bearing**: BSON binary subtype 3 is the legacy UUID format, and the Java, C# and Python drivers each wrote it with a different byte order with nothing in the stored bytes to say which. `MongoDBUuidCodec` therefore decodes subtype 3 only when the connection names one (`mongoUuidRepresentation`); subtype 4 is unambiguous and always decodes. The choice is made once per column from `BsonDocumentFlattener.columnKinds`' majority vote, never per value, because a decoded cell is `.text` and an undecoded one is `.bytes`, and `CellDisplayFormatter` runs blob formatting over a `.text` cell whenever its column type is BLOB. One UUID decoded inside a column the app still types `BLOB` renders as `0x4c65676163...`. For the same reason `BsonDocumentFlattener.typeName` must keep `BLOB` as the base name for undecoded binary: `ColumnTypeClassifier` splits a type name at the first `(` and looks the base up, so `BLOB` and `BLOB(3)` both classify as `.blob`, and that classification is the only thing keeping a binary cell out of the inline editor. The parenthesised part carries the BSON subtype so MQL export can write it back; `MongoDBUuidCodec.columnTypeName(forSubtype:)` and `binarySubtype(fromColumnTypeName:)` are the only two places that spelling is produced or read, and MQL export is `supportedDatabaseTypeIds = ["MongoDB"]`, so it never sees another driver's `BLOB`. Once a column does decode, both edit guards (`isBlobType` and `asBytes != nil`) fall together, so every write path must parse the wrapper back to `$binary`: `MongoDBStatementGenerator.jsonValue` and `idValueJson`, `MongoDBQueryBuilder.jsonValue` plus its `=`, `!=` and `IN` arms (a case-insensitive regex can never match a binary field), and `MQLExportHelpers.mqlJsonValue`. An `_id` filter left as wrapper text matches zero documents while the UI reports the save succeeded. (#2086) **A pooled metadata read assumes a second connection reaches the same database, and an embedded engine breaks that assumption**: `MetadataConnectionPool` builds a whole new driver, so it is only correct when the database lives on a server the driver reconnects to. When the database lives *inside* the driver instance, the pool gets a different database: a second `duckdb_open(":memory:")` is a fresh empty database, and a second `duckdb_open` on the same *file* is a second independent read-write instance that the first never sees (DuckDB's file lock does not conflict within one process). The failure is silent, because an empty catalog is indistinguishable from "no tables", which is why #2108 survived a manual refresh. `supportsConnectionPooling` is the opt-out, and it is read only by `DatabaseManager.canPool`; DuckDB and PGlite set it `false`. SQLite-family engines keep pooling, because multi-connection access to one file is what they are built for. Two rules follow. First, every metadata read goes through `DatabaseManager.withMetadataDriver` so `metadataRoute` can apply the rule; reaching for `MetadataConnectionPool.shared.withDriver` directly bypasses it, which is how routines kept pooling after the sidebar stopped. Second, a capability with no `DriverPlugin` static is curated per type and `buildMetadataSnapshot` must carry it over from the built-in snapshot, or `register(snapshot:forTypeId:)` resets it to the struct default the moment the plugin loads. That is not hypothetical: it silently disabled MongoDB's `authenticationIsDatabaseScoped` (#1970) for every build that had the plugin installed. `registerVariant` already treats the curated entry as authoritative, which is the only reason PGlite's flag ever worked. From 62bbc44395525e0ec11b71466357cbb63f4ef908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Sun, 23 Aug 2026 01:31:08 +0700 Subject: [PATCH 4/7] perf(datagrid): draw data cells instead of building a view for each one (#2381) (#2385) * refactor(datagrid): extract cell geometry, appearance and drawing from the cell view * perf(datagrid): draw data cells instead of building a view for each one (#2381) --- CHANGELOG.md | 5 + CLAUDE.md | 2 +- TablePro/Models/UI/ColumnIdentitySchema.swift | 12 - TablePro/Views/Results/CellOverlayBase.swift | 10 +- .../Cells/DataGridCellAccessoryGlyph.swift | 124 +++++ .../Cells/DataGridCellAppearance.swift | 106 ++++ .../Results/Cells/DataGridCellRegistry.swift | 15 - .../Results/Cells/DataGridCellRenderer.swift | 139 +++++ .../Results/Cells/DataGridCellView.swift | 487 ------------------ .../Views/Results/ColumnWindowResolver.swift | 130 ----- .../Views/Results/DataGridColumnPool.swift | 235 ++------- .../Views/Results/DataGridCoordinator.swift | 91 ++-- TablePro/Views/Results/DataGridRowView.swift | 218 +++++++- .../Extensions/DataGridView+Click.swift | 4 +- .../DataGridView+ColumnWidths.swift | 3 +- .../Extensions/DataGridView+Columns.swift | 128 +++-- .../Extensions/DataGridView+Sort.swift | 6 +- .../Views/Results/KeyHandlingTableView.swift | 33 +- .../Results/ColumnWindowResolverTests.swift | 191 ------- ...DataGridCellAccessoryAppearanceTests.swift | 26 +- .../Results/DataGridCellAppearanceTests.swift | 182 +++++++ .../DataGridCellViewDoubleClickTests.swift | 230 --------- .../Results/DataGridColumnPoolTests.swift | 401 +------------- .../FocusedColumnResolutionTests.swift | 41 +- 24 files changed, 1001 insertions(+), 1818 deletions(-) create mode 100644 TablePro/Views/Results/Cells/DataGridCellAccessoryGlyph.swift create mode 100644 TablePro/Views/Results/Cells/DataGridCellAppearance.swift create mode 100644 TablePro/Views/Results/Cells/DataGridCellRenderer.swift delete mode 100644 TablePro/Views/Results/Cells/DataGridCellView.swift delete mode 100644 TablePro/Views/Results/ColumnWindowResolver.swift delete mode 100644 TableProTests/Views/Results/ColumnWindowResolverTests.swift create mode 100644 TableProTests/Views/Results/DataGridCellAppearanceTests.swift delete mode 100644 TableProTests/Views/Results/DataGridCellViewDoubleClickTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index d690c45eb..5ac40ef51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- The data grid draws its cells instead of building a view for each one, so a result with hundreds of columns opens at once and holds a fraction of the memory. (#2381) + ### Fixed - Flickering columns, blank columns, and an unpainted gap while scrolling a result with about 100 columns sideways. (#2381) @@ -14,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Return opening no editor on a row selected with the arrow keys. - Tab out of a row's last cell and Shift+Tab out of its first doing nothing. - Size All Columns to Fit leaving the far columns of a wide result unreachable. +- A table with 500 columns pinning a core for 20 seconds and taking a gigabyte to open. (#2381) ## [0.67.1] - 2026-08-22 diff --git a/CLAUDE.md b/CLAUDE.md index 4341eec86..d0739f59c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -208,7 +208,7 @@ To ship one: add the record type or field in CloudKit Console (or `xcrun cktool **The data grid header owns all of its own chrome, so nothing may ask AppKit to paint any of it**: `NSTableHeaderCell` and `NSTableHeaderView` both paint a fixed 28pt band that they centre vertically in whatever frame they are given, a 16pt column divider on `midY` and a 1pt rule at `midY + 13`. The data grid grows its header to 42pt for a column comment, so that band lands mid-cell: the rule crosses the comment's descenders and sits 8pt above the real bottom edge. `SortableHeaderChrome` is therefore the single owner of header geometry and colours, `SortableHeaderCell.draw(withFrame:in:)` never calls `super`, and `SortableHeaderView.draw(_:)` fills the background and rules the bottom edge itself. The trap is that the header view paints a second copy of that same band for `NSTableView.highlightedTableColumn`, driven by *state* rather than by a drawing call, so no cell override can reach it: setting it gives the sorted column a stray divider and a rule no other column has. TablePro already draws the sorted-column affordance itself (bold title, chevron, priority number, with `drawSortIndicator` overridden to nothing), so `highlightedTableColumn` is a redundant second channel and must stay unset. All sorted-column presentation goes through `SortableHeaderView.applySortState(_:schema:)`, which publishes the order natively through `tableView.sortDescriptors` (for accessibility; it paints nothing) and updates the cells. `SortableHeaderRenderingTests` rasterises the header and guards this. This shipped as a rule through the comment line and a stray divider on the sorted column (#2017). -**The data grid's column window measures the viewport against the whole column run, so its rebase counts chrome only**: `ColumnWindowResolver` is pure and models every column from the first data column onward, and `DataGridColumnPool.applyColumnWindow` has to move the live viewport into that space before it can pick a range. Only chrome may be subtracted there. The leading spacer sits ahead of the first data column and holds exactly the width of the columns the window left out, which the resolver's model already carries, so counting it subtracts that width twice: the window walks left while the reader scrolls right, alternates between two ranges, and finally parks off screen, where the grid paints nothing until the table is reopened. That shipped as #2381, and a frame-by-frame capture of one scroll sweep across a 100-column table shows it plainly: the worst frame left 100% of the grid unpainted before the fix and 40pt, one column gutter, after it. Two measured AppKit facts hold the design up. A visible column occupies exactly `width + intercellSpacing.width` and a hidden one occupies nothing, spacing included, which is what lets the spacers keep the document width and therefore the scroll extent. And `rect(ofColumn:)` and `frameOfCell(atColumn:row:)` are both empty for a hidden column, so anything that reads a column's frame has to mount it first through `TableViewCoordinator.scrollColumnToVisible(tableColumnIndex:)`, which re-centres a bounded window instead of stretching the mounted range out to the target, because stretching mounts every column in between (848ms and 3,081 cell views for a match 90 columns away, 4.8s at 500). A test here measures through `rect(ofColumn:)` and never through the resolver, or it only proves the resolver agrees with itself. +**The data grid's column window measures the viewport against the whole column run, so its rebase counts chrome only**: `NSTableColumn.isHidden` costs O(attached columns) per write, because AppKit walks every row view and re-sorts its subviews to rebuild the key view loop. Hiding the columns outside a viewport is therefore quadratic in the column count: measured at 233ms for 100 columns, 8.1s for 500 and 34s for 1000, with the relayout debris of those writes never reclaimed, which is where a 500-column table's 837MB went. No AppKit knob helps: `autorecalculatesKeyViewLoop = false`, `beginUpdates`/`endUpdates` and hiding before any row exists were all measured and none of them changed it. The grid therefore keeps every column attached and visible, and pays nothing for the ones off screen by not building a view for them: `tableView(_:viewFor:row:)` returns nil for every data column and `DataGridRowView` draws the cells the viewport touches with CoreText. Measured on a 500-column table: opening it went from 12.4s to 26ms and from 837MB to 3.9MB, and the whole table holds 26 views rather than 12,500. Never reintroduce a column window built on `isHidden`. **No fixed position in `tableColumns` names a data column**: the attached order is `[__rowNumber__, __leadingSpacer__, data columns, surplus pool slots, __trailingSpacer__]`, so `presentsColumn` is the question to ask, with `firstPresentedColumnIndex` and its neighbours beside it on `DataGridColumnPool`. `DataGridView.firstDataTableColumnIndex` was a hardcoded `1` that the leading spacer took over when windowing landed, and `isDataTableColumn` accepted the trailing spacer at the other end. The cell cursor was seeded onto a spacer whenever the selection moved without a click, so Down then Return did nothing on any table while the Edit menu item still validated as enabled, Tab out of a row's last cell and Shift+Tab out of its first were swallowed, and `scrollColumnToVisible` on a column the window had unmounted scrolled to the document origin instead of the column (#2381). diff --git a/TablePro/Models/UI/ColumnIdentitySchema.swift b/TablePro/Models/UI/ColumnIdentitySchema.swift index ef74e074a..fe4e4f438 100644 --- a/TablePro/Models/UI/ColumnIdentitySchema.swift +++ b/TablePro/Models/UI/ColumnIdentitySchema.swift @@ -8,20 +8,8 @@ import AppKit struct ColumnIdentitySchema: Equatable { static let rowNumberIdentifier = NSUserInterfaceItemIdentifier("__rowNumber__") - /// Stand-ins for the columns the window leaves unmounted, holding their width so the document - /// keeps its full extent and the horizontal scroller still spans the whole result. - /// - /// Deliberately outside `dataColumnPrefix`: every loop that walks `tableView.tableColumns` - /// resolves a data index through `dataIndex(from:)` first, so a spacer is skipped by all of - /// them without any of those call sites learning about it. - static let leadingSpacerIdentifier = NSUserInterfaceItemIdentifier("__leadingSpacer__") - static let trailingSpacerIdentifier = NSUserInterfaceItemIdentifier("__trailingSpacer__") static let dataColumnPrefix = "dataColumn-" - static func isSpacer(_ identifier: NSUserInterfaceItemIdentifier) -> Bool { - identifier == leadingSpacerIdentifier || identifier == trailingSpacerIdentifier - } - let identifiers: [NSUserInterfaceItemIdentifier] let columnNames: [String] diff --git a/TablePro/Views/Results/CellOverlayBase.swift b/TablePro/Views/Results/CellOverlayBase.swift index 970b2e0f3..5b92dead6 100644 --- a/TablePro/Views/Results/CellOverlayBase.swift +++ b/TablePro/Views/Results/CellOverlayBase.swift @@ -52,13 +52,15 @@ class CellOverlayBase: NSObject { self.columnIndex = columnIndex tableView.addSubview(container) self.container = container - underlyingCell(in: tableView, row: row, column: column)?.applyOverlayActive(true) + setOverlayCell(CellPosition(row: row, column: columnIndex), in: tableView) selectionOverlay(in: tableView)?.needsDisplay = true installDismissObservers() } - private func underlyingCell(in tableView: NSTableView, row: Int, column: Int) -> DataGridCellView? { - tableView.view(atColumn: column, row: row, makeIfNecessary: false) as? DataGridCellView + /// The cell under the overlay draws no text of its own behind it. A drawn cell has no view to + /// carry that, so the coordinator holds it and repaints the cell either side of the change. + private func setOverlayCell(_ position: CellPosition?, in tableView: NSTableView) { + (tableView as? KeyHandlingTableView)?.coordinator?.overlayCell = position } private func selectionOverlay(in tableView: NSTableView) -> GridSelectionOverlay? { @@ -73,7 +75,7 @@ class CellOverlayBase: NSObject { guard let activeContainer = container else { return } removeDismissObservers() if let hostTableView { - underlyingCell(in: hostTableView, row: row, column: column)?.applyOverlayActive(false) + setOverlayCell(nil, in: hostTableView) selectionOverlay(in: hostTableView)?.needsDisplay = true } activeContainer.removeFromSuperview() diff --git a/TablePro/Views/Results/Cells/DataGridCellAccessoryGlyph.swift b/TablePro/Views/Results/Cells/DataGridCellAccessoryGlyph.swift new file mode 100644 index 000000000..82d6b0f0c --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridCellAccessoryGlyph.swift @@ -0,0 +1,124 @@ +// +// DataGridCellAccessoryGlyph.swift +// TablePro +// + +import AppKit + +/// The symbol a cell draws at its trailing edge, rasterised once per appearance. +/// +/// Shared by every cell in the grid, so the cache is static: the glyph depends on the role and the +/// appearance it is drawn into, never on which cell asked for it. +@MainActor +enum DataGridCellAccessoryGlyph { + enum Role: Hashable { + case foreignKeyNormal + case foreignKeyEmphasized + case chevronNormal + case chevronEmphasized + case chevronDisabled + + init?(accessory: DataGridCellAccessory, isEmphasized: Bool, isDisabled: Bool) { + switch accessory { + case .none: + return nil + case .foreignKey: + self = isEmphasized ? .foreignKeyEmphasized : .foreignKeyNormal + case .chevron: + if isDisabled { + self = .chevronDisabled + } else { + self = isEmphasized ? .chevronEmphasized : .chevronNormal + } + } + } + + var symbolName: String { + switch self { + case .foreignKeyNormal, .foreignKeyEmphasized: + return "arrow.forward" + case .chevronNormal, .chevronEmphasized, .chevronDisabled: + return "chevron.up.chevron.down" + } + } + + /// The bare arrow spends its whole point size on the arrow itself, where the circled variant + /// spent most of it on the ring, so 14 here would draw an arrow half again as large as the + /// one it replaced. 12 keeps the ink at 11 x 9 in the 16 x 16 accessory rect, close to the + /// dropdown chevron's weight and to the 13pt cell text. + var pointSize: CGFloat { + switch self { + case .foreignKeyNormal, .foreignKeyEmphasized: + return 12 + case .chevronNormal, .chevronEmphasized, .chevronDisabled: + return 10 + } + } + + var color: NSColor { + switch self { + case .foreignKeyNormal, .chevronNormal: + return .secondaryLabelColor + case .foreignKeyEmphasized, .chevronEmphasized: + return .alternateSelectedControlTextColor + case .chevronDisabled: + return .tertiaryLabelColor + } + } + } + + struct Glyph { + let image: CGImage + let pointSize: NSSize + } + + private struct Key: Hashable { + let role: Role + let appearance: NSAppearance.Name + let increasedContrast: Bool + } + + private static var glyphs: [Key: Glyph] = [:] + + /// Rasterizing resolves the dynamic symbol color, so a cached bitmap belongs to exactly one + /// appearance. Keying on the appearance is what keeps a dark window from being served the + /// light bitmap, and `NSAppearance.currentDrawing()` only reports the drawing appearance while + /// AppKit is inside `draw(_:)`, `updateLayer` or `layout`. + static func image(for role: Role) -> Glyph? { + let key = Key( + role: role, + appearance: NSAppearance.currentDrawing().name, + increasedContrast: NSWorkspace.shared.accessibilityDisplayShouldIncreaseContrast + ) + if let cached = glyphs[key] { return cached } + guard let glyph = make(role) else { return nil } + glyphs[key] = glyph + return glyph + } + + private static func make(_ role: Role) -> Glyph? { + let config = NSImage.SymbolConfiguration(pointSize: role.pointSize, weight: .regular) + .applying(.init(hierarchicalColor: role.color)) + guard let image = NSImage(systemSymbolName: role.symbolName, accessibilityDescription: nil)? + .withSymbolConfiguration(config) else { return nil } + var rect = CGRect(origin: .zero, size: image.size) + guard let cgImage = image.cgImage(forProposedRect: &rect, context: nil, hints: nil) else { return nil } + return Glyph(image: cgImage, pointSize: image.size) + } + + /// A symbol stretched to fill the accessory rect stops looking like a system symbol, so the + /// glyph draws at its own point size and the rect only ever clamps it. The clamp is one factor + /// across both axes, because clamping each axis on its own would distort the glyph exactly the + /// way filling the rect did. The origin rounds to whole points so a glyph narrower than its rect + /// by an odd number of points does not land on a half point and blur at 1x. + static func centeredRect(pointSize: NSSize, in rect: NSRect) -> NSRect { + let scale = min(1, rect.width / pointSize.width, rect.height / pointSize.height) + let size = NSSize(width: pointSize.width * scale, height: pointSize.height * scale) + return NSRect( + x: (rect.midX - size.width / 2).rounded(), + y: (rect.midY - size.height / 2).rounded(), + width: size.width, + height: size.height + ) + } +} diff --git a/TablePro/Views/Results/Cells/DataGridCellAppearance.swift b/TablePro/Views/Results/Cells/DataGridCellAppearance.swift new file mode 100644 index 000000000..9a0d23f2d --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridCellAppearance.swift @@ -0,0 +1,106 @@ +// +// DataGridCellAppearance.swift +// TablePro +// + +import AppKit + +/// What one data cell looks like, resolved from its content and state. +/// +/// Separated from the drawing so the decisions are testable without a view and without a graphics +/// context: which font a NULL takes, when a modified cell keeps its tint, what colour text turns on +/// a selected row. The renderer that consumes this makes no decisions of its own. +@MainActor +struct DataGridCellAppearance: Equatable { + let text: String + let font: NSFont + let textColor: NSColor + /// Painted behind the text, for a find match or a modified value. + let backgroundTint: NSColor? + let accessory: DataGridCellAccessory + /// Which symbol the accessory draws, resolved here because it follows the row's state rather + /// than anything the renderer can see. + let accessoryRole: DataGridCellAccessoryGlyph.Role? + /// The cell cursor on a selected row, which the selection fill would swallow a focus ring behind. + let drawsFocusBorder: Bool + /// The cell cursor on an unselected row. + let drawsFocusRing: Bool + + static func resolve( + kind: DataGridCellKind, + content: DataGridCellContent, + state: DataGridCellState, + palette: DataGridCellPalette, + nullDisplayString: String, + onEmphasizedSelection: Bool, + hasOverlay: Bool + ) -> DataGridCellAppearance { + let deletedTextColor = state.visualState.isDeleted ? palette.deletedRowText : nil + let font: NSFont + let baseColor: NSColor + + switch content.placeholder { + case .none: + font = palette.regularFont + baseColor = deletedTextColor ?? .labelColor + case .null, .empty: + font = palette.italicFont + baseColor = deletedTextColor ?? .secondaryLabelColor + case .defaultMarker: + font = palette.mediumFont + baseColor = deletedTextColor ?? .systemBlue + } + + let findTint: NSColor? = state.isCurrentFindMatch ? palette.findMatchTint : nil + let modifiedTint: NSColor? + if state.visualState.isDeleted || state.visualState.isInserted { + modifiedTint = nil + } else if state.visualState.isModified(columnIndex: state.columnIndex) { + modifiedTint = palette.modifiedColumnTint + } else { + modifiedTint = nil + } + + // A find match keeps its own highlight whatever else is true, and the text turns black + // against it. Otherwise a selected row's text takes the selection's own colour, and the + // modified tint stands down so the selection fill is not painted over. + let backgroundTint: NSColor? + let textColor: NSColor + if let findTint { + backgroundTint = findTint + textColor = .black + } else if onEmphasizedSelection { + backgroundTint = nil + textColor = .alternateSelectedControlTextColor + } else { + backgroundTint = modifiedTint + textColor = baseColor + } + + let accessory = DataGridCellAccessory.visible( + for: kind, + isEditable: state.isEditable, + rawValue: content.rawValue + ) + let isCursorVisible = state.isFocused && !hasOverlay + return DataGridCellAppearance( + text: DataGridCellContent.resolvedDisplayText( + content.displayText, + placeholder: content.placeholder, + isLargeDataset: state.isLargeDataset, + nullDisplayString: nullDisplayString + ), + font: font, + textColor: textColor, + backgroundTint: backgroundTint, + accessory: accessory, + accessoryRole: DataGridCellAccessoryGlyph.Role( + accessory: accessory, + isEmphasized: onEmphasizedSelection, + isDisabled: state.visualState.isDeleted + ), + drawsFocusBorder: isCursorVisible && onEmphasizedSelection, + drawsFocusRing: isCursorVisible && !onEmphasizedSelection + ) + } +} diff --git a/TablePro/Views/Results/Cells/DataGridCellRegistry.swift b/TablePro/Views/Results/Cells/DataGridCellRegistry.swift index 32888d745..f7bb521a1 100644 --- a/TablePro/Views/Results/Cells/DataGridCellRegistry.swift +++ b/TablePro/Views/Results/Cells/DataGridCellRegistry.swift @@ -33,21 +33,6 @@ final class DataGridCellRegistry { } } - func dequeueCell(in tableView: NSTableView) -> DataGridCellView { - if let reused = tableView.makeView( - withIdentifier: DataGridCellView.reuseIdentifier, - owner: nil - ) as? DataGridCellView { - reused.nullDisplayString = nullDisplayString - return reused - } - - let cell = DataGridCellView(frame: .zero) - cell.identifier = DataGridCellView.reuseIdentifier - cell.accessoryDelegate = accessoryDelegate - cell.nullDisplayString = nullDisplayString - return cell - } func makeRowNumberCell( in tableView: NSTableView, diff --git a/TablePro/Views/Results/Cells/DataGridCellRenderer.swift b/TablePro/Views/Results/Cells/DataGridCellRenderer.swift new file mode 100644 index 000000000..d17d8f47f --- /dev/null +++ b/TablePro/Views/Results/Cells/DataGridCellRenderer.swift @@ -0,0 +1,139 @@ +// +// DataGridCellRenderer.swift +// TablePro +// + +import AppKit +import CoreText + +/// Draws one data cell into a rect. +/// +/// The grid draws its cells rather than mounting a view for each, so the drawing has to live apart +/// from any view. Everything here is a function of the appearance it is handed; the only state is a +/// bounded cache of laid-out lines, because building a `CTLine` is the expensive part of a cell and +/// scrolling redraws the same values over and over. +@MainActor +final class DataGridCellRenderer { + /// A laid-out line depends on nothing but the text and the two attributes it carries, so this is + /// the whole identity of a cached line. + private struct LineKey: Hashable { + let text: String + let font: NSFont + let color: NSColor + } + + /// A viewport holds a few hundred cells and a scroll reuses them, so this only has to outlive a + /// few screens. Cleared wholesale rather than evicted one at a time: the cost of a miss is one + /// `CTLine`, and tracking recency would cost more than it saves. + private static let lineCacheLimit = 4_096 + /// Past this many characters a cell can only ever show an ellipsis, and laying out the rest is + /// wasted on a value the reader opens the inspector to read anyway. + private static let maximumLaidOutCharacters = 300 + + private var lineCache: [LineKey: CTLine] = [:] + + func invalidateCachedLines() { + lineCache.removeAll(keepingCapacity: true) + } + + func draw(_ appearance: DataGridCellAppearance, in rect: NSRect) { + guard rect.width > 0, rect.height > 0 else { return } + + if let tint = appearance.backgroundTint { + tint.setFill() + rect.fill() + } + + let accessoryRect = appearance.accessory.frame(in: rect) + + NSGraphicsContext.current?.saveGraphicsState() + NSBezierPath(rect: rect).addClip() + drawText(appearance, in: rect) + drawAccessory(appearance.accessoryRole, in: accessoryRect) + NSGraphicsContext.current?.restoreGraphicsState() + + if appearance.drawsFocusBorder { + drawFocusBorder(in: rect) + } else if appearance.drawsFocusRing { + drawFocusRing(in: rect) + } + } + + private func drawText(_ appearance: DataGridCellAppearance, in rect: NSRect) { + guard !appearance.text.isEmpty else { return } + let availableWidth = appearance.accessory.availableTextWidth(in: rect) + guard availableWidth > 0, let context = NSGraphicsContext.current?.cgContext else { return } + + let fullLine = line(for: appearance.text, font: appearance.font, color: appearance.textColor) + let typographicWidth = CTLineGetTypographicBounds(fullLine, nil, nil, nil) + let ellipsis = line(for: "\u{2026}", font: appearance.font, color: appearance.textColor) + let ellipsisWidth = CTLineGetTypographicBounds(ellipsis, nil, nil, nil) + guard Double(availableWidth) >= ellipsisWidth else { return } + + let lineToDraw = typographicWidth > Double(availableWidth) + ? (CTLineCreateTruncatedLine(fullLine, Double(availableWidth), .end, ellipsis) ?? ellipsis) + : fullLine + + let font = appearance.font + let baselineOffset = (rect.height - font.ascender + font.descender - font.leading) / 2 + font.ascender + + context.saveGState() + context.textMatrix = CGAffineTransform(scaleX: 1, y: -1) + context.textPosition = CGPoint( + x: rect.minX + DataGridMetrics.cellHorizontalInset, + y: rect.minY + baselineOffset + ) + CTLineDraw(lineToDraw, context) + context.restoreGState() + } + + private func line(for text: String, font: NSFont, color: NSColor) -> CTLine { + let key = LineKey(text: text, font: font, color: color) + if let cached = lineCache[key] { return cached } + + let source = text as NSString + let laidOut = source.length > Self.maximumLaidOutCharacters + ? source.substring(to: Self.maximumLaidOutCharacters) + "\u{2026}" + : text + let attributed = NSAttributedString( + string: laidOut, + attributes: [.font: font, .foregroundColor: color] + ) + let created = CTLineCreateWithAttributedString(attributed as CFAttributedString) + + if lineCache.count >= Self.lineCacheLimit { + lineCache.removeAll(keepingCapacity: true) + } + lineCache[key] = created + return created + } + + private func drawAccessory(_ role: DataGridCellAccessoryGlyph.Role?, in rect: NSRect) { + guard !rect.isEmpty, let role else { return } + guard let glyph = DataGridCellAccessoryGlyph.image(for: role), + let context = NSGraphicsContext.current?.cgContext else { return } + + let drawRect = DataGridCellAccessoryGlyph.centeredRect(pointSize: glyph.pointSize, in: rect) + context.saveGState() + context.translateBy(x: drawRect.minX, y: drawRect.maxY) + context.scaleBy(x: 1, y: -1) + context.draw(glyph.image, in: CGRect(origin: .zero, size: drawRect.size)) + context.restoreGState() + } + + private func drawFocusBorder(in rect: NSRect) { + let path = NSBezierPath(rect: rect.insetBy(dx: 1, dy: 1)) + path.lineWidth = 2 + NSColor.alternateSelectedControlTextColor.setStroke() + path.stroke() + } + + /// The cell cursor on an unselected row. A mounted cell got this from AppKit's exterior focus + /// ring; a drawn cell has no view to hang one on, so it is drawn to the same shape. + private func drawFocusRing(in rect: NSRect) { + let path = NSBezierPath(rect: rect.insetBy(dx: 1, dy: 1)) + path.lineWidth = 2 + NSColor.keyboardFocusIndicatorColor.setStroke() + path.stroke() + } +} diff --git a/TablePro/Views/Results/Cells/DataGridCellView.swift b/TablePro/Views/Results/Cells/DataGridCellView.swift deleted file mode 100644 index 2074042d4..000000000 --- a/TablePro/Views/Results/Cells/DataGridCellView.swift +++ /dev/null @@ -1,487 +0,0 @@ -// -// DataGridCellView.swift -// TablePro -// - -import AppKit -import CoreText - -@MainActor -final class DataGridCellView: NSView { - static let reuseIdentifier = NSUserInterfaceItemIdentifier("dataCell") - - weak var accessoryDelegate: DataGridCellAccessoryDelegate? - var nullDisplayString: String = "" - - private(set) var kind: DataGridCellKind = .text - private(set) var cellRow: Int = -1 - private(set) var cellColumnIndex: Int = -1 - - private var displayText: String = "" - private var rawValue: String? - private var placeholder: DataGridCellPlaceholder? - private var isLargeDataset: Bool = false - private var isEditableCell: Bool = false - - private var textFont = NSFont.systemFont(ofSize: NSFont.systemFontSize) - private var textColor: NSColor = .labelColor - private var modifiedColumnTint: NSColor? - - private var visualState: RowVisualState = .empty - private var isFocusedCell: Bool = false - private var onEmphasizedSelection: Bool = false - private var hasOverlay: Bool = false - private var findMatchTint: NSColor? - - private var cachedLine: CTLine? - - private enum AccessoryRole: Hashable { - case foreignKeyNormal - case foreignKeyEmphasized - case chevronNormal - case chevronEmphasized - case chevronDisabled - - var symbolName: String { - switch self { - case .foreignKeyNormal, .foreignKeyEmphasized: - return "arrow.forward" - case .chevronNormal, .chevronEmphasized, .chevronDisabled: - return "chevron.up.chevron.down" - } - } - - /// The bare arrow spends its whole point size on the arrow itself, where the circled variant - /// spent most of it on the ring, so 14 here would draw an arrow half again as large as the - /// one it replaced. 12 keeps the ink at 11 x 9 in the 16 x 16 accessory rect, close to the - /// dropdown chevron's weight and to the 13pt cell text. - var pointSize: CGFloat { - switch self { - case .foreignKeyNormal, .foreignKeyEmphasized: - return 12 - case .chevronNormal, .chevronEmphasized, .chevronDisabled: - return 10 - } - } - - var color: NSColor { - switch self { - case .foreignKeyNormal, .chevronNormal: - return .secondaryLabelColor - case .foreignKeyEmphasized, .chevronEmphasized: - return .alternateSelectedControlTextColor - case .chevronDisabled: - return .tertiaryLabelColor - } - } - } - - private struct AccessoryGlyphKey: Hashable { - let role: AccessoryRole - let appearance: NSAppearance.Name - let increasedContrast: Bool - } - - private struct AccessoryGlyph { - let image: CGImage - let pointSize: NSSize - } - - private static var accessoryGlyphs: [AccessoryGlyphKey: AccessoryGlyph] = [:] - - /// Rasterizing resolves the dynamic symbol color, so a cached bitmap belongs to exactly one - /// appearance. Keying on the appearance is what keeps a dark window from being served the - /// light bitmap, and `NSAppearance.currentDrawing()` only reports the cell's own appearance - /// while AppKit is inside `draw(_:)`, `updateLayer` or `layout`. - private static func accessoryGlyph(for role: AccessoryRole) -> AccessoryGlyph? { - let key = AccessoryGlyphKey( - role: role, - appearance: NSAppearance.currentDrawing().name, - increasedContrast: NSWorkspace.shared.accessibilityDisplayShouldIncreaseContrast - ) - if let cached = accessoryGlyphs[key] { - return cached - } - guard let glyph = makeAccessoryGlyph(role) else { return nil } - accessoryGlyphs[key] = glyph - return glyph - } - - private static func makeAccessoryGlyph(_ role: AccessoryRole) -> AccessoryGlyph? { - let config = NSImage.SymbolConfiguration(pointSize: role.pointSize, weight: .regular) - .applying(.init(hierarchicalColor: role.color)) - guard let image = NSImage(systemSymbolName: role.symbolName, accessibilityDescription: nil)? - .withSymbolConfiguration(config) else { return nil } - var rect = CGRect(origin: .zero, size: image.size) - guard let cgImage = image.cgImage(forProposedRect: &rect, context: nil, hints: nil) else { return nil } - return AccessoryGlyph(image: cgImage, pointSize: image.size) - } - - override init(frame frameRect: NSRect) { - super.init(frame: frameRect) - commonInit() - } - - required init?(coder: NSCoder) { - super.init(coder: coder) - commonInit() - } - - private func commonInit() { - setAccessibilityElement(true) - setAccessibilityRole(.cell) - } - - override var allowsVibrancy: Bool { false } - override var isFlipped: Bool { true } - - func configure( - kind: DataGridCellKind, - content: DataGridCellContent, - state: DataGridCellState, - palette: DataGridCellPalette - ) { - var needsRedraw = false - - if self.kind != kind { - self.kind = kind - needsRedraw = true - } - cellRow = state.row - cellColumnIndex = state.columnIndex - - if hasOverlay { - hasOverlay = false - updateFocusPresentation() - needsRedraw = true - } - - let nextFont: NSFont - let nextColor: NSColor - let deletedTextColor = state.visualState.isDeleted ? palette.deletedRowText : nil - - switch content.placeholder { - case .none: - nextFont = palette.regularFont - nextColor = deletedTextColor ?? .labelColor - case .null: - nextFont = palette.italicFont - nextColor = deletedTextColor ?? .secondaryLabelColor - case .empty: - nextFont = palette.italicFont - nextColor = deletedTextColor ?? .secondaryLabelColor - case .defaultMarker: - nextFont = palette.mediumFont - nextColor = deletedTextColor ?? .systemBlue - } - let nextDisplayText = DataGridCellContent.resolvedDisplayText( - content.displayText, - placeholder: content.placeholder, - isLargeDataset: state.isLargeDataset, - nullDisplayString: nullDisplayString - ) - - if displayText != nextDisplayText - || textFont != nextFont - || textColor != nextColor { - displayText = nextDisplayText - textFont = nextFont - textColor = nextColor - cachedLine = nil - needsRedraw = true - } - - if rawValue != content.rawValue { - rawValue = content.rawValue - needsRedraw = true - } - placeholder = content.placeholder - isLargeDataset = state.isLargeDataset - if isEditableCell != state.isEditable { - isEditableCell = state.isEditable - needsRedraw = true - } - - let nextTint: NSColor? - if state.visualState.isDeleted || state.visualState.isInserted { - nextTint = nil - } else if state.visualState.isModified(columnIndex: state.columnIndex) { - nextTint = palette.modifiedColumnTint - } else { - nextTint = nil - } - if !colorsEqual(modifiedColumnTint, nextTint) { - modifiedColumnTint = nextTint - needsRedraw = true - } - - let nextFindTint: NSColor? = state.isCurrentFindMatch ? palette.findMatchTint : nil - if !colorsEqual(findMatchTint, nextFindTint) { - findMatchTint = nextFindTint - cachedLine = nil - needsRedraw = true - } - - if visualState != state.visualState { - visualState = state.visualState - needsRedraw = true - } - if isFocusedCell != state.isFocused { - isFocusedCell = state.isFocused - updateFocusPresentation() - needsRedraw = true - } - setAccessibilityRowIndexRange(NSRange(location: state.row, length: 1)) - setAccessibilityColumnIndexRange(NSRange(location: state.columnIndex, length: 1)) - - if needsRedraw { - needsDisplay = true - } - } - - override func accessibilityValue() -> Any? { - accessibilityText - } - - override func accessibilityLabel() -> String? { - String( - format: String(localized: "Row %d, column %d: %@"), - cellRow + 1, - cellColumnIndex + 1, - accessibilityText - ) - } - - private var accessibilityText: String { - switch placeholder { - case .none: - return displayText - case .null: - return displayText.isEmpty ? String(localized: "NULL") : displayText - case .empty: - return displayText.isEmpty ? String(localized: "Empty") : displayText - case .defaultMarker: - return displayText.isEmpty ? String(localized: "DEFAULT") : displayText - } - } - - func applyEmphasizedSelection(_ value: Bool) { - guard onEmphasizedSelection != value else { return } - onEmphasizedSelection = value - cachedLine = nil - updateFocusPresentation() - } - - func applyOverlayActive(_ value: Bool) { - guard hasOverlay != value else { return } - hasOverlay = value - updateFocusPresentation() - needsDisplay = true - } - - private func updateFocusPresentation() { - let shouldShowRing = isFocusedCell && !onEmphasizedSelection && !hasOverlay - focusRingType = shouldShowRing ? .exterior : .none - noteFocusRingMaskChanged() - needsDisplay = true - } - - override var focusRingMaskBounds: NSRect { - (onEmphasizedSelection || hasOverlay) ? .zero : bounds - } - - override func drawFocusRingMask() { - guard !onEmphasizedSelection, !hasOverlay else { return } - NSBezierPath(rect: bounds).fill() - } - - override func setFrameSize(_ newSize: NSSize) { - super.setFrameSize(newSize) - needsDisplay = true - } - - override func draw(_ dirtyRect: NSRect) { - if let tint = findMatchTint { - tint.setFill() - bounds.fill() - } else if let tint = modifiedColumnTint, !onEmphasizedSelection { - tint.setFill() - bounds.fill() - } - - let accessory = currentAccessory - let accessoryRect = accessory.frame(in: bounds) - - NSGraphicsContext.current?.saveGraphicsState() - NSBezierPath(rect: bounds).addClip() - drawText(availableWidth: accessory.availableTextWidth(in: bounds)) - drawAccessory(accessory, in: accessoryRect) - NSGraphicsContext.current?.restoreGraphicsState() - - if isFocusedCell && onEmphasizedSelection && !hasOverlay { - drawFocusBorder() - } - } - - private func drawText(availableWidth: CGFloat) { - guard !displayText.isEmpty else { return } - guard availableWidth > 0 else { return } - guard let context = NSGraphicsContext.current?.cgContext else { return } - - let fullLine = cachedCTLine() - let typographicWidth = CTLineGetTypographicBounds(fullLine, nil, nil, nil) - let ellipsisLine = makeEllipsisLine() - let ellipsisWidth = CTLineGetTypographicBounds(ellipsisLine, nil, nil, nil) - guard Double(availableWidth) >= ellipsisWidth else { return } - - let lineToDraw: CTLine - if typographicWidth > Double(availableWidth) { - lineToDraw = CTLineCreateTruncatedLine(fullLine, Double(availableWidth), .end, ellipsisLine) ?? ellipsisLine - } else { - lineToDraw = fullLine - } - - let baselineY = (bounds.height - textFont.ascender + textFont.descender - textFont.leading) / 2 + textFont.ascender - - context.saveGState() - context.textMatrix = CGAffineTransform(scaleX: 1, y: -1) - context.textPosition = CGPoint(x: DataGridMetrics.cellHorizontalInset, y: baselineY) - CTLineDraw(lineToDraw, context) - context.restoreGState() - } - - private func resolvedTextColor() -> NSColor { - if findMatchTint != nil { return .black } - return onEmphasizedSelection ? .alternateSelectedControlTextColor : textColor - } - - private func cachedCTLine() -> CTLine { - if let cached = cachedLine { return cached } - let textNS = displayText as NSString - let truncated: String - if textNS.length > 300 { - truncated = textNS.substring(to: 300) + "\u{2026}" - } else { - truncated = displayText - } - let attr = NSAttributedString( - string: truncated, - attributes: [ - .font: textFont, - .foregroundColor: resolvedTextColor() - ] - ) - let line = CTLineCreateWithAttributedString(attr as CFAttributedString) - cachedLine = line - return line - } - - private func makeEllipsisLine() -> CTLine { - let attr = NSAttributedString( - string: "\u{2026}", - attributes: [ - .font: textFont, - .foregroundColor: resolvedTextColor() - ] - ) - return CTLineCreateWithAttributedString(attr as CFAttributedString) - } - - private var currentAccessory: DataGridCellAccessory { - DataGridCellAccessory.visible( - for: kind, - isEditable: isEditableCell, - rawValue: rawValue - ) - } - - private func drawAccessory(_ accessory: DataGridCellAccessory, in rect: NSRect) { - guard !rect.isEmpty else { return } - let role: AccessoryRole - switch accessory { - case .foreignKey: - role = onEmphasizedSelection ? .foreignKeyEmphasized : .foreignKeyNormal - case .chevron: - if visualState.isDeleted { - role = .chevronDisabled - } else if onEmphasizedSelection { - role = .chevronEmphasized - } else { - role = .chevronNormal - } - case .none: - return - } - guard let glyph = Self.accessoryGlyph(for: role), - let context = NSGraphicsContext.current?.cgContext else { return } - let drawRect = Self.centeredGlyphRect(pointSize: glyph.pointSize, in: rect) - context.saveGState() - context.translateBy(x: drawRect.minX, y: drawRect.maxY) - context.scaleBy(x: 1, y: -1) - context.draw(glyph.image, in: CGRect(origin: .zero, size: drawRect.size)) - context.restoreGState() - } - - /// A symbol stretched to fill the accessory rect stops looking like a system symbol, so the - /// glyph draws at its own point size and the rect only ever clamps it. The clamp is one factor - /// across both axes, because clamping each axis on its own would distort the glyph exactly the - /// way filling the rect did. The origin rounds to whole points so a glyph narrower than its rect - /// by an odd number of points does not land on a half point and blur at 1x. - private static func centeredGlyphRect(pointSize: NSSize, in rect: NSRect) -> NSRect { - let scale = min(1, rect.width / pointSize.width, rect.height / pointSize.height) - let size = NSSize( - width: pointSize.width * scale, - height: pointSize.height * scale - ) - return NSRect( - x: (rect.midX - size.width / 2).rounded(), - y: (rect.midY - size.height / 2).rounded(), - width: size.width, - height: size.height - ) - } - - private func drawFocusBorder() { - let path = NSBezierPath(rect: bounds.insetBy(dx: 1, dy: 1)) - path.lineWidth = 2 - NSColor.alternateSelectedControlTextColor.setStroke() - path.stroke() - } - - override func mouseDown(with event: NSEvent) { - let point = convert(event.locationInWindow, from: nil) - let accessory = currentAccessory - let accessoryRect = accessory.frame(in: bounds) - guard !accessoryRect.isEmpty, accessoryRect.contains(point) else { - if event.clickCount == 2 { - accessoryDelegate?.dataGridCellDidDoubleClick(row: cellRow, columnIndex: cellColumnIndex) - return - } - super.mouseDown(with: event) - return - } - switch accessory { - case .foreignKey: - let openInNewTab = event.modifierFlags.contains(.command) - accessoryDelegate?.dataGridCellDidClickFKArrow( - row: cellRow, - columnIndex: cellColumnIndex, - openInNewTab: openInNewTab - ) - return - case .chevron where !visualState.isDeleted: - accessoryDelegate?.dataGridCellDidClickChevron(row: cellRow, columnIndex: cellColumnIndex) - return - case .none, .chevron: - super.mouseDown(with: event) - } - } - - private func colorsEqual(_ lhs: NSColor?, _ rhs: NSColor?) -> Bool { - switch (lhs, rhs) { - case (nil, nil): return true - case let (l?, r?): return l == r - default: return false - } - } -} diff --git a/TablePro/Views/Results/ColumnWindowResolver.swift b/TablePro/Views/Results/ColumnWindowResolver.swift deleted file mode 100644 index 29494f041..000000000 --- a/TablePro/Views/Results/ColumnWindowResolver.swift +++ /dev/null @@ -1,130 +0,0 @@ -// -// ColumnWindowResolver.swift -// TablePro -// - -import Foundation - -/// Chooses which columns a wide result keeps mounted. -/// -/// `NSTableView` virtualises rows but never columns: a prepared row builds one cell view for every -/// column that is not hidden, whatever the horizontal viewport shows. At 500 columns that is -/// ~18,000 live views, and every scroll frame lays out and draws all of them. Hidden columns cost -/// almost nothing, so the fix is to keep only the columns near the viewport unhidden and give the -/// document its full width back through a spacer at each end. -/// -/// Pure on purpose: the geometry is the part worth testing, and it needs no table view to decide. -internal enum ColumnWindowResolver { - /// Columns kept mounted on each side of the viewport, so a small scroll does not re-window. - internal static let overscan = 10 - - /// Columns of headroom that must remain between the viewport and the edge of the mounted range - /// before the window is left alone. - /// - /// Must stay below `overscan`, or the margin can never be satisfied and every scroll re-windows. - /// Sliding per column put the re-window tile inside the scroll frame and traded a steady cost - /// for an intermittent stall, so the window holds until this headroom is spent and then jumps - /// by a whole overscan. - internal static let slideMargin = 3 - - internal struct Window: Equatable { - let range: Range - let leadingWidth: CGFloat - let trailingWidth: CGFloat - - internal static let empty = Window(range: 0..<0, leadingWidth: 0, trailingWidth: 0) - } - - /// - Parameter current: the mounted range, so an unchanged decision returns it verbatim and the - /// caller can skip the tile entirely. - internal static func resolve( - columnWidths: [CGFloat], - viewportMinX: CGFloat, - viewportWidth: CGFloat, - current: Range? - ) -> Window { - guard !columnWidths.isEmpty else { return .empty } - guard viewportWidth > 0 else { - return window(for: 0..= visible.upperBound { - let leadingSlack = visible.lowerBound - current.lowerBound - let trailingSlack = current.upperBound - visible.upperBound - let atStart = current.lowerBound == 0 - let atEnd = current.upperBound == columnWidths.count - if (leadingSlack >= slideMargin || atStart) && (trailingSlack >= slideMargin || atEnd) { - return window(for: current, columnWidths: columnWidths) - } - } - - return window(for: desired, columnWidths: columnWidths) - } - - /// A window over `index`, for a caller that needs a column's frame before the viewport has - /// reached it. - /// - /// Re-centres rather than stretching the mounted range out to reach the target. Spanning from - /// the current range to a far column mounts every column in between, which is the whole cost - /// the window exists to avoid: measured at 848ms and 3,081 cell views for one Find match 90 - /// columns away, and 4.8s at 500 columns. The caller scrolls in the same turn, so the columns - /// this drops were never drawn again anyway. - /// - /// - Returns: `nil` when the range already covers `index` and there is nothing to mount. - internal static func window( - containing index: Int, - columnWidths: [CGFloat], - current: Range? - ) -> Window? { - guard columnWidths.indices.contains(index) else { return nil } - if let current, current.contains(index) { return nil } - return window(for: padded(index..<(index + 1), count: columnWidths.count), columnWidths: columnWidths) - } - - /// The columns the viewport actually intersects. Always at least one column, so a viewport - /// narrower than a single column still mounts the one under it. - private static func visibleRange( - columnWidths: [CGFloat], - viewportMinX: CGFloat, - viewportWidth: CGFloat - ) -> Range { - let viewportMaxX = viewportMinX + viewportWidth - var offset: CGFloat = 0 - var first: Int? - var last = columnWidths.count - 1 - - for (index, width) in columnWidths.enumerated() { - let columnMaxX = offset + width - if first == nil, columnMaxX > viewportMinX { - first = index - } - if offset >= viewportMaxX { - last = max(index - 1, first ?? 0) - break - } - offset = columnMaxX - } - - let lower = first ?? max(columnWidths.count - 1, 0) - return lower.., count: Int) -> Range { - let lower = max(0, range.lowerBound - overscan) - let upper = min(count, range.upperBound + overscan) - return lower.., columnWidths: [CGFloat]) -> Window { - let leading = columnWidths[0.. = [] - private var windowedRange: Range? - - private lazy var leadingSpacer = Self.makeSpacer(ColumnIdentitySchema.leadingSpacerIdentifier) - private lazy var trailingSpacer = Self.makeSpacer(ColumnIdentitySchema.trailingSpacerIdentifier) var totalSlots: Int { pooledColumns.count } - private static func makeSpacer(_ identifier: NSUserInterfaceItemIdentifier) -> NSTableColumn { - let column = NSTableColumn(identifier: identifier) - column.minWidth = 0 - column.maxWidth = .greatestFiniteMagnitude - column.width = 0 - column.resizingMask = [] - column.isEditable = false - column.isHidden = true - // The data grid header owns all of its own chrome (#2017): NSTableHeaderCell paints a fixed - // 28pt band with a divider and a rule that land mid-cell in this header's 42pt. A spacer is - // normally off screen, but it must not be the one cell that asks AppKit to paint. - column.headerCell = SortableHeaderCell(textCell: "") - return column - } - func attach(to tableView: NSTableView) { attachedTableView = tableView } @@ -54,16 +35,41 @@ final class DataGridColumnPool { var hasUserHiddenColumns: Bool { !userHiddenIdentifiers.isEmpty } + /// Whether this position in `tableColumns` holds one of the columns the result presents. + /// + /// The row-number column is an attached column too, and the pool keeps the surplus slots of a + /// previously wider result attached and hidden, so no fixed position answers this. + func presentsColumn(atTableColumnIndex index: Int, in tableView: NSTableView) -> Bool { + guard index >= 0, index < tableView.tableColumns.count else { return false } + return presentsColumn(tableView.tableColumns[index]) + } + + func firstPresentedColumnIndex(in tableView: NSTableView) -> Int? { + tableView.tableColumns.firstIndex { presentsColumn($0) } + } + + func lastPresentedColumnIndex(in tableView: NSTableView) -> Int? { + tableView.tableColumns.lastIndex { presentsColumn($0) } + } + + func nextPresentedColumnIndex(after index: Int, in tableView: NSTableView) -> Int? { + let start = max(0, index + 1) + guard start < tableView.tableColumns.count else { return nil } + return tableView.tableColumns[start...].firstIndex { presentsColumn($0) } + } + + func previousPresentedColumnIndex(before index: Int, in tableView: NSTableView) -> Int? { + let end = min(max(0, index), tableView.tableColumns.count) + guard end > 0 else { return nil } + return tableView.tableColumns[.. 0 else { - unmountNothing(candidates) - return - } - - // A mounted column contributes its width plus one intercell gap; a hidden one contributes - // nothing. A spacer standing in for N columns has to carry their gaps too, or the document - // ends up short and the last columns cannot be reached. - let spacing = tableView.intercellSpacing.width - let window = ColumnWindowResolver.resolve( - columnWidths: candidates.map { $0.width + spacing }, - viewportMinX: viewport.minX - leadingChromeWidth(in: tableView, before: candidates[0]), - viewportWidth: viewport.width, - current: windowedRange - ) - guard window.range != windowedRange else { return } - mount(window, over: candidates, in: tableView) - } - - /// Mounts a column the window left out, so anything that reads its frame gets a real rect. - /// - /// `rect(ofColumn:)` and `frameOfCell(atColumn:row:)` are both empty for a hidden column, so - /// `scrollColumnToVisible` scrolls to the document origin instead of the column, and the inline - /// editor's own empty-frame guard makes it open nothing at all (#2381). - /// - Returns: whether the window had to widen, so the caller can drop it and let the next - /// resolve pick a tight one instead of leaving the widened range mounted. - @discardableResult - func mountColumn(_ column: NSTableColumn, in tableView: NSTableView) -> Bool { - guard presentsColumn(column) else { return false } - let candidates = presentedColumns(in: tableView) - guard let position = candidates.firstIndex(of: column) else { return false } - - let spacing = tableView.intercellSpacing.width - guard let window = ColumnWindowResolver.window( - containing: position, - columnWidths: candidates.map { $0.width + spacing }, - current: windowedRange - ) else { return false } - mount(window, over: candidates, in: tableView) - return true - } - - /// The first and last columns the result presents, in display order. The pool owns these - /// because the spacers are attached columns too and one of them sits immediately before the - /// first data column, so no fixed position can name either end. - func firstPresentedColumnIndex(in tableView: NSTableView) -> Int? { - tableView.tableColumns.firstIndex { presentsColumn($0) } - } - - func lastPresentedColumnIndex(in tableView: NSTableView) -> Int? { - tableView.tableColumns.lastIndex { presentsColumn($0) } - } - - func nextPresentedColumnIndex(after index: Int, in tableView: NSTableView) -> Int? { - let start = max(0, index + 1) - guard start < tableView.tableColumns.count else { return nil } - return tableView.tableColumns[start...].firstIndex { presentsColumn($0) } - } - - func previousPresentedColumnIndex(before index: Int, in tableView: NSTableView) -> Int? { - let end = min(max(0, index), tableView.tableColumns.count) - guard end > 0 else { return nil } - return tableView.tableColumns[.. Bool { - guard index >= 0, index < tableView.tableColumns.count else { return false } - return presentsColumn(tableView.tableColumns[index]) - } - - private func presentedColumns(in tableView: NSTableView) -> [NSTableColumn] { - tableView.tableColumns.filter { presentsColumn($0) } - } - - private func mount( - _ window: ColumnWindowResolver.Window, - over candidates: [NSTableColumn], - in tableView: NSTableView - ) { - windowedRange = window.range - for (index, column) in candidates.enumerated() { - let mounted = window.range.contains(index) - if column.isHidden == mounted { - column.isHidden = !mounted - } - } - let spacing = tableView.intercellSpacing.width - applySpacer(leadingSpacer, width: spacerWidth(window.leadingWidth, spacing: spacing), in: tableView) - applySpacer(trailingSpacer, width: spacerWidth(window.trailingWidth, spacing: spacing), in: tableView) - } - - /// The document starts at the row-number column, but the resolver measures from the first data - /// column, so the viewport has to be rebased before it can index into those widths. - /// - /// Only chrome counts. The leading spacer sits ahead of the first data column and holds exactly - /// the width of the columns the window left out, which the resolver's own model already carries, - /// so counting it here subtracts that width twice and walks the window left while the reader - /// scrolls right, until it parks off screen and the grid paints nothing (#2381). - private func leadingChromeWidth(in tableView: NSTableView, before firstColumn: NSTableColumn) -> CGFloat { - tableView.tableColumns - .prefix { $0.identifier != firstColumn.identifier } - .filter { !$0.isHidden && !ColumnIdentitySchema.isSpacer($0.identifier) } - .reduce(0) { $0 + $1.width + tableView.intercellSpacing.width } - } - - /// The resolver works in per-column slots that already include one gap each. A spacer is a - /// single column, so it keeps one gap of its own and absorbs the rest as width. - private func spacerWidth(_ slotWidth: CGFloat, spacing: CGFloat) -> CGFloat { - guard slotWidth > 0 else { return 0 } - return max(0, slotWidth - spacing) - } - - /// A table that has not been laid out yet reports no viewport, and windowing against that would - /// hide every column. Mount everything until there is a real width to measure against. - private func unmountNothing(_ candidates: [NSTableColumn]) { - windowedRange = nil - for column in candidates where column.isHidden { - column.isHidden = false - } - hideSpacers() - } - - private func applySpacer(_ spacer: NSTableColumn, width: CGFloat, in tableView: NSTableView) { - if spacer.width != width { - spacer.width = width - } - let hidden = width <= 0 - if spacer.isHidden != hidden { - spacer.isHidden = hidden - } - } - - private func hideSpacers() { - for spacer in [leadingSpacer, trailingSpacer] where !spacer.isHidden { - spacer.isHidden = true - spacer.width = 0 - } } private func growBackingPoolIfNeeded(to count: Int) { @@ -356,11 +200,6 @@ final class DataGridColumnPool { attached.insert(pooledColumns[slot].identifier) } - for spacer in [leadingSpacer, trailingSpacer] where !attached.contains(spacer.identifier) { - tableView.addTableColumn(spacer) - attached.insert(spacer.identifier) - } - NSAnimationContext.beginGrouping() NSAnimationContext.current.duration = 0 NSAnimationContext.current.allowsImplicitAnimation = false @@ -382,22 +221,6 @@ final class DataGridColumnPool { tableView.moveColumn(currentIndex, toColumn: desiredIndex) updateIndexMap(&indexByIdentifier, movedFrom: currentIndex, to: desiredIndex) } - - positionSpacers(in: tableView, baseOffset: baseOffset) - } - - /// The leading spacer stands in for everything scrolled off to the left, so it has to sit ahead - /// of the first data column; the trailing spacer holds the rest and sits last. - private func positionSpacers(in tableView: NSTableView, baseOffset: Int) { - move(leadingSpacer, to: baseOffset, in: tableView) - move(trailingSpacer, to: tableView.tableColumns.count - 1, in: tableView) - } - - private func move(_ column: NSTableColumn, to index: Int, in tableView: NSTableView) { - guard let current = tableView.tableColumns.firstIndex(of: column) else { return } - let clamped = max(0, min(index, tableView.tableColumns.count - 1)) - guard current != clamped else { return } - tableView.moveColumn(current, toColumn: clamped) } private func updateIndexMap( diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 32f83be5c..e120e0fe3 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -162,20 +162,9 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData } /// The single way to reach a column, for Find, cell navigation and the inline editor alike. - /// - /// A column the window left out has no frame at all, so `scrollColumnToVisible` scrolls to the - /// document origin instead of the column and the editor's own empty-frame guard opens nothing. - /// Mounting first gives it one. Only a mount that had to widen the window drops it afterwards, - /// so stepping column by column keeps the resolver's hysteresis instead of re-windowing on - /// every keystroke. func scrollColumnToVisible(tableColumnIndex index: Int) { guard let tableView, index >= 0, index < tableView.numberOfColumns else { return } - let widened = columnPool.mountColumn(tableView.tableColumns[index], in: tableView) tableView.scrollColumnToVisible(index) - if widened { - columnPool.invalidateColumnWindow() - } - updateColumnWindow() } /// The columns the user is looking at, which is every presented column and not merely the @@ -388,6 +377,34 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData let cellFactory = DataGridCellFactory() let cellRegistry: DataGridCellRegistry let columnPool = DataGridColumnPool() + /// Draws every data cell in the grid. One renderer for the whole table, because its only state + /// is a cache of laid-out lines and every row draws the same values as it scrolls. + let cellRenderer = DataGridCellRenderer() + /// The cell an overlay editor or viewer is open over, which draws no text of its own behind it. + var overlayCell: CellPosition? { + didSet { + guard overlayCell != oldValue else { return } + for position in [oldValue, overlayCell].compactMap({ $0 }) { + redrawCell(row: position.row, columnIndex: position.column) + } + } + } + + /// Repaints every drawn cell on screen, for a change that moves all of them at once. + func redrawVisibleCells() { + guard let tableView else { return } + tableView.enumerateAvailableRowViews { rowView, _ in + (rowView as? DataGridRowView)?.redrawCells() + } + } + + /// Repaints one drawn cell, which is what a mounted cell got from `setNeedsDisplay` on itself. + func redrawCell(row: Int, columnIndex: Int) { + guard let tableView, + let rowView = tableView.rowView(atRow: row, makeIfNecessary: false) as? DataGridRowView, + let position = tableColumnIndex(for: columnIndex) else { return } + rowView.redrawCell(atTableColumnIndex: position) + } let selectionController = GridSelectionController() var overlayEditor: CellOverlayEditor? var overlayViewer: CellOverlayViewer? @@ -842,36 +859,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData self?.schedulePrewarmResume() } } - scrollView.contentView.postsBoundsChangedNotifications = true - let bounds = NotificationCenter.default.addObserver( - forName: NSView.boundsDidChangeNotification, - object: scrollView.contentView, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.updateColumnWindow() - } - } - // A bounds change is scrolling; a frame change is the viewport resizing. Widening the - // window exposes area the window never covered, and only this fires for that. - scrollView.contentView.postsFrameChangedNotifications = true - let frame = NotificationCenter.default.addObserver( - forName: NSView.frameDidChangeNotification, - object: scrollView.contentView, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.updateColumnWindow() - } - } - scrollObservers = [start, end, bounds, frame] - } - - /// Re-mounts the columns the viewport can now reach. The resolver keeps the range stable while - /// the viewport stays inside its margin, so most scroll frames return without touching a column. - func updateColumnWindow() { - guard let tableView, !isRebuildingColumns else { return } - columnPool.applyColumnWindow(in: tableView) + scrollObservers = [start, end] } private func detachScrollObservers() { @@ -980,7 +968,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData for row in rowSet { visualIndex.updateRow(row, from: changeManager, displayIDs: displayIDs) } - tableView.reloadData(forRowIndexes: rowSet, columnIndexes: colSet) + redrawCells(rows: rowSet, tableColumnIndexes: colSet) case .rowsInserted(let indices): guard !indices.isEmpty else { return } overlayEditor?.dismiss(commit: false) @@ -1168,7 +1156,24 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData let visibleRows = IndexSet( integersIn: visibleRange.location..<(visibleRange.location + visibleRange.length) ) - tableView.reloadData(forRowIndexes: visibleRows, columnIndexes: changedTableColumnIndices) + redrawCells(rows: visibleRows, tableColumnIndexes: changedTableColumnIndices) + } + + /// Repaints a set of drawn cells. + /// + /// `reloadData(forRowIndexes:columnIndexes:)` rebuilt a cell view per pair, which is how a + /// mounted cell was refreshed. A data cell has no view now, so that call reaches nothing and the + /// change never appears; the rows that draw the cells are asked instead. + func redrawCells(rows: IndexSet, tableColumnIndexes: IndexSet) { + guard let tableView else { return } + for row in rows { + guard let rowView = tableView.rowView(atRow: row, makeIfNecessary: false) as? DataGridRowView else { + continue + } + for tableColumnIndex in tableColumnIndexes { + rowView.redrawCell(atTableColumnIndex: tableColumnIndex) + } + } } func flushPendingCellPresentationRefresh() { diff --git a/TablePro/Views/Results/DataGridRowView.swift b/TablePro/Views/Results/DataGridRowView.swift index 3e2ce0c00..30c95eb99 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -20,11 +20,186 @@ class DataGridRowView: NSTableRowView { private(set) var visualState: RowVisualState = .empty private var rowTint: NSColor? + /// Draws the row's data cells. + /// + /// A subview rather than the row view's own `draw(_:)`, so the cells land after AppKit has + /// painted the row background and the selection, which is the order a mounted cell view got. + private let contentView = DataGridRowContentView() + override init(frame frameRect: NSRect) { super.init(frame: frameRect) wantsLayer = true layerContentsRedrawPolicy = .onSetNeedsDisplay canDrawSubviewsIntoLayer = true + contentView.rowView = self + contentView.autoresizingMask = [.width, .height] + contentView.frame = bounds + addSubview(contentView) + } + + /// Repaints one cell, the way a mounted cell view repainted itself. + func redrawCell(atTableColumnIndex tableColumnIndex: Int) { + guard let tableView = coordinator?.tableView else { + contentView.needsDisplay = true + return + } + let columnRect = tableView.rect(ofColumn: tableColumnIndex) + contentView.setNeedsDisplay( + NSRect(x: columnRect.minX, y: 0, width: columnRect.width, height: contentView.bounds.height) + ) + } + + func redrawCells() { + contentView.needsDisplay = true + accessibilityCellsAreStale = true + } + + // MARK: - Accessibility + + /// One element per data column, vended as this row's accessibility children. + /// + /// A mounted cell view was its own accessibility element and AppKit built the AXCell tree from + /// those. Drawn cells have no view to carry that, so the row vends the elements itself, which is + /// what `NSAccessibilityElement` exists for. Owned for the row's lifetime and reconfigured in + /// place, because an element handed to an assistive client must not be replaced underneath it. + private var accessibilityCells: [NSAccessibilityElement] = [] + /// Accessibility is pull-based, so the elements are built when something asks for them and never + /// on the draw path. Building them there cost a formatted value per column per repaint, which is + /// 500 of them on a wide result every time the selection moved, with nothing reading the result + /// unless an assistive client is attached. + private var accessibilityCellsAreStale = true + + private func rebuildAccessibilityCellsIfStale() { + guard accessibilityCellsAreStale else { return } + rebuildAccessibilityCells() + } + + private func rebuildAccessibilityCells() { + accessibilityCellsAreStale = false + guard let coordinator, let tableView = coordinator.tableView else { return } + let columnCount = coordinator.identitySchema.totalDataColumns + if accessibilityCells.count != columnCount { + accessibilityCells = (0.. NSAccessibilityElement? { + rebuildAccessibilityCellsIfStale() + guard accessibilityCells.indices.contains(dataColumn) else { return nil } + return accessibilityCells[dataColumn] + } + + override func accessibilityChildren() -> [Any]? { + rebuildAccessibilityCellsIfStale() + return accessibilityCells.map { $0 as Any } + } + + override func accessibilityRole() -> NSAccessibility.Role? { .row } + + override func accessibilityHitTest(_ point: NSPoint) -> Any? { + guard let window else { return super.accessibilityHitTest(point) } + let local = convert(window.convertPoint(fromScreen: point), from: nil) + rebuildAccessibilityCellsIfStale() + for element in accessibilityCells where element.accessibilityFrameInParentSpace().contains(local) { + return element + } + return super.accessibilityHitTest(point) + } + + /// The click a cell view used to take for itself: the in-cell accessory, then a double click. + /// + /// - Returns: whether the click was consumed, leaving the table view's own selection handling + /// to everything else. + func handleCellClick(at point: NSPoint, in view: NSView, clickCount: Int, modifiers: NSEvent.ModifierFlags) -> Bool { + guard let coordinator, let tableView = coordinator.tableView else { return false } + let inTableView = view.convert(point, to: tableView) + let tableColumnIndex = tableView.column(at: inTableView) + guard tableColumnIndex >= 0, tableColumnIndex < tableView.tableColumns.count, + let dataColumn = coordinator.dataColumnIndex(from: tableView.tableColumns[tableColumnIndex].identifier) + else { return false } + + let columnRect = view.convert(tableView.rect(ofColumn: tableColumnIndex), from: tableView) + let cellRect = NSRect(x: columnRect.minX, y: 0, width: columnRect.width, height: view.bounds.height) + guard let appearance = coordinator.cellAppearance( + row: rowIndex, + columnIndex: dataColumn, + onEmphasizedSelection: isSelected && isEmphasized + ) else { return false } + + let accessoryRect = appearance.accessory.frame(in: cellRect) + guard !accessoryRect.isEmpty, accessoryRect.contains(point) else { + guard clickCount == 2 else { return false } + coordinator.dataGridCellDidDoubleClick(row: rowIndex, columnIndex: dataColumn) + return true + } + + switch appearance.accessory { + case .foreignKey: + coordinator.dataGridCellDidClickFKArrow( + row: rowIndex, + columnIndex: dataColumn, + openInNewTab: modifiers.contains(.command) + ) + return true + case .chevron where !visualState.isDeleted: + coordinator.dataGridCellDidClickChevron(row: rowIndex, columnIndex: dataColumn) + return true + case .none, .chevron: + return false + } + } + + /// Draws every data cell the dirty area touches. + /// + /// The columns are still real `NSTableColumn`s, so AppKit answers which of them the area covers + /// and where each one sits; only the cell content is drawn rather than mounted. + func drawCells(in dirtyRect: NSRect, of view: NSView) { + guard let coordinator, let tableView = coordinator.tableView else { return } + let inTableView = view.convert(dirtyRect, to: tableView) + let onEmphasizedSelection = isSelected && isEmphasized + + for tableColumnIndex in tableView.columnIndexes(in: inTableView) { + guard tableColumnIndex < tableView.tableColumns.count else { continue } + let identifier = tableView.tableColumns[tableColumnIndex].identifier + guard let dataColumn = coordinator.dataColumnIndex(from: identifier) else { continue } + guard let appearance = coordinator.cellAppearance( + row: rowIndex, + columnIndex: dataColumn, + onEmphasizedSelection: onEmphasizedSelection + ) else { continue } + + let columnRect = view.convert(tableView.rect(ofColumn: tableColumnIndex), from: tableView) + coordinator.cellRenderer.draw( + appearance, + in: NSRect(x: columnRect.minX, y: 0, width: columnRect.width, height: view.bounds.height) + ) + } } required init?(coder: NSCoder) { @@ -78,19 +253,10 @@ class DataGridRowView: NSTableRowView { } } - override func didAddSubview(_ subview: NSView) { - super.didAddSubview(subview) - guard let cell = subview as? DataGridCellView else { return } - cell.applyEmphasizedSelection(isSelected && isEmphasized) - } - + /// Selection recolours every cell's text, so the row repaints its own cells rather than telling + /// a set of cell views to repaint themselves. private func propagateEmphasisToCells() { - let emphasized = isSelected && isEmphasized - for subview in subviews { - guard let cell = subview as? DataGridCellView else { continue } - cell.applyEmphasizedSelection(emphasized) - cell.needsDisplay = true - } + redrawCells() } override func drawBackground(in dirtyRect: NSRect) { @@ -587,3 +753,31 @@ private final class DateSetterContext { self.value = value } } + +/// The view a row's data cells are drawn into. +/// +/// Its own class so the drawing lands after the row's background and selection, and so one row +/// costs exactly one view however many columns the result has. +@MainActor +final class DataGridRowContentView: NSView { + weak var rowView: DataGridRowView? + + override var isFlipped: Bool { true } + override var allowsVibrancy: Bool { false } + + override func draw(_ dirtyRect: NSRect) { + rowView?.drawCells(in: dirtyRect, of: self) + } + + override func mouseDown(with event: NSEvent) { + let point = convert(event.locationInWindow, from: nil) + let consumed = rowView?.handleCellClick( + at: point, + in: self, + clickCount: event.clickCount, + modifiers: event.modifierFlags.intersection(.deviceIndependentFlagsMask) + ) ?? false + guard !consumed else { return } + super.mouseDown(with: event) + } +} diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index ad409a4cc..6e5119ef6 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -12,7 +12,9 @@ extension TableViewCoordinator { func handleCellInteraction(row: Int, tableColumn: Int, columnIndex: Int, tableView: NSTableView) { guard let context = makeCellContext(row: row, columnIndex: columnIndex) else { return } - guard tableView.view(atColumn: tableColumn, row: row, makeIfNecessary: false) != nil else { return } + // A data cell is drawn rather than mounted, so the row being on screen is what says the + // interaction has somewhere to land. Asking for a cell view here always answered nil. + guard tableView.rowView(atRow: row, makeIfNecessary: false) != nil else { return } switch CellInteractionResolver().resolve(context) { case .blocked: diff --git a/TablePro/Views/Results/Extensions/DataGridView+ColumnWidths.swift b/TablePro/Views/Results/Extensions/DataGridView+ColumnWidths.swift index 235ba4dcf..5edbda7ba 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+ColumnWidths.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+ColumnWidths.swift @@ -77,8 +77,7 @@ extension TableViewCoordinator { tableRows: TableRows ) { guard widenAutomaticColumns(forPresentationChanges: changes, tableRows: tableRows) else { return } - columnPool.invalidateColumnWindow() - updateColumnWindow() + redrawVisibleCells() } private func widenAutomaticColumns( diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index 3e1bd6fc7..ba66a53b4 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -12,80 +12,104 @@ extension TableViewCoordinator { autoreleasepool { viewForCell(in: tableView, column: tableColumn, row: row) } } + /// Only the row-number column still mounts a view. + /// + /// A data cell is drawn by its row instead. `NSTableView` builds one cell view per column per + /// prepared row whatever the viewport shows, so a 500-column result carried 12,500 views and + /// 837MB of them; returning nil here leaves 26 views in the whole table and 3.9MB (#2381). private func viewForCell(in tableView: NSTableView, column tableColumn: NSTableColumn?, row: Int) -> NSView? { guard let column = tableColumn else { return nil } + guard column.identifier == ColumnIdentitySchema.rowNumberIdentifier else { return nil } let tableRows = tableRowsProvider() - let displayCount = displayIDs?.count ?? tableRows.count - - if column.identifier == ColumnIdentitySchema.rowNumberIdentifier { - return cellRegistry.makeRowNumberCell( - in: tableView, - row: row, - pageOffset: paginationOffsetProvider(), - cachedRowCount: displayCount, - visualState: visualState(for: row) - ) - } - - guard let columnIndex = dataColumnIndex(from: column.identifier) else { - return nil - } + return cellRegistry.makeRowNumberCell( + in: tableView, + row: row, + pageOffset: paginationOffsetProvider(), + cachedRowCount: displayIDs?.count ?? tableRows.count, + visualState: visualState(for: row) + ) + } - guard row >= 0 && row < displayCount, - columnIndex >= 0 && columnIndex < cachedColumnCount else { - return nil - } + /// What one data cell looks like, for the row that draws it. + /// + /// - Parameters: + /// - onEmphasizedSelection: whether the row is drawn as selected, which changes the text + /// colour and stands the modified tint down. + func cellAppearance( + row: Int, + columnIndex: Int, + onEmphasizedSelection: Bool + ) -> DataGridCellAppearance? { + let tableRows = tableRowsProvider() + let displayCount = displayIDs?.count ?? tableRows.count + guard row >= 0, row < displayCount, + columnIndex >= 0, columnIndex < cachedColumnCount, + let displayRow = displayRow(at: row, in: tableRows), + columnIndex < displayRow.values.count else { return nil } - guard let displayRow = displayRow(at: row, in: tableRows), - columnIndex < displayRow.values.count else { - return nil - } let rawValue = displayRow.values[columnIndex] - let columnType = columnIndex < tableRows.columnTypes.count - ? tableRows.columnTypes[columnIndex] - : nil + let columnType = columnIndex < tableRows.columnTypes.count ? tableRows.columnTypes[columnIndex] : nil let formattedValue = displayValue( forID: displayRow.id, column: columnIndex, rawValue: rawValue, columnType: columnType ) - let state = visualState(for: row) let isFocused: Bool = { guard let keyTableView = tableView as? KeyHandlingTableView, keyTableView.focusedRow == row, - let tableColumnIndex = tableColumnIndex(for: columnIndex), - keyTableView.focusedColumn == tableColumnIndex else { return false } + let position = tableColumnIndex(for: columnIndex), + keyTableView.focusedColumn == position else { return false } return true }() - let presentation = columnPresentation(for: columnIndex, in: tableRows) - - let content = DataGridCellContent( - displayText: formattedValue ?? "", - rawValue: rawValue.asText, - placeholder: DataGridCellContent.placeholder(for: rawValue) - ) - let cellState = DataGridCellState( - visualState: state, - isFocused: isFocused, - isEditable: isEditable, - isLargeDataset: isLargeDataset, - isCurrentFindMatch: currentFindMatch == FindMatch(displayRow: row, columnIndex: columnIndex), - row: row, - columnIndex: columnIndex + return DataGridCellAppearance.resolve( + kind: columnPresentation(for: columnIndex, in: tableRows).kind, + content: DataGridCellContent( + displayText: formattedValue ?? "", + rawValue: rawValue.asText, + placeholder: DataGridCellContent.placeholder(for: rawValue) + ), + state: DataGridCellState( + visualState: visualState(for: row), + isFocused: isFocused, + isEditable: isEditable, + isLargeDataset: isLargeDataset, + isCurrentFindMatch: currentFindMatch == FindMatch(displayRow: row, columnIndex: columnIndex), + row: row, + columnIndex: columnIndex + ), + palette: cellRegistry.palette, + nullDisplayString: cellRegistry.nullDisplayString, + onEmphasizedSelection: onEmphasizedSelection, + hasOverlay: overlayCell == CellPosition(row: row, column: columnIndex) ) + } - let cell = cellRegistry.dequeueCell(in: tableView) - cell.configure( - kind: presentation.kind, - content: content, - state: cellState, - palette: cellRegistry.palette - ) - return cell + /// What VoiceOver reads for one cell. A placeholder announces itself by name rather than as the + /// empty string a sighted reader sees as an italic marker. + func accessibilityText(row: Int, columnIndex: Int) -> String? { + let tableRows = tableRowsProvider() + guard let displayRow = displayRow(at: row, in: tableRows), + columnIndex < displayRow.values.count else { return nil } + let rawValue = displayRow.values[columnIndex] + let columnType = columnIndex < tableRows.columnTypes.count ? tableRows.columnTypes[columnIndex] : nil + let text = displayValue( + forID: displayRow.id, + column: columnIndex, + rawValue: rawValue, + columnType: columnType + ) ?? "" + guard text.isEmpty else { return text } + + switch DataGridCellContent.placeholder(for: rawValue) { + case .null: return String(localized: "NULL") + case .empty: return String(localized: "Empty") + case .defaultMarker: return String(localized: "DEFAULT") + case .none: return text + } } func tableView(_ tableView: NSTableView, typeSelectStringFor tableColumn: NSTableColumn?, row: Int) -> String? { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift index ca81bbe46..67b54c248 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift @@ -371,11 +371,7 @@ extension TableViewCoordinator { fittedColumnCount: fittedColumns.count ) } - // Sizing reaches the columns the window unmounted as well, and the spacers stand in for - // those at the width they had before, so the document is short by the whole delta until - // the window is resolved again. - columnPool.invalidateColumnWindow() - updateColumnWindow() + redrawVisibleCells() scheduleLayoutPersist() } diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index e48dad970..5f537eaa7 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -77,7 +77,7 @@ final class KeyHandlingTableView: NSTableView { let validRows = pendingRows.filteredIndexSet { $0 < numberOfRows } let validColumns = pendingColumns.filteredIndexSet { $0 < numberOfColumns } guard !validRows.isEmpty, !validColumns.isEmpty else { return } - reloadData(forRowIndexes: validRows, columnIndexes: validColumns) + coordinator?.redrawCells(rows: validRows, tableColumnIndexes: validColumns) } var focusedRow: Int { @@ -497,13 +497,29 @@ final class KeyHandlingTableView: NSTableView { /// cell the grid's own cursor is on, so the cursor was invisible to it. The selected-cells /// override is clamped to the visible rows: AppKit will happily ask for every cell in a /// million-row selection otherwise. - /// The cursor moved, so assistive technology is told to re-read where focus now is. The - /// element itself stays the table: `NSTableView`'s own focused-element resolution already - /// walks to the cell, and overriding it in Swift is not available on this type. + /// The cursor moved, so assistive technology is told to re-read where focus now is. + /// + /// A cell is drawn rather than mounted, so the element comes from the row's own accessibility + /// children rather than from a cell view. internal func postCellCursorMoved() { guard selectedRow >= 0, presentsDataColumn(at: focusedColumn) else { return } - guard let cell = view(atColumn: focusedColumn, row: selectedRow, makeIfNecessary: false) else { return } - NSAccessibility.post(element: cell, notification: .focusedUIElementChanged) + guard let element = accessibilityCellElement(row: selectedRow, tableColumnIndex: focusedColumn) else { return } + NSAccessibility.post(element: element, notification: .focusedUIElementChanged) + } + + /// The accessibility element standing for one drawn cell. + private func accessibilityCellElement(row: Int, tableColumnIndex: Int) -> NSAccessibilityElement? { + guard let coordinator, + tableColumnIndex >= 0, tableColumnIndex < tableColumns.count, + let dataColumn = coordinator.dataColumnIndex(from: tableColumns[tableColumnIndex].identifier), + let rowView = rowView(atRow: row, makeIfNecessary: false) as? DataGridRowView else { return nil } + return rowView.accessibilityCell(forDataColumn: dataColumn) + } + + /// VoiceOver's table navigation asks for a cell by coordinate, which AppKit answered from the + /// cell views. It now comes from the row that draws them. + override func accessibilityCell(forColumn column: Int, row: Int) -> Any? { + accessibilityCellElement(row: row, tableColumnIndex: column) ?? super.accessibilityCell(forColumn: column, row: row) } override func accessibilitySelectedCells() -> [Any]? { @@ -516,8 +532,9 @@ final class KeyHandlingTableView: NSTableView { for rectangle in controller.selection.rectangles { for row in rectangle.rows where NSLocationInRange(row, visible) { for column in rectangle.columns { - guard let cell = view(atColumn: column, row: row, makeIfNecessary: false) else { continue } - cells.append(cell) + guard let position = coordinator?.tableColumnIndex(for: column), + let element = accessibilityCellElement(row: row, tableColumnIndex: position) else { continue } + cells.append(element) } } } diff --git a/TableProTests/Views/Results/ColumnWindowResolverTests.swift b/TableProTests/Views/Results/ColumnWindowResolverTests.swift deleted file mode 100644 index 792f09121..000000000 --- a/TableProTests/Views/Results/ColumnWindowResolverTests.swift +++ /dev/null @@ -1,191 +0,0 @@ -// -// ColumnWindowResolverTests.swift -// TableProTests -// -// NSTableView virtualises rows but never columns, so a 500-column result mounts a cell view per -// column per prepared row and every scroll frame lays all of them out. The window keeps the -// document's full width through spacers, so scroll extent has to survive every decision here. -// - -import Foundation -@testable import TablePro -import Testing - -@Suite("Column window resolver") -struct ColumnWindowResolverTests { - private let wide = Array(repeating: CGFloat(100), count: 500) - - private func totalWidth(_ window: ColumnWindowResolver.Window, widths: [CGFloat]) -> CGFloat { - let mounted = widths[window.range].reduce(0, +) - return window.leadingWidth + mounted + window.trailingWidth - } - - @Test("A range that already covers the column needs no widening") - func containingIsNilWhenAlreadyMounted() { - #expect(ColumnWindowResolver.window(containing: 20, columnWidths: wide, current: 10..<40) == nil) - } - - @Test("An out-of-bounds column widens nothing") - func containingIsNilOutOfBounds() { - #expect(ColumnWindowResolver.window(containing: 500, columnWidths: wide, current: nil) == nil) - } - - /// Stretching the mounted range out to reach a far column would mount every column in between, - /// which is the cost the window exists to avoid. The caller scrolls in the same turn, so the - /// columns it leaves behind are never drawn again. - @Test("Reaching a far column mounts a window around it, not everything in between") - func containingRecentresRatherThanStretches() throws { - let window = try #require( - ColumnWindowResolver.window(containing: 400, columnWidths: wide, current: 0..<30) - ) - - #expect(window.range.contains(400)) - #expect(window.range.count <= ColumnWindowResolver.overscan * 2 + 1) - #expect(totalWidth(window, widths: wide) == wide.reduce(0, +)) - } - - @Test("Reaching a column with no window yet mounts a window around it") - func containingFromNoWindow() throws { - let window = try #require( - ColumnWindowResolver.window(containing: 250, columnWidths: wide, current: nil) - ) - - #expect(window.range.contains(250)) - #expect(window.range.count < 50) - #expect(totalWidth(window, widths: wide) == wide.reduce(0, +)) - } - - @Test("No columns resolves to an empty window") - func emptyColumns() { - let window = ColumnWindowResolver.resolve( - columnWidths: [], viewportMinX: 0, viewportWidth: 800, current: nil - ) - #expect(window == .empty) - } - - @Test("A window mounts far fewer columns than the result has") - func windowIsSmall() { - let window = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 0, viewportWidth: 800, current: nil - ) - #expect(window.range.count < 50) - #expect(window.range.lowerBound == 0) - } - - /// The spacers exist so the horizontal scroller still spans the whole result. If this drifts, - /// the grid silently loses columns the user can no longer reach. - @Test( - "Document width is preserved at every scroll position", - arguments: [CGFloat(0), 1_000, 24_950, 49_200] - ) - func documentWidthPreserved(minX: CGFloat) { - let window = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: minX, viewportWidth: 800, current: nil - ) - #expect(totalWidth(window, widths: wide) == 50_000) - } - - @Test("Document width is preserved for 0, 1, 50 and 500 columns") - func documentWidthAcrossColumnCounts() { - for count in [1, 50, 500] { - let widths = Array(repeating: CGFloat(100), count: count) - let window = ColumnWindowResolver.resolve( - columnWidths: widths, viewportMinX: 0, viewportWidth: 800, current: nil - ) - #expect(totalWidth(window, widths: widths) == CGFloat(count) * 100) - } - } - - @Test("The window covers the columns under the viewport") - func windowCoversViewport() { - let window = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 10_000, viewportWidth: 800, current: nil - ) - #expect(window.range.contains(100)) - #expect(window.range.contains(107)) - } - - @Test("Scrolled to the far end the window stops at the last column") - func windowClampsAtEnd() { - let window = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 49_200, viewportWidth: 800, current: nil - ) - #expect(window.range.upperBound == 500) - #expect(window.trailingWidth == 0) - } - - // MARK: - Hysteresis - - /// Re-windowing costs a tile, and doing it per column put that tile inside the scroll frame. - @Test("A small scroll inside the mounted range does not move the window") - func smallScrollKeepsWindow() { - let first = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 10_000, viewportWidth: 800, current: nil - ) - let second = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 10_100, viewportWidth: 800, current: first.range - ) - #expect(second.range == first.range) - } - - @Test("Scrolling past the margin moves the window") - func largeScrollMovesWindow() { - let first = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 10_000, viewportWidth: 800, current: nil - ) - let second = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 20_000, viewportWidth: 800, current: first.range - ) - #expect(second.range != first.range) - #expect(second.range.contains(200)) - } - - /// The margin has to be smaller than the overscan, or it can never be satisfied and every - /// scroll re-windows. That is exactly what the first version of this did. - @Test("The slide margin leaves room to move inside the mounted window") - func marginFitsInsideOverscan() { - #expect(ColumnWindowResolver.slideMargin < ColumnWindowResolver.overscan) - } - - @Test("A window already at the start does not slide just because there is no leading margin") - func atStartDoesNotThrash() { - let first = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 0, viewportWidth: 800, current: nil - ) - let second = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 0, viewportWidth: 800, current: first.range - ) - #expect(second.range == first.range) - #expect(second.leadingWidth == 0) - } - - @Test("A viewport wider than the result mounts everything") - func viewportWiderThanResult() { - let widths = Array(repeating: CGFloat(100), count: 20) - let window = ColumnWindowResolver.resolve( - columnWidths: widths, viewportMinX: 0, viewportWidth: 5_000, current: nil - ) - #expect(window.range == 0..<20) - #expect(window.leadingWidth == 0) - #expect(window.trailingWidth == 0) - } - - @Test("Uneven column widths still preserve the document width") - func unevenWidths() { - let widths: [CGFloat] = (0..<200).map { CGFloat(40 + ($0 % 7) * 30) } - let expected = widths.reduce(0, +) - let window = ColumnWindowResolver.resolve( - columnWidths: widths, viewportMinX: 3_000, viewportWidth: 700, current: nil - ) - #expect(totalWidth(window, widths: widths) == expected) - } - - @Test("A zero-width viewport still mounts a usable window") - func zeroWidthViewport() { - let window = ColumnWindowResolver.resolve( - columnWidths: wide, viewportMinX: 0, viewportWidth: 0, current: nil - ) - #expect(!window.range.isEmpty) - #expect(totalWidth(window, widths: wide) == 50_000) - } -} diff --git a/TableProTests/Views/Results/DataGridCellAccessoryAppearanceTests.swift b/TableProTests/Views/Results/DataGridCellAccessoryAppearanceTests.swift index 8b61163d1..82707f11f 100644 --- a/TableProTests/Views/Results/DataGridCellAccessoryAppearanceTests.swift +++ b/TableProTests/Views/Results/DataGridCellAccessoryAppearanceTests.swift @@ -17,12 +17,23 @@ struct DataGridCellAccessoryAppearanceTests { let height: Double } - /// The cell text is already appearance-reactive, so a whole-cell comparison passes even with - /// the accessory frozen. The text stays empty and only the accessory rect is read. - private func makeCell(kind: DataGridCellKind, appearance: NSAppearance) -> DataGridCellView { - let cell = DataGridCellView(frame: NSRect(x: 0, y: 0, width: 120, height: 24)) + /// A cell is drawn rather than mounted, so this renders one through the renderer the grid uses + /// and reads the accessory rect back off the bitmap. The text stays empty so only the accessory + /// contributes ink. + private final class RenderedCellView: NSView { + var appearanceToDraw: DataGridCellAppearance? + private let renderer = DataGridCellRenderer() + override var isFlipped: Bool { true } + override func draw(_ dirtyRect: NSRect) { + guard let appearanceToDraw else { return } + renderer.draw(appearanceToDraw, in: bounds) + } + } + + private func makeCell(kind: DataGridCellKind, appearance: NSAppearance) -> NSView { + let cell = RenderedCellView(frame: NSRect(x: 0, y: 0, width: 120, height: 24)) cell.appearance = appearance - cell.configure( + cell.appearanceToDraw = DataGridCellAppearance.resolve( kind: kind, content: DataGridCellContent(displayText: "", rawValue: "42", placeholder: nil), state: DataGridCellState( @@ -33,7 +44,10 @@ struct DataGridCellAccessoryAppearanceTests { row: 0, columnIndex: 0 ), - palette: .placeholder + palette: .placeholder, + nullDisplayString: "NULL", + onEmphasizedSelection: false, + hasOverlay: false ) return cell } diff --git a/TableProTests/Views/Results/DataGridCellAppearanceTests.swift b/TableProTests/Views/Results/DataGridCellAppearanceTests.swift new file mode 100644 index 000000000..731f8fbc3 --- /dev/null +++ b/TableProTests/Views/Results/DataGridCellAppearanceTests.swift @@ -0,0 +1,182 @@ +// +// DataGridCellAppearanceTests.swift +// TableProTests +// +// The grid draws its cells rather than mounting one view per cell, so what a cell looks like is +// decided here rather than by a view configuring itself. These are the decisions: which font a +// placeholder takes, when a tint survives, what colour text turns on a selected row. +// + +import AppKit +import Testing + +@testable import TablePro + +@Suite("Data grid cell appearance") +@MainActor +struct DataGridCellAppearanceTests { + private let palette = DataGridCellPalette( + regularFont: .systemFont(ofSize: 13), + italicFont: .systemFont(ofSize: 13), + mediumFont: .systemFont(ofSize: 13, weight: .medium), + deletedRowText: .systemRed, + modifiedColumnTint: .systemYellow, + findMatchTint: .systemOrange + ) + + private func resolve( + kind: DataGridCellKind = .text, + text: String = "value", + rawValue: String? = "value", + placeholder: DataGridCellPlaceholder? = nil, + visualState: RowVisualState = .empty, + isFocused: Bool = false, + isEditable: Bool = true, + isLargeDataset: Bool = false, + isCurrentFindMatch: Bool = false, + columnIndex: Int = 0, + onEmphasizedSelection: Bool = false, + hasOverlay: Bool = false + ) -> DataGridCellAppearance { + DataGridCellAppearance.resolve( + kind: kind, + content: DataGridCellContent(displayText: text, rawValue: rawValue, placeholder: placeholder), + state: DataGridCellState( + visualState: visualState, + isFocused: isFocused, + isEditable: isEditable, + isLargeDataset: isLargeDataset, + isCurrentFindMatch: isCurrentFindMatch, + row: 0, + columnIndex: columnIndex + ), + palette: palette, + nullDisplayString: "NULL", + onEmphasizedSelection: onEmphasizedSelection, + hasOverlay: hasOverlay + ) + } + + @Test("An ordinary value draws in the regular font at the label colour") + func ordinaryValue() { + let appearance = resolve() + + #expect(appearance.text == "value") + #expect(appearance.font == palette.regularFont) + #expect(appearance.textColor == .labelColor) + #expect(appearance.backgroundTint == nil) + } + + @Test("NULL and empty draw in the italic font as secondary text") + func placeholdersAreItalic() { + let null = resolve(text: "", placeholder: .null) + let empty = resolve(text: "", placeholder: .empty) + + #expect(null.font == palette.italicFont) + #expect(null.textColor == .secondaryLabelColor) + #expect(null.text == "NULL") + #expect(empty.font == palette.italicFont) + } + + @Test("A server default draws in the medium font, tinted") + func defaultMarker() { + let appearance = resolve(text: "", placeholder: .defaultMarker) + + #expect(appearance.font == palette.mediumFont) + #expect(appearance.textColor == .systemBlue) + } + + /// A large result blanks its placeholders rather than formatting every one of them. + @Test("A large result draws no placeholder text") + func largeDatasetBlanksPlaceholders() { + #expect(resolve(text: "", placeholder: .null, isLargeDataset: true).text.isEmpty) + #expect(resolve(text: "", placeholder: .empty, isLargeDataset: true).text.isEmpty) + } + + @Test("A deleted row recolours its text and keeps no modified tint") + func deletedRow() { + let appearance = resolve(visualState: RowVisualState(isDeleted: true, isInserted: false, modifiedColumns: []), columnIndex: 0) + + #expect(appearance.textColor == palette.deletedRowText) + #expect(appearance.backgroundTint == nil) + } + + @Test("The find match keeps its highlight and turns its text black") + func findMatchWins() { + let appearance = resolve(isCurrentFindMatch: true, onEmphasizedSelection: true) + + #expect(appearance.backgroundTint == palette.findMatchTint) + #expect(appearance.textColor == .black) + } + + /// The selection paints the whole row, so a modified cell's tint would be painted over it. + @Test("A selected row drops the modified tint and takes the selection's text colour") + func selectionSuppressesTheModifiedTint() { + let unselected = resolve(visualState: RowVisualState(isDeleted: false, isInserted: false, modifiedColumns: [0]), columnIndex: 0) + let selected = resolve(visualState: RowVisualState(isDeleted: false, isInserted: false, modifiedColumns: [0]), columnIndex: 0, onEmphasizedSelection: true) + + #expect(unselected.backgroundTint == palette.modifiedColumnTint) + #expect(selected.backgroundTint == nil) + #expect(selected.textColor == .alternateSelectedControlTextColor) + } + + @Test("Only the modified column carries the tint") + func onlyTheModifiedColumnIsTinted() { + #expect(resolve(visualState: RowVisualState(isDeleted: false, isInserted: false, modifiedColumns: [2]), columnIndex: 2).backgroundTint != nil) + #expect(resolve(visualState: RowVisualState(isDeleted: false, isInserted: false, modifiedColumns: [2]), columnIndex: 3).backgroundTint == nil) + } + + // MARK: - The cell cursor + + /// A mounted cell got its ring from AppKit. A drawn cell has no view to hang one on, so the + /// appearance has to say which of the two shapes to draw. + @Test("The cell cursor draws a ring on an unselected row and a border on a selected one") + func cursorShape() { + let unselected = resolve(isFocused: true) + let selected = resolve(isFocused: true, onEmphasizedSelection: true) + + #expect(unselected.drawsFocusRing) + #expect(!unselected.drawsFocusBorder) + #expect(selected.drawsFocusBorder) + #expect(!selected.drawsFocusRing) + } + + @Test("An open editor hides the cell cursor behind it") + func overlayHidesTheCursor() { + let appearance = resolve(isFocused: true, hasOverlay: true) + + #expect(!appearance.drawsFocusRing) + #expect(!appearance.drawsFocusBorder) + } + + @Test("An unfocused cell draws no cursor") + func unfocusedDrawsNothing() { + let appearance = resolve() + + #expect(!appearance.drawsFocusRing) + #expect(!appearance.drawsFocusBorder) + } + + // MARK: - Accessories + + @Test("A foreign key with a value gets the arrow, an empty one gets nothing") + func foreignKeyAccessory() { + #expect(resolve(kind: .foreignKey, rawValue: "42").accessory == .foreignKey) + #expect(resolve(kind: .foreignKey, rawValue: "").accessory == .none) + #expect(resolve(kind: .foreignKey, rawValue: nil).accessory == .none) + } + + @Test("The accessory symbol follows the row's state") + func accessoryRoleFollowsState() { + #expect(resolve(kind: .foreignKey, rawValue: "42").accessoryRole == .foreignKeyNormal) + #expect( + resolve(kind: .foreignKey, rawValue: "42", onEmphasizedSelection: true).accessoryRole + == .foreignKeyEmphasized + ) + } + + @Test("A cell with no accessory resolves no symbol") + func noAccessoryNoRole() { + #expect(resolve().accessoryRole == nil) + } +} diff --git a/TableProTests/Views/Results/DataGridCellViewDoubleClickTests.swift b/TableProTests/Views/Results/DataGridCellViewDoubleClickTests.swift deleted file mode 100644 index e4c908973..000000000 --- a/TableProTests/Views/Results/DataGridCellViewDoubleClickTests.swift +++ /dev/null @@ -1,230 +0,0 @@ -// -// DataGridCellViewDoubleClickTests.swift -// TableProTests -// - -import AppKit -@testable import TablePro -import Testing - -@MainActor -private final class RecordingAccessoryDelegate: DataGridCellAccessoryDelegate { - var doubleClicks: [(row: Int, columnIndex: Int)] = [] - var chevronClicks: [(row: Int, columnIndex: Int)] = [] - var fkClicks: [(row: Int, columnIndex: Int, openInNewTab: Bool)] = [] - - func dataGridCellDidClickFKArrow(row: Int, columnIndex: Int, openInNewTab: Bool) { - fkClicks.append((row, columnIndex, openInNewTab)) - } - - func dataGridCellDidClickChevron(row: Int, columnIndex: Int) { - chevronClicks.append((row, columnIndex)) - } - - func dataGridCellDidDoubleClick(row: Int, columnIndex: Int) { - doubleClicks.append((row, columnIndex)) - } -} - -@Suite("DataGridCellView double-click") -@MainActor -struct DataGridCellViewDoubleClickTests { - private func makeCell(row: Int, columnIndex: Int, isEditable: Bool = true) -> DataGridCellView { - let cell = DataGridCellView(frame: NSRect(x: 0, y: 0, width: 120, height: 24)) - cell.configure( - kind: .json, - content: DataGridCellContent(displayText: "{}", rawValue: "{}", placeholder: nil), - state: DataGridCellState( - visualState: .empty, - isFocused: false, - isEditable: isEditable, - isLargeDataset: false, - row: row, - columnIndex: columnIndex - ), - palette: .placeholder - ) - return cell - } - - private func mouseDownEvent(clickCount: Int, location: NSPoint = .zero) throws -> NSEvent { - try #require(NSEvent.mouseEvent( - with: .leftMouseDown, - location: location, - modifierFlags: [], - timestamp: 0, - windowNumber: 0, - context: nil, - eventNumber: 0, - clickCount: clickCount, - pressure: 1 - )) - } - - @Test("Double-click reports the cell's row and column to the delegate") - func doubleClickReportsCellPosition() throws { - let cell = makeCell(row: 3, columnIndex: 2) - let delegate = RecordingAccessoryDelegate() - cell.accessoryDelegate = delegate - - cell.mouseDown(with: try mouseDownEvent(clickCount: 2)) - - #expect(delegate.doubleClicks.count == 1) - #expect(delegate.doubleClicks.first?.row == 3) - #expect(delegate.doubleClicks.first?.columnIndex == 2) - #expect(delegate.chevronClicks.isEmpty) - #expect(delegate.fkClicks.isEmpty) - } - - @Test("Single click does not report a double-click") - func singleClickDoesNotReportDoubleClick() throws { - let cell = makeCell(row: 1, columnIndex: 0) - let delegate = RecordingAccessoryDelegate() - cell.accessoryDelegate = delegate - - cell.mouseDown(with: try mouseDownEvent(clickCount: 1)) - - #expect(delegate.doubleClicks.isEmpty) - } - - @Test("Current accessory geometry is hittable before a draw pass") - func currentAccessoryIsHittableBeforeDraw() throws { - let cell = makeCell(row: 2, columnIndex: 1) - let delegate = RecordingAccessoryDelegate() - cell.accessoryDelegate = delegate - - cell.mouseDown(with: try mouseDownEvent(clickCount: 1, location: NSPoint(x: 110, y: 12))) - - #expect(delegate.chevronClicks.count == 1) - } - - @Test("Reusing a cell cannot retain a stale accessory hit target") - func reuseCannotRetainStaleAccessoryHitTarget() throws { - let cell = makeCell(row: 2, columnIndex: 1) - let delegate = RecordingAccessoryDelegate() - cell.accessoryDelegate = delegate - let representation = try #require(cell.bitmapImageRepForCachingDisplay(in: cell.bounds)) - cell.cacheDisplay(in: cell.bounds, to: representation) - - cell.configure( - kind: .json, - content: DataGridCellContent(displayText: "{}", rawValue: "{}", placeholder: nil), - state: DataGridCellState( - visualState: .empty, - isFocused: false, - isEditable: false, - isLargeDataset: false, - row: 2, - columnIndex: 1 - ), - palette: .placeholder - ) - cell.mouseDown(with: try mouseDownEvent(clickCount: 1, location: NSPoint(x: 110, y: 12))) - - #expect(delegate.chevronClicks.isEmpty) - #expect(delegate.fkClicks.isEmpty) - } - - @Test("Accessibility exposes the formatted cell value") - func accessibilityUsesDisplayText() { - let cell = DataGridCellView(frame: NSRect(x: 0, y: 0, width: 300, height: 24)) - let uuid = "af49453b-7f2f-fb58-fcd3-2bd399599fa5" - cell.configure( - kind: .blob, - content: DataGridCellContent(displayText: uuid, rawValue: nil, placeholder: nil), - state: DataGridCellState( - visualState: .empty, - isFocused: false, - isEditable: false, - isLargeDataset: false, - row: 1, - columnIndex: 2 - ), - palette: .placeholder - ) - - #expect(cell.accessibilityValue() as? String == uuid) - #expect(cell.accessibilityLabel()?.contains(uuid) == true) - } -} - -@Suite("DataGridCell accessory layout") -@MainActor -struct DataGridCellAccessoryLayoutTests { - @Test("Measurement and drawing share one exact geometry contract") - func sharedGeometryContract() { - let bounds = NSRect(x: 0, y: 0, width: 120, height: 24) - func expandedBounds(for accessory: DataGridCellAccessory) -> NSRect { - NSRect( - x: bounds.minX, - y: bounds.minY, - width: bounds.width + accessory.columnWidthReservation, - height: bounds.height - ) - } - - #expect(DataGridCellAccessory.none.measurementPadding == 16) - #expect(DataGridCellAccessory.chevron.measurementPadding == 32) - #expect(DataGridCellAccessory.foreignKey.measurementPadding == 36) - #expect(DataGridCellAccessory.none.availableTextWidth(in: bounds) == 112) - #expect(DataGridCellAccessory.chevron.availableTextWidth(in: bounds) == 96) - #expect(DataGridCellAccessory.foreignKey.availableTextWidth(in: bounds) == 92) - #expect( - DataGridCellAccessory.chevron.availableTextWidth( - in: expandedBounds(for: .chevron) - ) == DataGridCellAccessory.none.availableTextWidth(in: bounds) - ) - #expect( - DataGridCellAccessory.foreignKey.availableTextWidth( - in: expandedBounds(for: .foreignKey) - ) == DataGridCellAccessory.none.availableTextWidth(in: bounds) - ) - #expect(DataGridCellAccessory.chevron.frame(in: bounds) == NSRect(x: 104, y: 5, width: 12, height: 14)) - #expect(DataGridCellAccessory.foreignKey.frame(in: bounds) == NSRect(x: 100, y: 4, width: 16, height: 16)) - } - - @Test("Column presentation preserves dropdown, foreign-key, and editability precedence") - func columnPresentationPrecedence() { - let explicitDropdown = DataGridColumnPresentation.resolve( - columnType: .text(rawType: "TEXT"), - isForeignKey: true, - isDropdown: true, - isTypePicker: false, - isEnumOrSet: false, - isEditable: true - ) - let enumForeignKey = DataGridColumnPresentation.resolve( - columnType: .text(rawType: "TEXT"), - isForeignKey: true, - isDropdown: false, - isTypePicker: false, - isEnumOrSet: true, - isEditable: true - ) - let readOnlyDate = DataGridColumnPresentation.resolve( - columnType: .date(rawType: "DATE"), - isForeignKey: false, - isDropdown: false, - isTypePicker: false, - isEnumOrSet: false, - isEditable: false - ) - let readOnlyForeignKey = DataGridColumnPresentation.resolve( - columnType: .text(rawType: "TEXT"), - isForeignKey: true, - isDropdown: false, - isTypePicker: false, - isEnumOrSet: false, - isEditable: false - ) - - #expect(explicitDropdown.kind == .dropdown) - #expect(explicitDropdown.accessory == .chevron) - #expect(enumForeignKey.kind == .foreignKey) - #expect(enumForeignKey.accessory == .foreignKey) - #expect(readOnlyDate.kind == .date) - #expect(readOnlyDate.accessory == .none) - #expect(readOnlyForeignKey.kind == .foreignKey) - #expect(readOnlyForeignKey.accessory == .foreignKey) - } -} diff --git a/TableProTests/Views/Results/DataGridColumnPoolTests.swift b/TableProTests/Views/Results/DataGridColumnPoolTests.swift index 3e1a9ab8c..5424ff74c 100644 --- a/TableProTests/Views/Results/DataGridColumnPoolTests.swift +++ b/TableProTests/Views/Results/DataGridColumnPoolTests.swift @@ -36,10 +36,7 @@ struct DataGridColumnPoolTests { } private func dataColumns(in tableView: NSTableView) -> [NSTableColumn] { - tableView.tableColumns.filter { - $0.identifier != ColumnIdentitySchema.rowNumberIdentifier - && !ColumnIdentitySchema.isSpacer($0.identifier) - } + tableView.tableColumns.filter { $0.identifier != ColumnIdentitySchema.rowNumberIdentifier } } @Test("reconcile grows pool when column count exceeds capacity") @@ -649,15 +646,6 @@ struct DataGridColumnPoolTests { // MARK: - Column windowing (#1219) - private func makeScrolledTableView(viewportWidth: CGFloat) -> (NSScrollView, NSTableView) { - let tableView = makeTableView() - let scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: viewportWidth, height: 600)) - scrollView.documentView = tableView - scrollView.hasHorizontalScroller = true - scrollView.layoutSubtreeIfNeeded() - return (scrollView, tableView) - } - private func reconcileWide( _ pool: DataGridColumnPool, tableView: NSTableView, @@ -676,112 +664,10 @@ struct DataGridColumnPoolTests { ) } - private func spacerWidth(in tableView: NSTableView) -> CGFloat { - tableView.tableColumns - .filter { ColumnIdentitySchema.isSpacer($0.identifier) } - .reduce(0) { $0 + $1.width } - } - - /// NSTableView builds a cell view per non-hidden column for every prepared row, so the whole - /// point is that a wide result leaves most columns unmounted. - @Test("A wide result mounts far fewer columns than it has") - func wideResultMountsAWindow() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 500) - - let mounted = dataColumns(in: tableView).filter { !$0.isHidden } - #expect(mounted.count < 60) - #expect(!mounted.isEmpty) - } - - /// The spacers exist so the horizontal scroller still spans the whole result. Lose this and the - /// user cannot reach the columns the window left out. - @Test("The spacers restore the width the window left out") - func spacersPreserveDocumentWidth() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 500) - - // A visible column occupies its width plus one intercell gap; a hidden one occupies - // nothing. The spacers stand in for the unmounted columns, gaps included, so the two - // layouts have to come to the same total. - let gap = tableView.intercellSpacing.width - let columns = dataColumns(in: tableView) - let everyColumnSlot = columns.reduce(0) { $0 + $1.width + gap } - let occupied = tableView.tableColumns - .filter { !$0.isHidden && $0.identifier != ColumnIdentitySchema.rowNumberIdentifier } - .reduce(0) { $0 + $1.width + gap } - - #expect(columns.count == 500) - #expect(spacerWidth(in: tableView) > 0) - #expect(columns.filter { !$0.isHidden }.count < columns.count) - #expect(occupied == everyColumnSlot) - } - - @Test("A narrow result mounts every column and needs no spacer") - func narrowResultMountsEverything() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 4) - - let allMounted = dataColumns(in: tableView).allSatisfy { !$0.isHidden } - #expect(allMounted) - #expect(spacerWidth(in: tableView) == 0) - } - - /// A window slide must never bring back a column the user hid, which is the one way windowing - /// could corrupt the visible column set. - @Test("Windowing never un-hides a column the user hid") - func windowNeverUnhidesUserHiddenColumn() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 500, hidden: ["c0", "c1", "c2"]) - scrollView.contentView.scroll(to: NSPoint(x: 0, y: 0)) - pool.applyColumnWindow(in: tableView) - - let hiddenByUser = dataColumns(in: tableView).prefix(3) - let allStillHidden = hiddenByUser.allSatisfy { $0.isHidden } - #expect(allStillHidden) - } - - @Test("A table with no laid-out viewport mounts everything rather than hiding it all") - func noViewportMountsEverything() { - let pool = DataGridColumnPool() - let tableView = makeTableView() - - reconcileWide(pool, tableView: tableView, count: 120) - - let allMounted = dataColumns(in: tableView).allSatisfy { !$0.isHidden } - #expect(allMounted) - } - - /// Copy, Find, cell navigation and Size All Columns to Fit all ask "which columns is the user - /// looking at". Answering that with isHidden narrows them to the mounted window, which silently - /// drops the off-screen columns from a copied row and makes Find miss them entirely. - @Test("Every column the result shows is presented, even when the window unmounts it") - func unmountedColumnsAreStillPresented() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 500) - - let columns = dataColumns(in: tableView) - let presented = columns.filter { pool.presentsColumn($0) } - let mounted = columns.filter { !$0.isHidden } - - #expect(presented.count == 500) - #expect(mounted.count < presented.count) - } - @Test("A column the user hid is not presented") func userHiddenColumnIsNotPresented() { let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + let tableView = makeTableView() reconcileWide(pool, tableView: tableView, count: 20, hidden: ["c3"]) @@ -793,7 +679,7 @@ struct DataGridColumnPoolTests { @Test("A surplus slot from a wider result is not presented") func surplusSlotIsNotPresented() { let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + let tableView = makeTableView() reconcileWide(pool, tableView: tableView, count: 40) reconcileWide(pool, tableView: tableView, count: 5) @@ -802,287 +688,14 @@ struct DataGridColumnPoolTests { #expect(presented.count == 5) } - @Test("Detaching removes the spacers along with the pooled columns") - func detachRemovesSpacers() { + @Test("Detaching removes the pooled columns") + func detachRemovesPooledColumns() { let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) + let tableView = makeTableView() reconcileWide(pool, tableView: tableView, count: 60) pool.detachFromTableView() - let hasSpacer = tableView.tableColumns.contains(where: { ColumnIdentitySchema.isSpacer($0.identifier) }) - #expect(!hasSpacer) - } - - // MARK: - Window geometry while scrolling - - private func scroll(_ scrollView: NSScrollView, to offsetX: CGFloat, tableView: NSTableView) { - scrollView.contentView.scroll(to: NSPoint(x: offsetX, y: scrollView.contentView.bounds.origin.y)) - scrollView.reflectScrolledClipView(scrollView.contentView) - tableView.layoutSubtreeIfNeeded() - } - - private func documentWidth(of tableView: NSTableView) -> CGFloat { - tableView.layoutSubtreeIfNeeded() - return tableView.frame.width - } - - /// How much of the viewport, past the row-number column, no mounted data column paints into. - /// - /// Measured through `rect(ofColumn:)`, which is `NSTableView`'s own answer for where a column - /// sits and is `NSZeroRect` for an unmounted one. Asking the resolver instead would only prove - /// the resolver agrees with itself, which is exactly how #2381 shipped. - private func unpaintedViewportWidth(in scrollView: NSScrollView, tableView: NSTableView) -> CGFloat { - let viewport = scrollView.contentView.bounds - let rowNumber = tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) - let contentStart = rowNumber >= 0 - ? max(viewport.minX, tableView.rect(ofColumn: rowNumber).maxX) - : viewport.minX - - var painted: CGFloat = 0 - for column in dataColumns(in: tableView) where !column.isHidden { - let index = tableView.column(withIdentifier: column.identifier) - guard index >= 0 else { continue } - let rect = tableView.rect(ofColumn: index) - painted += max(0, min(rect.maxX, viewport.maxX) - max(rect.minX, contentStart)) - } - return max(0, (viewport.maxX - contentStart) - painted) - } - - private func mountedIdentifiers(in tableView: NSTableView) -> Set { - Set(dataColumns(in: tableView).filter { !$0.isHidden }.map(\.identifier)) - } - - private func scrollOffsets(in scrollView: NSScrollView, tableView: NSTableView) -> [CGFloat] { - let maximum = documentWidth(of: tableView) - scrollView.contentView.bounds.width - guard maximum > 0 else { return [0] } - let forward = Array(stride(from: 0, through: maximum, by: 150)) + [maximum] - return forward + forward.reversed() - } - - /// The reported bug. The window is resolved against a model of the whole column run, so the - /// viewport has to be rebased into that model before it can pick a range. Counting the leading - /// spacer as chrome subtracted the columns it stands in for a second time, and the window - /// walked left while the reader scrolled right until it painted nothing at all. - @Test("The mounted columns cover the viewport at every horizontal scroll offset") - func windowCoversTheViewportWhileScrolling() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - var worstGap: CGFloat = 0 - for offset in scrollOffsets(in: scrollView, tableView: tableView) { - scroll(scrollView, to: offset, tableView: tableView) - pool.applyColumnWindow(in: tableView) - tableView.layoutSubtreeIfNeeded() - worstGap = max(worstGap, unpaintedViewportWidth(in: scrollView, tableView: tableView)) - } - - #expect(worstGap == 0) - } - - /// A window that alternates between two ranges re-mounts columns on every scroll event, which - /// is what the reader sees as flicker. - @Test("Resolving again at the same scroll offset settles rather than alternating") - func windowSettlesAtOneOffset() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - scroll(scrollView, to: documentWidth(of: tableView) / 2, tableView: tableView) - pool.applyColumnWindow(in: tableView) - tableView.layoutSubtreeIfNeeded() - let settled = mountedIdentifiers(in: tableView) - - for _ in 0..<5 { - pool.applyColumnWindow(in: tableView) - tableView.layoutSubtreeIfNeeded() - } - - #expect(mountedIdentifiers(in: tableView) == settled) - } - - /// The spacers exist to keep the scroll extent, so no window position may change it. - @Test("The document keeps its width at every window position") - func documentWidthSurvivesEveryWindowPosition() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - let expected = documentWidth(of: tableView) - - for offset in scrollOffsets(in: scrollView, tableView: tableView) { - scroll(scrollView, to: offset, tableView: tableView) - pool.applyColumnWindow(in: tableView) - #expect(documentWidth(of: tableView) == expected) - } - } - - @Test("Scrolled to the end, the last column is mounted") - func lastColumnIsMountedAtTheEnd() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - scroll( - scrollView, - to: documentWidth(of: tableView) - scrollView.contentView.bounds.width, - tableView: tableView - ) - pool.applyColumnWindow(in: tableView) - - let last = try #require(dataColumns(in: tableView).last) - #expect(!last.isHidden) - } - - // MARK: - Reaching a column the window left out - - /// `rect(ofColumn:)` and `frameOfCell(atColumn:row:)` are both empty for an unmounted column, - /// so Find scrolled to the document origin instead of the match and the inline editor opened - /// nothing at all. - @Test("A column the window left out can be mounted on demand") - func mountColumnReachesAnUnmountedColumn() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - - let last = try #require(dataColumns(in: tableView).last) - #expect(last.isHidden) - - pool.mountColumn(last, in: tableView) - tableView.layoutSubtreeIfNeeded() - - #expect(!last.isHidden) - #expect(tableView.rect(ofColumn: tableView.column(withIdentifier: last.identifier)).width > 0) - } - - /// Stretching the window out to reach a far column mounts every column in between, which is the - /// cost the window exists to avoid: measured at 848ms and 3,081 cell views for one match 90 - /// columns away, and 4.8s at 500 columns. - @Test("Mounting a far column does not mount everything in between") - func mountColumnStaysBounded() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - let mountedBefore = mountedIdentifiers(in: tableView).count - - let last = try #require(dataColumns(in: tableView).last) - pool.mountColumn(last, in: tableView) - tableView.layoutSubtreeIfNeeded() - - #expect(!last.isHidden) - #expect(mountedIdentifiers(in: tableView).count <= mountedBefore) - } - - @Test("Mounting a far column keeps the document width") - func mountColumnKeepsTheDocumentWidth() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - let expected = documentWidth(of: tableView) - - let last = try #require(dataColumns(in: tableView).last) - pool.mountColumn(last, in: tableView) - - #expect(documentWidth(of: tableView) == expected) - } - - @Test("A column the user hid is never mounted on demand") - func mountColumnRefusesAUserHiddenColumn() throws { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100, hidden: ["c99"]) - - let hidden = try #require(dataColumns(in: tableView).last) - pool.mountColumn(hidden, in: tableView) - - #expect(hidden.isHidden) - } - - // MARK: - Naming the ends of the data run - - /// The window's spacers are attached columns too, and the leading one sits immediately before - /// the first data column, so a fixed position names a spacer rather than data. - @Test("The first and last presented columns are data columns, not spacers") - func presentedEndsSkipTheSpacers() throws { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - let first = try #require(pool.firstPresentedColumnIndex(in: tableView)) - let last = try #require(pool.lastPresentedColumnIndex(in: tableView)) - - #expect(!ColumnIdentitySchema.isSpacer(tableView.tableColumns[first].identifier)) - #expect(!ColumnIdentitySchema.isSpacer(tableView.tableColumns[last].identifier)) - #expect(tableView.tableColumns[first].identifier == dataColumns(in: tableView).first?.identifier) - #expect(tableView.tableColumns[last].identifier == dataColumns(in: tableView).last?.identifier) - } - - @Test("Walking forward and back from an end stays inside the data run") - func presentedNeighboursStayInsideTheDataRun() throws { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 20) - - let first = try #require(pool.firstPresentedColumnIndex(in: tableView)) - let last = try #require(pool.lastPresentedColumnIndex(in: tableView)) - - #expect(pool.previousPresentedColumnIndex(before: first, in: tableView) == nil) - #expect(pool.nextPresentedColumnIndex(after: last, in: tableView) == nil) - #expect(pool.nextPresentedColumnIndex(after: first, in: tableView) != nil) - #expect(pool.previousPresentedColumnIndex(before: last, in: tableView) != nil) - } - - /// The create-table grid opens with no columns at all, where every attached column is chrome. - @Test("A result with no columns presents no column at either end") - func emptyResultHasNoPresentedEnds() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 0) - - #expect(pool.firstPresentedColumnIndex(in: tableView) == nil) - #expect(pool.lastPresentedColumnIndex(in: tableView) == nil) - } - - @Test("A single-column result presents that column at both ends") - func singleColumnResultHasOneEnd() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 1) - - #expect(pool.firstPresentedColumnIndex(in: tableView) == pool.lastPresentedColumnIndex(in: tableView)) - #expect(pool.firstPresentedColumnIndex(in: tableView) != nil) - } - - /// Size All Columns to Fit reaches the columns the window unmounted as well, so the spacers - /// stand in at the width those columns used to have and the document ends up short. - @Test("Resizing unmounted columns restores the full document width") - func widthChangeOutsideTheWindowRestoresDocumentWidth() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - - for column in dataColumns(in: tableView) { - column.width = 300 - } - pool.invalidateColumnWindow() - pool.applyColumnWindow(in: tableView) - - let gap = tableView.intercellSpacing.width - let everyColumnSlot = dataColumns(in: tableView).reduce(0) { $0 + $1.width + gap } - let occupied = tableView.tableColumns - .filter { !$0.isHidden && $0.identifier != ColumnIdentitySchema.rowNumberIdentifier } - .reduce(0) { $0 + $1.width + gap } - - #expect(occupied == everyColumnSlot) + #expect(dataColumns(in: tableView).isEmpty) } } diff --git a/TableProTests/Views/Results/FocusedColumnResolutionTests.swift b/TableProTests/Views/Results/FocusedColumnResolutionTests.swift index 73edc7b86..de0bdd4dc 100644 --- a/TableProTests/Views/Results/FocusedColumnResolutionTests.swift +++ b/TableProTests/Views/Results/FocusedColumnResolutionTests.swift @@ -18,9 +18,9 @@ private final class FocusedColumnLayoutPersister: ColumnLayoutPersisting { } /// `focusedColumn` is a position in `tableView.tableColumns`, which carries the row-number column -/// and a hidden spacer ahead of the data and which the reader can reorder. Preview FK Reference -/// used to turn it into a data index by subtracting 1, so the menu command previewed the wrong -/// column or silently nothing while the key-equivalent path on the same cell worked. +/// ahead of the data and which the reader can reorder. Preview FK Reference used to turn it into a +/// data index by subtracting 1, so the menu command previewed the wrong column or silently nothing +/// while the key-equivalent path on the same cell worked. @Suite("Focused column resolution") @MainActor struct FocusedColumnResolutionTests { @@ -90,10 +90,10 @@ struct FocusedColumnResolutionTests { } /// Moving the selection with the keyboard leaves no cell cursor behind, so the grid seeds one - /// from the selection change. Seeding it with a fixed position lands on the window's leading - /// spacer, and every command that reads the cursor then resolves it to no column at all: Return - /// opens no editor while the menu item still validates as enabled (#2381). - @Test("A selection with no cell cursor seeds one on a data column, not a spacer") + /// from the selection change. Seeding it with a fixed position landed on chrome, and every + /// command that reads the cursor then resolved it to no column at all: Return opened no editor + /// while the menu item still validated as enabled (#2381). + @Test("A selection with no cell cursor seeds one on a data column, not chrome") func selectionSeedsTheCursorOnADataColumn() throws { let coordinator = makeCoordinator(columns: ["id", "name"]) let tableView = try #require(coordinator.tableView as? KeyHandlingTableView) @@ -116,8 +116,8 @@ struct FocusedColumnResolutionTests { } /// The whole keystroke, not just the seed: with no cell cursor, a selection change seeds one and - /// Return has to open the editor on it. Seeded onto a spacer, every step past the seed resolved - /// to no column and the keystroke was swallowed (#2381). + /// Return has to open the editor on it. Seeded onto chrome, every step past the seed resolved to + /// no column and the keystroke was swallowed (#2381). @Test("Return opens the editor on the column a keyboard selection seeded") func returnOpensTheEditorOnTheSeededColumn() throws { let coordinator = makeCoordinator(columns: ["id", "name"]) @@ -142,15 +142,19 @@ struct FocusedColumnResolutionTests { #expect(coordinator.overlayEditor != nil) } - @Test("A data column does not sit one place after its data index") - func dataColumnsAreNotOffsetByOne() throws { + /// Resolution goes through the column's own identifier. Subtracting a fixed offset from the + /// position happens to agree while the row-number column is the only chrome, and stops agreeing + /// the moment the reader reorders a column or the grid grows another chrome column. + @Test("A data column resolves by identity, not by its distance from the start") + func dataColumnsResolveByIdentity() throws { let grid = makeGrid(columns: ["id", "name", "customer_id"]) let tableColumn = try #require(tableColumnIndex(of: "customer_id", in: grid)) - let resolved = DataGridView.dataColumnIndex(for: tableColumn, in: grid.tableView, schema: grid.schema) + #expect(DataGridView.dataColumnIndex(for: tableColumn, in: grid.tableView, schema: grid.schema) == 2) - #expect(resolved == 2) - #expect(tableColumn - 1 != resolved, "the subtract-one mapping is what shipped broken") + grid.tableView.moveColumn(tableColumn, toColumn: 1) + let moved = try #require(tableColumnIndex(of: "customer_id", in: grid)) + #expect(DataGridView.dataColumnIndex(for: moved, in: grid.tableView, schema: grid.schema) == 2) } @Test("Every data column resolves back to its own index") @@ -177,9 +181,9 @@ struct FocusedColumnResolutionTests { #expect(DataGridView.dataColumnIndex(for: after, in: grid.tableView, schema: grid.schema) == 2) } - /// The row-number column and the window's two spacers all sit in `tableColumns`, and one spacer - /// sits ahead of the first data column, so a fixed position names chrome rather than data. - @Test("Neither the row-number column nor the window's spacers are data columns") + /// The row-number column sits in `tableColumns` too, so a fixed position names chrome rather + /// than data. + @Test("The row-number column is not a data column") func chromeColumnsAreNotData() { let grid = makeGrid(columns: ["id", "name"]) @@ -187,7 +191,6 @@ struct FocusedColumnResolutionTests { DataGridView.dataColumnIndex(for: $0, in: grid.tableView, schema: grid.schema) == nil } - #expect(chrome.contains(0)) - #expect(chrome.count == 3) + #expect(chrome == [0]) } } From c2454b8a54bf2730189527d494f942dfe176f471 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 23 Aug 2026 03:37:43 +0700 Subject: [PATCH 5/7] perf(datagrid): draw the column separators instead of keeping one view per column (#2381) --- CHANGELOG.md | 3 + CLAUDE.md | 2 + TablePro/Views/Results/CellOverlayBase.swift | 23 ++- .../Views/Results/CellOverlayEditor.swift | 8 +- .../Views/Results/CellOverlayViewer.swift | 8 +- .../Views/Results/DataGridBodyChrome.swift | 73 +++++++++ .../Views/Results/DataGridCoordinator.swift | 43 +++-- TablePro/Views/Results/DataGridRowView.swift | 18 +++ .../Results/DataGridView+RowActions.swift | 4 +- TablePro/Views/Results/DataGridView.swift | 2 +- .../Extensions/DataGridView+CellCommit.swift | 8 +- .../Extensions/DataGridView+Click.swift | 17 +- .../Extensions/DataGridView+Popovers.swift | 22 +-- .../Views/Results/KeyHandlingTableView.swift | 20 +++ .../Results/CellOverlayTextLayoutTests.swift | 65 ++++++++ .../Results/DataGridBodyChromeTests.swift | 152 ++++++++++++++++++ .../Results/DrawnCellReachabilityTests.swift | 131 +++++++++++++++ 17 files changed, 537 insertions(+), 62 deletions(-) create mode 100644 TablePro/Views/Results/DataGridBodyChrome.swift create mode 100644 TableProTests/Views/Results/CellOverlayTextLayoutTests.swift create mode 100644 TableProTests/Views/Results/DataGridBodyChromeTests.swift create mode 100644 TableProTests/Views/Results/DrawnCellReachabilityTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ac40ef51..e96868c4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - The data grid draws its cells instead of building a view for each one, so a result with hundreds of columns opens at once and holds a fraction of the memory. (#2381) +- The data grid draws its own column separators. (#2381) +- The inline cell editor scrolls a long line instead of wrapping it. (#2381) ### Fixed +- A second of delay opening the inline editor on a result with hundreds of columns. (#2381) - Flickering columns, blank columns, and an unpainted gap while scrolling a result with about 100 columns sideways. (#2381) - Find, arrow keys, and the inline editor unable to reach a column scrolled off the side of a wide result. - Return opening no editor on a row selected with the arrow keys. diff --git a/CLAUDE.md b/CLAUDE.md index d0739f59c..4005ff2fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -208,6 +208,8 @@ To ship one: add the record type or field in CloudKit Console (or `xcrun cktool **The data grid header owns all of its own chrome, so nothing may ask AppKit to paint any of it**: `NSTableHeaderCell` and `NSTableHeaderView` both paint a fixed 28pt band that they centre vertically in whatever frame they are given, a 16pt column divider on `midY` and a 1pt rule at `midY + 13`. The data grid grows its header to 42pt for a column comment, so that band lands mid-cell: the rule crosses the comment's descenders and sits 8pt above the real bottom edge. `SortableHeaderChrome` is therefore the single owner of header geometry and colours, `SortableHeaderCell.draw(withFrame:in:)` never calls `super`, and `SortableHeaderView.draw(_:)` fills the background and rules the bottom edge itself. The trap is that the header view paints a second copy of that same band for `NSTableView.highlightedTableColumn`, driven by *state* rather than by a drawing call, so no cell override can reach it: setting it gives the sorted column a stray divider and a rule no other column has. TablePro already draws the sorted-column affordance itself (bold title, chevron, priority number, with `drawSortIndicator` overridden to nothing), so `highlightedTableColumn` is a redundant second channel and must stay unset. All sorted-column presentation goes through `SortableHeaderView.applySortState(_:schema:)`, which publishes the order natively through `tableView.sortDescriptors` (for accessibility; it paints nothing) and updates the cells. `SortableHeaderRenderingTests` rasterises the header and guards this. This shipped as a rule through the comment line and a stray divider on the sorted column (#2017). +**The data grid body owns its column separators too, because AppKit's do not scale**: `NSTableView` draws a vertical grid line by keeping one separator view per column as its own subview, and every layout pass re-sorts that whole subview list with an `-[NSArray containsObject:]` comparator inside the sort. That is O(columns) views and O(columns squared) work per pass. Measured on a 500-column result: one layout pass costs 518ms with `.solidVerticalGridLineMask` and 0.03ms with the mask cleared, and 535 of the table view's 536 subviews are separators. Adding or removing any subview of the table view forces such a pass, and the inline cell editor does exactly that on the way in and on the way out, so opening an editor cost about a second (#2381). Only the vertical mask behaves this way: `.solidHorizontalGridLineMask` adds no subviews at all, because AppKit draws a horizontal separator inside `NSTableRowView.drawSeparator(in:)`. So `gridStyleMask` stays empty and `DataGridBodyChrome` is the single owner of separator geometry, thickness and colour, exactly as `SortableHeaderChrome` is for the header. Where it is called from is not a free choice: a row view covers whatever the table view drew beneath it, measured, so `NSTableView.drawGrid(inClipRect:)` and `drawBackground(inClipRect:)` are both invisible behind the rows and visible only past the last row. A row therefore paints the separators crossing it, in a second pass after its cells so a modified or find-match tint cannot paint over one, and `KeyHandlingTableView.drawBackground(inClipRect:)` paints only the area below the last row. The separator stands at each presented column's *leading* edge, which is where AppKit put it and which keeps the row-number column's boundary; take the boundary from `presentsColumn` and `rect(ofColumn:)`, never from a fixed step, per the column rule above. + **The data grid's column window measures the viewport against the whole column run, so its rebase counts chrome only**: `NSTableColumn.isHidden` costs O(attached columns) per write, because AppKit walks every row view and re-sorts its subviews to rebuild the key view loop. Hiding the columns outside a viewport is therefore quadratic in the column count: measured at 233ms for 100 columns, 8.1s for 500 and 34s for 1000, with the relayout debris of those writes never reclaimed, which is where a 500-column table's 837MB went. No AppKit knob helps: `autorecalculatesKeyViewLoop = false`, `beginUpdates`/`endUpdates` and hiding before any row exists were all measured and none of them changed it. The grid therefore keeps every column attached and visible, and pays nothing for the ones off screen by not building a view for them: `tableView(_:viewFor:row:)` returns nil for every data column and `DataGridRowView` draws the cells the viewport touches with CoreText. Measured on a 500-column table: opening it went from 12.4s to 26ms and from 837MB to 3.9MB, and the whole table holds 26 views rather than 12,500. Never reintroduce a column window built on `isHidden`. **No fixed position in `tableColumns` names a data column**: the attached order is `[__rowNumber__, __leadingSpacer__, data columns, surplus pool slots, __trailingSpacer__]`, so `presentsColumn` is the question to ask, with `firstPresentedColumnIndex` and its neighbours beside it on `DataGridColumnPool`. `DataGridView.firstDataTableColumnIndex` was a hardcoded `1` that the leading spacer took over when windowing landed, and `isDataTableColumn` accepted the trailing spacer at the other end. The cell cursor was seeded onto a spacer whenever the selection moved without a click, so Down then Return did nothing on any table while the Edit menu item still validated as enabled, Tab out of a row's last cell and Shift+Tab out of its first were swallowed, and `scrollColumnToVisible` on a column the window had unmounted scrolled to the document origin instead of the column (#2381). diff --git a/TablePro/Views/Results/CellOverlayBase.swift b/TablePro/Views/Results/CellOverlayBase.swift index 5b92dead6..eba2e3e8e 100644 --- a/TablePro/Views/Results/CellOverlayBase.swift +++ b/TablePro/Views/Results/CellOverlayBase.swift @@ -108,11 +108,32 @@ class CellOverlayBase: NSObject { return container } + /// Lays a text view out the way an inline cell overlay needs. + /// + /// A cell holds one value, so the overlay behaves like a field editor and scrolls a long line + /// rather than wrapping it. Wrapping made TextKit 2 lay the whole value out before the overlay + /// could appear: measured at 206ms for a 256KB value and 816ms for 1MB, against 7ms unwrapped, + /// and the wrapped result was thousands of visual lines in a box 120pt tall (#2381). + static func applyCellTextLayout(to textView: NSTextView) { + let unbounded = NSSize( + width: CGFloat.greatestFiniteMagnitude, + height: CGFloat.greatestFiniteMagnitude + ) + textView.isVerticallyResizable = true + textView.isHorizontallyResizable = true + // A text view grows only as far as `maxSize`, which `init(frame:)` leaves at the frame, so + // without this the long line is clipped at the cell's width rather than scrolled: measured + // at a 140pt document for a 64,000-character value, against 344,166pt with it raised. + textView.maxSize = unbounded + textView.textContainer?.widthTracksTextView = false + textView.textContainer?.containerSize = unbounded + } + static func makeScrollView(in container: NSView) -> NSScrollView { let scrollView = NSScrollView(frame: container.bounds) scrollView.autoresizingMask = [.width, .height] scrollView.hasVerticalScroller = true - scrollView.hasHorizontalScroller = false + scrollView.hasHorizontalScroller = true scrollView.autohidesScrollers = true scrollView.borderType = .noBorder scrollView.drawsBackground = true diff --git a/TablePro/Views/Results/CellOverlayEditor.swift b/TablePro/Views/Results/CellOverlayEditor.swift index c195a4cca..7b3e7e3b8 100644 --- a/TablePro/Views/Results/CellOverlayEditor.swift +++ b/TablePro/Views/Results/CellOverlayEditor.swift @@ -39,13 +39,7 @@ final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate { textView.textColor = .labelColor textView.backgroundColor = .textBackgroundColor textView.focusRingType = .none - textView.isVerticallyResizable = true - textView.isHorizontallyResizable = false - textView.textContainer?.widthTracksTextView = true - textView.textContainer?.containerSize = NSSize( - width: scrollView.bounds.width, - height: CGFloat.greatestFiniteMagnitude - ) + Self.applyCellTextLayout(to: textView) textView.delegate = self textView.string = value textView.selectAll(nil) diff --git a/TablePro/Views/Results/CellOverlayViewer.swift b/TablePro/Views/Results/CellOverlayViewer.swift index 05f9c26a0..4d83d6537 100644 --- a/TablePro/Views/Results/CellOverlayViewer.swift +++ b/TablePro/Views/Results/CellOverlayViewer.swift @@ -31,13 +31,7 @@ final class CellOverlayViewer: CellOverlayBase, NSTextViewDelegate { textView.font = ThemeEngine.shared.dataGridFonts.regular textView.textColor = .labelColor textView.backgroundColor = .textBackgroundColor - textView.isVerticallyResizable = true - textView.isHorizontallyResizable = false - textView.textContainer?.widthTracksTextView = true - textView.textContainer?.containerSize = NSSize( - width: scrollView.bounds.width, - height: CGFloat.greatestFiniteMagnitude - ) + Self.applyCellTextLayout(to: textView) textView.delegate = self textView.string = value textView.selectAll(nil) diff --git a/TablePro/Views/Results/DataGridBodyChrome.swift b/TablePro/Views/Results/DataGridBodyChrome.swift new file mode 100644 index 000000000..60dc1b09c --- /dev/null +++ b/TablePro/Views/Results/DataGridBodyChrome.swift @@ -0,0 +1,73 @@ +// +// DataGridBodyChrome.swift +// TablePro +// + +import AppKit + +/// The single owner of the data grid body's column separators, as `SortableHeaderChrome` is for the +/// header. +/// +/// `NSTableView` draws vertical grid lines by keeping one separator view per column as its own +/// subview, and it re-sorts that whole subview list on every layout pass. That is O(columns) views +/// and O(columns squared) work per pass: measured at 518ms for a single pass on a 500-column result, +/// against 0.03ms with the mask cleared. Adding or removing any subview of the table view, which is +/// exactly what opening the inline cell editor does, forces such a pass. So the grid clears +/// `gridStyleMask` and draws the separators itself (#2381). +/// +/// Where they are drawn follows AppKit's own split rather than inventing one. A row view covers +/// whatever the table view draws underneath it, which is why AppKit puts its horizontal separator +/// inside `NSTableRowView.drawSeparator(in:)` and can only put a vertical one in a view above the +/// rows. So a row draws the separators crossing it, and the table view draws only the area below the +/// last row, where nothing covers it. +@MainActor +enum DataGridBodyChrome { + static let separatorThickness: CGFloat = 1 + + /// Where the separators fall: one standing at the leading edge of every presented column the + /// rect reaches. + /// + /// The leading edge, not the trailing one, is where AppKit put it, and taking the boundary from + /// the column rather than from a fixed step keeps the row-number column's edge and leaves the + /// pool's spacers without one. + /// + /// Pure, and separate from the drawing, so a test can measure the geometry against + /// `rect(ofColumn:)` without a graphics context. + /// + /// - Parameters: + /// - rect: the area being drawn, in the coordinate space of `view`. + /// - view: the view drawing, which supplies the space the column rects are converted into. + static func separatorRects( + in rect: NSRect, + of view: NSView, + tableView: NSTableView, + presentsColumn: (Int) -> Bool + ) -> [NSRect] { + guard rect.width > 0, rect.height > 0 else { return [] } + let inTableView = view.convert(rect, to: tableView) + return tableView.columnIndexes(in: inTableView).compactMap { tableColumnIndex in + guard presentsColumn(tableColumnIndex) else { return nil } + let columnRect = view.convert(tableView.rect(ofColumn: tableColumnIndex), from: tableView) + guard columnRect.width > 0 else { return nil } + let separator = NSRect( + x: columnRect.minX - separatorThickness, + y: rect.minY, + width: separatorThickness, + height: rect.height + ) + return separator.intersects(rect) ? separator : nil + } + } + + static func drawColumnSeparators( + in rect: NSRect, + of view: NSView, + tableView: NSTableView, + presentsColumn: (Int) -> Bool + ) { + let separators = separatorRects(in: rect, of: view, tableView: tableView, presentsColumn: presentsColumn) + guard !separators.isEmpty else { return } + tableView.gridColor.setFill() + separators.forEach { $0.fill() } + } +} diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index e120e0fe3..32e37e751 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -398,6 +398,22 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData } } + /// Repaints whole rows, across the row-number column and every drawn cell. + /// + /// `reloadData(forRowIndexes:columnIndexes:)` rebuilds a cell view, and the row-number column is + /// the only one that still mounts one, so on its own it repaints a row's number and nothing + /// else. Every caller that used to reload a row's full column range goes through here (#2381). + func repaintRows(_ rows: IndexSet) { + guard let tableView, !rows.isEmpty else { return } + let rowNumberColumn = tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) + if rowNumberColumn >= 0 { + tableView.reloadData(forRowIndexes: rows, columnIndexes: IndexSet(integer: rowNumberColumn)) + } + for row in rows { + (tableView.rowView(atRow: row, makeIfNecessary: false) as? DataGridRowView)?.redrawCells() + } + } + /// Repaints one drawn cell, which is what a mounted cell got from `setNeedsDisplay` on itself. func redrawCell(row: Int, columnIndex: Int) { guard let tableView, @@ -496,10 +512,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData let visibleRect = tableView.visibleRect let visibleRange = tableView.rows(in: visibleRect) if visibleRange.length > 0 { - tableView.reloadData( - forRowIndexes: IndexSet(integersIn: visibleRange.location..<(visibleRange.location + visibleRange.length)), - columnIndexes: IndexSet(integersIn: 0..= 0, row < tableView.numberOfRows else { return } invalidateDisplayCache(forDisplayRow: row, column: column) visualIndex.updateRow(row, from: changeManager, displayIDs: displayIDs) - tableView.reloadData( - forRowIndexes: IndexSet(integer: row), - columnIndexes: IndexSet(integer: tableColumn) - ) + redrawCells(rows: IndexSet(integer: row), tableColumnIndexes: IndexSet(integer: tableColumn)) case .cellsChanged(let positions): guard !positions.isEmpty, let tableView else { return } var rowSet = IndexSet() @@ -1006,10 +1016,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData let visibleRange = tableView.rows(in: tableView.visibleRect) guard visibleRange.length > 0 else { return } invalidateDisplayCache() - tableView.reloadData( - forRowIndexes: IndexSet(integersIn: visibleRange.location..<(visibleRange.location + visibleRange.length)), - columnIndexes: IndexSet(integersIn: 0..= 0, row < tableView.numberOfRows else { return } invalidateDisplayCache(forDisplayRow: row) - tableView.reloadData( - forRowIndexes: IndexSet(integer: row), - columnIndexes: IndexSet(integersIn: 0..= 0 && $0 < tableView.numberOfRows }) - if !affected.isEmpty { - tableView.reloadData( - forRowIndexes: affected, - columnIndexes: IndexSet(integersIn: 0 ..< tableView.numberOfColumns) - ) - } + repaintRows(affected) guard let match, match.displayRow >= 0, match.displayRow < tableView.numberOfRows else { return } tableView.scrollRowToVisible(match.displayRow) diff --git a/TablePro/Views/Results/DataGridRowView.swift b/TablePro/Views/Results/DataGridRowView.swift index 30c95eb99..07b6f61ba 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -54,6 +54,9 @@ class DataGridRowView: NSTableRowView { accessibilityCellsAreStale = true } + /// Whether this row's drawn cells are waiting to be repainted. + var needsToDrawCells: Bool { contentView.needsDisplay } + // MARK: - Accessibility /// One element per data column, vended as this row's accessibility children. @@ -202,6 +205,17 @@ class DataGridRowView: NSTableRowView { } } + /// Draws the column separators crossing this row. See `DataGridBodyChrome`. + func drawColumnSeparators(in dirtyRect: NSRect, of view: NSView) { + guard let coordinator, let tableView = coordinator.tableView else { return } + DataGridBodyChrome.drawColumnSeparators( + in: dirtyRect, + of: view, + tableView: tableView, + presentsColumn: { coordinator.presentsColumn(atTableColumnIndex: $0) } + ) + } + required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } @@ -765,8 +779,12 @@ final class DataGridRowContentView: NSView { override var isFlipped: Bool { true } override var allowsVibrancy: Bool { false } + /// The separators go down in a second pass, after every cell, because a cell fills its whole + /// rect for a modified or find-match tint and would paint over a line drawn beside it. AppKit's + /// own separator views composite above the rows for the same reason. override func draw(_ dirtyRect: NSRect) { rowView?.drawCells(in: dirtyRect, of: self) + rowView?.drawColumnSeparators(in: dirtyRect, of: self) } override func mouseDown(with event: NSEvent) { diff --git a/TablePro/Views/Results/DataGridView+RowActions.swift b/TablePro/Views/Results/DataGridView+RowActions.swift index 64dbaf253..2e6c53627 100644 --- a/TablePro/Views/Results/DataGridView+RowActions.swift +++ b/TablePro/Views/Results/DataGridView+RowActions.swift @@ -16,9 +16,7 @@ extension TableViewCoordinator { func undoDeleteRow(at index: Int) { changeManager.undoRowDeletion(rowIndex: index) visualIndex.updateRow(index, from: changeManager, displayIDs: displayIDs) - tableView?.reloadData( - forRowIndexes: IndexSet(integer: index), - columnIndexes: IndexSet(integersIn: 0..<(tableView?.numberOfColumns ?? 0))) + repaintRows(IndexSet(integer: index)) refreshRowVisualState(at: index) } diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 5d5e4528c..082e038f9 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -75,7 +75,7 @@ struct DataGridView: NSViewRepresentable { tableView.allowsColumnReordering = true tableView.allowsColumnResizing = true tableView.columnAutoresizingStyle = .noColumnAutoresizing - tableView.gridStyleMask = [.solidVerticalGridLineMask] + tableView.gridStyleMask = [] tableView.intercellSpacing = NSSize(width: 1, height: 0) tableView.rowHeight = CGFloat(settings.rowHeight.rawValue) tableView.usesAutomaticRowHeights = false diff --git a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift index 1d8edf12a..983b270f3 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift @@ -15,17 +15,13 @@ extension TableViewCoordinator { } func commitTypedCellEdit(row: Int, columnIndex: Int, newValue typedNewValue: PluginCellValue) { - guard let tableView else { return } - guard let delta = recordCellEdit(row: row, columnIndex: columnIndex, newValue: typedNewValue) else { return } + guard recordCellEdit(row: row, columnIndex: columnIndex, newValue: typedNewValue) != nil else { return } invalidateDisplayCache() visualIndex.updateRow(row, from: changeManager, displayIDs: displayIDs) guard let tableColumnIndex = tableColumnIndex(for: columnIndex) else { return } - tableView.reloadData( - forRowIndexes: IndexSet(integer: row), - columnIndexes: IndexSet(integer: tableColumnIndex) - ) + redrawCells(rows: IndexSet(integer: row), tableColumnIndexes: IndexSet(integer: tableColumnIndex)) } @discardableResult diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index 6e5119ef6..0d89b7751 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -10,11 +10,20 @@ import TableProPluginKit extension TableViewCoordinator { // MARK: - Cell Interaction + /// Whether a cell is on screen for something to anchor to. + /// + /// A data cell is drawn rather than mounted, so the row being on screen is what answers this; + /// `view(atColumn:row:makeIfNecessary:)` is always nil now and every guard that still asked it + /// closed the editor or popover it was guarding (#2381). + func presentsCell(row: Int, tableColumnIndex: Int) -> Bool { + guard let tableView, row >= 0, row < tableView.numberOfRows else { return false } + guard presentsColumn(atTableColumnIndex: tableColumnIndex) else { return false } + return tableView.rowView(atRow: row, makeIfNecessary: false) != nil + } + func handleCellInteraction(row: Int, tableColumn: Int, columnIndex: Int, tableView: NSTableView) { guard let context = makeCellContext(row: row, columnIndex: columnIndex) else { return } - // A data cell is drawn rather than mounted, so the row being on screen is what says the - // interaction has somewhere to land. Asking for a cell view here always answered nil. - guard tableView.rowView(atRow: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: tableColumn) else { return } switch CellInteractionResolver().resolve(context) { case .blocked: @@ -130,7 +139,7 @@ extension TableViewCoordinator { column: Int, columnIndex: Int ) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let currentValue = cellValue(at: row, column: columnIndex) ?? "" let dbType = databaseType ?? .mysql diff --git a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift index 559e08308..d28ab228a 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift @@ -40,7 +40,7 @@ extension TableViewCoordinator { guard let fkInfo = tableRows.columnForeignKeys[columnName] else { return } let cellValue = cellValue(at: row, column: columnIndex) guard let databaseType, let connectionId else { return } - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let model = FKPreviewModel(cellValue: cellValue, fkInfo: fkInfo) let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) @@ -118,7 +118,7 @@ extension TableViewCoordinator { guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } let columnName = tableRows.columns[columnIndex] - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) dismissActiveCellEditorPopover() @@ -152,7 +152,7 @@ extension TableViewCoordinator { func showBlobEditorPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { let currentValue = blobStringValue(at: row, columnIndex: columnIndex) - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) dismissActiveCellEditorPopover() @@ -177,7 +177,7 @@ extension TableViewCoordinator { func showDateTimePickerPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { let tableRows = tableRowsProvider() guard columnIndex >= 0, columnIndex < tableRows.columnTypes.count else { return } - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let columnType = tableRows.columnTypes[columnIndex] let parsed = DatabaseDateParser.parse(cellValue(at: row, column: columnIndex)) @@ -206,7 +206,7 @@ extension TableViewCoordinator { } func showEnumPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let tableRows = tableRowsProvider() guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } let columnName = tableRows.columns[columnIndex] @@ -230,7 +230,7 @@ extension TableViewCoordinator { } func showSetPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let tableRows = tableRowsProvider() guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } let columnName = tableRows.columns[columnIndex] @@ -249,7 +249,7 @@ extension TableViewCoordinator { } func showArrayEditorPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let tableRows = tableRowsProvider() guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } let columnName = tableRows.columns[columnIndex] @@ -309,7 +309,7 @@ extension TableViewCoordinator { } func showDropdownMenu(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let tableRows = tableRowsProvider() guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } @@ -381,7 +381,7 @@ extension TableViewCoordinator { guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } let columnName = tableRows.columns[columnIndex] - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) PopoverPresenter.show( @@ -412,7 +412,7 @@ extension TableViewCoordinator { guard columnIndex >= 0, columnIndex < tableRows.columns.count else { return } let columnName = tableRows.columns[columnIndex] - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) PopoverPresenter.show( @@ -435,7 +435,7 @@ extension TableViewCoordinator { func showBlobViewerPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { let currentValue = blobStringValue(at: row, columnIndex: columnIndex) - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: column) else { return } let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) PopoverPresenter.show( diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index 5f537eaa7..fa7b7243b 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -18,6 +18,26 @@ final class KeyHandlingTableView: NSTableView { window.makeFirstResponder(self) } + /// Continues the column separators past the last row. + /// + /// A row view covers whatever the table view drew beneath it, so this reaches only the area no + /// row occupies, which is exactly the area the rows cannot draw. See `DataGridBodyChrome`. + override func drawBackground(inClipRect clipRect: NSRect) { + super.drawBackground(inClipRect: clipRect) + guard let coordinator else { return } + let lastRowBottom = numberOfRows > 0 ? rect(ofRow: numberOfRows - 1).maxY : bounds.minY + let belowRows = clipRect.intersection( + NSRect(x: clipRect.minX, y: lastRowBottom, width: clipRect.width, height: bounds.height) + ) + guard !belowRows.isEmpty else { return } + DataGridBodyChrome.drawColumnSeparators( + in: belowRows, + of: self, + tableView: self, + presentsColumn: { coordinator.presentsColumn(atTableColumnIndex: $0) } + ) + } + override func didAddSubview(_ subview: NSView) { super.didAddSubview(subview) guard !isRaisingOverlay else { return } diff --git a/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift b/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift new file mode 100644 index 000000000..5a00ec081 --- /dev/null +++ b/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift @@ -0,0 +1,65 @@ +// +// CellOverlayTextLayoutTests.swift +// TableProTests +// + +import AppKit +import Testing + +@testable import TablePro + +/// A cell holds one value, so an inline overlay behaves like a field editor and scrolls a long line +/// rather than wrapping it. Wrapping made TextKit 2 lay the whole paragraph out before the overlay +/// could appear: 206ms for a 256KB value and 816ms for 1MB, against 7ms unwrapped (#2381). +@Suite("Cell overlay text layout") +@MainActor +struct CellOverlayTextLayoutTests { + private func makeTextView(width: CGFloat = 140) -> NSTextView { + let textView = NSTextView(frame: NSRect(x: 0, y: 0, width: width, height: 24)) + CellOverlayBase.applyCellTextLayout(to: textView) + return textView + } + + @Test("A long line is not wrapped into the cell's width") + func longLineIsNotWrapped() throws { + let textView = makeTextView() + let container = try #require(textView.textContainer) + + #expect(!container.widthTracksTextView) + #expect(container.size.width == CGFloat.greatestFiniteMagnitude) + } + + /// A text view grows only as far as `maxSize`, which `init(frame:)` leaves at the frame size, so + /// leaving it alone clips the long line at the cell's width instead of scrolling it. Measured: + /// a 64,000-character value produced a 140pt document view without this and 344,166pt with it. + @Test("A long line makes the editor scrollable rather than clipping it") + func longLineScrollsRatherThanClipping() { + let textView = makeTextView() + textView.string = String(repeating: "some cell value ", count: 4_000) + textView.layoutManager?.ensureLayout(for: textView.textContainer!) + + #expect(textView.maxSize.width == CGFloat.greatestFiniteMagnitude) + #expect(textView.frame.width > 1_000, "the document has to outgrow the cell for scrolling to reach the text") + } + + @Test("A short value still lays out inside the cell") + func shortValueStillLaysOut() { + let textView = makeTextView() + textView.string = "42" + textView.layoutManager?.ensureLayout(for: textView.textContainer!) + + #expect(textView.string == "42") + } + + /// The viewer shares the editor's layout, so a read-only table cannot reach the same freeze by + /// opening a huge value inline. + @Test("The overlay layout is one definition, shared by editor and viewer") + func editorAndViewerShareTheLayout() throws { + let editorTextView = makeTextView() + let viewerTextView = makeTextView(width: 300) + + #expect(editorTextView.textContainer?.widthTracksTextView == false) + #expect(viewerTextView.textContainer?.widthTracksTextView == false) + #expect(editorTextView.maxSize == viewerTextView.maxSize) + } +} diff --git a/TableProTests/Views/Results/DataGridBodyChromeTests.swift b/TableProTests/Views/Results/DataGridBodyChromeTests.swift new file mode 100644 index 000000000..64783d85c --- /dev/null +++ b/TableProTests/Views/Results/DataGridBodyChromeTests.swift @@ -0,0 +1,152 @@ +// +// DataGridBodyChromeTests.swift +// TableProTests +// + +import AppKit +import SwiftUI +import TableProPluginKit +import Testing + +@testable import TablePro + +@MainActor +private final class BodyChromeLayoutPersister: ColumnLayoutPersisting { + func load(for key: ColumnLayoutTableKey) -> ColumnLayoutState? { nil } + func save(_ layout: ColumnLayoutState, for key: ColumnLayoutTableKey) {} + func clear(for key: ColumnLayoutTableKey) {} +} + +/// The grid draws its own column separators because `NSTableView` draws vertical grid lines with one +/// separator view per column and re-sorts its whole subview list on every layout pass: 518ms for a +/// single pass on a 500-column result, against 0.03ms with the mask cleared (#2381). +/// +/// These measure through `rect(ofColumn:)` and through the rendered pixels, never through the chrome +/// type's own arithmetic, so they cannot pass by agreeing with themselves. +@Suite("Data grid body chrome") +@MainActor +struct DataGridBodyChromeTests { + private struct Grid { + let window: NSWindow + let tableView: KeyHandlingTableView + let coordinator: TableViewCoordinator + } + + private func makeGrid(columns: [String], rows: Int = 3, width: CGFloat = 600) -> Grid { + let coordinator = TableViewCoordinator( + changeManager: AnyChangeManager(DataChangeManager()), + isEditable: true, + selectedRowIndices: .constant([]), + delegate: nil, + layoutPersister: BodyChromeLayoutPersister() + ) + let queryRows = (0 ..< rows).map { row in + columns.map { PluginCellValue.text("\($0)-\(row)") } + } + let tableRows = TableRows.from( + queryRows: queryRows, + columns: columns, + columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: columns.count) + ) + coordinator.tableRowsProvider = { tableRows } + coordinator.rebuildColumnMetadataCache(from: tableRows) + coordinator.updateCache() + + let tableView = KeyHandlingTableView(frame: NSRect(x: 0, y: 0, width: width, height: 200)) + tableView.columnAutoresizingStyle = .noColumnAutoresizing + tableView.gridStyleMask = [] + tableView.intercellSpacing = NSSize(width: 1, height: 0) + tableView.rowHeight = 21 + tableView.coordinator = coordinator + tableView.dataSource = coordinator + tableView.delegate = coordinator + tableView.addTableColumn(DataGridView.makeRowNumberColumn()) + coordinator.tableView = tableView + coordinator.columnPool.reconcile( + tableView: tableView, + schema: coordinator.identitySchema, + columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: columns.count), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: { _, _ in 120 } + ) + + let scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: width, height: 200)) + scrollView.documentView = tableView + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: width, height: 200), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + window.contentView = scrollView + tableView.reloadData() + tableView.layoutSubtreeIfNeeded() + window.layoutIfNeeded() + return Grid(window: window, tableView: tableView, coordinator: coordinator) + } + + /// Clearing the mask is the fix, so a test that only checked the drawing would pass with the + /// separator views still attached and the cost still there. + @Test("The grid asks AppKit for no grid lines of its own") + func gridStyleMaskIsCleared() { + let grid = makeGrid(columns: ["id", "name"]) + #expect(grid.tableView.gridStyleMask == []) + } + + /// AppKit put its separator at the leading edge of every column, which is the boundary the + /// reader sees between two columns and the one that keeps the row-number column's edge. + @Test("A separator stands at the leading edge of every presented column") + func separatorsSitAtPresentedColumnLeadingEdges() { + let grid = makeGrid(columns: ["id", "name", "total"]) + let view = NSView(frame: grid.tableView.bounds) + + let separators = DataGridBodyChrome.separatorRects( + in: view.bounds, + of: view, + tableView: grid.tableView, + presentsColumn: { grid.coordinator.presentsColumn(atTableColumnIndex: $0) } + ) + + let expected = grid.tableView.tableColumns.indices + .filter { grid.coordinator.presentsColumn(atTableColumnIndex: $0) } + .map { grid.tableView.rect(ofColumn: $0).minX - DataGridBodyChrome.separatorThickness } + #expect(!expected.isEmpty) + #expect(separators.map(\.minX) == expected) + #expect(separators.allSatisfy { $0.width == DataGridBodyChrome.separatorThickness }) + } + + /// The row-number column is an attached column that the result does not present, so it gets no + /// separator of its own; the boundary the reader sees there is the first data column's edge. + @Test("The row-number column is not given a separator of its own") + func rowNumberColumnHasNoSeparator() { + let grid = makeGrid(columns: ["id", "name"]) + let rowNumber = grid.tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) + + #expect(rowNumber >= 0) + #expect(!grid.coordinator.presentsColumn(atTableColumnIndex: rowNumber)) + } + + /// Every data cell is drawn, so the row is what paints the separators crossing it; a separator + /// drawn by the table view underneath would be covered by the row's own background. + @Test("A row paints the separators crossing it") + func rowDrawsItsOwnSeparators() throws { + let grid = makeGrid(columns: ["id", "name"]) + let rowView = try #require(grid.tableView.rowView(atRow: 0, makeIfNecessary: true) as? DataGridRowView) + rowView.layoutSubtreeIfNeeded() + + let rep = try #require(rowView.bitmapImageRepForCachingDisplay(in: rowView.bounds)) + rowView.cacheDisplay(in: rowView.bounds, to: rep) + + let firstData = try #require(grid.coordinator.firstPresentedColumnIndex()) + let boundary = grid.tableView.rect(ofColumn: firstData).minX + let scale = CGFloat(rep.pixelsWide) / rowView.bounds.width + let onBoundary = rep.colorAt(x: Int((boundary - 0.5) * scale), y: Int(rowView.bounds.height * scale / 2)) + let insideCell = rep.colorAt(x: Int((boundary + 30) * scale), y: Int(rowView.bounds.height * scale / 2)) + + #expect(onBoundary != nil) + #expect(insideCell != nil) + #expect(onBoundary != insideCell, "the separator has to differ from the cell beside it") + } +} diff --git a/TableProTests/Views/Results/DrawnCellReachabilityTests.swift b/TableProTests/Views/Results/DrawnCellReachabilityTests.swift new file mode 100644 index 000000000..524f74c93 --- /dev/null +++ b/TableProTests/Views/Results/DrawnCellReachabilityTests.swift @@ -0,0 +1,131 @@ +// +// DrawnCellReachabilityTests.swift +// TableProTests +// + +import AppKit +import SwiftUI +import TableProPluginKit +import Testing + +@testable import TablePro + +@MainActor +private final class ReachabilityLayoutPersister: ColumnLayoutPersisting { + func load(for key: ColumnLayoutTableKey) -> ColumnLayoutState? { nil } + func save(_ layout: ColumnLayoutState, for key: ColumnLayoutTableKey) {} + func clear(for key: ColumnLayoutTableKey) {} +} + +/// A data cell is drawn rather than mounted, so `view(atColumn:row:makeIfNecessary:false)` is always +/// nil. Twelve guards still asked it before opening an editor or a popover, and every one of them +/// returned early: the JSON, blob, PHP, date, enum, set, array, dropdown and type-picker editors +/// were all unreachable. `presentsCell(row:tableColumnIndex:)` is the single replacement (#2381). +@Suite("Drawn cell reachability") +@MainActor +struct DrawnCellReachabilityTests { + private func makeCoordinator(columns: [String], rows: Int = 3) -> TableViewCoordinator { + let coordinator = TableViewCoordinator( + changeManager: AnyChangeManager(DataChangeManager()), + isEditable: true, + selectedRowIndices: .constant([]), + delegate: nil, + layoutPersister: ReachabilityLayoutPersister() + ) + let queryRows = (0 ..< rows).map { row in columns.map { PluginCellValue.text("\($0)-\(row)") } } + let tableRows = TableRows.from( + queryRows: queryRows, + columns: columns, + columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: columns.count) + ) + coordinator.tableRowsProvider = { tableRows } + coordinator.rebuildColumnMetadataCache(from: tableRows) + coordinator.updateCache() + + let tableView = KeyHandlingTableView(frame: NSRect(x: 0, y: 0, width: 600, height: 200)) + tableView.columnAutoresizingStyle = .noColumnAutoresizing + tableView.rowHeight = 21 + tableView.coordinator = coordinator + tableView.dataSource = coordinator + tableView.delegate = coordinator + tableView.addTableColumn(DataGridView.makeRowNumberColumn()) + coordinator.tableView = tableView + coordinator.columnPool.reconcile( + tableView: tableView, + schema: coordinator.identitySchema, + columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: columns.count), + savedLayout: nil, + isEditable: true, + hiddenColumnNames: [], + widthCalculator: { _, _ in 120 } + ) + let scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: 600, height: 200)) + scrollView.documentView = tableView + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 600, height: 200), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + window.contentView = scrollView + tableView.reloadData() + tableView.layoutSubtreeIfNeeded() + window.layoutIfNeeded() + return coordinator + } + + /// The regression itself: the question the guards used to ask now answers nil for every data + /// cell, so anything still asking it is dead code. + @Test("No data cell mounts a view any more") + func dataCellsMountNoView() throws { + let coordinator = makeCoordinator(columns: ["id", "name"]) + let tableView = try #require(coordinator.tableView) + let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) + + #expect(tableView.view(atColumn: dataColumn, row: 0, makeIfNecessary: false) == nil) + #expect(tableView.view(atColumn: dataColumn, row: 0, makeIfNecessary: true) == nil) + } + + @Test("An on-screen data cell is reachable") + func onScreenCellIsReachable() throws { + let coordinator = makeCoordinator(columns: ["id", "name"]) + let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) + + #expect(coordinator.presentsCell(row: 0, tableColumnIndex: dataColumn)) + } + + @Test("A row outside the result is not reachable") + func rowOutOfRangeIsNotReachable() throws { + let coordinator = makeCoordinator(columns: ["id", "name"]) + let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) + + #expect(!coordinator.presentsCell(row: -1, tableColumnIndex: dataColumn)) + #expect(!coordinator.presentsCell(row: 99, tableColumnIndex: dataColumn)) + } + + @Test("The row-number column is not a cell anything opens on") + func rowNumberColumnIsNotReachable() throws { + let coordinator = makeCoordinator(columns: ["id", "name"]) + let tableView = try #require(coordinator.tableView) + let rowNumber = tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) + + #expect(rowNumber >= 0) + #expect(!coordinator.presentsCell(row: 0, tableColumnIndex: rowNumber)) + } + + /// `reloadData(forRowIndexes:columnIndexes:)` rebuilds a cell view, and the row-number column is + /// the only one that still has one. Seven callers reloaded a row's whole column range and so + /// repainted nothing, which is how a committed cell edit and an undo stopped showing (#2381). + @Test("Repainting a row reaches the row's drawn cells") + func repaintingARowReachesItsDrawnCells() throws { + let coordinator = makeCoordinator(columns: ["id", "name"]) + let tableView = try #require(coordinator.tableView) + let rowView = try #require(tableView.rowView(atRow: 0, makeIfNecessary: true) as? DataGridRowView) + rowView.layoutSubtreeIfNeeded() + rowView.displayIfNeeded() + + coordinator.repaintRows(IndexSet(integer: 0)) + + #expect(rowView.needsToDrawCells, "the row's drawn cells must be marked for redisplay") + } +} From 54a4625c62557c5cf7db32448ad75fdb04d7e350 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 23 Aug 2026 03:49:44 +0700 Subject: [PATCH 6/7] fix(datagrid): bound the rows a repaint reloads and pin the separator geometry (#2381) --- TablePro/Views/Results/CellOverlayBase.swift | 9 +- .../Views/Results/DataGridCoordinator.swift | 19 ++-- TablePro/Views/Results/DataGridRowView.swift | 8 +- .../Results/CellOverlayTextLayoutTests.swift | 11 ++- .../Results/DataGridBodyChromeTests.swift | 93 ++++++++++++++++--- .../Results/DrawnCellReachabilityTests.swift | 68 ++++++++++---- docs/features/data-grid.mdx | 2 +- 7 files changed, 163 insertions(+), 47 deletions(-) diff --git a/TablePro/Views/Results/CellOverlayBase.swift b/TablePro/Views/Results/CellOverlayBase.swift index eba2e3e8e..2da88e804 100644 --- a/TablePro/Views/Results/CellOverlayBase.swift +++ b/TablePro/Views/Results/CellOverlayBase.swift @@ -114,6 +114,10 @@ class CellOverlayBase: NSObject { /// rather than wrapping it. Wrapping made TextKit 2 lay the whole value out before the overlay /// could appear: measured at 206ms for a 256KB value and 816ms for 1MB, against 7ms unwrapped, /// and the wrapped result was thousands of visual lines in a box 120pt tall (#2381). + /// + /// `maxSize` is raised with the container because a text view grows only as far as `maxSize`, + /// which `init(frame:)` leaves at the frame: without it the long line is clipped at the cell's + /// width instead of scrolled, measured as a 140pt document against 344,166pt with it raised. static func applyCellTextLayout(to textView: NSTextView) { let unbounded = NSSize( width: CGFloat.greatestFiniteMagnitude, @@ -121,9 +125,6 @@ class CellOverlayBase: NSObject { ) textView.isVerticallyResizable = true textView.isHorizontallyResizable = true - // A text view grows only as far as `maxSize`, which `init(frame:)` leaves at the frame, so - // without this the long line is clipped at the cell's width rather than scrolled: measured - // at a 140pt document for a 64,000-character value, against 344,166pt with it raised. textView.maxSize = unbounded textView.textContainer?.widthTracksTextView = false textView.textContainer?.containerSize = unbounded @@ -133,7 +134,7 @@ class CellOverlayBase: NSObject { let scrollView = NSScrollView(frame: container.bounds) scrollView.autoresizingMask = [.width, .height] scrollView.hasVerticalScroller = true - scrollView.hasHorizontalScroller = true + scrollView.hasHorizontalScroller = false scrollView.autohidesScrollers = true scrollView.borderType = .noBorder scrollView.drawsBackground = true diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 32e37e751..70a8f40c2 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -403,8 +403,13 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData /// `reloadData(forRowIndexes:columnIndexes:)` rebuilds a cell view, and the row-number column is /// the only one that still mounts one, so on its own it repaints a row's number and nothing /// else. Every caller that used to reload a row's full column range goes through here (#2381). + /// + /// A row past the end is dropped rather than passed on: `reloadData(forRowIndexes:)` raises + /// `NSRangeException` for one, and a row view can outlive the result that shrank under it. func repaintRows(_ rows: IndexSet) { - guard let tableView, !rows.isEmpty else { return } + guard let tableView else { return } + let rows = rows.filteredIndexSet { $0 >= 0 && $0 < tableView.numberOfRows } + guard !rows.isEmpty else { return } let rowNumberColumn = tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) if rowNumberColumn >= 0 { tableView.reloadData(forRowIndexes: rows, columnIndexes: IndexSet(integer: rowNumberColumn)) @@ -1004,13 +1009,11 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData reloadVisibleRowsAndStates() } - /// Repaint visible rows in two layers Apple's NSTableView contract requires: - /// `reloadData(forRowIndexes:columnIndexes:)` re-fetches cells via - /// `tableView(_:viewFor:row:)` but does not touch row views, so per-row - /// decoration (deleted/inserted tint, deleted-row context menu state) goes - /// stale. `enumerateAvailableRowViews` then visits each live `NSTableRowView` - /// so `applyVisualState` can mutate row-level state without recreating views. - /// Both delegates call this after model mutations that don't change row count. + /// Repaints visible rows in the two layers a row needs: `repaintRows` covers the row-number + /// column and the drawn cells, and `refreshVisibleRowVisualStates` then visits each live + /// `NSTableRowView` so `applyVisualState` can carry the per-row decoration (deleted or inserted + /// tint, deleted-row context menu state) without recreating a view. Both delegates call this + /// after a model mutation that leaves the row count alone. func reloadVisibleRowsAndStates() { guard let tableView else { return } let visibleRange = tableView.rows(in: tableView.visibleRect) diff --git a/TablePro/Views/Results/DataGridRowView.swift b/TablePro/Views/Results/DataGridRowView.swift index 07b6f61ba..88f78b04d 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -38,7 +38,12 @@ class DataGridRowView: NSTableRowView { } /// Repaints one cell, the way a mounted cell view repainted itself. + /// + /// The accessibility elements carry the cell's value, so a repaint has to stand them down too, + /// or VoiceOver keeps reading what the cell said before the edit. A committed cell edit takes + /// this path, so it is the ordinary case rather than a rare one. func redrawCell(atTableColumnIndex tableColumnIndex: Int) { + accessibilityCellsAreStale = true guard let tableView = coordinator?.tableView else { contentView.needsDisplay = true return @@ -54,9 +59,6 @@ class DataGridRowView: NSTableRowView { accessibilityCellsAreStale = true } - /// Whether this row's drawn cells are waiting to be repainted. - var needsToDrawCells: Bool { contentView.needsDisplay } - // MARK: - Accessibility /// One element per data column, vended as this row's accessibility children. diff --git a/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift b/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift index 5a00ec081..470f93832 100644 --- a/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift +++ b/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift @@ -33,20 +33,23 @@ struct CellOverlayTextLayoutTests { /// leaving it alone clips the long line at the cell's width instead of scrolling it. Measured: /// a 64,000-character value produced a 140pt document view without this and 344,166pt with it. @Test("A long line makes the editor scrollable rather than clipping it") - func longLineScrollsRatherThanClipping() { + func longLineScrollsRatherThanClipping() throws { let textView = makeTextView() + let layoutManager = try #require(textView.textLayoutManager, "the overlay has to be on TextKit 2") textView.string = String(repeating: "some cell value ", count: 4_000) - textView.layoutManager?.ensureLayout(for: textView.textContainer!) + layoutManager.ensureLayout(for: layoutManager.documentRange) + textView.layout() #expect(textView.maxSize.width == CGFloat.greatestFiniteMagnitude) #expect(textView.frame.width > 1_000, "the document has to outgrow the cell for scrolling to reach the text") } @Test("A short value still lays out inside the cell") - func shortValueStillLaysOut() { + func shortValueStillLaysOut() throws { let textView = makeTextView() + let layoutManager = try #require(textView.textLayoutManager) textView.string = "42" - textView.layoutManager?.ensureLayout(for: textView.textContainer!) + layoutManager.ensureLayout(for: layoutManager.documentRange) #expect(textView.string == "42") } diff --git a/TableProTests/Views/Results/DataGridBodyChromeTests.swift b/TableProTests/Views/Results/DataGridBodyChromeTests.swift index 64783d85c..52422628d 100644 --- a/TableProTests/Views/Results/DataGridBodyChromeTests.swift +++ b/TableProTests/Views/Results/DataGridBodyChromeTests.swift @@ -88,11 +88,50 @@ struct DataGridBodyChromeTests { } /// Clearing the mask is the fix, so a test that only checked the drawing would pass with the - /// separator views still attached and the cost still there. - @Test("The grid asks AppKit for no grid lines of its own") - func gridStyleMaskIsCleared() { - let grid = makeGrid(columns: ["id", "name"]) - #expect(grid.tableView.gridStyleMask == []) + /// separator views back and the cost with them. The fixture sets the mask itself, so asserting + /// on the fixture proves nothing; this reads the one line in the app that decides it. + @Test("The grid never asks AppKit for vertical grid lines") + func gridStyleMaskIsClearedInTheAppItself() throws { + let source = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("TablePro/Views/Results/DataGridView.swift") + let text = try String(contentsOf: source, encoding: .utf8) + + #expect(text.contains("gridStyleMask = []")) + #expect( + !text.contains("solidVerticalGridLineMask"), + "one separator view per column costs 518ms of layout per pass on a 500-column result" + ) + } + + /// A partial repaint invalidates one cell's rect, and `rect(ofColumn:)` includes the intercell + /// spacing, so the next column's separator lives inside the rect being repainted. It has to be + /// redrawn there or every visited cell loses its right-hand rule until a full-row repaint. + @Test("Repainting one cell redraws the separator standing inside its rect") + func partialRepaintRedrawsTheSeparatorInsideIt() throws { + let grid = makeGrid(columns: ["id", "name", "total"]) + let view = NSView(frame: grid.tableView.bounds) + let first = try #require(grid.coordinator.firstPresentedColumnIndex()) + let next = try #require(grid.coordinator.nextPresentedColumnIndex(after: first)) + let cellRect = grid.tableView.rect(ofColumn: first) + let neighbourSeparator = grid.tableView.rect(ofColumn: next).minX - DataGridBodyChrome.separatorThickness + + #expect( + neighbourSeparator >= cellRect.minX && neighbourSeparator < cellRect.maxX, + "the neighbour's separator sits inside the repainted cell's rect, so the repaint erases it" + ) + + let separators = DataGridBodyChrome.separatorRects( + in: cellRect, + of: view, + tableView: grid.tableView, + presentsColumn: { grid.coordinator.presentsColumn(atTableColumnIndex: $0) } + ) + + #expect(separators.map(\.minX).contains(neighbourSeparator)) } /// AppKit put its separator at the leading edge of every column, which is the boundary the @@ -128,6 +167,36 @@ struct DataGridBodyChromeTests { #expect(!grid.coordinator.presentsColumn(atTableColumnIndex: rowNumber)) } + /// The colour has to come from `tableView.gridColor`, which is the dynamic catalog colour AppKit + /// was filling with, so an appearance change carries the separator with it and there is no + /// second spelling to keep in sync. A hardcoded colour would pass a geometry test and be wrong + /// in dark mode. + @Test("The separator is drawn in the table view's own grid colour") + func separatorUsesTheTableViewGridColor() throws { + let grid = makeGrid(columns: ["id", "name"]) + grid.tableView.gridColor = .systemRed + let rowView = try #require(grid.tableView.rowView(atRow: 0, makeIfNecessary: true) as? DataGridRowView) + rowView.layoutSubtreeIfNeeded() + + let rep = try #require(rowView.bitmapImageRepForCachingDisplay(in: rowView.bounds)) + rowView.cacheDisplay(in: rowView.bounds, to: rep) + + let firstData = try #require(grid.coordinator.firstPresentedColumnIndex()) + let boundary = grid.tableView.rect(ofColumn: firstData).minX + let scale = CGFloat(rep.pixelsWide) / rowView.bounds.width + let sampled = rep.colorAt( + x: Int((boundary - 0.5) * scale), + y: Int(rowView.bounds.height * scale / 2) + )?.usingColorSpace(.deviceRGB) + let expected = NSColor.systemRed.usingColorSpace(.deviceRGB) + + let sampledRed = try #require(sampled?.redComponent) + let sampledGreen = try #require(sampled?.greenComponent) + let expectedRed = try #require(expected?.redComponent) + #expect(abs(sampledRed - expectedRed) < 0.15) + #expect(sampledRed > sampledGreen + 0.3, "the separator has to carry the grid colour, not a fixed grey") + } + /// Every data cell is drawn, so the row is what paints the separators crossing it; a separator /// drawn by the table view underneath would be covered by the row's own background. @Test("A row paints the separators crossing it") @@ -142,11 +211,13 @@ struct DataGridBodyChromeTests { let firstData = try #require(grid.coordinator.firstPresentedColumnIndex()) let boundary = grid.tableView.rect(ofColumn: firstData).minX let scale = CGFloat(rep.pixelsWide) / rowView.bounds.width - let onBoundary = rep.colorAt(x: Int((boundary - 0.5) * scale), y: Int(rowView.bounds.height * scale / 2)) - let insideCell = rep.colorAt(x: Int((boundary + 30) * scale), y: Int(rowView.bounds.height * scale / 2)) - - #expect(onBoundary != nil) - #expect(insideCell != nil) - #expect(onBoundary != insideCell, "the separator has to differ from the cell beside it") + let onBoundary = try #require( + rep.colorAt(x: Int((boundary - 0.5) * scale), y: Int(rowView.bounds.height * scale / 2)) + ).usingColorSpace(.deviceRGB) + let expected = grid.tableView.gridColor.usingColorSpace(.deviceRGB) + + let drawn = try #require(onBoundary?.brightnessComponent) + let wanted = try #require(expected?.brightnessComponent) + #expect(abs(drawn - wanted) < 0.2, "the boundary pixel has to carry the grid colour") } } diff --git a/TableProTests/Views/Results/DrawnCellReachabilityTests.swift b/TableProTests/Views/Results/DrawnCellReachabilityTests.swift index 524f74c93..24bfdd106 100644 --- a/TableProTests/Views/Results/DrawnCellReachabilityTests.swift +++ b/TableProTests/Views/Results/DrawnCellReachabilityTests.swift @@ -24,7 +24,14 @@ private final class ReachabilityLayoutPersister: ColumnLayoutPersisting { @Suite("Drawn cell reachability") @MainActor struct DrawnCellReachabilityTests { - private func makeCoordinator(columns: [String], rows: Int = 3) -> TableViewCoordinator { + /// Holds the window: `TableViewCoordinator.tableView` is weak both ways, so a suite that + /// dropped the hierarchy would only survive on `NSApplication` incidentally retaining it. + private struct Grid { + let window: NSWindow + let coordinator: TableViewCoordinator + } + + private func makeGrid(columns: [String], rows: Int = 3) -> Grid { let coordinator = TableViewCoordinator( changeManager: AnyChangeManager(DataChangeManager()), isEditable: true, @@ -71,14 +78,14 @@ struct DrawnCellReachabilityTests { tableView.reloadData() tableView.layoutSubtreeIfNeeded() window.layoutIfNeeded() - return coordinator + return Grid(window: window, coordinator: coordinator) } /// The regression itself: the question the guards used to ask now answers nil for every data /// cell, so anything still asking it is dead code. @Test("No data cell mounts a view any more") func dataCellsMountNoView() throws { - let coordinator = makeCoordinator(columns: ["id", "name"]) + let coordinator = makeGrid(columns: ["id", "name"]).coordinator let tableView = try #require(coordinator.tableView) let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) @@ -88,7 +95,7 @@ struct DrawnCellReachabilityTests { @Test("An on-screen data cell is reachable") func onScreenCellIsReachable() throws { - let coordinator = makeCoordinator(columns: ["id", "name"]) + let coordinator = makeGrid(columns: ["id", "name"]).coordinator let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) #expect(coordinator.presentsCell(row: 0, tableColumnIndex: dataColumn)) @@ -96,7 +103,7 @@ struct DrawnCellReachabilityTests { @Test("A row outside the result is not reachable") func rowOutOfRangeIsNotReachable() throws { - let coordinator = makeCoordinator(columns: ["id", "name"]) + let coordinator = makeGrid(columns: ["id", "name"]).coordinator let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) #expect(!coordinator.presentsCell(row: -1, tableColumnIndex: dataColumn)) @@ -105,7 +112,7 @@ struct DrawnCellReachabilityTests { @Test("The row-number column is not a cell anything opens on") func rowNumberColumnIsNotReachable() throws { - let coordinator = makeCoordinator(columns: ["id", "name"]) + let coordinator = makeGrid(columns: ["id", "name"]).coordinator let tableView = try #require(coordinator.tableView) let rowNumber = tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) @@ -113,19 +120,48 @@ struct DrawnCellReachabilityTests { #expect(!coordinator.presentsCell(row: 0, tableColumnIndex: rowNumber)) } - /// `reloadData(forRowIndexes:columnIndexes:)` rebuilds a cell view, and the row-number column is - /// the only one that still has one. Seven callers reloaded a row's whole column range and so - /// repainted nothing, which is how a committed cell edit and an undo stopped showing (#2381). - @Test("Repainting a row reaches the row's drawn cells") - func repaintingARowReachesItsDrawnCells() throws { - let coordinator = makeCoordinator(columns: ["id", "name"]) + /// `reloadData(forRowIndexes:columnIndexes:)` raises `NSRangeException` for a row past the end, + /// and a row view can outlive a result that shrank under it, so undoing a delete on a stale row + /// used to crash rather than no-op. + @Test("Repainting a row past the end of the result is a no-op") + func repaintingAnOutOfRangeRowIsANoOp() throws { + let coordinator = makeGrid(columns: ["id", "name"], rows: 2).coordinator + + coordinator.repaintRows(IndexSet(integer: 99)) + coordinator.repaintRows(IndexSet(integer: -1)) + } + + /// A drawn cell has no view of its own, so the row vends the accessibility element that carries + /// the value. Repainting one cell has to stand that element down as well, or VoiceOver keeps + /// reading what the cell said before the edit. A committed cell edit takes this exact path. + @Test("Repainting one cell refreshes the value VoiceOver reads") + func repaintingOneCellRefreshesItsAccessibilityValue() throws { + let columns = ["id", "name"] + var current = TableRows.from( + queryRows: [columns.map { PluginCellValue.text("before-\($0)") }], + columns: columns, + columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: columns.count) + ) + let coordinator = makeGrid(columns: columns, rows: 1).coordinator + coordinator.tableRowsProvider = { current } + coordinator.updateCache() + let tableView = try #require(coordinator.tableView) + tableView.reloadData() let rowView = try #require(tableView.rowView(atRow: 0, makeIfNecessary: true) as? DataGridRowView) - rowView.layoutSubtreeIfNeeded() - rowView.displayIfNeeded() + let before = (rowView.accessibilityChildren()?.first as? NSAccessibilityElement)?.accessibilityValue() as? String + #expect(before?.contains("before") == true) - coordinator.repaintRows(IndexSet(integer: 0)) + current = TableRows.from( + queryRows: [columns.map { _ in PluginCellValue.text("after") }], + columns: columns, + columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: columns.count) + ) + coordinator.invalidateDisplayCache() + let dataColumn = try #require(coordinator.firstPresentedColumnIndex()) + rowView.redrawCell(atTableColumnIndex: dataColumn) - #expect(rowView.needsToDrawCells, "the row's drawn cells must be marked for redisplay") + let after = (rowView.accessibilityChildren()?.first as? NSAccessibilityElement)?.accessibilityValue() as? String + #expect(after == "after", "the element still held the pre-edit value") } } diff --git a/docs/features/data-grid.mdx b/docs/features/data-grid.mdx index b7729c745..3b7d925d6 100644 --- a/docs/features/data-grid.mdx +++ b/docs/features/data-grid.mdx @@ -116,6 +116,6 @@ A chart draws the loaded rows, up to 2,000 points, 20 series, and 50,000 inspect ## Editing and display -Double-click a cell to edit it. Nothing reaches the database until you save, and [Change Tracking](/features/change-tracking) covers the type-specific editors, the row operations, and what a Save runs. +Double-click a cell to edit it, or press `Return` on the focused cell. The inline editor keeps a value on one line and scrolls it, the way a text field does, rather than wrapping it into the cell's width. `Option+Return` inserts a line break, `Return` commits, `Escape` cancels, and `Tab` commits and moves to the next cell. Nothing reaches the database until you save, and [Change Tracking](/features/change-tracking) covers the type-specific editors, the row operations, and what a Save runs. NULL renders as styled `NULL` text. That text, the date format, row height, row numbers, and alternate row backgrounds are [data settings](/customization/data-settings). Every shortcut the grid answers to, and how to rebind it, is in [Keyboard Shortcuts](/features/keyboard-shortcuts). From 6a7f3d2f521bfdd6ceb417a0b445b5f56a250c19 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 23 Aug 2026 09:37:41 +0700 Subject: [PATCH 7/7] docs(data-grid): move the cell editor shortcuts to the page that owns them (#2381) --- .../Results/DataGridBodyChromeTests.swift | 23 -- .../Results/DataGridColumnPoolTests.swift | 272 ------------------ docs/features/data-grid.mdx | 2 +- docs/features/keyboard-shortcuts.mdx | 1 + 4 files changed, 2 insertions(+), 296 deletions(-) diff --git a/TableProTests/Views/Results/DataGridBodyChromeTests.swift b/TableProTests/Views/Results/DataGridBodyChromeTests.swift index 52422628d..c9e99ec3c 100644 --- a/TableProTests/Views/Results/DataGridBodyChromeTests.swift +++ b/TableProTests/Views/Results/DataGridBodyChromeTests.swift @@ -197,27 +197,4 @@ struct DataGridBodyChromeTests { #expect(sampledRed > sampledGreen + 0.3, "the separator has to carry the grid colour, not a fixed grey") } - /// Every data cell is drawn, so the row is what paints the separators crossing it; a separator - /// drawn by the table view underneath would be covered by the row's own background. - @Test("A row paints the separators crossing it") - func rowDrawsItsOwnSeparators() throws { - let grid = makeGrid(columns: ["id", "name"]) - let rowView = try #require(grid.tableView.rowView(atRow: 0, makeIfNecessary: true) as? DataGridRowView) - rowView.layoutSubtreeIfNeeded() - - let rep = try #require(rowView.bitmapImageRepForCachingDisplay(in: rowView.bounds)) - rowView.cacheDisplay(in: rowView.bounds, to: rep) - - let firstData = try #require(grid.coordinator.firstPresentedColumnIndex()) - let boundary = grid.tableView.rect(ofColumn: firstData).minX - let scale = CGFloat(rep.pixelsWide) / rowView.bounds.width - let onBoundary = try #require( - rep.colorAt(x: Int((boundary - 0.5) * scale), y: Int(rowView.bounds.height * scale / 2)) - ).usingColorSpace(.deviceRGB) - let expected = grid.tableView.gridColor.usingColorSpace(.deviceRGB) - - let drawn = try #require(onBoundary?.brightnessComponent) - let wanted = try #require(expected?.brightnessComponent) - #expect(abs(drawn - wanted) < 0.2, "the boundary pixel has to carry the grid colour") - } } diff --git a/TableProTests/Views/Results/DataGridColumnPoolTests.swift b/TableProTests/Views/Results/DataGridColumnPoolTests.swift index 22893f5a8..5424ff74c 100644 --- a/TableProTests/Views/Results/DataGridColumnPoolTests.swift +++ b/TableProTests/Views/Results/DataGridColumnPoolTests.swift @@ -698,276 +698,4 @@ struct DataGridColumnPoolTests { #expect(dataColumns(in: tableView).isEmpty) } - - // MARK: - Window geometry while scrolling - - private func scroll(_ scrollView: NSScrollView, to offsetX: CGFloat, tableView: NSTableView) { - scrollView.contentView.scroll(to: NSPoint(x: offsetX, y: scrollView.contentView.bounds.origin.y)) - scrollView.reflectScrolledClipView(scrollView.contentView) - tableView.layoutSubtreeIfNeeded() - } - - private func documentWidth(of tableView: NSTableView) -> CGFloat { - tableView.layoutSubtreeIfNeeded() - return tableView.frame.width - } - - /// How much of the viewport, past the row-number column, no mounted data column paints into. - /// - /// Measured through `rect(ofColumn:)`, which is `NSTableView`'s own answer for where a column - /// sits and is `NSZeroRect` for an unmounted one. Asking the resolver instead would only prove - /// the resolver agrees with itself, which is exactly how #2381 shipped. - private func unpaintedViewportWidth(in scrollView: NSScrollView, tableView: NSTableView) -> CGFloat { - let viewport = scrollView.contentView.bounds - let rowNumber = tableView.column(withIdentifier: ColumnIdentitySchema.rowNumberIdentifier) - let contentStart = rowNumber >= 0 - ? max(viewport.minX, tableView.rect(ofColumn: rowNumber).maxX) - : viewport.minX - - var painted: CGFloat = 0 - for column in dataColumns(in: tableView) where !column.isHidden { - let index = tableView.column(withIdentifier: column.identifier) - guard index >= 0 else { continue } - let rect = tableView.rect(ofColumn: index) - painted += max(0, min(rect.maxX, viewport.maxX) - max(rect.minX, contentStart)) - } - return max(0, (viewport.maxX - contentStart) - painted) - } - - private func mountedIdentifiers(in tableView: NSTableView) -> Set { - Set(dataColumns(in: tableView).filter { !$0.isHidden }.map(\.identifier)) - } - - private func scrollOffsets(in scrollView: NSScrollView, tableView: NSTableView) -> [CGFloat] { - let maximum = documentWidth(of: tableView) - scrollView.contentView.bounds.width - guard maximum > 0 else { return [0] } - let forward = Array(stride(from: 0, through: maximum, by: 150)) + [maximum] - return forward + forward.reversed() - } - - /// The reported bug. The window is resolved against a model of the whole column run, so the - /// viewport has to be rebased into that model before it can pick a range. Counting the leading - /// spacer as chrome subtracted the columns it stands in for a second time, and the window - /// walked left while the reader scrolled right until it painted nothing at all. - @Test("The mounted columns cover the viewport at every horizontal scroll offset") - func windowCoversTheViewportWhileScrolling() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - var worstGap: CGFloat = 0 - for offset in scrollOffsets(in: scrollView, tableView: tableView) { - scroll(scrollView, to: offset, tableView: tableView) - pool.applyColumnWindow(in: tableView) - tableView.layoutSubtreeIfNeeded() - worstGap = max(worstGap, unpaintedViewportWidth(in: scrollView, tableView: tableView)) - } - - #expect(worstGap == 0) - } - - /// A window that alternates between two ranges re-mounts columns on every scroll event, which - /// is what the reader sees as flicker. - @Test("Resolving again at the same scroll offset settles rather than alternating") - func windowSettlesAtOneOffset() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - scroll(scrollView, to: documentWidth(of: tableView) / 2, tableView: tableView) - pool.applyColumnWindow(in: tableView) - tableView.layoutSubtreeIfNeeded() - let settled = mountedIdentifiers(in: tableView) - - for _ in 0..<5 { - pool.applyColumnWindow(in: tableView) - tableView.layoutSubtreeIfNeeded() - } - - #expect(mountedIdentifiers(in: tableView) == settled) - } - - /// The spacers exist to keep the scroll extent, so no window position may change it. - @Test("The document keeps its width at every window position") - func documentWidthSurvivesEveryWindowPosition() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - let expected = documentWidth(of: tableView) - - for offset in scrollOffsets(in: scrollView, tableView: tableView) { - scroll(scrollView, to: offset, tableView: tableView) - pool.applyColumnWindow(in: tableView) - #expect(documentWidth(of: tableView) == expected) - } - } - - @Test("Scrolled to the end, the last column is mounted") - func lastColumnIsMountedAtTheEnd() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - scroll( - scrollView, - to: documentWidth(of: tableView) - scrollView.contentView.bounds.width, - tableView: tableView - ) - pool.applyColumnWindow(in: tableView) - - let last = try #require(dataColumns(in: tableView).last) - #expect(!last.isHidden) - } - - // MARK: - Reaching a column the window left out - - /// `rect(ofColumn:)` and `frameOfCell(atColumn:row:)` are both empty for an unmounted column, - /// so Find scrolled to the document origin instead of the match and the inline editor opened - /// nothing at all. - @Test("A column the window left out can be mounted on demand") - func mountColumnReachesAnUnmountedColumn() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - - let last = try #require(dataColumns(in: tableView).last) - #expect(last.isHidden) - - pool.mountColumn(last, in: tableView) - tableView.layoutSubtreeIfNeeded() - - #expect(!last.isHidden) - #expect(tableView.rect(ofColumn: tableView.column(withIdentifier: last.identifier)).width > 0) - } - - /// Stretching the window out to reach a far column mounts every column in between, which is the - /// cost the window exists to avoid: measured at 848ms and 3,081 cell views for one match 90 - /// columns away, and 4.8s at 500 columns. - @Test("Mounting a far column does not mount everything in between") - func mountColumnStaysBounded() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - let mountedBefore = mountedIdentifiers(in: tableView).count - - let last = try #require(dataColumns(in: tableView).last) - pool.mountColumn(last, in: tableView) - tableView.layoutSubtreeIfNeeded() - - #expect(!last.isHidden) - #expect(mountedIdentifiers(in: tableView).count <= mountedBefore) - } - - @Test("Mounting a far column keeps the document width") - func mountColumnKeepsTheDocumentWidth() throws { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - let expected = documentWidth(of: tableView) - - let last = try #require(dataColumns(in: tableView).last) - pool.mountColumn(last, in: tableView) - - #expect(documentWidth(of: tableView) == expected) - } - - @Test("A column the user hid is never mounted on demand") - func mountColumnRefusesAUserHiddenColumn() throws { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100, hidden: ["c99"]) - - let hidden = try #require(dataColumns(in: tableView).last) - pool.mountColumn(hidden, in: tableView) - - #expect(hidden.isHidden) - } - - // MARK: - Naming the ends of the data run - - /// The window's spacers are attached columns too, and the leading one sits immediately before - /// the first data column, so a fixed position names a spacer rather than data. - @Test("The first and last presented columns are data columns, not spacers") - func presentedEndsSkipTheSpacers() throws { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - - let first = try #require(pool.firstPresentedColumnIndex(in: tableView)) - let last = try #require(pool.lastPresentedColumnIndex(in: tableView)) - - #expect(!ColumnIdentitySchema.isSpacer(tableView.tableColumns[first].identifier)) - #expect(!ColumnIdentitySchema.isSpacer(tableView.tableColumns[last].identifier)) - #expect(tableView.tableColumns[first].identifier == dataColumns(in: tableView).first?.identifier) - #expect(tableView.tableColumns[last].identifier == dataColumns(in: tableView).last?.identifier) - } - - @Test("Walking forward and back from an end stays inside the data run") - func presentedNeighboursStayInsideTheDataRun() throws { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 20) - - let first = try #require(pool.firstPresentedColumnIndex(in: tableView)) - let last = try #require(pool.lastPresentedColumnIndex(in: tableView)) - - #expect(pool.previousPresentedColumnIndex(before: first, in: tableView) == nil) - #expect(pool.nextPresentedColumnIndex(after: last, in: tableView) == nil) - #expect(pool.nextPresentedColumnIndex(after: first, in: tableView) != nil) - #expect(pool.previousPresentedColumnIndex(before: last, in: tableView) != nil) - } - - /// The create-table grid opens with no columns at all, where every attached column is chrome. - @Test("A result with no columns presents no column at either end") - func emptyResultHasNoPresentedEnds() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 0) - - #expect(pool.firstPresentedColumnIndex(in: tableView) == nil) - #expect(pool.lastPresentedColumnIndex(in: tableView) == nil) - } - - @Test("A single-column result presents that column at both ends") - func singleColumnResultHasOneEnd() { - let pool = DataGridColumnPool() - let (_, tableView) = makeScrolledTableView(viewportWidth: 800) - - reconcileWide(pool, tableView: tableView, count: 1) - - #expect(pool.firstPresentedColumnIndex(in: tableView) == pool.lastPresentedColumnIndex(in: tableView)) - #expect(pool.firstPresentedColumnIndex(in: tableView) != nil) - } - - /// Size All Columns to Fit reaches the columns the window unmounted as well, so the spacers - /// stand in at the width those columns used to have and the document ends up short. - @Test("Resizing unmounted columns restores the full document width") - func widthChangeOutsideTheWindowRestoresDocumentWidth() { - let pool = DataGridColumnPool() - let (scrollView, tableView) = makeScrolledTableView(viewportWidth: 800) - reconcileWide(pool, tableView: tableView, count: 100) - scroll(scrollView, to: 0, tableView: tableView) - pool.applyColumnWindow(in: tableView) - - for column in dataColumns(in: tableView) { - column.width = 300 - } - pool.invalidateColumnWindow() - pool.applyColumnWindow(in: tableView) - - let gap = tableView.intercellSpacing.width - let everyColumnSlot = dataColumns(in: tableView).reduce(0) { $0 + $1.width + gap } - let occupied = tableView.tableColumns - .filter { !$0.isHidden && $0.identifier != ColumnIdentitySchema.rowNumberIdentifier } - .reduce(0) { $0 + $1.width + gap } - - #expect(occupied == everyColumnSlot) - } } diff --git a/docs/features/data-grid.mdx b/docs/features/data-grid.mdx index 3b7d925d6..42a8606d0 100644 --- a/docs/features/data-grid.mdx +++ b/docs/features/data-grid.mdx @@ -116,6 +116,6 @@ A chart draws the loaded rows, up to 2,000 points, 20 series, and 50,000 inspect ## Editing and display -Double-click a cell to edit it, or press `Return` on the focused cell. The inline editor keeps a value on one line and scrolls it, the way a text field does, rather than wrapping it into the cell's width. `Option+Return` inserts a line break, `Return` commits, `Escape` cancels, and `Tab` commits and moves to the next cell. Nothing reaches the database until you save, and [Change Tracking](/features/change-tracking) covers the type-specific editors, the row operations, and what a Save runs. +Double-click a cell to edit it. A long value stays on one line and scrolls sideways rather than wrapping into the column's width. Nothing reaches the database until you save, and [Change Tracking](/features/change-tracking) covers the type-specific editors, the row operations, and what a Save runs. NULL renders as styled `NULL` text. That text, the date format, row height, row numbers, and alternate row backgrounds are [data settings](/customization/data-settings). Every shortcut the grid answers to, and how to rebind it, is in [Keyboard Shortcuts](/features/keyboard-shortcuts). diff --git a/docs/features/keyboard-shortcuts.mdx b/docs/features/keyboard-shortcuts.mdx index 4c59a02d1..24ae67bb2 100644 --- a/docs/features/keyboard-shortcuts.mdx +++ b/docs/features/keyboard-shortcuts.mdx @@ -100,6 +100,7 @@ Every row except Find Next and Find Previous is built into the editor and cannot | Action | Shortcut | |--------|----------| | Edit cell | `Enter` | +| Insert a line break while editing | `Option+Enter` | | Cancel edit | `Escape` | | Add row | `Cmd+Shift+N` | | Duplicate row | `Cmd+Shift+D` |