fix(datagrid): cap the width of a column sized to fit its content#1859
Merged
Conversation
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Running Size All Columns to Fit (or Size to Fit, or double-clicking a column divider) on a table with a long TEXT/JSON value stretched that column across the screen.
DataGridCellFactoryhad three near-identical width calculators. Two clamped to an 800pt ceiling;calculateFitToContentWidthwas a copy that had lost both bounds, so it measured the full length of a cell's text and returned it unclamped. A 4,000 character value produced a column thousands of points wide. All three fit entry points call that one function.Second half of the bug: a programmatic
column.width = xposts the samecolumnDidResizeNotificationa drag does, so the oversized width was captured into the saved column layout and restored unclamped on every later open. Capping the calculation alone would leave an already stretched column stretched after a restart.Fix
min()into the copy.clamp(0.5 x visible grid width, 300, 800). A fitted column never takes more than half the visible grid, never drops below 300pt in a narrow window, and never exceeds the 800pt ceiling the initial layout path already applied, so first load and explicit "Size to Fit" stop disagreeing.NSTableColumn.maxWidth = 1200. AppKit documents this as a ceiling "whether the column is resized by the user or programmatically", so it clamps drags, auto-fit, and widths restored from disk in one place. That is what corrects a layout already saved with an oversized width: it is clamped on open and rewritten on the next capture.calculateColumnWidth, which no production code called.Prior art
Sequel Ace (same AppKit hook) hard caps auto-fit at 400pt and truncates the measured string at 500 chars. Excel caps at 255 characters. TablePlus has no cap: TablePlus/TablePlus#1461 is this same bug, and their answer was a "reset column widths" command rather than a limit.
Tests
calculateFitToContentWidthhad no coverage at all, which is how this shipped.FitToContentWidthTests: long value caps at 800, half-viewport rule, 300pt floor in a narrow window, short values still size to content, long header alone, fitted width never exceeds the column ceiling.DataGridColumnPoolTests: an oversized saved width is clamped to the ceiling on restore; data columns carry the min and max bounds.All pass, along with the existing column width and column pool suites.