diff --git a/CHANGELOG.md b/CHANGELOG.md index d690c45eb..e96868c4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,13 +7,21 @@ 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) +- 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. - 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 db510997a..4a014735f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -208,7 +208,9 @@ 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 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/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..2da88e804 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() @@ -106,6 +108,28 @@ 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). + /// + /// `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, + height: CGFloat.greatestFiniteMagnitude + ) + textView.isVerticallyResizable = true + textView.isHorizontallyResizable = true + 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] 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/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.. 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/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift index 8b517b9c9..f44210f28 100644 --- a/TablePro/Views/Results/DataGridColumnPool.swift +++ b/TablePro/Views/Results/DataGridColumnPool.swift @@ -16,28 +16,9 @@ final class DataGridColumnPool { /// Slots the current result actually uses. The pool only grows, so the surplus slots past the /// result's column count stay attached and hidden, and windowing must never mount one. private var activeIdentifiers: Set = [] - 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..70a8f40c2 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,55 @@ 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 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). + /// + /// 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 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)) + } + 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, + 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? @@ -479,10 +517,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() @@ -980,7 +983,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) @@ -1006,22 +1009,17 @@ 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) 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) @@ -1168,7 +1158,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..88f78b04d 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -20,11 +20,202 @@ 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. + /// + /// 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 + } + 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) + ) + } + } + + /// 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) { @@ -78,19 +269,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 +769,35 @@ 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 } + + /// 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) { + 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/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 ad409a4cc..0d89b7751 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -10,9 +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 } - guard tableView.view(atColumn: tableColumn, row: row, makeIfNecessary: false) != nil else { return } + guard presentsCell(row: row, tableColumnIndex: tableColumn) else { return } switch CellInteractionResolver().resolve(context) { case .blocked: @@ -128,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+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+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/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..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 } @@ -77,7 +97,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 +517,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 +552,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/CellOverlayTextLayoutTests.swift b/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift new file mode 100644 index 000000000..470f93832 --- /dev/null +++ b/TableProTests/Views/Results/CellOverlayTextLayoutTests.swift @@ -0,0 +1,68 @@ +// +// 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() 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) + 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() throws { + let textView = makeTextView() + let layoutManager = try #require(textView.textLayoutManager) + textView.string = "42" + layoutManager.ensureLayout(for: layoutManager.documentRange) + + #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/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/DataGridBodyChromeTests.swift b/TableProTests/Views/Results/DataGridBodyChromeTests.swift new file mode 100644 index 000000000..c9e99ec3c --- /dev/null +++ b/TableProTests/Views/Results/DataGridBodyChromeTests.swift @@ -0,0 +1,200 @@ +// +// 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 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 + /// 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)) + } + + /// 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") + } + +} 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/DrawnCellReachabilityTests.swift b/TableProTests/Views/Results/DrawnCellReachabilityTests.swift new file mode 100644 index 000000000..24bfdd106 --- /dev/null +++ b/TableProTests/Views/Results/DrawnCellReachabilityTests.swift @@ -0,0 +1,167 @@ +// +// 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 { + /// 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, + 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 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 = makeGrid(columns: ["id", "name"]).coordinator + 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 = makeGrid(columns: ["id", "name"]).coordinator + 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 = makeGrid(columns: ["id", "name"]).coordinator + 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 = makeGrid(columns: ["id", "name"]).coordinator + 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:)` 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) + let before = (rowView.accessibilityChildren()?.first as? NSAccessibilityElement)?.accessibilityValue() as? String + #expect(before?.contains("before") == true) + + 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) + + let after = (rowView.accessibilityChildren()?.first as? NSAccessibilityElement)?.accessibilityValue() as? String + #expect(after == "after", "the element still held the pre-edit value") + } +} 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]) } } diff --git a/docs/features/data-grid.mdx b/docs/features/data-grid.mdx index b7729c745..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. 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` |