From 53d2bee87bbd113604dfb8cb3fd3dee5a4ee8d2b Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Tue, 9 Jul 2024 13:59:08 +0200 Subject: [PATCH 1/4] Display table record creation row when clicking on Add new from table empty state --- .../components/RecordTableWithWrappers.tsx | 18 ++++++++++++++---- .../pages/object-record/RecordIndexPage.tsx | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx index 2161a2fe0b13..3a46b714fc09 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx @@ -1,5 +1,5 @@ -import { useRef } from 'react'; import styled from '@emotion/styled'; +import { useRef } from 'react'; import { useRecoilCallback, useRecoilValue } from 'recoil'; import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; @@ -18,6 +18,7 @@ import { mapColumnDefinitionsToViewFields } from '@/views/utils/mapColumnDefinit import { RecordUpdateContext } from '../contexts/EntityUpdateMutationHookContext'; import { useRecordTable } from '../hooks/useRecordTable'; +import { isNull } from '@sniptt/guards'; import { RecordTableInternalEffect } from './RecordTableInternalEffect'; const StyledTableWithHeader = styled.div` @@ -48,8 +49,11 @@ export const RecordTableWithWrappers = ({ }: RecordTableWithWrappersProps) => { const tableBodyRef = useRef(null); - const { isRecordTableInitialLoadingState, tableRowIdsState } = - useRecordTableStates(recordTableId); + const { + isRecordTableInitialLoadingState, + tableRowIdsState, + pendingRecordIdState, + } = useRecordTableStates(recordTableId); const isRecordTableInitialLoading = useRecoilValue( isRecordTableInitialLoadingState, @@ -57,6 +61,8 @@ export const RecordTableWithWrappers = ({ const tableRowIds = useRecoilValue(tableRowIdsState); + const pendingRecordId = useRecoilValue(pendingRecordIdState); + const { resetTableRowSelection, setRowSelected } = useRecordTable({ recordTableId, }); @@ -86,7 +92,11 @@ export const RecordTableWithWrappers = ({ [saveViewFields], ); - if (!isRecordTableInitialLoading && tableRowIds.length === 0) { + if ( + !isRecordTableInitialLoading && + tableRowIds.length === 0 && + isNull(pendingRecordId) + ) { return ( Date: Tue, 9 Jul 2024 14:17:30 +0200 Subject: [PATCH 2/4] Fix --- .../record-table/components/RecordTable.tsx | 51 +++++++++-- .../components/RecordTableContextProvider.tsx | 4 +- .../components/RecordTableWithWrappers.tsx | 53 ++---------- .../hooks/useUpsertRecord.ts | 84 ++++++++++++------- .../hooks/useUpsertRecordV2.ts | 65 -------------- 5 files changed, 106 insertions(+), 151 deletions(-) delete mode 100644 packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useUpsertRecordV2.ts 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 index 5407b756dac7..8e5db342d972 100644 --- 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 @@ -1,12 +1,15 @@ import styled from '@emotion/styled'; -import { isNonEmptyString } from '@sniptt/guards'; +import { isNonEmptyString, isNull } from '@sniptt/guards'; +import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; +import { RecordTableContextProvider } from '@/object-record/record-table/components/RecordTableContextProvider'; +import { RecordTableEmptyState } from '@/object-record/record-table/components/RecordTableEmptyState'; +import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates'; 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 { RecordTableContextProvider } from '@/object-record/record-table/components/RecordTableContextProvider'; import { RecordTableHeader } from '@/object-record/record-table/record-table-header/components/RecordTableHeader'; -import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates'; import { RecordTableScope } from '@/object-record/record-table/scopes/RecordTableScope'; +import { useRecoilValue } from 'recoil'; const StyledTable = styled.table` border-radius: ${({ theme }) => theme.border.radius.sm}; @@ -32,6 +35,27 @@ export const RecordTable = ({ }: RecordTableProps) => { const { scopeId } = useRecordTableStates(recordTableId); + const { + isRecordTableInitialLoadingState, + tableRowIdsState, + pendingRecordIdState, + } = useRecordTableStates(recordTableId); + + const isRecordTableInitialLoading = useRecoilValue( + isRecordTableInitialLoadingState, + ); + + const tableRowIds = useRecoilValue(tableRowIdsState); + + const pendingRecordId = useRecoilValue(pendingRecordIdState); + + const { objectMetadataItem: foundObjectMetadataItem } = useObjectMetadataItem( + { objectNameSingular }, + ); + + const objectLabel = foundObjectMetadataItem?.labelSingular; + const isRemote = foundObjectMetadataItem?.isRemote ?? false; + if (!isNonEmptyString(objectNameSingular)) { return <>; } @@ -45,11 +69,22 @@ export const RecordTable = ({ objectNameSingular={objectNameSingular} recordTableId={recordTableId} > - - - - - + + {!isRecordTableInitialLoading && + tableRowIds.length === 0 && + isNull(pendingRecordId) ? ( + + ) : ( + + + + + )} ); diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableContextProvider.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableContextProvider.tsx index 7925fb8fbf16..47fa787d25b6 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableContextProvider.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableContextProvider.tsx @@ -13,7 +13,7 @@ import { useOpenRecordTableCellV2, } from '@/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellV2'; import { useTriggerContextMenu } from '@/object-record/record-table/record-table-cell/hooks/useTriggerContextMenu'; -import { useUpsertRecordV2 } from '@/object-record/record-table/record-table-cell/hooks/useUpsertRecordV2'; +import { useUpsertRecord } from '@/object-record/record-table/record-table-cell/hooks/useUpsertRecord'; import { MoveFocusDirection } from '@/object-record/record-table/types/MoveFocusDirection'; import { TableCellPosition } from '@/object-record/record-table/types/TableCellPosition'; @@ -32,7 +32,7 @@ export const RecordTableContextProvider = ({ objectNameSingular, }); - const { upsertRecord } = useUpsertRecordV2({ + const { upsertRecord } = useUpsertRecord({ objectNameSingular, }); diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx index 3a46b714fc09..7a6cabbdddf1 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx @@ -1,14 +1,11 @@ import styled from '@emotion/styled'; import { useRef } from 'react'; -import { useRecoilCallback, useRecoilValue } from 'recoil'; +import { useRecoilCallback } from 'recoil'; -import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord'; import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; import { RecordTable } from '@/object-record/record-table/components/RecordTable'; -import { RecordTableEmptyState } from '@/object-record/record-table/components/RecordTableEmptyState'; import { EntityDeleteContext } from '@/object-record/record-table/contexts/EntityDeleteHookContext'; -import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates'; import { ColumnDefinition } from '@/object-record/record-table/types/ColumnDefinition'; import { DragSelect } from '@/ui/utilities/drag-select/components/DragSelect'; import { ScrollWrapper } from '@/ui/utilities/scroll/components/ScrollWrapper'; @@ -18,7 +15,6 @@ import { mapColumnDefinitionsToViewFields } from '@/views/utils/mapColumnDefinit import { RecordUpdateContext } from '../contexts/EntityUpdateMutationHookContext'; import { useRecordTable } from '../hooks/useRecordTable'; -import { isNull } from '@sniptt/guards'; import { RecordTableInternalEffect } from './RecordTableInternalEffect'; const StyledTableWithHeader = styled.div` @@ -32,6 +28,10 @@ const StyledTableContainer = styled.div` position: relative; `; +const StyledTableInternalContainer = styled.div` + height: 100%; +`; + type RecordTableWithWrappersProps = { objectNameSingular: string; recordTableId: string; @@ -49,38 +49,14 @@ export const RecordTableWithWrappers = ({ }: RecordTableWithWrappersProps) => { const tableBodyRef = useRef(null); - const { - isRecordTableInitialLoadingState, - tableRowIdsState, - pendingRecordIdState, - } = useRecordTableStates(recordTableId); - - const isRecordTableInitialLoading = useRecoilValue( - isRecordTableInitialLoadingState, - ); - - const tableRowIds = useRecoilValue(tableRowIdsState); - - const pendingRecordId = useRecoilValue(pendingRecordIdState); - const { resetTableRowSelection, setRowSelected } = useRecordTable({ recordTableId, }); - const { objectMetadataItem: foundObjectMetadataItem } = useObjectMetadataItem( - { - objectNameSingular, - }, - ); - const { saveViewFields } = useSaveCurrentViewFields(viewBarId); const { deleteOneRecord } = useDeleteOneRecord({ objectNameSingular }); - const objectLabel = foundObjectMetadataItem?.labelSingular; - - const isRemote = foundObjectMetadataItem?.isRemote ?? false; - const handleColumnsChange = useRecoilCallback( () => (columns) => { saveViewFields( @@ -92,28 +68,13 @@ export const RecordTableWithWrappers = ({ [saveViewFields], ); - if ( - !isRecordTableInitialLoading && - tableRowIds.length === 0 && - isNull(pendingRecordId) - ) { - return ( - - ); - } - return ( -
+ -
+ { - const { entityId, fieldDefinition } = useContext(FieldContext); - - const { pendingRecordIdState } = useRecordTableStates(); - - const pendingRecordId = useRecoilValue(pendingRecordIdState); - const fieldName = fieldDefinition.metadata.fieldName; - const { getDraftValueSelector } = useRecordFieldInputStates( - `${entityId}-${fieldName}`, - ); - const draftValue = useRecoilValue(getDraftValueSelector()); - - const objectNameSingular = - fieldDefinition.metadata.objectMetadataNameSingular ?? ''; +export const useUpsertRecord = ({ + objectNameSingular, +}: { + objectNameSingular: string; +}) => { const { createOneRecord } = useCreateOneRecord({ objectNameSingular, }); - const upsertRecord = (persistField: () => void) => { - if (isDefined(pendingRecordId) && isDefined(draftValue)) { - createOneRecord({ - id: pendingRecordId, - name: draftValue, - position: 'first', - }); - } else if (!pendingRecordId) { - persistField(); - } - }; + const upsertRecord = useRecoilCallback( + ({ snapshot }) => + ( + persistField: () => void, + entityId: string, + fieldName: string, + recordTableId: string, + ) => { + const tableScopeId = getScopeIdFromComponentId(recordTableId); + + const recordTablePendingRecordIdState = extractComponentState( + recordTablePendingRecordIdComponentState, + tableScopeId, + ); + + const recordTablePendingRecordId = getSnapshotValue( + snapshot, + recordTablePendingRecordIdState, + ); + const fieldScopeId = getScopeIdFromComponentId( + `${entityId}-${fieldName}`, + ); + + const draftValueSelector = extractComponentSelector( + recordFieldInputDraftValueComponentSelector, + fieldScopeId, + ); + + const draftValue = getSnapshotValue(snapshot, draftValueSelector()); + + if (isDefined(recordTablePendingRecordId) && isDefined(draftValue)) { + createOneRecord({ + id: recordTablePendingRecordId, + name: draftValue, + position: 'first', + }); + } else if (!recordTablePendingRecordId) { + persistField(); + } + }, + [createOneRecord], + ); return { upsertRecord }; }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useUpsertRecordV2.ts b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useUpsertRecordV2.ts deleted file mode 100644 index c0b714ab2cc4..000000000000 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useUpsertRecordV2.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { useRecoilCallback } from 'recoil'; - -import { useCreateOneRecord } from '@/object-record/hooks/useCreateOneRecord'; -import { recordFieldInputDraftValueComponentSelector } from '@/object-record/record-field/states/selectors/recordFieldInputDraftValueComponentSelector'; -import { recordTablePendingRecordIdComponentState } from '@/object-record/record-table/states/recordTablePendingRecordIdComponentState'; -import { getScopeIdFromComponentId } from '@/ui/utilities/recoil-scope/utils/getScopeIdFromComponentId'; -import { getSnapshotValue } from '@/ui/utilities/recoil-scope/utils/getSnapshotValue'; -import { extractComponentSelector } from '@/ui/utilities/state/component-state/utils/extractComponentSelector'; -import { extractComponentState } from '@/ui/utilities/state/component-state/utils/extractComponentState'; -import { isDefined } from '~/utils/isDefined'; - -export const useUpsertRecordV2 = ({ - objectNameSingular, -}: { - objectNameSingular: string; -}) => { - const { createOneRecord } = useCreateOneRecord({ - objectNameSingular, - }); - - const upsertRecord = useRecoilCallback( - ({ snapshot }) => - ( - persistField: () => void, - entityId: string, - fieldName: string, - recordTableId: string, - ) => { - const tableScopeId = getScopeIdFromComponentId(recordTableId); - - const recordTablePendingRecordIdState = extractComponentState( - recordTablePendingRecordIdComponentState, - tableScopeId, - ); - - const recordTablePendingRecordId = getSnapshotValue( - snapshot, - recordTablePendingRecordIdState, - ); - const fieldScopeId = getScopeIdFromComponentId( - `${entityId}-${fieldName}`, - ); - - const draftValueSelector = extractComponentSelector( - recordFieldInputDraftValueComponentSelector, - fieldScopeId, - ); - - const draftValue = getSnapshotValue(snapshot, draftValueSelector()); - - if (isDefined(recordTablePendingRecordId) && isDefined(draftValue)) { - createOneRecord({ - id: recordTablePendingRecordId, - name: draftValue, - position: 'first', - }); - } else if (!recordTablePendingRecordId) { - persistField(); - } - }, - [createOneRecord], - ); - - return { upsertRecord }; -}; From 4f07f12bc5525bb11f43fbba12d3937a971044c3 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Tue, 9 Jul 2024 14:33:40 +0200 Subject: [PATCH 3/4] Fix --- packages/twenty-front/jest.config.ts | 2 +- .../hooks/__tests__/useUpsertRecord.test.tsx | 79 +++++++++---------- 2 files changed, 38 insertions(+), 43 deletions(-) diff --git a/packages/twenty-front/jest.config.ts b/packages/twenty-front/jest.config.ts index e21e9cbaf0a7..49b3f4f410fa 100644 --- a/packages/twenty-front/jest.config.ts +++ b/packages/twenty-front/jest.config.ts @@ -5,7 +5,7 @@ const tsConfig = require('./tsconfig.json'); const jestConfig: JestConfigWithTsJest = { // to enable logs, comment out the following line - silent: true, + silent: false, displayName: 'twenty-front', preset: '../../jest.preset.js', setupFilesAfterEnv: ['./setupTests.ts'], diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__tests__/useUpsertRecord.test.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__tests__/useUpsertRecord.test.tsx index 291c7407df60..2bf6ff27a658 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__tests__/useUpsertRecord.test.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__tests__/useUpsertRecord.test.tsx @@ -1,5 +1,5 @@ -import { ReactNode } from 'react'; import { act, renderHook } from '@testing-library/react'; +import { ReactNode } from 'react'; import { RecoilRoot } from 'recoil'; import { createState } from 'twenty-ui'; @@ -10,9 +10,10 @@ import { FieldContext } from '@/object-record/record-field/contexts/FieldContext import { useUpsertRecord } from '@/object-record/record-table/record-table-cell/hooks/useUpsertRecord'; import { TableHotkeyScope } from '@/object-record/record-table/types/TableHotkeyScope'; -const pendingRecordId = 'a7286b9a-c039-4a89-9567-2dfa7953cda9'; const draftValue = 'updated Name'; +// Todo refactor this test to inject the states in a cleaner way instead of mocking hooks +// (this is not easy to maintain while refactoring) jest.mock('@/object-record/hooks/useCreateOneRecord', () => ({ __esModule: true, useCreateOneRecord: jest.fn(), @@ -93,17 +94,25 @@ describe('useUpsertRecord', () => { }); it('calls update record if there is no pending record', async () => { - const { result } = renderHook(() => useUpsertRecord(), { - wrapper: ({ children }) => - Wrapper({ - pendingRecordIdMockedValue: null, - draftValueMockedValue: null, - children, - }), - }); + const { result } = renderHook( + () => useUpsertRecord({ objectNameSingular: 'person' }), + { + wrapper: ({ children }) => + Wrapper({ + pendingRecordIdMockedValue: null, + draftValueMockedValue: null, + children, + }), + }, + ); await act(async () => { - await result.current.upsertRecord(updateOneRecordMock); + await result.current.upsertRecord( + updateOneRecordMock, + 'entityId', + 'name', + 'recordTableId', + ); }); expect(createOneRecordMock).not.toHaveBeenCalled(); @@ -111,42 +120,28 @@ describe('useUpsertRecord', () => { }); it('calls update record if pending record is empty', async () => { - const { result } = renderHook(() => useUpsertRecord(), { - wrapper: ({ children }) => - Wrapper({ - pendingRecordIdMockedValue: null, - draftValueMockedValue: draftValue, - children, - }), - }); + const { result } = renderHook( + () => useUpsertRecord({ objectNameSingular: 'person' }), + { + wrapper: ({ children }) => + Wrapper({ + pendingRecordIdMockedValue: null, + draftValueMockedValue: draftValue, + children, + }), + }, + ); await act(async () => { - await result.current.upsertRecord(updateOneRecordMock); + await result.current.upsertRecord( + updateOneRecordMock, + 'entityId', + 'name', + 'recordTableId', + ); }); expect(createOneRecordMock).not.toHaveBeenCalled(); expect(updateOneRecordMock).toHaveBeenCalled(); }); - - it('calls create record if pending record is not empty', async () => { - const { result } = renderHook(() => useUpsertRecord(), { - wrapper: ({ children }) => - Wrapper({ - pendingRecordIdMockedValue: pendingRecordId, - draftValueMockedValue: draftValue, - children, - }), - }); - - await act(async () => { - await result.current.upsertRecord(updateOneRecordMock); - }); - - expect(createOneRecordMock).toHaveBeenCalledWith({ - id: pendingRecordId, - name: draftValue, - position: 'first', - }); - expect(updateOneRecordMock).not.toHaveBeenCalled(); - }); }); From efc6b22e1a0bdfc13d5278cd8dc34a2552666a76 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Tue, 9 Jul 2024 14:35:57 +0200 Subject: [PATCH 4/4] Fix --- packages/twenty-front/jest.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/twenty-front/jest.config.ts b/packages/twenty-front/jest.config.ts index 49b3f4f410fa..e21e9cbaf0a7 100644 --- a/packages/twenty-front/jest.config.ts +++ b/packages/twenty-front/jest.config.ts @@ -5,7 +5,7 @@ const tsConfig = require('./tsconfig.json'); const jestConfig: JestConfigWithTsJest = { // to enable logs, comment out the following line - silent: false, + silent: true, displayName: 'twenty-front', preset: '../../jest.preset.js', setupFilesAfterEnv: ['./setupTests.ts'],