From 97f9fc3f8194623f523fb28ee1141c7c22138656 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Mon, 8 Apr 2024 17:08:30 +0200 Subject: [PATCH] Fixed hotkey bug with Select component and added debug logs for hotkeys (#4879) - Select component was adding a duplicate useListenClickOutside already present in useDropdown for closing dropdown. - Added debug logs for hotkeys scopes --- .../src/modules/ui/input/components/Select.tsx | 10 ---------- .../modules/ui/input/components/TextInput.tsx | 2 ++ .../hotkey/hooks/usePreviousHotkeyScope.ts | 17 +++++++++++++++++ .../hotkey/hooks/useScopedHotkeyCallback.ts | 6 +++--- .../utilities/hotkey/hooks/useSetHotkeyScope.ts | 13 +++++++++++++ 5 files changed, 35 insertions(+), 13 deletions(-) diff --git a/packages/twenty-front/src/modules/ui/input/components/Select.tsx b/packages/twenty-front/src/modules/ui/input/components/Select.tsx index 980a521d3..783958fbd 100644 --- a/packages/twenty-front/src/modules/ui/input/components/Select.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/Select.tsx @@ -10,7 +10,6 @@ import { DropdownMenuSearchInput } from '@/ui/layout/dropdown/components/Dropdow import { DropdownMenuSeparator } from '@/ui/layout/dropdown/components/DropdownMenuSeparator'; import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown'; import { MenuItem } from '@/ui/navigation/menu-item/components/MenuItem'; -import { useClickOutsideListener } from '@/ui/utilities/pointer-event/hooks/useClickOutsideListener'; import { SelectHotkeyScope } from '../types/SelectHotkeyScope'; @@ -113,15 +112,6 @@ export const Select = ({ const { closeDropdown } = useDropdown(dropdownId); - const { useListenClickOutside } = useClickOutsideListener(dropdownId); - - useListenClickOutside({ - refs: [selectContainerRef], - callback: () => { - closeDropdown(); - }, - }); - const selectControl = ( diff --git a/packages/twenty-front/src/modules/ui/input/components/TextInput.tsx b/packages/twenty-front/src/modules/ui/input/components/TextInput.tsx index 206910e8e..f7453fbb1 100644 --- a/packages/twenty-front/src/modules/ui/input/components/TextInput.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/TextInput.tsx @@ -145,6 +145,7 @@ const TextInputComponent = ( const handleFocus: FocusEventHandler = (e) => { onFocus?.(e); + if (!disableHotkeys) { setHotkeyScopeAndMemorizePreviousScope(InputHotkeyScope.TextInput); } @@ -152,6 +153,7 @@ const TextInputComponent = ( const handleBlur: FocusEventHandler = (e) => { onBlur?.(e); + if (!disableHotkeys) { goBackToPreviousHotkeyScope(); } diff --git a/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/usePreviousHotkeyScope.ts b/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/usePreviousHotkeyScope.ts index d332b9df0..a8a9f9011 100644 --- a/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/usePreviousHotkeyScope.ts +++ b/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/usePreviousHotkeyScope.ts @@ -1,5 +1,8 @@ import { useRecoilCallback } from 'recoil'; +import { DEBUG_HOTKEY_SCOPE } from '@/ui/utilities/hotkey/hooks/useScopedHotkeyCallback'; +import { logDebug } from '~/utils/logDebug'; + import { currentHotkeyScopeState } from '../states/internal/currentHotkeyScopeState'; import { previousHotkeyScopeState } from '../states/internal/previousHotkeyScopeState'; import { CustomHotkeyScopes } from '../types/CustomHotkeyScope'; @@ -20,6 +23,11 @@ export const usePreviousHotkeyScope = () => { return; } + // eslint-disable-next-line @nx/workspace-explicit-boolean-predicates-in-if + if (DEBUG_HOTKEY_SCOPE) { + logDebug('DEBUG: goBackToPreviousHotkeyScope', previousHotkeyScope); + } + setHotkeyScope( previousHotkeyScope.scope, previousHotkeyScope.customScopes, @@ -37,6 +45,15 @@ export const usePreviousHotkeyScope = () => { .getLoadable(currentHotkeyScopeState) .getValue(); + // eslint-disable-next-line @nx/workspace-explicit-boolean-predicates-in-if + if (DEBUG_HOTKEY_SCOPE) { + logDebug('DEBUG: setHotkeyScopeAndMemorizePreviousScope', { + currentHotkeyScope, + scope, + customScopes, + }); + } + setHotkeyScope(scope, customScopes); set(previousHotkeyScopeState, currentHotkeyScope); }, diff --git a/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useScopedHotkeyCallback.ts b/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useScopedHotkeyCallback.ts index 6e3d2d102..053ac4c85 100644 --- a/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useScopedHotkeyCallback.ts +++ b/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useScopedHotkeyCallback.ts @@ -5,7 +5,7 @@ import { logDebug } from '~/utils/logDebug'; import { internalHotkeysEnabledScopesState } from '../states/internal/internalHotkeysEnabledScopesState'; -const DEBUG_HOTKEY_SCOPE = false; +export const DEBUG_HOTKEY_SCOPE = true; export const useScopedHotkeyCallback = () => useRecoilCallback( @@ -31,7 +31,7 @@ export const useScopedHotkeyCallback = () => // eslint-disable-next-line @nx/workspace-explicit-boolean-predicates-in-if if (DEBUG_HOTKEY_SCOPE) { logDebug( - `%cI can't call hotkey (${ + `DEBUG: %cI can't call hotkey (${ hotkeysEvent.keys }) because I'm in scope [${scope}] and the active scopes are : [${currentHotkeyScopes.join( ', ', @@ -46,7 +46,7 @@ export const useScopedHotkeyCallback = () => // eslint-disable-next-line @nx/workspace-explicit-boolean-predicates-in-if if (DEBUG_HOTKEY_SCOPE) { logDebug( - `%cI can call hotkey (${ + `DEBUG: %cI can call hotkey (${ hotkeysEvent.keys }) because I'm in scope [${scope}] and the active scopes are : [${currentHotkeyScopes.join( ', ', diff --git a/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useSetHotkeyScope.ts b/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useSetHotkeyScope.ts index ad890e08b..65f4261c4 100644 --- a/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useSetHotkeyScope.ts +++ b/packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useSetHotkeyScope.ts @@ -1,6 +1,8 @@ import { useRecoilCallback } from 'recoil'; +import { DEBUG_HOTKEY_SCOPE } from '@/ui/utilities/hotkey/hooks/useScopedHotkeyCallback'; import { isDefined } from '~/utils/isDefined'; +import { logDebug } from '~/utils/logDebug'; import { DEFAULT_HOTKEYS_SCOPE_CUSTOM_SCOPES } from '../constants/DefaultHotkeysScopeCustomScopes'; import { currentHotkeyScopeState } from '../states/internal/currentHotkeyScopeState'; @@ -76,6 +78,17 @@ export const useSetHotkeyScope = () => } scopesToSet.push(newHotkeyScope.scope); + + // TODO: fix eslint rule not understanding a boolean constant + // See issue https://github.com/twentyhq/twenty/issues/4881 + // eslint-disable-next-line @nx/workspace-explicit-boolean-predicates-in-if + if (DEBUG_HOTKEY_SCOPE) { + logDebug('DEBUG: set new hotkey scope', { + scopesToSet, + newHotkeyScope, + }); + } + set(internalHotkeysEnabledScopesState, scopesToSet); set(currentHotkeyScopeState, newHotkeyScope); },