Skip to content
Merged
5 changes: 5 additions & 0 deletions src/components/Search/SearchList/BaseSearchList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useStableIndexedHandler from '@hooks/useStableIndexedHandler';
import {isMobileChrome} from '@libs/Browser';
import {addKeyDownPressListener, removeKeyDownPressListener} from '@libs/KeyboardShortcut/KeyDownPressListener';
import {isFocusRestoreInProgress} from '@libs/NavigationFocusReturn';
import CONST from '@src/CONST';
import type BaseSearchListProps from './types';

Expand Down Expand Up @@ -63,6 +64,10 @@ function BaseSearchList({
});

const handleFocusByIndex = (index: number, event: NativeSyntheticEvent<ExtendedTargetedEvent>) => {
// The focus-return restore shouldn't move the keyboard cursor here, or the row gets highlighted and scrolled into view on back nav. The .focus() still restores DOM focus for screen readers.
if (isFocusRestoreInProgress()) {
return;
}
// Prevent unexpected scrolling on mobile Chrome after the context menu closes by ignoring programmatic focus not triggered by direct user interaction.
if (isMobileChrome() && event.nativeEvent) {
if (!event.nativeEvent.sourceCapabilities) {
Expand Down
14 changes: 13 additions & 1 deletion src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ 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';
Expand Down Expand Up @@ -227,6 +228,17 @@ function BaseSelectionList<TItem extends ListItem>({
onArrowUpDownCallback,
});

// 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 = useCallback(
(index: number) => {
if (isFocusRestoreInProgress() && index !== focusedIndex) {
suppressNextFocusScrollRef.current = true;
}
setFocusedIndex(index);
},
[focusedIndex, 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
Expand Down Expand Up @@ -373,7 +385,7 @@ function BaseSelectionList<TItem extends ListItem>({
isSelected: selected,
...item,
}}
setFocusedIndex={setFocusedIndex}
setFocusedIndex={setFocusedIndexFromRowFocus}
index={index}
isFocused={isItemFocused}
isFocusVisible={isItemVisuallyFocused}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ 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';
Expand Down Expand Up @@ -165,6 +166,23 @@ function BaseSelectionListWithSections<TItem extends ListItem>({
onArrowUpDownCallback: () => setShouldDisableHoverStyle(true),
});

// 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) => {
if (isFocusRestoreInProgress()) {
setFocusedIndexWithoutScrollOnChange(index);
} else {
setFocusedIndex(index);
}
};

const getFocusedItem = (): TItem | undefined => {
if (focusedIndex < 0 || focusedIndex >= flattenedData.length) {
return;
Expand All @@ -190,10 +208,7 @@ function BaseSelectionListWithSections<TItem extends ListItem>({
}
}
if (shouldUpdateFocusedIndex && typeof indexToFocus === 'number') {
if (indexToFocus !== focusedIndex) {
suppressNextFocusScrollRef.current = true;
}
setFocusedIndex(indexToFocus);
setFocusedIndexWithoutScrollOnChange(indexToFocus);
}
onSelectRow(item);

Expand Down Expand Up @@ -382,7 +397,7 @@ function BaseSelectionListWithSections<TItem extends ListItem>({
shouldSingleExecuteRowSelect={shouldSingleExecuteRowSelect}
onDismissError={onDismissError}
rightHandSideComponent={rightHandSideComponent}
setFocusedIndex={setFocusedIndex}
setFocusedIndex={setFocusedIndexFromRowFocus}
singleExecution={singleExecution}
shouldSyncFocus={!isTextInputFocusedRef.current && isKeyboardNavigating}
shouldIgnoreFocus={shouldIgnoreFocus}
Expand Down
15 changes: 14 additions & 1 deletion src/libs/NavigationFocusReturn.native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,22 @@ function notifyPushParamsForward(_routeKey: string, _prevParams: unknown): void
function notifyPushParamsBackward(_routeKey: string, _targetParams: unknown): void {}
/* eslint-enable @typescript-eslint/no-unused-vars */
function cancelPendingFocusRestore(): void {}
function skipNextFocusRestore(): void {}
function isFocusRestoreInProgress(): boolean {
return false;
}
// Web-only guard; native has no DOM activeElement, so AUTO never needs to skip.
function shouldSkipAutoFocusDueToExistingFocus(): boolean {
return false;
}

export {setupNavigationFocusReturn, teardownNavigationFocusReturn, notifyPushParamsForward, notifyPushParamsBackward, cancelPendingFocusRestore, shouldSkipAutoFocusDueToExistingFocus};
export {
setupNavigationFocusReturn,
teardownNavigationFocusReturn,
notifyPushParamsForward,
notifyPushParamsBackward,
cancelPendingFocusRestore,
skipNextFocusRestore,
isFocusRestoreInProgress,
shouldSkipAutoFocusDueToExistingFocus,
};
32 changes: 30 additions & 2 deletions src/libs/NavigationFocusReturn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ function setTriggerEntry(routeKey: string, entry: TriggerEntry): void {

let prevState: NavigationState | undefined;
let pendingRestore: {cancel: () => void} | null = null;
let skipNextRestore = false;
let isRestoringFocus = false;
let focusinHandler: ((e: FocusEvent) => void) | null = null;
let mouseActivationHandler: ((e: MouseEvent) => void) | null = null;
let stateUnsubscribe: (() => void) | null = null;
Expand Down Expand Up @@ -138,6 +140,16 @@ function notifyPushParamsBackward(routeKey: string, targetParams: unknown): void
scheduleRestore(compoundParamsKey(routeKey, targetParams));
}

/** Skips the focus restore for the next back navigation. Call it before a form-submit goBack so the re-focused row doesn't eat the next Enter (which should hit the page's submit). Back and Esc don't call it, so they still restore focus. */
function skipNextFocusRestore(): void {
skipNextRestore = true;
}

/** True only while restoreTriggerForRoute is in its .focus() call. Lists use it to tell the restore apart from a real keyboard Tab, which also has no sourceCapabilities. */
function isFocusRestoreInProgress(): boolean {
return isRestoringFocus;
}

// 'retry' = in DOM but cannot accept focus now; 'gone' = detached, drop the entry.
type RestorePick = {target: HTMLElement; source: 'primary' | 'fallback'} | 'retry' | 'gone';

Expand Down Expand Up @@ -260,7 +272,12 @@ function restoreTriggerForRoute(routeKey: string): boolean {
const focusOptions: FocusOptions = {preventScroll: true, focusVisible: getHadTabNavigation()};
for (const candidate of candidates) {
const before = document.activeElement;
candidate.focus(focusOptions);
isRestoringFocus = true;
try {
candidate.focus(focusOptions);
} finally {
isRestoringFocus = false;
}
const after = document.activeElement;
if (after === candidate) {
triggerMap.delete(routeKey);
Expand Down Expand Up @@ -356,15 +373,22 @@ function handleStateChange(newState: NavigationState | undefined): void {
}

if (action.type === 'forward') {
skipNextRestore = false;
cancelPendingRestore();
captureTriggerForRoute(action.captureKey);
// Loose refs would pin detached unmounted nodes; triggerMap holds the captured copy.
lastInteractiveElement = null;
lastMouseTrigger = null;
lastMouseTriggerAt = 0;
} else if (action.type === 'backward') {
scheduleRestore(action.restoreKey);
if (skipNextRestore) {
skipNextRestore = false;
cancelPendingRestore();
} else {
scheduleRestore(action.restoreKey);
}
} else if (action.type === 'lateral') {
skipNextRestore = false;
// Stale restore would steal focus back on sibling nav.
cancelPendingRestore();
}
Expand Down Expand Up @@ -447,6 +471,7 @@ function teardownNavigationFocusReturn(): void {
lastInteractiveElement = null;
lastMouseTrigger = null;
lastMouseTriggerAt = 0;
skipNextRestore = false;
if (typeof document !== 'undefined') {
if (focusinHandler) {
document.removeEventListener('focusin', focusinHandler, true);
Expand Down Expand Up @@ -474,6 +499,7 @@ function resetForTests(): void {
lastMouseTrigger = null;
lastMouseTriggerAt = 0;
lastRestoreTarget = null;
skipNextRestore = false;
}

function setLastInteractiveElementForTests(element: HTMLElement | null): void {
Expand All @@ -496,6 +522,8 @@ export {
notifyPushParamsForward,
notifyPushParamsBackward,
cancelPendingFocusRestore,
skipNextFocusRestore,
isFocusRestoreInProgress,
compoundParamsKey,
shouldSkipAutoFocusDueToExistingFocus,
resetForTests,
Expand Down
3 changes: 3 additions & 0 deletions src/pages/iou/request/step/IOURequestStepMerchant.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import useShowNotFoundPageInIOUStep from '@hooks/useShowNotFoundPageInIOUStep';
import useThemeStyles from '@hooks/useThemeStyles';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import Navigation from '@libs/Navigation/Navigation';
import {skipNextFocusRestore} from '@libs/NavigationFocusReturn';
import {getTransactionDetails, isExpenseRequest, isPolicyExpenseChat} from '@libs/ReportUtils';
import {hasReceipt} from '@libs/TransactionUtils';
import {isInvalidMerchantValue, isValidInputLength} from '@libs/ValidationUtils';
Expand Down Expand Up @@ -82,6 +83,8 @@ function IOURequestStepMerchant({
return;
}
shouldNavigateAfterSaveRef.current = false;
// Only on the save path. The Back button (onBackButtonPress) should still restore focus.
skipNextFocusRestore();
navigateBack();
}, [isSaved, navigateBack]);

Expand Down
50 changes: 50 additions & 0 deletions tests/unit/BaseSelectionListTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import BaseSelectionList from '@components/SelectionList/BaseSelectionList';
import SingleSelectListItem from '@components/SelectionList/ListItem/SingleSelectListItem';
import type {ListItem} from '@components/SelectionList/types';
import type Navigation from '@libs/Navigation/Navigation';
import type * as NavigationFocusReturnModule from '@libs/NavigationFocusReturn';
import CONST from '@src/CONST';

// Captures scrollToIndex calls so tests can assert on scroll behaviour
Expand Down Expand Up @@ -112,12 +113,19 @@ jest.mock('@hooks/useLocalize', () =>

jest.mock('@hooks/useKeyboardShortcut', () => jest.fn());

const mockIsFocusRestoreInProgress = jest.fn<boolean, []>(() => false);
jest.mock('@libs/NavigationFocusReturn', () => ({
...jest.requireActual<typeof NavigationFocusReturnModule>('@libs/NavigationFocusReturn'),
isFocusRestoreInProgress: () => mockIsFocusRestoreInProgress(),
}));

describe('BaseSelectionList', () => {
const onSelectRowMock = jest.fn();

beforeEach(() => {
onSelectRowMock.mockClear();
mockScrollToIndex.mockClear();
mockIsFocusRestoreInProgress.mockReturnValue(false);
});

function SelectionListRenderer<TItem extends ListItem>(props: BaseSelectionListTestProps<TItem>) {
Expand Down Expand Up @@ -315,4 +323,46 @@ 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)', () => {
(NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true);
mockIsFocusRestoreInProgress.mockReturnValue(true);
render(<SelectionListRenderer data={mockItems} />);
mockScrollToIndex.mockClear();

const row = screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}5`);
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', () => {
(NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true);
mockIsFocusRestoreInProgress.mockReturnValue(false);
render(<SelectionListRenderer data={mockItems} />);
mockScrollToIndex.mockClear();

const row = screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}5`);
fireEvent(row, 'focus', {nativeEvent: {sourceCapabilities: null}});

expect(mockScrollToIndex).toHaveBeenCalledWith(expect.objectContaining({index: 5}));
});

it('still scrolls to a row on genuine pointer focus (sourceCapabilities present)', () => {
(NativeNavigation.useIsFocused as jest.Mock).mockReturnValue(true);
mockIsFocusRestoreInProgress.mockReturnValue(false);
render(<SelectionListRenderer data={mockItems} />);
mockScrollToIndex.mockClear();

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}));
});
});
Loading
Loading