From 4f7f330dad9a0071cdce89f386506402cb34801c Mon Sep 17 00:00:00 2001 From: Sergei Sharabai <105950333+sharabai@users.noreply.github.com> Date: Mon, 25 May 2026 13:49:54 +0200 Subject: [PATCH 1/7] Fix lists with unselectable row items --- .../SelectionList/BaseSelectionList.tsx | 31 ++-- .../ListItem/ListItemRenderer.tsx | 8 +- .../BaseSelectionListWithSections.tsx | 35 ++--- .../SelectionList/hooks/useSearchFocusSync.ts | 19 +-- src/hooks/useArrowKeyFocusManager.ts | 43 ++++-- tests/unit/useArrowKeyFocusManagerTest.ts | 132 ++++++++++++++++++ 6 files changed, 192 insertions(+), 76 deletions(-) create mode 100644 tests/unit/useArrowKeyFocusManagerTest.ts diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 38f899f71f65..070b37bd3fcc 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -114,7 +114,6 @@ function BaseSelectionList({ const listRef = useRef | null>(null); const itemFocusTimeoutRef = useRef(null); const keyboardListenerRef = useRef | null>(null); - const suppressNextFocusScrollRef = useRef(false); const initialFocusedIndex = useMemo(() => data.findIndex((i) => i.keyForList === initiallyFocusedItemKey), [data, initiallyFocusedItemKey]); const [itemsToHighlight, setItemsToHighlight] = useState | null>(null); @@ -219,9 +218,8 @@ function BaseSelectionList({ maxIndex: data.length - 1, disabledIndexes: dataDetails.disabledArrowKeyIndexes, isActive: isFocused, - onFocusedIndexChange: (index: number) => { - if (suppressNextFocusScrollRef.current) { - suppressNextFocusScrollRef.current = false; + onFocusedIndexChange: (index, {shouldScroll}) => { + if (!shouldScroll) { return; } if (!shouldScrollToFocusedIndex) { @@ -239,14 +237,16 @@ function BaseSelectionList({ }); // Keep the cursor on the restored row so keyboard nav continues from there, but don't scroll to it on the way back. + // Options are forwarded so onFocus callers (e.g. ListItemRenderer) keep their {shouldScroll: false} intent. const setFocusedIndexFromRowFocus = useCallback( - (index: number) => { - if (isFocusRestoreInProgress() && index !== focusedIndex) { - suppressNextFocusScrollRef.current = true; + (index: number, options?: {shouldScroll?: boolean}) => { + if (isFocusRestoreInProgress()) { + setFocusedIndex(index, {shouldScroll: false}); + return; } - setFocusedIndex(index); + setFocusedIndex(index, options); }, - [focusedIndex, setFocusedIndex], + [setFocusedIndex], ); // extraData helps FlashList detect when data changes significantly (e.g., during filtering) @@ -271,10 +271,7 @@ function BaseSelectionList({ } } if (shouldUpdateFocusedIndex && typeof indexToFocus === 'number') { - if (indexToFocus !== focusedIndex) { - suppressNextFocusScrollRef.current = true; - } - setFocusedIndex(indexToFocus); + setFocusedIndex(indexToFocus, {shouldScroll: false}); } onSelectRow(item); @@ -286,7 +283,6 @@ function BaseSelectionList({ isFocused, canSelectMultiple, shouldUpdateFocusedIndex, - focusedIndex, onSelectRow, shouldShowTextInput, shouldClearInputOnSelect, @@ -540,7 +536,7 @@ function BaseSelectionList({ if (newFocusedIndex < 0 || newFocusedIndex >= data.length) { return; } - setFocusedIndex(newFocusedIndex); + setFocusedIndex(newFocusedIndex, {shouldScroll: false}); if (shouldScroll) { scrollToIndex(newFocusedIndex); } @@ -557,10 +553,6 @@ function BaseSelectionList({ setFocusedIndex, }); - const suppressNextFocusScroll = useCallback(() => { - suppressNextFocusScrollRef.current = true; - }, []); - useSearchFocusSync({ searchValue: syncedSearchValue, data, @@ -571,7 +563,6 @@ function BaseSelectionList({ scrollToIndex, setFocusedIndex, focusedIndex, - suppressNextFocusScroll, }); useEffect(() => { diff --git a/src/components/SelectionList/ListItem/ListItemRenderer.tsx b/src/components/SelectionList/ListItem/ListItemRenderer.tsx index 7449ba212602..e4bcce0ae57d 100644 --- a/src/components/SelectionList/ListItem/ListItemRenderer.tsx +++ b/src/components/SelectionList/ListItem/ListItemRenderer.tsx @@ -97,7 +97,13 @@ function ListItemRenderer({ if (isMobileChrome() && event.nativeEvent && !event.nativeEvent.sourceCapabilities) { return; } - setFocusedIndex(normalizedIndex ?? index); + // The row is already tracked as focused by useArrowKeyFocusManager. This focus event is the synchronous + // result of useSyncFocus's layout effect calling el.focus(); calling setFocusedIndex again here would + // clobber shouldScrollNextChangeRef and suppress the arrow-key-driven scroll. See useArrowKeyFocusManager. + if (isFocused) { + return; + } + setFocusedIndex(normalizedIndex ?? index, {shouldScroll: false}); }} shouldSyncFocus={shouldSyncFocus} wrapperStyle={wrapperStyle} diff --git a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx index 342ffc10c15a..848be2b0e69a 100644 --- a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx +++ b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx @@ -95,7 +95,6 @@ function BaseSelectionListWithSections({ const isTextInputFocusedRef = useRef(false); const hasKeyBeenPressed = useRef(false); const [isKeyboardNavigating, setIsKeyboardNavigating] = useState(false); - const suppressNextFocusScrollRef = useRef(false); const activeElementRole = useActiveElementRole(); const {isKeyboardShown} = useKeyboardState(); const {safeAreaPaddingBottomStyle} = useSafeAreaPaddings(); @@ -151,9 +150,8 @@ function BaseSelectionListWithSections({ maxIndex: flattenedData.length - 1, disabledIndexes, isActive: isScreenFocused && itemsCount > 0, - onFocusedIndexChange: (index: number) => { - if (suppressNextFocusScrollRef.current) { - suppressNextFocusScrollRef.current = false; + onFocusedIndexChange: (index, {shouldScroll}) => { + if (!shouldScroll) { return; } if (!shouldScrollToFocusedIndex) { @@ -170,21 +168,14 @@ function BaseSelectionListWithSections({ }, }); - // Move the cursor, and skip the scroll the move would otherwise trigger when the index actually changes. - const setFocusedIndexWithoutScrollOnChange = (index: number) => { - if (index !== focusedIndex) { - suppressNextFocusScrollRef.current = true; - } - setFocusedIndex(index); - }; - // Keep the cursor on the restored row so keyboard nav continues from there, but don't scroll to it on the way back. - const setFocusedIndexFromRowFocus = (index: number) => { + // Options are forwarded so onFocus callers (e.g. ListItemRenderer) keep their {shouldScroll: false} intent. + const setFocusedIndexFromRowFocus = (index: number, options?: {shouldScroll?: boolean}) => { if (isFocusRestoreInProgress()) { - setFocusedIndexWithoutScrollOnChange(index); - } else { - setFocusedIndex(index); + setFocusedIndex(index, {shouldScroll: false}); + return; } + setFocusedIndex(index, options); }; const getFocusedItem = (): TItem | undefined => { @@ -212,7 +203,7 @@ function BaseSelectionListWithSections({ } } if (shouldUpdateFocusedIndex && typeof indexToFocus === 'number') { - setFocusedIndexWithoutScrollOnChange(indexToFocus); + setFocusedIndex(indexToFocus, {shouldScroll: false}); } onSelectRow(item); @@ -238,10 +229,7 @@ function BaseSelectionListWithSections({ }; const updateAndScrollToFocusedIndex = (index: number, shouldScroll = true) => { - if (!shouldScroll) { - suppressNextFocusScrollRef.current = true; - } - setFocusedIndex(index); + setFocusedIndex(index, {shouldScroll: false}); if (shouldScroll) { scrollToIndex(index); } @@ -312,10 +300,6 @@ function BaseSelectionListWithSections({ setFocusedIndex, }); - const suppressNextFocusScroll = () => { - suppressNextFocusScrollRef.current = true; - }; - useSearchFocusSync({ searchValue: syncedSearchValue, data: flattenedData, @@ -327,7 +311,6 @@ function BaseSelectionListWithSections({ setFocusedIndex, focusedIndex, firstFocusableIndex, - suppressNextFocusScroll, }); const textInputComponent = () => { diff --git a/src/components/SelectionList/hooks/useSearchFocusSync.ts b/src/components/SelectionList/hooks/useSearchFocusSync.ts index f11157ad3f62..0dfb78e2a4bf 100644 --- a/src/components/SelectionList/hooks/useSearchFocusSync.ts +++ b/src/components/SelectionList/hooks/useSearchFocusSync.ts @@ -25,16 +25,13 @@ type UseSearchFocusSyncParams = { scrollToIndex: (index: number, animated?: boolean) => void; /** Function to set the focused index */ - setFocusedIndex: (index: number) => void; + setFocusedIndex: (index: number, options?: {shouldScroll?: boolean}) => void; - /** The current focused index — needed to avoid arming scroll suppression when the index won't actually change */ + /** The current focused index — kept for backwards-compatible parameter ordering, no longer used for scroll-suppression bookkeeping */ focusedIndex?: number; /** The first focusable index in the list (useful when index 0 is a header). Defaults to 0. */ firstFocusableIndex?: number; - - /** Optional callback to suppress the scroll that onFocusedIndexChange would otherwise trigger when setFocusedIndex is called */ - suppressNextFocusScroll?: () => void; }; /** @@ -55,7 +52,6 @@ function useSearchFocusSync({ setFocusedIndex, focusedIndex, firstFocusableIndex = 0, - suppressNextFocusScroll, }: UseSearchFocusSyncParams) { const prevSearchValue = usePrevious(searchValue); const prevSelectedOptionsCount = usePrevious(selectedOptionsCount); @@ -80,10 +76,7 @@ function useSearchFocusSync({ if (foundSelectedItemIndex !== -1 && !canSelectMultiple) { scrollToIndex(foundSelectedItemIndex, false); - if (foundSelectedItemIndex !== focusedIndex) { - suppressNextFocusScroll?.(); - } - setFocusedIndex(foundSelectedItemIndex); + setFocusedIndex(foundSelectedItemIndex, {shouldScroll: false}); return; } } @@ -102,10 +95,7 @@ function useSearchFocusSync({ // Scroll to top of list and focus on first focusable item (not header) scrollToIndex(0, false); - if (firstFocusableIndex !== focusedIndex) { - suppressNextFocusScroll?.(); - } - setFocusedIndex(firstFocusableIndex); + setFocusedIndex(firstFocusableIndex, {shouldScroll: false}); }, [ canSelectMultiple, data, @@ -120,7 +110,6 @@ function useSearchFocusSync({ isItemSelected, focusedIndex, firstFocusableIndex, - suppressNextFocusScroll, ]); } diff --git a/src/hooks/useArrowKeyFocusManager.ts b/src/hooks/useArrowKeyFocusManager.ts index 56e233266464..65684c743cf6 100644 --- a/src/hooks/useArrowKeyFocusManager.ts +++ b/src/hooks/useArrowKeyFocusManager.ts @@ -1,11 +1,13 @@ -import {useCallback, useEffect, useMemo, useState} from 'react'; +import {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import CONST from '@src/CONST'; import useKeyboardShortcut from './useKeyboardShortcut'; import usePrevious from './usePrevious'; +type FocusedIndexChangeOptions = {shouldScroll?: boolean}; + type Config = { maxIndex: number; - onFocusedIndexChange?: (index: number) => void; + onFocusedIndexChange?: (index: number, options: Required) => void; initialFocusedIndex?: number; disabledIndexes?: readonly number[]; captureOnInputs?: boolean; @@ -20,7 +22,7 @@ type Config = { onArrowUpDownCallback?: () => void; }; -type UseArrowKeyFocusManager = [number, (index: number) => void]; +type UseArrowKeyFocusManager = [number, (index: number, options?: FocusedIndexChangeOptions) => void]; /** * A hook that makes it easy to use the arrow keys to manage focus of items in a list @@ -59,6 +61,19 @@ export default function useArrowKeyFocusManager({ }: Config): UseArrowKeyFocusManager { const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex); const prevIsFocusedIndex = usePrevious(focusedIndex); + + const shouldScrollNextChangeRef = useRef(true); + + const setFocusedIndexExternal = useCallback((index: number, {shouldScroll = true}: FocusedIndexChangeOptions = {}) => { + shouldScrollNextChangeRef.current = shouldScroll; + setFocusedIndex(index); + }, []); + + const setFocusedIndexInternal = useCallback((updater: (prev: number) => number) => { + shouldScrollNextChangeRef.current = true; + setFocusedIndex(updater); + }, []); + const arrowConfig = useMemo( () => ({ excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], @@ -81,7 +96,7 @@ export default function useArrowKeyFocusManager({ if (prevIsFocusedIndex === focusedIndex) { return; } - onFocusedIndexChange(focusedIndex); + onFocusedIndexChange(focusedIndex, {shouldScroll: shouldScrollNextChangeRef.current}); // eslint-disable-next-line react-hooks/exhaustive-deps }, [focusedIndex, prevIsFocusedIndex]); @@ -91,7 +106,7 @@ export default function useArrowKeyFocusManager({ } const nextIndex = disableCyclicTraversal ? -1 : maxIndex; setHasKeyBeenPressed?.(); - setFocusedIndex((actualIndex) => { + setFocusedIndexInternal((actualIndex) => { const currentFocusedIndex = actualIndex > 0 ? actualIndex - (itemsPerRow ?? 1) : nextIndex; let newFocusedIndex = currentFocusedIndex; @@ -114,7 +129,7 @@ export default function useArrowKeyFocusManager({ return newFocusedIndex; }); onArrowUpDownCallback(); - }, [maxIndex, isFocused, disableCyclicTraversal, itemsPerRow, disabledIndexes, allowNegativeIndexes, setHasKeyBeenPressed, onArrowUpDownCallback]); + }, [maxIndex, isFocused, disableCyclicTraversal, itemsPerRow, disabledIndexes, allowNegativeIndexes, setHasKeyBeenPressed, onArrowUpDownCallback, setFocusedIndexInternal]); useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_UP, arrowUpCallback, arrowConfig); @@ -126,7 +141,7 @@ export default function useArrowKeyFocusManager({ const nextIndex = disableCyclicTraversal ? maxIndex : 0; - setFocusedIndex((actualIndex) => { + setFocusedIndexInternal((actualIndex) => { let currentFocusedIndex = -1; if (actualIndex === -1) { @@ -161,7 +176,7 @@ export default function useArrowKeyFocusManager({ return newFocusedIndex; }); onArrowUpDownCallback(); - }, [disableCyclicTraversal, disabledIndexes, isFocused, itemsPerRow, maxIndex, setHasKeyBeenPressed, onArrowUpDownCallback]); + }, [disableCyclicTraversal, disabledIndexes, isFocused, itemsPerRow, maxIndex, setHasKeyBeenPressed, onArrowUpDownCallback, setFocusedIndexInternal]); useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, arrowDownCallback, arrowConfig); @@ -172,7 +187,7 @@ export default function useArrowKeyFocusManager({ const nextIndex = disableCyclicTraversal ? -1 : maxIndex; - setFocusedIndex((actualIndex) => { + setFocusedIndexInternal((actualIndex) => { const currentFocusedIndex = actualIndex > 0 ? actualIndex - 1 : nextIndex; let newFocusedIndex = currentFocusedIndex; @@ -187,7 +202,7 @@ export default function useArrowKeyFocusManager({ } return newFocusedIndex; }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex]); + }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex, setFocusedIndexInternal]); useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_LEFT, arrowLeftCallback, horizontalArrowConfig); @@ -198,7 +213,7 @@ export default function useArrowKeyFocusManager({ const nextIndex = disableCyclicTraversal ? maxIndex : 0; - setFocusedIndex((actualIndex) => { + setFocusedIndexInternal((actualIndex) => { const currentFocusedIndex = actualIndex < maxIndex ? actualIndex + 1 : nextIndex; let newFocusedIndex = currentFocusedIndex; @@ -213,10 +228,10 @@ export default function useArrowKeyFocusManager({ } return newFocusedIndex; }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex]); + }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex, setFocusedIndexInternal]); useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_RIGHT, arrowRightCallback, horizontalArrowConfig); - // Note: you don't need to manually manage focusedIndex in the parent. setFocusedIndex is only exposed in case you want to reset focusedIndex or focus a specific item - return [focusedIndex, setFocusedIndex]; + // Note: you don't need to manually manage focusedIndex in the parent. setFocusedIndexExternal is only exposed in case you want to reset focusedIndex or focus a specific item + return [focusedIndex, setFocusedIndexExternal]; } diff --git a/tests/unit/useArrowKeyFocusManagerTest.ts b/tests/unit/useArrowKeyFocusManagerTest.ts new file mode 100644 index 000000000000..4c3d3e6988bc --- /dev/null +++ b/tests/unit/useArrowKeyFocusManagerTest.ts @@ -0,0 +1,132 @@ +import {act, renderHook} from '@testing-library/react-native'; +import useArrowKeyFocusManager from '@hooks/useArrowKeyFocusManager'; + +type ShortcutCallback = () => void; +type ShortcutConfig = {isActive?: boolean}; + +const shortcuts: Record = {}; + +jest.mock('@hooks/useKeyboardShortcut', () => (key: {shortcutKey: string}, callback: ShortcutCallback, config?: ShortcutConfig) => { + shortcuts[key.shortcutKey] = {callback, isActive: config?.isActive ?? true}; +}); + +function pressArrowDown() { + act(() => { + if (!shortcuts.ArrowDown?.isActive) { + return; + } + shortcuts.ArrowDown.callback(); + }); +} + +describe('useArrowKeyFocusManager', () => { + afterEach(() => { + for (const key of Object.keys(shortcuts)) { + delete shortcuts[key]; + } + }); + + it('passes shouldScroll: true when setFocusedIndex is called without options (default contract)', () => { + const onFocusedIndexChange = jest.fn(); + const {result} = renderHook(() => + useArrowKeyFocusManager({ + maxIndex: 5, + initialFocusedIndex: 0, + onFocusedIndexChange, + }), + ); + + act(() => { + result.current[1](2); + }); + + expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); + expect(onFocusedIndexChange).toHaveBeenCalledWith(2, {shouldScroll: true}); + }); + + it('plumbs shouldScroll: false through to onFocusedIndexChange', () => { + const onFocusedIndexChange = jest.fn(); + const {result} = renderHook(() => + useArrowKeyFocusManager({ + maxIndex: 5, + initialFocusedIndex: 0, + onFocusedIndexChange, + }), + ); + + act(() => { + result.current[1](2, {shouldScroll: false}); + }); + + expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); + expect(onFocusedIndexChange).toHaveBeenCalledWith(2, {shouldScroll: false}); + }); + + it('arrow key navigation passes shouldScroll: true', () => { + const onFocusedIndexChange = jest.fn(); + renderHook(() => + useArrowKeyFocusManager({ + maxIndex: 5, + initialFocusedIndex: 0, + onFocusedIndexChange, + isActive: true, + }), + ); + + pressArrowDown(); + + expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); + expect(onFocusedIndexChange).toHaveBeenCalledWith(1, {shouldScroll: true}); + }); + + it('arrow keys override a previous shouldScroll: false from setFocusedIndex', () => { + const onFocusedIndexChange = jest.fn(); + const {result} = renderHook(() => + useArrowKeyFocusManager({ + maxIndex: 5, + initialFocusedIndex: 0, + onFocusedIndexChange, + isActive: true, + }), + ); + + // Public setter parks the ref at false (e.g., focus-driven update). + act(() => { + result.current[1](2, {shouldScroll: false}); + }); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(2, {shouldScroll: false}); + + // Arrow key must reassert shouldScroll: true; otherwise a focus event + // before an arrow press would silently suppress the next scroll. + pressArrowDown(); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(3, {shouldScroll: true}); + }); + + it('emits independent shouldScroll values on successive calls', () => { + const onFocusedIndexChange = jest.fn(); + const {result} = renderHook(() => + useArrowKeyFocusManager({ + maxIndex: 5, + initialFocusedIndex: 0, + onFocusedIndexChange, + }), + ); + + act(() => { + result.current[1](2, {shouldScroll: false}); + }); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(2, {shouldScroll: false}); + + act(() => { + result.current[1](3); + }); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(3, {shouldScroll: true}); + + act(() => { + result.current[1](4, {shouldScroll: false}); + }); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(4, {shouldScroll: false}); + + expect(onFocusedIndexChange).toHaveBeenCalledTimes(3); + }); +}); From 2b510ea5c4d4ef5ea03b73736f4da6b640ba9647 Mon Sep 17 00:00:00 2001 From: Sergei Sharabai <105950333+sharabai@users.noreply.github.com> Date: Mon, 25 May 2026 17:40:09 +0200 Subject: [PATCH 2/7] Update tests --- .../SelectionList/BaseSelectionList.tsx | 5 +-- tests/unit/BaseSelectionListTest.tsx | 31 ++++++++++++------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 070b37bd3fcc..c4311c128a16 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -219,10 +219,7 @@ function BaseSelectionList({ disabledIndexes: dataDetails.disabledArrowKeyIndexes, isActive: isFocused, onFocusedIndexChange: (index, {shouldScroll}) => { - if (!shouldScroll) { - return; - } - if (!shouldScrollToFocusedIndex) { + if (!shouldScroll || !shouldScrollToFocusedIndex) { return; } diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index badcdf40013d..1a4c28e90911 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -324,7 +324,7 @@ describe('BaseSelectionList', () => { expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeNull(); }); - it('suppresses the scroll on a focus-return restore but still syncs the cursor (no scroll-jump, no stale focusedIndex)', () => { + it('suppresses the scroll on a focus-return restore', () => { (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); mockIsFocusRestoreInProgress.mockReturnValue(true); render(); @@ -334,15 +334,9 @@ describe('BaseSelectionList', () => { fireEvent(row, 'focus', {nativeEvent: {sourceCapabilities: null}}); expect(mockScrollToIndex).not.toHaveBeenCalled(); - - // Cursor still moved: a later non-restore focus on another row scrolls, so focusedIndex wasn't left stale. - mockIsFocusRestoreInProgress.mockReturnValue(false); - const otherRow = screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}7`); - fireEvent(otherRow, 'focus', {nativeEvent: {sourceCapabilities: {firesTouchEvents: false}}}); - expect(mockScrollToIndex).toHaveBeenCalledWith(expect.objectContaining({index: 7})); }); - it('still syncs the cursor on genuine keyboard Tab focus (no sourceCapabilities, restore NOT in progress) — regression guard for Codex P2', () => { + it('does not auto-scroll on genuine keyboard Tab focus (programmatic focus is non-scrolling)', () => { (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); mockIsFocusRestoreInProgress.mockReturnValue(false); render(); @@ -351,10 +345,10 @@ describe('BaseSelectionList', () => { const row = screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}5`); fireEvent(row, 'focus', {nativeEvent: {sourceCapabilities: null}}); - expect(mockScrollToIndex).toHaveBeenCalledWith(expect.objectContaining({index: 5})); + expect(mockScrollToIndex).not.toHaveBeenCalled(); }); - it('still scrolls to a row on genuine pointer focus (sourceCapabilities present)', () => { + it('does not auto-scroll on genuine pointer focus (jump-on-click prevented)', () => { (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); mockIsFocusRestoreInProgress.mockReturnValue(false); render(); @@ -363,6 +357,21 @@ describe('BaseSelectionList', () => { const row = screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}5`); fireEvent(row, 'focus', {nativeEvent: {sourceCapabilities: {firesTouchEvents: false}}}); - expect(mockScrollToIndex).toHaveBeenCalledWith(expect.objectContaining({index: 5})); + expect(mockScrollToIndex).not.toHaveBeenCalled(); + }); + + it('restore-mode suppression does not leak into the next focus event', () => { + (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); + mockIsFocusRestoreInProgress.mockReturnValue(true); + render(); + + fireEvent(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}5`), 'focus', {nativeEvent: {sourceCapabilities: null}}); + + mockScrollToIndex.mockClear(); + mockIsFocusRestoreInProgress.mockReturnValue(false); + + fireEvent(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}7`), 'focus', {nativeEvent: {sourceCapabilities: {firesTouchEvents: false}}}); + + expect(mockScrollToIndex).not.toHaveBeenCalled(); }); }); From 80dfd28f3d7b026c73f266cc10b5ef472f901c56 Mon Sep 17 00:00:00 2001 From: Sergei Sharabai <105950333+sharabai@users.noreply.github.com> Date: Mon, 25 May 2026 19:51:50 +0200 Subject: [PATCH 3/7] Udate test file, remove redundant code --- .../SelectionList/BaseSelectionList.tsx | 17 +---------------- .../BaseSelectionListWithSections.tsx | 14 +------------- .../SelectionList/hooks/useSearchFocusSync.ts | 7 +------ .../SelectionList/useSearchFocusSync.test.ts | 2 +- 4 files changed, 4 insertions(+), 36 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index c4311c128a16..49aa2ef65672 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -18,7 +18,6 @@ import useSingleExecution from '@hooks/useSingleExecution'; import {focusedItemRef} from '@hooks/useSyncFocus/useSyncFocusImplementation'; import useThemeStyles from '@hooks/useThemeStyles'; import {addKeyDownPressListener, removeKeyDownPressListener} from '@libs/KeyboardShortcut/KeyDownPressListener'; -import {isFocusRestoreInProgress} from '@libs/NavigationFocusReturn'; import type {SkeletonSpanReasonAttributes} from '@libs/telemetry/useSkeletonSpan'; import CONST from '@src/CONST'; import getEmptyArray from '@src/types/utils/getEmptyArray'; @@ -233,19 +232,6 @@ function BaseSelectionList({ }, }); - // Keep the cursor on the restored row so keyboard nav continues from there, but don't scroll to it on the way back. - // Options are forwarded so onFocus callers (e.g. ListItemRenderer) keep their {shouldScroll: false} intent. - const setFocusedIndexFromRowFocus = useCallback( - (index: number, options?: {shouldScroll?: boolean}) => { - if (isFocusRestoreInProgress()) { - setFocusedIndex(index, {shouldScroll: false}); - return; - } - setFocusedIndex(index, options); - }, - [setFocusedIndex], - ); - // extraData helps FlashList detect when data changes significantly (e.g., during filtering) // Including data.length ensures FlashList resets its layout cache when the list size changes // This prevents "index out of bounds" errors when filtering reduces the list size @@ -389,7 +375,7 @@ function BaseSelectionList({ isSelected: selected, ...item, }} - setFocusedIndex={setFocusedIndexFromRowFocus} + setFocusedIndex={setFocusedIndex} index={index} isFocused={isItemFocused} isFocusVisible={isItemVisuallyFocused} @@ -559,7 +545,6 @@ function BaseSelectionList({ shouldUpdateFocusedIndex, scrollToIndex, setFocusedIndex, - focusedIndex, }); useEffect(() => { diff --git a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx index 848be2b0e69a..16c84bd78290 100644 --- a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx +++ b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx @@ -29,7 +29,6 @@ import {focusedItemRef} from '@hooks/useSyncFocus/useSyncFocusImplementation'; import useThemeStyles from '@hooks/useThemeStyles'; import {addKeyDownPressListener, removeKeyDownPressListener} from '@libs/KeyboardShortcut/KeyDownPressListener'; import Log from '@libs/Log'; -import {isFocusRestoreInProgress} from '@libs/NavigationFocusReturn'; import type {SkeletonSpanReasonAttributes} from '@libs/telemetry/useSkeletonSpan'; import CONST from '@src/CONST'; import type {FlattenedItem, ListItem, SelectionListWithSectionsProps} from './types'; @@ -168,16 +167,6 @@ function BaseSelectionListWithSections({ }, }); - // Keep the cursor on the restored row so keyboard nav continues from there, but don't scroll to it on the way back. - // Options are forwarded so onFocus callers (e.g. ListItemRenderer) keep their {shouldScroll: false} intent. - const setFocusedIndexFromRowFocus = (index: number, options?: {shouldScroll?: boolean}) => { - if (isFocusRestoreInProgress()) { - setFocusedIndex(index, {shouldScroll: false}); - return; - } - setFocusedIndex(index, options); - }; - const getFocusedItem = (): TItem | undefined => { if (focusedIndex < 0 || focusedIndex >= flattenedData.length) { return; @@ -309,7 +298,6 @@ function BaseSelectionListWithSections({ shouldUpdateFocusedIndex, scrollToIndex, setFocusedIndex, - focusedIndex, firstFocusableIndex, }); @@ -385,7 +373,7 @@ function BaseSelectionListWithSections({ shouldSingleExecuteRowSelect={shouldSingleExecuteRowSelect} onDismissError={onDismissError} rightHandSideComponent={rightHandSideComponent} - setFocusedIndex={setFocusedIndexFromRowFocus} + setFocusedIndex={setFocusedIndex} singleExecution={singleExecution} shouldSyncFocus={!isTextInputFocusedRef.current && isKeyboardNavigating} shouldIgnoreFocus={shouldIgnoreFocus} diff --git a/src/components/SelectionList/hooks/useSearchFocusSync.ts b/src/components/SelectionList/hooks/useSearchFocusSync.ts index 0dfb78e2a4bf..64c5101e8156 100644 --- a/src/components/SelectionList/hooks/useSearchFocusSync.ts +++ b/src/components/SelectionList/hooks/useSearchFocusSync.ts @@ -27,9 +27,6 @@ type UseSearchFocusSyncParams = { /** Function to set the focused index */ setFocusedIndex: (index: number, options?: {shouldScroll?: boolean}) => void; - /** The current focused index — kept for backwards-compatible parameter ordering, no longer used for scroll-suppression bookkeeping */ - focusedIndex?: number; - /** The first focusable index in the list (useful when index 0 is a header). Defaults to 0. */ firstFocusableIndex?: number; }; @@ -50,7 +47,6 @@ function useSearchFocusSync({ shouldUpdateFocusedIndex, scrollToIndex, setFocusedIndex, - focusedIndex, firstFocusableIndex = 0, }: UseSearchFocusSyncParams) { const prevSearchValue = usePrevious(searchValue); @@ -89,7 +85,7 @@ function useSearchFocusSync({ const shouldResetFocus = isSearchIdle || (selectedOptionsChanged && prevItemsLength === data.length); if (shouldResetFocus) { - setFocusedIndex(-1); + setFocusedIndex(-1, {shouldScroll: false}); return; } @@ -108,7 +104,6 @@ function useSearchFocusSync({ shouldUpdateFocusedIndex, searchValue, isItemSelected, - focusedIndex, firstFocusableIndex, ]); } diff --git a/tests/unit/components/SelectionList/useSearchFocusSync.test.ts b/tests/unit/components/SelectionList/useSearchFocusSync.test.ts index b40b53e8ce2b..d35856d91886 100644 --- a/tests/unit/components/SelectionList/useSearchFocusSync.test.ts +++ b/tests/unit/components/SelectionList/useSearchFocusSync.test.ts @@ -39,6 +39,6 @@ describe('useSearchFocusSync', () => { }); expect(scrollToIndex).toHaveBeenCalledWith(2, false); - expect(setFocusedIndex).toHaveBeenCalledWith(2); + expect(setFocusedIndex).toHaveBeenCalledWith(2, {shouldScroll: false}); }); }); From befd57becf2d0f6a9a0e3e045f81b367f045be1b Mon Sep 17 00:00:00 2001 From: Sergei Sharabai <105950333+sharabai@users.noreply.github.com> Date: Tue, 26 May 2026 16:51:28 +0200 Subject: [PATCH 4/7] refactor parameters --- .../SelectionList/BaseSelectionList.tsx | 6 +- .../ListItem/ListItemRenderer.tsx | 2 +- .../BaseSelectionListWithSections.tsx | 6 +- .../SelectionList/hooks/useSearchFocusSync.ts | 8 +- src/hooks/useArrowKeyFocusManager.ts | 89 +++++++++++-------- .../SelectionList/useSearchFocusSync.test.ts | 2 +- tests/unit/useArrowKeyFocusManagerTest.ts | 52 +++++++---- 7 files changed, 98 insertions(+), 67 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 49aa2ef65672..3debeedd0c6d 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -217,7 +217,7 @@ function BaseSelectionList({ maxIndex: data.length - 1, disabledIndexes: dataDetails.disabledArrowKeyIndexes, isActive: isFocused, - onFocusedIndexChange: (index, {shouldScroll}) => { + onFocusedIndexChange: (index, shouldScroll) => { if (!shouldScroll || !shouldScrollToFocusedIndex) { return; } @@ -254,7 +254,7 @@ function BaseSelectionList({ } } if (shouldUpdateFocusedIndex && typeof indexToFocus === 'number') { - setFocusedIndex(indexToFocus, {shouldScroll: false}); + setFocusedIndex(indexToFocus); } onSelectRow(item); @@ -519,7 +519,7 @@ function BaseSelectionList({ if (newFocusedIndex < 0 || newFocusedIndex >= data.length) { return; } - setFocusedIndex(newFocusedIndex, {shouldScroll: false}); + setFocusedIndex(newFocusedIndex); if (shouldScroll) { scrollToIndex(newFocusedIndex); } diff --git a/src/components/SelectionList/ListItem/ListItemRenderer.tsx b/src/components/SelectionList/ListItem/ListItemRenderer.tsx index e4bcce0ae57d..2a52c42b1f78 100644 --- a/src/components/SelectionList/ListItem/ListItemRenderer.tsx +++ b/src/components/SelectionList/ListItem/ListItemRenderer.tsx @@ -103,7 +103,7 @@ function ListItemRenderer({ if (isFocused) { return; } - setFocusedIndex(normalizedIndex ?? index, {shouldScroll: false}); + setFocusedIndex(normalizedIndex ?? index); }} shouldSyncFocus={shouldSyncFocus} wrapperStyle={wrapperStyle} diff --git a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx index 16c84bd78290..5d1ad478bae5 100644 --- a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx +++ b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx @@ -149,7 +149,7 @@ function BaseSelectionListWithSections({ maxIndex: flattenedData.length - 1, disabledIndexes, isActive: isScreenFocused && itemsCount > 0, - onFocusedIndexChange: (index, {shouldScroll}) => { + onFocusedIndexChange: (index, shouldScroll) => { if (!shouldScroll) { return; } @@ -192,7 +192,7 @@ function BaseSelectionListWithSections({ } } if (shouldUpdateFocusedIndex && typeof indexToFocus === 'number') { - setFocusedIndex(indexToFocus, {shouldScroll: false}); + setFocusedIndex(indexToFocus); } onSelectRow(item); @@ -218,7 +218,7 @@ function BaseSelectionListWithSections({ }; const updateAndScrollToFocusedIndex = (index: number, shouldScroll = true) => { - setFocusedIndex(index, {shouldScroll: false}); + setFocusedIndex(index); if (shouldScroll) { scrollToIndex(index); } diff --git a/src/components/SelectionList/hooks/useSearchFocusSync.ts b/src/components/SelectionList/hooks/useSearchFocusSync.ts index 64c5101e8156..5cd8b1162cf1 100644 --- a/src/components/SelectionList/hooks/useSearchFocusSync.ts +++ b/src/components/SelectionList/hooks/useSearchFocusSync.ts @@ -25,7 +25,7 @@ type UseSearchFocusSyncParams = { scrollToIndex: (index: number, animated?: boolean) => void; /** Function to set the focused index */ - setFocusedIndex: (index: number, options?: {shouldScroll?: boolean}) => void; + setFocusedIndex: (index: number) => void; /** The first focusable index in the list (useful when index 0 is a header). Defaults to 0. */ firstFocusableIndex?: number; @@ -72,7 +72,7 @@ function useSearchFocusSync({ if (foundSelectedItemIndex !== -1 && !canSelectMultiple) { scrollToIndex(foundSelectedItemIndex, false); - setFocusedIndex(foundSelectedItemIndex, {shouldScroll: false}); + setFocusedIndex(foundSelectedItemIndex); return; } } @@ -85,13 +85,13 @@ function useSearchFocusSync({ const shouldResetFocus = isSearchIdle || (selectedOptionsChanged && prevItemsLength === data.length); if (shouldResetFocus) { - setFocusedIndex(-1, {shouldScroll: false}); + setFocusedIndex(-1); return; } // Scroll to top of list and focus on first focusable item (not header) scrollToIndex(0, false); - setFocusedIndex(firstFocusableIndex, {shouldScroll: false}); + setFocusedIndex(firstFocusableIndex); }, [ canSelectMultiple, data, diff --git a/src/hooks/useArrowKeyFocusManager.ts b/src/hooks/useArrowKeyFocusManager.ts index 65684c743cf6..8fa9144c586d 100644 --- a/src/hooks/useArrowKeyFocusManager.ts +++ b/src/hooks/useArrowKeyFocusManager.ts @@ -1,13 +1,24 @@ -import {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import {useEffect, useRef, useState} from 'react'; import CONST from '@src/CONST'; import useKeyboardShortcut from './useKeyboardShortcut'; import usePrevious from './usePrevious'; -type FocusedIndexChangeOptions = {shouldScroll?: boolean}; - type Config = { maxIndex: number; - onFocusedIndexChange?: (index: number, options: Required) => void; + /** + * Optional callback fired whenever `focusedIndex` changes (from arrow keys or the + * setter returned by this hook). The hook itself never scrolls — this callback is + * where any scroll reaction must live. + * + * `shouldScroll` is a forwarded hint, NOT something the hook acts on: + * - Internal arrow-key navigation always passes `true`. + * - The setter returned by this hook forwards its optional + * `shouldScrollOnFocusedIndexChange` arg (default `false`) here. + * + * If you don't wire this callback, the second arg on the setter has no observable + * effect — consumers can simply call `setFocusedIndex(index)`. + */ + onFocusedIndexChange?: (index: number, shouldScroll: boolean) => void; initialFocusedIndex?: number; disabledIndexes?: readonly number[]; captureOnInputs?: boolean; @@ -22,7 +33,15 @@ type Config = { onArrowUpDownCallback?: () => void; }; -type UseArrowKeyFocusManager = [number, (index: number, options?: FocusedIndexChangeOptions) => void]; +/** + * Tuple of [focusedIndex, setFocusedIndex]. + * + * The setter's optional `shouldScrollOnFocusedIndexChange` flag (default `false`) is NOT + * a direct scroll control — it is forwarded as the `shouldScroll` argument to + * `onFocusedIndexChange`. Consumers without `onFocusedIndexChange` can ignore it and + * just call `setFocusedIndex(index)`. + */ +type UseArrowKeyFocusManager = [number, (index: number, shouldScrollOnFocusedIndexChange?: boolean) => void]; /** * A hook that makes it easy to use the arrow keys to manage focus of items in a list @@ -62,45 +81,39 @@ export default function useArrowKeyFocusManager({ const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex); const prevIsFocusedIndex = usePrevious(focusedIndex); - const shouldScrollNextChangeRef = useRef(true); + const shouldScrollNextChangeRef = useRef(false); - const setFocusedIndexExternal = useCallback((index: number, {shouldScroll = true}: FocusedIndexChangeOptions = {}) => { - shouldScrollNextChangeRef.current = shouldScroll; + const setFocusedIndexExternal = (index: number, shouldScrollOnFocusedIndexChange = false) => { + shouldScrollNextChangeRef.current = shouldScrollOnFocusedIndexChange; setFocusedIndex(index); - }, []); + }; - const setFocusedIndexInternal = useCallback((updater: (prev: number) => number) => { + const setFocusedIndexInternal = (updater: (prev: number) => number) => { shouldScrollNextChangeRef.current = true; setFocusedIndex(updater); - }, []); - - const arrowConfig = useMemo( - () => ({ - excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], - isActive, - captureOnInputs, - }), - [captureOnInputs, isActive, shouldExcludeTextAreaNodes], - ); - - const horizontalArrowConfig = useMemo( - () => ({ - excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], - isActive: isActive && allowHorizontalArrowKeys, - captureOnInputs, - }), - [allowHorizontalArrowKeys, captureOnInputs, isActive, shouldExcludeTextAreaNodes], - ); + }; + + const arrowConfig = { + excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], + isActive, + captureOnInputs, + }; + + const horizontalArrowConfig = { + excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], + isActive: isActive && allowHorizontalArrowKeys, + captureOnInputs, + }; useEffect(() => { if (prevIsFocusedIndex === focusedIndex) { return; } - onFocusedIndexChange(focusedIndex, {shouldScroll: shouldScrollNextChangeRef.current}); + onFocusedIndexChange(focusedIndex, shouldScrollNextChangeRef.current); // eslint-disable-next-line react-hooks/exhaustive-deps }, [focusedIndex, prevIsFocusedIndex]); - const arrowUpCallback = useCallback(() => { + const arrowUpCallback = () => { if (maxIndex < 0 || !isFocused) { return; } @@ -129,11 +142,11 @@ export default function useArrowKeyFocusManager({ return newFocusedIndex; }); onArrowUpDownCallback(); - }, [maxIndex, isFocused, disableCyclicTraversal, itemsPerRow, disabledIndexes, allowNegativeIndexes, setHasKeyBeenPressed, onArrowUpDownCallback, setFocusedIndexInternal]); + }; useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_UP, arrowUpCallback, arrowConfig); - const arrowDownCallback = useCallback(() => { + const arrowDownCallback = () => { if (maxIndex < 0 || !isFocused) { return; } @@ -176,11 +189,11 @@ export default function useArrowKeyFocusManager({ return newFocusedIndex; }); onArrowUpDownCallback(); - }, [disableCyclicTraversal, disabledIndexes, isFocused, itemsPerRow, maxIndex, setHasKeyBeenPressed, onArrowUpDownCallback, setFocusedIndexInternal]); + }; useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, arrowDownCallback, arrowConfig); - const arrowLeftCallback = useCallback(() => { + const arrowLeftCallback = () => { if (maxIndex < 0 || !allowHorizontalArrowKeys) { return; } @@ -202,11 +215,11 @@ export default function useArrowKeyFocusManager({ } return newFocusedIndex; }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex, setFocusedIndexInternal]); + }; useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_LEFT, arrowLeftCallback, horizontalArrowConfig); - const arrowRightCallback = useCallback(() => { + const arrowRightCallback = () => { if (maxIndex < 0 || !allowHorizontalArrowKeys) { return; } @@ -228,7 +241,7 @@ export default function useArrowKeyFocusManager({ } return newFocusedIndex; }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex, setFocusedIndexInternal]); + }; useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_RIGHT, arrowRightCallback, horizontalArrowConfig); diff --git a/tests/unit/components/SelectionList/useSearchFocusSync.test.ts b/tests/unit/components/SelectionList/useSearchFocusSync.test.ts index d35856d91886..b40b53e8ce2b 100644 --- a/tests/unit/components/SelectionList/useSearchFocusSync.test.ts +++ b/tests/unit/components/SelectionList/useSearchFocusSync.test.ts @@ -39,6 +39,6 @@ describe('useSearchFocusSync', () => { }); expect(scrollToIndex).toHaveBeenCalledWith(2, false); - expect(setFocusedIndex).toHaveBeenCalledWith(2, {shouldScroll: false}); + expect(setFocusedIndex).toHaveBeenCalledWith(2); }); }); diff --git a/tests/unit/useArrowKeyFocusManagerTest.ts b/tests/unit/useArrowKeyFocusManagerTest.ts index 4c3d3e6988bc..5d702208fa15 100644 --- a/tests/unit/useArrowKeyFocusManagerTest.ts +++ b/tests/unit/useArrowKeyFocusManagerTest.ts @@ -26,7 +26,7 @@ describe('useArrowKeyFocusManager', () => { } }); - it('passes shouldScroll: true when setFocusedIndex is called without options (default contract)', () => { + it('defaults shouldScroll to false when the setter is called without the optional flag', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -41,10 +41,10 @@ describe('useArrowKeyFocusManager', () => { }); expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); - expect(onFocusedIndexChange).toHaveBeenCalledWith(2, {shouldScroll: true}); + expect(onFocusedIndexChange).toHaveBeenCalledWith(2, false); }); - it('plumbs shouldScroll: false through to onFocusedIndexChange', () => { + it('forwards shouldScroll: true through to onFocusedIndexChange', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -55,14 +55,32 @@ describe('useArrowKeyFocusManager', () => { ); act(() => { - result.current[1](2, {shouldScroll: false}); + result.current[1](2, true); }); expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); - expect(onFocusedIndexChange).toHaveBeenCalledWith(2, {shouldScroll: false}); + expect(onFocusedIndexChange).toHaveBeenCalledWith(2, true); }); - it('arrow key navigation passes shouldScroll: true', () => { + it('forwards shouldScroll: false through to onFocusedIndexChange', () => { + const onFocusedIndexChange = jest.fn(); + const {result} = renderHook(() => + useArrowKeyFocusManager({ + maxIndex: 5, + initialFocusedIndex: 0, + onFocusedIndexChange, + }), + ); + + act(() => { + result.current[1](2, false); + }); + + expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); + expect(onFocusedIndexChange).toHaveBeenCalledWith(2, false); + }); + + it('arrow key navigation forwards shouldScroll: true', () => { const onFocusedIndexChange = jest.fn(); renderHook(() => useArrowKeyFocusManager({ @@ -76,7 +94,7 @@ describe('useArrowKeyFocusManager', () => { pressArrowDown(); expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); - expect(onFocusedIndexChange).toHaveBeenCalledWith(1, {shouldScroll: true}); + expect(onFocusedIndexChange).toHaveBeenCalledWith(1, true); }); it('arrow keys override a previous shouldScroll: false from setFocusedIndex', () => { @@ -92,17 +110,17 @@ describe('useArrowKeyFocusManager', () => { // Public setter parks the ref at false (e.g., focus-driven update). act(() => { - result.current[1](2, {shouldScroll: false}); + result.current[1](2, false); }); - expect(onFocusedIndexChange).toHaveBeenLastCalledWith(2, {shouldScroll: false}); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(2, false); // Arrow key must reassert shouldScroll: true; otherwise a focus event // before an arrow press would silently suppress the next scroll. pressArrowDown(); - expect(onFocusedIndexChange).toHaveBeenLastCalledWith(3, {shouldScroll: true}); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(3, true); }); - it('emits independent shouldScroll values on successive calls', () => { + it('forwards independent shouldScroll values on successive calls', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -113,19 +131,19 @@ describe('useArrowKeyFocusManager', () => { ); act(() => { - result.current[1](2, {shouldScroll: false}); + result.current[1](2, false); }); - expect(onFocusedIndexChange).toHaveBeenLastCalledWith(2, {shouldScroll: false}); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(2, false); act(() => { - result.current[1](3); + result.current[1](3, true); }); - expect(onFocusedIndexChange).toHaveBeenLastCalledWith(3, {shouldScroll: true}); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(3, true); act(() => { - result.current[1](4, {shouldScroll: false}); + result.current[1](4, false); }); - expect(onFocusedIndexChange).toHaveBeenLastCalledWith(4, {shouldScroll: false}); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(4, false); expect(onFocusedIndexChange).toHaveBeenCalledTimes(3); }); From 6423a209de695570b5fc5b602aba18e5175ccf25 Mon Sep 17 00:00:00 2001 From: Sergei Sharabai <105950333+sharabai@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:54:57 +0200 Subject: [PATCH 5/7] Refactor to state --- .../SelectionList/BaseSelectionList.tsx | 4 +- .../ListItem/ListItemRenderer.tsx | 6 -- .../BaseSelectionListWithSections.tsx | 4 +- src/hooks/useArrowKeyFocusManager.ts | 63 +++++++++---------- tests/unit/useArrowKeyFocusManagerTest.ts | 42 ++++++++++--- 5 files changed, 67 insertions(+), 52 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 3debeedd0c6d..c2a8e093ec19 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -217,8 +217,8 @@ function BaseSelectionList({ maxIndex: data.length - 1, disabledIndexes: dataDetails.disabledArrowKeyIndexes, isActive: isFocused, - onFocusedIndexChange: (index, shouldScroll) => { - if (!shouldScroll || !shouldScrollToFocusedIndex) { + onFocusedIndexChange: (index, shouldScrollHint) => { + if (!shouldScrollHint || !shouldScrollToFocusedIndex) { return; } diff --git a/src/components/SelectionList/ListItem/ListItemRenderer.tsx b/src/components/SelectionList/ListItem/ListItemRenderer.tsx index 2a52c42b1f78..7449ba212602 100644 --- a/src/components/SelectionList/ListItem/ListItemRenderer.tsx +++ b/src/components/SelectionList/ListItem/ListItemRenderer.tsx @@ -97,12 +97,6 @@ function ListItemRenderer({ if (isMobileChrome() && event.nativeEvent && !event.nativeEvent.sourceCapabilities) { return; } - // The row is already tracked as focused by useArrowKeyFocusManager. This focus event is the synchronous - // result of useSyncFocus's layout effect calling el.focus(); calling setFocusedIndex again here would - // clobber shouldScrollNextChangeRef and suppress the arrow-key-driven scroll. See useArrowKeyFocusManager. - if (isFocused) { - return; - } setFocusedIndex(normalizedIndex ?? index); }} shouldSyncFocus={shouldSyncFocus} diff --git a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx index 5d1ad478bae5..22f5030d078a 100644 --- a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx +++ b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx @@ -149,8 +149,8 @@ function BaseSelectionListWithSections({ maxIndex: flattenedData.length - 1, disabledIndexes, isActive: isScreenFocused && itemsCount > 0, - onFocusedIndexChange: (index, shouldScroll) => { - if (!shouldScroll) { + onFocusedIndexChange: (index, shouldScrollHint) => { + if (!shouldScrollHint) { return; } if (!shouldScrollToFocusedIndex) { diff --git a/src/hooks/useArrowKeyFocusManager.ts b/src/hooks/useArrowKeyFocusManager.ts index 8fa9144c586d..8ce9a40632d3 100644 --- a/src/hooks/useArrowKeyFocusManager.ts +++ b/src/hooks/useArrowKeyFocusManager.ts @@ -1,4 +1,4 @@ -import {useEffect, useRef, useState} from 'react'; +import {useEffect, useEffectEvent, useState} from 'react'; import CONST from '@src/CONST'; import useKeyboardShortcut from './useKeyboardShortcut'; import usePrevious from './usePrevious'; @@ -6,19 +6,14 @@ import usePrevious from './usePrevious'; type Config = { maxIndex: number; /** - * Optional callback fired whenever `focusedIndex` changes (from arrow keys or the - * setter returned by this hook). The hook itself never scrolls — this callback is - * where any scroll reaction must live. - * - * `shouldScroll` is a forwarded hint, NOT something the hook acts on: - * - Internal arrow-key navigation always passes `true`. - * - The setter returned by this hook forwards its optional - * `shouldScrollOnFocusedIndexChange` arg (default `false`) here. - * - * If you don't wire this callback, the second arg on the setter has no observable - * effect — consumers can simply call `setFocusedIndex(index)`. + * Optional callback fired whenever `focusedIndex` changes. The hook itself never + * scrolls — any scroll reaction must live in this callback. `shouldScrollHint` is a + * hint forwarded from whoever wrote the new index; it is only ever set in the same + * commit that writes a new `index`, and is never updated on its own. If you need to + * scroll without changing the focused index, call the underlying list's + * `scrollToIndex` directly rather than going through this hook. */ - onFocusedIndexChange?: (index: number, shouldScroll: boolean) => void; + onFocusedIndexChange?: (index: number, shouldScrollHint: boolean) => void; initialFocusedIndex?: number; disabledIndexes?: readonly number[]; captureOnInputs?: boolean; @@ -34,19 +29,14 @@ type Config = { }; /** - * Tuple of [focusedIndex, setFocusedIndex]. - * - * The setter's optional `shouldScrollOnFocusedIndexChange` flag (default `false`) is NOT - * a direct scroll control — it is forwarded as the `shouldScroll` argument to - * `onFocusedIndexChange`. Consumers without `onFocusedIndexChange` can ignore it and - * just call `setFocusedIndex(index)`. + * The setter's optional `shouldScrollHint` flag (default `false`) is forwarded as the + * second argument to `onFocusedIndexChange`; it is not a direct scroll control. + * Same-index writes are dropped — see the comment on `setFocusedIndexExternal` for why. */ -type UseArrowKeyFocusManager = [number, (index: number, shouldScrollOnFocusedIndexChange?: boolean) => void]; +type UseArrowKeyFocusManager = [number, (index: number, shouldScrollHint?: boolean) => void]; /** - * A hook that makes it easy to use the arrow keys to manage focus of items in a list - * - * Recommendation: To ensure stability, wrap the `onFocusedIndexChange` function with the useCallback hook before using it with this hook. + * A hook that makes it easy to use the arrow keys to manage focus of items in a list. * * @param config.maxIndex – typically the number of items in your list * @param [config.onFocusedIndexChange] – optional callback to execute when focusedIndex changes @@ -78,19 +68,23 @@ export default function useArrowKeyFocusManager({ setHasKeyBeenPressed, onArrowUpDownCallback = () => {}, }: Config): UseArrowKeyFocusManager { - const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex); + const [focusState, setFocusState] = useState({index: initialFocusedIndex, shouldScrollHint: false}); + const {index: focusedIndex, shouldScrollHint} = focusState; const prevIsFocusedIndex = usePrevious(focusedIndex); - const shouldScrollNextChangeRef = useRef(false); - - const setFocusedIndexExternal = (index: number, shouldScrollOnFocusedIndexChange = false) => { - shouldScrollNextChangeRef.current = shouldScrollOnFocusedIndexChange; - setFocusedIndex(index); + /* + * Same-index bail-out example path: after an arrow-key commit, `useSyncFocus`'s + * layout effect (`src/hooks/useSyncFocus/useSyncFocusImplementation.ts`) calls + * `el.focus()`, the browser fires a synchronous `focus` event, and the `onFocus` + * handler in `src/components/SelectionList/ListItem/ListItemRenderer.tsx` re-enters + * this setter with the same index and no hint. + */ + const setFocusedIndexExternal = (index: number, shouldScrollHintArg = false) => { + setFocusState((prev) => (prev.index === index ? prev : {index, shouldScrollHint: shouldScrollHintArg})); }; const setFocusedIndexInternal = (updater: (prev: number) => number) => { - shouldScrollNextChangeRef.current = true; - setFocusedIndex(updater); + setFocusState((prev) => ({index: updater(prev.index), shouldScrollHint: true})); }; const arrowConfig = { @@ -105,13 +99,14 @@ export default function useArrowKeyFocusManager({ captureOnInputs, }; + const onFocusedIndexChangeEvent = useEffectEvent(onFocusedIndexChange); + useEffect(() => { if (prevIsFocusedIndex === focusedIndex) { return; } - onFocusedIndexChange(focusedIndex, shouldScrollNextChangeRef.current); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [focusedIndex, prevIsFocusedIndex]); + onFocusedIndexChangeEvent(focusedIndex, shouldScrollHint); + }, [focusedIndex, prevIsFocusedIndex, shouldScrollHint]); const arrowUpCallback = () => { if (maxIndex < 0 || !isFocused) { diff --git a/tests/unit/useArrowKeyFocusManagerTest.ts b/tests/unit/useArrowKeyFocusManagerTest.ts index 5d702208fa15..0be953fd8cc3 100644 --- a/tests/unit/useArrowKeyFocusManagerTest.ts +++ b/tests/unit/useArrowKeyFocusManagerTest.ts @@ -26,7 +26,7 @@ describe('useArrowKeyFocusManager', () => { } }); - it('defaults shouldScroll to false when the setter is called without the optional flag', () => { + it('defaults shouldScrollHint to false when the setter is called without the optional flag', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -44,7 +44,7 @@ describe('useArrowKeyFocusManager', () => { expect(onFocusedIndexChange).toHaveBeenCalledWith(2, false); }); - it('forwards shouldScroll: true through to onFocusedIndexChange', () => { + it('forwards shouldScrollHint: true through to onFocusedIndexChange', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -62,7 +62,7 @@ describe('useArrowKeyFocusManager', () => { expect(onFocusedIndexChange).toHaveBeenCalledWith(2, true); }); - it('forwards shouldScroll: false through to onFocusedIndexChange', () => { + it('forwards shouldScrollHint: false through to onFocusedIndexChange', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -80,7 +80,7 @@ describe('useArrowKeyFocusManager', () => { expect(onFocusedIndexChange).toHaveBeenCalledWith(2, false); }); - it('arrow key navigation forwards shouldScroll: true', () => { + it('arrow key navigation forwards shouldScrollHint: true', () => { const onFocusedIndexChange = jest.fn(); renderHook(() => useArrowKeyFocusManager({ @@ -97,7 +97,7 @@ describe('useArrowKeyFocusManager', () => { expect(onFocusedIndexChange).toHaveBeenCalledWith(1, true); }); - it('arrow keys override a previous shouldScroll: false from setFocusedIndex', () => { + it('arrow keys override a previous shouldScrollHint: false from setFocusedIndex', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -108,19 +108,19 @@ describe('useArrowKeyFocusManager', () => { }), ); - // Public setter parks the ref at false (e.g., focus-driven update). + // Public setter parks the hint at false (e.g., focus-driven update). act(() => { result.current[1](2, false); }); expect(onFocusedIndexChange).toHaveBeenLastCalledWith(2, false); - // Arrow key must reassert shouldScroll: true; otherwise a focus event + // Arrow key must reassert shouldScrollHint: true; otherwise a focus event // before an arrow press would silently suppress the next scroll. pressArrowDown(); expect(onFocusedIndexChange).toHaveBeenLastCalledWith(3, true); }); - it('forwards independent shouldScroll values on successive calls', () => { + it('forwards independent shouldScrollHint values on successive calls', () => { const onFocusedIndexChange = jest.fn(); const {result} = renderHook(() => useArrowKeyFocusManager({ @@ -147,4 +147,30 @@ describe('useArrowKeyFocusManager', () => { expect(onFocusedIndexChange).toHaveBeenCalledTimes(3); }); + + it('bails out on same-index external writes and preserves the previously-set hint', () => { + // Arrow key sets index=1 with shouldScrollHint=true. Then a focus-event-style + // external write hits with the same index and no hint. The bail-out must prevent + // a second onFocusedIndexChange invocation, so the consumer's scroll for the + // arrow-key transition is not clobbered. + const onFocusedIndexChange = jest.fn(); + const {result} = renderHook(() => + useArrowKeyFocusManager({ + maxIndex: 5, + initialFocusedIndex: 0, + onFocusedIndexChange, + isActive: true, + }), + ); + + pressArrowDown(); + expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); + expect(onFocusedIndexChange).toHaveBeenLastCalledWith(1, true); + + act(() => { + result.current[1](1, false); + }); + + expect(onFocusedIndexChange).toHaveBeenCalledTimes(1); + }); }); From aca39759fe34d33d42ae4da04a3ec4ea7633c51d Mon Sep 17 00:00:00 2001 From: Sergei Sharabai <105950333+sharabai@users.noreply.github.com> Date: Tue, 16 Jun 2026 15:16:07 +0200 Subject: [PATCH 6/7] Fix test file --- tests/unit/BaseSelectionListTest.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 1a4c28e90911..f2fd70f4baf7 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -153,14 +153,14 @@ describe('BaseSelectionList', () => { } it('should not trigger item press if screen is not focused', () => { - (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(false); + jest.mocked(NativeNavigation.useIsFocused).mockReturnValue(false); render(); fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}1`)); expect(onSelectRowMock).toHaveBeenCalledTimes(0); }); it('should handle item press correctly', () => { - (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); + jest.mocked(NativeNavigation.useIsFocused).mockReturnValue(true); render(); fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}1`)); @@ -172,7 +172,7 @@ describe('BaseSelectionList', () => { }); it('should update selected item on rerender', () => { - (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); + jest.mocked(NativeNavigation.useIsFocused).mockReturnValue(true); const updatedMockItems = mockItems.map((item) => ({ ...item, isSelected: item.keyForList === '2', @@ -325,7 +325,7 @@ describe('BaseSelectionList', () => { }); it('suppresses the scroll on a focus-return restore', () => { - (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); + jest.mocked(NativeNavigation.useIsFocused).mockReturnValue(true); mockIsFocusRestoreInProgress.mockReturnValue(true); render(); mockScrollToIndex.mockClear(); @@ -337,7 +337,7 @@ describe('BaseSelectionList', () => { }); it('does not auto-scroll on genuine keyboard Tab focus (programmatic focus is non-scrolling)', () => { - (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); + jest.mocked(NativeNavigation.useIsFocused).mockReturnValue(true); mockIsFocusRestoreInProgress.mockReturnValue(false); render(); mockScrollToIndex.mockClear(); @@ -349,7 +349,7 @@ describe('BaseSelectionList', () => { }); it('does not auto-scroll on genuine pointer focus (jump-on-click prevented)', () => { - (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); + jest.mocked(NativeNavigation.useIsFocused).mockReturnValue(true); mockIsFocusRestoreInProgress.mockReturnValue(false); render(); mockScrollToIndex.mockClear(); @@ -361,7 +361,7 @@ describe('BaseSelectionList', () => { }); it('restore-mode suppression does not leak into the next focus event', () => { - (NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true); + jest.mocked(NativeNavigation.useIsFocused).mockReturnValue(true); mockIsFocusRestoreInProgress.mockReturnValue(true); render(); From a28d510fa15ebfac50f8f234adde49eac8664f5b Mon Sep 17 00:00:00 2001 From: Sergei Sharabai <105950333+sharabai@users.noreply.github.com> Date: Tue, 16 Jun 2026 15:40:47 +0200 Subject: [PATCH 7/7] Fix regression --- .../hooks/useSelectedItemFocusSync.ts | 4 +- tests/unit/useSelectedItemFocusSyncTest.ts | 72 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 tests/unit/useSelectedItemFocusSyncTest.ts diff --git a/src/components/SelectionList/hooks/useSelectedItemFocusSync.ts b/src/components/SelectionList/hooks/useSelectedItemFocusSync.ts index 4eb472a9b0c6..1f7e7cc70775 100644 --- a/src/components/SelectionList/hooks/useSelectedItemFocusSync.ts +++ b/src/components/SelectionList/hooks/useSelectedItemFocusSync.ts @@ -18,7 +18,7 @@ type UseSelectedItemFocusSyncParams = { searchValue: string | undefined; /** Function to set the focused index */ - setFocusedIndex: (index: number) => void; + setFocusedIndex: (index: number, shouldScrollHint?: boolean) => void; }; /** @@ -39,7 +39,7 @@ function useSelectedItemFocusSync({ if (selectedItemIndex === -1 || selectedItemIndex === focusedIndex || searchValue) { return; } - setFocusedIndex(selectedItemIndex); + setFocusedIndex(selectedItemIndex, true); // Only sync focus when selectedItemIndex changes, not when other dependencies update // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/tests/unit/useSelectedItemFocusSyncTest.ts b/tests/unit/useSelectedItemFocusSyncTest.ts new file mode 100644 index 000000000000..c8a31a9f168a --- /dev/null +++ b/tests/unit/useSelectedItemFocusSyncTest.ts @@ -0,0 +1,72 @@ +import {renderHook} from '@testing-library/react-native'; +import useSelectedItemFocusSync from '@components/SelectionList/hooks/useSelectedItemFocusSync'; + +type Item = {keyForList: string; text: string; isSelected: boolean}; + +const isItemSelected = (item: Item) => item.isSelected; + +const buildItems = (length: number, selectedIndex: number): Item[] => + Array.from({length}, (_, i) => ({ + keyForList: `item-${i}`, + text: `Item ${i}`, + isSelected: i === selectedIndex, + })); + +describe('useSelectedItemFocusSync', () => { + it('passes shouldScrollHint: true when the selected item becomes known after mount', () => { + const setFocusedIndex = jest.fn(); + // Mount with the selected key known but data not yet loaded. + // The memo returns -1, so the effect bails on first render. + const initialProps = { + data: [] as Item[], + initiallyFocusedItemKey: 'item-5' as string | undefined, + isItemSelected, + focusedIndex: 0, + searchValue: undefined as string | undefined, + setFocusedIndex, + }; + const {rerender} = renderHook((props: typeof initialProps) => useSelectedItemFocusSync(props), {initialProps}); + + expect(setFocusedIndex).not.toHaveBeenCalled(); + + // Async data arrives: the selected row is now at index 5. + // Regression guard for PR #91605: the sync path must pass shouldScrollHint: true + // so the consumer scrolls — FlashList's initialScrollIndex has already been consumed. + rerender({...initialProps, data: buildItems(10, 5)}); + + expect(setFocusedIndex).toHaveBeenCalledTimes(1); + expect(setFocusedIndex).toHaveBeenCalledWith(5, true); + }); + + it('does not call setFocusedIndex when the cursor is already on the selected item', () => { + const setFocusedIndex = jest.fn(); + renderHook(() => + useSelectedItemFocusSync({ + data: buildItems(10, 3), + initiallyFocusedItemKey: 'item-3', + isItemSelected, + focusedIndex: 3, + searchValue: undefined, + setFocusedIndex, + }), + ); + + expect(setFocusedIndex).not.toHaveBeenCalled(); + }); + + it('does not call setFocusedIndex while a search is active', () => { + const setFocusedIndex = jest.fn(); + renderHook(() => + useSelectedItemFocusSync({ + data: buildItems(10, 5), + initiallyFocusedItemKey: 'item-5', + isItemSelected, + focusedIndex: 0, + searchValue: 'item', + setFocusedIndex, + }), + ); + + expect(setFocusedIndex).not.toHaveBeenCalled(); + }); +});