DataTable: Add current-page row selection by rickyzhangca · Pull Request #8415 · primer/react

rickyzhangca · 2026-09-14T14:21:15Z

Refs github/primer#6718

Implements the row-selection stage for DataTable, building on the grouping support merged in #8386. This PR targets main directly and does not close the tracking issue: bulk actions remain a follow-up.

CleanShot.2026-09-14.at.10.28.16.AM.mp4

Changelog

New

Opt-in selection for flat, grouped, and mixed standalone/grouped rows.
rowSelection, selectedRows, defaultSelectedRows, onSelectionChange, and isRowSelectable props, using the existing getRowId for stable identity.
Composable Table.SelectionHeader and Table.RowSelection parts using Primer Checkbox.
Current-page select-all, mixed state, disabled rows, documentation, and flat/grouped/mixed examples.

Changed

Selection integrates with the private table model and includes the leading selection column in group spans and accessible header associations.
Sorting preserves selection by stable ID. Select-all covers every selectable supplied row, including rows outside the scroll viewport.

Removed

None.

Rollout strategy

Patch release
Minor release
Major release; if selected, include a written rollout or migration plan
None; if selected, include a brief description as to why

Includes a minor changeset. Selection is opt-in; existing consumers do not need to migrate.

Testing & Reviewing

State ownership and scope

selectedRows / defaultSelectedRows accept ReadonlySet<string | number>; onSelectionChange receives {selectedRows: Set<string | number>}.
Pagination/filter owners clear controlled selection on scope changes. A new data reference or changed membership alone does not reset selection or emit a callback.
Missing or currently non-selectable IDs remain in the owning set, but are excluded from effective selection and counts. Permanent removal is owner-reconciled; future bulk actions must use effective selection rather than blindly consume the stored set.
Mixed-state activation selects the remaining rows. With zero selectable rows, controls remain disabled and the header is unchecked/non-mixed.
Bulk toolbar, cross-page selection, group-level select-all, and collapsible groups are out of scope.

Automated validation on this HEAD

Build, full type-check, JS/TS lint, CSS lint, and formatting checks passed.
Full unit suite: 159 files passed; 2,704 tests passed, 2 skipped, 10 todo.
Focused DataTable suite: 183 tests passed across 8 files.
Playwright DataTable accessibility/interaction suite: 49 tests passed, including controlled selection round-trips, mixed-data sorting, themed Axe checks, and upstream grouping pagination.
Flat/grouped/mixed hydration tests verify selection ID references.

Reviewer focus / outstanding validation

Review the selection API and owner-reconciled reset/removal policy.
Check repeated row selection/deselection, accumulation of selected rows, mixed-to-all-to-clear, and focus preservation on select-all.
Stories: WithRowSelection, WithGroupedRowSelection, and WithMixedRowSelection under DataTable / Features.
Manual NVDA, JAWS, and VoiceOver validation remains outstanding, especially header announcements and navigation across group boundaries. Automated Axe and DOM checks do not establish screen-reader parity.
Selection stories have interaction/Axe coverage, not visual regression snapshots. The mixed story was also inspected in the browser.
Storybook MCP was unavailable; browser verification used local Playwright and the integrated browser instead.

Kept as draft pending review and manual accessibility validation.

Add controlled and uncontrolled selection for flat and grouped tables with accessible selection controls and regression coverage.\n\nRefs github/primer#6718 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

changeset-bot · 2026-09-14T14:21:22Z

🦋 Changeset detected

Latest commit: 76eb548

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2026-09-14T14:21:47Z

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

primer-integration · 2026-09-14T17:09:00Z

Integration test results from github/github-ui PR:

Changes Waiting
CI Failed
VRT Passed
Projects Passed

CI check runs linting, type checking, and unit tests. Check the workflow logs for specific failures.

Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance.

Copilot

Copilot review overview

🔵 Needs a closer look

A moderate accessibility issue remains unresolved in row checkbox fallback naming.

Review tier: Lite
Findings: None

What changed in this PR

Adds opt-in current-page row selection to DataTable, including controlled state, grouped rows, accessibility support, documentation, and tests.

Changes:

Added stable-ID selection APIs and select-all behavior.
Added composable selection components.
Added examples, accessibility documentation, browser tests, and a minor changeset.

