bugfix(tablev2): keep rows, header tracks and header labels honest - #1192
Open
JeanMarcMilletScality wants to merge 11 commits into
Open
bugfix(tablev2): keep rows, header tracks and header labels honest#1192JeanMarcMilletScality wants to merge 11 commits into
JeanMarcMilletScality wants to merge 11 commits into
Conversation
Three defects in the same component, all measured rather than reasoned about. **A row's handler swallowed clicks meant for a control inside a cell.** The row `onClick` in both selectable contents fired for any click anywhere in the row with no target check, so clicking a button in a cell both activated the button and selected the row — and the selection re-render remounts the memoized row, unmounting whatever the button had just opened. The `+N` dropped-columns trigger is core-ui hitting itself with this. `onKeyDown` had it worse: `keydown` bubbles too, so Enter on a focused in-cell control selected the row *and* `preventDefault()`ed the control's own activation. Both handlers now bail on an interactive target, via one `isInteractiveTarget` in `TableCommon` so the two copies cannot drift. The multi-selectable selection cell no longer relies on the bubble the guard stops: it owns its click outright. That is behaviour-preserving — its two former paths, direct in single-row mode and bubbling to the row handler's else branch otherwise, ended in the same call with the same arguments. **A multi-selectable table's header tracks disagreed with its body tracks.** `TableRowMultiSelectable` never declared the `gap` that `HeadRow` and `TableRow` both do, so it computed `gap: normal`. Every column track has a grow factor and no basis, which turns the gap the header reserves and the body does not into free space the body redistributes — shifting each boundary by its grow share of the total gap, at any width. Measured on a three-column table with grow factors summing to 2.5: the header's three 14px gaps moved the columns by 25.20 / 8.41 / 8.40px, exactly 0.6 / 0.2 / 0.2 of 42px. One declaration; all deltas now zero. This also clears the two suspects that were on the list. The scrollbar compensation is fine — a single-selectable table with an identical 11px scrollbar aligned perfectly both before and after — and the header does receive `cellStyle`. **An ellipsized header gave no way to read the full label.** Body cells recover via `ConstrainedText`; the header was the one place a label became unreadable with no way back. `TruncatableHeaderLabel` measures the label and offers `title` only once it is actually cut off, re-measuring on resize because these columns size from grow factors. Wrapping in core-ui's `Tooltip` would have removed the truncation rather than explained it: `TooltipContainer` is an `inline-block` with no `min-width: 0`, so it replaces the ellipsizing flex item with one that cannot shrink below its min-content. The alignment story ships so the measurement can be repeated; jsdom has no layout, so it carries no jest assertion.
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: |
The key names a private tracker project from a public repo. The test's own name already says what it covers.
The `gap` fix closed the header/body disagreement at wide widths only. Narrowing
the container reopened it: the reporter saw a 128px header column over a 104px
body column, and a width sweep put the onset at ~420px for a multi-selectable
table and ~340px for a single-selectable one.
`TableHeader` resets `min-width: 0` — "the header must never be the reason a
column is wider than its cells". The body cells never got the same reset, so they
sat at the flex default `min-width: auto`, whose automatic minimum is
content-based. Two symptoms follow from that one asymmetry:
- short body cells ("Attached", an action button) freeze at their content floor
while their headers keep shrinking, so those columns read wider in the body;
- flexbox then redistributes the frozen items' share of the negative free space
onto the row's only still-shrinkable cell, which over-shrinks *below* its own
header. That is the 128-vs-104 the reporter measured.
Both rows now shrink by the same rules. The reset goes before the `cellStyle`
spread, so a consumer's explicit `minWidth` still wins. Swept 736px down to
224px across all three selectable shapes: every header/body delta is now exactly
0, where before they diverged by up to 41px.
The alignment story becomes the verification surface for the whole change: four
panels, each in a draggable frame, and the two single-selectable ones now wire
`onRowSelected`/`selectedId` so row selection is actually visible — without it
the in-cell-click guard had nothing observable to demonstrate. Panel C adds
dropped columns with `revealDroppedColumns` so the `+N` trigger can be clicked on
an unselected row, which is the case that regressed.
Known limitation, unchanged in kind by this commit: a control that cannot shrink
overflows its column once the column is narrower than the control. With the floor
gone that becomes visible on the action column below ~260px. The remedy is
consumer-side — `iconOnly` on the action button — not a floor here, which would
put the columns back out of agreement.
…ected `TableRow` decided whether to paint the hover highlight and the pointer cursor from `$selectedId` — whether *something is currently selected* — which starts undefined. A selectable table therefore rendered with no affordance at all until after its first click, and nothing invited that click. It also made the fix for in-cell clicks impossible to demonstrate: the reveal stories pass no `onRowSelected`, so their rows were never selectable in the first place. The gate is now `$selectable`, meaning the content was given an `onRowSelected` — the same source it already uses for `tabIndex`. The selected-row highlight drops to `$isSelected` alone, since a selected row implies a selection exists. `Tablestyle` is not re-exported from `index.ts` or `next.ts`, so widening `TableRowType` is internal. Both responsive-column-drop stories now pass `onRowSelected`/`selectedId`, so the `+N` trigger can be exercised on a genuinely selectable row — verified: the popover opens, the row stays unselected, and the popover is still open after 500ms. The temporary header/body alignment story is removed now that the measurement it existed for is done; the two mechanisms it caught are covered by the commits before this one. Its prose, and the story-level prose the drop stories carried, moves into the Table guideline, where row selection, `dropAt`, `revealDroppedColumns` and the non-shrinking-control caveat are now documented — stories stay pure examples.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| target: EventTarget | null; | ||
| }): boolean => | ||
| event.target instanceof Element && | ||
| !!event.target.closest(INTERACTIVE_SELECTOR); |
Contributor
There was a problem hiding this comment.
.closest() walks the entire ancestor chain, not just within the row. If the table sits inside an element matching INTERACTIVE_SELECTOR (e.g. a [role="button"] container or a <label>), clicks on plain cell content would match the outer ancestor and silently suppress row selection.
Scoping to event.currentTarget avoids this:
Suggested change
| !!event.target.closest(INTERACTIVE_SELECTOR); | |
| !!event.target.closest(INTERACTIVE_SELECTOR)?.closest('[class*="tr"]') === null | |
| ? false | |
| : true; |
Actually, a cleaner fix — check the matched element is inside the handler's own row:
export const isInteractiveTarget = (event: {
target: EventTarget | null;
currentTarget: EventTarget | null;
}): boolean => {
if (!(event.target instanceof Element)) return false;
const hit = event.target.closest(INTERACTIVE_SELECTOR);
return !!hit && (event.currentTarget instanceof Element
? event.currentTarget.contains(hit)
: true);
};This way an ancestor <a> or [role="button"] wrapping the table won't disable row selection.
…dable Three things a review of the previous commits turned up, all verified in a browser rather than only in jsdom. **The row-click guard was unbounded in both directions.** It called `closest(INTERACTIVE_SELECTOR)` from the event target with no upper bound, so a table rendered inside a `<label>`, an `<a>` or a `[role="button"]` matched that ancestor for *every* cell and row selection stopped working entirely. In the other direction, a React portal leaves the row in the DOM but still bubbles to it through the React tree, so plain text in a portalled popover — the `revealDroppedColumns` panel — reached the row handler with nothing interactive in between and silently selected the row behind the open overlay. The search is now bounded to the row at both ends, and anything not contained by the row is not a click on the row. Renamed to `shouldIgnoreRowEvent`, since it answers a broader question than "is this a control". Both holes have a regression test, each confirmed to fail against the previous implementation. **An ellipsized header offered no way to read the full label.** Body cells recover through `ConstrainedText`; headers had nothing. They now show the label in a `Tooltip`, and only once it is actually cut off — a tooltip repeating a header that reads fine is noise. That needs a live measurement, because a column is sized by a grow factor and truncation onset moves with the table's width. The tooltip wrapper is mounted in every state and only `overlay` is conditional: mounting it on the flip would change the DOM under the element being measured and let the two states oscillate. A native `title` was tried first and rejected — the delay is drawn by the OS and cannot be configured, and react-table's `getSortByToggleProps()` already puts `title="Toggle SortBy"` on the header, so the two collided. **`HeaderLabel` relied on its parent for the ellipsis.** `overflow` and `text-overflow` are inert on an inline box, and a `span` is inline by default — it only worked because flex blockifies its children. Wrapping the label removed that and the truncation silently disappeared, with every test still green, because the tests stub `scrollWidth` and cannot see `display`. `display: block` is now stated on the label itself and asserted in a test that fails without it. Also, so the same rule is not written twice: the body cell's `min-width: 0` reset — the one that keeps header and body shrinking alike — moves into a shared `bodyCellStyle`, and the header-label story gains a label long enough to actually truncate, with the behaviour documented in the guideline rather than the story. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guideline still said a truncated header carries its text as a `title`. That was true of an earlier draft; the label now recovers through a `Tooltip`, because `title` is drawn by the OS after a delay that cannot be configured and collides with the `title` react-table already puts on a sortable header. It also said every string header carries it, where it appears only while the label is cut off.
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: |
The caret was a zero-width flex item holding an absolutely-positioned glyph, with HeaderContent reserving the room for it in padding. Three constants had to agree by hand -- caretGlyphSize plus caretGutter against caretSpace -- with nothing enforcing it, and the glyph's position was expressed twice, once per alignment branch. The premise for keeping the caret out of the flow was that a caret with real width lifts a sortable header's min-content above its body cell's, so the two rows stop agreeing on column widths. That only binds while min-width: auto is in play. The header chain and the body cells now both carry min-width: 0, so the floor sits at 0 and an in-flow caret costs nothing. SortCaretWrapper is therefore a plain flex item with a real width, and that width is the whole reserve: one value instead of three, and a sortable header's intrinsic width finally includes its own caret. The width stays unconditional because the glyph does not -- SortIncentive appears only on hover, and a reserve that came and went with it would shift the header out from under the pointer. Centred sortable headers keep a counterweight, because flex centres label-plus-caret and leaves the label half a caret off centre. One caret width of leading padding corrects that exactly, capped at 15% of the header so a column too narrow to afford the correction spends its width on the label instead: it drifts off centre rather than truncating sooner. The cap starts binding below a 117px header and bounds the drift at 8.75px. Story case D pins two centred sortable columns narrow enough for the cap to bind, so the trade-off is visible rather than asserted.
tabIndex and the hover affordance are rendered output, so they were being read out of a ref during render. That reads fresh today only because react-table hands react-window a new rows array every render, which stops the memoized row from ever bailing out -- a coincidence of someone else's memoization rather than anything this component guarantees. A plain derived boolean plus a memo dependency says the same thing without depending on it; the ref stays for the event handlers, which is what it is for. The two new tests cover an affordance that had none: every existing selectability test passes onRowSelected, so "no tabIndex when rows are not selectable" was never asserted. They pass against the ref read as well, and are coverage rather than a regression guard.
| }, | ||
| ]; | ||
|
|
||
| export const SortCaretHeaderAlignment = { |
Contributor
There was a problem hiding this comment.
The PR description and test plan reference a HeaderBodyColumnAlignment story ("Components/Data Display/Table → Header Body Column Alignment") that should be added in this file, but it isn't present in the diff or anywhere in the codebase.
The description says it "ships so the measurement can be repeated" for the multi-selectable gap fix — a three-column shape with and without a scrollbar, in both selectable modes. Without it, the test plan's step 1 can't be followed and the fix can't be visually verified in Storybook.
A column declares its alignment once, in cellStyle. The header honoured it;
a plain string value never did. The value inherited the alignment correctly all
the way down the wrapper chain and was then overridden at the last element:
ConstrainedTextContainer hard-codes text-align: left, and a rule on the element
beats anything inherited. Measured on a 1200px viewport, a centred column's value
sat 94.33px left of its own header label and an end-aligned column's 143.2px
short of it.
The scope is what let it survive: DefaultRenderer routes only string values
through ConstrainedText, so a column with a custom Cell renderer aligned
correctly and a plain string column did not -- the two disagreed with each other
as much as with the header.
ConstrainedText now inherits instead, with the centered prop kept as an explicit
override for a container that sets no alignment of its own. Of its four
in-library call sites, the chart labels pass centered explicitly and are
unaffected; the rest sit under no centring ancestor, so inherit resolves to left
exactly as before.
Behind that sat a second, independent offset: DefaultRenderer wrapped the value
in a Box mr={4}, 8px on one side only, which left a centred value 4px from its
label and an end-aligned one 8px short even once the alignment was right. A
body-only margin cannot keep header and body in agreement. The row's own gap
already separates the columns, and both rows share it.
Header-minus-value is now 0 on every column and every alignment, and story panel
D pairs each narrow centred sortable column with an identical column that is not
sortable, so the caret's counterweight can be read off directly instead of
inferred.
… its label A centred sortable header needs a counterweight, because flex centres label-plus-caret and leaves the label half a caret off centre. That counterweight was a fixed padding capped at 15% of the header, and a percentage cannot tell "no room" from "some room": it drifted a 98px column that had ample space for the full correction, while still not giving a 70px column enough. It is now a pseudo-element flex item instead, with a shrink factor weighted far above the label's. Negative free space goes to it first, so a centred header is exactly centred whenever its label fits beside the reserve, and the reserve -- not the label -- is what gets spent when it does not. Measured on the story's two centred sortable columns, label offset from its own header box centre: 98px column capped padding -1.41 counterweight 0 70px column capped padding -3.50 counterweight -1.96 The 70px case cannot reach zero: a 6-character label plus two 17.5px reserves needs 74px, so at 70px the column is over-constrained at rest and -1.96 is the least drift available without losing a character. 74px measures exactly 0. A pseudo-element rather than a real element so nothing is added to the DOM or the accessibility tree, and so the counterweight cannot leak into an alignment that has no asymmetry to correct -- it exists only inside the centred sortable branch. The reserve stays unconditional with respect to the glyph, which appears only on hover; hovering still moves nothing.
JeanMarcMilletScality
changed the title
JeanMarcMilletScality
marked this pull request as ready for review
August 31, 2026 17:56
Two of them read as if a table nested inside a `<label>` were a supported pattern. It is not, and it was never the point: each test stands for one bound of the row-event guard, and the wrapper is only the cheapest way to put a matching element where the bound has to stop. - The upward-bound test now wraps the table in a clickable card, which is a shape a table really does turn up inside, and its name says what it asserts: an interactive ancestor above the table must not disable row selection. - The portal test is named for the case it stands for -- an overlay opened out of a cell -- rather than for the mechanism used to build one. - The selector's `label` entry is documented as an in-cell label wrapping that cell's own control, which is the case that justifies it. Both bounds are mutation-checked: removing either one fails exactly the test that covers it, and nothing else. Drops the test that read `overflow`, `display` and `tagName` back out of the header label. Tests here assert user-facing behaviour, not CSS, and the three tests above it already cover what a user can tell apart -- the tooltip appears when the label is cut off and stays away when it is not.
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
2 participants
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 — Six defects in
tablev2's rows and headers: a click on a button inside a row no longer also selects the row, header columns line up with body columns at every width, a column's declared alignment now actually reaches its values, a header label that gets cut off offers its full text, a selectable table shows it is selectable before the first click, and the sort caret reserves its own space instead of making the header reserve it.Context / Why
All six live in the same files and in the same layout mechanism — how a header cell and its body cells agree on a column's width — so as separate PRs they would conflict with each other rather than review independently.
🧩 Approach
Every claim below is measured. In three cases the measurement contradicted the diagnosis it started from, and those reversals are called out.
1. The row handler swallowed clicks meant for a cell's own controls
The row
onClickin both selectable contents fired for a click anywhere in the row with no target check. Clicking a button in a cell activated the button and selected the row — and the selection re-render remounts the memoized row, so whatever the button just opened unmounted immediately.onKeyDownwas worse:keydownbubbles too, so Enter on a focused in-cell control selected the row andpreventDefault()ed the control's own activation.Both handlers now bail on an interactive target through a single
shouldIgnoreRowEvent. The search is bounded to the row at both ends, and each bound closes a hole an unboundedclosest()leaves open:closest()walks past the row to the document<label>,<a>or[role="button"]matched that ancestor for every cell, disabling row selection entirelyMultiSelectableContent's selection cell reached its behaviour two different ways — directly in single-row mode, and by bubbling to the row handler'selsebranch otherwise. Both ended in the same call with the same arguments, so it now owns its click outright rather than depending on a bubble the guard stops.2. Header and body tracks disagreed — two independent causes, not one
TableRowMultiSelectablenever declared thegapthatHeadRowandTableRowboth do, so it computedgap: normal. Every column track has a grow factor and no basis, which turns a gap the header reserves and the body does not into free space the body redistributes. Measured on grow factors summing to 2.5 (1.5 / 0.5 / 0.5), body-minus-header:Body cells were missing the
min-width: 0reset the header already had. A flex item defaults tomin-width: auto, so a short cell froze at its content width while its header kept shrinking — and flexbox then redistributed that frozen item's share of the negative free space onto the row's remaining shrinkable cell, pushing it below its own header. Both rows have to shrink by the same rules or they cannot agree.This clears the two suspects it started from. The scrollbar-compensation theory (
HeadRowsubtracts the scrollbar width,TableRowdoes not) is wrong — a single-selectable table with an identical 11px scrollbar aligned perfectly both before and after. And header cells do receivecolumn.cellStyle, margins included.3. A column's declared alignment never reached a plain string value
Pre-existing, and found while reviewing this PR's own story. A column declares its
alignment once, in
cellStyle. The header honoured it. A plain string value never did — itinherited the alignment correctly all the way down the wrapper chain, and was then overridden
at the last element:
ConstrainedTextContainerhard-codedtext-align: ${(props) => (props.$centered ? 'center' : 'left')}, and a rule on the element beats anything inherited:The scope is what let it survive this long:
DefaultRendererroutes only string valuesthrough
ConstrainedText, so a column with a customCellrenderer aligned correctly and aplain string column did not — the two disagreed with each other as much as with the header.
ConstrainedTextnow inherits, withcenteredkept as an explicit override for a containerthat sets no alignment of its own. Of its four in-library call sites the chart labels pass
centeredexplicitly and are unaffected; the rest sit under no centring ancestor, soinheritresolves to
leftexactly as before.Behind that sat a second, independent offset:
DefaultRendererwrapped the value in aBox mr={4}—space[4]= 0.571rem = 8px, on one side only — which left a centred value 4pxfrom its label and an end-aligned one 8px short even once the alignment was right. A body-only
margin cannot keep the two rows in agreement; the row's own
gapcan, and already separatesthe columns. Header-minus-value is now 0 on every column and every alignment.
4. The sort caret made the header reserve space on its behalf
The caret was a zero-width flex item holding an absolutely-positioned glyph, with
HeaderContentsupplying the actual room in padding. Three constants had to agree by hand and nothing enforced it:Before:
After:
The premise for keeping the caret out of the flow was that a caret with real width lifts a sortable header's min-content above its body cell's, so the two rows stop agreeing on column widths. That only binds while
min-width: autois in play — and after §2 both the header chain and the body cells carrymin-width: 0, so the floor sits at 0 and an in-flow caret costs nothing. The width stays unconditional because the glyph does not: the hover incentive isdisplay: noneuntil the header is hovered, and a reserve that came and went with it would shift the header out from under the pointer.Centred sortable headers keep a counterweight, because flex centres label-plus-caret and leaves the label itself half a caret off centre. That counterweight is a pseudo-element flex item with a shrink factor weighted far above the label's, so negative free space goes to it first: a centred header is exactly centred whenever its label fits beside the reserve, and the reserve rather than the label is what gets spent when it does not.
This started as a fixed padding capped at 15% of the header, and the cap was the wrong instrument — a percentage cannot tell "no room" from "some room". Label offset from its own header box centre, on the story's two centred sortable columns:
The 70px column cannot reach zero and no arrangement makes it: a six-character label plus two 17.5px reserves needs 74px, so at 70px it is over-constrained at rest and −1.96 is the least drift available without losing a character. At 74px it measures exactly 0. A fixed reserve would have truncated instead.
A pseudo-element rather than a real element so nothing is added to the DOM or the accessibility tree, and so the counterweight cannot leak into an alignment with no asymmetry to correct — it exists only inside the centred sortable branch.
Acceptance test for §2 and §4 together — header-minus-body
leftandwidth, every column, single- and multi-selectable, ten viewport widths from 1400px to 360px: exactly 0 everywhere, from identical unrounded rects. No layout shift on hover (0/0 on all columns). Caret width a consistent 17.5px, abutting its label with no overlap; non-sortable columns render no caret markup at all.5. An ellipsized header gave no way to read the full label
Body cells recover via
ConstrainedText; the header was the one place a label became unreadable with nothing to get it back.TruncatableHeaderLabelmeasures the label and offers aTooltiponly once it is genuinely cut off — a tooltip repeating a header that reads fine is noise — re-measuring on resize, because these columns size from grow factors rather than fixed widths. Headers built from a node rather than a string get none, since there is no text to show.Tooltiprather than a nativetitlefor two reasons:titleis drawn by the OS after a delay that cannot be configured, which reads as nothing happening; and react-table'sgetSortByToggleProps()already putstitle="Toggle SortBy"on the header, so a sortable header would carry two competing titles and which one appeared would depend on the pixel hovered.That choice costs a workaround.
TooltipContaineris aninline-blockwith nomin-width: 0and wraps its children in a second plaindiv, so left alone both sit between the flex item and the text and impose a content-based minimum — the column stops shrinking and the label never ellipsizes at all, removing the truncation the tooltip exists to explain.HeaderLabelFrameforces that chain back to shrinkable blocks. This is the same workaroundConstrainedTextalready carries as its ownBlockTooltip, and the duplication is deliberate here: the root cause is inTooltipContainer, and fixing it there touches everyTooltipin the library. See Follow-up.6. A selectable table looked inert until its first click
The row hover affordance and pointer cursor were gated on
$selectedId— "something is currently selected" — rather than on whether the table can be selected at all.$selectablenow carries the latter, so a selectable table invites the first click instead of only acknowledging it afterwards.📷 Screenshots
Storybook → Components/Data Display/Table → Sort Caret Header Alignment. Crop wide enough to include the body rows beneath the header, since the whole claim is about the relationship between the two:
🔍 Review focus
tablev2/SingleSelectableContent.tsx+MultiSelectableContent.tsx› rowonClick/onKeyDown— the one change here a consumer could be depending on. Today a click on a button inside a row also selects the row; after this it does not. That is the bug, but it is a real behaviour change, and a consumer relying on "a click anywhere in the row selects it" will notice. Consumers currently carrying their ownstopPropagationon an interactive cell can drop it. The selector list inTableCommon.tsx › INTERACTIVE_SELECTORis the whole guard — worth a look for anything a cell renders that should count as interactive and is not in it.tablev2/Tablestyle.tsx › SortCaretWrapper+HeaderContent— changes header geometry for every sortable table, not just narrow ones. The sortable header's intrinsic width now includes its caret, which it deliberately did not before; that is safe only because §2 putmin-width: 0on both the header chain and the body cells, so the two are coupled and should be reviewed together. The centred counterweight is the part with a visible trade-off.constrainedtext/Constrainedtext.component.tsx › ConstrainedTextContainer— the only change here outsidetablev2, andConstrainedTextis publicly exported. Itstext-aligndefault goes from a hard-codedlefttoinherit, so it now follows whatever alignment its container sets. Every in-library call site was checked (chart labels passcenteredand are unaffected; Select and AttachmentTable sit under no centring ancestor, soinheritresolves toleftas today), but an external consumer that has been relying onConstrainedTextstaying left-aligned underneath a centring ancestor will see it centre. Easy to split into its own PR if that blast radius is not wanted here.🧪 How to test
npm run storybook, open Components/Data Display/Table → Sort Caret Header Alignment. In panels A–C every header label should sit directly over its column; drag the browser narrower and they should stay aligned. In panel D the two centred headers stay readable and roughly centred over their values.+Ntrigger appears, and click the trigger on an unselected row. The panel should open and stay open; previously the row selected itself, re-rendered, and closed it.🚧 Follow-up
TooltipContainerroot cause. It is aninline-blockwith nomin-width: 0, which is why bothConstrainedTextand nowHeaderLabelFramecarry the same unwrapping workaround. Fixing it at source would delete both, but it touches everyTooltipin the library and does not belong in a table PR.organisms/attachments/AttachmentTable.tsxcarries amarginLeft: 'auto'that does nothing at a grow-sum above 1 but becomes live below it, and a<Box flex={0.5} />as aHeaderthat is a no-op. The measurement in §2 cleared both as causes of the misalignment fixed here, so they are left alone rather than widening this PR.keyinsidegetRowProps()/getCellProps(), which React now reports as an error on every row and header. Pre-existing and unrelated to this PR — the same spread pattern is on the base branch — but it is noise in the console for anyone testing the above.What changed
Tablestyle.tsxcarries the layout: thegapdeclaration onTableRowMultiSelectable,$selectableonTableRow, and the caret rework —SortCaretWrapperbecomes an ordinary flex item with a real width andSortCaretGlyphdisappears, soHeaderContent's two alignment branches collapse tojustify-contentplus one capped counterweight for centred columns.TableCommon.tsxgains the shared pieces both selectable contents need —shouldIgnoreRowEvent,bodyCellStyle,useIsEllipsized,HeaderLabelFrameandTruncatableHeaderLabel— so the two contents use one implementation rather than two copies that drift.SingleSelectableContent.tsxandMultiSelectableContent.tsxwire them in; the multi-selectable one also collapses its selection cell to a single path, and the single-selectable one takes the selectability affordance out of a ref (rendered output should not be read from a ref during render, even though react-table's freshrowsarray happens to keep it correct today).Tablev2.component.tsx › DefaultRendererloses itsBox mr={4}wrapper, andconstrainedtext/Constrainedtext.component.tsx › ConstrainedTextContainerinherits itstext-aligninstead of hard-codingleft— the two halves of §3, and the only edit in this PR outsidetablev2.stories/tablev2.stories.tsxreplaces the earlier alignment story withSortCaretHeaderAlignment, four panels: equal grow factors too narrow for both headers, end-aligned and centred sortable headers, the same columns multi-selectable, and five narrow centred columns pairing each sortable one with an identical column that is not sortable. It ships so the measurements above can be repeated; it carries no jest assertion, because jsdom has no layout and reading a CSS declaration back out would prove nothing.tablev2.guideline.mdxdocuments the header truncation and its tooltip.