ref(tables): unify table components onto one shared Table shell - #120745
Draft
JoshuaKGoldberg wants to merge 2 commits into
Draft
ref(tables): unify table components onto one shared Table shell#120745JoshuaKGoldberg wants to merge 2 commits into
Table shell#120745JoshuaKGoldberg wants to merge 2 commits into
Conversation
Sentry had two sanctioned table components that shared no code, plus a third de-facto shell leaking out of `gridEditable/styles.tsx`, two column-resize implementations, and two competing column-track models. Adds `components/tables/table/`: a dumb shell that owns only table geometry — real `<table>/<thead>/<tbody>/<th>/<td>` on `display: grid` with `grid-template-columns: subgrid`, column tracks, and the resize drag (delegated to `useColumnResize`). Columns are declared as data because a resize needs stable identity and a known column count; cell contents stay as children. Widths are controlled or uncontrolled. Explicit `role` attributes are kept because `display: grid` drops implicit table semantics in Chrome and Safari. GridEditable, SimpleTable, the explore table kit and InfiniteTable are now facades over it: - `useTableStyles` is deleted; its callers go through the shell - the second resize drag loop and second track builder are gone - InfiniteTable no longer overrides `grid-template-columns` per row, so there is one track model - SimpleTable renders real cells instead of `div[role=cell]` Sortable headers become `<th aria-sort><button>` rather than a `div[role=columnheader]` that was itself the button, which is the standard accessible pattern. Ref DE-1392 Ref DE-1405
github-actions
Bot
added
the
Scope: Frontend
Contributor
Story previewsPreview the stories changed in this PR on the Vercel deployment:
Preview deployment: https://sentry-mcxtao1vd.sentry.dev |
Contributor
📊 Type Coverage Diff
🔍 6 new type safety issues introduced
Non-null assertions (
This is informational only and does not block the PR. |
Follow-ups from the shell PR. `infiniteTable` and `logsInfiniteTable` now share one spacer-padding virtualization hook. Its padding formula accounts for `scrollMargin`, which fixes the offset InfiniteTable had after its scroll element moved to the table. Replays' `useVirtualizedGrid` is left alone: it measures column widths rather than spacing rows, so it shares only `useVirtualizer` boilerplate. Removes shell surface that turned out to be unreachable or unused: - `lastColumnFlexible` was only read by the default track rule, which its one caller bypasses by supplying `getColumnTrack` - `dataRows` sized a resize handle that is invisible until hover, long after the `--table-resizer-height` var is written - `stickyHeader` and `writeResizerHeightVar` had no callers Collapses the remaining duplicated styles: the two `grid-column` header-cell variants and the full-span row cell move into SimpleTable, and the two teams tables stop declaring byte-identical table and link styles. Ref DE-1392 Ref DE-1398
JoshuaKGoldberg
force-pushed
the
de-1392-unified-table
branch
from
July 28, 2026 15:57
99607c8 to
7733c19
Compare
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
1 participant
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.
Stacked on #120563 — base is that branch, not master, so this diff is only the new work. #120563 needs to land first.
What
Sentry had two sanctioned table components sharing no code, a third de-facto shell leaking out of
gridEditable/styles.tsx, two column-resize implementations, and two competing column-track models.This adds
components/tables/table/— a dumb shell that owns only table geometry:<table>/<thead>/<tbody>/<th>/<td>ondisplay: gridwithgrid-template-columns: subgriduseColumnResizefrom ref(tables): consolidate column-resize handling into useColumnResize #120563)onColumnResize) or kept internallyGridEditable, SimpleTable, the explore table kit and InfiniteTable become facades over it.
useTableStylesis deleted, the second resize drag loop and second track builder are gone, and InfiniteTable no longer writesgrid-template-columnsonto every row — so there is now one resize implementation and one track model.Net effect on the touched files is −215 lines, and
gridEditable/styles.tsxdrops from a structural layer to a thin theming layer.Reviewer notes
Three intentional behaviour changes worth a look:
roleattributes are kept alongside the real elements.display: griddrops implicit table/row/cell semantics in Chrome and Safari, so real<td>alone would have been an a11y regression. Keeping the roles also means existinggetByRolequeries and[role='cell']CSS keep working.<th aria-sort><button>instead of adiv[role=columnheader]that was itself the button. That's the standard accessible pattern, but it changes the DOM contract — the button is stretched to fill the cell so the click target is unchanged.logsInfiniteTablealready uses). Forced, not incidental: an absolutely positioned element is not a grid item, sosubgridon it computes tonone, which would reintroduce the per-row track model this PR removes.Not visually verified. Everything here is type-, lint- and DOM-semantics-checked; this changes the rendered element type of most tables in the app, and CSS behaves differently on table elements. Worth eyeballing Discover, Explore, Logs, Issues and a settings page.
Not in this PR
Row/Cellabstraction, so its ~41 consumers hand-roll<div>s, and the wrapper and cells are coupled per consumer (any intermediate state is invalid DOM). It gets its own earlier PR that introduces the abstraction first.resultGrid(app + gsAdmin) and the three key/value tables — separately tracked.Follow-ups
useVirtualRows:infiniteTableandlogsInfiniteTablenow use identical spacer-padding math, and replaysuseVirtualizedGridis a third copy (reopens DE-1398)scrollMarginis unset, so the virtual window is offset by the sticky header (~40px, absorbed byoverscantoday)dividerrow prop yet — their rules are entangled with head-row exclusion and radiusdisableHeaders/disableHeaderBorderBottomon PanelTable have zero consumersRef DE-1392, DE-1405
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.