Skip to content

feat(scraps): Add a shared Table shell - #121178

Open
JoshuaKGoldberg wants to merge 1 commit into
de-1392-02-teams-table-stylesfrom
de-1392-03-core-table-shell
Open

feat(scraps): Add a shared Table shell#121178
JoshuaKGoldberg wants to merge 1 commit into
de-1392-02-teams-table-stylesfrom
de-1392-03-core-table-shell

Conversation

@JoshuaKGoldberg

@JoshuaKGoldberg JoshuaKGoldberg commented Aug 4, 2026

Copy link
Copy Markdown
Member

Stacked on top of #121177 -> #121176.

Adds core/table: a UI shell that owns only table geometry. That's <table>/<thead>/<tbody>/<th>/<td> on display: grid with grid-template-columns: subgrid, column tracks, and the resize drag (delegated to the existing useColumnResize consolidated in #120563).

Columns are declared as data because a resize needs stable identity and a known column count. Cell contents stay as children.

Split out of #120745, as part of DE-1392. See that PR for the full end state.

@linear-code

linear-code Bot commented Aug 4, 2026

Copy link
Copy Markdown

DE-1392

DE-1405

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Story previews

Preview the stories changed in this PR on the Vercel deployment:

Preview deployment: https://sentry-gk6soxxl7.sentry.dev

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📊 Type Coverage Diff

Metric Before After Delta
Coverage 94.06% 94.06% ±0%
Typed 136,931 137,052 🟢 +121
Untyped 8,652 8,653 🔴 +1
🔍 1 new type safety issue introduced

Type assertions (as) (1 new)

File Line Detail
static/app/components/core/table/__stories__/components.tsx 33 as keyof typeof rowcolumn.key as keyof typeof row

This is informational only and does not block the PR.

Sentry has two sanctioned table components that share no code, plus a third
de-facto shell leaking out of `gridEditable/styles`, two column-resize
implementations, and two competing column-track models.

Adds `core/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.

Nothing consumes it yet — the existing table components become facades over it
in follow-ups, so this lands with only its own tests and stories.

Ref DE-1392
Ref DE-1405
Comment on lines +139 to +142
&:active::after,
&:focus::after {
background-color: ${p => p.theme.tokens.focus.default};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The TableResizer component is not keyboard accessible, preventing users who rely on keyboard navigation from resizing table columns.
Severity: MEDIUM

Suggested Fix

To make the TableResizer accessible, add tabIndex="0" to make it focusable. Implement keyboard event handlers (e.g., onKeyDown) to allow resizing with arrow keys. Add appropriate ARIA roles (e.g., role="separator") and attributes (aria-valuenow, aria-valuemin, aria-valuemax, aria-controls) to communicate the resizer's state and function to assistive technologies.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/components/core/table/styles.tsx#L139-L142

Potential issue: The `TableResizer` component is implemented as a `<div>` that only
responds to mouse events like `onMouseDown`. It lacks keyboard event handlers, a
`tabIndex` to make it focusable, and appropriate ARIA roles to be discoverable by
assistive technologies. This violates WCAG 2.1 (Level A 2.1.1 Keyboard) which requires
that all functionality be operable through a keyboard. As a result, users who cannot use
a mouse are unable to resize table columns, making this feature inaccessible to them.

Also affects:

  • static/app/components/core/table/table.tsx:313~316

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryan953

ryan953 commented Aug 4, 2026

Copy link
Copy Markdown
Member

the header cells look like they need some rounded corners:
SCR-20260804-izfo

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2250019. Configure here.

render(<TestTable flexibleLastColumn={false} />);

expect(gridTemplate()).toBe('200px 150px minmax(90px, auto)');
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ineffective flexible column test

Medium Severity

The test that claims to pin the last column when flexibleLastColumn is false reuses COLUMNS, whose last entry has no declared width. For that config, true and false both produce 200px 150px minmax(90px, auto), so the assertion never exercises the pinning branch for a numeric last-column width.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2250019. Configure here.

justify-content: center;
`;

export const TableResizer = styled('div')`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance we could leverage @sentry/scraps/dragHandle for this? TableResizer could wrap it with the absolute positioning, but we might need to update the underlying component to support visible-only-on-hover? Also not documented yet!

Comment on lines +13 to +16
definiteHeadRow?: boolean;
fit?: 'max-content';
height?: CSSProperties['height'];
scrollable?: boolean;

@natemoo-re natemoo-re Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the props interface strikes me as a bit too opinionated for pure styled table primitives... could we consider simplifying to similar primitives as the markdown uses (which would ideally be swapped for these)?

to avoid coupling to our current API (which we'll hopefully go migrate), if either GridEditable or SimpleTable need these props for backwards-compat, we should put them in the local styled override.

export function DefaultTable({children}: {children: ReactNode}) {
return (
<Container border="primary" radius="md" overflowX="auto">
<StyledTable>{children}</StyledTable>
</Container>
);
}
const StyledTable = styled('table')`
min-width: 100%;
border-collapse: collapse;
`;
export const DefaultTableHead = styled('thead')`
background: ${p => p.theme.tokens.background.tertiary};
border-bottom: 4px solid ${p => p.theme.tokens.border.primary};
white-space: nowrap;
`;
export const DefaultTableBody = styled('tbody')`
background: ${p => p.theme.tokens.background.primary};
border-radius: 0 0 ${p => p.theme.radius.md} ${p => p.theme.radius.md};
`;
export const DefaultTableRow = styled('tr')`
border-bottom: 1px solid ${p => p.theme.tokens.border.secondary};
vertical-align: baseline;
&:last-child {
border-bottom: 0;
border-radius: 0 0 ${p => p.theme.radius.md} ${p => p.theme.radius.md};
}
`;
type Align = 'left' | 'center' | 'right';
export const DefaultTableHeaderCell = styled('th')<{align?: Align}>`
padding-inline: ${p => p.theme.space.xl};
padding-block: ${p => p.theme.space.sm};
text-align: ${p => p.align ?? 'left'};
&:first-of-type {
border-radius: ${p => p.theme.radius.md} 0 0 0;
}
&:last-of-type {
border-radius: 0 ${p => p.theme.radius.md} 0 0;
}
`;
export const DefaultTableCell = styled('td')<{align?: Align}>`
padding-inline: ${p => p.theme.space.xl};
padding-block: ${p => p.theme.space.lg};
text-align: ${p => p.align ?? 'left'};
`;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants