From 079040f1c0d0a450e075e2a1f82fde97b2c164ee Mon Sep 17 00:00:00 2001 From: Thomas des Francs Date: Wed, 17 Jun 2026 10:46:07 +0200 Subject: [PATCH] Fix selectable list arrow focus (#21679) ## Summary Tested on all select fields one by one - Keep searchable selectable-list inputs focused while ArrowUp/ArrowDown moves the selected item. - Remove the old global "grid focused" mode and blur/refocus recovery path. - Scroll the selected item into view with `block: 'nearest'`, which restores keyboard scrolling in long relation pickers without forcing the row to the top. - Add focused regressions for command-menu input focus and selected-item scrolling. ## Root Cause `SelectableList` hotkeys blurred the active input before arrow navigation and stored a global grid-focused state. That let ArrowDown move selection, but focus could fall back to the underlying page/table instead of remaining in the command menu input. ## Recording ### Before https://github.com/user-attachments/assets/a802cbc3-4cfd-4466-bc22-274935a77715 ### After https://github.com/user-attachments/assets/9c315d2e-dcb1-424d-80c9-a5942eb1b6bd ## QA Note I checked all inputs one by one with a 2h30 agent in goal mode. Review in cubic --- .../__tests__/SidePanelTopBar.test.tsx | 137 ++++++++++++++++++ .../useInputFocusWithoutScrollOnMount.ts | 9 +- .../components/SelectableList.tsx | 9 -- .../components/SelectableListItem.tsx | 12 +- .../__tests__/SelectableListItem.test.tsx | 78 ++++++++++ .../internal/useSelectableListHotKeys.ts | 84 +---------- .../isSelectableListGridFocusedState.ts | 6 - 7 files changed, 224 insertions(+), 111 deletions(-) create mode 100644 packages/twenty-front/src/modules/side-panel/components/__tests__/SidePanelTopBar.test.tsx create mode 100644 packages/twenty-front/src/modules/ui/layout/selectable-list/components/__tests__/SelectableListItem.test.tsx delete mode 100644 packages/twenty-front/src/modules/ui/layout/selectable-list/states/isSelectableListGridFocusedState.ts diff --git a/packages/twenty-front/src/modules/side-panel/components/__tests__/SidePanelTopBar.test.tsx b/packages/twenty-front/src/modules/side-panel/components/__tests__/SidePanelTopBar.test.tsx new file mode 100644 index 0000000000..86efc56dda --- /dev/null +++ b/packages/twenty-front/src/modules/side-panel/components/__tests__/SidePanelTopBar.test.tsx @@ -0,0 +1,137 @@ +import { i18n } from '@lingui/core'; +import { I18nProvider } from '@lingui/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { createStore, Provider as JotaiProvider } from 'jotai'; + +import { SIDE_PANEL_FOCUS_ID } from '@/side-panel/constants/SidePanelFocusId'; +import { SIDE_PANEL_SELECTABLE_LIST_ID } from '@/side-panel/constants/SidePanelSelectableListId'; +import { SidePanelList } from '@/side-panel/components/SidePanelList'; +import { SidePanelTopBar } from '@/side-panel/components/SidePanelTopBar'; +import { isSidePanelOpenedState } from '@/side-panel/states/isSidePanelOpenedState'; +import { sidePanelNavigationStackState } from '@/side-panel/states/sidePanelNavigationStackState'; +import { sidePanelPageState } from '@/side-panel/states/sidePanelPageState'; +import { SelectableListItem } from '@/ui/layout/selectable-list/components/SelectableListItem'; +import { selectedItemIdComponentState } from '@/ui/layout/selectable-list/states/selectedItemIdComponentState'; +import { PageFocusId } from '@/types/PageFocusId'; +import { focusStackState } from '@/ui/utilities/focus/states/focusStackState'; +import { FocusComponentType } from '@/ui/utilities/focus/types/FocusComponentType'; +import { SidePanelPages } from 'twenty-shared/types'; +import { IconDotsVertical } from 'twenty-ui-deprecated/display'; + +jest.mock('@/side-panel/components/SidePanelTopBarInputFocusEffect', () => ({ + SidePanelTopBarInputFocusEffect: () => null, +})); + +jest.mock('@/side-panel/components/SidePanelTopBarRightCornerIcon', () => ({ + SidePanelTopBarRightCornerIcon: () => null, +})); + +jest.mock('@/side-panel/hooks/useSidePanelContextChips', () => ({ + useSidePanelContextChips: () => ({ contextChips: [] }), +})); + +jest.mock('@/side-panel/hooks/useSidePanelMenu', () => ({ + useSidePanelMenu: () => ({ + closeSidePanelMenu: jest.fn(), + }), +})); + +jest.mock('twenty-ui-deprecated/utilities', () => ({ + useIsMobile: () => true, +})); + +const recordIndexFocusItem = { + focusId: PageFocusId.RecordIndex, + componentInstance: { + componentType: FocusComponentType.PAGE, + componentInstanceId: PageFocusId.RecordIndex, + }, + globalHotkeysConfig: { + enableGlobalHotkeysWithModifiers: true, + enableGlobalHotkeysConflictingWithKeyboard: true, + }, +}; + +const createSidePanelTopBarStore = () => { + const store = createStore(); + + store.set(isSidePanelOpenedState.atom, true); + store.set(sidePanelPageState.atom, SidePanelPages.CommandMenuDisplay); + store.set(sidePanelNavigationStackState.atom, [ + { + page: SidePanelPages.CommandMenuDisplay, + pageTitle: 'Command Menu', + pageIcon: IconDotsVertical, + pageId: 'command-menu', + }, + ]); + store.set(focusStackState.atom, [recordIndexFocusItem]); + + return store; +}; + +const renderSidePanelCommandMenu = () => { + const store = createSidePanelTopBarStore(); + + render( + + + + + + First item + + + Second item + + + + , + ); + + return { store }; +}; + +describe('SidePanelTopBar', () => { + it('keeps the command menu search input focused while arrowing through items', async () => { + const { store } = renderSidePanelCommandMenu(); + + const input = screen.getByTestId(SIDE_PANEL_FOCUS_ID); + + input.focus(); + + await waitFor(() => { + expect( + store.get( + selectedItemIdComponentState.atomFamily({ + instanceId: SIDE_PANEL_SELECTABLE_LIST_ID, + }), + ), + ).toBe('first-item'); + }); + + fireEvent.keyDown(input, { + key: 'ArrowDown', + code: 'ArrowDown', + }); + + await waitFor(() => { + expect( + store.get( + selectedItemIdComponentState.atomFamily({ + instanceId: SIDE_PANEL_SELECTABLE_LIST_ID, + }), + ), + ).toBe('second-item'); + }); + + expect(document.activeElement).toBe(input); + expect(store.get(focusStackState.atom).at(-1)).toMatchObject({ + focusId: SIDE_PANEL_FOCUS_ID, + componentInstance: { + componentType: FocusComponentType.TEXT_INPUT, + componentInstanceId: SIDE_PANEL_FOCUS_ID, + }, + }); + }); +}); diff --git a/packages/twenty-front/src/modules/ui/input/hooks/useInputFocusWithoutScrollOnMount.ts b/packages/twenty-front/src/modules/ui/input/hooks/useInputFocusWithoutScrollOnMount.ts index 30b834fb78..21d9c5f4c9 100644 --- a/packages/twenty-front/src/modules/ui/input/hooks/useInputFocusWithoutScrollOnMount.ts +++ b/packages/twenty-front/src/modules/ui/input/hooks/useInputFocusWithoutScrollOnMount.ts @@ -1,18 +1,11 @@ -import { useStore } from 'jotai'; import { useEffect, useRef } from 'react'; import { isDefined } from 'twenty-shared/utils'; -import { isSelectableListGridFocusedState } from '@/ui/layout/selectable-list/states/isSelectableListGridFocusedState'; - export const useInputFocusWithoutScrollOnMount = () => { const inputRef = useRef(null); - const store = useStore(); useEffect(() => { - if ( - isDefined(inputRef.current) && - !store.get(isSelectableListGridFocusedState.atom) - ) { + if (isDefined(inputRef.current)) { inputRef.current.focus({ preventScroll: true }); } }); 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 ce65f22573..3cb927874d 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 @@ -1,10 +1,8 @@ -import { useStore } from 'jotai'; import { type ReactNode, useEffect } from 'react'; import { useSelectableListHotKeys } from '@/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys'; import { SelectableListComponentInstanceContext } from '@/ui/layout/selectable-list/states/contexts/SelectableListComponentInstanceContext'; import { SelectableListContextProvider } from '@/ui/layout/selectable-list/states/contexts/SelectableListContext'; -import { isSelectableListGridFocusedState } from '@/ui/layout/selectable-list/states/isSelectableListGridFocusedState'; import { selectableItemIdsComponentState } from '@/ui/layout/selectable-list/states/selectableItemIdsComponentState'; import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState'; import { isDefined } from 'twenty-shared/utils'; @@ -29,7 +27,6 @@ export const SelectableList = ({ }: SelectableListProps) => { useSelectableListHotKeys(selectableListInstanceId, focusId, onSelect); - const store = useStore(); const setSelectableItemIds = useSetAtomComponentState( selectableItemIdsComponentState, selectableListInstanceId, @@ -51,12 +48,6 @@ export const SelectableList = ({ } }, [selectableItemIdArray, selectableItemIdMatrix, setSelectableItemIds]); - useEffect(() => { - return () => { - store.set(isSelectableListGridFocusedState.atom, false); - }; - }, [store]); - return ( { + const scrollIntoViewMock = jest.fn(); + + beforeEach(() => { + scrollIntoViewMock.mockClear(); + + Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', { + configurable: true, + value: scrollIntoViewMock, + }); + }); + + afterAll(() => { + if (typeof originalScrollIntoView === 'function') { + Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', { + configurable: true, + value: originalScrollIntoView, + }); + + return; + } + + delete ( + HTMLElement.prototype as { + scrollIntoView?: HTMLElement['scrollIntoView']; + } + ).scrollIntoView; + }); + + it('scrolls the selected item into view even when the scroll wrapper is at the top', async () => { + const store = createStore(); + + store.set( + isSelectedItemIdComponentFamilyState.atomFamily({ + instanceId: selectableListInstanceId, + familyKey: 'second-item', + }), + true, + ); + + render( + +
+ + + First item + + + Second item + + +
+ , + ); + + await waitFor(() => { + expect(scrollIntoViewMock).toHaveBeenCalledWith({ + behavior: 'auto', + block: 'nearest', + }); + }); + }); +}); 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 8f02acac7f..4bad752347 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 @@ -1,9 +1,8 @@ import { isNonEmptyString } from '@sniptt/guards'; import { useStore } from 'jotai'; -import { useCallback, useRef } from 'react'; +import { useCallback } from 'react'; import { Key } from 'ts-key-enum'; -import { isSelectableListGridFocusedState } from '@/ui/layout/selectable-list/states/isSelectableListGridFocusedState'; import { isSelectedItemIdComponentFamilyState } from '@/ui/layout/selectable-list/states/isSelectedItemIdComponentFamilyState'; import { selectableItemIdsComponentState } from '@/ui/layout/selectable-list/states/selectableItemIdsComponentState'; import { selectedItemIdComponentState } from '@/ui/layout/selectable-list/states/selectedItemIdComponentState'; @@ -16,41 +15,8 @@ export const useSelectableListHotKeys = ( focusId: string, onSelect?: (itemId: string) => void, ) => { - // oxlint-disable-next-line twenty/no-state-useref - const lastBlurredInputRef = useRef(null); - const store = useStore(); - const blurActiveInputIfNeeded = () => { - if (document.activeElement instanceof HTMLInputElement) { - lastBlurredInputRef.current = document.activeElement; - store.set(isSelectableListGridFocusedState.atom, true); - document.activeElement.blur(); - } - }; - - const refocusBlurredInput = () => { - if (!lastBlurredInputRef.current) { - return; - } - store.set(isSelectableListGridFocusedState.atom, false); - lastBlurredInputRef.current.focus(); - lastBlurredInputRef.current = null; - }; - - const clearSelection = (selectedItemId: string | null) => { - if (isNonEmptyString(selectedItemId)) { - store.set(selectedItemIdComponentState.atomFamily({ instanceId }), null); - store.set( - isSelectedItemIdComponentFamilyState.atomFamily({ - instanceId, - familyKey: selectedItemId, - }), - false, - ); - } - }; - const findPosition = ( selectableItemIds: string[][], selectedItemId?: string | null, @@ -173,57 +139,19 @@ export const useSelectableListHotKeys = ( useHotkeysOnFocusedElement({ keys: Key.ArrowUp, callback: () => { - blurActiveInputIfNeeded(); - - const selectedItemId = store.get( - selectedItemIdComponentState.atomFamily({ instanceId }), - ); - const selectableItemIds = store.get( - selectableItemIdsComponentState.atomFamily({ instanceId }), - ); - const position = findPosition(selectableItemIds, selectedItemId); - const isAtTop = position !== undefined && position.row === 0; - - if (!isAtTop || !lastBlurredInputRef.current) { - handleSelect('up'); - return; - } - - clearSelection(selectedItemId); - refocusBlurredInput(); - }, - focusId, - dependencies: [handleSelect, store, instanceId], - }); - - useHotkeysOnFocusedElement({ - keys: Key.ArrowDown, - callback: () => { - blurActiveInputIfNeeded(); - handleSelect('down'); + handleSelect('up'); }, focusId, dependencies: [handleSelect], }); useHotkeysOnFocusedElement({ - keys: '*', - callback: (keyboardEvent) => { - if (keyboardEvent.key.length !== 1) { - return; - } - if ( - keyboardEvent.metaKey || - keyboardEvent.ctrlKey || - keyboardEvent.altKey - ) { - return; - } - refocusBlurredInput(); + keys: Key.ArrowDown, + callback: () => { + handleSelect('down'); }, focusId, - dependencies: [], - options: { enableOnFormTags: false, preventDefault: false }, + dependencies: [handleSelect], }); useHotkeysOnFocusedElement({ diff --git a/packages/twenty-front/src/modules/ui/layout/selectable-list/states/isSelectableListGridFocusedState.ts b/packages/twenty-front/src/modules/ui/layout/selectable-list/states/isSelectableListGridFocusedState.ts deleted file mode 100644 index 6781643228..0000000000 --- a/packages/twenty-front/src/modules/ui/layout/selectable-list/states/isSelectableListGridFocusedState.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { createAtomState } from '@/ui/utilities/state/jotai/utils/createAtomState'; - -export const isSelectableListGridFocusedState = createAtomState({ - key: 'isSelectableListGridFocusedState', - defaultValue: false, -});