fix: rerendering issues on DiffScene options change (#1168)

* Allow correct card to be rendered on rerender after toggling comparison options * Fix for pageCount calculation to use ceil rather than round
looker-open-source · Aug 30, 2022 · a4ac3bc · a4ac3bc
1 parent b01770c
commit a4ac3bc
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 7 deletions.
diff --git a/packages/api-explorer/src/scenes/DiffScene/DocDiff/DocDiff.spec.tsx b/packages/api-explorer/src/scenes/DiffScene/DocDiff/DocDiff.spec.tsx
@@ -38,9 +38,9 @@ describe('DocDiff', () => {
   const rightKey = specs['4.0'].key
   const leftApi = specs['3.1'].api!
   const rightApi = specs['4.0'].api!
+  const delta = diffSpecs(leftApi, rightApi, standardDiffToggles)
 
   it('renders', () => {
-    const delta = diffSpecs(leftApi, rightApi, standardDiffToggles)
     renderWithReduxProvider(
       <DocDiff
         leftKey={leftKey}
@@ -61,6 +61,7 @@ describe('DocDiff', () => {
       screen.getByRole('button', { name: 'Next page of results' })
     ).toBeInTheDocument()
   })
+
   it('renders when there is no delta', () => {
     renderWithReduxProvider(
       <DocDiff
@@ -75,7 +76,6 @@ describe('DocDiff', () => {
   })
 
   it('paginates', async () => {
-    const delta = diffSpecs(leftApi, rightApi, standardDiffToggles)
     renderWithReduxProvider(
       <DocDiff
         leftKey={leftKey}
@@ -111,7 +111,6 @@ describe('DocDiff', () => {
   })
 
   it('paginates with correct number of entries per page', async () => {
-    const delta = diffSpecs(leftApi, rightApi, standardDiffToggles)
     const pageSize = 5
     renderWithReduxProvider(
       <DocDiff
@@ -136,8 +135,23 @@ describe('DocDiff', () => {
     expect(screen.queryByText(notDisplayedRow.name)).not.toBeInTheDocument()
   })
 
+  it('paginates with correct total page count', async () => {
+    const pageSize = 5
+    renderWithReduxProvider(
+      <DocDiff
+        leftKey={leftKey}
+        leftSpec={leftApi}
+        rightKey={rightKey}
+        rightSpec={rightApi}
+        delta={delta}
+        pageSize={pageSize}
+      />
+    )
+    const expectedPageCount = Math.ceil(delta.length / pageSize)
+    expect(screen.getByText(`of ${expectedPageCount}`)).toBeInTheDocument()
+  })
+
   it('final diff entry of one page does not appear in next page', async () => {
-    const delta = diffSpecs(leftApi, rightApi, standardDiffToggles)
     const pageSize = 5
     renderWithReduxProvider(
       <DocDiff

diff --git a/packages/api-explorer/src/scenes/DiffScene/DocDiff/DocDiff.tsx b/packages/api-explorer/src/scenes/DiffScene/DocDiff/DocDiff.tsx
@@ -63,7 +63,7 @@ export const DocDiff: FC<DocDiffProps> = ({
 
   if (delta.length === 0) return <Text>{'No differences found'}</Text>
 
-  const pageCount = Math.round((delta.length - 1) / pageSize)
+  const pageCount = Math.ceil(delta.length / pageSize)
   const pageItemData = delta.slice((page - 1) * pageSize, page * pageSize)
 
   return (
@@ -76,9 +76,9 @@ export const DocDiff: FC<DocDiffProps> = ({
             <Heading as="h2">{`${delta.length} differences between ${leftKey} and ${rightKey}`}</Heading>
           </Space>
           <SpaceVertical mt="large" gap="xxsmall">
-            {pageItemData.map((item, index) => (
+            {pageItemData.map((item) => (
               <DiffItem
-                key={`page-${page} item-${index}`}
+                key={item.name}
                 item={item}
                 leftKey={leftKey}
                 leftSpec={leftSpec}