From 52e5f7daeb21eb501c256099f5a0f53b4f77d958 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Mon, 4 Nov 2024 17:44:50 +0100 Subject: [PATCH] Unselect record table records on table body click (#8306) We have previously fixed the unselection of table records on click outside. However, the ref was mispositioned as it selected the full height table. In the case of low record numbers, we also want the unselection to happen on table body click --- .../record-table/components/RecordTable.tsx | 43 ++++++++++++++++--- .../components/RecordTableWithWrappers.tsx | 24 +---------- .../RecordTableBodyUnselectEffect.tsx} | 12 +++--- .../drag-select/components/DragSelect.tsx | 7 ++- .../hooks/useClickOustideListenerStates.ts | 7 ++- .../hooks/useClickOutsideListener.ts | 15 ++++--- .../hooks/useListenClickOutsideV2.ts | 30 +++++++++++-- ...ListenerMouseDownHappenedComponentState.ts | 7 +++ .../states/lockedListenerIdState.ts | 6 --- 9 files changed, 98 insertions(+), 53 deletions(-) rename packages/twenty-front/src/modules/object-record/record-table/{components/RecordTableInternalEffect.tsx => record-table-body/components/RecordTableBodyUnselectEffect.tsx} (87%) create mode 100644 packages/twenty-front/src/modules/ui/utilities/pointer-event/states/clickOutsideListenerMouseDownHappenedComponentState.ts delete mode 100644 packages/twenty-front/src/modules/ui/utilities/pointer-event/states/lockedListenerIdState.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 3efc8739f..485f4b049 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 @@ -3,14 +3,20 @@ import { isNonEmptyString, isNull } from '@sniptt/guards'; import { RecordTableComponentInstance } from '@/object-record/record-table/components/RecordTableComponentInstance'; import { RecordTableContextProvider } from '@/object-record/record-table/components/RecordTableContextProvider'; +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 { RecordTableBodyUnselectEffect } from '@/object-record/record-table/record-table-body/components/RecordTableBodyUnselectEffect'; 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'; import { tableRowIdsComponentState } from '@/object-record/record-table/states/tableRowIdsComponentState'; +import { DragSelect } from '@/ui/utilities/drag-select/components/DragSelect'; +import { useClickOutsideListener } from '@/ui/utilities/pointer-event/hooks/useClickOutsideListener'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; +import { useRef } from 'react'; const StyledTable = styled.table` border-radius: ${({ theme }) => theme.border.radius.sm}; @@ -32,11 +38,17 @@ export const RecordTable = ({ objectNameSingular, onColumnsChange, }: RecordTableProps) => { + const tableBodyRef = useRef(null); + const isRecordTableInitialLoading = useRecoilComponentValueV2( isRecordTableInitialLoadingComponentState, recordTableId, ); + const { toggleClickOutsideListener } = useClickOutsideListener( + RECORD_TABLE_CLICK_OUTSIDE_LISTENER_ID, + ); + const tableRowIds = useRecoilComponentValueV2( tableRowIdsComponentState, recordTableId, @@ -47,6 +59,10 @@ export const RecordTable = ({ recordTableId, ); + const { resetTableRowSelection, setRowSelected } = useRecordTable({ + recordTableId, + }); + const recordTableIsEmpty = !isRecordTableInitialLoading && tableRowIds.length === 0 && @@ -67,15 +83,32 @@ export const RecordTable = ({ viewBarId={viewBarId} > + {recordTableIsEmpty ? ( ) : ( - - + + + + + { + resetTableRowSelection(); + toggleClickOutsideListener(false); + }} + onDragSelectionChange={setRowSelected} + onDragSelectionEnd={() => { + toggleClickOutsideListener(true); + }} /> - - + )} 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 c91df83a3..d2a82737d 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,4 @@ import styled from '@emotion/styled'; -import { useRef } from 'react'; import { useRecoilCallback } from 'recoil'; import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord'; @@ -7,15 +6,11 @@ import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata' import { RecordTable } from '@/object-record/record-table/components/RecordTable'; import { EntityDeleteContext } from '@/object-record/record-table/contexts/EntityDeleteHookContext'; 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'; import { useSaveCurrentViewFields } from '@/views/hooks/useSaveCurrentViewFields'; import { mapColumnDefinitionsToViewFields } from '@/views/utils/mapColumnDefinitionToViewField'; import { RecordUpdateContext } from '../contexts/EntityUpdateMutationHookContext'; -import { useRecordTable } from '../hooks/useRecordTable'; - -import { RecordTableInternalEffect } from './RecordTableInternalEffect'; const StyledTableWithHeader = styled.div` height: 100%; @@ -45,12 +40,6 @@ export const RecordTableWithWrappers = ({ recordTableId, viewBarId, }: RecordTableWithWrappersProps) => { - const tableBodyRef = useRef(null); - - const { resetTableRowSelection, setRowSelected } = useRecordTable({ - recordTableId, - }); - const { saveViewFields } = useSaveCurrentViewFields(viewBarId); const { deleteOneRecord } = useDeleteOneRecord({ objectNameSingular }); @@ -72,25 +61,14 @@ export const RecordTableWithWrappers = ({ - + - { - resetTableRowSelection(); - }} - onDragSelectionChange={setRowSelected} - /> - diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableInternalEffect.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyUnselectEffect.tsx similarity index 87% rename from packages/twenty-front/src/modules/object-record/record-table/components/RecordTableInternalEffect.tsx rename to packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyUnselectEffect.tsx index ce44b28de..674a37b0f 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableInternalEffect.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyUnselectEffect.tsx @@ -7,16 +7,16 @@ import { TableHotkeyScope } from '@/object-record/record-table/types/TableHotkey import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { useListenClickOutsideV2 } from '@/ui/utilities/pointer-event/hooks/useListenClickOutsideV2'; -type RecordTableInternalEffectProps = { - recordTableId: string; +type RecordTableBodyUnselectEffectProps = { tableBodyRef: React.RefObject; + recordTableId: string; }; -export const RecordTableInternalEffect = ({ - recordTableId, +export const RecordTableBodyUnselectEffect = ({ tableBodyRef, -}: RecordTableInternalEffectProps) => { - const leaveTableFocus = useLeaveTableFocus(recordTableId); + recordTableId, +}: RecordTableBodyUnselectEffectProps) => { + const leaveTableFocus = useLeaveTableFocus(); const { resetTableRowSelection, useMapKeyboardToSoftFocus } = useRecordTable({ recordTableId, diff --git a/packages/twenty-front/src/modules/ui/utilities/drag-select/components/DragSelect.tsx b/packages/twenty-front/src/modules/ui/utilities/drag-select/components/DragSelect.tsx index a9dca6c5e..c298b3e8e 100644 --- a/packages/twenty-front/src/modules/ui/utilities/drag-select/components/DragSelect.tsx +++ b/packages/twenty-front/src/modules/ui/utilities/drag-select/components/DragSelect.tsx @@ -1,9 +1,9 @@ -import { RefObject } from 'react'; import { boxesIntersect, useSelectionContainer, } from '@air/react-drag-to-select'; import { useTheme } from '@emotion/react'; +import { RefObject } from 'react'; import { RGBA } from 'twenty-ui'; import { useDragSelect } from '../hooks/useDragSelect'; @@ -11,13 +11,15 @@ import { useDragSelect } from '../hooks/useDragSelect'; type DragSelectProps = { dragSelectable: RefObject; onDragSelectionChange: (id: string, selected: boolean) => void; - onDragSelectionStart?: () => void; + onDragSelectionStart?: (event: MouseEvent) => void; + onDragSelectionEnd?: (event: MouseEvent) => void; }; export const DragSelect = ({ dragSelectable, onDragSelectionChange, onDragSelectionStart, + onDragSelectionEnd, }: DragSelectProps) => { const theme = useTheme(); const { isDragSelectionStartEnabled } = useDragSelect(); @@ -37,6 +39,7 @@ export const DragSelect = ({ return true; }, onSelectionStart: onDragSelectionStart, + onSelectionEnd: onDragSelectionEnd, onSelectionChange: (box) => { const scrollAwareBox = { ...box, diff --git a/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOustideListenerStates.ts b/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOustideListenerStates.ts index dc2675440..26e7ebca3 100644 --- a/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOustideListenerStates.ts +++ b/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOustideListenerStates.ts @@ -1,7 +1,7 @@ import { clickOutsideListenerCallbacksComponentState } from '@/ui/utilities/pointer-event/states/clickOutsideListenerCallbacksComponentState'; import { clickOutsideListenerIsActivatedComponentState } from '@/ui/utilities/pointer-event/states/clickOutsideListenerIsActivatedComponentState'; import { clickOutsideListenerIsMouseDownInsideComponentState } from '@/ui/utilities/pointer-event/states/clickOutsideListenerIsMouseDownInsideComponentState'; -import { lockedListenerIdState } from '@/ui/utilities/pointer-event/states/lockedListenerIdState'; +import { clickOutsideListenerMouseDownHappenedComponentState } from '@/ui/utilities/pointer-event/states/clickOutsideListenerMouseDownHappenedComponentState'; import { getScopeIdFromComponentId } from '@/ui/utilities/recoil-scope/utils/getScopeIdFromComponentId'; import { extractComponentState } from '@/ui/utilities/state/component-state/utils/extractComponentState'; @@ -22,6 +22,9 @@ export const useClickOustideListenerStates = (componentId: string) => { clickOutsideListenerIsActivatedComponentState, scopeId, ), - lockedListenerIdState, + getClickOutsideListenerMouseDownHappenedState: extractComponentState( + clickOutsideListenerMouseDownHappenedComponentState, + scopeId, + ), }; }; diff --git a/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOutsideListener.ts b/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOutsideListener.ts index b69e06f8b..5a693e1f7 100644 --- a/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOutsideListener.ts +++ b/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOutsideListener.ts @@ -7,17 +7,14 @@ import { useListenClickOutsideV2, } from '@/ui/utilities/pointer-event/hooks/useListenClickOutsideV2'; import { ClickOutsideListenerCallback } from '@/ui/utilities/pointer-event/types/ClickOutsideListenerCallback'; -import { getScopeIdFromComponentId } from '@/ui/utilities/recoil-scope/utils/getScopeIdFromComponentId'; import { toSpliced } from '~/utils/array/toSpliced'; import { isDefined } from '~/utils/isDefined'; export const useClickOutsideListener = (componentId: string) => { - // TODO: improve typing - const scopeId = getScopeIdFromComponentId(componentId) ?? ''; - const { getClickOutsideListenerIsActivatedState, getClickOutsideListenerCallbacksState, + getClickOutsideListenerMouseDownHappenedState, } = useClickOustideListenerStates(componentId); const useListenClickOutside = ({ @@ -53,8 +50,15 @@ export const useClickOutsideListener = (componentId: string) => { ({ set }) => (activated: boolean) => { set(getClickOutsideListenerIsActivatedState, activated); + + if (!activated) { + set(getClickOutsideListenerMouseDownHappenedState, false); + } }, - [getClickOutsideListenerIsActivatedState], + [ + getClickOutsideListenerIsActivatedState, + getClickOutsideListenerMouseDownHappenedState, + ], ); const registerOnClickOutsideCallback = useRecoilCallback( @@ -148,7 +152,6 @@ export const useClickOutsideListener = (componentId: string) => { }; return { - scopeId, useListenClickOutside, toggleClickOutsideListener, useRegisterClickOutsideListenerCallback, diff --git a/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useListenClickOutsideV2.ts b/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useListenClickOutsideV2.ts index 82611db7e..0d2f84c05 100644 --- a/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useListenClickOutsideV2.ts +++ b/packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useListenClickOutsideV2.ts @@ -28,6 +28,7 @@ export const useListenClickOutsideV2 = ({ const { getClickOutsideListenerIsMouseDownInsideState, getClickOutsideListenerIsActivatedState, + getClickOutsideListenerMouseDownHappenedState, } = useClickOustideListenerStates(listenerId); const handleMouseDown = useRecoilCallback( @@ -37,6 +38,8 @@ export const useListenClickOutsideV2 = ({ .getLoadable(getClickOutsideListenerIsActivatedState) .getValue(); + set(getClickOutsideListenerMouseDownHappenedState, true); + const isListening = clickOutsideListenerIsActivated && enabled; if (!isListening) { @@ -92,21 +95,32 @@ export const useListenClickOutsideV2 = ({ } }, [ + getClickOutsideListenerIsActivatedState, + enabled, mode, refs, getClickOutsideListenerIsMouseDownInsideState, - enabled, - getClickOutsideListenerIsActivatedState, + getClickOutsideListenerMouseDownHappenedState, ], ); const handleClickOutside = useRecoilCallback( ({ snapshot }) => (event: MouseEvent | TouchEvent) => { + const clickOutsideListenerIsActivated = snapshot + .getLoadable(getClickOutsideListenerIsActivatedState) + .getValue(); + + const isListening = clickOutsideListenerIsActivated && enabled; + const isMouseDownInside = snapshot .getLoadable(getClickOutsideListenerIsMouseDownInsideState) .getValue(); + const hasMouseDownHappened = snapshot + .getLoadable(getClickOutsideListenerMouseDownHappenedState) + .getValue(); + if (mode === ClickOutsideMode.compareHTMLRef) { const clickedElement = event.target as HTMLElement; let isClickedOnExcluded = false; @@ -132,6 +146,8 @@ export const useListenClickOutsideV2 = ({ .some((ref) => ref.current?.contains(event.target as Node)); if ( + isListening && + hasMouseDownHappened && !clickedOnAtLeastOneRef && !isMouseDownInside && !isClickedOnExcluded @@ -171,13 +187,21 @@ export const useListenClickOutsideV2 = ({ return true; }); - if (!clickedOnAtLeastOneRef && !isMouseDownInside) { + if ( + !clickedOnAtLeastOneRef && + !isMouseDownInside && + isListening && + hasMouseDownHappened + ) { callback(event); } } }, [ + getClickOutsideListenerIsActivatedState, + enabled, getClickOutsideListenerIsMouseDownInsideState, + getClickOutsideListenerMouseDownHappenedState, mode, refs, excludeClassNames, diff --git a/packages/twenty-front/src/modules/ui/utilities/pointer-event/states/clickOutsideListenerMouseDownHappenedComponentState.ts b/packages/twenty-front/src/modules/ui/utilities/pointer-event/states/clickOutsideListenerMouseDownHappenedComponentState.ts new file mode 100644 index 000000000..c9c9298c9 --- /dev/null +++ b/packages/twenty-front/src/modules/ui/utilities/pointer-event/states/clickOutsideListenerMouseDownHappenedComponentState.ts @@ -0,0 +1,7 @@ +import { createComponentState } from '@/ui/utilities/state/component-state/utils/createComponentState'; + +export const clickOutsideListenerMouseDownHappenedComponentState = + createComponentState({ + key: 'clickOutsideListenerMouseDownHappenedComponentState', + defaultValue: false, + }); diff --git a/packages/twenty-front/src/modules/ui/utilities/pointer-event/states/lockedListenerIdState.ts b/packages/twenty-front/src/modules/ui/utilities/pointer-event/states/lockedListenerIdState.ts deleted file mode 100644 index c91421dae..000000000 --- a/packages/twenty-front/src/modules/ui/utilities/pointer-event/states/lockedListenerIdState.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { createState } from 'twenty-ui'; - -export const lockedListenerIdState = createState({ - key: 'lockedListenerIdState', - defaultValue: null, -});