Skip to content

fix: align data grid header padding#1040

Merged
datlechin merged 1 commit intoTableProApp:mainfrom
overtrue-forks:feat/data-grid-header-padding
May 6, 2026
Merged

fix: align data grid header padding#1040
datlechin merged 1 commit intoTableProApp:mainfrom
overtrue-forks:feat/data-grid-header-padding

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

@overtrue overtrue commented May 6, 2026

Summary

  • Add the same 4pt horizontal inset to data grid column headers that result cells use.
  • Render the row-number # header with the same header cell and right alignment as row-number cells.
  • Add coverage for SortableHeaderCell.titleRect(forBounds:).

before

Clipboard_Screenshot_1778067344

after

Clipboard_Screenshot_1778067352

Root Cause

Data cells applied 4pt leading and trailing constraints, but unsorted headers used the default NSTableHeaderCell interior drawing. That left header titles flush against grid edges, and the row-number header used a different cell path from the row-number content cells.

Validation

  • xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation -only-testing:TableProTests/SortableHeaderCellTests CODE_SIGNING_ALLOWED=NO
  • swiftlint lint --strict --config .swiftlint.yml using the SwiftLint plugin binary
  • xcodebuild -project TablePro.xcodeproj -scheme TablePro -configuration Debug build -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO

@overtrue overtrue changed the title [codex] fix data grid header padding fix: align data grid header padding May 6, 2026
@overtrue overtrue force-pushed the feat/data-grid-header-padding branch from 34d657e to 636e8ec Compare May 6, 2026 11:38
@overtrue overtrue marked this pull request as ready for review May 6, 2026 11:38
Copilot AI review requested due to automatic review settings May 6, 2026 11:38
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a visual inconsistency in the Results data grid by ensuring table column headers use the same horizontal padding as result cells, and by rendering the row-number # header using the same custom header cell path as other headers (including right alignment). It also adds targeted unit coverage for the header title inset logic.

Changes:

  • Replace default unsorted header interior drawing with custom title drawing that applies consistent horizontal insets.
  • Use SortableHeaderCell for the row-number (#) header and align it to the right to match row-number cells.
  • Add tests validating SortableHeaderCell.titleRect(forBounds:) padding behavior and clamping for narrow widths.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
TableProTests/Views/Results/SortableHeaderCellTests.swift Adds tests verifying header title rect padding and non-negative width behavior.
TablePro/Views/Results/SortableHeaderCell.swift Implements padded titleRect(forBounds:) and unifies title drawing for sorted/unsorted headers.
TablePro/Views/Results/DataGridView.swift Switches the row-number column header to use SortableHeaderCell with right alignment.
CHANGELOG.md Documents the header padding fix in the “Fixed” section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member

@datlechin datlechin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you

@datlechin datlechin merged commit b31768f into TableProApp:main May 6, 2026
4 of 5 checks passed
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.

3 participants