From e8f386ce4360ded3b1cc4230a63f0521affbd306 Mon Sep 17 00:00:00 2001
From: Charles Bochet <charlesBochet@users.noreply.github.com>
Date: Mon, 24 Jun 2024 11:20:16 +0200
Subject: [PATCH] Fix infinite scroll issue on table (#5996)

We had an issue on infinite scroll on table view.
The fetch more logic was modifying isTableLastRowVisible state (which is
wrong, how could it know)? This was done to prevent loading too much
data at once. This was causing some race condition on
isTableLastRowVisible (as the table itself was also changing it
depending on the real visibility of the line)

I have remove this hacky usage of isTableLastRowVisible and replaced it
by a setTimeout to let the user some time to scroll and introduce a
throttle logic.
---
 .../hooks/useLoadRecordIndexTable.ts          |  5 +---
 .../components/RecordTableBodyEffect.tsx      | 30 ++++++++-----------
 .../components/RecordTableBodyLoading.tsx     |  1 +
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts
index c6a26daad..99f613521 100644
--- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts
+++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts
@@ -1,4 +1,4 @@
-import { useRecoilValue, useSetRecoilState } from 'recoil';
+import { useRecoilValue } from 'recoil';
 
 import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState';
 import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
@@ -41,8 +41,6 @@ export const useLoadRecordIndexTable = (objectNameSingular: string) => {
 
   const { setRecordTableData, setIsRecordTableInitialLoading } =
     useRecordTable();
-  const { tableLastRowVisibleState } = useRecordTableStates();
-  const setLastRowVisible = useSetRecoilState(tableLastRowVisibleState);
   const currentWorkspace = useRecoilValue(currentWorkspaceState);
   const params = useFindManyParams(objectNameSingular);
 
@@ -58,7 +56,6 @@ export const useLoadRecordIndexTable = (objectNameSingular: string) => {
     ...params,
     recordGqlFields,
     onCompleted: () => {
-      setLastRowVisible(false);
       setIsRecordTableInitialLoading(false);
     },
     onError: () => {
diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyEffect.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyEffect.tsx
index 4bea69f3d..8d3422a21 100644
--- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyEffect.tsx
+++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyEffect.tsx
@@ -1,5 +1,5 @@
 import { useEffect } from 'react';
-import { useRecoilState, useRecoilValue } from 'recoil';
+import { useRecoilValue } from 'recoil';
 
 import { useLoadRecordIndexTable } from '@/object-record/record-index/hooks/useLoadRecordIndexTable';
 import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates';
@@ -18,20 +18,18 @@ export const RecordTableBodyEffect = ({
     records,
     totalCount,
     setRecordTableData,
-    queryStateIdentifier,
     loading,
+    queryStateIdentifier,
   } = useLoadRecordIndexTable(objectNameSingular);
 
-  const { tableLastRowVisibleState } = useRecordTableStates();
-
-  const [tableLastRowVisible, setTableLastRowVisible] = useRecoilState(
-    tableLastRowVisibleState,
-  );
-
   const isFetchingMoreObjects = useRecoilValue(
     isFetchingMoreRecordsFamilyState(queryStateIdentifier),
   );
 
+  const { tableLastRowVisibleState } = useRecordTableStates();
+
+  const tableLastRowVisible = useRecoilValue(tableLastRowVisibleState);
+
   const rowHeight = 32;
   const viewportHeight = records.length * rowHeight;
 
@@ -44,15 +42,13 @@ export const RecordTableBodyEffect = ({
   }, [records, totalCount, setRecordTableData, loading]);
 
   useEffect(() => {
-    if (tableLastRowVisible && !isFetchingMoreObjects) {
-      fetchMoreObjects();
-    }
-  }, [
-    fetchMoreObjects,
-    isFetchingMoreObjects,
-    setTableLastRowVisible,
-    tableLastRowVisible,
-  ]);
+    // We are adding a setTimeout here to give the user some room to scroll if they want to within this throttle window
+    setTimeout(async () => {
+      if (!isFetchingMoreObjects && tableLastRowVisible) {
+        await fetchMoreObjects();
+      }
+    }, 100);
+  }, [fetchMoreObjects, isFetchingMoreObjects, tableLastRowVisible]);
 
   return <></>;
 };
diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyLoading.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyLoading.tsx
index d8df4058b..0f89384ec 100644
--- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyLoading.tsx
+++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableBodyLoading.tsx
@@ -20,6 +20,7 @@ export const RecordTableBodyLoading = () => {
           isDragging={false}
           data-testid={`row-id-${rowIndex}`}
           data-selectable-id={`row-id-${rowIndex}`}
+          key={rowIndex}
         >
           <StyledTd data-select-disable>
             <GripCell isDragging={false} />