Fix bug reoder on table in view groups mode (#8894)

Fixes #8403 

Added comments on the PR to explain changes

packages/twenty-front/src/modules/object-record/record-table/components/RecordTableNoRecordGroupRows.tsx

@@ -15,7 +15,8 @@ export const RecordTableNoRecordGroupRows = () => {
           <RecordTableRow
             key={recordId}
             recordId={recordId}
-            rowIndex={rowIndex}
+            rowIndexForFocus={rowIndex}
+            rowIndexForDrag={rowIndex}
           />
         );
       })}

packages/twenty-front/src/modules/object-record/record-table/components/RecordTableRecordGroupRows.tsx

@@ -32,7 +32,7 @@ export const RecordTableRecordGroupRows = () => {
 
   return (
     isRecordGroupTableSectionToggled &&
-    recordIdsByGroup.map((recordId) => {
+    recordIdsByGroup.map((recordId, rowIndexInGroup) => {
       const rowIndex = rowIndexMap.get(recordId);
 
       if (!isDefined(rowIndex)) {
@@ -43,7 +43,8 @@ export const RecordTableRecordGroupRows = () => {
         <RecordTableRow
           key={recordId}
           recordId={recordId}
-          rowIndex={rowIndex}
+          rowIndexForFocus={rowIndex}
+          rowIndexForDrag={rowIndexInGroup}
           isPendingRow={!isRecordGroupTableSectionToggled}
         />
       );

packages/twenty-front/src/modules/object-record/record-table/hooks/useComputeNewRowPosition.ts

@@ -1,90 +0,0 @@
-import { DropResult } from '@hello-pangea/dnd';
-import { useRecoilCallback } from 'recoil';
-
-import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
-import { isDefined } from '~/utils/isDefined';
-
-export const useComputeNewRowPosition = () => {
-  return useRecoilCallback(
-    ({ snapshot }) =>
-      (result: DropResult, tableRowIds: string[]) => {
-        if (!isDefined(result.destination)) {
-          return;
-        }
-
-        const draggedRecordId = result.draggableId;
-        const destinationIndex = result.destination.index;
-        const sourceIndex = result.source.index;
-
-        const recordBeforeId = tableRowIds[destinationIndex - 1];
-        const recordDestinationId = tableRowIds[destinationIndex];
-        const recordAfterDestinationId = tableRowIds[destinationIndex + 1];
-
-        const recordBefore = recordBeforeId
-          ? snapshot
-              .getLoadable(recordStoreFamilyState(recordBeforeId))
-              .getValue()
-          : null;
-        const recordDestination = recordDestinationId
-          ? snapshot
-              .getLoadable(recordStoreFamilyState(recordDestinationId))
-              .getValue()
-          : null;
-        const recordAfterDestination = recordAfterDestinationId
-          ? snapshot
-              .getLoadable(recordStoreFamilyState(recordAfterDestinationId))
-              .getValue()
-          : null;
-
-        const computeNewPosition = (destIndex: number, sourceIndex: number) => {
-          const moveToFirstPosition = destIndex === 0;
-          const moveToLastPosition = destIndex === tableRowIds.length - 1;
-          const moveAfterSource = destIndex > sourceIndex;
-
-          const firstRecord = tableRowIds[0]
-            ? snapshot
-                .getLoadable(recordStoreFamilyState(tableRowIds[0]))
-                .getValue()
-            : null;
-
-          const lastRecord = tableRowIds[tableRowIds.length - 1]
-            ? snapshot
-                .getLoadable(
-                  recordStoreFamilyState(tableRowIds[tableRowIds.length - 1]),
-                )
-                .getValue()
-            : null;
-
-          const firstRecordPosition = firstRecord?.position ?? 0;
-
-          if (moveToFirstPosition) {
-            if (firstRecordPosition <= 0) {
-              return firstRecordPosition - 1;
-            } else {
-              return firstRecordPosition / 2;
-            }
-          } else if (moveToLastPosition) {
-            return lastRecord?.position + 1;
-          } else if (moveAfterSource) {
-            const recordAfterDestinationPosition =
-              recordAfterDestination?.position ?? 0;
-            const recordDestinationPosition = recordDestination?.position ?? 0;
-
-            return (
-              (recordAfterDestinationPosition + recordDestinationPosition) / 2
-            );
-          } else {
-            return (
-              recordDestination?.position -
-              (recordDestination?.position - recordBefore?.position) / 2
-            );
-          }
-        };
-
-        const newPosition = computeNewPosition(destinationIndex, sourceIndex);
-
-        return { draggedRecordId, newPosition };
-      },
-    [],
-  );
-};

packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyDragDropContextProvider.tsx

@@ -1,13 +1,15 @@
 import { DragDropContext, DropResult } from '@hello-pangea/dnd';
 import { ReactNode, useContext } from 'react';
-import { useSetRecoilState } from 'recoil';
+import { useRecoilCallback, useSetRecoilState } from 'recoil';
 
 import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord';
+import { getDraggedRecordPosition } from '@/object-record/record-board/utils/getDraggedRecordPosition';
 import { recordIndexAllRecordIdsComponentSelector } from '@/object-record/record-index/states/selectors/recordIndexAllRecordIdsComponentSelector';
+import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
 import { RecordTableContext } from '@/object-record/record-table/contexts/RecordTableContext';
-import { useComputeNewRowPosition } from '@/object-record/record-table/hooks/useComputeNewRowPosition';
 import { isRemoveSortingModalOpenState } from '@/object-record/record-table/states/isRemoveSortingModalOpenState';
-import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
+import { getSnapshotValue } from '@/ui/utilities/recoil-scope/utils/getSnapshotValue';
+import { useRecoilComponentCallbackStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentCallbackStateV2';
 import { useGetCurrentView } from '@/views/hooks/useGetCurrentView';
 import { isDefined } from '~/utils/isDefined';
 
@@ -22,7 +24,7 @@ export const RecordTableBodyDragDropContextProvider = ({
     objectNameSingular,
   });
 
-  const allRecordIds = useRecoilComponentValueV2(
+  const recordIndexAllRecordIdsSelector = useRecoilComponentCallbackStateV2(
     recordIndexAllRecordIdsComponentSelector,
   );
 
@@ -35,27 +37,75 @@ export const RecordTableBodyDragDropContextProvider = ({
     isRemoveSortingModalOpenState,
   );
 
-  const computeNewRowPosition = useComputeNewRowPosition();
-
-  const handleDragEnd = (result: DropResult) => {
+  const handleDragEnd = useRecoilCallback(
+    ({ snapshot }) =>
+      (result: DropResult) => {
         if (viewSorts.length > 0) {
           setIsRemoveSortingModalOpenState(true);
           return;
         }
 
-    const computeResult = computeNewRowPosition(result, allRecordIds);
+        if (!isDefined(result.destination)) {
+          throw new Error('Drop Destination is not defined');
+        }
 
-    if (!isDefined(computeResult)) {
+        const allRecordIds = getSnapshotValue(
+          snapshot,
+          recordIndexAllRecordIdsSelector,
+        );
+
+        const isSourceIndexBeforeDestinationIndex =
+          result.source.index < result.destination.index;
+
+        const recordBeforeDestinationId =
+          allRecordIds[
+            isSourceIndexBeforeDestinationIndex
+              ? result.destination.index
+              : result.destination.index - 1
+          ];
+
+        const recordBeforeDestination = recordBeforeDestinationId
+          ? snapshot
+              .getLoadable(recordStoreFamilyState(recordBeforeDestinationId))
+              .getValue()
+          : null;
+
+        const recordAfterDestinationId =
+          allRecordIds[
+            isSourceIndexBeforeDestinationIndex
+              ? result.destination.index + 1
+              : result.destination.index
+          ];
+
+        const recordAfterDestination = recordAfterDestinationId
+          ? snapshot
+              .getLoadable(recordStoreFamilyState(recordAfterDestinationId))
+              .getValue()
+          : null;
+
+        const newPosition = getDraggedRecordPosition(
+          recordBeforeDestination?.position,
+          recordAfterDestination?.position,
+        );
+
+        if (!isDefined(newPosition)) {
           return;
         }
 
         updateOneRow({
-      idToUpdate: computeResult.draggedRecordId,
+          idToUpdate: result.draggableId,
           updateOneRecordInput: {
-        position: computeResult.newPosition,
+            position: newPosition,
           },
         });
-  };
+      },
+    [
+      recordIndexAllRecordIdsSelector,
+      setIsRemoveSortingModalOpenState,
+      updateOneRow,
+      viewSorts.length,
+    ],
+  );
 
   return (
     <DragDropContext onDragEnd={handleDragEnd}>{children}</DragDropContext>

packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyRecordGroupDragDropContextProvider.tsx

@@ -3,10 +3,11 @@ import { ReactNode, useContext } from 'react';
 import { useRecoilCallback, useSetRecoilState } from 'recoil';
 
 import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord';
+import { getDraggedRecordPosition } from '@/object-record/record-board/utils/getDraggedRecordPosition';
 import { recordGroupDefinitionFamilyState } from '@/object-record/record-group/states/recordGroupDefinitionFamilyState';
-import { recordIndexAllRecordIdsComponentSelector } from '@/object-record/record-index/states/selectors/recordIndexAllRecordIdsComponentSelector';
+import { recordIndexRecordIdsByGroupComponentFamilyState } from '@/object-record/record-index/states/recordIndexRecordIdsByGroupComponentFamilyState';
+import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
 import { RecordTableContext } from '@/object-record/record-table/contexts/RecordTableContext';
-import { useComputeNewRowPosition } from '@/object-record/record-table/hooks/useComputeNewRowPosition';
 import { isRemoveSortingModalOpenState } from '@/object-record/record-table/states/isRemoveSortingModalOpenState';
 import { useRecoilComponentCallbackStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentCallbackStateV2';
 import { getSnapshotValue } from '@/ui/utilities/state/utils/getSnapshotValue';
@@ -25,10 +26,6 @@ export const RecordTableBodyRecordGroupDragDropContextProvider = ({
     objectNameSingular,
   });
 
-  const recordIndexAllRecordIdsSelector = useRecoilComponentCallbackStateV2(
-    recordIndexAllRecordIdsComponentSelector,
-  );
-
   const { currentViewWithCombinedFiltersAndSorts } =
     useGetCurrentView(recordTableId);
 
@@ -38,33 +35,34 @@ export const RecordTableBodyRecordGroupDragDropContextProvider = ({
     isRemoveSortingModalOpenState,
   );
 
-  const computeNewRowPosition = useComputeNewRowPosition();
+  const recordIdsByGroupFamilyState = useRecoilComponentCallbackStateV2(
+    recordIndexRecordIdsByGroupComponentFamilyState,
+  );
 
   const handleDragEnd = useRecoilCallback(
     ({ snapshot }) =>
       (result: DropResult) => {
-        const tableAllRecordIds = getSnapshotValue(
-          snapshot,
-          recordIndexAllRecordIdsSelector,
-        );
+        const destinationRecordGroupId = result.destination?.droppableId;
 
-        const recordGroupId = result.destination?.droppableId;
+        if (!isDefined(result.destination)) {
+          throw new Error('Drop Destination is not defined');
+        }
 
-        if (!isDefined(recordGroupId)) {
+        if (!isDefined(destinationRecordGroupId)) {
           throw new Error('Record group id is not defined');
         }
 
-        const recordGroup = getSnapshotValue(
+        const destinationRecordGroup = getSnapshotValue(
           snapshot,
-          recordGroupDefinitionFamilyState(recordGroupId),
+          recordGroupDefinitionFamilyState(destinationRecordGroupId),
         );
 
-        if (!isDefined(recordGroup)) {
+        if (!isDefined(destinationRecordGroup)) {
           throw new Error('Record group is not defined');
         }
 
         const fieldMetadata = objectMetadataItem.fields.find(
-          (field) => field.id === recordGroup.fieldMetadataId,
+          (field) => field.id === destinationRecordGroup.fieldMetadataId,
         );
 
         if (!isDefined(fieldMetadata)) {
@@ -76,25 +74,62 @@ export const RecordTableBodyRecordGroupDragDropContextProvider = ({
           return;
         }
 
-        const computeResult = computeNewRowPosition(result, tableAllRecordIds);
+        const isSourceIndexBeforeDestinationIndexInSameGroup =
+          result.source.index < result.destination.index &&
+          result.source.droppableId === result.destination.droppableId;
 
-        if (!isDefined(computeResult)) {
+        const destinationGroupRecordIds = getSnapshotValue(
+          snapshot,
+          recordIdsByGroupFamilyState(destinationRecordGroupId),
+        );
+
+        const recordBeforeDestinationId =
+          destinationGroupRecordIds[
+            isSourceIndexBeforeDestinationIndexInSameGroup
+              ? result.destination.index
+              : result.destination.index - 1
+          ];
+
+        const recordBeforeDestination = recordBeforeDestinationId
+          ? snapshot
+              .getLoadable(recordStoreFamilyState(recordBeforeDestinationId))
+              .getValue()
+          : null;
+
+        const recordAfterDestinationId =
+          destinationGroupRecordIds[
+            isSourceIndexBeforeDestinationIndexInSameGroup
+              ? result.destination.index + 1
+              : result.destination.index
+          ];
+
+        const recordAfterDestination = recordAfterDestinationId
+          ? snapshot
+              .getLoadable(recordStoreFamilyState(recordAfterDestinationId))
+              .getValue()
+          : null;
+
+        const newPosition = getDraggedRecordPosition(
+          recordBeforeDestination?.position,
+          recordAfterDestination?.position,
+        );
+
+        if (!isDefined(newPosition)) {
           return;
         }
 
         updateOneRow({
-          idToUpdate: computeResult.draggedRecordId,
+          idToUpdate: result.draggableId,
           updateOneRecordInput: {
-            position: computeResult.newPosition,
-            [fieldMetadata.name]: recordGroup.value,
+            position: newPosition,
+            [fieldMetadata.name]: destinationRecordGroup.value,
           },
         });
       },
     [
-      recordIndexAllRecordIdsSelector,
       objectMetadataItem.fields,
       viewSorts.length,
-      computeNewRowPosition,
+      recordIdsByGroupFamilyState,
       updateOneRow,
       setIsRemoveSortingModalOpenState,
     ],

packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTablePendingRow.tsx

@@ -13,7 +13,8 @@ export const RecordTablePendingRow = () => {
     <RecordTableRow
       key={pendingRecordId}
       recordId={pendingRecordId}
-      rowIndex={-1}
+      rowIndexForDrag={-1}
+      rowIndexForFocus={-1}
       isPendingRow
     />
   );

packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRow.tsx

@@ -7,19 +7,22 @@ import { RecordTableRowWrapper } from '@/object-record/record-table/record-table
 
 type RecordTableRowProps = {
   recordId: string;
-  rowIndex: number;
+  rowIndexForFocus: number;
+  rowIndexForDrag: number;
   isPendingRow?: boolean;
 };
 
 export const RecordTableRow = ({
   recordId,
-  rowIndex,
+  rowIndexForFocus,
+  rowIndexForDrag,
   isPendingRow,
 }: RecordTableRowProps) => {
   return (
     <RecordTableRowWrapper
       recordId={recordId}
-      rowIndex={rowIndex}
+      rowIndexForFocus={rowIndexForFocus}
+      rowIndexForDrag={rowIndexForDrag}
       isPendingRow={isPendingRow}
     >
       <RecordTableCellGrip />

packages/twenty-front/src/modules/object-record/record-table/record-table-row/components/RecordTableRowWrapper.tsx

@@ -16,14 +16,16 @@ import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-sta
 
 type RecordTableRowWrapperProps = {
   recordId: string;
-  rowIndex: number;
+  rowIndexForFocus: number;
+  rowIndexForDrag: number;
   isPendingRow?: boolean;
   children: ReactNode;
 };
 
 export const RecordTableRowWrapper = ({
   recordId,
-  rowIndex,
+  rowIndexForFocus,
+  rowIndexForDrag,
   isPendingRow,
   children,
 }: RecordTableRowWrapperProps) => {
@@ -55,7 +57,7 @@ export const RecordTableRowWrapper = ({
   );
 
   useEffect(() => {
-    if (rowIndex === 0) {
+    if (rowIndexForFocus === 0) {
       const tdArray = Array.from(
         trRef.current?.getElementsByTagName('td') ?? [],
       );
@@ -66,7 +68,7 @@ export const RecordTableRowWrapper = ({
 
       setTableCellWidths(tdWidths);
     }
-  }, [trRef, rowIndex, setTableCellWidths]);
+  }, [trRef, rowIndexForFocus, setTableCellWidths]);
 
   // TODO: find a better way to emit this event
   useEffect(() => {
@@ -76,7 +78,7 @@ export const RecordTableRowWrapper = ({
   }, [inView, onIndexRecordsLoaded]);
 
   return (
-    <Draggable key={recordId} draggableId={recordId} index={rowIndex}>
+    <Draggable key={recordId} draggableId={recordId} index={rowIndexForDrag}>
       {(draggableProvided, draggableSnapshot) => (
         <RecordTableTr
           ref={(node) => {
@@ -103,7 +105,7 @@ export const RecordTableRowWrapper = ({
           <RecordTableRowContext.Provider
             value={{
               recordId,
-              rowIndex,
+              rowIndex: rowIndexForFocus,
               pathToShowPage:
                 getBasePathToShowPage({
                   objectNameSingular: objectMetadataItem.nameSingular,

packages/twenty-front/src/modules/ui/utilities/state/component-state/hooks/useRecoilCallbackState.ts

@@ -1,27 +0,0 @@
-import { useAvailableScopeIdOrThrow } from '@/ui/utilities/recoil-scope/scopes-internal/hooks/useAvailableScopeId';
-import { getScopeIdOrUndefinedFromComponentId } from '@/ui/utilities/recoil-scope/utils/getScopeIdOrUndefinedFromComponentId';
-import { RecoilComponentState } from '@/ui/utilities/state/component-state/types/RecoilComponentState';
-
-export const useRecoilCallbackState = <Value>(
-  componentState: RecoilComponentState<Value>,
-  componentId?: string,
-) => {
-  const componentContext = (window as any).componentContextStateMap?.get(
-    componentState.key,
-  );
-
-  if (!componentContext) {
-    throw new Error(
-      `Component context for key "${componentState.key}" is not defined`,
-    );
-  }
-
-  const internalScopeId = useAvailableScopeIdOrThrow(
-    componentContext,
-    getScopeIdOrUndefinedFromComponentId(componentId),
-  );
-
-  return componentState.atomFamily({
-    scopeId: internalScopeId,
-  });
-};