Skip to content

fix(datagrid): draw the header chrome to fit a comment line - #2017

Open
datlechin wants to merge 2 commits into
mainfrom
fix/datagrid-header-comment-separator
Open

fix(datagrid): draw the header chrome to fit a comment line#2017
datlechin wants to merge 2 commits into
mainfrom
fix/datagrid-header-comment-separator

Conversation

@datlechin

Copy link
Copy Markdown
Member

Problem

With Show object comments on, the comment under each data grid column name is crossed by a horizontal line, and there is an empty strip between that line and the first row.

Root cause

NSTableHeaderCell.draw(withFrame:in:) paints its chrome as a fixed 28pt band that it centres vertically in whatever frame it is given: the opaque background, a 16pt column divider tick on midY, and a 1pt bottom separator at midY + 13. Outside that band it paints nothing.

TablePro grows the header to 42pt (28 natural plus 14 for the comment line). That arithmetic is correct and is unchanged here. AppKit's chrome cannot follow the taller frame, so at 42pt:

  • the separator lands at 34, inside the comment's line box (22 to 35), and runs through its descenders
  • 35 to 42 is left unpainted, which is the empty strip

Measured on a headless harness across header heights 28, 30, 34, 36, 42, 50 and 64: the separator is always at midY + 13. Header font size, controlSize, rowSizeStyle, and all four NSTableView.style values leave the band at exactly 28pt. Overriding draw(withFrame:in:) is the only way to suppress it.

Second bug, same cause. AppKit hands drawInterior(withFrame:in:) a 16pt sub-rect (0,13,397,16), not the cell frame, so isColumnSelected's cellFrame.fill() painted a band across the middle of the header instead of filling it. That was wrong at the natural 28pt height too.

Fix

  • SortableHeaderChrome is the single owner of the header chrome geometry and colours, so the measure path and the draw path cannot drift apart. DBeaver ships this same empty strip because its height measurement and its painting read different margin constants.
  • SortableHeaderCell.draw(withFrame:in:) no longer calls super, which stops the mis-centred band. It draws the column divider, then passes the full cell frame to drawInterior.
  • SortableHeaderView.draw(_:) fills the background, calls super so AppKit still drives cell drawing and column drag-reorder, then rules the separator on the real bottom edge across the full width.

The header background is now ours, because NSColor.headerColor is deprecated and returns white in dark mode. windowBackgroundColor matched stock exactly in a light-mode render and is within about 2/255 in dark.

Verification

check result
swiftlint lint --strict, project wide clean
header and data grid suites all pass
negative control, overrides removed 4 of 5 new tests fail, parity guard passes
full TableProTests 72 failures against 73 on clean main, no new failures

The negative control is the part that matters: the four bug-targeting tests genuinely fail against the old code. naturalHeaderRuleSitsAlongBottomEdge passes both ways by design, which is what proves the no-comments header is untouched.

The 72 remaining failures are pre-existing and unrelated (SSH host keys, MCP, Oracle, Etcd, AWS SSO). I ran the full suite on a clean tree to confirm rather than assume.

No TableProUITests coverage: XCUITest cannot assert the pixel geometry of an NSTableHeaderCell, so the regression tests rasterise the header with bitmapImageRepForCachingDisplay and cacheDisplay and assert on the rendered rules instead.

Left out on purpose

NSTableView.cornerView has the identical defect. I built and tested a fix and then reverted it: with overlay scrollers, the macOS default, AppKit creates no corner view at all, so installing one adds an 18pt view over the header's right edge, exactly where the last column's sort and filter icons sit. Doing it safely needs runtime observation of the scroller style preference plus hit-test suppression, which is out of proportion to a strip only "Always show scroll bars" users ever see.

Worth a look when reviewing

  • the header background in a live window, since the harness has no window material
  • dragging a column to reorder it, which I could not exercise headlessly

@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.

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