feat(scraps): Add a shared Table shell - #121178
Conversation
Story previewsPreview the stories changed in this PR on the Vercel deployment: Preview deployment: https://sentry-gk6soxxl7.sentry.dev |
📊 Type Coverage Diff
🔍 1 new type safety issue introducedType assertions (
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
b5d8bd4 to
2250019
Compare
| &:active::after, | ||
| &:focus::after { | ||
| background-color: ${p => p.theme.tokens.focus.default}; | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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)'); | ||
| }); |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 2250019. Configure here.
| justify-content: center; | ||
| `; | ||
|
|
||
| export const TableResizer = styled('div')` |
There was a problem hiding this comment.
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!
| definiteHeadRow?: boolean; | ||
| fit?: 'max-content'; | ||
| height?: CSSProperties['height']; | ||
| scrollable?: boolean; |
There was a problem hiding this comment.
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.
sentry/static/app/components/core/markdown/defaultComponents.tsx
Lines 157 to 209 in 1ed2aff



Stacked on top of #121177 -> #121176.
Adds
core/table: a UI shell that owns only table geometry. That's<table>/<thead>/<tbody>/<th>/<td>ondisplay: gridwithgrid-template-columns: subgrid, column tracks, and the resize drag (delegated to the existinguseColumnResizeconsolidated 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.