From 4f4f3a64fd95cef4d8f1a58118384048748ad0e5 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Mon, 19 May 2025 22:44:51 +0200 Subject: [PATCH] Fix table re-renders on update or keyboard navigation (#12127) This PR fixes the problem of full table re-render on any update or keyboard navigation. This was due to a recoil state subscribe in the RecordTable component, I just moved it in the children effect components so that the Flux dependency becomes inoffensive. I also extracted one hook from the useRecordTable hook that we have to refactor gradually. Fixes https://github.com/twentyhq/core-team-issues/issues/979 Fixes https://github.com/twentyhq/core-team-issues/issues/932 --- .../record-table/components/RecordTable.tsx | 19 +----- .../RecordTableScrollToFocusedCellEffect.tsx | 12 +++- .../hooks/useMapKeyboardToFocus.ts | 68 +++++++++++++++++++ .../record-table/hooks/useRecordTable.ts | 66 ------------------ .../RecordTableBodyFocusKeyboardEffect.tsx | 8 +-- 5 files changed, 83 insertions(+), 90 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-table/hooks/useMapKeyboardToFocus.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 510694369..cf4512878 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 @@ -11,9 +11,7 @@ import { RecordTableScrollToFocusedRowEffect } from '@/object-record/record-tabl import { RECORD_TABLE_CLICK_OUTSIDE_LISTENER_ID } from '@/object-record/record-table/constants/RecordTableClickOutsideListenerId'; import { useRecordTableContextOrThrow } from '@/object-record/record-table/contexts/RecordTableContext'; import { useRecordTable } from '@/object-record/record-table/hooks/useRecordTable'; -import { isRecordTableCellFocusActiveComponentState } from '@/object-record/record-table/states/isRecordTableCellFocusActiveComponentState'; import { isRecordTableInitialLoadingComponentState } from '@/object-record/record-table/states/isRecordTableInitialLoadingComponentState'; -import { isRecordTableRowFocusActiveComponentState } from '@/object-record/record-table/states/isRecordTableRowFocusActiveComponentState'; import { useClickOutsideListener } from '@/ui/utilities/pointer-event/hooks/useClickOutsideListener'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; @@ -48,16 +46,6 @@ export const RecordTable = () => { const recordTableIsEmpty = !isRecordTableInitialLoading && allRecordIds.length === 0; - const isRecordTableCellFocusActive = useRecoilComponentValueV2( - isRecordTableCellFocusActiveComponentState, - recordTableId, - ); - - const isRecordTableRowFocusActive = useRecoilComponentValueV2( - isRecordTableRowFocusActiveComponentState, - recordTableId, - ); - if (!isNonEmptyString(objectNameSingular)) { return <>; } @@ -77,11 +65,8 @@ export const RecordTable = () => { hasRecordGroups={hasRecordGroups} tableBodyRef={tableBodyRef} /> - - {isRecordTableCellFocusActive && } - - {isRecordTableRowFocusActive && } - + + {recordTableIsEmpty && !hasRecordGroups ? ( { const { recordTableId } = useRecordTableContextOrThrow(); + const isRecordTableCellFocusActive = useRecoilComponentValueV2( + isRecordTableCellFocusActiveComponentState, + recordTableId, + ); + const focusPosition = useRecoilComponentValueV2( recordTableFocusPositionComponentState, recordTableId, @@ -14,6 +20,10 @@ export const RecordTableScrollToFocusedCellEffect = () => { // Handle cell focus useEffect(() => { + if (!isRecordTableCellFocusActive) { + return; + } + if (!focusPosition) { return; } @@ -52,7 +62,7 @@ export const RecordTableScrollToFocusedCellEffect = () => { focusElement.style.scrollMarginBottom = ''; } }; - }, [focusPosition]); + }, [focusPosition, isRecordTableCellFocusActive]); return null; }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/hooks/useMapKeyboardToFocus.ts b/packages/twenty-front/src/modules/object-record/record-table/hooks/useMapKeyboardToFocus.ts new file mode 100644 index 000000000..6cbb1edaf --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-table/hooks/useMapKeyboardToFocus.ts @@ -0,0 +1,68 @@ +import { RecordIndexHotkeyScope } from '@/object-record/record-index/types/RecordIndexHotkeyScope'; +import { useRecordTableMove } from '@/object-record/record-table/hooks/useRecordTableMove'; +import { TableHotkeyScope } from '@/object-record/record-table/types/TableHotkeyScope'; +import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope'; +import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; +import { Key } from 'ts-key-enum'; + +export const useMapKeyboardToFocus = (recordTableId?: string) => { + const { setHotkeyScopeAndMemorizePreviousScope } = usePreviousHotkeyScope(); + + const { move } = useRecordTableMove(recordTableId); + + useScopedHotkeys( + [Key.ArrowUp, `${Key.Shift}+${Key.Enter}`], + () => { + move('up'); + }, + TableHotkeyScope.TableFocus, + [move], + ); + + useScopedHotkeys( + Key.ArrowDown, + () => { + move('down'); + }, + TableHotkeyScope.TableFocus, + [move], + ); + + useScopedHotkeys( + [Key.ArrowUp], + () => { + setHotkeyScopeAndMemorizePreviousScope(TableHotkeyScope.TableFocus); + move('up'); + }, + RecordIndexHotkeyScope.RecordIndex, + [move], + ); + + useScopedHotkeys( + [Key.ArrowDown], + () => { + setHotkeyScopeAndMemorizePreviousScope(TableHotkeyScope.TableFocus); + move('down'); + }, + RecordIndexHotkeyScope.RecordIndex, + [move], + ); + + useScopedHotkeys( + [Key.ArrowLeft, `${Key.Shift}+${Key.Tab}`], + () => { + move('left'); + }, + TableHotkeyScope.TableFocus, + [move], + ); + + useScopedHotkeys( + [Key.ArrowRight, Key.Tab], + () => { + move('right'); + }, + TableHotkeyScope.TableFocus, + [move], + ); +}; diff --git a/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts b/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts index ab4bdf8af..3295b0bb2 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/hooks/useRecordTable.ts @@ -1,15 +1,11 @@ import { useRecoilCallback } from 'recoil'; -import { Key } from 'ts-key-enum'; import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; import { useSetHasUserSelectedAllRows } from '@/object-record/record-table/hooks/internal/useSetAllRowSelectedState'; -import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { getSnapshotValue } from '@/ui/utilities/recoil-scope/utils/getSnapshotValue'; import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; -import { RecordIndexHotkeyScope } from '@/object-record/record-index/types/RecordIndexHotkeyScope'; import { ColumnDefinition } from '../types/ColumnDefinition'; -import { TableHotkeyScope } from '../types/TableHotkeyScope'; import { availableTableColumnsComponentState } from '@/object-record/record-table/states/availableTableColumnsComponentState'; import { RecordTableComponentInstanceContext } from '@/object-record/record-table/states/context/RecordTableComponentInstanceContext'; @@ -20,7 +16,6 @@ import { onEntityCountChangeComponentState } from '@/object-record/record-table/ import { useRecordTableMove } from '@/object-record/record-table/hooks/useRecordTableMove'; import { onToggleColumnSortComponentState } from '@/object-record/record-table/states/onToggleColumnSortComponentState'; import { tableLastRowVisibleComponentState } from '@/object-record/record-table/states/tableLastRowVisibleComponentState'; -import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope'; import { useAvailableComponentInstanceIdOrThrow } from '@/ui/utilities/state/component-state/hooks/useAvailableComponentInstanceIdOrThrow'; import { useRecoilComponentCallbackStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentCallbackStateV2'; import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2'; @@ -137,68 +132,8 @@ export const useRecordTable = (props?: useRecordTableProps) => { const setFocusPosition = useSetRecordTableFocusPosition(recordTableId); - const { setHotkeyScopeAndMemorizePreviousScope } = usePreviousHotkeyScope(); - const { move } = useRecordTableMove(recordTableId); - const useMapKeyboardToFocus = () => { - useScopedHotkeys( - [Key.ArrowUp, `${Key.Shift}+${Key.Enter}`], - () => { - move('up'); - }, - TableHotkeyScope.TableFocus, - [move], - ); - - useScopedHotkeys( - Key.ArrowDown, - () => { - move('down'); - }, - TableHotkeyScope.TableFocus, - [move], - ); - - useScopedHotkeys( - [Key.ArrowUp], - () => { - setHotkeyScopeAndMemorizePreviousScope(TableHotkeyScope.TableFocus); - move('up'); - }, - RecordIndexHotkeyScope.RecordIndex, - [move], - ); - - useScopedHotkeys( - [Key.ArrowDown], - () => { - setHotkeyScopeAndMemorizePreviousScope(TableHotkeyScope.TableFocus); - move('down'); - }, - RecordIndexHotkeyScope.RecordIndex, - [move], - ); - - useScopedHotkeys( - [Key.ArrowLeft, `${Key.Shift}+${Key.Tab}`], - () => { - move('left'); - }, - TableHotkeyScope.TableFocus, - [move], - ); - - useScopedHotkeys( - [Key.ArrowRight, Key.Tab], - () => { - move('right'); - }, - TableHotkeyScope.TableFocus, - [move], - ); - }; - const { selectAllRows } = useSelectAllRows(recordTableId); return { @@ -210,7 +145,6 @@ export const useRecordTable = (props?: useRecordTableProps) => { setRowSelected, resetTableRowSelection, move, - useMapKeyboardToFocus, selectAllRows, setOnColumnsChange, setIsRecordTableInitialLoading, diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFocusKeyboardEffect.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFocusKeyboardEffect.tsx index 98ae39819..fac0a6392 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFocusKeyboardEffect.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFocusKeyboardEffect.tsx @@ -1,7 +1,7 @@ import { RecordIndexHotkeyScope } from '@/object-record/record-index/types/RecordIndexHotkeyScope'; import { useRecordTableContextOrThrow } from '@/object-record/record-table/contexts/RecordTableContext'; import { useFocusedRecordTableRow } from '@/object-record/record-table/hooks/useFocusedRecordTableRow'; -import { useRecordTable } from '@/object-record/record-table/hooks/useRecordTable'; +import { useMapKeyboardToFocus } from '@/object-record/record-table/hooks/useMapKeyboardToFocus'; import { useSetIsRecordTableFocusActive } from '@/object-record/record-table/record-table-cell/hooks/useSetIsRecordTableFocusActive'; import { isRecordTableCellFocusActiveComponentState } from '@/object-record/record-table/states/isRecordTableCellFocusActiveComponentState'; import { TableHotkeyScope } from '@/object-record/record-table/types/TableHotkeyScope'; @@ -13,10 +13,6 @@ import { Key } from 'ts-key-enum'; export const RecordTableBodyFocusKeyboardEffect = () => { const { recordTableId } = useRecordTableContextOrThrow(); - const { useMapKeyboardToFocus } = useRecordTable({ - recordTableId, - }); - const setHotkeyScope = useSetHotkeyScope(); const { restoreRecordTableRowFocusFromCellPosition } = @@ -29,7 +25,7 @@ export const RecordTableBodyFocusKeyboardEffect = () => { isRecordTableCellFocusActiveComponentState, ); - useMapKeyboardToFocus(); + useMapKeyboardToFocus(recordTableId); useScopedHotkeys( [Key.Escape],