fix(tablev2): keep the sort caret out of the header's intrinsic width - #1180
Merged
Conversation
… (CUI) 0.226.0 gave the sort caret a real 20px flex item so the hover-reveal `SortIncentive` would stop overlapping the label. That fixed the overlap but lifted every sortable header's min-content 20px above the matching body cell's, so as soon as that floor binds the header row and the body row stop agreeing on column widths — and right-aligned headers sit ~20px short of their values. The caret is now a zero-width flex item (`SortCaretWrapper`) holding an out-of-flow `SortCaretGlyph`, and `HeaderContent` reserves the room with `padding-inline-*` instead. Being a real flex item is what keeps the glyph beside the label; being zero-width is what keeps it out of the sizing equation. Anchoring the glyph to `HeaderContent`'s edge was tried first and stranded it at the far side of a wide column. Which side gets the padding follows the alignment: end-aligned columns reserve the start side and move the caret there with `order: -1`, since the label has to stay flush with the trailing edge to line up with the values below it; centred columns reserve both sides so the label stays centred; everything else reserves the end side. Non-sortable columns reserve nothing. MUI resolves this the same way — `TableCell` sets `flex-direction: row-reverse` for `align: 'right'` only, and `TableSortLabel` inherits it while rendering label-then-icon. Headers also get `min-width: 0` plus an ellipsizing `HeaderLabel`, so a long header truncates rather than becoming the reason a column is wider than its cells. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jsdom does not lay out, so the widths and alignments this guards can only be checked in a real browser. Three sections: two identical-flex columns (one sortable, one not) in a 220px container, which is narrow enough that the caret's floor actually binds; a certificate table with a centred and a right-aligned column; and the same columns through `MultiSelectableContent`. Note the container width matters — at 400px the floor never binds and a broken build measures as passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Hello jeanmarcmilletscality,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
JBWatenbergScality
approved these changes
Aug 11, 2026
Contributor
Author
|
/approve |
Contributor
|
I have successfully merged the changeset of this pull request
Please check the status of the associated issue None. Goodbye jeanmarcmilletscality. The following options are set: approve |
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.
TL;DR — Sortable
Tablev2headers have been 20px wider than they should be since 0.226.0, which knocks the header row out of alignment with the body rows below it. The sort caret no longer takes part in that width calculation.Context / Why
#1168 gave the sort caret a real, in-flow width so the hover-reveal caret would stop overlapping the header label. It fixed the overlap, but it also made every sortable header 20px wider at its narrowest — so header columns and body columns stop agreeing on widths, and right-aligned headers sit short of the values underneath them. Reported on the ARTESCA Certificates page, where the
Expire Onheader visibly fails to line up with its dates.🧩 Approach
The caret is still an in-flow flex item — it has to be, or it can't sit next to the label — but it is now zero-width, and
HeaderContentreserves the room it needs with padding instead.Before —
min-width: 16px+margin-left: 4pxmeans the caret contributes 20px to the header'smin-content, which the body cell below it doesn't have:After — a zero-width anchor holding an out-of-flow glyph, so the caret contributes nothing to
min-content:Which side
HeaderContentreserves follows the column's alignment, because the label has to stay flush with whichever edge its values are flush with:textAlignright/endpadding-inline-startorder: -1)centerpadding-inline(both)leftpadding-inline-endThe caret-before-label case is the one that looks unusual, so worth noting it's the convention: MUI's
TableCellsetsflex-direction: row-reverseforalign: 'right'only — no other alignment touches flex-direction — andTableSortLabelinherits it while rendering label-then-icon, with the same 4px gutter. Their issue #21085 asking for an explicit position prop was closed not planned. We useorder: -1rather thanrow-reversebecauseHeaderContentcomputesjustify-contentexplicitly androw-reversewould invert whatflex-start/flex-endmean.Verified in Chrome at 1280×900 against a reverted baseline (jsdom doesn't lay out, so this can't be a Jest assertion):
Headers also get
min-width: 0and an ellipsizingHeaderLabel, so a long header truncates rather than becoming the reason a column outgrows its cells.This does not fully fix the reported Certificates symptom. Only ~17px of that ~160px gap was the caret. The rest is a separate, pre-existing defect: body cells ignore
textAlignentirely, because the default stringCellwraps values inConstrainedText, whose container isdisplay: -webkit-boxand doesn't honourtext-align. That needs its own ticket and its own PR.📷 Screenshots
🔍 Review focus
src/lib/components/tablev2/Tablestyle.tsx › HeaderContent— the alignment→padding mapping and theorder: -1flip apply to everyTablev2header. Worth checking the centred case in particular: it reserves 20px on both sides unconditionally, which costs a centred sortable column 40px of label space whether or not it's currently sorted.src/lib/components/tablev2/Tablestyle.tsx › HeaderLabel,TableHeader—min-width: 0plustext-overflow: ellipsisis a behaviour change for callers who pass no new prop: a header that used to force its column wider now truncates instead. Intended, but it's the part most likely to surprise a consumer.src/lib/components/tablev2/MultiSelectableContent.tsx—HeaderLabelis deliberately not applied to theselectionbranch, so the checkbox keeps its click-handlingdivwithout inheriting the ellipsis clipping.🧪 How to test
npm run storybook, then open Components / Data Display / Table → Sort Caret Header Alignment.Temperaturecolumns are identically sized and differ only indisableSortBy. Both headers should be exactly as wide as, and start at the same x as, the cells beneath them. Before this change the sortable one was 6px wider.Expire Onheader text should end flush with the column's right content edge, with the caret to its left. (The dates below it will still be left-aligned — that's the separateConstrainedTextdefect above, not this change.) The centredStatusheader should stay centred.MultiSelectableContent. Note the checkbox column still causes a 12–16px header/body disagreement; that is pre-existing and unchanged by this PR.Tablev2→ the standard examples) — headers should look untouched.🔗 References
What changed
Tablestyle.tsxcarries the fix:SortCaretWrapperbecomes a zero-width anchor, the painted glyph moves into a new out-of-flowSortCaretGlyph,HeaderContentgains a$sortableflag and the alignment-driven padding, and a newHeaderLabelhandles truncation.SingleSelectableContent.tsxandMultiSelectableContent.tsxonly pass$sortableand wrap the rendered header inHeaderLabel.stories/tablev2.stories.tsxadds the repro story used for the measurements above. The container widths in it matter: at 400px the caret's 20px floor never binds, so a broken build measures as passing — section (A) uses 220px deliberately.No public API change: no new props, no changed prop types or defaults.