DataTable: add integrated pagination via the pagination prop#7856
Open
ianwinsemius wants to merge 3 commits into
Open
DataTable: add integrated pagination via the pagination prop#7856ianwinsemius wants to merge 3 commits into
ianwinsemius wants to merge 3 commits into
Conversation
Consumers that today have to wire `<Table.Pagination>` and manually slice rows can now opt in with a single prop. The existing manual composition path is unchanged. - New: `DataTableProps.pagination` — `true`, an options object (pageSize, defaultPageIndex, aria-label, showPages), or `false` to opt out. The DataTable renders `<Table.Pagination>` and slices the rows. - New: `DataTableProps.pageIndex` / `onPageChange` for controlled mode. - New: `DataTableProps.externalPagination` for server-driven pagination (parallels the existing `externalSorting` escape hatch). - Auto-reset to page 0 when the underlying data identity changes (uncontrolled mode only). - Pagination remount strategy avoids a setState-in-render feedback loop between DataTable and Pagination's defaultPageIndex sync. - Stories: WithIntegratedPagination, WithIntegratedPaginationControlled, WithIntegratedPaginationExternal. - Tests: 12 new cases covering opt-in, row slicing, defaultPageIndex, data-identity reset, controlled mode, externalPagination, and pagination + sorting composition. - Docs: DataTable.docs.json updated with new story ids and prop docs. - Changeset: minor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 406f656 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class, integrated pagination to DataTable so consumers can opt into built-in paging via a new pagination prop (while keeping the existing manual <Table.Pagination /> composition path available).
Changes:
- Introduces
DataTableProps.pagination,pageIndex/onPageChange(controlled mode), andexternalPagination(server-driven escape hatch), and wiresDataTableto render pagination + slice rows. - Adds Storybook feature stories demonstrating integrated pagination in uncontrolled, controlled, and external/server-driven modes.
- Adds a new unit test suite covering pagination opt-in behavior, slicing, resets, controlled mode, external pagination, and sorting composition; updates docs metadata and includes a changeset for a minor release.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/DataTable/DataTable.tsx | Implements integrated pagination API, row slicing, and renders <Pagination> beneath the table. |
| packages/react/src/DataTable/DataTable.features.stories.tsx | Adds 3 new stories showcasing integrated pagination variants. |
| packages/react/src/DataTable/DataTable.docs.json | Registers new story IDs and documents new pagination-related props. |
| packages/react/src/DataTable/tests/integrated-pagination.test.tsx | Adds coverage for integrated pagination behavior and feature composition. |
| .changeset/datatable-integrated-pagination.md | Declares a minor bump for the new DataTable pagination feature. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 4
Comment on lines
+160
to
+168
| const [prevDataIdentity, setPrevDataIdentity] = React.useState(data) | ||
| const [paginationResetCounter, setPaginationResetCounter] = React.useState(0) | ||
| if (!isControlledPage && data !== prevDataIdentity) { | ||
| setPrevDataIdentity(data) | ||
| if (uncontrolledPageIndex !== 0) { | ||
| setUncontrolledPageIndex(0) | ||
| setPaginationResetCounter(prev => prev + 1) | ||
| } | ||
| } |
Comment on lines
+179
to
+199
| // 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 { | ||
| const pageStart = effectivePageIndex * 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) | ||
| } |
Comment on lines
+154
to
+157
| // `defaultPageIndex` retriggers Pagination's render-time sync), we hold | ||
| // the value passed to `defaultPageIndex` in a ref that we bump only on | ||
| // intentional external resets (data identity changes, controlled-prop | ||
| // updates, or initial mount). |
Comment on lines
+168
to
+176
| // Simulate a server-paginated context where the consumer has already | ||
| // sliced — totalCount is what the component sees, so pretend we have | ||
| // 30 rows but pass only the current page's 10. | ||
| render( | ||
| <DataTable | ||
| aria-labelledby="t" | ||
| data={makeItems(30)} | ||
| columns={buildColumns()} | ||
| pagination={{pageSize: 10}} |
- Add integrated-pagination.test.tsx to check-classname-tests.mjs
IGNORED_FILES (feature tests, not a component with className prop).
- Collapse three setState calls during render into ONE atomic update
of a combined pageState object ({pageIndex, resetKey, prevData}).
Documented React 'storing information from previous renders'
pattern: https://react.dev/reference/react/useState#storing-information-from-previous-renders.
Eliminates the cascading-render risk Copilot flagged while still
avoiding the lint conflict between react-hooks/refs and
react-hooks/set-state-in-effect.
- Defensive validation: clamp pageSize (default 25 if NaN / <= 0) and
controlled pageIndex (>= 0) so a bogus prop cannot produce Infinity
in Pagination's page-count math.
- Slice math now clamps the page index to the valid pageCount range so
a stale page from before a data shrink cannot show an empty page
while the dataset still has rows.
- Comment fixed: pagination state is held in useState (not a ref);
description updated to match.
- Test fixed: externalPagination test now actually passes a single
page of data (matching the scenario the comment describes).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ianwinsemius
had a problem deploying
to
github-pages
GitHub Actions
Failure
13 tasks
Author
|
Note The failing |
joshblack
reviewed
May 20, 2026
Member
joshblack
left a comment
joshblack
left a comment
There was a problem hiding this comment.
Thanks for taking the time to put this together! Just had a question on the pagination type real quick 👀
Comment on lines
+90
to
+98
| pagination?: | ||
| | false | ||
| | true | ||
| | { | ||
| pageSize?: number | ||
| defaultPageIndex?: number | ||
| 'aria-label'?: string | ||
| showPages?: boolean | ResponsiveValue<boolean> | ||
| } |
Member
There was a problem hiding this comment.
What would you think about having these each be their own prop so that way we don't need the pagination prop? The table could then be in pagination mode if these are set (and provide sensible fallbacks)
Ah I guess this might not make sense because if they want pagination with default behavior they still need a prop to indicate that 😅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
Today, paginating a
DataTablerequires consumers to:pageIndexstate manually.DataTable.<Table.Pagination>as a sibling and wire itsonChangeback to step 1.This PR adds first-class integrated pagination so the common case becomes:
The existing manual composition path (
<Table.Pagination />as a sibling) continues to work for callers that want finer control, parallel to howexternalSortingcoexists with built-in sort.Implementation notes
<Pagination>owns its own UI state.<DataTable>mirrors itsonChangeto slice rows.defaultPageIndexsync would call back into DataTable's setState.dataidentity changes (uncontrolled mode only) so consumers refetching from a server can't land on a phantom page.Changelog
New
DataTableProps.pagination—true, an options object (pageSize,defaultPageIndex,aria-label,showPages), orfalseto opt out.DataTableProps.pageIndex/onPageChange— controlled-mode page state.DataTableProps.externalPagination— defer slicing to the server.WithIntegratedPagination,WithIntegratedPaginationControlled,WithIntegratedPaginationExternal.Changed
DataTable.docs.jsonupdated with new prop docs and story IDs.Removed
(none)
Rollout strategy
Testing & Reviewing
packages/react/src/DataTable/__tests__/integrated-pagination.test.tsxcover opt-in, row slicing,defaultPageIndex, empty-data handling, data-identity reset, controlled mode (parent ownspageIndex),externalPagination, and pagination + sorting composition.npm run build,npm run type-check,npm run lint,npm run lint:css, andnpm test -- --run packages/react/src/DataTable/all pass locally.Merge checklist