Fix show page navigation bugs (#9199)

Fixes total count bug that was -1 the total count Fixes a bug when trying to go from first to last or the other way around Fixes a React array key bug Follow-up issue (non critical) : #9197
twentyhq · Dec 23, 2024 · 691fbbe · 691fbbe
1 parent 6abe735
commit 691fbbe
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 22 deletions.
diff --git a/packages/twenty-front/src/modules/action-menu/components/RecordShowActionMenuButtons.tsx b/packages/twenty-front/src/modules/action-menu/components/RecordShowActionMenuButtons.tsx
@@ -15,10 +15,10 @@ export const RecordShowActionMenuButtons = () => {
   return (
     <>
       {!isMobile &&
-        pinnedEntries.map((entry, index) =>
+        pinnedEntries.map((entry) =>
           entry.shortLabel ? (
             <Button
-              key={index}
+              key={entry.key}
               Icon={entry.Icon}
               size="small"
               variant="secondary"
@@ -29,6 +29,7 @@ export const RecordShowActionMenuButtons = () => {
             />
           ) : (
             <IconButton
+              key={entry.key}
               Icon={entry.Icon}
               size="small"
               variant="secondary"

diff --git a/...s/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts b/...s/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts
@@ -21,6 +21,7 @@ export const useRecordShowPagePagination = (
     objectNameSingular: paramObjectNameSingular,
     objectRecordId: paramObjectRecordId,
   } = useParams();
+
   const navigate = useNavigate();
   const [searchParams] = useSearchParams();
   const viewIdQueryParam = searchParams.get('view');
@@ -53,7 +54,7 @@ export const useRecordShowPagePagination = (
       recordGqlFields: { id: true },
     });
 
-  const cursorFromRequest = currentRecordsPageInfo?.endCursor;
+  const currentRecordCursorFromRequest = currentRecordsPageInfo?.endCursor;
 
   const [totalCountBefore, setTotalCountBefore] = useState<number>(0);
   const [totalCountAfter, setTotalCountAfter] = useState<number>(0);
@@ -67,10 +68,10 @@ export const useRecordShowPagePagination = (
         id: { neq: objectRecordId },
       },
       orderBy,
-      cursorFilter: isNonEmptyString(cursorFromRequest)
+      cursorFilter: isNonEmptyString(currentRecordCursorFromRequest)
         ? {
             cursorDirection: 'before',
-            cursor: cursorFromRequest,
+            cursor: currentRecordCursorFromRequest,
             limit: 1,
           }
         : undefined,
@@ -90,10 +91,10 @@ export const useRecordShowPagePagination = (
       },
       fetchPolicy: 'network-only',
       orderBy,
-      cursorFilter: cursorFromRequest
+      cursorFilter: currentRecordCursorFromRequest
         ? {
             cursorDirection: 'after',
-            cursor: cursorFromRequest,
+            cursor: currentRecordCursorFromRequest,
             limit: 1,
           }
         : undefined,
@@ -109,6 +110,9 @@ export const useRecordShowPagePagination = (
   const recordBefore = recordsBefore[0];
   const recordAfter = recordsAfter[0];
 
+  const isFirstRecord = !loading && !isDefined(recordBefore);
+  const isLastRecord = !loading && !isDefined(recordAfter);
+
   const { recordIdsInCache } = useRecordIdsFromFindManyCacheRootQuery({
     objectNamePlural: objectMetadataItem.namePlural,
     fieldVariables: {
@@ -117,32 +121,56 @@ export const useRecordShowPagePagination = (
     },
   });
 
+  const cacheIsAvailableForNavigation =
+    !loading &&
+    (totalCountAfter > 0 || totalCountBefore > 0) &&
+    recordIdsInCache.length > 0;
+
+  const canNavigateToPreviousRecord =
+    !isFirstRecord || (isFirstRecord && cacheIsAvailableForNavigation);
+
   const navigateToPreviousRecord = () => {
-    if (isDefined(recordBefore)) {
+    if (isFirstRecord) {
+      if (cacheIsAvailableForNavigation) {
+        const lastRecordIdFromCache =
+          recordIdsInCache[recordIdsInCache.length - 1];
+
+        navigate(
+          buildShowPageURL(
+            objectNameSingular,
+            lastRecordIdFromCache,
+            viewIdQueryParam,
+          ),
+        );
+      }
+    } else {
       navigate(
         buildShowPageURL(objectNameSingular, recordBefore.id, viewIdQueryParam),
       );
     }
-    if (!loadingRecordBefore && !isDefined(recordBefore)) {
-      const firstRecordId = recordIdsInCache[recordIdsInCache.length - 1];
-      navigate(
-        buildShowPageURL(objectNameSingular, firstRecordId, viewIdQueryParam),
-      );
-    }
   };
 
+  const canNavigateToNextRecord =
+    !isLastRecord || (isLastRecord && cacheIsAvailableForNavigation);
+
   const navigateToNextRecord = () => {
-    if (isDefined(recordAfter)) {
+    if (isLastRecord) {
+      if (cacheIsAvailableForNavigation) {
+        const firstRecordIdFromCache = recordIdsInCache[0];
+
+        navigate(
+          buildShowPageURL(
+            objectNameSingular,
+            firstRecordIdFromCache,
+            viewIdQueryParam,
+          ),
+        );
+      }
+    } else {
       navigate(
         buildShowPageURL(objectNameSingular, recordAfter.id, viewIdQueryParam),
       );
     }
-    if (!loadingRecordAfter && !isDefined(recordAfter)) {
-      const lastRecordId = recordIdsInCache[0];
-      navigate(
-        buildShowPageURL(objectNameSingular, lastRecordId, viewIdQueryParam),
-      );
-    }
   };
 
   const navigateToIndexView = () => {
@@ -162,7 +190,7 @@ export const useRecordShowPagePagination = (
 
   const objectLabel = capitalize(objectMetadataItem.labelPlural);
 
-  const totalCount = Math.max(1, totalCountBefore, totalCountAfter);
+  const totalCount = 1 + Math.max(totalCountBefore, totalCountAfter);
 
   const viewNameWithCount = rankFoundInView
     ? `${rankInView + 1} of ${totalCount} in ${objectLabel}`
@@ -174,5 +202,7 @@ export const useRecordShowPagePagination = (
     navigateToPreviousRecord,
     navigateToNextRecord,
     navigateToIndexView,
+    canNavigateToNextRecord,
+    canNavigateToPreviousRecord,
   };
 };