diff --git a/.changeset/datatable-integrated-pagination.md b/.changeset/datatable-integrated-pagination.md new file mode 100644 index 00000000000..00b7ad91475 --- /dev/null +++ b/.changeset/datatable-integrated-pagination.md @@ -0,0 +1,13 @@ +--- +'@primer/react': minor +--- + +DataTable: Add integrated pagination via the `pagination` prop. Pass +`pagination={true}` (or an options object) and `DataTable` renders the +existing `` for you and slices the rows automatically — +consumers no longer need to wire `` and manual row +slicing themselves. Supports controlled (`pageIndex` / `onPageChange`) and +uncontrolled modes, plus an `externalPagination` escape hatch for +server-driven pagination. The existing manual `` +composition pattern continues to work unchanged for callers that want +finer control. diff --git a/packages/react/src/DataTable/DataTable.docs.json b/packages/react/src/DataTable/DataTable.docs.json index 3b0f5c7006e..95ed1bc5ff1 100644 --- a/packages/react/src/DataTable/DataTable.docs.json +++ b/packages/react/src/DataTable/DataTable.docs.json @@ -42,6 +42,15 @@ }, { "id": "experimental-components-datatable-features--with-pagination" + }, + { + "id": "experimental-components-datatable-features--with-integrated-pagination" + }, + { + "id": "experimental-components-datatable-features--with-integrated-pagination-controlled" + }, + { + "id": "experimental-components-datatable-features--with-integrated-pagination-external" } ], "importPath": "@primer/react/experimental", @@ -112,6 +121,34 @@ "required": false, "description": "Fires every time the user clicks a sortable column header. It reports the column id that is now sorted and the direction after the toggle (never 'NONE').", "defaultValue": "" + }, + { + "name": "pagination", + "type": "false | true | { pageSize?: number; defaultPageIndex?: number; 'aria-label'?: string; showPages?: boolean | ResponsiveValue }", + "required": false, + "description": "Render an integrated pagination control beneath the table. Pass `true` for defaults, an options object to customize, or omit to opt out and continue composing `` manually.", + "defaultValue": "false" + }, + { + "name": "pageIndex", + "type": "number", + "required": false, + "description": "Controlled page index. When provided, the parent owns the page state and `pagination.defaultPageIndex` is ignored. Pair with `onPageChange`.", + "defaultValue": "" + }, + { + "name": "onPageChange", + "type": "(pageIndex: number) => void", + "required": false, + "description": "Called whenever the page index changes (controlled or uncontrolled).", + "defaultValue": "" + }, + { + "name": "externalPagination", + "type": "boolean", + "required": false, + "description": "When `true`, disables client-side row slicing. The pagination control still renders and `onPageChange` still fires, but `data` is rendered as-is. Use for server-driven pagination.", + "defaultValue": "false" } ], "subcomponents": [ diff --git a/packages/react/src/DataTable/DataTable.features.stories.tsx b/packages/react/src/DataTable/DataTable.features.stories.tsx index 6563a3054bd..db9729461cc 100644 --- a/packages/react/src/DataTable/DataTable.features.stories.tsx +++ b/packages/react/src/DataTable/DataTable.features.stories.tsx @@ -1715,3 +1715,98 @@ export const WithNetworkError = () => { ) } + +export const WithIntegratedPagination = () => ( + + + Repositories + + + The `pagination` prop renders an integrated pager and slices the rows for you. No manual `Table.Pagination` wiring + needed. + + , + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => , + }, + ]} + pagination={{pageSize: 10, 'aria-label': 'Pagination for Repositories'}} + /> + +) + +export const WithIntegratedPaginationControlled = () => { + const [pageIndex, setPageIndex] = React.useState(0) + return ( + + + Repositories + + + The parent owns `pageIndex`. Combine with custom buttons or a URL query parameter for deep-linkable pagination. + + { + action('onPageChange')(next) + setPageIndex(next) + }} + /> + + ) +} + +export const WithIntegratedPaginationExternal = () => { + const pageSize = 10 + const [pageIndex, setPageIndex] = React.useState(0) + // Simulate server-side pagination: only this page's slice is in scope. + const start = pageIndex * pageSize + const visible = repos.slice(start, start + pageSize) + return ( + + + Repositories + + + `externalPagination` lets the consumer fetch one page of data at a time. The component renders whatever rows are + in `data`. + + { + action('onPageChange')(next) + setPageIndex(next) + }} + /> + + ) +} diff --git a/packages/react/src/DataTable/DataTable.tsx b/packages/react/src/DataTable/DataTable.tsx index ea682fe3b90..23f186c44aa 100644 --- a/packages/react/src/DataTable/DataTable.tsx +++ b/packages/react/src/DataTable/DataTable.tsx @@ -1,10 +1,12 @@ -import type React from 'react' +import React from 'react' import type {Column} from './column' import {useTable} from './useTable' import type {SortDirection} from './sorting' import type {UniqueRow} from './row' import type {ObjectPaths} from './utils' import {Table, TableHead, TableBody, TableRow, TableHeader, TableSortHeader, TableCell} from './Table' +import {Pagination} from './Pagination' +import type {ResponsiveValue} from '../hooks/useResponsiveValue' // ---------------------------------------------------------------------------- // DataTable @@ -73,6 +75,46 @@ export type DataTableProps = { * (never `"NONE"`). */ onToggleSort?: (columnId: ObjectPaths | string | number, direction: Exclude) => void + + /** + * Render an integrated pagination control beneath the table. Pass `false` + * (or omit) to opt out and continue composing `` + * manually. + * + * Provide either `true` to use defaults, or an options object: + * - `pageSize` — items per page (default `25`) + * - `defaultPageIndex` — initial page index (uncontrolled mode only) + * - `aria-label` — landmark label (default `'Pagination'`) + * - `showPages` — show numbered pages (default `{narrow: false}`) + */ + pagination?: + | false + | true + | { + pageSize?: number + defaultPageIndex?: number + 'aria-label'?: string + showPages?: boolean | ResponsiveValue + } + + /** + * Controlled page index. When provided, the parent owns the page state and + * `defaultPageIndex` is ignored. Pair with `onPageChange`. + */ + pageIndex?: number + + /** + * Called whenever the page index changes (controlled or uncontrolled). + */ + onPageChange?: (pageIndex: number) => void + + /** + * When `true`, disables client-side row slicing. The pagination control + * still renders and `onPageChange` still fires, but `data` is rendered + * as-is. Use this for server-driven pagination where the consumer fetches + * one page at a time. + */ + externalPagination?: boolean } function defaultGetRowId(row: D) { @@ -88,9 +130,46 @@ function DataTable({ initialSortColumn, initialSortDirection, externalSorting, + externalPagination, + pagination, + pageIndex, + onPageChange, getRowId = defaultGetRowId, onToggleSort, }: DataTableProps) { + // Normalize the `pagination` prop. `true` is a shortcut for defaults; an + // object provides explicit overrides; `false`/`undefined` keeps the + // pre-existing behaviour (no integrated pagination — consumers can still + // compose `` themselves). + const paginationOptions = pagination === true ? {} : pagination || undefined + const paginationEnabled = paginationOptions !== undefined + // Defensive clamp so a bogus `pageSize` (0, negative, NaN) cannot produce + // Infinity in ``'s page-count math. + const rawPageSize = paginationOptions?.pageSize ?? 25 + const pageSize = Number.isFinite(rawPageSize) && rawPageSize > 0 ? Math.floor(rawPageSize) : 25 + const defaultPageIndex = paginationOptions?.defaultPageIndex ?? 0 + const paginationAriaLabel = paginationOptions?.['aria-label'] ?? 'Pagination' + + const isControlledPage = pageIndex !== undefined + // All uncontrolled page state lives in a single object so the + // "reset when data identity changes" path is a single setState rather + // than three — keeps the React reconciler happy and prevents cascading + // renders under Strict / Concurrent mode. Documented React pattern: + // https://react.dev/reference/react/useState#storing-information-from-previous-renders + const [pageState, setPageState] = React.useState<{ + pageIndex: number + resetKey: number + prevData: ReadonlyArray + }>(() => ({pageIndex: defaultPageIndex, resetKey: 0, prevData: data})) + const controlledPageIndex = isControlledPage ? Math.max(0, pageIndex as number) : undefined + const effectivePageIndex = controlledPageIndex ?? pageState.pageIndex + if (!isControlledPage && pageState.prevData !== data) { + // Derived-state-from-props reset. The functional setState body is + // idempotent and only triggers an additional render the first time + // we see a new `data` identity. + setPageState(prev => ({pageIndex: 0, resetKey: prev.resetKey + 1, prevData: data})) + } + const {headers, rows, actions, gridTemplateColumns} = useTable({ data, columns, @@ -100,59 +179,118 @@ function DataTable({ externalSorting, }) + // Slice the sorted rows down to the visible page when integrated pagination + // is enabled and the consumer hasn't taken over with externalPagination. + let visibleRows = rows + let totalCount = rows.length + if (paginationEnabled) { + if (externalPagination) { + // Consumer is feeding one page of data already. `data.length` is the + // page size; totalCount is unknown to us, but Pagination needs a + // sensible value to compute its model. Default to the larger of + // (pageIndex+1)*pageSize and the visible row count so the "next" + // button stays enabled while there might be more pages. + totalCount = Math.max(rows.length, (effectivePageIndex + 1) * pageSize + 1) + } else { + // Clamp the slice window to valid bounds so a bogus pageIndex (out + // of range in controlled mode, or stale across data shrinks) can't + // produce an empty page when one exists. + const pageCount = Math.max(1, Math.ceil(rows.length / pageSize)) + const clampedIndex = Math.min(Math.max(0, effectivePageIndex), pageCount - 1) + const pageStart = clampedIndex * pageSize + const pageEnd = pageStart + pageSize + visibleRows = rows.slice(pageStart, pageEnd) + // Ensure Pagination sees at least one page even when the dataset is + // empty, otherwise its `defaultPageIndex` validation logs a warning + // about an out-of-range index. + totalCount = Math.max(rows.length, 1) + } + } + return ( - - - - {headers.map(header => { - if (header.isSortable()) { + <> +
+ + + {headers.map(header => { + if (header.isSortable()) { + return ( + { + const nextDirection: Exclude = + header.getSortDirection() === 'ASC' ? 'DESC' : 'ASC' + actions.sortBy(header) + onToggleSort?.(header.id, nextDirection) + }} + > + {typeof header.column.header === 'string' ? header.column.header : header.column.header()} + + ) + } return ( - { - const nextDirection: Exclude = - header.getSortDirection() === 'ASC' ? 'DESC' : 'ASC' - actions.sortBy(header) - onToggleSort?.(header.id, nextDirection) - }} - > + {typeof header.column.header === 'string' ? header.column.header : header.column.header()} - + ) - } + })} + + + + {visibleRows.map(row => { return ( - - {typeof header.column.header === 'string' ? header.column.header : header.column.header()} - + + {row.getCells().map(cell => { + return ( + + {cell.column.renderCell + ? cell.column.renderCell(row.getValue()) + : (cell.getValue() as React.ReactNode)} + + ) + })} + ) })} - - - - {rows.map(row => { - return ( - - {row.getCells().map(cell => { - return ( - - {cell.column.renderCell - ? cell.column.renderCell(row.getValue()) - : (cell.getValue() as React.ReactNode)} - - ) - })} - - ) - })} - -
+ + + {paginationEnabled ? ( + // The component owns its own UI page state. We must + // pass `defaultPageIndex` only when intending to (re)initialise + // that internal state — otherwise Pagination's render-time + // `defaultPageIndex` sync would call back into DataTable's + // setState during Pagination's render, triggering React's + // cross-component setState warning. + // + // - Controlled mode: every new `pageIndex` value bumps the remount + // key so the parent's value takes effect cleanly. + // - Uncontrolled mode: we never change the prop after mount; + // navigation updates Pagination's internal state directly and we + // mirror it via onChange for row slicing. Data-identity changes + // bump the reset counter so Pagination remounts and clamps. + { + if (!isControlledPage) { + setPageState(prev => ({...prev, pageIndex: nextPageIndex})) + } + onPageChange?.(nextPageIndex) + }} + /> + ) : null} + ) } diff --git a/packages/react/src/DataTable/__tests__/integrated-pagination.test.tsx b/packages/react/src/DataTable/__tests__/integrated-pagination.test.tsx new file mode 100644 index 00000000000..13b15071aeb --- /dev/null +++ b/packages/react/src/DataTable/__tests__/integrated-pagination.test.tsx @@ -0,0 +1,204 @@ +import {describe, expect, it, vi} from 'vitest' +import userEvent from '@testing-library/user-event' +import {render, screen} from '@testing-library/react' +import {DataTable} from '../../DataTable' +import {createColumnHelper} from '../column' + +type Item = {id: number; name: string} + +function makeItems(n: number): Item[] { + return Array.from({length: n}, (_, i) => ({id: i + 1, name: `item-${i + 1}`})) +} + +function buildColumns() { + const ch = createColumnHelper() + return [ + ch.column({header: 'ID', field: 'id', rowHeader: true, sortBy: 'basic'}), + ch.column({header: 'Name', field: 'name'}), + ] +} + +describe('DataTable integrated pagination', () => { + describe('opt-in', () => { + it('does not render a pagination nav when the prop is omitted', () => { + render() + expect(screen.queryByRole('navigation', {name: /pagination/i})).not.toBeInTheDocument() + }) + + it('renders a pagination nav when `pagination={true}`', () => { + render() + expect(screen.getByRole('navigation', {name: /pagination/i})).toBeInTheDocument() + }) + + it('renders a pagination nav when an options object is provided', () => { + render( + , + ) + expect(screen.getByRole('navigation', {name: 'Repo pagination'})).toBeInTheDocument() + }) + }) + + describe('row slicing', () => { + it('slices the row order to `pageSize` items per page', () => { + render( + , + ) + // 1 header row + 10 data rows + expect(screen.getAllByRole('row')).toHaveLength(11) + // First page contains items 1..10. + expect(screen.getByText('item-1')).toBeInTheDocument() + expect(screen.getByText('item-10')).toBeInTheDocument() + expect(screen.queryByText('item-11')).not.toBeInTheDocument() + }) + + it('navigating to the next page slices the next window', async () => { + const user = userEvent.setup() + render( + , + ) + await user.click(screen.getByRole('button', {name: /next/i})) + expect(screen.queryByText('item-10')).not.toBeInTheDocument() + expect(screen.getByText('item-11')).toBeInTheDocument() + expect(screen.getByText('item-20')).toBeInTheDocument() + }) + + it('respects `defaultPageIndex` for the initial render', () => { + render( + , + ) + // Third page (index 2) contains items 21..30. + expect(screen.getByText('item-21')).toBeInTheDocument() + expect(screen.queryByText('item-1')).not.toBeInTheDocument() + }) + + it('handles an empty dataset without crashing', () => { + render() + expect(screen.getAllByRole('row')).toHaveLength(1) // header only + // Pagination still renders (1 of 1) so consumers see consistent chrome. + expect(screen.getByRole('navigation', {name: /pagination/i})).toBeInTheDocument() + }) + + it('resets to the first page when the data identity changes', () => { + const first = makeItems(30) + const second = makeItems(5) + const {rerender} = render( + , + ) + expect(screen.getByText('item-21')).toBeInTheDocument() + rerender() + // Only 5 items in the new data, all on page 0. + expect(screen.getByText('item-1')).toBeInTheDocument() + expect(screen.getByText('item-5')).toBeInTheDocument() + }) + }) + + describe('controlled mode', () => { + it('respects the `pageIndex` prop and ignores user clicks unless the parent updates it', async () => { + const user = userEvent.setup() + const onPageChange = vi.fn() + const {rerender} = render( + , + ) + expect(screen.getByText('item-1')).toBeInTheDocument() + await user.click(screen.getByRole('button', {name: /next/i})) + // Internal state cannot advance because pageIndex={0} is controlled — + // but onPageChange should still fire so the parent can react. + expect(onPageChange).toHaveBeenLastCalledWith(1) + // Page contents stay on page 0 because the parent hasn't bumped pageIndex. + expect(screen.getByText('item-1')).toBeInTheDocument() + expect(screen.queryByText('item-11')).not.toBeInTheDocument() + + rerender( + , + ) + expect(screen.getByText('item-11')).toBeInTheDocument() + }) + }) + + describe('externalPagination', () => { + it('does not slice rows when externalPagination is true', () => { + // The consumer is responsible for fetching one page of data at a time; + // the component renders whatever `data` it receives. + render( + , + ) + // All 5 rows visible despite pageSize 10. + expect(screen.getAllByRole('row')).toHaveLength(6) + expect(screen.getByText('item-5')).toBeInTheDocument() + }) + + it('still fires onPageChange so the consumer can fetch the next page', async () => { + const user = userEvent.setup() + const onPageChange = vi.fn() + // Simulate a server-paginated context where the consumer feeds the + // component just the current page (10 of a notional 30 total). The + // component must not slice further — externalPagination defers that + // to the parent — and must still fire onPageChange so the parent + // can fetch the next slice. + const page1 = makeItems(30).slice(0, 10) + render( + , + ) + // All 10 rows the consumer supplied are visible (no further slicing). + expect(screen.getAllByRole('row')).toHaveLength(11) + expect(screen.getByText('item-10')).toBeInTheDocument() + await user.click(screen.getByRole('button', {name: /next/i})) + expect(onPageChange).toHaveBeenLastCalledWith(1) + }) + }) + + describe('composition with existing features', () => { + it('pagination + sorting yields the sorted-and-sliced rows', async () => { + const user = userEvent.setup() + render() + // Click "ID" header to sort descending (default is asc on click). + await user.click(screen.getByRole('button', {name: /id/i})) + await user.click(screen.getByRole('button', {name: /id/i})) + // Page 0 of DESC sort contains 15..11. + expect(screen.getByText('item-15')).toBeInTheDocument() + expect(screen.queryByText('item-10')).not.toBeInTheDocument() + }) + }) +}) diff --git a/script/check-classname-tests.mjs b/script/check-classname-tests.mjs index dd756028ec7..9f5f1bc84c2 100755 --- a/script/check-classname-tests.mjs +++ b/script/check-classname-tests.mjs @@ -17,6 +17,7 @@ const IGNORED_FILES = [ 'packages/react/src/DataTable/__tests__/DataTable.test.tsx', 'packages/react/src/DataTable/__tests__/ErrorDialog.test.tsx', 'packages/react/src/DataTable/__tests__/Pagination.test.tsx', + 'packages/react/src/DataTable/__tests__/integrated-pagination.test.tsx', 'packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx', 'packages/react/src/FormControl/__tests__/useFormControlForwardedProps.test.tsx', 'packages/react/src/experimental/SelectPanel2/__tests__/SelectPanelLoading.test.tsx',