fix: apply review

twentyhq · Nov 14, 2024 · 98c27c4 · 98c27c4
1 parent b62677c
commit 98c27c4
Show file tree

Hide file tree

Showing 21 changed files with 60 additions and 90 deletions.
diff --git a/packages/twenty-front/src/modules/object-record/record-group/types/RecordGroupDefinition.ts b/packages/twenty-front/src/modules/object-record/record-group/types/RecordGroupDefinition.ts
@@ -15,5 +15,3 @@ export type RecordGroupDefinition = {
   position: number;
   isVisible: boolean;
 };
-
-export type RecordGroupDefinitionId = RecordGroupDefinition['id'];
diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTable.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTable.tsx
@@ -8,11 +8,11 @@ import { RecordTableStickyEffect } from '@/object-record/record-table/components
 import { RECORD_TABLE_CLICK_OUTSIDE_LISTENER_ID } from '@/object-record/record-table/constants/RecordTableClickOutsideListenerId';
 import { RecordTableEmptyState } from '@/object-record/record-table/empty-state/components/RecordTableEmptyState';
 import { useRecordTable } from '@/object-record/record-table/hooks/useRecordTable';
-import { RecordTableBody } from '@/object-record/record-table/record-table-body/components/RecordTableBody';
-import { RecordTableBodyEffect } from '@/object-record/record-table/record-table-body/components/RecordTableBodyEffect';
-import { RecordTableBodyRecordGroupEffects } from '@/object-record/record-table/record-table-body/components/RecordTableBodyRecordGroupEffects';
-import { RecordTableBodyRecordGroups } from '@/object-record/record-table/record-table-body/components/RecordTableBodyRecordGroups';
 import { RecordTableBodyUnselectEffect } from '@/object-record/record-table/record-table-body/components/RecordTableBodyUnselectEffect';
+import { RecordTableNoRecordGroupBody } from '@/object-record/record-table/record-table-body/components/RecordTableNoRecordGroupBody';
+import { RecordTableNoRecordGroupBodyEffect } from '@/object-record/record-table/record-table-body/components/RecordTableNoRecordGroupBodyEffect';
+import { RecordTableRecordGroupBodyEffects } from '@/object-record/record-table/record-table-body/components/RecordTableRecordGroupBodyEffects';
+import { RecordTableRecordGroupsBody } from '@/object-record/record-table/record-table-body/components/RecordTableRecordGroupsBody';
 import { RecordTableHeader } from '@/object-record/record-table/record-table-header/components/RecordTableHeader';
 import { isRecordTableInitialLoadingComponentState } from '@/object-record/record-table/states/isRecordTableInitialLoadingComponentState';
 import { recordTablePendingRecordIdComponentState } from '@/object-record/record-table/states/recordTablePendingRecordIdComponentState';