File	Summary
`packages/react/src/experimental/index.ts`	Exports selection prop types.
`packages/react/src/DataTable/useTable.ts`	Manages selection state and derived row status.
`packages/react/src/DataTable/Table.tsx`	Adds composable selection parts.
`packages/react/src/DataTable/Table.module.css`	Styles selection controls.
`packages/react/src/DataTable/Table.features.stories.tsx`	Demonstrates composed selection.
`packages/react/src/DataTable/index.ts`	Exports selection components and types.
`packages/react/src/DataTable/DataTable.types.test.tsx`	Validates selection typings.
`packages/react/src/DataTable/DataTable.tsx`	Integrates selection into `DataTable`.
`packages/react/src/DataTable/DataTable.features.stories.tsx`	Adds selection examples.
`packages/react/src/DataTable/DataTable.docs.json`	Documents selection APIs.
`packages/react/src/DataTable/DataTable.accessibility.md`	Documents selection semantics.
`packages/react/src/DataTable/__tests__/DataTableSelection.test.tsx`	Tests selection behavior and hydration.
`packages/react/src/__tests__/__snapshots__/exports.test.ts.snap`	Updates export snapshots.
`e2e/components/Table.test.ts`	Tests composed-selection interactions and accessibility.
`e2e/components/DataTable.test.ts`	Tests DataTable selection scenarios.
`.changeset/selectable-data-table-rows.md`	Declares the minor release.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

joshblack

Looking great so far! Thanks for putting this up. The config-based API makes a lot of sense to me 💯 Left a couple of comments/questions and then also had some broader things:

One though, do you think it'd be nice for data table to start to break down the sections more? We don't really do this for other components but I bet for data table it's worth it
For select all, are we including the "select all" bit or is that in the batch actions stuff you mentioned is coming later on? It'd be nice to include that with pagination because without it the example is kind of a bummer 😞
I was curious about some of the demo controls (like the buttons to refresh pages), what are they for? Would they be better as dev stories?
For the selectable standalone and grouped repo story I was a little confused since I couldn't select the standalone one, what is the intended behavior?

Thanks again for putting this up! Hope these all make sense, can't wait to hear what you think 👀

joshblack · 2026-09-17T00:56:17Z

