Skip to content

fix(datagrid): show column comments in headers on first load (#1861)#1863

Merged
datlechin merged 1 commit into
mainfrom
fix/1861-header-comment-first-load
Jul 13, 2026
Merged

fix(datagrid): show column comments in headers on first load (#1861)#1863
datlechin merged 1 commit into
mainfrom
fix/1861-header-comment-first-load

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #1861.

Column comments were missing from the data grid header until the query was run again, and once they appeared the header was too short, so the comment was cut off until a column was resized.

That one report turned out to be three defects in the header comment feature added in #1842.

1. Comments never reached the header on first load

Column comments are not part of the row result. They arrive from the background schema fetch (phase 2), which mutates TableRows.columnComments in place. The grid rebuild gate, DataGridUpdateSnapshot, had no field derived from comment content, only the global showObjectComments toggle. So the snapshot compared equal, reconcileColumnPool was skipped, and that is the only code that writes headerComment.

Re-running the query worked by accident: configureForTable bumps reloadVersion, which trips the gate for an unrelated reason.

The snapshot now carries the effective columnComments (setting applied), which subsumes the old bool and makes the gate reflect what the header actually draws.

2. The header height never applied

applyHeaderHeight() called enclosingScrollView?.tile(), and enclosingScrollView is always nil on an NSTableHeaderView: the header lives in the scroll view's header clip view, not its contentView, and NSView.enclosingScrollView only resolves through the content clip. The header view grew to 40pt while the scroll view's header region stayed at 28pt, which is what clipped the comment.

Only an NSScrollView tiling pass picks up header height. NSTableView.tile() does not, despite what its documentation implies. A column resize drag happens to make the scroll view re-tile, which is why dragging appeared to fix it.

The tile now goes through tableView?.enclosingScrollView.

Two follow-ons from that:

  • The setFrameSize clamp, added with a comment claiming tile() would otherwise shrink the header back, was a workaround for this misdiagnosis. The height survives tableView.tile(), scroll view resize, column width changes, addTableColumn, reloadData, and layout passes without it, so it is removed.
  • commentHeaderHeight is no longer a hardcoded 40. It is the natural header height plus the measured comment line height. AppKit's default header height has changed twice (17 to 23 to 28), so the compact baseline should not be assumed.

3. The first reconcile ran too early

In makeNSView, reconcileColumnPool ran before the SortableHeaderView was installed and before the table view joined the scroll view, so the first pass cast the header to nil and could never apply a height. The header and documentView are now set up before the initial reconcile.

Also: an edited comment now repaints the header. Mutating a header cell's content does not set needsDisplay, so a comment change at an unchanged header height would otherwise keep drawing the old text.

Tests

  • SortableHeaderViewTests (new): with a real NSScrollView and NSTableView, asserts that showing comments grows both the header view and the scroll view's header clip, that hiding restores the platform height, and that a later resize/reload keeps the grown header. The clip assertion is the point: the old code already set the header frame correctly, so the existing test (a bare NSTableView with no scroll view) passed while the feature was visibly broken.
  • DataGridUpdateSnapshotTests: comments arriving after the rows, and an edited comment, must both change the snapshot.
  • DataGridColumnPoolTests: comments arriving on a second reconcile (the phase 2 case) grow the header; an edited comment reaches the cell and tooltip at an unchanged height.

28 tests pass, swiftlint lint --strict is clean.

Notes

The two-line header is a database client convention rather than an Apple one; the HIG's mechanism for supplementary column information is headerToolTip, which this feature already sets alongside the inline line. Mature clients converge on the tooltip, and the inline comment line is a long-standing open request in DataGrip and DBeaver, so keeping it is the right call. The header still only grows when a visible column actually has a comment, so tables without comments cost no vertical space.

@datlechin datlechin merged commit ca9d1dd into main Jul 13, 2026
2 checks passed
@datlechin datlechin deleted the fix/1861-header-comment-first-load branch July 13, 2026 12:08
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.

Table Header Field Tooltip Display Issue

1 participant