From c4d814c1cb0c924364ce8bb43f8d6001311c6b4c Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 20:43:57 +0000 Subject: [PATCH 01/11] Data table pagination add re-render tests highlighting issues with component state for total pages and current page --- package-lock.json | 10 ++-- .../DataTable/__tests__/Pagination.test.tsx | 47 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 17cb868d1f7..db2bf113590 100644 --- a/package-lock.json +++ b/package-lock.json @@ -64,7 +64,7 @@ "name": "example-app-router", "version": "0.0.0", "dependencies": { - "@primer/react": "37.0.0-rc.6", + "@primer/react": "37.0.0-rc.9", "next": "^14.2.10", "react": "^18.3.1", "react-dom": "^18.3.1", @@ -83,7 +83,7 @@ "react-dom": "^18.3.1" }, "devDependencies": { - "@primer/react": "37.0.0-rc.6", + "@primer/react": "37.0.0-rc.9", "@types/react": "^18.3.3", "@types/react-dom": "^18.3.0", "@typescript-eslint/eslint-plugin": "^7.11.0", @@ -101,7 +101,7 @@ "name": "example-consumer-test", "version": "0.0.0", "dependencies": { - "@primer/react": "37.0.0-rc.6", + "@primer/react": "37.0.0-rc.9", "@types/react": "^18.2.14", "@types/react-dom": "^18.2.19", "@types/styled-components": "^5.1.11", @@ -127,7 +127,7 @@ "version": "0.0.0", "dependencies": { "@primer/octicons-react": "^19.9.0", - "@primer/react": "37.0.0-rc.6", + "@primer/react": "37.0.0-rc.9", "clsx": "^1.2.1", "next": "^14.2.10", "react": "^18.3.1", @@ -30605,7 +30605,7 @@ }, "packages/react": { "name": "@primer/react", - "version": "37.0.0-rc.6", + "version": "37.0.0-rc.9", "license": "MIT", "dependencies": { "@github/relative-time-element": "^4.4.2", diff --git a/packages/react/src/DataTable/__tests__/Pagination.test.tsx b/packages/react/src/DataTable/__tests__/Pagination.test.tsx index 0d5c93e1bb0..e25e2b4bf1e 100644 --- a/packages/react/src/DataTable/__tests__/Pagination.test.tsx +++ b/packages/react/src/DataTable/__tests__/Pagination.test.tsx @@ -84,6 +84,21 @@ describe('Table.Pagination', () => { await user.click(getPreviousPage()) expect(onChange).not.toHaveBeenCalled() }) + + it('should rerender many pages correctly', async () => { + const {rerender} = render( + , + ) + expect(getPages()).toHaveLength(1) + expect(getCurrentPage()).toEqual(getPage(0)) + expect(getPageRange()).toEqual('1 through 25 of 25') + + rerender() + expect(getPageRange()).toEqual('16 through 20 of 300') + expect(getCurrentPage()).toEqual(getPage(2)) + const negativePages = getPages().filter(p => p.textContent?.includes('-')) + expect(negativePages).toHaveLength(0) + }) }) describe('with two pages', () => { @@ -197,6 +212,20 @@ describe('Table.Pagination', () => { pageIndex: 0, }) }) + + it('should rerender many pages correctly', async () => { + const {rerender} = render( + , + ) + expect(getPages()).toHaveLength(2) + expect(getCurrentPage()).toEqual(getPage(1)) + expect(getPageRange()).toEqual('26 through 50 of 50') + + rerender() + expect(getPageRange()).toEqual('1 through 5 of 300') + expect(getFirstPage()).toEqual(getCurrentPage()) + expect(getInvalidPages()).toHaveLength(0) + }) }) describe('with three or more pages', () => { @@ -242,6 +271,20 @@ describe('Table.Pagination', () => { expect(pages).toHaveLength(7) }) }) + + it('should rerender many pages correctly', async () => { + const {rerender} = render( + , + ) + expect(getPages()).toHaveLength(8) + expect(getCurrentPage()).toEqual(getPage(1)) + expect(getPageRange()).toEqual('11 through 20 of 1000') + + rerender() + expect(getPageRange()).toEqual('1 through 5 of 300') + expect(getFirstPage()).toEqual(getCurrentPage()) + expect(getInvalidPages()).toHaveLength(0) + }) }) function getPages() { @@ -288,6 +331,10 @@ function getLastPage() { return pages[pages.length - 1] } +function getInvalidPages() { + getPages().filter(p => p.textContent?.match(/Page\s-/g) || p.textContent?.match(/Page\s0$/g)) +} + function getPageRange() { const element = document.querySelector('.TablePaginationRange') if (element && element.textContent) { From d7f8fd47c81810eb06f22291fa7ee563d14b6afb Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 21:12:54 +0000 Subject: [PATCH 02/11] add use effect to update component state if we see that initial state has changed --- packages/react/src/DataTable/Pagination.tsx | 16 +++++++++++++++- .../src/DataTable/__tests__/Pagination.test.tsx | 9 ++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/react/src/DataTable/Pagination.tsx b/packages/react/src/DataTable/Pagination.tsx index 938cb4fb3d4..ab169634816 100644 --- a/packages/react/src/DataTable/Pagination.tsx +++ b/packages/react/src/DataTable/Pagination.tsx @@ -1,5 +1,5 @@ import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' -import React, {useCallback, useState} from 'react' +import React, {useCallback, useState, useEffect} from 'react' import styled from 'styled-components' import {get} from '../constants' import {Button} from '../internal/components/ButtonReset' @@ -195,6 +195,13 @@ export function Pagination({ const [offsetStartIndex, setOffsetStartIndex] = useState(() => { return getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) }) + useEffect(() => { + const startIndex = getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) + if (offsetStartIndex !== startIndex) { + setOffsetStartIndex(startIndex) + } + }, [pageIndex, pageCount, truncatedPageCount, offsetStartIndex]) + const offsetEndIndex = offsetStartIndex + truncatedPageCount - 1 const hasLeadingTruncation = offsetStartIndex >= 2 const hasTrailingTruncation = pageCount - 1 - offsetEndIndex > 1 @@ -510,6 +517,13 @@ function usePagination(config: PaginationConfig): PaginationResult { return 0 }) + useEffect(() => { + const validDefaultPageCount = + defaultPageIndex !== undefined && defaultPageIndex >= 0 && defaultPageIndex < pageCount + if (validDefaultPageCount && pageIndex !== defaultPageIndex) { + setPageIndex(defaultPageIndex) + } + }, [defaultPageIndex, pageCount, pageIndex]) const pageStart = pageIndex * pageSize const pageEnd = Math.min(pageIndex * pageSize + pageSize, totalCount - 1) const hasNextPage = pageIndex + 1 < pageCount diff --git a/packages/react/src/DataTable/__tests__/Pagination.test.tsx b/packages/react/src/DataTable/__tests__/Pagination.test.tsx index e25e2b4bf1e..6820316efe5 100644 --- a/packages/react/src/DataTable/__tests__/Pagination.test.tsx +++ b/packages/react/src/DataTable/__tests__/Pagination.test.tsx @@ -94,10 +94,9 @@ describe('Table.Pagination', () => { expect(getPageRange()).toEqual('1 through 25 of 25') rerender() - expect(getPageRange()).toEqual('16 through 20 of 300') + expect(getPageRange()).toEqual('11 through 15 of 300') expect(getCurrentPage()).toEqual(getPage(2)) - const negativePages = getPages().filter(p => p.textContent?.includes('-')) - expect(negativePages).toHaveLength(0) + expect(getInvalidPages()).toHaveLength(0) }) }) @@ -223,7 +222,7 @@ describe('Table.Pagination', () => { rerender() expect(getPageRange()).toEqual('1 through 5 of 300') - expect(getFirstPage()).toEqual(getCurrentPage()) + expect(getCurrentPage()).toEqual(getPage(0)) expect(getInvalidPages()).toHaveLength(0) }) }) @@ -332,7 +331,7 @@ function getLastPage() { } function getInvalidPages() { - getPages().filter(p => p.textContent?.match(/Page\s-/g) || p.textContent?.match(/Page\s0$/g)) + return getPages().filter(p => p.textContent?.match(/Page\s-/g) || p.textContent?.match(/Page\s0$/g)) } function getPageRange() { From c54c9e7d5d48847e2e92ccc6d58ce67204476b2f Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 21:26:34 +0000 Subject: [PATCH 03/11] do not use use effect --- packages/react/src/DataTable/Pagination.tsx | 25 +++++++++------------ 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/packages/react/src/DataTable/Pagination.tsx b/packages/react/src/DataTable/Pagination.tsx index ab169634816..ed260db6f42 100644 --- a/packages/react/src/DataTable/Pagination.tsx +++ b/packages/react/src/DataTable/Pagination.tsx @@ -1,5 +1,5 @@ import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' -import React, {useCallback, useState, useEffect} from 'react' +import React, {useCallback, useState} from 'react' import styled from 'styled-components' import {get} from '../constants' import {Button} from '../internal/components/ButtonReset' @@ -192,15 +192,13 @@ export function Pagination({ totalCount, }) const truncatedPageCount = pageCount > 2 ? Math.min(pageCount - 2, MAX_TRUNCATED_STEP_COUNT) : 0 + const defaultOffsetStartIndex = getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) const [offsetStartIndex, setOffsetStartIndex] = useState(() => { - return getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) + return defaultOffsetStartIndex }) - useEffect(() => { - const startIndex = getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) - if (offsetStartIndex !== startIndex) { - setOffsetStartIndex(startIndex) - } - }, [pageIndex, pageCount, truncatedPageCount, offsetStartIndex]) + if (offsetStartIndex !== defaultOffsetStartIndex) { + setOffsetStartIndex(defaultOffsetStartIndex) + } const offsetEndIndex = offsetStartIndex + truncatedPageCount - 1 const hasLeadingTruncation = offsetStartIndex >= 2 @@ -517,13 +515,10 @@ function usePagination(config: PaginationConfig): PaginationResult { return 0 }) - useEffect(() => { - const validDefaultPageCount = - defaultPageIndex !== undefined && defaultPageIndex >= 0 && defaultPageIndex < pageCount - if (validDefaultPageCount && pageIndex !== defaultPageIndex) { - setPageIndex(defaultPageIndex) - } - }, [defaultPageIndex, pageCount, pageIndex]) + const validDefaultPageCount = defaultPageIndex !== undefined && defaultPageIndex >= 0 && defaultPageIndex < pageCount + if (validDefaultPageCount && pageIndex !== defaultPageIndex) { + setPageIndex(defaultPageIndex) + } const pageStart = pageIndex * pageSize const pageEnd = Math.min(pageIndex * pageSize + pageSize, totalCount - 1) const hasNextPage = pageIndex + 1 < pageCount From 440dd73a7ae6f69729dfe8a9c73f5ba50c6af3b2 Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 21:48:01 +0000 Subject: [PATCH 04/11] only change when defaults change --- packages/react/src/DataTable/Pagination.tsx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/react/src/DataTable/Pagination.tsx b/packages/react/src/DataTable/Pagination.tsx index ed260db6f42..fdc611bc949 100644 --- a/packages/react/src/DataTable/Pagination.tsx +++ b/packages/react/src/DataTable/Pagination.tsx @@ -192,12 +192,13 @@ export function Pagination({ totalCount, }) const truncatedPageCount = pageCount > 2 ? Math.min(pageCount - 2, MAX_TRUNCATED_STEP_COUNT) : 0 - const defaultOffsetStartIndex = getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) - const [offsetStartIndex, setOffsetStartIndex] = useState(() => { - return defaultOffsetStartIndex - }) - if (offsetStartIndex !== defaultOffsetStartIndex) { - setOffsetStartIndex(defaultOffsetStartIndex) + const defaultOffset = getDefaultOffsetStartIndex(pageIndex, pageCount, truncatedPageCount) + const [defaultOffsetStartIndex, setDefaultOffsetStartIndex] = useState(defaultOffset) + const [offsetStartIndex, setOffsetStartIndex] = useState(defaultOffsetStartIndex) + + if (defaultOffsetStartIndex !== defaultOffset) { + setOffsetStartIndex(defaultOffset) + setDefaultOffsetStartIndex(defaultOffset) } const offsetEndIndex = offsetStartIndex + truncatedPageCount - 1 @@ -496,7 +497,7 @@ type PaginationResult = { function usePagination(config: PaginationConfig): PaginationResult { const {defaultPageIndex, onChange, pageSize, totalCount} = config const pageCount = Math.ceil(totalCount / pageSize) - const [pageIndex, setPageIndex] = useState(() => { + const [defaultIndex, setDefaultIndex] = useState(() => { if (defaultPageIndex !== undefined) { if (defaultPageIndex >= 0 && defaultPageIndex < pageCount) { return defaultPageIndex @@ -515,8 +516,10 @@ function usePagination(config: PaginationConfig): PaginationResult { return 0 }) + const [pageIndex, setPageIndex] = useState(defaultIndex) const validDefaultPageCount = defaultPageIndex !== undefined && defaultPageIndex >= 0 && defaultPageIndex < pageCount - if (validDefaultPageCount && pageIndex !== defaultPageIndex) { + if (validDefaultPageCount && defaultIndex !== defaultPageIndex) { + setDefaultIndex(defaultPageIndex) setPageIndex(defaultPageIndex) } const pageStart = pageIndex * pageSize From f50ba8c55d1ff582197982cd7e397b40bc1fe1cf Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 22:00:15 +0000 Subject: [PATCH 05/11] add more test cases --- .../DataTable/__tests__/Pagination.test.tsx | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/packages/react/src/DataTable/__tests__/Pagination.test.tsx b/packages/react/src/DataTable/__tests__/Pagination.test.tsx index 6820316efe5..287a5aa7760 100644 --- a/packages/react/src/DataTable/__tests__/Pagination.test.tsx +++ b/packages/react/src/DataTable/__tests__/Pagination.test.tsx @@ -128,6 +128,32 @@ describe('Table.Pagination', () => { expect(getPageRange()).toEqual('1 through 25 of 50') }) + it('should rerender pager with correct page highlighted when clicking on pages and defaultPageIndex set', async () => { + const user = userEvent.setup() + const onChange = jest.fn() + + render( + , + ) + + expect(getPageRange()).toEqual('76 through 100 of 200') + expect(getCurrentPage()).toEqual(getPage(3)) + + await user.click(getPage(1)) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 1, + }) + expect(getPageRange()).toEqual('26 through 50 of 200') + expect(getCurrentPage()).toEqual(getPage(1)) + + await user.click(getPage(0)) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 0, + }) + expect(getPageRange()).toEqual('1 through 25 of 200') + expect(getCurrentPage()).toEqual(getPage(0)) + }) + it('should call `onChange` when using the keyboard to interact with pages', async () => { const user = userEvent.setup() const onChange = jest.fn() @@ -179,6 +205,31 @@ describe('Table.Pagination', () => { }) }) + it('should rerender pager with correct page highlighted when clicking on previous or next and defaultPageIndex set', async () => { + const user = userEvent.setup() + const onChange = jest.fn() + + render( + , + ) + + expect(getPageRange()).toEqual('76 through 100 of 200') + + await user.click(getNextPage()) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 4, + }) + expect(getPageRange()).toEqual('101 through 125 of 200') + expect(getCurrentPage()).toEqual(getPage(4)) + + await user.click(getPreviousPage()) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 3, + }) + expect(getPageRange()).toEqual('76 through 100 of 200') + expect(getCurrentPage()).toEqual(getPage(3)) + }) + it('should call `onChange` when using the keyboard to interact with previous or next', async () => { const user = userEvent.setup() const onChange = jest.fn() From 4973d27a096f1fe35c08ebd535e3d9ef870895c3 Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 22:42:38 +0000 Subject: [PATCH 06/11] also change table content --- .../react/src/DataTable/DataTable.stories.tsx | 28 ++++++++++++++++++- packages/react/src/DataTable/Pagination.tsx | 1 + 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/react/src/DataTable/DataTable.stories.tsx b/packages/react/src/DataTable/DataTable.stories.tsx index f57b6843eba..215f2e1151c 100644 --- a/packages/react/src/DataTable/DataTable.stories.tsx +++ b/packages/react/src/DataTable/DataTable.stories.tsx @@ -195,6 +195,11 @@ export const Playground = (args: DataTableProps & ColWidthArgTypes) = const align = args.align as CellAlignment + const [pageIndex, setPageIndex] = React.useState(0) + const start = pageIndex * args.pageSize + const end = start + args.pageSize + const rows = data.slice(start, end) + return ( @@ -207,7 +212,7 @@ export const Playground = (args: DataTableProps & ColWidthArgTypes) = {...args} aria-labelledby="repositories" aria-describedby="repositories-subtitle" - data={data} + data={rows} columns={[ { header: 'Repository', @@ -276,6 +281,15 @@ export const Playground = (args: DataTableProps & ColWidthArgTypes) = }, ]} /> + { + setPageIndex(pageIndex) + }} + defaultPageIndex={parseInt(args.defaultPageIndex, 10)} + /> ) } @@ -318,6 +332,18 @@ Playground.argTypes = { disable: true, }, }, + pageSize: { + control: { + defaultValue: 5, + type: 'number', + min: 1, + }, + }, + defaultPageIndex: { + control: { + type: 'number', + }, + }, cellPadding: { control: { type: 'radio', diff --git a/packages/react/src/DataTable/Pagination.tsx b/packages/react/src/DataTable/Pagination.tsx index fdc611bc949..0a13d304e1a 100644 --- a/packages/react/src/DataTable/Pagination.tsx +++ b/packages/react/src/DataTable/Pagination.tsx @@ -521,6 +521,7 @@ function usePagination(config: PaginationConfig): PaginationResult { if (validDefaultPageCount && defaultIndex !== defaultPageIndex) { setDefaultIndex(defaultPageIndex) setPageIndex(defaultPageIndex) + onChange?.({pageIndex: defaultPageIndex}) } const pageStart = pageIndex * pageSize const pageEnd = Math.min(pageIndex * pageSize + pageSize, totalCount - 1) From a292661d65a1126a3020ddc9c7c573e17786c004 Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 22:48:39 +0000 Subject: [PATCH 07/11] add test coverage for onchange --- .../DataTable/__tests__/Pagination.test.tsx | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/react/src/DataTable/__tests__/Pagination.test.tsx b/packages/react/src/DataTable/__tests__/Pagination.test.tsx index 287a5aa7760..c2f0676867c 100644 --- a/packages/react/src/DataTable/__tests__/Pagination.test.tsx +++ b/packages/react/src/DataTable/__tests__/Pagination.test.tsx @@ -86,17 +86,24 @@ describe('Table.Pagination', () => { }) it('should rerender many pages correctly', async () => { + const onChange = jest.fn() + const {rerender} = render( - , + , ) expect(getPages()).toHaveLength(1) expect(getCurrentPage()).toEqual(getPage(0)) expect(getPageRange()).toEqual('1 through 25 of 25') - rerender() + rerender( + , + ) expect(getPageRange()).toEqual('11 through 15 of 300') expect(getCurrentPage()).toEqual(getPage(2)) expect(getInvalidPages()).toHaveLength(0) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 2, + }) }) }) @@ -264,17 +271,24 @@ describe('Table.Pagination', () => { }) it('should rerender many pages correctly', async () => { + const onChange = jest.fn() + const {rerender} = render( - , + , ) expect(getPages()).toHaveLength(2) expect(getCurrentPage()).toEqual(getPage(1)) expect(getPageRange()).toEqual('26 through 50 of 50') - rerender() + rerender( + , + ) expect(getPageRange()).toEqual('1 through 5 of 300') expect(getCurrentPage()).toEqual(getPage(0)) expect(getInvalidPages()).toHaveLength(0) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 0, + }) }) }) @@ -323,17 +337,23 @@ describe('Table.Pagination', () => { }) it('should rerender many pages correctly', async () => { + const onChange = jest.fn() const {rerender} = render( - , + , ) expect(getPages()).toHaveLength(8) expect(getCurrentPage()).toEqual(getPage(1)) expect(getPageRange()).toEqual('11 through 20 of 1000') - rerender() + rerender( + , + ) expect(getPageRange()).toEqual('1 through 5 of 300') expect(getFirstPage()).toEqual(getCurrentPage()) expect(getInvalidPages()).toHaveLength(0) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 0, + }) }) }) From cde0485fea9ed44ea4d758ba4f0ea1824821c2da Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 22:49:43 +0000 Subject: [PATCH 08/11] provide default page size --- packages/react/src/DataTable/DataTable.stories.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/DataTable/DataTable.stories.tsx b/packages/react/src/DataTable/DataTable.stories.tsx index 215f2e1151c..46b43e71d79 100644 --- a/packages/react/src/DataTable/DataTable.stories.tsx +++ b/packages/react/src/DataTable/DataTable.stories.tsx @@ -296,6 +296,7 @@ export const Playground = (args: DataTableProps & ColWidthArgTypes) = Playground.args = { cellPadding: 'normal', + pageSize: 5, } Playground.argTypes = { From b5e9179a5d0a8732bdef0991024a91bffed2c4e0 Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Thu, 3 Oct 2024 23:10:30 +0000 Subject: [PATCH 09/11] appease types --- packages/react/src/DataTable/DataTable.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/DataTable/DataTable.stories.tsx b/packages/react/src/DataTable/DataTable.stories.tsx index 46b43e71d79..7726463411d 100644 --- a/packages/react/src/DataTable/DataTable.stories.tsx +++ b/packages/react/src/DataTable/DataTable.stories.tsx @@ -196,8 +196,8 @@ export const Playground = (args: DataTableProps & ColWidthArgTypes) = const align = args.align as CellAlignment const [pageIndex, setPageIndex] = React.useState(0) - const start = pageIndex * args.pageSize - const end = start + args.pageSize + const start = pageIndex * parseInt(args.pageSize, 10) + const end = start + parseInt(args.pageSize, 10) const rows = data.slice(start, end) return ( From 7bf3b6dcbb833c2c4488cafa89de7f0953f2ac9c Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Fri, 4 Oct 2024 00:05:06 +0000 Subject: [PATCH 10/11] fix corner case --- packages/react/src/DataTable/Pagination.tsx | 8 ++++---- .../react/src/DataTable/__tests__/Pagination.test.tsx | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/react/src/DataTable/Pagination.tsx b/packages/react/src/DataTable/Pagination.tsx index 0a13d304e1a..7a3bfdfea70 100644 --- a/packages/react/src/DataTable/Pagination.tsx +++ b/packages/react/src/DataTable/Pagination.tsx @@ -339,15 +339,15 @@ function getDefaultOffsetStartIndex(pageIndex: number, pageCount: number, trunca // 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 Math.max(pageIndex - 3, 1) } - return pageCount - 1 - truncatedPageCount + return Math.max(pageCount - 1 - truncatedPageCount, 1) } // 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 Math.max(pageIndex - 3, 1) } return 1 } @@ -356,7 +356,7 @@ function getDefaultOffsetStartIndex(pageIndex: number, pageCount: number, trunca if (pageIndex < truncatedPageCount) { return pageIndex } - return pageIndex - 3 + return Math.max(pageIndex - 3, 1) } type RangeProps = { diff --git a/packages/react/src/DataTable/__tests__/Pagination.test.tsx b/packages/react/src/DataTable/__tests__/Pagination.test.tsx index c2f0676867c..47338126fe9 100644 --- a/packages/react/src/DataTable/__tests__/Pagination.test.tsx +++ b/packages/react/src/DataTable/__tests__/Pagination.test.tsx @@ -355,6 +355,14 @@ describe('Table.Pagination', () => { pageIndex: 0, }) }) + + it('when rendering 3 pages and the second page is selected we should render a page number not ...', async () => { + const onChange = jest.fn() + render() + expect(getPageRange()).toEqual('3 through 4 of 6') + expect(getCurrentPage()).toEqual(getPage(1)) + expect(getInvalidPages()).toHaveLength(0) + }) }) function getPages() { From 480dfe0cbb78353627a2937d736bd90afa85713f Mon Sep 17 00:00:00 2001 From: Kerry Liu Date: Fri, 4 Oct 2024 17:47:14 +0000 Subject: [PATCH 11/11] Add changeset --- .changeset/dry-pens-pay.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dry-pens-pay.md diff --git a/.changeset/dry-pens-pay.md b/.changeset/dry-pens-pay.md new file mode 100644 index 00000000000..342d81cd0de --- /dev/null +++ b/.changeset/dry-pens-pay.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Fixes negative and invalid pages in data table pagination on re-render.