+  {
+    title: 'With Row Selection',
+    id: 'experimental-components-datatable-features--with-row-selection',
+    aat: true,


I guess I missed this in the last one but why did we need an aat flag for stories? 👀

it's storybook's automated a11y testing, somehow I thought we always want it, removed!

joshblack · 2026-09-17T00:57:17Z

 ]

 test.describe('DataTable', () => {
+  test('selection scope is owned by pagination and filtering @aat', async ({page}) => {


Organization-wise this should probably live after the loop tests we have (just as the convention we have for other ones)

joshblack · 2026-09-17T01:00:06Z

 ]

 test.describe('DataTable', () => {
+  test('selection scope is owned by pagination and filtering @aat', async ({page}) => {


Separately, what are we looking to test with this in Playwright? I wasn't quite sure so wanted to check. Is it that we want to run axe at the end?

not just for axe, mainly for testing the interactions

this end to end one is too long to follow, they are covered by the dev story anyway, removed!

joshblack · 2026-09-17T01:00:57Z

+    'experimental-components-datatable-features--with-grouped-row-selection',
+    'experimental-components-datatable-features--with-mixed-row-selection',
+  ]) {
+    test(`controlled selection round-trip ${id} @aat`, async ({page}) => {


Had a similar question with this one, just wanted to see what we wanted to capture with these tests in playwright 👀

checking checkboxes are clickable in no grouping + all rows are in groups + some rows are standalone and some are in groups

it's a bit hard to read, updated it to be testing all 3 cases in one some rows are standalone and some are in groups setting!

joshblack · 2026-09-17T01:07:29Z

+        {
+          "name": "checked",
+          "type": "boolean",
+          "required": true,
+          "description": "Whether every selectable row is selected."
+        },
+        {
+          "name": "indeterminate",
+          "type": "boolean",
+          "description": "Whether some, but not all, selectable rows are selected."
+        },


Would it help at all to collapse these into one prop? e.g. selection="all|some|none" just to make it more semantic and remove the ambiguity if checked and indeterminate are applied

joshblack · 2026-09-17T01:09:14Z

+        {
+          "name": "onChange",
+          "type": "React.ChangeEventHandler<HTMLInputElement>",
+          "description": "Handle changes to the native selection checkbox."
+        },


Do we need to expose the underlying checkbox even for this or could it be like an onToggleSelect kind of thing?

changed to onToggleSelect?: () => void!

joshblack · 2026-09-17T01:11:33Z

+        {
+          "name": "indeterminate",
+          "type": "boolean",
+          "description": "Whether the row is in a mixed selection state."
+        },


I wasn't sure what this one would mean with mixed row selection, could you share more about when this would show up? I assume for groups, potentially? 👀 The mixed row selection story didn't make it clear to me what was going on

yea you right, individual rows shouldn't have intermediate state, removed

“Mixed” referred to standalone rows alongside groups, not mixed row selection, fixed the name so it's clear, my bad

joshblack · 2026-09-17T01:11:51Z

+      ]
+    },
+    {
+      "name": "Table.RowSelection",


Similar prop-style questions to the header one above, curious what you think!

joshblack · 2026-09-17T01:13:12Z

 ]

+export const WithRowSelection = () => {
+  const [selectedRows, setSelectedRows] = React.useState<ReadonlySet<string | number>>(() => new Set([2]))


More of a general question, should we make the row id type consumable instead of doing string | number everywhere?

joshblack · 2026-09-17T01:16:38Z

+  ...rest
+}: TableSelectionHeaderProps) {
+  return (
+    <th


Random, is there a reason we couldn't use the TableHeader component directly and then add on top of it? My thought was that maybe it didn't allow us to override something that we wanted but wanted to check-in to see 👀

rickyzhangca · 2026-09-17T18:01:24Z

One though, do you think it'd be nice for data table to start to break down the sections more? We don't really do this for other components but I bet for data table it's worth it

totally, i also found Features getting so long, will make a follow up PR just to regroup stories

For select all, are we including the "select all" bit or is that in the batch actions stuff you mentioned is coming later on? It'd be nice to include that with pagination because without it the example is kind of a bummer 😞

sorry I didn't follow this, do you mean the ability to select all and do batch actions? this pr will enable selecting all, later on we add bulk actions

I was curious about some of the demo controls (like the buttons to refresh pages), what are they for? Would they be better as dev stories?

they are for demoing certain behaviours, for example the refresh button demos the selection will persist between data refreshes

For the selectable standalone and grouped repo story I was a little confused since I couldn't select the standalone one, what is the intended behavior?

yea that can be confusing, remove that disabled row

Address review feedback with semantic selection props and a shared row ID type. Simplify examples and browser coverage without changing current-page selection ownership. Refs github/primer#6718 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

DataTable: Add current-page row selection

82193de

Add controlled and uncontrolled selection for flat and grouped tables with accessible selection controls and regression coverage.\n\nRefs github/primer#6718 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Sep 14, 2026

rickyzhangca deployed to github-pages September 14, 2026 14:32 — with GitHub Actions View deployment

github-actions Bot temporarily deployed to storybook-preview-8415 September 14, 2026 14:33 Inactive

Implement row selection in DataTable component tests and documentation

f225a39

rickyzhangca deployed to github-pages September 14, 2026 16:33 — with GitHub Actions View deployment

github-actions Bot temporarily deployed to storybook-preview-8415 September 14, 2026 16:34 Inactive

rickyzhangca added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Sep 14, 2026

test(vrt): update snapshots

2a15882

github-actions Bot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Sep 14, 2026

primer Bot deployed to github-pages September 14, 2026 17:01 View deployment

github-actions Bot temporarily deployed to storybook-preview-8415 September 14, 2026 17:02 Inactive

github-actions Bot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Sep 14, 2026

rickyzhangca requested a lite review from Copilot September 14, 2026 18:42

rickyzhangca marked this pull request as ready for review September 14, 2026 18:42

rickyzhangca requested a review from a team as a code owner September 14, 2026 18:42

rickyzhangca requested a review from siddharthkp September 14, 2026 18:42

Copilot started reviewing on behalf of rickyzhangca September 14, 2026 18:43 View session

Copilot AI reviewed Sep 14, 2026

View reviewed changes

joshblack reviewed Sep 17, 2026

View reviewed changes

rickyzhangca deployed to github-pages September 17, 2026 18:36 — with GitHub Actions View deployment

github-actions Bot deployed to storybook-preview-8415 September 17, 2026 18:37 View deployment

Conversation

rickyzhangca commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

New

Changed

Removed

Rollout strategy

Testing & Reviewing

Uh oh!

changeset-bot Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions Bot commented Sep 14, 2026

⚠️ Action required

Uh oh!

primer-integration Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

joshblack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rickyzhangca Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rickyzhangca commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

rickyzhangca commented Sep 14, 2026 •

edited

Loading

changeset-bot Bot commented Sep 14, 2026 •

edited

Loading

primer-integration Bot commented Sep 14, 2026 •

edited

Loading

rickyzhangca Sep 17, 2026 •

edited

Loading