From 7ed142e987db3be9238134bd768396840632c7d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Bosi?= <71827178+bosiraphael@users.noreply.github.com> Date: Mon, 17 Feb 2025 12:02:17 +0100 Subject: [PATCH] Fix command menu selection (#10248) - Created a state `hasUserSelectedCommandState` : This state is set to `true` when the user selects an element in the command menu list. It is set to false upon redirection or when the command menu is closed. - Modified `CommandMenuDefaultSelectionEffect` to have the expected selection behavior for the command menu --- .../CommandMenuDefaultSelectionEffect.tsx | 13 +++++++++++-- .../command-menu/components/CommandMenuList.tsx | 9 +++++++++ .../modules/command-menu/hooks/useCommandMenu.ts | 8 ++++++++ .../states/hasUserSelectedCommandState.ts | 6 ++++++ .../selectable-list/components/SelectableList.tsx | 3 ++- .../hooks/internal/useSelectableListHotKeys.ts | 9 ++++++++- 6 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 packages/twenty-front/src/modules/command-menu/states/hasUserSelectedCommandState.ts diff --git a/packages/twenty-front/src/modules/command-menu/components/CommandMenuDefaultSelectionEffect.tsx b/packages/twenty-front/src/modules/command-menu/components/CommandMenuDefaultSelectionEffect.tsx index 136494f5f..95c27ed91 100644 --- a/packages/twenty-front/src/modules/command-menu/components/CommandMenuDefaultSelectionEffect.tsx +++ b/packages/twenty-front/src/modules/command-menu/components/CommandMenuDefaultSelectionEffect.tsx @@ -1,3 +1,4 @@ +import { hasUserSelectedCommandState } from '@/command-menu/states/hasUserSelectedCommandState'; import { useSelectableList } from '@/ui/layout/selectable-list/hooks/useSelectableList'; import { useEffect } from 'react'; import { useRecoilValue } from 'recoil'; @@ -13,10 +14,13 @@ export const CommandMenuDefaultSelectionEffect = ({ const selectedItemId = useRecoilValue(selectedItemIdState); + const hasUserSelectedCommand = useRecoilValue(hasUserSelectedCommandState); + useEffect(() => { if ( isDefined(selectedItemId) && - selectableItemIds.includes(selectedItemId) + selectableItemIds.includes(selectedItemId) && + hasUserSelectedCommand ) { return; } @@ -24,7 +28,12 @@ export const CommandMenuDefaultSelectionEffect = ({ if (selectableItemIds.length > 0) { setSelectedItemId(selectableItemIds[0]); } - }, [selectableItemIds, selectedItemId, setSelectedItemId]); + }, [ + hasUserSelectedCommand, + selectableItemIds, + selectedItemId, + setSelectedItemId, + ]); return null; }; diff --git a/packages/twenty-front/src/modules/command-menu/components/CommandMenuList.tsx b/packages/twenty-front/src/modules/command-menu/components/CommandMenuList.tsx index 2b0c61739..a9fa22e05 100644 --- a/packages/twenty-front/src/modules/command-menu/components/CommandMenuList.tsx +++ b/packages/twenty-front/src/modules/command-menu/components/CommandMenuList.tsx @@ -7,11 +7,13 @@ import { COMMAND_MENU_SEARCH_BAR_PADDING } from '@/command-menu/constants/Comman import { RESET_CONTEXT_TO_SELECTION } from '@/command-menu/constants/ResetContextToSelection'; import { useCommandMenuOnItemClick } from '@/command-menu/hooks/useCommandMenuOnItemClick'; import { useResetPreviousCommandMenuContext } from '@/command-menu/hooks/useResetPreviousCommandMenuContext'; +import { hasUserSelectedCommandState } from '@/command-menu/states/hasUserSelectedCommandState'; import { SelectableItem } from '@/ui/layout/selectable-list/components/SelectableItem'; import { SelectableList } from '@/ui/layout/selectable-list/components/SelectableList'; import { AppHotkeyScope } from '@/ui/utilities/hotkey/types/AppHotkeyScope'; import { ScrollWrapper } from '@/ui/utilities/scroll/components/ScrollWrapper'; import styled from '@emotion/styled'; +import { useSetRecoilState } from 'recoil'; import { isDefined } from 'twenty-shared'; import { MOBILE_VIEWPORT } from 'twenty-ui'; @@ -75,6 +77,10 @@ export const CommandMenuList = ({ const { resetPreviousCommandMenuContext } = useResetPreviousCommandMenuContext(); + const setHasUserSelectedCommand = useSetRecoilState( + hasUserSelectedCommandState, + ); + return ( <> { + setHasUserSelectedCommand(true); + }} > {children} {commandGroups.map(({ heading, items }) => diff --git a/packages/twenty-front/src/modules/command-menu/hooks/useCommandMenu.ts b/packages/twenty-front/src/modules/command-menu/hooks/useCommandMenu.ts index ee37b894d..ed5673b14 100644 --- a/packages/twenty-front/src/modules/command-menu/hooks/useCommandMenu.ts +++ b/packages/twenty-front/src/modules/command-menu/hooks/useCommandMenu.ts @@ -13,6 +13,7 @@ import { } from '@/command-menu/states/commandMenuNavigationStackState'; import { commandMenuPageState } from '@/command-menu/states/commandMenuPageState'; import { commandMenuPageInfoState } from '@/command-menu/states/commandMenuPageTitle'; +import { hasUserSelectedCommandState } from '@/command-menu/states/hasUserSelectedCommandState'; import { CommandMenuPages } from '@/command-menu/types/CommandMenuPages'; import { contextStoreCurrentViewTypeComponentState } from '@/context-store/states/contextStoreCurrentViewTypeComponentState'; import { contextStoreFiltersComponentState } from '@/context-store/states/contextStoreFiltersComponentState'; @@ -60,6 +61,7 @@ export const useCommandMenu = () => { set(isCommandMenuOpenedState, true); setHotkeyScopeAndMemorizePreviousScope(AppHotkeyScope.CommandMenuOpen); + set(hasUserSelectedCommandState, false); }, [ copyContextStoreStates, @@ -89,6 +91,7 @@ export const useCommandMenu = () => { set(commandMenuSearchState, ''); set(commandMenuNavigationStackState, []); resetSelectedItem(); + set(hasUserSelectedCommandState, false); goBackToPreviousHotkeyScope(); emitRightDrawerCloseEvent(); @@ -173,6 +176,7 @@ export const useCommandMenu = () => { }); set(commandMenuNavigationStackState, newNavigationStack); + set(hasUserSelectedCommandState, false); }; }, [closeCommandMenu], @@ -201,6 +205,8 @@ export const useCommandMenu = () => { title: newNavigationStackItem?.pageTitle, Icon: newNavigationStackItem?.pageIcon, }); + + set(hasUserSelectedCommandState, false); }; }, []); @@ -270,6 +276,8 @@ export const useCommandMenu = () => { title: undefined, Icon: undefined, }); + + set(hasUserSelectedCommandState, false); }; }, [copyContextStoreStates], diff --git a/packages/twenty-front/src/modules/command-menu/states/hasUserSelectedCommandState.ts b/packages/twenty-front/src/modules/command-menu/states/hasUserSelectedCommandState.ts new file mode 100644 index 000000000..f029ba375 --- /dev/null +++ b/packages/twenty-front/src/modules/command-menu/states/hasUserSelectedCommandState.ts @@ -0,0 +1,6 @@ +import { createState } from 'twenty-ui'; + +export const hasUserSelectedCommandState = createState({ + key: 'hasUserSelectedCommandState', + defaultValue: false, +}); diff --git a/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx b/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx index 1816b60dc..eebfc4b67 100644 --- a/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx +++ b/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx @@ -23,8 +23,9 @@ export const SelectableList = ({ selectableItemIdArray, selectableItemIdMatrix, onEnter, + onSelect, }: SelectableListProps) => { - useSelectableListHotKeys(selectableListId, hotkeyScope); + useSelectableListHotKeys(selectableListId, hotkeyScope, onSelect); const { setSelectableItemIds, setSelectableListOnEnter, setSelectedItemId } = useSelectableList(selectableListId); diff --git a/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.ts b/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.ts index 12c100ed6..60832a53b 100644 --- a/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.ts +++ b/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.ts @@ -11,6 +11,7 @@ type Direction = 'up' | 'down' | 'left' | 'right'; export const useSelectableListHotKeys = ( scopeId: string, hotkeyScope: string, + onSelect?: (itemId: string) => void, ) => { const findPosition = ( selectableItemIds: string[][], @@ -105,6 +106,7 @@ export const useSelectableListHotKeys = ( if (isNonEmptyString(nextId)) { set(isSelectedItemIdSelector(nextId), true); set(selectedItemIdState, nextId); + onSelect?.(nextId); } if (isNonEmptyString(selectedItemId)) { @@ -112,7 +114,12 @@ export const useSelectableListHotKeys = ( } } }, - [isSelectedItemIdSelector, selectableItemIdsState, selectedItemIdState], + [ + isSelectedItemIdSelector, + onSelect, + selectableItemIdsState, + selectedItemIdState, + ], ); useScopedHotkeys(Key.ArrowUp, () => handleSelect('up'), hotkeyScope, []);