Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/selectable-data-table-rows.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

DataTable: Add controlled and uncontrolled row selection.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
138 changes: 138 additions & 0 deletions e2e/components/DataTable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,128 @@ const stories: ReadonlyArray<{
id: 'experimental-components-datatable-features--with-groups',
aat: true,
},
{
title: 'With Row Selection',
id: 'experimental-components-datatable-features--with-row-selection',
aat: true,
},
{
title: 'With Grouped Row Selection',
id: 'experimental-components-datatable-features--with-grouped-row-selection',
aat: true,
},
{
title: 'With Sortable Groups',
id: 'experimental-components-datatable-features--with-sortable-groups',
aat: true,
},
{
title: 'With Mixed Row Selection',
id: 'experimental-components-datatable-features--with-mixed-row-selection',
aat: true,
},
{
title: 'With Paginated Row Selection',
id: 'experimental-components-datatable-features--with-paginated-row-selection',
aat: true,
},
]

test.describe('DataTable', () => {
test('selection scope is owned by pagination and filtering @aat', async ({page}) => {
await visit(page, {id: 'experimental-components-datatable-features--with-paginated-row-selection'})
const selectAll = page.getByRole('checkbox', {name: 'Select rows', exact: true})
const firstRepository = page.getByRole('checkbox', {name: 'Select public/repository-1', exact: true})
const secondRepository = page.getByRole('checkbox', {name: 'Select public/repository-2', exact: true})
const checkedRows = page.locator('tbody input[type="checkbox"]:checked')

await expect(selectAll).toHaveAccessibleDescription('Select all 10 rows')
await firstRepository.click()
await expect(selectAll).toBeChecked({indeterminate: true})
await page.getByRole('button', {name: 'Name', exact: true}).click()
await page.getByRole('button', {name: 'Refresh rows', exact: true}).click()
await expect(firstRepository).toBeChecked()
await expect(selectAll).toBeChecked({indeterminate: true})
await secondRepository.click()
await expect(firstRepository).toBeChecked()
await expect(secondRepository).toBeChecked()

await page.getByRole('button', {name: 'Next page', exact: true}).click()
await expect(page.getByRole('rowheader')).toHaveCount(7)
await expect(checkedRows).toHaveCount(0)
await expect(selectAll).not.toBeChecked()
await expect(selectAll).not.toBeChecked({indeterminate: true})
await selectAll.click()
await expect(checkedRows).toHaveCount(7)
await expect(selectAll).toBeFocused()

await page.getByRole('button', {name: 'Previous page', exact: true}).click()
await expect(checkedRows).toHaveCount(0)
await expect(firstRepository).not.toBeChecked()
await expect(secondRepository).not.toBeChecked()
await page.getByRole('button', {name: 'Next page', exact: true}).click()
const retainedRepository = page.getByRole('checkbox', {name: 'Select public/repository-10', exact: true})
await retainedRepository.click()

const filter = page.getByRole('button', {name: 'Public only', exact: true})
await filter.click()
await expect(filter).toHaveAttribute('aria-pressed', 'true')
await expect(page.getByRole('rowheader')).toHaveCount(10)
await expect(retainedRepository).not.toBeChecked()
await expect(checkedRows).toHaveCount(0)
await firstRepository.click()
await page.getByRole('button', {name: 'Refresh rows', exact: true}).click()
await expect(firstRepository).toBeChecked()

await filter.click()
await expect(firstRepository).not.toBeChecked()
await expect(checkedRows).toHaveCount(0)
await expect(selectAll).not.toBeChecked({indeterminate: true})
await expect(page).toHaveNoViolations()
})

for (const id of [
'experimental-components-datatable-features--with-row-selection',
'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}) => {
await visit(page, {id})
const rowCheckbox = page.getByRole('checkbox', {name: 'Select strapi', exact: true})
const otherCheckbox = page.getByRole('checkbox', {name: 'Select bootstrap', exact: true})
const selectAll = page.getByRole('checkbox', {name: 'Select rows', exact: true})

await expect(rowCheckbox).not.toBeChecked()
await rowCheckbox.click()
await expect(rowCheckbox).toBeChecked()
await rowCheckbox.click()
await expect(rowCheckbox).not.toBeChecked()
await expect(rowCheckbox).toBeFocused()

await rowCheckbox.press('Space')
await otherCheckbox.click()
await expect(rowCheckbox).toBeChecked()
await expect(otherCheckbox).toBeChecked()
await selectAll.click()
await expect(selectAll).toBeChecked()
await rowCheckbox.click()
await expect(rowCheckbox).not.toBeChecked()
await expect(otherCheckbox).toBeChecked()
await expect(selectAll).toBeChecked({indeterminate: true})
if (id === 'experimental-components-datatable-features--with-mixed-row-selection') {
await page.getByRole('button', {name: 'Name', exact: true}).click()
await expect(rowCheckbox).not.toBeChecked()
await expect(otherCheckbox).toBeChecked()
await expect(selectAll).toBeChecked({indeterminate: true})
}
await selectAll.click()
await selectAll.click()
await expect(selectAll).not.toBeChecked()
await expect(selectAll).toBeFocused()
await expect(page).toHaveNoViolations()
})
}

for (const story of stories) {
test.describe(story.title, () => {
for (const theme of themes) {
Expand All @@ -79,6 +193,7 @@ test.describe('DataTable', () => {
// Default state
expect(
await page.screenshot({
animations: 'disabled',
mask: await page
.locator('td', {
has: page.locator('relative-time'),
Expand All @@ -88,6 +203,29 @@ test.describe('DataTable', () => {
).toMatchSnapshot(`DataTable.${story.title}.${theme}.png`)
})

if (story.id === 'experimental-components-datatable-features--with-grouped-row-selection') {
for (const state of ['mixed', 'all selected'] as const) {
test(`${state} @vrt`, async ({page}) => {
await visit(page, {id: story.id, globals: {colorScheme: theme}})
const selectAll = page.getByRole('checkbox', {name: 'Select rows', exact: true})
if (state === 'mixed') {
await page.getByRole('checkbox', {name: 'Select strapi', exact: true}).click()
await expect(selectAll).toBeChecked({indeterminate: true})
} else {
await selectAll.click()
await expect(selectAll).toBeChecked()
}
await expect(page.getByRole('checkbox', {name: 'Select codeql-dca-worker'})).toBeDisabled()
expect(
await page.screenshot({
animations: 'disabled',
mask: await page.locator('td', {has: page.locator('relative-time')}).all(),
}),
).toMatchSnapshot(`DataTable.${story.title}.${state}.${theme}.png`)
})
}
}

if (story.aat) {
test('axe @aat', async ({page}) => {
await visit(page, {
Expand Down
82 changes: 82 additions & 0 deletions e2e/components/Table.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,88 @@ import {test, expect} from '@playwright/test'
import {visit} from '../test-helpers/storybook'
import {themes} from '../test-helpers/themes'

test.describe('Table With Row Selection', () => {
const storyId = 'experimental-components-table-features--with-row-selection'

for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {id: storyId, globals: {colorScheme: theme}})
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(`Table.With Row Selection.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {id: storyId, globals: {colorScheme: theme}})
await expect(page).toHaveNoViolations()
})
})
}

test('consumer-owned selection and header associations @aat', async ({page}) => {
await visit(page, {id: storyId})
const table = page.getByRole('table', {name: 'Selectable repositories using Table'})
const selectionColumn = table.getByRole('columnheader', {name: 'Select rows', exact: true})
const selectionColumnId = await selectionColumn.getAttribute('id')
const selectAll = selectionColumn.getByRole('checkbox', {name: 'Select rows', exact: true})
const react = table.getByRole('checkbox', {name: 'Select primer/react', exact: true})
const css = table.getByRole('checkbox', {name: 'Select primer/css', exact: true})
const disabled = table.getByRole('checkbox', {name: 'Select github/github', exact: true})

await expect(selectAll).toHaveAccessibleDescription('Select all 2 rows')
await expect(selectAll).toBeChecked({indeterminate: true})
await expect(react).toBeChecked()
await expect(css).not.toBeChecked()
await expect(disabled).toBeDisabled()
await expect(disabled).not.toBeChecked()

for (const group of [
{name: /^Internal\s*, 1 row$/, rows: ['github/github']},
{name: /^Public\s*, 2 rows$/, rows: ['primer/react', 'primer/css']},
]) {
const groupHeader = table.getByRole('columnheader', {name: group.name})
await expect(groupHeader).toHaveAttribute('colspan', '3')
const groupHeaderId = await groupHeader.getAttribute('id')

for (const name of group.rows) {
const rowHeaderId = await table.getByRole('rowheader', {name, exact: true}).getAttribute('id')
const checkbox = table.getByRole('checkbox', {name: `Select ${name}`, exact: true})
const cell = table.getByRole('cell', {name: `Select ${name}`, exact: true})
await expect(cell).toHaveAttribute('headers', `${groupHeaderId} ${selectionColumnId}`)
const labelledBy = await checkbox.getAttribute('aria-labelledby')
expect(labelledBy?.split(' ')).toContain(rowHeaderId)
}
}

await react.click()
await expect(react).not.toBeChecked()
await expect(selectAll).not.toBeChecked()
await expect(selectAll).not.toBeChecked({indeterminate: true})
await selectAll.focus()
await page.keyboard.press('Space')
await expect(react).toBeChecked()
await expect(css).toBeChecked()
await expect(selectAll).toBeFocused()
await expect(disabled).not.toBeChecked()

await css.click()
await expect(css).not.toBeChecked()
await expect(react).toBeChecked()
await expect(selectAll).toBeChecked({indeterminate: true})
await selectAll.focus()
await page.keyboard.press('Space')
await expect(selectAll).toBeChecked()
await expect(selectAll).not.toBeChecked({indeterminate: true})
await expect(selectAll).toBeFocused()
await page.keyboard.press('Space')
await expect(selectAll).not.toBeChecked()
await expect(react).not.toBeChecked()
await expect(css).not.toBeChecked()
await expect(selectAll).toBeFocused()
await expect(selectionColumn).toHaveAccessibleName('Select rows')
await expect(page).toHaveNoViolations()
})
})

test.describe('Table With Groups', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
64 changes: 59 additions & 5 deletions packages/react/src/DataTable/DataTable.accessibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ assigning different responsibilities to consumers.
| Localized group name | Consumer may provide `aria-label` | Consumer may provide `aria-label` in the group data |
| Column span | Consumer provides `colSpan` | Derived from the column count |
| Group header ID | Generated by `Table.Group` | Inherited from `Table.Group` |
| Column header IDs | Consumer provides stable IDs | Generated for grouped tables |
| Column header IDs | Consumer provides stable IDs | Generated for grouped tables and the selection column |
| Row header | Consumer renders `Table.Cell scope="row"` | Consumer marks a column with `rowHeader` |
| Cell header associations | Consumer references column and row headers; `Table.Group` prepends the group header | Automatically references group, row, and column headers |
| Ungrouped tables | Native `scope` associations | Preserves native `scope` associations without explicit ID references |
| Ungrouped tables | Native `scope` associations | Native `scope` for data cells; ID references for selection controls |
| Sorting | Consumer manages row order | Sorts each group and contiguous standalone-row run independently |
| Empty groups | Always renders the member `<tbody>` | Preserves the empty member `<tbody>` |
| Nested tables | Clears inherited group context | Inherited from the presentational API |
Expand All @@ -42,8 +42,9 @@ An optional subtitle may be referenced by `aria-describedby`. These references
must point to rendered elements. Use the native table structure rather than
turning the table into an interactive grid.

Ungrouped tables continue to use native column and row `scope` associations.
Grouping must not add explicit `headers` attributes to the ungrouped path.
Without row selection, ungrouped tables use native column and row `scope`
associations without explicit `headers` attributes. Selection adds ID references
for the selection column and checkbox labels.

```html
<h2 id="repositories-title">Repositories</h2>
Expand Down Expand Up @@ -112,7 +113,8 @@ prepends it to member cells' references.
In mixed tables, each contiguous run of standalone rows renders in its own
`<tbody>`, as a sibling of the group sections. Standalone cells reference only
their row and column headers, never an adjacent group's header. Entirely
ungrouped tables retain native `scope` associations without generated IDs.
ungrouped tables without row selection retain native `scope` associations
without generated IDs.

With direct composition, consumers supply column IDs, row-header IDs, and the
corresponding `headers` references. `Table.Group` generates its own header ID
Expand Down Expand Up @@ -207,13 +209,61 @@ Generated IDs must not collide between table instances, and every `headers`
reference must resolve before and after hydration. Direct-composition consumers
must likewise provide instance-scoped, SSR-safe IDs.

## Row selection

`DataTable` manages selection by stable row ID. With `Table.SelectionHeader`
and `Table.RowSelection`, consumers own state, counts, and header references.

### Names and associations

Use `Select rows` as the header name and put the count in its description,
per the [follow-up audit](https://github.com/github/accessibility/issues/10807).
Row checkboxes reference the visible row header, not a duplicated hidden name.
The fragments below show these associations; `sr-only` denotes hidden text.

```html
<th id="selection-column" scope="col">
<input type="checkbox" aria-label="Select rows" aria-description="Select all 2 rows" />
</th>
<!-- Selection cell and row header -->
<td headers="selection-column">
<span id="select-react" class="sr-only">Select</span>
<input type="checkbox" aria-labelledby="select-react react-row" />
</td>
<th id="react-row" scope="row">primer/react</th>
```

Without a row header, `DataTable` uses `Select row {id}` from `getRowId`.
Place selection first and include it in group `colSpan`. `Table.Group` prepends
its header ID to selection cells' `headers`; group headings have no checkbox.

### State, scope, and focus

- Select-all includes every selectable supplied row across groups and outside
the scroll viewport. Disabled rows do not affect selection totals.
- `Checkbox` sets the native `indeterminate` property for mixed state.
Activating a mixed header selects the remaining rows; a checked header clears them.
- With no selectable rows, keep disabled, unchecked, non-mixed checkboxes and
omit the header's count description.
- Page/filter owners clear controlled `selectedRows` when scope changes.
Sorting and replacement data preserve selection and do not emit `onSelectionChange`.
- Missing or non-selectable IDs stay stored and become selected again if eligible.
Consumers reconcile permanent removals; uncontrolled selection is for static pages.
- Preserve native Tab/Space behavior and keep the focused checkbox mounted after
selection changes. Do not add row-level `aria-selected` or arrow-key navigation.

Examples: `WithRowSelection` in [Table features](./Table.features.stories.tsx)
and `WithPaginatedRowSelection` in [DataTable features](./DataTable.features.stories.tsx).

## Verification

- [Grouping tests](./__tests__/DataTableGrouping.test.tsx) cover mixed body
structure, independent sorting runs, row identity, replacement data, empty
groups, and the server-rendered/hydrated ID-reference graph across multiple tables.
- [Presentational group tests](./__tests__/TableGroup.test.tsx) cover native
spanning headers, accessible names, and nested-table isolation.
- [Selection tests](./__tests__/DataTableSelection.test.tsx) cover controlled
updates, disabled rows, focus, and hydration.
- [DataTable browser tests](../../../../e2e/components/DataTable.test.ts) cover
themed axe checks, visual snapshots, and a group split across pages.
- [Table browser tests](../../../../e2e/components/Table.test.ts) cover explicit
Expand All @@ -224,12 +274,16 @@ testing. For changes to these semantics, verify cell navigation through group
boundaries with NVDA and JAWS, and check VoiceOver while accounting for its
documented spanning-header limitation. Verify that sorting controls announce
their name, state, and next action without polluting cell header names.
For selection, also verify mixed-state announcements and focus after select-all.

## Links and resources

- [Tetralogical audit: verbose interactive header names](https://github.com/github/accessibility/issues/10272)
- [Multi-level headers and native colspan investigation](https://github.com/github/accessibility/issues/10885)
- [Follow-up accessibility audit tracking](https://github.com/github/accessibility/issues/10807)
- [Selection focus audit](https://github.com/github/accessibility/issues/10268)
- [Select-all naming audit](https://github.com/github/accessibility/issues/10271)
- [Agreed row-selection scope and ownership](https://github.com/github/primer/issues/6718#issuecomment-5588699022)
- [Grouping requirements and implementation discussion](https://github.com/github/primer/issues/6720)
- [Presentational grouping implementation](https://github.com/primer/react/pull/8372)
- [Sortable-header name and description implementation](https://github.com/primer/react/pull/8371)
Expand Down
Loading
Loading