From 22fa0928a162bd19b509773a90631c9e53e24de5 Mon Sep 17 00:00:00 2001 From: David Strack Date: Thu, 2 Nov 2023 10:59:13 -0500 Subject: [PATCH] fix(DataTable): Fix pagination step generation when `defaultPageIndex` is provided (#3866) * fix pagination steps when defaultPageIndex is set * refactor pagination step generation Co-authored-by: John Crepezzi * add story for pagination w/ default page index * return to using offsetStartIndex with better bounds control Co-authored-by: Josh Black * add changeset --------- Co-authored-by: John Crepezzi Co-authored-by: Josh Black --- .changeset/bright-cougars-switch.md | 7 ++ src/DataTable/DataTable.features.stories.tsx | 80 ++++++++++++++++++++ src/DataTable/Pagination.tsx | 33 ++++++-- src/DataTable/__tests__/Pagination.test.tsx | 18 +++++ 4 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 .changeset/bright-cougars-switch.md diff --git a/.changeset/bright-cougars-switch.md b/.changeset/bright-cougars-switch.md new file mode 100644 index 00000000000..0b574281147 --- /dev/null +++ b/.changeset/bright-cougars-switch.md @@ -0,0 +1,7 @@ +--- +'@primer/react': patch +--- + +DataTable: fix incorrect pagination steps when defaultPageIndex is provided + + \ No newline at end of file diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index 38fde55dc8c..3dc578355dd 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -1525,6 +1525,86 @@ export const WithPagination = () => { ) } +export const WithPaginationUsingDefaultPageIndex = () => { + const pageSize = 10 + const [pageIndex, setPageIndex] = React.useState(0) + const start = pageIndex * pageSize + const end = start + pageSize + const rows = repos.slice(start, end) + + return ( + + + Repositories + + + A subtitle could appear here to give extra context to the data. + + { + return + }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + ]} + /> + { + setPageIndex(pageIndex) + }} + defaultPageIndex={49} + /> + + ) +} + export const WithNetworkError = () => { const pageSize = 10 const [pageIndex, setPageIndex] = React.useState(0) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index 9dc14038e50..a0a792e7305 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -108,7 +108,7 @@ const StyledPagination = styled.nav` .TablePaginationSteps[data-hidden-viewport-ranges*='${viewportRangeKey}'] > *:first-child { margin-inline-end: 0; } - + .TablePaginationSteps[data-hidden-viewport-ranges*='${viewportRangeKey}'] > *:last-child { margin-inline-start: 0; } @@ -191,12 +191,7 @@ export function Pagination({ }) const truncatedPageCount = pageCount > 2 ? Math.min(pageCount - 2, MAX_TRUNCATED_STEP_COUNT) : 0 const [offsetStartIndex, setOffsetStartIndex] = useState(() => { - // Set the offset start index to the page at index 1 since we will have the - // first page already visible - if (pageIndex === 0) { - return 1 - } - return pageIndex + return getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) }) const offsetEndIndex = offsetStartIndex + truncatedPageCount - 1 const hasLeadingTruncation = offsetStartIndex >= 2 @@ -332,6 +327,30 @@ export function Pagination({ ) } +function getDefaultOffsetStartIndex(pageIndex: number, pageCount: number, truncatedPageCount: number): number { + // When the current page is closer to the end of the list than the beginning + if (pageIndex > pageCount - 1 - pageIndex) { + if (pageCount - 1 - pageIndex >= truncatedPageCount) { + return pageIndex - 3 + } + return pageCount - 1 - truncatedPageCount + } + + // When the current page is closer to the beginning of the list than the end + if (pageIndex < pageCount - 1 - pageIndex) { + if (pageIndex >= truncatedPageCount) { + return pageIndex - 3 + } + return 1 + } + + // When the current page is the midpoint between the beginning and the end + if (pageIndex < truncatedPageCount) { + return pageIndex + } + return pageIndex - 3 +} + type RangeProps = { pageStart: number pageEnd: number diff --git a/src/DataTable/__tests__/Pagination.test.tsx b/src/DataTable/__tests__/Pagination.test.tsx index d412a66cc99..0d5c93e1bb0 100644 --- a/src/DataTable/__tests__/Pagination.test.tsx +++ b/src/DataTable/__tests__/Pagination.test.tsx @@ -23,6 +23,18 @@ describe('Table.Pagination', () => { expect(getCurrentPage()).toEqual(getLastPage()) }) + it('should select the correct page with `defaultPageIndex`', () => { + render() + const expectedPage = getPages().filter(p => p.textContent?.includes('5'))[0] + expect(getCurrentPage()).toEqual(expectedPage) + }) + + it('should show the expected steps when `defaultPageIndex` is provided', () => { + render() + const pages = getPages().map(p => p.textContent?.replace(/[a-z]|\s+/gi, '')) + expect(pages).toEqual(['1…', '4', '5', '6', '7', '8', '9', '10']) + }) + it('should warn if `defaultPageIndex` is not a valid `pageIndex`', () => { const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}) render() @@ -223,6 +235,12 @@ describe('Table.Pagination', () => { expect(previousStep).toHaveAttribute('aria-hidden', 'true') expect(previousStep).toHaveTextContent('…') }) + + it('should not have more than 7 pages when leading and trailing truncation are present', () => { + render() + const pages = getPages() + expect(pages).toHaveLength(7) + }) }) })