From 2f6cf38f37045df334792fc33d3f573517b3b53b Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 28 Jul 2026 05:21:49 +0530 Subject: [PATCH 01/10] Fix workspace keyboard focus not returning to 3-dot trigger after Back Signed-off-by: krishna2323 --- .../FocusTrap/FocusTrapForModal/index.web.tsx | 10 +++--- .../WorkspaceRowThreeDotsMenu.tsx | 3 ++ src/components/ThreeDotsMenu/index.tsx | 33 +++++++++++++++++-- src/libs/LauncherStack.ts | 18 +++++++++- tests/unit/FocusTrapForModalTest.tsx | 23 +++++++++++-- tests/unit/NavigationFocusReturnTest.ts | 25 ++++++++++++++ 6 files changed, 103 insertions(+), 9 deletions(-) diff --git a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx index e320cafd34da..c26e853038b7 100644 --- a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx @@ -1,5 +1,5 @@ import blurActiveElement from '@libs/Accessibility/blurActiveElement'; -import {markActivePopoverLauncherDeactivated, setActivePopoverLauncher} from '@libs/LauncherStack'; +import {markActivePopoverLauncherDeactivated, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; import ReportActionComposeFocusManager from '@libs/ReportActionComposeFocusManager'; import restoreFocusWithModality from '@libs/restoreFocusWithModality'; import sharedTrapStack from '@libs/sharedTrapStack'; @@ -17,10 +17,12 @@ function FocusTrapForModal({children, active, initialFocus = false, shouldPreven active={active} focusTrapOptions={{ onActivate: () => { - // Capture for nav-back return — independent of shouldReturnFocus (which gates only focus-trap-react's same-screen return below). - const launcher = document.activeElement; + // Prefer the focused opener; if it was blurred before open (ThreeDotsMenu), fall back to LauncherStack. + const activeElement = document.activeElement; + const fromActive = activeElement instanceof HTMLElement && activeElement !== document.body ? activeElement : null; + const launcher = fromActive ?? pickLauncher(); blurActiveElement(); - if (launcher instanceof HTMLElement && launcher !== document.body) { + if (launcher && document.contains(launcher)) { cachedLauncherRef.current = launcher; setActivePopoverLauncher(launcher); } diff --git a/src/components/Tables/WorkspaceListTable/WorkspaceRowThreeDotsMenu.tsx b/src/components/Tables/WorkspaceListTable/WorkspaceRowThreeDotsMenu.tsx index c1dac29d095f..06c5e51d8ed9 100644 --- a/src/components/Tables/WorkspaceListTable/WorkspaceRowThreeDotsMenu.tsx +++ b/src/components/Tables/WorkspaceListTable/WorkspaceRowThreeDotsMenu.tsx @@ -107,12 +107,15 @@ function WorkspaceRowThreeDotsMenu({item, onDeleteWorkspace, pendingDeletePolicy menuItems.push({ icon: icons.Plus, text: translate('workspace.common.duplicateWorkspace'), + // After the popover hides so the 3-dot anchor is focused again and NavigationFocusReturn can capture it. + shouldCallAfterModalHide: true, onSelected: () => (item.policyID ? Navigation.navigate(ROUTES.WORKSPACE_DUPLICATE.getRoute(item.policyID)) : undefined), }); if (item.isEligibleToCopy) { menuItems.push({ icon: icons.Copy, text: translate('workspace.copyPolicySettings.title'), + shouldCallAfterModalHide: true, onSelected: () => { if (!item.policyID) { return; diff --git a/src/components/ThreeDotsMenu/index.tsx b/src/components/ThreeDotsMenu/index.tsx index 5245e35be926..ffafe5f360ef 100644 --- a/src/components/ThreeDotsMenu/index.tsx +++ b/src/components/ThreeDotsMenu/index.tsx @@ -16,6 +16,8 @@ import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import {isMobile} from '@libs/Browser'; +import {resolvePopoverLauncherElement, setActivePopoverLauncher} from '@libs/LauncherStack'; +import restoreFocusWithModality from '@libs/restoreFocusWithModality'; import type {AnchorPosition} from '@styles/index'; import variables from '@styles/variables'; @@ -67,6 +69,8 @@ function ThreeDotsMenu({ const [restoreFocusType, setRestoreFocusType] = useState(); const [position, setPosition] = useState(); const buttonRef = useRef(null); + // When an item uses shouldCallAfterModalHide, restore the anchor before onSelected so nav/confirm capture it. + const shouldRestoreAnchorOnHideRef = useRef(false); const {translate} = useLocalize(); const expensifyIcons = useMemoizedLazyExpensifyIcons(['ThreeDots']); const isBehindModal = modal?.willAlertModalBecomeVisible && !modal?.isPopover && !shouldOverlay; @@ -94,6 +98,13 @@ function ThreeDotsMenu({ return; } hideProductTrainingTooltip?.(); + + // Register before blur — FocusTrapForModal.onActivate only sees document.activeElement, which is + // body after this blur, so without this NavigationFocusReturn has no launcher for Back restore. + const anchor = resolvePopoverLauncherElement(buttonRef); + if (anchor) { + setActivePopoverLauncher(anchor); + } buttonRef.current?.blur(); // Dismiss the keyboard before opening the menu so the menu doesn't @@ -202,12 +213,30 @@ function ThreeDotsMenu({ setRestoreFocusType(undefined)} + onModalHide={() => { + setRestoreFocusType(undefined); + if (!shouldRestoreAnchorOnHideRef.current) { + return; + } + shouldRestoreAnchorOnHideRef.current = false; + // ComposerFocusManager often has a null saved input because we blur before open; + // put focus back on the anchor before shouldCallAfterModalHide runs onSelected. + const anchor = resolvePopoverLauncherElement(buttonRef); + if (anchor) { + restoreFocusWithModality(anchor); + } + }} isVisible={isPopupMenuVisible && !isBehindModal && isContainerFocused} anchorPosition={position ?? anchorPosition ?? {horizontal: 0, vertical: 0}} anchorAlignment={anchorAlignment} onItemSelected={(item) => { - setRestoreFocusType(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE); + if (item.shouldCallAfterModalHide) { + // Let the anchor regain focus before the deferred action (nav / confirm modal). + shouldRestoreAnchorOnHideRef.current = true; + } else { + // Navigating immediately — skip flashing focus back onto the 3-dot button. + setRestoreFocusType(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE); + } hidePopoverMenu(item); }} menuItems={menuItems} diff --git a/src/libs/LauncherStack.ts b/src/libs/LauncherStack.ts index 2de159227450..0489e3156832 100644 --- a/src/libs/LauncherStack.ts +++ b/src/libs/LauncherStack.ts @@ -2,6 +2,9 @@ * Stack of popover/modal launcher elements — the element that opened a focus trap. Top is the most recent. * pickLauncher prefers the topmost active entry, else the most recent deactivated-within-LAUNCHER_CLEAR_DELAY_MS. */ +import type {RefObject} from 'react'; +import type {View} from 'react-native'; + import {LAUNCHER_CLEAR_DELAY_MS, LAUNCHER_STACK_MAX} from './focusReturnTimings'; // deactivatedAt is set on trap close; entry lives LAUNCHER_CLEAR_DELAY_MS so deferred-nav popovers can still consume it. @@ -11,6 +14,19 @@ type LauncherEntry = {element: HTMLElement; deactivatedAt?: number}; const launcherStack: LauncherEntry[] = []; let hasWarnedAboutOverflow = false; +/** Resolve a RN View ref to its web host node for LauncherStack registration. No-op on native. */ +function resolvePopoverLauncherElement(ref: RefObject | null | undefined): HTMLElement | null { + if (typeof document === 'undefined' || !ref?.current) { + return null; + } + // On web, RN View refs are DOM nodes; instanceof avoids an unsafe cast. + const node = ref.current; + if (!(node instanceof HTMLElement) || !document.contains(node)) { + return null; + } + return node; +} + // Two passes so nested traps resolve to the outer (active) launcher, not the just-closed inner. function pickLauncher(): HTMLElement | null { if (typeof document === 'undefined') { @@ -97,4 +113,4 @@ function resetLauncherStackForTests(): void { hasWarnedAboutOverflow = false; } -export {pickLauncher, consumeLauncher, setActivePopoverLauncher, markActivePopoverLauncherDeactivated, resetLauncherStackForTests}; +export {pickLauncher, consumeLauncher, setActivePopoverLauncher, markActivePopoverLauncherDeactivated, resetLauncherStackForTests, resolvePopoverLauncherElement}; diff --git a/tests/unit/FocusTrapForModalTest.tsx b/tests/unit/FocusTrapForModalTest.tsx index 283f7399b387..de1fdc2a8c50 100644 --- a/tests/unit/FocusTrapForModalTest.tsx +++ b/tests/unit/FocusTrapForModalTest.tsx @@ -2,13 +2,14 @@ import {render} from '@testing-library/react-native'; import FocusTrapForModal from '@components/FocusTrap/FocusTrapForModal/index.web'; -import {markActivePopoverLauncherDeactivated, setActivePopoverLauncher} from '@libs/LauncherStack'; +import {markActivePopoverLauncherDeactivated, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; import React from 'react'; jest.mock('@libs/LauncherStack', () => ({ setActivePopoverLauncher: jest.fn(), markActivePopoverLauncherDeactivated: jest.fn(), + pickLauncher: jest.fn(() => null), })); let capturedOptions: {onActivate?: () => void; onPostDeactivate?: () => void} | null = null; @@ -48,6 +49,8 @@ describe('FocusTrapForModal — launcher capture', () => { capturedOptions = null; (setActivePopoverLauncher as jest.Mock).mockClear(); (markActivePopoverLauncherDeactivated as jest.Mock).mockClear(); + (pickLauncher as jest.Mock).mockReset(); + (pickLauncher as jest.Mock).mockReturnValue(null); mockRestoreFocusWithModality.mockReset(); document.body.innerHTML = ''; }); @@ -111,7 +114,7 @@ describe('FocusTrapForModal — launcher capture', () => { expect(markActivePopoverLauncherDeactivated).toHaveBeenCalledWith(launcher); }); - it('skips launcher capture when activeElement is document.body (nothing to capture)', () => { + it('skips launcher capture when activeElement is document.body and LauncherStack is empty', () => { render({null}); withActiveElement(document.body, () => { @@ -122,4 +125,20 @@ describe('FocusTrapForModal — launcher capture', () => { expect(setActivePopoverLauncher).not.toHaveBeenCalled(); expect(markActivePopoverLauncherDeactivated).not.toHaveBeenCalled(); }); + + it('falls back to pickLauncher when activeElement is document.body (ThreeDots pre-blur)', () => { + const launcher = document.createElement('button'); + document.body.appendChild(launcher); + (pickLauncher as jest.Mock).mockReturnValue(launcher); + + render({null}); + + withActiveElement(document.body, () => { + capturedOptions?.onActivate?.(); + capturedOptions?.onPostDeactivate?.(); + }); + + expect(setActivePopoverLauncher).toHaveBeenCalledWith(launcher); + expect(markActivePopoverLauncherDeactivated).toHaveBeenCalledWith(launcher); + }); }); diff --git a/tests/unit/NavigationFocusReturnTest.ts b/tests/unit/NavigationFocusReturnTest.ts index 52aee72c0617..36b35ba15ada 100644 --- a/tests/unit/NavigationFocusReturnTest.ts +++ b/tests/unit/NavigationFocusReturnTest.ts @@ -440,6 +440,31 @@ describe('captureTriggerForRoute', () => { expect(launcherSpy).toHaveBeenCalled(); }); + it('should capture a pre-blurred ThreeDots launcher when the menu item unmounts on navigate', () => { + // ThreeDotsMenu blurs the anchor before FocusTrap activates, then registers via setActivePopoverLauncher. + const launcher = document.createElement('button'); + const menuItem = document.createElement('button'); + document.body.appendChild(launcher); + document.body.appendChild(menuItem); + + setActivePopoverLauncher(launcher); + launcher.blur(); + expect(document.activeElement).toBe(document.body); + + // User activates Duplicate while the disposable menu item holds focus. + menuItem.focus(); + menuItem.dispatchEvent(new FocusEvent('focusin', {bubbles: true})); + captureTriggerForRoute('workspaces-list'); + + // Popover unmounts the menu item; only the 3-dot launcher remains. + menuItem.remove(); + markActivePopoverLauncherDeactivated(launcher); + + const launcherSpy = jest.spyOn(launcher, 'focus'); + expect(restoreTriggerForRoute('workspaces-list')).toBe(true); + expect(launcherSpy).toHaveBeenCalled(); + }); + it('should fall through to lastInteractiveElement when the launcher is gone', () => { const fallback = document.createElement('button'); document.body.appendChild(fallback); From 8160c2fc6c8fbb9e54d7f89fdcfcf7125d02c2e2 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 28 Jul 2026 05:38:19 +0530 Subject: [PATCH 02/10] Fix focus return to Overview Name/Description rows after Back Signed-off-by: krishna2323 --- src/pages/workspace/WorkspaceNamePage.tsx | 4 +++- .../workspace/WorkspaceOverviewDescriptionPage.tsx | 13 ++++++------- src/pages/workspace/WorkspaceOverviewPage.tsx | 2 ++ 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/pages/workspace/WorkspaceNamePage.tsx b/src/pages/workspace/WorkspaceNamePage.tsx index e28a3a1e5f0a..cfd42e33da49 100644 --- a/src/pages/workspace/WorkspaceNamePage.tsx +++ b/src/pages/workspace/WorkspaceNamePage.tsx @@ -5,6 +5,7 @@ import HeaderWithBackButton from '@components/HeaderWithBackButton'; import ScreenWrapper from '@components/ScreenWrapper'; import TextInput from '@components/TextInput'; +import useAutoFocusInput from '@hooks/useAutoFocusInput'; import useLocalize from '@hooks/useLocalize'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -30,6 +31,7 @@ type Props = WithPolicyProps; function WorkspaceNamePage({policy}: Props) { const styles = useThemeStyles(); const {translate} = useLocalize(); + const {inputCallbackRef} = useAutoFocusInput(); const submit = useCallback( (values: FormOnyxValues) => { @@ -97,7 +99,7 @@ function WorkspaceNamePage({policy}: Props) { accessibilityLabel={translate('workspace.common.workspaceName')} defaultValue={policy?.name} spellCheck={false} - autoFocus + ref={inputCallbackRef} /> diff --git a/src/pages/workspace/WorkspaceOverviewDescriptionPage.tsx b/src/pages/workspace/WorkspaceOverviewDescriptionPage.tsx index ac55f15a079f..802d77c2e06d 100644 --- a/src/pages/workspace/WorkspaceOverviewDescriptionPage.tsx +++ b/src/pages/workspace/WorkspaceOverviewDescriptionPage.tsx @@ -5,8 +5,8 @@ import HeaderWithBackButton from '@components/HeaderWithBackButton'; import ScreenWrapper from '@components/ScreenWrapper'; import Text from '@components/Text'; import TextInput from '@components/TextInput'; -import type {BaseTextInputRef} from '@components/TextInput/BaseTextInput/types'; +import useAutoFocusInput from '@hooks/useAutoFocusInput'; import useLocalize from '@hooks/useLocalize'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -21,7 +21,7 @@ import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import React, {useCallback, useRef, useState} from 'react'; +import React, {useCallback, useState} from 'react'; import {Keyboard, View} from 'react-native'; import type {WithPolicyProps} from './withPolicy'; @@ -34,7 +34,7 @@ type Props = WithPolicyProps; function WorkspaceOverviewDescriptionPage({policy}: Props) { const styles = useThemeStyles(); const {translate} = useLocalize(); - const isInputInitializedRef = useRef(false); + const {inputCallbackRef, inputRef} = useAutoFocusInput(true); const [description, setDescription] = useState(() => Parser.htmlToMarkdown(policy?.description ?? translate('workspace.common.defaultDescription'))); /** @@ -105,15 +105,14 @@ function WorkspaceOverviewDescriptionPage({policy}: Props) { maxAutoGrowHeight={variables.textInputAutoGrowMaxHeight} value={description} spellCheck={false} - autoFocus onChangeText={setDescription} autoGrowHeight type="markdown" - ref={(el: BaseTextInputRef | null): void => { - if (!isInputInitializedRef.current) { + ref={(el) => { + if (!inputRef.current) { updateMultilineInputRange(el); } - isInputInitializedRef.current = true; + inputCallbackRef(el); }} /> diff --git a/src/pages/workspace/WorkspaceOverviewPage.tsx b/src/pages/workspace/WorkspaceOverviewPage.tsx index 0c01d5893b6b..de8922e11430 100644 --- a/src/pages/workspace/WorkspaceOverviewPage.tsx +++ b/src/pages/workspace/WorkspaceOverviewPage.tsx @@ -567,6 +567,7 @@ function WorkspaceOverviewPage({policyDraft, policy: policyProp, route}: Workspa titleStyle={styles.workspaceTitleStyle} description={translate('workspace.common.workspaceName')} sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.OVERVIEW.NAME} + pressableTestID="workspace-overview-name-menu-item" shouldShowRightIcon={!readOnly} interactive={!readOnly} wrapperStyle={[styles.sectionMenuItemTopDescription, shouldUseNarrowLayout ? styles.mt3 : {}]} @@ -592,6 +593,7 @@ function WorkspaceOverviewPage({policyDraft, policy: policyProp, route}: Workspa title={policyDescription} description={translate('workspace.editor.descriptionInputLabel')} sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.OVERVIEW.DESCRIPTION} + pressableTestID="workspace-overview-description-menu-item" shouldShowRightIcon={!readOnly} interactive={!readOnly} wrapperStyle={styles.sectionMenuItemTopDescription} From b25fd3c0a2646b6248baba6225e8680beb37ac61 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 28 Jul 2026 19:30:17 +0530 Subject: [PATCH 03/10] Add ThreeDotsMenu unit tests for deferred focus restore handshake Signed-off-by: krishna2323 --- tests/unit/ThreeDotsMenuFocusRestoreTest.tsx | 177 +++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 tests/unit/ThreeDotsMenuFocusRestoreTest.tsx diff --git a/tests/unit/ThreeDotsMenuFocusRestoreTest.tsx b/tests/unit/ThreeDotsMenuFocusRestoreTest.tsx new file mode 100644 index 000000000000..440848f03918 --- /dev/null +++ b/tests/unit/ThreeDotsMenuFocusRestoreTest.tsx @@ -0,0 +1,177 @@ +import {act, fireEvent, render, screen} from '@testing-library/react-native'; + +import type {PopoverMenuItem, PopoverMenuProps} from '@components/PopoverMenu'; +import ThreeDotsMenu from '@components/ThreeDotsMenu'; + +import {resolvePopoverLauncherElement, setActivePopoverLauncher} from '@libs/LauncherStack'; +import restoreFocusWithModality from '@libs/restoreFocusWithModality'; + +import CONST from '@src/CONST'; + +import React from 'react'; + +const mockAnchor = document.createElement('button'); + +jest.mock('@libs/LauncherStack', () => ({ + resolvePopoverLauncherElement: jest.fn(), + setActivePopoverLauncher: jest.fn(), +})); + +jest.mock('@libs/restoreFocusWithModality', () => ({ + __esModule: true, + default: jest.fn(), +})); + +jest.mock('@hooks/useOnyx', () => ({__esModule: true, default: () => [undefined]})); + +jest.mock('@hooks/useLocalize', () => () => ({translate: (key: string) => key})); + +jest.mock('@hooks/useLazyAsset', () => ({ + useMemoizedLazyExpensifyIcons: () => new Proxy({}, {get: (_, name) => String(name)}), +})); + +jest.mock('@hooks/useTheme', () => () => ({icon: '#000', success: '#0f0'})); + +jest.mock('@hooks/useThemeStyles', () => () => ({ + touchableButtonImage: {}, + threeDotsMenuIconWidth: {}, + mh4: {}, + pv2: {}, + productTrainingTooltipWrapper: {}, +})); + +jest.mock('@hooks/useWindowDimensions', () => () => ({windowWidth: 1024, windowHeight: 768})); + +jest.mock('@hooks/usePopoverPosition', () => () => ({ + calculatePopoverPosition: jest.fn(() => Promise.resolve({horizontal: 0, vertical: 0})), +})); + +jest.mock('@libs/Browser', () => ({ + isMobile: () => false, +})); + +jest.mock('@components/Pressable/PressableWithoutFeedback', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- jest.requireActual returns an untyped module + const {Pressable} = jest.requireActual('react-native'); + const ReactActual = jest.requireActual('react'); + return ReactActual.forwardRef( + ( + {children, testID, onPress, accessibilityLabel, disabled}: {children?: React.ReactNode; testID?: string; onPress?: () => void; accessibilityLabel?: string; disabled?: boolean}, + ref: React.Ref, + ) => ( + + {children} + + ), + ); +}); + +jest.mock('@components/Icon', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- jest.requireActual returns an untyped module + const {View} = jest.requireActual('react-native'); + return () => ; +}); + +jest.mock('@components/Tooltip/PopoverAnchorTooltip', () => { + return ({children}: {children: React.ReactNode}) => children; +}); + +jest.mock('@components/Tooltip/EducationalTooltip', () => { + return ({children}: {children: React.ReactNode}) => children; +}); + +const latestPopoverProps: {current: PopoverMenuProps | null} = {current: null}; + +jest.mock('@components/PopoverMenu', () => { + return (props: PopoverMenuProps) => { + latestPopoverProps.current = props; + return null; + }; +}); + +const TRIGGER_TEST_ID = 'three-dots-trigger'; + +function renderMenu(menuItems: PopoverMenuItem[]) { + return render( + , + ); +} + +describe('ThreeDotsMenu focus restore handshake', () => { + beforeEach(() => { + latestPopoverProps.current = null; + jest.mocked(setActivePopoverLauncher).mockClear(); + jest.mocked(resolvePopoverLauncherElement).mockClear(); + jest.mocked(resolvePopoverLauncherElement).mockReturnValue(mockAnchor); + jest.mocked(restoreFocusWithModality).mockClear(); + document.body.appendChild(mockAnchor); + }); + + afterEach(() => { + mockAnchor.remove(); + }); + + it('registers the anchor into LauncherStack before opening, and restores it on hide when shouldCallAfterModalHide', () => { + renderMenu([{text: 'Duplicate', shouldCallAfterModalHide: true}]); + + fireEvent.press(screen.getByTestId(TRIGGER_TEST_ID)); + + expect(resolvePopoverLauncherElement).toHaveBeenCalled(); + expect(setActivePopoverLauncher).toHaveBeenCalledWith(mockAnchor); + expect(latestPopoverProps.current?.isVisible).toBe(true); + + const deferredItem = latestPopoverProps.current?.menuItems.at(0); + if (!deferredItem) { + throw new Error('Expected deferred menu item'); + } + expect(deferredItem.shouldCallAfterModalHide).toBe(true); + + act(() => { + latestPopoverProps.current?.onItemSelected?.(deferredItem, 0); + }); + + // Deferred path must not set PRESERVE — default restore runs so the hide callback can re-focus the anchor. + expect(latestPopoverProps.current?.restoreFocusType).toBeUndefined(); + expect(latestPopoverProps.current?.isVisible).toBe(false); + + act(() => { + latestPopoverProps.current?.onModalHide?.(); + }); + + expect(restoreFocusWithModality).toHaveBeenCalledWith(mockAnchor); + }); + + it('sets restoreFocusType to PRESERVE for immediate (non-deferred) item selection', () => { + renderMenu([{text: 'Settings'}]); + + fireEvent.press(screen.getByTestId(TRIGGER_TEST_ID)); + + const immediateItem = latestPopoverProps.current?.menuItems.at(0); + if (!immediateItem) { + throw new Error('Expected immediate menu item'); + } + + act(() => { + latestPopoverProps.current?.onItemSelected?.(immediateItem, 0); + }); + + expect(latestPopoverProps.current?.restoreFocusType).toBe(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE); + + act(() => { + latestPopoverProps.current?.onModalHide?.(); + }); + + // Non-deferred path must not flash focus back onto the 3-dot button on hide. + expect(restoreFocusWithModality).not.toHaveBeenCalled(); + }); +}); From f6e34ed7bff9c0b9c8e13c5e78625492e5a567c1 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 28 Jul 2026 19:47:28 +0530 Subject: [PATCH 04/10] Fix FocusTrapForModalTest unsafe type assertions for lint seatbelt Signed-off-by: krishna2323 --- tests/unit/FocusTrapForModalTest.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/unit/FocusTrapForModalTest.tsx b/tests/unit/FocusTrapForModalTest.tsx index de1fdc2a8c50..77a6157365a6 100644 --- a/tests/unit/FocusTrapForModalTest.tsx +++ b/tests/unit/FocusTrapForModalTest.tsx @@ -12,11 +12,13 @@ jest.mock('@libs/LauncherStack', () => ({ pickLauncher: jest.fn(() => null), })); -let capturedOptions: {onActivate?: () => void; onPostDeactivate?: () => void} | null = null; +type CapturedFocusTrapOptions = {onActivate?: () => void; onPostDeactivate?: () => void}; + +let capturedOptions: CapturedFocusTrapOptions | null = null; jest.mock('focus-trap-react', () => ({ - FocusTrap: ({focusTrapOptions, children}: {focusTrapOptions: unknown; children: React.ReactNode}) => { - capturedOptions = focusTrapOptions as typeof capturedOptions; + FocusTrap: ({focusTrapOptions, children}: {focusTrapOptions: CapturedFocusTrapOptions; children: React.ReactNode}) => { + capturedOptions = focusTrapOptions; return children; }, })); @@ -47,10 +49,10 @@ function withActiveElement(element: HTMLElement, fn: () => T): T { describe('FocusTrapForModal — launcher capture', () => { beforeEach(() => { capturedOptions = null; - (setActivePopoverLauncher as jest.Mock).mockClear(); - (markActivePopoverLauncherDeactivated as jest.Mock).mockClear(); - (pickLauncher as jest.Mock).mockReset(); - (pickLauncher as jest.Mock).mockReturnValue(null); + jest.mocked(setActivePopoverLauncher).mockClear(); + jest.mocked(markActivePopoverLauncherDeactivated).mockClear(); + jest.mocked(pickLauncher).mockReset(); + jest.mocked(pickLauncher).mockReturnValue(null); mockRestoreFocusWithModality.mockReset(); document.body.innerHTML = ''; }); @@ -129,7 +131,7 @@ describe('FocusTrapForModal — launcher capture', () => { it('falls back to pickLauncher when activeElement is document.body (ThreeDots pre-blur)', () => { const launcher = document.createElement('button'); document.body.appendChild(launcher); - (pickLauncher as jest.Mock).mockReturnValue(launcher); + jest.mocked(pickLauncher).mockReturnValue(launcher); render({null}); From bc52a320118a5b9b3bca061c68e0e45948900247 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 29 Jul 2026 02:13:53 +0530 Subject: [PATCH 05/10] Prefer registered ThreeDots launcher over nested menu focus Signed-off-by: krishna2323 --- .../FocusTrap/FocusTrapForModal/index.web.tsx | 8 ++++--- src/libs/LauncherStack.ts | 19 ++++++++++++---- tests/unit/FocusTrapForModalTest.tsx | 22 ++++++++++++++++++- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx index c26e853038b7..d73e231c46da 100644 --- a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx @@ -1,5 +1,5 @@ import blurActiveElement from '@libs/Accessibility/blurActiveElement'; -import {markActivePopoverLauncherDeactivated, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; +import {markActivePopoverLauncherDeactivated, pickActiveLauncher, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; import ReportActionComposeFocusManager from '@libs/ReportActionComposeFocusManager'; import restoreFocusWithModality from '@libs/restoreFocusWithModality'; import sharedTrapStack from '@libs/sharedTrapStack'; @@ -17,10 +17,12 @@ function FocusTrapForModal({children, active, initialFocus = false, shouldPreven active={active} focusTrapOptions={{ onActivate: () => { - // Prefer the focused opener; if it was blurred before open (ThreeDotsMenu), fall back to LauncherStack. + // Prefer a still-active registered opener (ThreeDots pre-blur) over document.activeElement. + // PopoverMenu nests Modal + content FocusTraps: after the first moves focus into the menu, + // the second must not push that ephemeral menu item onto the stack (Back would have nothing). const activeElement = document.activeElement; const fromActive = activeElement instanceof HTMLElement && activeElement !== document.body ? activeElement : null; - const launcher = fromActive ?? pickLauncher(); + const launcher = pickActiveLauncher() ?? fromActive ?? pickLauncher(); blurActiveElement(); if (launcher && document.contains(launcher)) { cachedLauncherRef.current = launcher; diff --git a/src/libs/LauncherStack.ts b/src/libs/LauncherStack.ts index 0489e3156832..c15e5e9db153 100644 --- a/src/libs/LauncherStack.ts +++ b/src/libs/LauncherStack.ts @@ -28,12 +28,10 @@ function resolvePopoverLauncherElement(ref: RefObject | null | unde } // Two passes so nested traps resolve to the outer (active) launcher, not the just-closed inner. -function pickLauncher(): HTMLElement | null { +function pickActiveLauncher(): HTMLElement | null { if (typeof document === 'undefined') { return null; } - // Monotonic — Date.now() would misbehave on clock jumps. - const now = performance.now(); for (let i = launcherStack.length - 1; i >= 0; i -= 1) { const entry = launcherStack.at(i); if (!entry) { @@ -47,6 +45,19 @@ function pickLauncher(): HTMLElement | null { return entry.element; } } + return null; +} + +function pickLauncher(): HTMLElement | null { + if (typeof document === 'undefined') { + return null; + } + const active = pickActiveLauncher(); + if (active) { + return active; + } + // Monotonic — Date.now() would misbehave on clock jumps. + const now = performance.now(); for (let i = launcherStack.length - 1; i >= 0; i -= 1) { const entry = launcherStack.at(i); if (entry?.deactivatedAt === undefined) { @@ -113,4 +124,4 @@ function resetLauncherStackForTests(): void { hasWarnedAboutOverflow = false; } -export {pickLauncher, consumeLauncher, setActivePopoverLauncher, markActivePopoverLauncherDeactivated, resetLauncherStackForTests, resolvePopoverLauncherElement}; +export {pickLauncher, pickActiveLauncher, consumeLauncher, setActivePopoverLauncher, markActivePopoverLauncherDeactivated, resetLauncherStackForTests, resolvePopoverLauncherElement}; diff --git a/tests/unit/FocusTrapForModalTest.tsx b/tests/unit/FocusTrapForModalTest.tsx index 77a6157365a6..e974fb73a0d0 100644 --- a/tests/unit/FocusTrapForModalTest.tsx +++ b/tests/unit/FocusTrapForModalTest.tsx @@ -2,7 +2,7 @@ import {render} from '@testing-library/react-native'; import FocusTrapForModal from '@components/FocusTrap/FocusTrapForModal/index.web'; -import {markActivePopoverLauncherDeactivated, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; +import {markActivePopoverLauncherDeactivated, pickActiveLauncher, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; import React from 'react'; @@ -10,6 +10,7 @@ jest.mock('@libs/LauncherStack', () => ({ setActivePopoverLauncher: jest.fn(), markActivePopoverLauncherDeactivated: jest.fn(), pickLauncher: jest.fn(() => null), + pickActiveLauncher: jest.fn(() => null), })); type CapturedFocusTrapOptions = {onActivate?: () => void; onPostDeactivate?: () => void}; @@ -53,6 +54,8 @@ describe('FocusTrapForModal — launcher capture', () => { jest.mocked(markActivePopoverLauncherDeactivated).mockClear(); jest.mocked(pickLauncher).mockReset(); jest.mocked(pickLauncher).mockReturnValue(null); + jest.mocked(pickActiveLauncher).mockReset(); + jest.mocked(pickActiveLauncher).mockReturnValue(null); mockRestoreFocusWithModality.mockReset(); document.body.innerHTML = ''; }); @@ -143,4 +146,21 @@ describe('FocusTrapForModal — launcher capture', () => { expect(setActivePopoverLauncher).toHaveBeenCalledWith(launcher); expect(markActivePopoverLauncherDeactivated).toHaveBeenCalledWith(launcher); }); + + it('keeps the registered ThreeDots trigger when a nested trap activates with a menu item focused', () => { + const trigger = document.createElement('button'); + const menuItem = document.createElement('button'); + document.body.appendChild(trigger); + document.body.appendChild(menuItem); + jest.mocked(pickActiveLauncher).mockReturnValue(trigger); + + render({null}); + + withActiveElement(menuItem, () => { + capturedOptions?.onActivate?.(); + }); + + expect(setActivePopoverLauncher).toHaveBeenCalledTimes(1); + expect(setActivePopoverLauncher).toHaveBeenCalledWith(trigger); + }); }); From 393467533cfae2cb44ac51382496a7b07d87d783 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 29 Jul 2026 02:17:34 +0530 Subject: [PATCH 06/10] Only reuse stack launcher when focus is already inside the trap Signed-off-by: krishna2323 --- .../FocusTrap/FocusTrapForModal/index.web.tsx | 19 +++++--- src/libs/resolveFocusTrapLauncher.ts | 16 +++++++ tests/unit/FocusTrapForModalTest.tsx | 17 ------- tests/unit/resolveFocusTrapLauncherTest.ts | 44 +++++++++++++++++++ 4 files changed, 74 insertions(+), 22 deletions(-) create mode 100644 src/libs/resolveFocusTrapLauncher.ts create mode 100644 tests/unit/resolveFocusTrapLauncherTest.ts diff --git a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx index d73e231c46da..3048b340be38 100644 --- a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx @@ -1,28 +1,31 @@ import blurActiveElement from '@libs/Accessibility/blurActiveElement'; import {markActivePopoverLauncherDeactivated, pickActiveLauncher, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; import ReportActionComposeFocusManager from '@libs/ReportActionComposeFocusManager'; +import resolveFocusTrapLauncher from '@libs/resolveFocusTrapLauncher'; import restoreFocusWithModality from '@libs/restoreFocusWithModality'; import sharedTrapStack from '@libs/sharedTrapStack'; import {FocusTrap} from 'focus-trap-react'; import React, {useRef} from 'react'; +import {View} from 'react-native'; import type FocusTrapForModalProps from './FocusTrapForModalProps'; function FocusTrapForModal({children, active, initialFocus = false, shouldPreventScroll = false, shouldReturnFocus = true}: FocusTrapForModalProps) { // Track this trap's own launcher so onPostDeactivate targets the right shared-stack entry. const cachedLauncherRef = useRef(null); + // Host node for this trap — used to detect nested activation after a parent trap moved focus inside. + const trapContainerRef = useRef(null); + return ( { - // Prefer a still-active registered opener (ThreeDots pre-blur) over document.activeElement. - // PopoverMenu nests Modal + content FocusTraps: after the first moves focus into the menu, - // the second must not push that ephemeral menu item onto the stack (Back would have nothing). const activeElement = document.activeElement; const fromActive = activeElement instanceof HTMLElement && activeElement !== document.body ? activeElement : null; - const launcher = pickActiveLauncher() ?? fromActive ?? pickLauncher(); + const container = trapContainerRef.current instanceof HTMLElement ? trapContainerRef.current : null; + const launcher = resolveFocusTrapLauncher(fromActive, pickActiveLauncher(), container, pickLauncher()); blurActiveElement(); if (launcher && document.contains(launcher)) { cachedLauncherRef.current = launcher; @@ -50,7 +53,13 @@ function FocusTrapForModal({children, active, initialFocus = false, shouldPreven setReturnFocus: false, }} > - {children} + + {children} + ); } diff --git a/src/libs/resolveFocusTrapLauncher.ts b/src/libs/resolveFocusTrapLauncher.ts new file mode 100644 index 000000000000..f4d76d42c351 --- /dev/null +++ b/src/libs/resolveFocusTrapLauncher.ts @@ -0,0 +1,16 @@ +/** + * Chooses which element FocusTrapForModal should register as the launcher on activate. + * Pure so nested-trap vs new-modal cases can be unit-tested without the focus-trap harness. + */ +function resolveFocusTrapLauncher(fromActive: HTMLElement | null, activeStacked: HTMLElement | null, container: HTMLElement | null, fallback: HTMLElement | null): HTMLElement | null { + // Parent trap already moved focus into this container (PopoverMenu Modal → content). + // Keep the registered outside opener — do not capture the ephemeral in-menu node. + // A newly opened nested modal still uses fromActive when the opener is outside this container. + const focusAlreadyInsideTrap = !!fromActive && !!container?.contains(fromActive); + if (focusAlreadyInsideTrap && activeStacked && fromActive !== activeStacked) { + return activeStacked; + } + return fromActive ?? fallback; +} + +export default resolveFocusTrapLauncher; diff --git a/tests/unit/FocusTrapForModalTest.tsx b/tests/unit/FocusTrapForModalTest.tsx index e974fb73a0d0..43e78cb5dbc0 100644 --- a/tests/unit/FocusTrapForModalTest.tsx +++ b/tests/unit/FocusTrapForModalTest.tsx @@ -146,21 +146,4 @@ describe('FocusTrapForModal — launcher capture', () => { expect(setActivePopoverLauncher).toHaveBeenCalledWith(launcher); expect(markActivePopoverLauncherDeactivated).toHaveBeenCalledWith(launcher); }); - - it('keeps the registered ThreeDots trigger when a nested trap activates with a menu item focused', () => { - const trigger = document.createElement('button'); - const menuItem = document.createElement('button'); - document.body.appendChild(trigger); - document.body.appendChild(menuItem); - jest.mocked(pickActiveLauncher).mockReturnValue(trigger); - - render({null}); - - withActiveElement(menuItem, () => { - capturedOptions?.onActivate?.(); - }); - - expect(setActivePopoverLauncher).toHaveBeenCalledTimes(1); - expect(setActivePopoverLauncher).toHaveBeenCalledWith(trigger); - }); }); diff --git a/tests/unit/resolveFocusTrapLauncherTest.ts b/tests/unit/resolveFocusTrapLauncherTest.ts new file mode 100644 index 000000000000..9d83f20d9f61 --- /dev/null +++ b/tests/unit/resolveFocusTrapLauncherTest.ts @@ -0,0 +1,44 @@ +import resolveFocusTrapLauncher from '../../src/libs/resolveFocusTrapLauncher'; + +describe('resolveFocusTrapLauncher', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('prefers the focused opener when the stack is empty', () => { + const opener = document.createElement('button'); + document.body.appendChild(opener); + expect(resolveFocusTrapLauncher(opener, null, document.createElement('div'), null)).toBe(opener); + }); + + it('falls back to pickLauncher when nothing is focused (ThreeDots pre-blur)', () => { + const trigger = document.createElement('button'); + document.body.appendChild(trigger); + expect(resolveFocusTrapLauncher(null, null, document.createElement('div'), trigger)).toBe(trigger); + }); + + it('keeps the registered trigger when focus is already inside the trap (nested PopoverMenu FocusTrap)', () => { + const trigger = document.createElement('button'); + const container = document.createElement('div'); + const menuItem = document.createElement('button'); + document.body.appendChild(trigger); + document.body.appendChild(container); + container.appendChild(menuItem); + + expect(resolveFocusTrapLauncher(menuItem, trigger, container, trigger)).toBe(trigger); + }); + + it('still registers a nested-modal opener that sits outside this trap container', () => { + const outerOpener = document.createElement('button'); + const innerOpener = document.createElement('button'); + const outerDialog = document.createElement('div'); + const innerTrapContainer = document.createElement('div'); + document.body.appendChild(outerOpener); + document.body.appendChild(outerDialog); + outerDialog.appendChild(innerOpener); + document.body.appendChild(innerTrapContainer); + + // Inner modal trap activates while focus is still on the button inside the outer dialog (outside the new trap). + expect(resolveFocusTrapLauncher(innerOpener, outerOpener, innerTrapContainer, outerOpener)).toBe(innerOpener); + }); +}); From cff41306b1ac3941f1fd62b3583b36fad0dca369 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 29 Jul 2026 02:31:01 +0530 Subject: [PATCH 07/10] Use display:contents host for FocusTrap launcher container Signed-off-by: krishna2323 --- src/components/FocusTrap/FocusTrapForModal/index.web.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx index 3048b340be38..b604432d8606 100644 --- a/src/components/FocusTrap/FocusTrapForModal/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForModal/index.web.tsx @@ -1,3 +1,5 @@ +import useThemeStyles from '@hooks/useThemeStyles'; + import blurActiveElement from '@libs/Accessibility/blurActiveElement'; import {markActivePopoverLauncherDeactivated, pickActiveLauncher, pickLauncher, setActivePopoverLauncher} from '@libs/LauncherStack'; import ReportActionComposeFocusManager from '@libs/ReportActionComposeFocusManager'; @@ -12,9 +14,10 @@ import {View} from 'react-native'; import type FocusTrapForModalProps from './FocusTrapForModalProps'; function FocusTrapForModal({children, active, initialFocus = false, shouldPreventScroll = false, shouldReturnFocus = true}: FocusTrapForModalProps) { + const styles = useThemeStyles(); // Track this trap's own launcher so onPostDeactivate targets the right shared-stack entry. const cachedLauncherRef = useRef(null); - // Host node for this trap — used to detect nested activation after a parent trap moved focus inside. + // Host we own (same pattern as FormElement) — dContents so it does not affect modal layout/alignment. const trapContainerRef = useRef(null); return ( @@ -55,8 +58,7 @@ function FocusTrapForModal({children, active, initialFocus = false, shouldPreven > {children} From 716191e055d71c0021a0e022a129d330a2717d31 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 29 Jul 2026 02:48:33 +0530 Subject: [PATCH 08/10] Skip ThreeDots anchor restore on Safari immediate selection path Signed-off-by: krishna2323 --- src/components/ThreeDotsMenu/index.tsx | 9 ++++--- tests/unit/FocusTrapForModalTest.tsx | 6 +++++ tests/unit/ThreeDotsMenuFocusRestoreTest.tsx | 27 ++++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/components/ThreeDotsMenu/index.tsx b/src/components/ThreeDotsMenu/index.tsx index ffafe5f360ef..0d991ed170da 100644 --- a/src/components/ThreeDotsMenu/index.tsx +++ b/src/components/ThreeDotsMenu/index.tsx @@ -15,7 +15,7 @@ import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; -import {isMobile} from '@libs/Browser'; +import {isMobile, isSafari} from '@libs/Browser'; import {resolvePopoverLauncherElement, setActivePopoverLauncher} from '@libs/LauncherStack'; import restoreFocusWithModality from '@libs/restoreFocusWithModality'; @@ -230,11 +230,14 @@ function ThreeDotsMenu({ anchorPosition={position ?? anchorPosition ?? {horizontal: 0, vertical: 0}} anchorAlignment={anchorAlignment} onItemSelected={(item) => { - if (item.shouldCallAfterModalHide) { + // Match PopoverMenu: Safari runs shouldCallAfterModalHide immediately (no defer), + // so do not arm post-hide restore — that would refocus the anchor behind the destination. + const willDeferSelection = !!item.shouldCallAfterModalHide && !isSafari(); + if (willDeferSelection) { // Let the anchor regain focus before the deferred action (nav / confirm modal). shouldRestoreAnchorOnHideRef.current = true; } else { - // Navigating immediately — skip flashing focus back onto the 3-dot button. + // Immediate selection (incl. Safari) — skip flashing focus back onto the 3-dot button. setRestoreFocusType(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE); } hidePopoverMenu(item); diff --git a/tests/unit/FocusTrapForModalTest.tsx b/tests/unit/FocusTrapForModalTest.tsx index 43e78cb5dbc0..86b2cc77ff78 100644 --- a/tests/unit/FocusTrapForModalTest.tsx +++ b/tests/unit/FocusTrapForModalTest.tsx @@ -6,6 +6,12 @@ import {markActivePopoverLauncherDeactivated, pickActiveLauncher, pickLauncher, import React from 'react'; +// useThemeStyles throws without a ThemeStylesProvider; these tests only exercise focus-trap options. +jest.mock('@hooks/useThemeStyles', () => ({ + __esModule: true, + default: () => ({dContents: {display: 'contents'}}), +})); + jest.mock('@libs/LauncherStack', () => ({ setActivePopoverLauncher: jest.fn(), markActivePopoverLauncherDeactivated: jest.fn(), diff --git a/tests/unit/ThreeDotsMenuFocusRestoreTest.tsx b/tests/unit/ThreeDotsMenuFocusRestoreTest.tsx index 440848f03918..a94c6bb1dcdb 100644 --- a/tests/unit/ThreeDotsMenuFocusRestoreTest.tsx +++ b/tests/unit/ThreeDotsMenuFocusRestoreTest.tsx @@ -46,8 +46,10 @@ jest.mock('@hooks/usePopoverPosition', () => () => ({ calculatePopoverPosition: jest.fn(() => Promise.resolve({horizontal: 0, vertical: 0})), })); +let mockIsSafari = false; jest.mock('@libs/Browser', () => ({ isMobile: () => false, + isSafari: () => mockIsSafari, })); jest.mock('@components/Pressable/PressableWithoutFeedback', () => { @@ -109,6 +111,7 @@ function renderMenu(menuItems: PopoverMenuItem[]) { describe('ThreeDotsMenu focus restore handshake', () => { beforeEach(() => { + mockIsSafari = false; latestPopoverProps.current = null; jest.mocked(setActivePopoverLauncher).mockClear(); jest.mocked(resolvePopoverLauncherElement).mockClear(); @@ -174,4 +177,28 @@ describe('ThreeDotsMenu focus restore handshake', () => { // Non-deferred path must not flash focus back onto the 3-dot button on hide. expect(restoreFocusWithModality).not.toHaveBeenCalled(); }); + + it('does not restore the anchor on hide for shouldCallAfterModalHide items in Safari (immediate path)', () => { + mockIsSafari = true; + renderMenu([{text: 'Duplicate', shouldCallAfterModalHide: true}]); + + fireEvent.press(screen.getByTestId(TRIGGER_TEST_ID)); + + const safariItem = latestPopoverProps.current?.menuItems.at(0); + if (!safariItem) { + throw new Error('Expected Safari menu item'); + } + + act(() => { + latestPopoverProps.current?.onItemSelected?.(safariItem, 0); + }); + + expect(latestPopoverProps.current?.restoreFocusType).toBe(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE); + + act(() => { + latestPopoverProps.current?.onModalHide?.(); + }); + + expect(restoreFocusWithModality).not.toHaveBeenCalled(); + }); }); From e89efd9b59c0df3d6ff90e7a9dd35c88162b7a5e Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 29 Jul 2026 02:56:20 +0530 Subject: [PATCH 09/10] Restore More-button focus after ButtonWithDropdownMenu navigation Signed-off-by: krishna2323 --- .../ButtonWithDropdownMenu/index.tsx | 61 +++++- ...ButtonWithDropdownMenuFocusRestoreTest.tsx | 199 ++++++++++++++++++ 2 files changed, 255 insertions(+), 5 deletions(-) create mode 100644 tests/unit/ButtonWithDropdownMenuFocusRestoreTest.tsx diff --git a/src/components/ButtonWithDropdownMenu/index.tsx b/src/components/ButtonWithDropdownMenu/index.tsx index 4e98d2022edd..03e4330fa482 100644 --- a/src/components/ButtonWithDropdownMenu/index.tsx +++ b/src/components/ButtonWithDropdownMenu/index.tsx @@ -1,6 +1,7 @@ import Button from '@components/ButtonComposed'; import Icon from '@components/Icon'; import InlineIcon from '@components/Icon/InlineIcon'; +import type BaseModalProps from '@components/Modal/types'; import PopoverMenu from '@components/PopoverMenu'; import Text from '@components/Text'; @@ -13,7 +14,10 @@ import useStyleUtils from '@hooks/useStyleUtils'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; +import {isSafari} from '@libs/Browser'; +import {resolvePopoverLauncherElement, setActivePopoverLauncher} from '@libs/LauncherStack'; import mergeRefs from '@libs/mergeRefs'; +import restoreFocusWithModality from '@libs/restoreFocusWithModality'; import CONST from '@src/CONST'; import type {AnchorPosition} from '@src/styles'; @@ -21,7 +25,7 @@ import type {AnchorPosition} from '@src/styles'; import type {GestureResponderEvent, StyleProp, TextStyle} from 'react-native'; import type {ValueOf} from 'type-fest'; -import React, {useEffect, useImperativeHandle, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useImperativeHandle, useRef, useState} from 'react'; import {View} from 'react-native'; import type {ButtonWithDropdownMenuProps} from './types'; @@ -102,10 +106,35 @@ function ButtonWithDropdownMenu({ref, ...props}: ButtonWithDropdownM const StyleUtils = useStyleUtils(); const [selectedItemIndex, setSelectedItemIndex] = useState(defaultSelectedIndex); const [isMenuVisible, setIsMenuVisible] = useState(false); + const [restoreFocusType, setRestoreFocusType] = useState(); + // When an item uses shouldCallAfterModalHide, restore the anchor before onSelected so nav can capture it. + const shouldRestoreAnchorOnHideRef = useRef(false); // In tests, skip the popover anchor position calculation. The default values are needed for popover menu to be rendered in tests. const defaultPopoverAnchorPosition = process.env.NODE_ENV === 'test' ? {horizontal: 100, vertical: 100} : null; const [popoverAnchorPosition, setPopoverAnchorPosition] = useState(defaultPopoverAnchorPosition); const dropdownAnchor = useRef(null); + + const registerDropdownLauncher = useCallback(() => { + // Register before open — mouse-open often leaves activeElement as body, so FocusTrap can't infer the launcher. + const anchor = resolvePopoverLauncherElement(dropdownAnchor); + if (anchor) { + setActivePopoverLauncher(anchor); + } + }, []); + + const setMenuVisible = useCallback( + (visible: boolean) => { + if (visible) { + registerDropdownLauncher(); + } + setIsMenuVisible(visible); + }, + [registerDropdownLauncher], + ); + + const toggleMenuVisible = useCallback(() => { + setMenuVisible(!isMenuVisible); + }, [isMenuVisible, setMenuVisible]); // We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to apply correct popover styles // eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth const {isSmallScreenWidth} = useResponsiveLayout(); @@ -164,7 +193,7 @@ function ButtonWithDropdownMenu({ref, ...props}: ButtonWithDropdownM (e) => { if (shouldAlwaysShowDropdownMenu || options.length) { if (!isSplitButton) { - setIsMenuVisible(!isMenuVisible); + toggleMenuVisible(); return; } if (selectedItem?.value) { @@ -193,14 +222,14 @@ function ButtonWithDropdownMenu({ref, ...props}: ButtonWithDropdownM const handlePress = (event?: GestureResponderEvent | KeyboardEvent) => { if (!isSplitButton) { - setIsMenuVisible(!isMenuVisible); + toggleMenuVisible(); } else if (selectedItem?.value) { onPress(event, selectedItem.value); } }; useImperativeHandle(ref, () => ({ - setIsMenuVisible, + setIsMenuVisible: setMenuVisible, })); const IconComponent = shouldUseShortForm ? InlineIcon : Icon; @@ -265,7 +294,7 @@ function ButtonWithDropdownMenu({ref, ...props}: ButtonWithDropdownM accessibilityState={{expanded: isMenuVisible}} stayNormalOnDisable={stayNormalOnDisable} style={[styles.pl0]} - onPress={() => setIsMenuVisible(!isMenuVisible)} + onPress={toggleMenuVisible} removeBorderRadius={CONST.BUTTON_REMOVE_BORDER_RADIUS.LEFT} size={size} innerStyles={[styles.dropDownButtonCartIconContainerPadding, innerStyleDropButton, isButtonSizeSmall && styles.dropDownButtonCartIcon]} @@ -333,8 +362,28 @@ function ButtonWithDropdownMenu({ref, ...props}: ButtonWithDropdownM onOptionsMenuHide?.(); }} onModalShow={onOptionsMenuShow} + onModalHide={() => { + setRestoreFocusType(undefined); + if (!shouldRestoreAnchorOnHideRef.current) { + return; + } + shouldRestoreAnchorOnHideRef.current = false; + // Put focus back on the anchor before shouldCallAfterModalHide runs onSelected / NavigationFocusReturn captures. + const anchor = resolvePopoverLauncherElement(dropdownAnchor); + if (anchor) { + restoreFocusWithModality(anchor); + } + }} onItemSelected={(selectedSubitem, index, event) => { onSubItemSelected?.(selectedSubitem, index, event); + // Match PopoverMenu: Safari runs shouldCallAfterModalHide immediately (no defer), + // so do not arm post-hide restore — that would refocus the anchor behind the destination. + const willDeferSelection = !!selectedSubitem.shouldCallAfterModalHide && !isSafari(); + if (willDeferSelection) { + shouldRestoreAnchorOnHideRef.current = true; + } else { + setRestoreFocusType(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE); + } if (selectedSubitem.shouldCloseModalOnSelect !== false) { setIsMenuVisible(false); } @@ -348,6 +397,8 @@ function ButtonWithDropdownMenu({ref, ...props}: ButtonWithDropdownM headerText={menuHeaderText} shouldUseScrollView={shouldPopoverUseScrollView} containerStyles={containerStyles} + shouldEnableNewFocusManagement + restoreFocusType={restoreFocusType} menuItems={options.map((item, index) => ({ ...item, onSelected: item.onSelected diff --git a/tests/unit/ButtonWithDropdownMenuFocusRestoreTest.tsx b/tests/unit/ButtonWithDropdownMenuFocusRestoreTest.tsx new file mode 100644 index 000000000000..dd73cdb3b04a --- /dev/null +++ b/tests/unit/ButtonWithDropdownMenuFocusRestoreTest.tsx @@ -0,0 +1,199 @@ +import {act, fireEvent, render, screen} from '@testing-library/react-native'; + +import ButtonWithDropdownMenu from '@components/ButtonWithDropdownMenu'; +import type {PopoverMenuProps} from '@components/PopoverMenu'; + +import {resolvePopoverLauncherElement, setActivePopoverLauncher} from '@libs/LauncherStack'; +import restoreFocusWithModality from '@libs/restoreFocusWithModality'; + +import CONST from '@src/CONST'; + +import React from 'react'; + +const mockAnchor = document.createElement('button'); + +jest.mock('@libs/LauncherStack', () => ({ + resolvePopoverLauncherElement: jest.fn(), + setActivePopoverLauncher: jest.fn(), +})); + +jest.mock('@libs/restoreFocusWithModality', () => ({ + __esModule: true, + default: jest.fn(), +})); + +jest.mock('@hooks/useTheme', () => () => ({ + icon: '#000', + buttonIcon: '#111', + buttonSuccessText: '#fff', + danger: '#f00', +})); + +jest.mock('@hooks/useThemeStyles', () => () => new Proxy({}, {get: () => ({})})); + +jest.mock('@hooks/useStyleUtils', () => () => ({ + getDropDownButtonHeight: () => ({}), +})); + +jest.mock('@hooks/useResponsiveLayout', () => () => ({isSmallScreenWidth: false})); + +jest.mock('@hooks/useSafeAreaPaddings', () => () => ({paddingBottom: 0})); + +jest.mock('@hooks/usePopoverPosition', () => () => ({ + calculatePopoverPosition: jest.fn(() => Promise.resolve({horizontal: 0, vertical: 0})), +})); + +jest.mock('@hooks/useLazyAsset', () => ({ + useMemoizedLazyExpensifyIcons: () => new Proxy({}, {get: (_, name) => String(name)}), +})); + +jest.mock('@hooks/useKeyboardShortcut', () => () => undefined); + +let mockIsSafari = false; +jest.mock('@libs/Browser', () => ({ + isSafari: () => mockIsSafari, +})); + +jest.mock('@components/ButtonComposed', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- jest.requireActual returns an untyped module + const {Pressable, Text} = jest.requireActual('react-native'); + const ReactActual = jest.requireActual('react'); + + const MockButton = ReactActual.forwardRef( + ( + { + children, + onPress, + testID, + disabled, + accessibilityState, + }: { + children?: React.ReactNode; + onPress?: () => void; + testID?: string; + disabled?: boolean; + accessibilityState?: {expanded?: boolean}; + }, + // forwardRef requires the second arg; Pressable does not need the host ref in this stub. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _ref: React.Ref, + ) => ( + + {children} + + ), + ); + + return { + __esModule: true, + default: Object.assign(MockButton, { + Text: ({children}: {children?: React.ReactNode}) => {children}, + Icon: () => null, + KeyboardShortcut: () => null, + }), + }; +}); + +jest.mock('@components/Icon', () => () => null); +jest.mock('@components/Icon/InlineIcon', () => () => null); +jest.mock('@components/Text', () => ({__esModule: true, default: () => null})); + +const latestPopoverProps: {current: PopoverMenuProps | null} = {current: null}; + +jest.mock('@components/PopoverMenu', () => { + return (props: PopoverMenuProps) => { + latestPopoverProps.current = props; + return null; + }; +}); + +const TRIGGER_TEST_ID = 'more-dropdown-trigger'; + +function renderMenu() { + return render( + {}} + shouldAlwaysShowDropdownMenu + isSplitButton={false} + customText="More" + testID={TRIGGER_TEST_ID} + options={[ + {text: 'Settings', value: 'settings', onSelected: jest.fn()}, + {text: 'Import', value: 'import', onSelected: jest.fn()}, + ]} + />, + ); +} + +describe('ButtonWithDropdownMenu focus restore handshake', () => { + beforeEach(() => { + mockIsSafari = false; + latestPopoverProps.current = null; + jest.mocked(setActivePopoverLauncher).mockClear(); + jest.mocked(resolvePopoverLauncherElement).mockClear(); + jest.mocked(resolvePopoverLauncherElement).mockReturnValue(mockAnchor); + jest.mocked(restoreFocusWithModality).mockClear(); + }); + + it('registers the dropdown launcher when opening the menu', () => { + renderMenu(); + + fireEvent.press(screen.getByTestId(TRIGGER_TEST_ID)); + + expect(setActivePopoverLauncher).toHaveBeenCalledWith(mockAnchor); + }); + + it('restores the anchor on hide for deferred shouldCallAfterModalHide selection', () => { + renderMenu(); + + fireEvent.press(screen.getByTestId(TRIGGER_TEST_ID)); + + const settingsItem = latestPopoverProps.current?.menuItems.at(0); + if (!settingsItem) { + throw new Error('Expected Settings menu item'); + } + + expect(settingsItem.shouldCallAfterModalHide).toBe(true); + expect(latestPopoverProps.current?.shouldEnableNewFocusManagement).toBe(true); + + act(() => { + latestPopoverProps.current?.onItemSelected?.(settingsItem, 0); + }); + + act(() => { + latestPopoverProps.current?.onModalHide?.(); + }); + + expect(restoreFocusWithModality).toHaveBeenCalledWith(mockAnchor); + }); + + it('does not restore the anchor on hide for shouldCallAfterModalHide items in Safari (immediate path)', () => { + mockIsSafari = true; + renderMenu(); + + fireEvent.press(screen.getByTestId(TRIGGER_TEST_ID)); + + const settingsItem = latestPopoverProps.current?.menuItems.at(0); + if (!settingsItem) { + throw new Error('Expected Safari menu item'); + } + + act(() => { + latestPopoverProps.current?.onItemSelected?.(settingsItem, 0); + }); + + expect(latestPopoverProps.current?.restoreFocusType).toBe(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE); + + act(() => { + latestPopoverProps.current?.onModalHide?.(); + }); + + expect(restoreFocusWithModality).not.toHaveBeenCalled(); + }); +}); From f2a933a80c9fc354dd146c73a611030799137a6c Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 29 Jul 2026 19:51:45 +0530 Subject: [PATCH 10/10] Add BaseModal coverage for modal-hide callback ordering Signed-off-by: krishna2323 --- tests/ui/BaseModalTest.tsx | 72 ++++++++++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/tests/ui/BaseModalTest.tsx b/tests/ui/BaseModalTest.tsx index da3a4f8ee87c..d6bcb3f18e36 100644 --- a/tests/ui/BaseModalTest.tsx +++ b/tests/ui/BaseModalTest.tsx @@ -1,34 +1,36 @@ -import {render} from '@testing-library/react-native'; +import {act, render} from '@testing-library/react-native'; +import BaseModal from '@components/Modal/BaseModal'; import type ReanimatedModalProps from '@components/Modal/ReanimatedModal/types'; +import {close} from '@userActions/Modal'; + import CONST from '@src/CONST'; import React from 'react'; +let mockCapturedProps: ReanimatedModalProps | undefined; + +jest.mock('@components/Modal/ReanimatedModal', () => ({ + __esModule: true, + default: (props: ReanimatedModalProps) => { + mockCapturedProps = props; + return null; + }, +})); + describe('BaseModal', () => { - afterEach(() => { - jest.resetModules(); + beforeEach(() => { + mockCapturedProps = undefined; }); it('passes a non-null initialFocus for a bottom-docked modal when the dismiss-button ref is unmounted', () => { // focus-trap throws when `initialFocus` resolves to `null` (vs `false`/`undefined`). For a bottom-docked // modal, the dismiss-button ref can be `null` by the time focus-trap reads it (the read is deferred), so - // the getter must coerce that `null` to `false`. The ReanimatedModal mock is scoped to this test (via - // jest.doMock) so it doesn't leak into other BaseModal cases. - let captured: ReanimatedModalProps | undefined; - jest.doMock('@components/Modal/ReanimatedModal', () => ({ - __esModule: true, - default: (props: ReanimatedModalProps) => { - captured = props; - return null; - }, - })); - const BaseModal = (require('@components/Modal/BaseModal') as {default: React.ComponentType>}).default; - + // the getter must coerce that `null` to `false`. render( @@ -36,9 +38,43 @@ describe('BaseModal', () => { , ); - const initialFocus = captured?.initialFocus; + const initialFocus = mockCapturedProps?.initialFocus; expect(typeof initialFocus).toBe('function'); + if (typeof initialFocus !== 'function') { + throw new Error('Expected initialFocus to be a function'); + } // dismiss button never mounted -> ref.current is null -> the getter resolves to false (no crash) - expect((initialFocus as () => unknown)()).toBe(false); + expect(initialFocus()).toBe(false); + }); + + it('calls onModalHide before the callback deferred by close', () => { + const events: string[] = []; + + render( + { + events.push('close'); + }} + onModalHide={() => { + events.push('modalHide'); + }} + > + {null} + , + ); + + act(() => { + close(() => { + events.push('deferred'); + }); + }); + expect(events).toEqual(['close']); + + act(() => { + mockCapturedProps?.onModalHide?.(); + }); + expect(events).toEqual(['close', 'modalHide', 'deferred']); }); });