@@ -92,9 +92,9 @@ export const RecordTable = ({
         viewBarId={viewBarId}
       >
         {!hasRecordGroups ? (
-          <RecordTableBodyEffect />
+          <RecordTableNoRecordGroupBodyEffect />
         ) : (
-          <RecordTableBodyRecordGroupEffects />
+          <RecordTableRecordGroupBodyEffects />
         )}
         <RecordTableBodyUnselectEffect
           tableBodyRef={tableBodyRef}
@@ -107,9 +107,9 @@ export const RecordTable = ({
             <StyledTable className="entity-table-cell" ref={tableBodyRef}>
               <RecordTableHeader objectNameSingular={objectNameSingular} />
               {!hasRecordGroups ? (
-                <RecordTableBody />
+                <RecordTableNoRecordGroupBody />
               ) : (
-                <RecordTableBodyRecordGroups
+                <RecordTableRecordGroupsBody
                   objectNameSingular={objectNameSingular}
                 />
               )}

diff --git a/...cord-table/components/RecordTableRows.tsx → ...mponents/RecordTableNoRecordGroupRows.tsx b/...cord-table/components/RecordTableRows.tsx → ...mponents/RecordTableNoRecordGroupRows.tsx
@@ -3,7 +3,7 @@ import { RecordTableRow } from '@/object-record/record-table/record-table-row/co
 import { tableAllRowIdsComponentState } from '@/object-record/record-table/states/tableAllRowIdsComponentState';
 import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
 
-export const RecordTableRows = () => {
+export const RecordTableNoRecordGroupRows = () => {
   const rowIds = useRecoilComponentValueV2(tableAllRowIdsComponentState);
 
   return (

diff --git a/.../components/RecordTableRecordGroupRow.tsx → ...components/RecordTableRecordGroupRows.tsx b/.../components/RecordTableRecordGroupRow.tsx → ...components/RecordTableRecordGroupRows.tsx
@@ -1,17 +1,24 @@
 import { useCurrentRecordGroupId } from '@/object-record/record-group/hooks/useCurrentRecordGroupId';
 import { RecordTableRow } from '@/object-record/record-table/record-table-row/components/RecordTableRow';
+import { tableAllRowIdsComponentState } from '@/object-record/record-table/states/tableAllRowIdsComponentState';
 import { tableRowIdsByGroupComponentFamilyState } from '@/object-record/record-table/states/tableRowIdsByGroupComponentFamilyState';
 import { useRecoilComponentFamilyValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentFamilyValueV2';
+import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
 
 export const RecordTableRecordGroupRows = () => {
   const recordGroupId = useCurrentRecordGroupId();
 
+  const allRowIds = useRecoilComponentValueV2(tableAllRowIdsComponentState);
+
   const recordGroupRowIds = useRecoilComponentFamilyValueV2(
     tableRowIdsByGroupComponentFamilyState,
     recordGroupId,
   );
 
-  return recordGroupRowIds.map((recordId, rowIndex) => {
+  return recordGroupRowIds.map((recordId) => {
+    // Find the index of the recordId in allRowIds
+    const rowIndex = allRowIds.indexOf(recordId);
+
     return (
       <RecordTableRow key={recordId} recordId={recordId} rowIndex={rowIndex} />
     );

diff --git a/...s/object-record/record-table/components/__stories__/perf/RecordTableCell.perf.stories.tsx b/...s/object-record/record-table/components/__stories__/perf/RecordTableCell.perf.stories.tsx
@@ -105,7 +105,7 @@ const meta: Meta = {
                   value={{
                     columnDefinition: mockPerformance.fieldDefinition,
                     columnIndex: 0,
-                    cellPosition: { recordId: 'recordId', column: 0 },
+                    cellPosition: { row: 0, column: 0 },
                     hasSoftFocus: false,
                     isInEditMode: false,
                   }}

diff --git a/...ges/twenty-front/src/modules/object-record/record-table/hooks/useCreateNewTableRecords.ts b/...ges/twenty-front/src/modules/object-record/record-table/hooks/useCreateNewTableRecords.ts
@@ -23,7 +23,7 @@ export const useCreateNewTableRecord = (recordTableIdFromProps?: string) => {
 
   const createNewTableRecord = () => {
     setPendingRecordId(v4());
-    setSelectedTableCellEditMode('', 0);
+    setSelectedTableCellEditMode(-1, 0);
     setHotkeyScope(DEFAULT_CELL_SCOPE.scope, DEFAULT_CELL_SCOPE.customScopes);
   };
 

diff --git a/...ages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTableMoveFocus.ts b/...ages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTableMoveFocus.ts
@@ -7,22 +7,8 @@ import { numberOfTableColumnsComponentSelector } from '@/object-record/record-ta
 import { softFocusPositionComponentState } from '@/object-record/record-table/states/softFocusPositionComponentState';
 import { tableAllRowIdsComponentState } from '@/object-record/record-table/states/tableAllRowIdsComponentState';
 import { useRecoilComponentCallbackStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentCallbackStateV2';
-import { isDefined } from '~/utils/isDefined';
 import { useSetSoftFocusPosition } from './internal/useSetSoftFocusPosition';
 
-const getRecordIdFromRowIndex = (
-  rowIds: string[],
-  rowIndex: number,
-): string => {
-  const rowId = rowIds?.[rowIndex];
-
-  if (!isDefined(rowId)) {
-    throw new Error(`Record ID not found at index ${rowIndex}`);
-  }
-
-  return rowId;
-};
-
 export const useRecordTableMoveFocus = (recordTableId?: string) => {
   const setSoftFocusPosition = useSetSoftFocusPosition(recordTableId);
 
@@ -39,30 +25,23 @@ export const useRecordTableMoveFocus = (recordTableId?: string) => {
   const moveUp = useRecoilCallback(
     ({ snapshot }) =>
       () => {
-        const allRowIds = getSnapshotValue(snapshot, tableAllRowIdsState);
         const softFocusPosition = getSnapshotValue(
           snapshot,
           softFocusPositionState,
         );
 
-        const currentRowIndex = allRowIds.indexOf(softFocusPosition.recordId);
-
-        if (currentRowIndex === -1) {
-          return;
-        }
-
-        let newRowIndex = currentRowIndex - 1;
+        let newRowIndex = softFocusPosition.row - 1;
 
         if (newRowIndex < 0) {
           newRowIndex = 0;
         }
 
         setSoftFocusPosition({
           ...softFocusPosition,
-          recordId: getRecordIdFromRowIndex(allRowIds, newRowIndex),
+          row: newRowIndex,
         });
       },
-    [tableAllRowIdsState, softFocusPositionState, setSoftFocusPosition],
+    [softFocusPositionState, setSoftFocusPosition],
   );
 
   const moveDown = useRecoilCallback(
@@ -74,21 +53,15 @@ export const useRecordTableMoveFocus = (recordTableId?: string) => {
           softFocusPositionState,
         );
 
-        const currentRowIndex = allRowIds.indexOf(softFocusPosition.recordId);
-
-        if (currentRowIndex === -1) {
-          return;
-        }
-
-        let newRowIndex = currentRowIndex + 1;
+        let newRowIndex = softFocusPosition.row + 1;
 
         if (newRowIndex >= allRowIds.length) {
           newRowIndex = allRowIds.length - 1;
         }
 
         setSoftFocusPosition({
           ...softFocusPosition,
-          recordId: getRecordIdFromRowIndex(allRowIds, newRowIndex),
+          row: newRowIndex,
         });
       },
     [tableAllRowIdsState, setSoftFocusPosition, softFocusPositionState],
@@ -114,11 +87,7 @@ export const useRecordTableMoveFocus = (recordTableId?: string) => {
         );
 
         const currentColumnIndex = softFocusPosition.column;
-        const currentRowIndex = allRowIds.indexOf(softFocusPosition.recordId);
-
-        if (currentRowIndex === -1) {
-          return;
-        }
+        const currentRowIndex = softFocusPosition.row;
 
         const isLastRowAndLastColumn =
           currentColumnIndex === numberOfTableColumns - 1 &&
@@ -136,12 +105,12 @@ export const useRecordTableMoveFocus = (recordTableId?: string) => {
 
         if (isNotLastColumn) {
           setSoftFocusPosition({
-            recordId: getRecordIdFromRowIndex(allRowIds, currentRowIndex),
+            row: currentRowIndex,
             column: currentColumnIndex + 1,
           });
         } else if (isLastColumnButNotLastRow) {
           setSoftFocusPosition({
-            recordId: getRecordIdFromRowIndex(allRowIds, currentRowIndex + 1),
+            row: currentRowIndex + 1,
             column: 0,
           });
         }
@@ -169,11 +138,7 @@ export const useRecordTableMoveFocus = (recordTableId?: string) => {
         );
 
         const currentColumnIndex = softFocusPosition.column;
-        const currentRowIndex = allRowIds.indexOf(softFocusPosition.recordId);
-
-        if (currentRowIndex === -1) {
-          return;
-        }
+        const currentRowIndex = softFocusPosition.row;
 
         const isFirstRowAndFirstColumn =
           currentColumnIndex === 0 && currentRowIndex === 0;
@@ -189,12 +154,12 @@ export const useRecordTableMoveFocus = (recordTableId?: string) => {
 
         if (isNotFirstColumn) {
           setSoftFocusPosition({
-            recordId: getRecordIdFromRowIndex(allRowIds, currentRowIndex),
+            row: currentRowIndex,
             column: currentColumnIndex - 1,
           });
         } else if (isFirstColumnButNotFirstRow) {
           setSoftFocusPosition({
-            recordId: getRecordIdFromRowIndex(allRowIds, currentRowIndex - 1),
+            row: currentRowIndex - 1,
             column: numberOfTableColumns - 1,
           });
         }

diff --git a/...table-body/components/RecordTableBody.tsx → ...mponents/RecordTableNoRecordGroupBody.tsx b/...table-body/components/RecordTableBody.tsx → ...mponents/RecordTableNoRecordGroupBody.tsx
@@ -1,4 +1,4 @@
-import { RecordTableRows } from '@/object-record/record-table/components/RecordTableRows';
+import { RecordTableNoRecordGroupRows } from '@/object-record/record-table/components/RecordTableNoRecordGroupRows';
 import { RecordTableBodyDragDropContext } from '@/object-record/record-table/record-table-body/components/RecordTableBodyDragDropContext';
 import { RecordTableBodyDroppable } from '@/object-record/record-table/record-table-body/components/RecordTableBodyDroppable';
 import { RecordTableBodyLoading } from '@/object-record/record-table/record-table-body/components/RecordTableBodyLoading';
@@ -7,7 +7,7 @@ import { isRecordTableInitialLoadingComponentState } from '@/object-record/recor
 import { tableAllRowIdsComponentState } from '@/object-record/record-table/states/tableAllRowIdsComponentState';
 import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
 
-export const RecordTableBody = () => {
+export const RecordTableNoRecordGroupBody = () => {
   const tableAllRowIds = useRecoilComponentValueV2(
     tableAllRowIdsComponentState,
   );
@@ -24,7 +24,7 @@ export const RecordTableBody = () => {
     <RecordTableBodyDragDropContext>
       <RecordTableBodyDroppable>
         <RecordTablePendingRow />
-        <RecordTableRows />
+        <RecordTableNoRecordGroupRows />
       </RecordTableBodyDroppable>
     </RecordTableBodyDragDropContext>
   );

diff --git a/...body/components/RecordTableBodyEffect.tsx → ...ts/RecordTableNoRecordGroupBodyEffect.tsx b/...body/components/RecordTableBodyEffect.tsx → ...ts/RecordTableNoRecordGroupBodyEffect.tsx
@@ -14,7 +14,7 @@ import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-sta
 import { isNonEmptyString } from '@sniptt/guards';
 import { useScrollToPosition } from '~/hooks/useScrollToPosition';
 
-export const RecordTableBodyEffect = () => {
+export const RecordTableNoRecordGroupBodyEffect = () => {
   const { objectNameSingular } = useContext(RecordTableContext);
 
   const [hasInitializedScroll, setHasInitializedScroll] = useState(false);

diff --git a/...ents/RecordTableBodyRecordGroupEffect.tsx → ...ents/RecordTableRecordGroupBodyEffect.tsx b/...ents/RecordTableBodyRecordGroupEffect.tsx → ...ents/RecordTableRecordGroupBodyEffect.tsx
@@ -11,7 +11,7 @@ import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-sta
 import { isNonEmptyString } from '@sniptt/guards';
 import { useScrollToPosition } from '~/hooks/useScrollToPosition';
 
-export const RecordTableBodyRecordGroupEffect = () => {
+export const RecordTableRecordGroupBodyEffect = () => {
   const { objectNameSingular } = useContext(RecordTableContext);
 
   const recordGroupId = useCurrentRecordGroupId();

diff --git a/...nts/RecordTableBodyRecordGroupEffects.tsx → ...nts/RecordTableRecordGroupBodyEffects.tsx b/...nts/RecordTableBodyRecordGroupEffects.tsx → ...nts/RecordTableRecordGroupBodyEffects.tsx
@@ -1,9 +1,9 @@
 import { RecordGroupContext } from '@/object-record/record-group/states/context/RecordGroupContext';
 import { recordGroupDefinitionsComponentState } from '@/object-record/record-group/states/recordGroupDefinitionsComponentState';
-import { RecordTableBodyRecordGroupEffect } from '@/object-record/record-table/record-table-body/components/RecordTableBodyRecordGroupEffect';
+import { RecordTableRecordGroupBodyEffect } from '@/object-record/record-table/record-table-body/components/RecordTableRecordGroupBodyEffect';
 import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
 
-export const RecordTableBodyRecordGroupEffects = () => {
+export const RecordTableRecordGroupBodyEffects = () => {
   const recordGroupDefinitions = useRecoilComponentValueV2(
     recordGroupDefinitionsComponentState,
   );
@@ -12,7 +12,7 @@ export const RecordTableBodyRecordGroupEffects = () => {
     <RecordGroupContext.Provider
       value={{ recordGroupId: recordGroupDefinition.id }}
     >
-      <RecordTableBodyRecordGroupEffect />
+      <RecordTableRecordGroupBodyEffect />
     </RecordGroupContext.Provider>
   ));
 };
diff --git a/...omponents/RecordTableBodyRecordGroups.tsx → ...omponents/RecordTableRecordGroupsBody.tsx b/...omponents/RecordTableBodyRecordGroups.tsx → ...omponents/RecordTableRecordGroupsBody.tsx
@@ -1,6 +1,6 @@
 import { useRecordGroups } from '@/object-record/record-group/hooks/useRecordGroups';
 import { RecordGroupContext } from '@/object-record/record-group/states/context/RecordGroupContext';
-import { RecordTableRecordGroupRows } from '@/object-record/record-table/components/RecordTableRecordGroupRow';
+import { RecordTableRecordGroupRows } from '@/object-record/record-table/components/RecordTableRecordGroupRows';
 import { RecordTableBodyDragDropContext } from '@/object-record/record-table/record-table-body/components/RecordTableBodyDragDropContext';
 import { RecordTableBodyDroppable } from '@/object-record/record-table/record-table-body/components/RecordTableBodyDroppable';
 import { RecordTableBodyLoading } from '@/object-record/record-table/record-table-body/components/RecordTableBodyLoading';
@@ -9,13 +9,13 @@ import { isRecordTableInitialLoadingComponentState } from '@/object-record/recor
 import { tableAllRowIdsComponentState } from '@/object-record/record-table/states/tableAllRowIdsComponentState';
 import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
 
-type RecordTableBodyRecordGroupsProps = {
+type RecordTableRecordGroupsBodyProps = {
   objectNameSingular: string;
 };
 
-export const RecordTableBodyRecordGroups = ({
+export const RecordTableRecordGroupsBody = ({
   objectNameSingular,
-}: RecordTableBodyRecordGroupsProps) => {
+}: RecordTableRecordGroupsBodyProps) => {
   const tableAllRowIds = useRecoilComponentValueV2(
     tableAllRowIdsComponentState,
   );

diff --git a/...odules/object-record/record-table/record-table-cell/components/RecordTableCellWrapper.tsx b/...odules/object-record/record-table/record-table-cell/components/RecordTableCellWrapper.tsx
@@ -19,14 +19,14 @@ export const RecordTableCellWrapper = ({
   columnIndex: number;
   children: React.ReactNode;
 }) => {
-  const { recordId } = useContext(RecordTableRowContext);
+  const { rowIndex } = useContext(RecordTableRowContext);
 
   const currentTableCellPosition: TableCellPosition = useMemo(
     () => ({
       column: columnIndex,
-      recordId,
+      row: rowIndex,
     }),
-    [columnIndex, recordId],
+    [columnIndex, rowIndex],
   );
 
   const isInEditMode = useRecoilComponentFamilyValueV2(

diff --git a/...ty-front/src/modules/object-record/record-table/record-table-cell/hooks/__mocks__/cell.ts b/...ty-front/src/modules/object-record/record-table/record-table-cell/hooks/__mocks__/cell.ts
@@ -30,7 +30,7 @@ export const recordTableCell: RecordTableCellContextProps = {
     },
   },
   cellPosition: {
-    recordId: 'recordId',
+    row: 2,
     column: 3,
   },
   hasSoftFocus: false,

diff --git a/...cord/record-table/record-table-cell/hooks/__tests__/useSelectedTableCellEditMode.test.tsx b/...cord/record-table/record-table-cell/hooks/__tests__/useSelectedTableCellEditMode.test.tsx
@@ -11,7 +11,7 @@ const mockCallbackInterface = {
   set: jest.fn(),
   snapshot: {
     getLoadable: () => ({
-      getValue: () => ({ recordId: 'initialRecordId', column: 0 }),
+      getValue: () => ({ row: 0, column: 0 }),
     }),
   },
 } as unknown as CallbackInterface;
@@ -28,7 +28,7 @@ jest.mock('recoil', () => ({
 describe('useSelectedTableCellEditMode', () => {
   it('should have property setSelectedTableCellEditMode', async () => {
     const tableCellPosition: TableCellPosition = {
-      recordId: 'newRecordId',
+      row: 1,
       column: 5,
     };
 
@@ -41,21 +41,21 @@ describe('useSelectedTableCellEditMode', () => {
 
     act(() => {
       result.current.setSelectedTableCellEditMode(
-        tableCellPosition.recordId,
+        tableCellPosition.row,
         tableCellPosition.column,
       );
     });
 
     expect(mockCallbackInterface.set).toHaveBeenCalledWith(
       {
-        key: 'isTableCellInEditModeComponentFamilyState__{"familyKey":{"column":0,"recordId":"initialRecordId"},"instanceId":"yourScopeId"}',
+        key: 'isTableCellInEditModeComponentFamilyState__{"familyKey":{"column":0,"row":0},"instanceId":"yourScopeId"}',
       },
       false,
     );
 
     expect(mockCallbackInterface.set).toHaveBeenCalledWith(
       {
-        key: 'isTableCellInEditModeComponentFamilyState__{"familyKey":{"column":5,"recordId":"newRecordId"},"instanceId":"yourScopeId"}',
+        key: 'isTableCellInEditModeComponentFamilyState__{"familyKey":{"column":5,"row":1},"instanceId":"yourScopeId"}',
       },
       true,
     );