Skip to content

refactor(datagrid): direct-draw NSView cell, subview-based overlay editor#1135

Merged
datlechin merged 3 commits intomainfrom
refactor/datagrid-cell-direct-draw
May 9, 2026
Merged

refactor(datagrid): direct-draw NSView cell, subview-based overlay editor#1135
datlechin merged 3 commits intomainfrom
refactor/datagrid-cell-direct-draw

Conversation

@datlechin
Copy link
Copy Markdown
Member

Summary

Fixes the 97% CPU spike during sustained scroll over the data grid. Replaces the NSTableCellView + NSTextField + NSButton + Auto Layout cell with a direct-draw NSView, and replaces the NSPanel-based overlay editor with a subview of the table view so the parent window stays key during edit (toolbar/buttons keep their accent color).

What changed

DataGridCellView: NSTableCellViewNSView direct-draw

  • No subviews, no constraints, no NSTextField/NSButton per cell
  • draw(_:) paints text via cached NSAttributedString (invalidated only on text/font/color change), then accessory icon (FK arrow or chevron), then optional focus border
  • Per-cell-during-scroll cost: zero Auto Layout solving, zero NSCell internal layout, zero NSButton tracking-area work
  • Cell text uses NSColor.alternateSelectedControlTextColor (white) when on emphasized selection, falling back to .labelColor otherwise. Matches macOS HIG.
  • Accessory SF Symbols use NSImage.SymbolConfiguration(hierarchicalColor:) (correct API for multi-layer symbols like arrow.right.circle.fill); two cached variants per icon (normal + emphasized)
  • Right-click walks up to the row view's menu(for:) for context menus
  • Accessibility role/label/index range preserved

DataGridRowView: emphasis propagation to cell subviews

  • Overrides isSelected and isEmphasized setters to call setNeedsDisplay(true) on every DataGridCellView subview when emphasis changes. NSTableCellView got this for free via interiorBackgroundStyle inheritance from NSCell binding; custom NSView cells need explicit propagation. Without this, only the focused cell would flip text color on row select; sibling cells stayed black.

CellOverlayEditor: NSPanel → subview of the table view

  • The editor (NSScrollView containing NSTextView with isFieldEditor = true) is now added as a subview of the table view in the same window
  • Window stays key throughout the edit → toolbar/button accent colors don't desaturate
  • Outside-click dismiss via NSEvent.addLocalMonitorForEvents
  • App-deactivate dismiss via NSApplication.didResignActiveNotification
  • Dismiss-on-scroll and dismiss-on-column-resize observers preserved
  • Same keyboard policy: Return commits, Option+Return inserts newline, Esc cancels, Tab/Shift+Tab navigates between cells

Editing path consolidation

  • tableView(_:shouldEdit:row:) is now a no-op (AppKit doesn't invoke it for custom NSView cells anyway)
  • New beginCellEdit(row:tableColumnIndex:) on TableViewCoordinator is the single entry point — called from double-click, single-click-on-focused, Return key, programmatic beginEditing. Routes to editEligibility then showOverlayEditor.
  • All edits route through the overlay editor (previously only multi-line did)
  • Deleted CellTextField.swift (83 LoC), DataGridFieldEditor, the windowWillReturnFieldEditor branch on TabWindowController, the NSControlTextEditingDelegate / NSTextFieldDelegate conformances on TableViewCoordinator, and the restoreTruncatedDisplay glue

Resize cursor fix

  • Stage 9b-revised in refactor(datagrid): drop redundant resize cursor and consolidate sort dispatch #1131 incorrectly assumed NSTableHeaderView auto-handles the resize cursor when column.resizingMask.contains(.userResizingMask). That auto-mechanism doesn't fire for subclassed NSTableHeaderView instances on macOS 14+. Restored the updateTrackingAreas + mouseMoved cursor handling in SortableHeaderView. (Tested: resetCursorRects alone doesn't fire reliably for our subclass; mouseMoved does.)

Diff stats

  • 12 files changed, ~424 insertions, ~557 deletions
  • 1 file deleted (CellTextField.swift)

Test plan

  • Open a 100k+ row table (e.g. seed stress_data from docs/test/sample_postgres_02_data.sql). Sustained scroll (hold arrow, drag scrollbar). CPU should stay reasonable; no 97% spike.
  • Click a row. ALL cells in the selected row show white text on accent-blue background. Navigate up/down: text color flips correctly across all cells, not just the focused one.
  • FK arrow icon visible against both emphasized and non-emphasized backgrounds (no empty/inverted circles).
  • Double-click a plain-text cell → overlay editor appears anchored to the cell. Toolbar buttons (Filters, Apply, Data segment) retain their accent color while editing — no desaturation.
  • Edit, press Return → commit. Press Esc → cancel. Tab → next cell. Shift+Tab → previous cell. Cmd+S during edit → commits and triggers menu Save.
  • Open a multi-line cell (newlines in value). Overlay shows multi-line content with vertical scroll. Option+Return inserts newline.
  • Right-click a cell → row context menu appears.
  • Hover column dividers in the header → resize cursor (↔) appears reliably.
  • Drag column edge to resize → live resize, cursor consistent.
  • Scroll table while overlay editor is open → editor dismisses (commits) on scroll.
  • Resize a column while overlay editor is open → editor dismisses (cancels) on column resize.
  • Cmd+Tab to another app, come back → overlay dismisses (commits) on app deactivate.
  • Modified-column tint, deleted-row strikethrough, focus border on emphasized selection — all preserved.
  • VoiceOver: cell announces "Row N, column M: value" with correct role.

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin datlechin merged commit 1a19531 into main May 9, 2026
2 checks passed
@datlechin datlechin deleted the refactor/datagrid-cell-direct-draw branch May 9, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant