Skip to content

Commit

Permalink
fix(DataTable): Fix pagination step generation when `defaultPageIndex…
Browse files Browse the repository at this point in the history
…` is provided (#3866)

* fix pagination steps when defaultPageIndex is set

* refactor pagination step generation

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>

* add story for pagination w/ default page index

* return to using offsetStartIndex with better bounds control

Co-authored-by: Josh Black <joshblack@github.com>

* add changeset

---------

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Co-authored-by: Josh Black <joshblack@github.com>
  • Loading branch information
3 people authored Nov 2, 2023
1 parent 3e20ab1 commit 22fa092
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 7 deletions.
7 changes: 7 additions & 0 deletions .changeset/bright-cougars-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': patch
---

DataTable: fix incorrect pagination steps when defaultPageIndex is provided

<!-- Changed components: DataTable -->
80 changes: 80 additions & 0 deletions src/DataTable/DataTable.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Table.Container>
<Table.Title as="h2" id="repositories">
Repositories
</Table.Title>
<Table.Subtitle as="p" id="repositories-subtitle">
A subtitle could appear here to give extra context to the data.
</Table.Subtitle>
<DataTable
aria-labelledby="repositories"
aria-describedby="repositories-subtitle"
data={rows}
columns={[
{
header: 'Repository',
field: 'name',
rowHeader: true,
},
{
header: 'Type',
field: 'type',
renderCell: row => {
return <Label>{uppercase(row.type)}</Label>
},
},
{
header: 'Updated',
field: 'updatedAt',
renderCell: row => {
return <RelativeTime date={new Date(row.updatedAt)} />
},
},
{
header: 'Dependabot',
field: 'securityFeatures.dependabot',
renderCell: row => {
return row.securityFeatures.dependabot.length > 0 ? (
<LabelGroup>
{row.securityFeatures.dependabot.map(feature => {
return <Label key={feature}>{uppercase(feature)}</Label>
})}
</LabelGroup>
) : null
},
},
{
header: 'Code scanning',
field: 'securityFeatures.codeScanning',
renderCell: row => {
return row.securityFeatures.codeScanning.length > 0 ? (
<LabelGroup>
{row.securityFeatures.codeScanning.map(feature => {
return <Label key={feature}>{uppercase(feature)}</Label>
})}
</LabelGroup>
) : null
},
},
]}
/>
<Table.Pagination
aria-label="Pagination for Repositories"
pageSize={pageSize}
totalCount={repos.length}
onChange={({pageIndex}) => {
setPageIndex(pageIndex)
}}
defaultPageIndex={49}
/>
</Table.Container>
)
}

export const WithNetworkError = () => {
const pageSize = 10
const [pageIndex, setPageIndex] = React.useState(0)
Expand Down
33 changes: 26 additions & 7 deletions src/DataTable/Pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions src/DataTable/__tests__/Pagination.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ describe('Table.Pagination', () => {
expect(getCurrentPage()).toEqual(getLastPage())
})

it('should select the correct page with `defaultPageIndex`', () => {
render(<Pagination aria-label="Pagination" defaultPageIndex={4} pageSize={10} totalCount={100} />)
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(<Pagination aria-label="Pagination" defaultPageIndex={6} pageSize={10} totalCount={100} />)
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(<Pagination aria-label="Pagination" defaultPageIndex={4} pageSize={25} totalCount={100} />)
Expand Down Expand Up @@ -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(<Pagination aria-label="Test label" defaultPageIndex={49} pageSize={10} totalCount={1000} />)
const pages = getPages()
expect(pages).toHaveLength(7)
})
})
})

Expand Down

0 comments on commit 22fa092

Please sign in to comment.