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 <[email protected]> * add story for pagination w/ default page index * return to using offsetStartIndex with better bounds control Co-authored-by: Josh Black <[email protected]> * add changeset --------- Co-authored-by: John Crepezzi <[email protected]> Co-authored-by: Josh Black <[email protected]>
primer · Nov 2, 2023 · 22fa092 · 22fa092
1 parent 3e20ab1
commit 22fa092
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 7 deletions.
diff --git a/.changeset/bright-cougars-switch.md b/.changeset/bright-cougars-switch.md
@@ -0,0 +1,7 @@
+---
+'@primer/react': patch
+---
+
+DataTable: fix incorrect pagination steps when defaultPageIndex is provided
+
+<!-- Changed components: DataTable -->
diff --git 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 (
+    <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)

diff --git 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
@@ -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} />)
@@ -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)
+    })
   })
 })