From 1f1318febfe631c81b6025874fd869db005bfe9e Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Thu, 3 Jul 2025 14:04:37 +0200 Subject: [PATCH] Fix a hotkey scope race condition in command menu (#13025) Fixes https://github.com/twentyhq/twenty/issues/12885 This PR fixes a hotkey scope race condition happening on note/task creation. The problem is that `ActivityRichTextEditor` catches the click event before the title cell. So here we prevent this from happening by checking if the record title cell is. This is only temporary and should be improved after the persist logic refactor : https://github.com/twentyhq/core-team-issues/issues/192 --- .../components/ActivityRichTextEditor.tsx | 77 +++++++++++++++---- .../components/RecordTitleCell.tsx | 44 +++++++---- .../RecordTitleCellTextFieldDisplay.tsx | 18 ++--- .../hooks/useRecordTitleCell.tsx | 60 +++++++++++---- .../focus/types/FocusComponentType.ts | 1 + 5 files changed, 147 insertions(+), 53 deletions(-) diff --git a/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx b/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx index 84f64995b6..9c4d7f3ab8 100644 --- a/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx +++ b/packages/twenty-front/src/modules/activities/components/ActivityRichTextEditor.tsx @@ -11,7 +11,6 @@ import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSi import { modifyRecordFromCache } from '@/object-record/cache/utils/modifyRecordFromCache'; import { isFieldValueReadOnly } from '@/object-record/record-field/utils/isFieldValueReadOnly'; import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; -import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope'; import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { isNonTextWritingKey } from '@/ui/utilities/hotkey/utils/isNonTextWritingKey'; import { Key } from 'ts-key-enum'; @@ -33,7 +32,14 @@ import { useDeleteManyRecords } from '@/object-record/hooks/useDeleteManyRecords import { useLazyFetchAllRecords } from '@/object-record/hooks/useLazyFetchAllRecords'; import { useRestoreManyRecords } from '@/object-record/hooks/useRestoreManyRecords'; import { useIsRecordReadOnly } from '@/object-record/record-field/hooks/useIsRecordReadOnly'; +import { isInlineCellInEditModeScopedState } from '@/object-record/record-inline-cell/states/isInlineCellInEditModeScopedState'; +import { useRecordShowContainerData } from '@/object-record/record-show/hooks/useRecordShowContainerData'; +import { RecordTitleCellContainerType } from '@/object-record/record-title-cell/types/RecordTitleCellContainerType'; +import { getRecordTitleCellId } from '@/object-record/record-title-cell/utils/getRecordTitleCellId'; import { BlockEditor } from '@/ui/input/editor/components/BlockEditor'; +import { usePushFocusItemToFocusStack } from '@/ui/utilities/focus/hooks/usePushFocusItemToFocusStack'; +import { useRemoveFocusItemFromFocusStackById } from '@/ui/utilities/focus/hooks/useRemoveFocusItemFromFocusStackById'; +import { FocusComponentType } from '@/ui/utilities/focus/types/FocusComponentType'; import type { PartialBlock } from '@blocknote/core'; import '@blocknote/core/fonts/inter.css'; import '@blocknote/mantine/style.css'; @@ -86,6 +92,10 @@ export const ActivityRichTextEditor = ({ objectNameSingular: CoreObjectNameSingular.Attachment, }); + const { pushFocusItemToFocusStack } = usePushFocusItemToFocusStack(); + const { removeFocusItemFromFocusStackById } = + useRemoveFocusItemFromFocusStackById(); + const { fetchAllRecords: findSoftDeletedAttachments } = useLazyFetchAllRecords({ objectNameSingular: CoreObjectNameSingular.Attachment, @@ -96,11 +106,6 @@ export const ActivityRichTextEditor = ({ }, }); - const { - goBackToPreviousHotkeyScope, - setHotkeyScopeAndMemorizePreviousScope, - } = usePreviousHotkeyScope(); - const { upsertActivity } = useUpsertActivity({ activityObjectNameSingular: activityObjectNameSingular, }); @@ -354,16 +359,60 @@ export const ActivityRichTextEditor = ({ preventDefault: false, }, ); + const { labelIdentifierFieldMetadataItem } = useRecordShowContainerData({ + objectNameSingular: activityObjectNameSingular, + objectRecordId: activityId, + }); - const handleBlockEditorFocus = () => { - setHotkeyScopeAndMemorizePreviousScope({ - scope: ActivityEditorHotkeyScope.ActivityBody, - }); - }; + const recordTitleCellId = getRecordTitleCellId( + activityId, + labelIdentifierFieldMetadataItem?.id, + RecordTitleCellContainerType.ShowPage, + ); - const handlerBlockEditorBlur = () => { - goBackToPreviousHotkeyScope(); - }; + const handleBlockEditorFocus = useRecoilCallback( + ({ snapshot }) => + () => { + const isRecordTitleCellOpen = snapshot + .getLoadable(isInlineCellInEditModeScopedState(recordTitleCellId)) + .getValue(); + + if (isRecordTitleCellOpen) { + editor.domElement?.blur(); + return; + } + + pushFocusItemToFocusStack({ + component: { + instanceId: activityId, + type: FocusComponentType.ACTIVITY_RICH_TEXT_EDITOR, + }, + focusId: activityId, + hotkeyScope: { + scope: ActivityEditorHotkeyScope.ActivityBody, + }, + }); + }, + [recordTitleCellId, activityId, editor, pushFocusItemToFocusStack], + ); + + const handlerBlockEditorBlur = useRecoilCallback( + ({ snapshot }) => + () => { + const isRecordTitleCellOpen = snapshot + .getLoadable(isInlineCellInEditModeScopedState(recordTitleCellId)) + .getValue(); + + if (isRecordTitleCellOpen) { + return; + } + + removeFocusItemFromFocusStackById({ + focusId: activityId, + }); + }, + [activityId, recordTitleCellId, removeFocusItemFromFocusStackById], + ); return ( <> diff --git a/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCell.tsx b/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCell.tsx index afbcfb4008..a949f89e86 100644 --- a/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCell.tsx +++ b/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCell.tsx @@ -8,8 +8,6 @@ import { FieldInputEvent, } from '@/object-record/record-field/types/FieldInputEvent'; -import { useInlineCell } from '../../record-inline-cell/hooks/useInlineCell'; - import { RecordFieldComponentInstanceContext } from '@/object-record/record-field/states/contexts/RecordFieldComponentInstanceContext'; import { RecordTitleCellContainer } from '@/object-record/record-title-cell/components/RecordTitleCellContainer'; import { @@ -18,6 +16,7 @@ import { } from '@/object-record/record-title-cell/components/RecordTitleCellContext'; import { RecordTitleCellFieldDisplay } from '@/object-record/record-title-cell/components/RecordTitleCellFieldDisplay'; import { RecordTitleCellFieldInput } from '@/object-record/record-title-cell/components/RecordTitleCellFieldInput'; +import { useRecordTitleCell } from '@/object-record/record-title-cell/hooks/useRecordTitleCell'; import { RecordTitleCellContainerType } from '@/object-record/record-title-cell/types/RecordTitleCellContainerType'; import { getRecordTitleCellId } from '@/object-record/record-title-cell/utils/getRecordTitleCellId'; @@ -36,35 +35,50 @@ export const RecordTitleCell = ({ const isFieldInputOnly = useIsFieldInputOnly(); - const { closeInlineCell } = useInlineCell( - getRecordTitleCellId( - recordId, - fieldDefinition?.fieldMetadataId, - containerType, - ), - ); + const { closeRecordTitleCell } = useRecordTitleCell(); const handleEnter: FieldInputEvent = (persistField) => { - closeInlineCell(); + closeRecordTitleCell({ + recordId, + fieldMetadataId: fieldDefinition?.fieldMetadataId, + containerType, + }); persistField(); }; - const handleEscape = () => { - closeInlineCell(); + const handleEscape: FieldInputEvent = (persistField) => { + closeRecordTitleCell({ + recordId, + fieldMetadataId: fieldDefinition?.fieldMetadataId, + containerType, + }); + persistField(); }; const handleTab: FieldInputEvent = (persistField) => { - closeInlineCell(); + closeRecordTitleCell({ + recordId, + fieldMetadataId: fieldDefinition?.fieldMetadataId, + containerType, + }); persistField(); }; const handleShiftTab: FieldInputEvent = (persistField) => { - closeInlineCell(); + closeRecordTitleCell({ + recordId, + fieldMetadataId: fieldDefinition?.fieldMetadataId, + containerType, + }); persistField(); }; const handleClickOutside: FieldInputClickOutsideEvent = (persistField) => { - closeInlineCell(); + closeRecordTitleCell({ + recordId, + fieldMetadataId: fieldDefinition?.fieldMetadataId, + containerType, + }); persistField(); }; diff --git a/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCellTextFieldDisplay.tsx b/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCellTextFieldDisplay.tsx index cb7a15b9a0..5e11ae18d4 100644 --- a/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCellTextFieldDisplay.tsx +++ b/packages/twenty-front/src/modules/object-record/record-title-cell/components/RecordTitleCellTextFieldDisplay.tsx @@ -1,9 +1,7 @@ import { FieldContext } from '@/object-record/record-field/contexts/FieldContext'; -import { INLINE_CELL_HOTKEY_SCOPE_MEMOIZE_KEY } from '@/object-record/record-inline-cell/constants/InlineCellHotkeyScopeMemoizeKey'; -import { useInlineCell } from '@/object-record/record-inline-cell/hooks/useInlineCell'; import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; -import { TitleInputHotkeyScope } from '@/ui/input/types/TitleInputHotkeyScope'; -import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope'; +import { useRecordTitleCell } from '@/object-record/record-title-cell/hooks/useRecordTitleCell'; +import { RecordTitleCellContainerType } from '@/object-record/record-title-cell/types/RecordTitleCellContainerType'; import { Theme, withTheme } from '@emotion/react'; import styled from '@emotion/styled'; import { useContext } from 'react'; @@ -40,18 +38,16 @@ export const RecordTitleCellSingleTextDisplayMode = () => { const isEmpty = recordValue?.[fieldDefinition.metadata.fieldName]?.trim() === ''; - const { openInlineCell } = useInlineCell(); - - const { setHotkeyScopeAndMemorizePreviousScope } = usePreviousHotkeyScope(); + const { openRecordTitleCell } = useRecordTitleCell(); return ( { - setHotkeyScopeAndMemorizePreviousScope({ - scope: TitleInputHotkeyScope.TitleInput, - memoizeKey: INLINE_CELL_HOTKEY_SCOPE_MEMOIZE_KEY, + openRecordTitleCell({ + recordId, + fieldMetadataId: fieldDefinition.fieldMetadataId, + containerType: RecordTitleCellContainerType.ShowPage, }); - openInlineCell(); }} > {isEmpty ? ( diff --git a/packages/twenty-front/src/modules/object-record/record-title-cell/hooks/useRecordTitleCell.tsx b/packages/twenty-front/src/modules/object-record/record-title-cell/hooks/useRecordTitleCell.tsx index a77615ebb2..4e18a4808f 100644 --- a/packages/twenty-front/src/modules/object-record/record-title-cell/hooks/useRecordTitleCell.tsx +++ b/packages/twenty-front/src/modules/object-record/record-title-cell/hooks/useRecordTitleCell.tsx @@ -6,7 +6,9 @@ import { RecordTitleCellContainerType } from '@/object-record/record-title-cell/ import { getRecordTitleCellId } from '@/object-record/record-title-cell/utils/getRecordTitleCellId'; import { TitleInputHotkeyScope } from '@/ui/input/types/TitleInputHotkeyScope'; import { useGoBackToPreviousDropdownFocusId } from '@/ui/layout/dropdown/hooks/useGoBackToPreviousDropdownFocusId'; -import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope'; +import { usePushFocusItemToFocusStack } from '@/ui/utilities/focus/hooks/usePushFocusItemToFocusStack'; +import { useRemoveFocusItemFromFocusStackById } from '@/ui/utilities/focus/hooks/useRemoveFocusItemFromFocusStackById'; +import { FocusComponentType } from '@/ui/utilities/focus/types/FocusComponentType'; import { HotkeyScope } from '@/ui/utilities/hotkey/types/HotkeyScope'; import { useRecoilCallback } from 'recoil'; import { isDefined } from 'twenty-shared/utils'; @@ -15,10 +17,9 @@ export const useRecordTitleCell = () => { const { goBackToPreviousDropdownFocusId } = useGoBackToPreviousDropdownFocusId(); - const { - setHotkeyScopeAndMemorizePreviousScope, - goBackToPreviousHotkeyScope, - } = usePreviousHotkeyScope(); + const { pushFocusItemToFocusStack } = usePushFocusItemToFocusStack(); + const { removeFocusItemFromFocusStackById } = + useRemoveFocusItemFromFocusStackById(); const closeRecordTitleCell = useRecoilCallback( ({ set }) => @@ -38,11 +39,17 @@ export const useRecordTitleCell = () => { false, ); - goBackToPreviousHotkeyScope(INLINE_CELL_HOTKEY_SCOPE_MEMOIZE_KEY); + removeFocusItemFromFocusStackById({ + focusId: getRecordTitleCellId( + recordId, + fieldMetadataId, + containerType, + ), + }); goBackToPreviousDropdownFocusId(); }, - [goBackToPreviousDropdownFocusId, goBackToPreviousHotkeyScope], + [goBackToPreviousDropdownFocusId, removeFocusItemFromFocusStackById], ); const initFieldInputDraftValue = useInitDraftValueV2(); @@ -61,14 +68,41 @@ export const useRecordTitleCell = () => { customEditHotkeyScopeForField?: HotkeyScope; }) => { if (isDefined(customEditHotkeyScopeForField)) { - setHotkeyScopeAndMemorizePreviousScope({ - scope: customEditHotkeyScopeForField.scope, - customScopes: customEditHotkeyScopeForField.customScopes, + pushFocusItemToFocusStack({ + focusId: getRecordTitleCellId( + recordId, + fieldMetadataId, + containerType, + ), + component: { + type: FocusComponentType.OPENED_FIELD_INPUT, + instanceId: getRecordTitleCellId( + recordId, + fieldMetadataId, + containerType, + ), + }, + hotkeyScope: customEditHotkeyScopeForField, memoizeKey: INLINE_CELL_HOTKEY_SCOPE_MEMOIZE_KEY, }); } else { - setHotkeyScopeAndMemorizePreviousScope({ - scope: TitleInputHotkeyScope.TitleInput, + pushFocusItemToFocusStack({ + focusId: getRecordTitleCellId( + recordId, + fieldMetadataId, + containerType, + ), + component: { + type: FocusComponentType.OPENED_FIELD_INPUT, + instanceId: getRecordTitleCellId( + recordId, + fieldMetadataId, + containerType, + ), + }, + hotkeyScope: { + scope: TitleInputHotkeyScope.TitleInput, + }, memoizeKey: INLINE_CELL_HOTKEY_SCOPE_MEMOIZE_KEY, }); } @@ -98,7 +132,7 @@ export const useRecordTitleCell = () => { fieldComponentInstanceId: recordTitleCellId, }); }, - [initFieldInputDraftValue, setHotkeyScopeAndMemorizePreviousScope], + [initFieldInputDraftValue, pushFocusItemToFocusStack], ); return { diff --git a/packages/twenty-front/src/modules/ui/utilities/focus/types/FocusComponentType.ts b/packages/twenty-front/src/modules/ui/utilities/focus/types/FocusComponentType.ts index 0fc4b16185..446552cbed 100644 --- a/packages/twenty-front/src/modules/ui/utilities/focus/types/FocusComponentType.ts +++ b/packages/twenty-front/src/modules/ui/utilities/focus/types/FocusComponentType.ts @@ -9,4 +9,5 @@ export enum FocusComponentType { RECORD_TABLE_CELL = 'record-table-cell', RECORD_BOARD = 'record-board', RECORD_BOARD_CARD = 'record-board-card', + ACTIVITY_RICH_TEXT_EDITOR = 'activity-rich-text-editor', }