feat: Table atomic components (extracted from classic Table) - #4977
Draft
gethinwebster wants to merge 36 commits into
Draft
feat: Table atomic components (extracted from classic Table)#4977gethinwebster wants to merge 36 commits into
gethinwebster wants to merge 36 commits into
Conversation
…ment Introduce src/table/table-cell (internal, not yet exported) holding the pure body-cell substrate: the td/th element, base .body-cell padding, the .body-cell-content wrapper, disablePaddings support, and raw-node ref forwarding. TableTdElement now composes InternalTableCell, keeping all feature layering (sticky/resize/editable/selection/expandable + compound .body-cell.* CSS) on its own wrapper. DOM/CSS-identical by construction: same classes, same child order, feature SCSS unchanged; only an inert .body-cell-content.disable-paddings rule is added (never applied by classic).
Carve the <tbody> and <thead> element substrates out of classic's render sites into thin internal forwardRef components (src/table/table-body, src/table/table-head), mirroring the Inc1/Inc2/Inc3a pattern. Render-neutral: each substrate renders only the bare element and forwards ref/className/nativeAttributes/children; all feature wiring stays on the composing sites (internal.tsx, thead.tsx). TableRoot's <table> is deferred (performanceMarkAttributes data-* threading is incompatible with the data-*-free substrate contract) -- see evidence FINDINGS/DECISIONS.
…ubstrates (Inc4a)
…internal-only) + review fixes (Inc7b)
…adding + header dividers (v1/v3)
…root-classic) Recreate each pages/table-root atomic demo with the shipped classic Table as a per-scenario VR oracle. Same data (makeItems/makeLine), columns, counts and initial state so the atomic-vs-classic diff is content-identical. Auto-registered at table-root-classic/<name>. Multi-column sort and row virtualization have no classic equivalent (see FINDINGS). Pages only; src untouched.
Set variant="borderless" on the classic Table in every table-root-classic demo so the classic twin is chrome-free (no outer Container border/radius/ shadow), matching the atomic TableRoot. This is the agreed VR oracle for the atomic-vs-classic parity work.
Two dominant atomic-vs-classic (borderless) gaps from the VR punch-list: a1 header row height (29px -> 37px): the public TableHeaderCell rendered its children with no content box, so the header row was 8px short. Wrap the children in a .header-cell-content box carrying padding-block:$space-scaled-xxs (+ line-height:$line-height-body-m), reproducing classic's header-cell-content vertical sizing. No inline padding, so the v1 is-visual-refresh first-column inset and the v3 ::after divider are untouched. a2 edge-row height (first/last body rows 1px taller): TableBody now publishes each row's first/last position via a RowPositionContext (a no-DOM provider), and TableCell applies classic's own body-cell-first-row / body-cell-last-row edge classes. Reusing classic's classes means the 2px transparent placeholder border and the body-cell-last-row:not(.body-cell-selected) selection guard come for free -- no reimplementation, no data-* hook, no element/universal selector. Auto layout verified pixel-exact vs the borderless classic twin (simple page: header 37, rows 40/39/.../40). Atomic layer only; classic composition (the src/table substrates) is untouched.
…tes (drop redundant typed props)
… inset, grid row height (a3/a4/a5)
The .body-cell-content truncation clip (overflow:hidden) also applied to disablePaddings cells, whose content box collapses to the control height. That cropped the focus ring of a checkbox/radio in a selection-control column. Opt disable-paddings cells back out of the clip (overflow:visible), mirroring the existing body-cell-edit-active idiom. Classic Table is unaffected (disable-paddings is atomic-only). VR unchanged, jest 673/673.
…th body (== classic)
… via data-selected adjacency TableRow auto-emits data-selected from variant='selected'; the atomic cell module reads sibling adjacency to collapse the shared edge into a single 1px divider, square the inner corners, and (via has-selection + a full-height stretch) enclose the selection-control column in the box. Keyed on data-selected, a hook classic never emits, so classic VR stays 0.0000%.
The simple, striped-rows and single-selection atomic demos have no sort UI, but their classic twins reused sorting-capable column sets. Classic Table renders a sort caret for any column declaring sortingField/sortingComparator (independent of sortingColumn/onSortingChange), so those twins showed carets the atomic lacks. Add non-sorting column sets (dataColumnsPlain, nameStatusColumnsPlain) and point the no-sort twins at them; sorting twins keep the sortable sets.
…assic
The grid columns [{size:40},{minWidth:160},{size:140}] flexed Name to fill and pinned Status to a fixed 140px, shoving Status far right. Classic Table splits the non-control space ~53:47 (measured, proportional across viewports). Reproduce with proportional flex weights [{size:40},{size:{flex:53}},{size:{flex:47}}] so atomic per-column widths match classic to the pixel.
Point the classic loading-and-empty VR oracle at dataColumnsPlain (no sortingField/sortingComparator) so it renders no sort carets, matching the non-sortable atomic demo. Removes the stray down-carets on all 4 headers and converges column widths to the atomic (was Size -26.5px).
The header column-divider ::after painted a 1px divider (border-inline-start,
color-border-divider-default) on disablePaddings header cells (the selection
control column). Classic Table draws no divider there. Add
.header-cell.disable-paddings::after { display: none }. Classic never emits
disable-paddings, so this is inert on the classic path (selection-single VR
0.0000% vs oracle).
…ia data-shaded, align first-column header
…header divider, and equal-height cells
Standalone visual-parity fixes surfaced by eyeballing the demo pages
against borderless classic Table twins (percentages proved unreliable for
sparse table UI):
- Equal-height cells: move the stretch+center treatment onto the grid-mode
markers (.cell-grid / .header-cell-grid) instead of the unconditional
.cell, so auto-layout tables keep native display:table-cell (fixes the
collapsed simple/auto layout) while grid tables get colinear control/data
boxes and centered controls.
- Consecutive-selected seam: set the shared bottom edge to the 1px list
divider (mirroring classic body-cell-next-selected) so the two-row seam
paints 2px like classic, not 1px.
- Control-column header divider: remove the erroneous
.header-cell.disable-paddings::after { display:none } suppression -
mainline classic DOES draw this divider. The empty single-selection
control header no longer collapses (equal-height fix), so the restored
divider renders full-cell like classic with no stray streak.
All keyed on atomic-only selectors classic never emits; classic Table VR
byte-identical (0%), jest green.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4977 +/- ##
========================================
Coverage 97.67% 97.67%
========================================
Files 960 985 +25
Lines 31376 31542 +166
Branches 11597 11637 +40
========================================
+ Hits 30645 30809 +164
- Misses 724 726 +2
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…Properties>
Replace TableRowProps.Style / TableBodyProps.Style Pick<React.CSSProperties, ...>
with explicit per-property type aliases using indexed access
(React.CSSProperties['position'] etc.) — reuses the canonical property types
without redefining unions.
Uses a type alias (not an interface) deliberately: a type-alias object literal
gets an implicit index signature and stays assignable to React.CSSProperties
(the style prop), whereas an interface lacks the CSS custom-property
(--${string}) index signature and would need a boundary cast. No cast needed.
Regenerated the documenter snapshot: table-row/table-body now record the named
csstype aliases (Property.Height/Property.Position) instead of Pick's fully
inlined unions — cleaner generated docs, same underlying type. Full snapshot +
functional + table-root suites: 10 suites / 730 tests green.
…(invert table/ -> table-*)
Move the 5 shared element substrates (InternalTableBody/Head/Row/Cell/HeaderCell)
out of src/table/ into their atomic src/table-*/ dirs, and re-point every importer
so the existing Table builds ON TOP OF the atomic layer. No atomic file imports a
JS/TS module from src/table/ anymore; the only remaining table/ imports are the two
one-way .css.js style-module reuses (body-cell / header-cell feature stylesheets),
which stay in table/ by design. This breaks the previous two-way dependency cycle
(td-element -> atomic Cell -> table/ substrate) into clean one-directional layering.
InternalTableRow's base .row is re-based onto the atomic table-row stylesheet; the
classic row render sites stamp their own .row, so classic rows are unaffected.
Also restores the two `as React.CSSProperties` boundary casts on TableRow/TableBody
style application: manually-typed Style (per the manual-typing preference) drops
CSSProperties' `--${string}` index signature that Pick's homomorphic mapped type
preserved, so a widening cast at the DOM boundary is required. Fixes the CI build
that was red on 434718a for the missing cast.
Verified: gulp build clean; jest 1158 + documenter 104 snapshots pass (no regen);
classic table/__tests__ green; classic VR 0% on all render-bearing states (one
0.0744%/472px sub-pixel AA delta on the sticky-pinned header label at fractional
scroll, outside the changed surface, accepted as benign noise).
…ren inspection
(1) Fold each relocated InternalTable<X> substrate back into its component's
internal.tsx (one file exports both the public wrapper and the substrate);
delete the 5 internal-table-<x>.tsx files and repoint the classic consumers.
(2) Remove the TableBody React.Children inspection and the RowPositionContext it
fed. That context existed only so cells could emit body-cell-first-row/last-row
in auto mode. The edge treatment is reproduced directly in CSS
(tr:first-child/:last-child > .cell:not(.cell-grid), a transparent placeholder
border) — auto-mode only, keyed on atomic classes the existing Table never emits,
so classic-inert. TableBody now renders {children} directly; RowPositionContext /
useRowPosition / the provider are deleted.
Verified: gulp build clean; jest 1262 tests / 107 snapshots green (no regen);
all 7 atomic auto pages 0.0000% EXACT vs pre-change baseline (edge rows restored);
classic VR 16/16 0.0000% EXACT (no new non-zero state).
Collapse each atomic internal.tsx from two exports (substrate + wrapper) to one:
- Body/Head/Row: the InternalTable{Body,Head,Row} substrates were near-empty
element wrappers sharing nothing, so they're removed. The atomic component
renders its <tbody>/<thead>/<tr> directly, and classic inlines the bare
element at its call sites (byte-identical to what the substrate produced).
- Cell/HeaderCell: merge substrate + wrapper into a single InternalTableCell /
InternalTableHeaderCell that serves BOTH the public index.tsx and classic's
td-element/th-element. It keeps the low-level composition API
(tag/beforeContent/wrapLines/handlers/etc.) and reads the atomic contexts, but
all atomic-only output (.cell/.header-cell marker, grid role, selection/shading,
is-visual-refresh, header content wrapper) is gated behind a new internal
`atomic` flag that only index.tsx sets. With atomic off (classic's path) the
component collapses to `body-cell + className`, so classic renders identically —
no leaked marker, no duplicate is-visual-refresh (classic adds its own).
Verified: gulp build clean; jest 1158 + documenter/test-utils 693/107 green
(0 regen); classic VR 15/16 EXACT (the 1 non-zero is the known held-scroll
pinned-header AA non-determinism, self-diff larger); atomic pages 7/7 EXACT.
Remove refactor-history narrative and duplicate-rationale comments across the 7 atomic components (e.g. 'renders directly', 'extracted substrate', 'classic call site inlines'), and compress kept rationale to 1-2 lines. Preserve public JSDoc and the load-bearing WHY (classic-inert / atomic-flag gating / data-selected adjacency / one-way SCSS reuse / selection-ring + edge-placeholder geometry). Behavior-neutral: gulp build clean, 569 unit tests green, no code change.
…l-component idiom
Match the src/status-indicator idiom across all 7 atomic components:
- named InternalTable<X>Props interfaces (extends Table<X>Props, InternalBaseComponentProps)
instead of inline intersection types in the signature
- uniform InternalTable* internal names
- default-export idiom (export default function InternalTable<X>(...))
- value-style export { Table<X>Props } in index.tsx
- rename the merged cell/header-cell flag atomic -> __atomic (house __-prefix convention)
Behavior-neutral: the existing Table's td-/th-element are untouched and never pass __atomic,
so classic output is unchanged by construction. Verified against the fresh mainline merge:
gulp build clean, jest 45 suites / 1262 tests / 107 snapshots green (zero snapshot drift),
eslint + stylelint clean.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
ARIA row counts and selection semantics are inconsistent, and capped columns discard their configured flex weight.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Introduces composable public table primitives while sharing cell substrates and styling with the existing Table implementation.
Changes:
- Adds seven atomic table components with auto/grid layouts and ARIA props.
- Extracts shared header/body cell rendering from the existing Table.
- Adds test utilities, tests, snapshots, and demonstration pages.
File summaries
| File | Description |
|---|---|
src/test-utils/dom/table-row/index.ts |
Adds row wrapper. |
src/test-utils/dom/table-root/index.ts |
Adds root wrapper. |
src/test-utils/dom/table-header-row/index.ts |
Adds header-row wrapper. |
src/test-utils/dom/table-header-cell/index.ts |
Adds header-cell wrapper. |
src/test-utils/dom/table-head/index.ts |
Adds head wrapper. |
src/test-utils/dom/table-cell/index.ts |
Adds cell wrapper. |
src/test-utils/dom/table-body/index.ts |
Adds body wrapper. |
src/table/internal.tsx |
Reorders row attribute composition. |
src/table/header-cell/th-element.tsx |
Uses shared header-cell substrate. |
src/table/body-cell/td-element.tsx |
Uses shared body-cell substrate. |
src/table/body-cell/styles.scss |
Adds padding opt-out styles. |
src/table-row/styles.scss |
Defines atomic row layouts. |
src/table-row/internal.tsx |
Implements atomic rows. |
src/table-row/interfaces.ts |
Defines row API. |
src/table-row/index.tsx |
Exports public row. |
src/table-row/context.ts |
Shares row visual state. |
src/table-root/use-table-root.ts |
Compiles grid columns. |
src/table-root/styles.scss |
Defines root layouts and scrolling. |
src/table-root/internal.tsx |
Implements table root structure. |
src/table-root/interfaces.ts |
Defines root API. |
src/table-root/index.tsx |
Exports public root. |
src/table-root/context.ts |
Shares root layout state. |
src/table-root/__tests__/use-table-root.test.tsx |
Tests grid-track compilation. |
src/table-root/__tests__/basic-table.test.tsx |
Tests composition and semantics. |
src/table-root/__tests__/basic-table-styling-props.test.tsx |
Tests styling props. |
src/table-root/__tests__/basic-table-roles.test.tsx |
Tests role structure. |
src/table-root/__tests__/basic-table-aria-label.test.tsx |
Tests accessible naming. |
src/table-header-row/styles.scss |
Defines header-row layouts. |
src/table-header-row/internal.tsx |
Implements header rows. |
src/table-header-row/interfaces.ts |
Defines header-row API. |
src/table-header-row/index.tsx |
Exports public header row. |
src/table-header-cell/styles.scss |
Adds atomic header styling. |
src/table-header-cell/internal.tsx |
Implements shared header cells. |
src/table-header-cell/interfaces.ts |
Defines header-cell API. |
src/table-header-cell/index.tsx |
Exports public header cell. |
src/table-head/styles.scss |
Defines head layouts. |
src/table-head/internal.tsx |
Implements table head. |
src/table-head/interfaces.ts |
Defines head API. |
src/table-head/index.tsx |
Exports public head. |
src/table-cell/styles.scss |
Adds atomic cell state styling. |
src/table-cell/internal.tsx |
Implements shared body cells. |
src/table-cell/interfaces.ts |
Defines cell API. |
src/table-cell/index.tsx |
Exports public cell. |
src/table-body/styles.scss |
Defines body layouts. |
src/table-body/internal.tsx |
Implements table body. |
src/table-body/interfaces.ts |
Defines body API. |
src/table-body/index.tsx |
Exports public body. |
src/__tests__/snapshot-tests/__snapshots__/test-utils-wrappers.test.tsx.snap |
Updates wrapper snapshots. |
src/__tests__/snapshot-tests/__snapshots__/test-utils-selectors.test.tsx.snap |
Updates selector snapshots. |
src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap |
Adds generated API snapshots. |
src/__tests__/functional-tests/test-utils.test.tsx |
Aligns finder pluralization. |
pages/table-root/virtualization.page.tsx |
Demonstrates virtualization. |
pages/table-root/styles.scss |
Styles table demonstrations. |
pages/table-root/striped-rows.page.tsx |
Demonstrates striped rows. |
pages/table-root/sorting.page.tsx |
Demonstrates sorting. |
pages/table-root/single-selection.page.tsx |
Demonstrates radio selection. |
pages/table-root/simple.page.tsx |
Demonstrates a basic table. |
pages/table-root/selection.page.tsx |
Demonstrates selectable sorting. |
pages/table-root/loading-and-empty.page.tsx |
Demonstrates status states. |
pages/table-root/common.tsx |
Provides shared demo data. |
build-tools/utils/pluralize.js |
Adds component plurals. |
Review details
- Files reviewed: 61/61 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| aria-label={ariaLabel} | ||
| aria-labelledby={ariaLabelledby} | ||
| aria-describedby={ariaDescribedby} | ||
| aria-rowcount={ariaRowcount} |
Comment on lines
+26
to
+30
| if (column.maxWidth !== undefined) { | ||
| return `minmax(${min}, ${column.maxWidth}px)`; | ||
| } | ||
| // `size` is narrowed to `{ flex: number } | undefined` here (the numeric case returned above). | ||
| const flex = column.size?.flex ?? 1; |
| aria-label={ariaLabel} | ||
| aria-labelledby={ariaLabelledby} | ||
| aria-describedby={ariaDescribedby} | ||
| aria-selected={ariaSelected} |
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.
What
Introduces 7 public Table atomic components —
TableRoot,TableHead,TableHeaderRow,TableHeaderCell,TableBody,TableRow,TableCell— extracted from the existingTable's own cell/row/section elements so the existingTableand the atomics share one substrate.Approach (extract from the existing Table)
Instead of building atomic parts separately (which diverged from the existing box model — sticky columns and geometry broke), this works backwards: internal element substrates (
TableCell/TableHeaderCell/TableRow/TableBody/TableHead) are carved out of the existingTable'std-element/th-element/internal.tsx/thead.tsx, and the existingTablecomposes those same substrates. The public atomic components layer the R1 API + standalone styling on top.TableRoot's<table>carries too much wiring to carve render-neutrally, so it is public-only for now.Because the existing
Tableand the atomics share the substrate, the existingTableis unchanged by construction, and the atomics reuse its proven box model + feature CSS (selection, shading, dividers). Where the atomic public layer must re-emit a class the existingTablecomputes centrally, it does so via atomic-only hooks (.cell-grid/.header-cell-gridgrid markers,data-selected/data-shadedrow adjacency) that the existingTablenever emits — so the shared CSS is reused, not duplicated, and stays inert for the existingTable.Verification
Tableis provably unchanged: byte-identical vs a pre-change 16-state oracle (incl. held sticky horizontal scroll); the two residual non-zero states match the baseline to the pixel and predate this work.Tabletwin pages (an apples-to-apples, chrome-free oracle) built during development and since removed as scaffolding.jestgreen; the existingTableVR 0% reconfirmed after every change.Fixes landed from the eyeball audit
.header-cell-content's inlinecell-offset); aligned to body.data-selectedadjacency; the seam is 2px, matching the outer ring..cell-grid/.header-cell-grid), so auto-layout tables keep nativedisplay:table-cell(an earlier unconditional rule broke auto-layout column sizing) while grid tables get colinear control/data boxes and vertically-centered controls.$color-border-cell-shadedby reusing the existingTable's body-cell striped rule, triggered viadata-shadedrow adjacency (dedup intact — the rule exists once in the substrate; only the trigger class was missing).Tabledraws it); the empty single-selection control header no longer collapses (equal-height fix), so the divider renders cleanly with no stray streak.::afterring on the selected row, and the reused selection border is neutralized to a constant 1px, so real border widths never change between selected/unselected. Grid content-centering is therefore border-insensitive: on select/deselect the cell content, control, row top, and table height are all invariant (measured dy=0, every row 39px) — reproducing the existingTable's zero content movement, which a real-border approach cannot do under grid layout. This retired an earlier constant-height padding hack and the per-state seam border-width juggling.disablePaddingscontrol cells fixed (overflow:visible).Known minor deltas (documented, not regressions)
columnLayout='auto'is zero-config intrinsictable-layout:auto;gridmode is the opt-in for explicit widths). Accepted.<button>-based sort header vs the existing native control — accepted.Table's (39 vs 40) — the deliberate constant-height/overlay tradeoff for zero toggle-shift. Selection ring is unchanged (2px); imperceptible.Notes
Dev pages:
pages/table-root/(atomic demos). Supersedes the Path-C dedup approach ondev-v3-gethinw-basic-table-mlp.Draft — pending human bug-bash + publish.