From a1587bb0d5619b7a1a064be0dc27520ff14c6f8d Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Sat, 6 Jul 2024 14:37:23 +0700 Subject: [PATCH 01/12] prevent initial focus trap while has a focused element. --- patches/focus-trap+7.5.4.patch | 88 +++++++++++++++++++ .../FocusTrapForScreen/index.web.tsx | 10 ++- .../MoneyRequestConfirmationList.tsx | 9 +- 3 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 patches/focus-trap+7.5.4.patch diff --git a/patches/focus-trap+7.5.4.patch b/patches/focus-trap+7.5.4.patch new file mode 100644 index 000000000000..247bf26a0917 --- /dev/null +++ b/patches/focus-trap+7.5.4.patch @@ -0,0 +1,88 @@ +diff --git a/node_modules/focus-trap/dist/focus-trap.esm.js b/node_modules/focus-trap/dist/focus-trap.esm.js +index 10d56db..a0ed057 100644 +--- a/node_modules/focus-trap/dist/focus-trap.esm.js ++++ b/node_modules/focus-trap/dist/focus-trap.esm.js +@@ -100,8 +100,8 @@ var isKeyForward = function isKeyForward(e) { + var isKeyBackward = function isKeyBackward(e) { + return isTabEvent(e) && e.shiftKey; + }; +-var delay = function delay(fn) { +- return setTimeout(fn, 0); ++var delay = function delay(fn, delayTime = 0) { ++ return setTimeout(fn, delayTime); + }; + + // Array.find/findIndex() are not supported on IE; this replicates enough +@@ -283,7 +283,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + return node; + }; + var getInitialFocusNode = function getInitialFocusNode() { +- var node = getNodeForOption('initialFocus'); ++ var node = getNodeForOption('initialFocus', state.containers); + + // false explicitly indicates we want no initialFocus at all + if (node === false) { +@@ -744,7 +744,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + // that caused the focus trap activation. + state.delayInitialFocusTimer = config.delayInitialFocus ? delay(function () { + tryFocus(getInitialFocusNode()); +- }) : tryFocus(getInitialFocusNode()); ++ }, typeof config.delayInitialFocus === 'number' ? config.delayInitialFocus : undefined) : tryFocus(getInitialFocusNode()); + doc.addEventListener('focusin', checkFocusIn, true); + doc.addEventListener('mousedown', checkPointerDown, { + capture: true, +diff --git a/node_modules/focus-trap/index.d.ts b/node_modules/focus-trap/index.d.ts +index 400db1b..813f427 100644 +--- a/node_modules/focus-trap/index.d.ts ++++ b/node_modules/focus-trap/index.d.ts +@@ -118,7 +118,7 @@ declare module 'focus-trap' { + * Setting this option to `undefined` (or a function that returns `undefined`) + * will result in the default behavior. + */ +- initialFocus?: FocusTargetOrFalse | undefined | (() => void); ++ initialFocus?: (containers?: HTMLElement[]) => FocusTargetOrFalse | FocusTargetOrFalse | undefined | (() => void); + /** + * By default, an error will be thrown if the focus trap contains no + * elements in its tab order. With this option you can specify a +@@ -185,7 +185,7 @@ declare module 'focus-trap' { + * This prevents elements within the focusable element from capturing + * the event that triggered the focus trap activation. + */ +- delayInitialFocus?: boolean; ++ delayInitialFocus?: boolean | number; + /** + * Default: `window.document`. Document where the focus trap will be active. + * This allows to use FocusTrap in an iFrame context. +diff --git a/node_modules/focus-trap/index.js b/node_modules/focus-trap/index.js +index de8e46a..7ab9d89 100644 +--- a/node_modules/focus-trap/index.js ++++ b/node_modules/focus-trap/index.js +@@ -63,8 +63,8 @@ const isKeyBackward = function (e) { + return isTabEvent(e) && e.shiftKey; + }; + +-const delay = function (fn) { +- return setTimeout(fn, 0); ++const delay = function (fn, delayTime = 0) { ++ return setTimeout(fn, delayTime); + }; + + // Array.find/findIndex() are not supported on IE; this replicates enough +@@ -267,7 +267,7 @@ const createFocusTrap = function (elements, userOptions) { + }; + + const getInitialFocusNode = function () { +- let node = getNodeForOption('initialFocus'); ++ let node = getNodeForOption('initialFocus', state.containers); + + // false explicitly indicates we want no initialFocus at all + if (node === false) { +@@ -817,7 +817,7 @@ const createFocusTrap = function (elements, userOptions) { + state.delayInitialFocusTimer = config.delayInitialFocus + ? delay(function () { + tryFocus(getInitialFocusNode()); +- }) ++ }, typeof config.delayInitialFocus === 'number' ? config.delayInitialFocus : undefined) + : tryFocus(getInitialFocusNode()); + + doc.addEventListener('focusin', checkFocusIn, true); diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index d81293729b94..2933edda0b60 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -8,6 +8,7 @@ import sharedTrapStack from '@components/FocusTrap/sharedTrapStack'; import TOP_TAB_SCREENS from '@components/FocusTrap/TOP_TAB_SCREENS'; import WIDE_LAYOUT_INACTIVE_SCREENS from '@components/FocusTrap/WIDE_LAYOUT_INACTIVE_SCREENS'; import useWindowDimensions from '@hooks/useWindowDimensions'; +import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type FocusTrapProps from './FocusTrapProps'; @@ -51,12 +52,13 @@ function FocusTrapForScreen({children}: FocusTrapProps) { trapStack: sharedTrapStack, allowOutsideClick: true, fallbackFocus: document.body, - // We don't want to ovverride autofocus on these screens. - initialFocus: () => { - if (screensWithAutofocus.includes(activeRouteName)) { + delayInitialFocus: CONST.ANIMATED_TRANSITION, + // We don’t want to override autofocus while there is a focused element. + initialFocus: (containers) => { + const hasFocusedElement = containers?.some((container) => container.contains(document.activeElement)); + if (hasFocusedElement) { return false; } - return undefined; }, setReturnFocus: (element) => { if (screensWithAutofocus.includes(activeRouteName)) { diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index 7e6682492eb2..8f97701fd615 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -1,6 +1,6 @@ import {useIsFocused} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; -import React, {memo, useCallback, useEffect, useMemo, useState} from 'react'; +import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import type {TextStyle} from 'react-native'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; @@ -714,6 +714,12 @@ function MoneyRequestConfirmationList({ ], ); + const confirmButtonRef = useRef(null); + + useEffect(() => { + confirmButtonRef.current?.focus(); + }, []); + const footerContent = useMemo(() => { if (isReadOnly) { return; @@ -745,6 +751,7 @@ function MoneyRequestConfirmationList({ /> ) : ( Date: Sat, 6 Jul 2024 15:46:46 +0700 Subject: [PATCH 02/12] fix typecheck --- patches/focus-trap+7.5.4.patch | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patches/focus-trap+7.5.4.patch b/patches/focus-trap+7.5.4.patch index 247bf26a0917..3eb1e3cb8234 100644 --- a/patches/focus-trap+7.5.4.patch +++ b/patches/focus-trap+7.5.4.patch @@ -32,7 +32,7 @@ index 10d56db..a0ed057 100644 doc.addEventListener('mousedown', checkPointerDown, { capture: true, diff --git a/node_modules/focus-trap/index.d.ts b/node_modules/focus-trap/index.d.ts -index 400db1b..813f427 100644 +index 400db1b..e6d1bc3 100644 --- a/node_modules/focus-trap/index.d.ts +++ b/node_modules/focus-trap/index.d.ts @@ -118,7 +118,7 @@ declare module 'focus-trap' { @@ -40,7 +40,7 @@ index 400db1b..813f427 100644 * will result in the default behavior. */ - initialFocus?: FocusTargetOrFalse | undefined | (() => void); -+ initialFocus?: (containers?: HTMLElement[]) => FocusTargetOrFalse | FocusTargetOrFalse | undefined | (() => void); ++ initialFocus?: FocusTargetValueOrFalse | ((containers?: HTMLElement[]) => FocusTargetValueOrFalse | undefined); /** * By default, an error will be thrown if the focus trap contains no * elements in its tab order. With this option you can specify a From ecdc8611941b42f4449179a68123833af3e3b18a Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Mon, 8 Jul 2024 22:21:18 +0700 Subject: [PATCH 03/12] update code review --- .../FocusTrapForScreen/FocusTrapProps.ts | 1 + .../FocusTrapForScreen/index.web.tsx | 36 ++++++------------- .../FocusTrap/SCREENS_WITH_AUTOFOCUS.ts | 24 ------------- .../MoneyRequestConfirmationList.tsx | 14 +++++--- src/components/ScreenWrapper.tsx | 2 +- 5 files changed, 22 insertions(+), 55 deletions(-) delete mode 100644 src/components/FocusTrap/SCREENS_WITH_AUTOFOCUS.ts diff --git a/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts b/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts index d2f6e5323445..2d2ba703a04b 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts +++ b/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts @@ -1,5 +1,6 @@ type FocusTrapForScreenProps = { children: React.ReactNode; + testID?: string; }; export default FocusTrapForScreenProps; diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index 2933edda0b60..2b93a79f1a6e 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -1,24 +1,17 @@ -import {useFocusEffect, useIsFocused, useRoute} from '@react-navigation/native'; +import {useIsFocused, useRoute} from '@react-navigation/native'; import FocusTrap from 'focus-trap-react'; -import React, {useCallback, useMemo} from 'react'; -import {useOnyx} from 'react-native-onyx'; +import React, {useEffect, useMemo, useRef} from 'react'; import BOTTOM_TAB_SCREENS from '@components/FocusTrap/BOTTOM_TAB_SCREENS'; -import getScreenWithAutofocus from '@components/FocusTrap/SCREENS_WITH_AUTOFOCUS'; import sharedTrapStack from '@components/FocusTrap/sharedTrapStack'; import TOP_TAB_SCREENS from '@components/FocusTrap/TOP_TAB_SCREENS'; import WIDE_LAYOUT_INACTIVE_SCREENS from '@components/FocusTrap/WIDE_LAYOUT_INACTIVE_SCREENS'; import useWindowDimensions from '@hooks/useWindowDimensions'; -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; import type FocusTrapProps from './FocusTrapProps'; -let activeRouteName = ''; function FocusTrapForScreen({children}: FocusTrapProps) { const isFocused = useIsFocused(); const route = useRoute(); const {isSmallScreenWidth} = useWindowDimensions(); - const [isAuthenticated] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => !!session?.authToken}); - const screensWithAutofocus = useMemo(() => getScreenWithAutofocus(!!isAuthenticated), [isAuthenticated]); const isActive = useMemo(() => { // Focus trap can't be active on bottom tab screens because it would block access to the tab bar. @@ -38,11 +31,11 @@ function FocusTrapForScreen({children}: FocusTrapProps) { return true; }, [isFocused, isSmallScreenWidth, route.name]); - useFocusEffect( - useCallback(() => { - activeRouteName = route.name; - }, [route]), - ); + const mountedRef = useRef(false); + + useEffect(() => { + mountedRef.current = true; + }, []); return ( { - const hasFocusedElement = containers?.some((container) => container.contains(document.activeElement)); - if (hasFocusedElement) { - return false; - } - }, - setReturnFocus: (element) => { - if (screensWithAutofocus.includes(activeRouteName)) { + delayInitialFocus: 400, + initialFocus: (focusTrapContainers) => { + const hasFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); + if (hasFocusedElementInsideContainer) { return false; } - return element; }, }} > diff --git a/src/components/FocusTrap/SCREENS_WITH_AUTOFOCUS.ts b/src/components/FocusTrap/SCREENS_WITH_AUTOFOCUS.ts deleted file mode 100644 index 7af327d35ac4..000000000000 --- a/src/components/FocusTrap/SCREENS_WITH_AUTOFOCUS.ts +++ /dev/null @@ -1,24 +0,0 @@ -import {CENTRAL_PANE_WORKSPACE_SCREENS} from '@libs/Navigation/AppNavigator/Navigators/FullScreenNavigator'; -import NAVIGATORS from '@src/NAVIGATORS'; -import SCREENS from '@src/SCREENS'; - -const SCREENS_WITH_AUTOFOCUS: string[] = [ - ...Object.keys(CENTRAL_PANE_WORKSPACE_SCREENS), - SCREENS.REPORT, - SCREENS.REPORT_DESCRIPTION_ROOT, - SCREENS.PRIVATE_NOTES.EDIT, - SCREENS.SETTINGS.PROFILE.STATUS, - SCREENS.SETTINGS.PROFILE.PRONOUNS, - SCREENS.NEW_TASK.DETAILS, - SCREENS.MONEY_REQUEST.CREATE, - SCREENS.SIGN_IN_ROOT, -]; - -function getScreenWithAutofocus(isAuthenticated: boolean) { - if (!isAuthenticated) { - return [...SCREENS_WITH_AUTOFOCUS, NAVIGATORS.BOTTOM_TAB_NAVIGATOR]; - } - return SCREENS_WITH_AUTOFOCUS; -} - -export default getScreenWithAutofocus; diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index 8f97701fd615..1eecc4ad720b 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -1,8 +1,8 @@ -import {useIsFocused} from '@react-navigation/native'; +import {useFocusEffect, useIsFocused} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import type {TextStyle} from 'react-native'; -import {View} from 'react-native'; +import {InteractionManager, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import type {OnyxEntry} from 'react-native-onyx'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; @@ -716,9 +716,13 @@ function MoneyRequestConfirmationList({ const confirmButtonRef = useRef(null); - useEffect(() => { - confirmButtonRef.current?.focus(); - }, []); + useFocusEffect( + useCallback(() => { + InteractionManager.runAfterInteractions(() => { + confirmButtonRef.current?.focus(); + }); + }, []), + ); const footerContent = useMemo(() => { if (isReadOnly) { diff --git a/src/components/ScreenWrapper.tsx b/src/components/ScreenWrapper.tsx index 1d5d65d9874d..08350cbf8d61 100644 --- a/src/components/ScreenWrapper.tsx +++ b/src/components/ScreenWrapper.tsx @@ -242,7 +242,7 @@ function ScreenWrapper( } return ( - + Date: Mon, 8 Jul 2024 22:36:14 +0700 Subject: [PATCH 04/12] update delay time by constant --- src/components/FocusTrap/FocusTrapForScreen/index.web.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index 2b93a79f1a6e..f791f6431bf0 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -6,6 +6,7 @@ import sharedTrapStack from '@components/FocusTrap/sharedTrapStack'; import TOP_TAB_SCREENS from '@components/FocusTrap/TOP_TAB_SCREENS'; import WIDE_LAYOUT_INACTIVE_SCREENS from '@components/FocusTrap/WIDE_LAYOUT_INACTIVE_SCREENS'; import useWindowDimensions from '@hooks/useWindowDimensions'; +import CONST from '@src/CONST'; import type FocusTrapProps from './FocusTrapProps'; function FocusTrapForScreen({children}: FocusTrapProps) { @@ -45,7 +46,7 @@ function FocusTrapForScreen({children}: FocusTrapProps) { trapStack: sharedTrapStack, allowOutsideClick: true, fallbackFocus: document.body, - delayInitialFocus: 400, + delayInitialFocus: CONST.ANIMATED_TRANSITION, initialFocus: (focusTrapContainers) => { const hasFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); if (hasFocusedElementInsideContainer) { From 08c6f0380dd1628d5101cabb9c98b075b007bd23 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Mon, 8 Jul 2024 23:24:50 +0700 Subject: [PATCH 05/12] Clean up redundant code --- patches/focus-trap+7.5.4.patch | 20 +++++++++---------- .../FocusTrapForScreen/index.web.tsx | 10 ++-------- .../MoneyRequestConfirmationList.tsx | 14 +++++-------- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/patches/focus-trap+7.5.4.patch b/patches/focus-trap+7.5.4.patch index 3eb1e3cb8234..140001835040 100644 --- a/patches/focus-trap+7.5.4.patch +++ b/patches/focus-trap+7.5.4.patch @@ -32,18 +32,18 @@ index 10d56db..a0ed057 100644 doc.addEventListener('mousedown', checkPointerDown, { capture: true, diff --git a/node_modules/focus-trap/index.d.ts b/node_modules/focus-trap/index.d.ts -index 400db1b..e6d1bc3 100644 +index 400db1b..69f4b94 100644 --- a/node_modules/focus-trap/index.d.ts +++ b/node_modules/focus-trap/index.d.ts -@@ -118,7 +118,7 @@ declare module 'focus-trap' { - * Setting this option to `undefined` (or a function that returns `undefined`) - * will result in the default behavior. - */ -- initialFocus?: FocusTargetOrFalse | undefined | (() => void); -+ initialFocus?: FocusTargetValueOrFalse | ((containers?: HTMLElement[]) => FocusTargetValueOrFalse | undefined); - /** - * By default, an error will be thrown if the focus trap contains no - * elements in its tab order. With this option you can specify a +@@ -16,7 +16,7 @@ declare module 'focus-trap' { + * `document.querySelector()` to find the DOM node), `false` to explicitly indicate + * an opt-out, or a function that returns a DOM node or `false`. + */ +- export type FocusTargetOrFalse = FocusTargetValueOrFalse | (() => FocusTargetValueOrFalse); ++ export type FocusTargetOrFalse = FocusTargetValueOrFalse | ((containers?: HTMLElement[]) => FocusTargetValueOrFalse | undefined); + + type MouseEventToBoolean = (event: MouseEvent | TouchEvent) => boolean; + type KeyboardEventToBoolean = (event: KeyboardEvent) => boolean; @@ -185,7 +185,7 @@ declare module 'focus-trap' { * This prevents elements within the focusable element from capturing * the event that triggered the focus trap activation. diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index f791f6431bf0..d7532caf45ba 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -1,6 +1,6 @@ import {useIsFocused, useRoute} from '@react-navigation/native'; import FocusTrap from 'focus-trap-react'; -import React, {useEffect, useMemo, useRef} from 'react'; +import React, {useMemo} from 'react'; import BOTTOM_TAB_SCREENS from '@components/FocusTrap/BOTTOM_TAB_SCREENS'; import sharedTrapStack from '@components/FocusTrap/sharedTrapStack'; import TOP_TAB_SCREENS from '@components/FocusTrap/TOP_TAB_SCREENS'; @@ -32,12 +32,6 @@ function FocusTrapForScreen({children}: FocusTrapProps) { return true; }, [isFocused, isSmallScreenWidth, route.name]); - const mountedRef = useRef(false); - - useEffect(() => { - mountedRef.current = true; - }, []); - return ( { const hasFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); if (hasFocusedElementInsideContainer) { diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index 92703a582a80..0ef9819bdf66 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -1,8 +1,8 @@ -import {useFocusEffect, useIsFocused} from '@react-navigation/native'; +import {useIsFocused} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import type {TextStyle} from 'react-native'; -import {InteractionManager, View} from 'react-native'; +import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import type {OnyxEntry} from 'react-native-onyx'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; @@ -728,13 +728,9 @@ function MoneyRequestConfirmationList({ const confirmButtonRef = useRef(null); - useFocusEffect( - useCallback(() => { - InteractionManager.runAfterInteractions(() => { - confirmButtonRef.current?.focus(); - }); - }, []), - ); + useEffect(() => { + confirmButtonRef.current?.focus(); + }, []); const footerContent = useMemo(() => { if (isReadOnly) { From dc878140cfe3fe5a50ed899cd0690711bf35901d Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Thu, 11 Jul 2024 01:46:25 +0700 Subject: [PATCH 06/12] handle set return focus --- patches/focus-trap+7.5.4.patch | 95 ++++++++++++++++++- patches/focus-trap-react+10.2.3.patch | 46 +++++++++ .../ButtonWithDropdownMenu/types.ts | 2 +- .../FocusTrapForScreen/index.web.tsx | 14 ++- .../MoneyRequestConfirmationList.tsx | 15 ++- src/hooks/useAutoFocusInput.ts | 9 +- 6 files changed, 165 insertions(+), 16 deletions(-) create mode 100644 patches/focus-trap-react+10.2.3.patch diff --git a/patches/focus-trap+7.5.4.patch b/patches/focus-trap+7.5.4.patch index 140001835040..2d78aa125497 100644 --- a/patches/focus-trap+7.5.4.patch +++ b/patches/focus-trap+7.5.4.patch @@ -1,5 +1,5 @@ diff --git a/node_modules/focus-trap/dist/focus-trap.esm.js b/node_modules/focus-trap/dist/focus-trap.esm.js -index 10d56db..a0ed057 100644 +index 10d56db..17c4375 100644 --- a/node_modules/focus-trap/dist/focus-trap.esm.js +++ b/node_modules/focus-trap/dist/focus-trap.esm.js @@ -100,8 +100,8 @@ var isKeyForward = function isKeyForward(e) { @@ -13,16 +13,59 @@ index 10d56db..a0ed057 100644 }; // Array.find/findIndex() are not supported on IE; this replicates enough -@@ -283,7 +283,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { +@@ -283,7 +283,8 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { return node; }; var getInitialFocusNode = function getInitialFocusNode() { - var node = getNodeForOption('initialFocus'); ++ console.log(`############ GetInitialFocusNode:${config.testID} ############`, state.containers); + var node = getNodeForOption('initialFocus', state.containers); // false explicitly indicates we want no initialFocus at all if (node === false) { -@@ -744,7 +744,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { +@@ -440,7 +441,8 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + } + }; + var getReturnFocusNode = function getReturnFocusNode(previousActiveElement) { +- var node = getNodeForOption('setReturnFocus', previousActiveElement); ++ console.log(`############ GetReturnFocusNode:${config.testID} ############`, state.containers); ++ var node = getNodeForOption('setReturnFocus', previousActiveElement, state.containers); + return node ? node : node === false ? false : previousActiveElement; + }; + +@@ -556,6 +558,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + } + if (valueOrHandler(config.clickOutsideDeactivates, e)) { + // immediately deactivate the trap ++ console.log(`############ CheckPointerDown: ${config.testID} ############`); + trap.deactivate({ + // NOTE: by setting `returnFocus: false`, deactivate() will do nothing, + // which will result in the outside click setting focus to the node +@@ -585,6 +588,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + // scrolling if the node that got focused was out of view; there's nothing we can do to + // prevent that from happening by the time we discover that focus escaped + var checkFocusIn = function checkFocusIn(event) { ++ console.log(`############ CheckFocusIn: ${config.testID} ############`); + var target = getActualTarget(event); + var targetContained = findContainerIndex(target, event) >= 0; + +@@ -690,6 +694,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + event: event, + isBackward: isBackward + }); ++ console.log(`############ CheckKeyNav: ${config.testID} ############`, destinationNode); + if (destinationNode) { + if (isTabEvent(event)) { + // since tab natively moves focus, we wouldn't have a destination node unless we +@@ -706,6 +711,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + var checkKey = function checkKey(event) { + if (isEscapeEvent(event) && valueOrHandler(config.escapeDeactivates, event) !== false) { + event.preventDefault(); ++ console.log(`############ CheckKey: ${config.testID} ############`); + trap.deactivate(); + return; + } +@@ -744,7 +750,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { // that caused the focus trap activation. state.delayInitialFocusTimer = config.delayInitialFocus ? delay(function () { tryFocus(getInitialFocusNode()); @@ -31,8 +74,31 @@ index 10d56db..a0ed057 100644 doc.addEventListener('focusin', checkFocusIn, true); doc.addEventListener('mousedown', checkPointerDown, { capture: true, +@@ -854,6 +860,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + return this; + }, + deactivate: function deactivate(deactivateOptions) { ++ console.log(`############ Deactivate: ${config.testID} ############`); + if (!state.active) { + return this; + } +@@ -875,12 +882,13 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { + var returnFocus = getOption(options, 'returnFocus', 'returnFocusOnDeactivate'); + onDeactivate === null || onDeactivate === void 0 || onDeactivate(); + var finishDeactivation = function finishDeactivation() { ++ console.log(`############ FinishDeactivation:${config.testID} ############`, { returnFocus }) + delay(function () { + if (returnFocus) { + tryFocus(getReturnFocusNode(state.nodeFocusedBeforeActivation)); + } + onPostDeactivate === null || onPostDeactivate === void 0 || onPostDeactivate(); +- }); ++ }, typeof config.delayInitialFocus === 'number' ? config.delayInitialFocus : undefined); + }; + if (returnFocus && checkCanReturnFocus) { + checkCanReturnFocus(getReturnFocusNode(state.nodeFocusedBeforeActivation)).then(finishDeactivation, finishDeactivation); diff --git a/node_modules/focus-trap/index.d.ts b/node_modules/focus-trap/index.d.ts -index 400db1b..69f4b94 100644 +index 400db1b..b49618c 100644 --- a/node_modules/focus-trap/index.d.ts +++ b/node_modules/focus-trap/index.d.ts @@ -16,7 +16,7 @@ declare module 'focus-trap' { @@ -44,7 +110,17 @@ index 400db1b..69f4b94 100644 type MouseEventToBoolean = (event: MouseEvent | TouchEvent) => boolean; type KeyboardEventToBoolean = (event: KeyboardEvent) => boolean; -@@ -185,7 +185,7 @@ declare module 'focus-trap' { +@@ -147,7 +147,8 @@ declare module 'focus-trap' { + setReturnFocus?: + | FocusTargetValueOrFalse + | (( +- nodeFocusedBeforeActivation: HTMLElement | SVGElement ++ nodeFocusedBeforeActivation: HTMLElement | SVGElement, ++ containers: Array + ) => FocusTargetValueOrFalse); + /** + * Default: `true`. If `false` or returns `false`, the `Escape` key will not trigger +@@ -185,7 +186,7 @@ declare module 'focus-trap' { * This prevents elements within the focusable element from capturing * the event that triggered the focus trap activation. */ @@ -53,6 +129,15 @@ index 400db1b..69f4b94 100644 /** * Default: `window.document`. Document where the focus trap will be active. * This allows to use FocusTrap in an iFrame context. +@@ -222,6 +223,8 @@ declare module 'focus-trap' { + * keyboard navigation within the trap, for example. Also see `isKeyForward()` option. + */ + isKeyBackward?: KeyboardEventToBoolean; ++ ++ testID?: string; + } + + type ActivateOptions = Pick; diff --git a/node_modules/focus-trap/index.js b/node_modules/focus-trap/index.js index de8e46a..7ab9d89 100644 --- a/node_modules/focus-trap/index.js diff --git a/patches/focus-trap-react+10.2.3.patch b/patches/focus-trap-react+10.2.3.patch new file mode 100644 index 000000000000..159a1cde6e72 --- /dev/null +++ b/patches/focus-trap-react+10.2.3.patch @@ -0,0 +1,46 @@ +diff --git a/node_modules/focus-trap-react/src/focus-trap-react.js b/node_modules/focus-trap-react/src/focus-trap-react.js +index c11345c..2c003d3 100644 +--- a/node_modules/focus-trap-react/src/focus-trap-react.js ++++ b/node_modules/focus-trap-react/src/focus-trap-react.js +@@ -154,7 +154,8 @@ class FocusTrap extends React.Component { + getReturnFocusNode() { + const node = this.getNodeForOption( + 'setReturnFocus', +- this.previouslyFocusedElement ++ this.previouslyFocusedElement, ++ this.focusTrapElements, + ); + return node ? node : node === false ? false : this.previouslyFocusedElement; + } +@@ -215,6 +216,7 @@ class FocusTrap extends React.Component { + } + + handleDeactivate() { ++ console.log(`<<<<<<<<<<<<<<< HandleDeactivate: ${this.internalOptions.testID} >>>>>>>>>>>>>>>`); + if (this.originalOptions.onDeactivate) { + this.originalOptions.onDeactivate.call(null); // call user's handler out of context + } +@@ -222,6 +224,7 @@ class FocusTrap extends React.Component { + } + + handlePostDeactivate() { ++ console.log(`<<<<<<<<<<<<<<< HandlePostDeactivate: ${this.internalOptions.testID} >>>>>>>>>>>>>>>`, this.focusTrapElements); + const finishDeactivation = () => { + const returnFocusNode = this.getReturnFocusNode(); + const canReturnFocus = !!( +@@ -338,6 +341,7 @@ class FocusTrap extends React.Component { + } + + if (hasDeactivated) { ++ console.log(`<<<<<<<<<<<<<<< ComponentDidUpdate:${this.internalOptions.testID}:DeactivateTrap >>>>>>>>>>>>>>>`); + this.deactivateTrap(); + return; // un/pause does nothing on an inactive trap + } +@@ -370,6 +374,7 @@ class FocusTrap extends React.Component { + } + + componentWillUnmount() { ++ console.log(`<<<<<<<<<<<<<<< ComponentWillUnmount:${this.internalOptions.testID} >>>>>>>>>>>>>>>`); + this.deactivateTrap(); + } + diff --git a/src/components/ButtonWithDropdownMenu/types.ts b/src/components/ButtonWithDropdownMenu/types.ts index d1eedd560694..94e5b6aa5e5d 100644 --- a/src/components/ButtonWithDropdownMenu/types.ts +++ b/src/components/ButtonWithDropdownMenu/types.ts @@ -65,7 +65,7 @@ type ButtonWithDropdownMenuProps = { anchorAlignment?: AnchorAlignment; /* ref for the button */ - buttonRef?: RefObject; + buttonRef?: RefObject | ((ref: View) => void); /** The priority to assign the enter key event listener to buttons. 0 is the highest priority. */ enterKeyEventListenerPriority?: number; diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index d7532caf45ba..9861e3fae1f3 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -9,7 +9,7 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import CONST from '@src/CONST'; import type FocusTrapProps from './FocusTrapProps'; -function FocusTrapForScreen({children}: FocusTrapProps) { +function FocusTrapForScreen({children, testID}: FocusTrapProps) { const isFocused = useIsFocused(); const route = useRoute(); const {isSmallScreenWidth} = useWindowDimensions(); @@ -37,16 +37,28 @@ function FocusTrapForScreen({children}: FocusTrapProps) { active={isActive} paused={!isFocused} focusTrapOptions={{ + testID, trapStack: sharedTrapStack, allowOutsideClick: true, fallbackFocus: document.body, delayInitialFocus: CONST.ANIMATED_TRANSITION + CONST.ANIMATION_IN_TIMING, initialFocus: (focusTrapContainers) => { + console.log(`*********** InitialFocus: ${testID} ***********`, focusTrapContainers, document.activeElement); const hasFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); if (hasFocusedElementInsideContainer) { return false; } }, + onPostDeactivate: () => { + console.log(`*********** OnPostDeactivate: ${testID} ***********`, document.activeElement); + }, + setReturnFocus: (node, containers) => { + console.log(`*********** SetReturnFocus: ${testID} ***********`, { node, containers }, document.activeElement); + if (document.activeElement !== document.body) { + return false; + } + return node; + } }} > {children} diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index 68f478c404e2..d8b3a598f7cf 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -1,4 +1,4 @@ -import {useIsFocused} from '@react-navigation/native'; +import {useFocusEffect, useIsFocused} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import type {TextStyle} from 'react-native'; @@ -48,6 +48,7 @@ import type {SectionListDataType} from './SelectionList/types'; import UserListItem from './SelectionList/UserListItem'; import SettlementButton from './SettlementButton'; import Text from './Text'; +import useAutoFocusInput from '@hooks/useAutoFocusInput'; type MoneyRequestConfirmationListOnyxProps = { /** Collection of categories attached to a policy */ @@ -726,11 +727,12 @@ function MoneyRequestConfirmationList({ ], ); + const {inputCallbackRef} = useAutoFocusInput(); const confirmButtonRef = useRef(null); - useEffect(() => { - confirmButtonRef.current?.focus(); - }, []); + useFocusEffect(useCallback(() => { + // confirmButtonRef.current?.focus(); + },[])) const footerContent = useMemo(() => { if (isReadOnly) { @@ -763,7 +765,10 @@ function MoneyRequestConfirmationList({ /> ) : ( { + inputCallbackRef(ref); + }} success pressOnEnter isDisabled={shouldDisableButton} diff --git a/src/hooks/useAutoFocusInput.ts b/src/hooks/useAutoFocusInput.ts index 2e2d9a086ab1..75bf6fa8670b 100644 --- a/src/hooks/useAutoFocusInput.ts +++ b/src/hooks/useAutoFocusInput.ts @@ -1,12 +1,13 @@ import {useFocusEffect} from '@react-navigation/native'; import {useCallback, useContext, useEffect, useRef, useState} from 'react'; -import type {TextInput} from 'react-native'; +import type {View, TextInput} from 'react-native'; import {InteractionManager} from 'react-native'; import CONST from '@src/CONST'; import * as Expensify from '@src/Expensify'; +type FocusRef = View | TextInput | null type UseAutoFocusInput = { - inputCallbackRef: (ref: TextInput | null) => void; + inputCallbackRef: (ref: FocusRef) => void; }; export default function useAutoFocusInput(): UseAutoFocusInput { @@ -15,7 +16,7 @@ export default function useAutoFocusInput(): UseAutoFocusInput { const {isSplashHidden} = useContext(Expensify.SplashScreenHiddenContext); - const inputRef = useRef(null); + const inputRef = useRef(null); const focusTimeoutRef = useRef(null); useEffect(() => { @@ -47,7 +48,7 @@ export default function useAutoFocusInput(): UseAutoFocusInput { }, []), ); - const inputCallbackRef = (ref: TextInput | null) => { + const inputCallbackRef = (ref: FocusRef) => { inputRef.current = ref; if (isInputInitialized) { return; From bb24cd90e665c7bd2ca523e8e0dd70a1c3759953 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Thu, 11 Jul 2024 14:03:05 +0700 Subject: [PATCH 07/12] remove debug log --- patches/focus-trap+7.5.4.patch | 69 ++++--------------- patches/focus-trap-react+10.2.3.patch | 46 ------------- .../FocusTrapForScreen/index.web.tsx | 7 +- 3 files changed, 16 insertions(+), 106 deletions(-) delete mode 100644 patches/focus-trap-react+10.2.3.patch diff --git a/patches/focus-trap+7.5.4.patch b/patches/focus-trap+7.5.4.patch index 2d78aa125497..8d3bd5856b4d 100644 --- a/patches/focus-trap+7.5.4.patch +++ b/patches/focus-trap+7.5.4.patch @@ -1,5 +1,5 @@ diff --git a/node_modules/focus-trap/dist/focus-trap.esm.js b/node_modules/focus-trap/dist/focus-trap.esm.js -index 10d56db..17c4375 100644 +index 10d56db..7873392 100644 --- a/node_modules/focus-trap/dist/focus-trap.esm.js +++ b/node_modules/focus-trap/dist/focus-trap.esm.js @@ -100,8 +100,8 @@ var isKeyForward = function isKeyForward(e) { @@ -13,59 +13,25 @@ index 10d56db..17c4375 100644 }; // Array.find/findIndex() are not supported on IE; this replicates enough -@@ -283,7 +283,8 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { +@@ -283,7 +283,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { return node; }; var getInitialFocusNode = function getInitialFocusNode() { - var node = getNodeForOption('initialFocus'); -+ console.log(`############ GetInitialFocusNode:${config.testID} ############`, state.containers); + var node = getNodeForOption('initialFocus', state.containers); // false explicitly indicates we want no initialFocus at all if (node === false) { -@@ -440,7 +441,8 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { +@@ -440,7 +440,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { } }; var getReturnFocusNode = function getReturnFocusNode(previousActiveElement) { - var node = getNodeForOption('setReturnFocus', previousActiveElement); -+ console.log(`############ GetReturnFocusNode:${config.testID} ############`, state.containers); + var node = getNodeForOption('setReturnFocus', previousActiveElement, state.containers); return node ? node : node === false ? false : previousActiveElement; }; -@@ -556,6 +558,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { - } - if (valueOrHandler(config.clickOutsideDeactivates, e)) { - // immediately deactivate the trap -+ console.log(`############ CheckPointerDown: ${config.testID} ############`); - trap.deactivate({ - // NOTE: by setting `returnFocus: false`, deactivate() will do nothing, - // which will result in the outside click setting focus to the node -@@ -585,6 +588,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { - // scrolling if the node that got focused was out of view; there's nothing we can do to - // prevent that from happening by the time we discover that focus escaped - var checkFocusIn = function checkFocusIn(event) { -+ console.log(`############ CheckFocusIn: ${config.testID} ############`); - var target = getActualTarget(event); - var targetContained = findContainerIndex(target, event) >= 0; - -@@ -690,6 +694,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { - event: event, - isBackward: isBackward - }); -+ console.log(`############ CheckKeyNav: ${config.testID} ############`, destinationNode); - if (destinationNode) { - if (isTabEvent(event)) { - // since tab natively moves focus, we wouldn't have a destination node unless we -@@ -706,6 +711,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { - var checkKey = function checkKey(event) { - if (isEscapeEvent(event) && valueOrHandler(config.escapeDeactivates, event) !== false) { - event.preventDefault(); -+ console.log(`############ CheckKey: ${config.testID} ############`); - trap.deactivate(); - return; - } -@@ -744,7 +750,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { +@@ -744,7 +744,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { // that caused the focus trap activation. state.delayInitialFocusTimer = config.delayInitialFocus ? delay(function () { tryFocus(getInitialFocusNode()); @@ -74,21 +40,7 @@ index 10d56db..17c4375 100644 doc.addEventListener('focusin', checkFocusIn, true); doc.addEventListener('mousedown', checkPointerDown, { capture: true, -@@ -854,6 +860,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { - return this; - }, - deactivate: function deactivate(deactivateOptions) { -+ console.log(`############ Deactivate: ${config.testID} ############`); - if (!state.active) { - return this; - } -@@ -875,12 +882,13 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { - var returnFocus = getOption(options, 'returnFocus', 'returnFocusOnDeactivate'); - onDeactivate === null || onDeactivate === void 0 || onDeactivate(); - var finishDeactivation = function finishDeactivation() { -+ console.log(`############ FinishDeactivation:${config.testID} ############`, { returnFocus }) - delay(function () { - if (returnFocus) { +@@ -880,7 +880,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { tryFocus(getReturnFocusNode(state.nodeFocusedBeforeActivation)); } onPostDeactivate === null || onPostDeactivate === void 0 || onPostDeactivate(); @@ -139,7 +91,7 @@ index 400db1b..b49618c 100644 type ActivateOptions = Pick; diff --git a/node_modules/focus-trap/index.js b/node_modules/focus-trap/index.js -index de8e46a..7ab9d89 100644 +index de8e46a..287b03c 100644 --- a/node_modules/focus-trap/index.js +++ b/node_modules/focus-trap/index.js @@ -63,8 +63,8 @@ const isKeyBackward = function (e) { @@ -171,3 +123,12 @@ index de8e46a..7ab9d89 100644 : tryFocus(getInitialFocusNode()); doc.addEventListener('focusin', checkFocusIn, true); +@@ -989,7 +989,7 @@ const createFocusTrap = function (elements, userOptions) { + tryFocus(getReturnFocusNode(state.nodeFocusedBeforeActivation)); + } + onPostDeactivate?.(); +- }); ++ }, typeof config.delayInitialFocus === 'number' ? config.delayInitialFocus : undefined); + }; + + if (returnFocus && checkCanReturnFocus) { diff --git a/patches/focus-trap-react+10.2.3.patch b/patches/focus-trap-react+10.2.3.patch deleted file mode 100644 index 159a1cde6e72..000000000000 --- a/patches/focus-trap-react+10.2.3.patch +++ /dev/null @@ -1,46 +0,0 @@ -diff --git a/node_modules/focus-trap-react/src/focus-trap-react.js b/node_modules/focus-trap-react/src/focus-trap-react.js -index c11345c..2c003d3 100644 ---- a/node_modules/focus-trap-react/src/focus-trap-react.js -+++ b/node_modules/focus-trap-react/src/focus-trap-react.js -@@ -154,7 +154,8 @@ class FocusTrap extends React.Component { - getReturnFocusNode() { - const node = this.getNodeForOption( - 'setReturnFocus', -- this.previouslyFocusedElement -+ this.previouslyFocusedElement, -+ this.focusTrapElements, - ); - return node ? node : node === false ? false : this.previouslyFocusedElement; - } -@@ -215,6 +216,7 @@ class FocusTrap extends React.Component { - } - - handleDeactivate() { -+ console.log(`<<<<<<<<<<<<<<< HandleDeactivate: ${this.internalOptions.testID} >>>>>>>>>>>>>>>`); - if (this.originalOptions.onDeactivate) { - this.originalOptions.onDeactivate.call(null); // call user's handler out of context - } -@@ -222,6 +224,7 @@ class FocusTrap extends React.Component { - } - - handlePostDeactivate() { -+ console.log(`<<<<<<<<<<<<<<< HandlePostDeactivate: ${this.internalOptions.testID} >>>>>>>>>>>>>>>`, this.focusTrapElements); - const finishDeactivation = () => { - const returnFocusNode = this.getReturnFocusNode(); - const canReturnFocus = !!( -@@ -338,6 +341,7 @@ class FocusTrap extends React.Component { - } - - if (hasDeactivated) { -+ console.log(`<<<<<<<<<<<<<<< ComponentDidUpdate:${this.internalOptions.testID}:DeactivateTrap >>>>>>>>>>>>>>>`); - this.deactivateTrap(); - return; // un/pause does nothing on an inactive trap - } -@@ -370,6 +374,7 @@ class FocusTrap extends React.Component { - } - - componentWillUnmount() { -+ console.log(`<<<<<<<<<<<<<<< ComponentWillUnmount:${this.internalOptions.testID} >>>>>>>>>>>>>>>`); - this.deactivateTrap(); - } - diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index 9861e3fae1f3..0c11500e5872 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -43,17 +43,12 @@ function FocusTrapForScreen({children, testID}: FocusTrapProps) { fallbackFocus: document.body, delayInitialFocus: CONST.ANIMATED_TRANSITION + CONST.ANIMATION_IN_TIMING, initialFocus: (focusTrapContainers) => { - console.log(`*********** InitialFocus: ${testID} ***********`, focusTrapContainers, document.activeElement); const hasFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); if (hasFocusedElementInsideContainer) { return false; } }, - onPostDeactivate: () => { - console.log(`*********** OnPostDeactivate: ${testID} ***********`, document.activeElement); - }, - setReturnFocus: (node, containers) => { - console.log(`*********** SetReturnFocus: ${testID} ***********`, { node, containers }, document.activeElement); + setReturnFocus: (node) => { if (document.activeElement !== document.body) { return false; } From 1e590d4f5578de0553fed4b3c12303f45ccb689a Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Thu, 11 Jul 2024 18:44:25 +0700 Subject: [PATCH 08/12] update code review --- .../FocusTrap/FocusTrapForScreen/FocusTrapProps.ts | 1 - .../FocusTrap/FocusTrapForScreen/index.web.tsx | 11 +++++------ src/components/MoneyRequestConfirmationList.tsx | 12 +++--------- src/hooks/useAutoFocusInput.ts | 4 ++-- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts b/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts index 2d2ba703a04b..d2f6e5323445 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts +++ b/src/components/FocusTrap/FocusTrapForScreen/FocusTrapProps.ts @@ -1,6 +1,5 @@ type FocusTrapForScreenProps = { children: React.ReactNode; - testID?: string; }; export default FocusTrapForScreenProps; diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index 0c11500e5872..64ece576eaf3 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -9,7 +9,7 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import CONST from '@src/CONST'; import type FocusTrapProps from './FocusTrapProps'; -function FocusTrapForScreen({children, testID}: FocusTrapProps) { +function FocusTrapForScreen({children}: FocusTrapProps) { const isFocused = useIsFocused(); const route = useRoute(); const {isSmallScreenWidth} = useWindowDimensions(); @@ -37,7 +37,6 @@ function FocusTrapForScreen({children, testID}: FocusTrapProps) { active={isActive} paused={!isFocused} focusTrapOptions={{ - testID, trapStack: sharedTrapStack, allowOutsideClick: true, fallbackFocus: document.body, @@ -48,12 +47,12 @@ function FocusTrapForScreen({children, testID}: FocusTrapProps) { return false; } }, - setReturnFocus: (node) => { - if (document.activeElement !== document.body) { + setReturnFocus: (element) => { + if (document.activeElement && document.activeElement !== document.body) { return false; } - return node; - } + return element; + }, }} > {children} diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index d8b3a598f7cf..42a89d4c8c13 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -1,10 +1,11 @@ -import {useFocusEffect, useIsFocused} from '@react-navigation/native'; +import {useIsFocused} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; -import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {memo, useCallback, useEffect, useMemo, useState} from 'react'; import type {TextStyle} from 'react-native'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import type {OnyxEntry} from 'react-native-onyx'; +import useAutoFocusInput from '@hooks/useAutoFocusInput'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useDebouncedState from '@hooks/useDebouncedState'; import useLocalize from '@hooks/useLocalize'; @@ -48,7 +49,6 @@ import type {SectionListDataType} from './SelectionList/types'; import UserListItem from './SelectionList/UserListItem'; import SettlementButton from './SettlementButton'; import Text from './Text'; -import useAutoFocusInput from '@hooks/useAutoFocusInput'; type MoneyRequestConfirmationListOnyxProps = { /** Collection of categories attached to a policy */ @@ -728,11 +728,6 @@ function MoneyRequestConfirmationList({ ); const {inputCallbackRef} = useAutoFocusInput(); - const confirmButtonRef = useRef(null); - - useFocusEffect(useCallback(() => { - // confirmButtonRef.current?.focus(); - },[])) const footerContent = useMemo(() => { if (isReadOnly) { @@ -765,7 +760,6 @@ function MoneyRequestConfirmationList({ /> ) : ( { inputCallbackRef(ref); }} diff --git a/src/hooks/useAutoFocusInput.ts b/src/hooks/useAutoFocusInput.ts index 75bf6fa8670b..0938f77a2668 100644 --- a/src/hooks/useAutoFocusInput.ts +++ b/src/hooks/useAutoFocusInput.ts @@ -1,11 +1,11 @@ import {useFocusEffect} from '@react-navigation/native'; import {useCallback, useContext, useEffect, useRef, useState} from 'react'; -import type {View, TextInput} from 'react-native'; +import type {TextInput, View} from 'react-native'; import {InteractionManager} from 'react-native'; import CONST from '@src/CONST'; import * as Expensify from '@src/Expensify'; -type FocusRef = View | TextInput | null +type FocusRef = View | TextInput | null; type UseAutoFocusInput = { inputCallbackRef: (ref: FocusRef) => void; }; From c6e1bb669127e3669b7bbcb89d0c4e88a098c253 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Thu, 11 Jul 2024 21:26:14 +0700 Subject: [PATCH 09/12] fix typecheck --- src/components/FocusTrap/FocusTrapForScreen/index.web.tsx | 1 + src/components/MoneyRequestConfirmationList.tsx | 1 + src/components/ScreenWrapper.tsx | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index 64ece576eaf3..68b53833e56c 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -46,6 +46,7 @@ function FocusTrapForScreen({children}: FocusTrapProps) { if (hasFocusedElementInsideContainer) { return false; } + return undefined; }, setReturnFocus: (element) => { if (document.activeElement && document.activeElement !== document.body) { diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index 42a89d4c8c13..b9a722471a8b 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -802,6 +802,7 @@ function MoneyRequestConfirmationList({ shouldShowReadOnlySplits, debouncedFormError, translate, + inputCallbackRef, ]); const listFooterContent = ( diff --git a/src/components/ScreenWrapper.tsx b/src/components/ScreenWrapper.tsx index 166626ee0fcc..f845cfda3638 100644 --- a/src/components/ScreenWrapper.tsx +++ b/src/components/ScreenWrapper.tsx @@ -242,7 +242,7 @@ function ScreenWrapper( } return ( - + Date: Fri, 12 Jul 2024 22:10:33 +0700 Subject: [PATCH 10/12] update code review --- patches/focus-trap+7.5.4.patch | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/patches/focus-trap+7.5.4.patch b/patches/focus-trap+7.5.4.patch index 8d3bd5856b4d..b74733664f2e 100644 --- a/patches/focus-trap+7.5.4.patch +++ b/patches/focus-trap+7.5.4.patch @@ -1,5 +1,5 @@ diff --git a/node_modules/focus-trap/dist/focus-trap.esm.js b/node_modules/focus-trap/dist/focus-trap.esm.js -index 10d56db..7873392 100644 +index 10d56db..60fccbe 100644 --- a/node_modules/focus-trap/dist/focus-trap.esm.js +++ b/node_modules/focus-trap/dist/focus-trap.esm.js @@ -100,8 +100,8 @@ var isKeyForward = function isKeyForward(e) { @@ -9,7 +9,7 @@ index 10d56db..7873392 100644 -var delay = function delay(fn) { - return setTimeout(fn, 0); +var delay = function delay(fn, delayTime = 0) { -+ return setTimeout(fn, delayTime); ++ return setTimeout(() => setTimeout(fn, delayTime), 0); }; // Array.find/findIndex() are not supported on IE; this replicates enough @@ -91,7 +91,7 @@ index 400db1b..b49618c 100644 type ActivateOptions = Pick; diff --git a/node_modules/focus-trap/index.js b/node_modules/focus-trap/index.js -index de8e46a..287b03c 100644 +index de8e46a..bfc8b63 100644 --- a/node_modules/focus-trap/index.js +++ b/node_modules/focus-trap/index.js @@ -63,8 +63,8 @@ const isKeyBackward = function (e) { @@ -101,7 +101,7 @@ index de8e46a..287b03c 100644 -const delay = function (fn) { - return setTimeout(fn, 0); +const delay = function (fn, delayTime = 0) { -+ return setTimeout(fn, delayTime); ++ return setTimeout(() => setTimeout(fn, delayTime), 0); }; // Array.find/findIndex() are not supported on IE; this replicates enough From bd4a9353590819dfecc3e0f7f0a9e701e2b38a1c Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Sun, 14 Jul 2024 00:22:52 +0700 Subject: [PATCH 11/12] update code review --- patches/focus-trap+7.5.4.patch | 34 ++----------------- .../FocusTrapForScreen/index.web.tsx | 2 +- .../MoneyRequestConfirmationList.tsx | 4 +-- 3 files changed, 5 insertions(+), 35 deletions(-) diff --git a/patches/focus-trap+7.5.4.patch b/patches/focus-trap+7.5.4.patch index b74733664f2e..c7b2aef2b51f 100644 --- a/patches/focus-trap+7.5.4.patch +++ b/patches/focus-trap+7.5.4.patch @@ -1,5 +1,5 @@ diff --git a/node_modules/focus-trap/dist/focus-trap.esm.js b/node_modules/focus-trap/dist/focus-trap.esm.js -index 10d56db..60fccbe 100644 +index 10d56db..a6d76d8 100644 --- a/node_modules/focus-trap/dist/focus-trap.esm.js +++ b/node_modules/focus-trap/dist/focus-trap.esm.js @@ -100,8 +100,8 @@ var isKeyForward = function isKeyForward(e) { @@ -22,15 +22,6 @@ index 10d56db..60fccbe 100644 // false explicitly indicates we want no initialFocus at all if (node === false) { -@@ -440,7 +440,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { - } - }; - var getReturnFocusNode = function getReturnFocusNode(previousActiveElement) { -- var node = getNodeForOption('setReturnFocus', previousActiveElement); -+ var node = getNodeForOption('setReturnFocus', previousActiveElement, state.containers); - return node ? node : node === false ? false : previousActiveElement; - }; - @@ -744,7 +744,7 @@ var createFocusTrap = function createFocusTrap(elements, userOptions) { // that caused the focus trap activation. state.delayInitialFocusTimer = config.delayInitialFocus ? delay(function () { @@ -50,7 +41,7 @@ index 10d56db..60fccbe 100644 if (returnFocus && checkCanReturnFocus) { checkCanReturnFocus(getReturnFocusNode(state.nodeFocusedBeforeActivation)).then(finishDeactivation, finishDeactivation); diff --git a/node_modules/focus-trap/index.d.ts b/node_modules/focus-trap/index.d.ts -index 400db1b..b49618c 100644 +index 400db1b..69f4b94 100644 --- a/node_modules/focus-trap/index.d.ts +++ b/node_modules/focus-trap/index.d.ts @@ -16,7 +16,7 @@ declare module 'focus-trap' { @@ -62,17 +53,7 @@ index 400db1b..b49618c 100644 type MouseEventToBoolean = (event: MouseEvent | TouchEvent) => boolean; type KeyboardEventToBoolean = (event: KeyboardEvent) => boolean; -@@ -147,7 +147,8 @@ declare module 'focus-trap' { - setReturnFocus?: - | FocusTargetValueOrFalse - | (( -- nodeFocusedBeforeActivation: HTMLElement | SVGElement -+ nodeFocusedBeforeActivation: HTMLElement | SVGElement, -+ containers: Array - ) => FocusTargetValueOrFalse); - /** - * Default: `true`. If `false` or returns `false`, the `Escape` key will not trigger -@@ -185,7 +186,7 @@ declare module 'focus-trap' { +@@ -185,7 +185,7 @@ declare module 'focus-trap' { * This prevents elements within the focusable element from capturing * the event that triggered the focus trap activation. */ @@ -81,15 +62,6 @@ index 400db1b..b49618c 100644 /** * Default: `window.document`. Document where the focus trap will be active. * This allows to use FocusTrap in an iFrame context. -@@ -222,6 +223,8 @@ declare module 'focus-trap' { - * keyboard navigation within the trap, for example. Also see `isKeyForward()` option. - */ - isKeyBackward?: KeyboardEventToBoolean; -+ -+ testID?: string; - } - - type ActivateOptions = Pick; diff --git a/node_modules/focus-trap/index.js b/node_modules/focus-trap/index.js index de8e46a..bfc8b63 100644 --- a/node_modules/focus-trap/index.js diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index 68b53833e56c..d649c91f9c72 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -40,7 +40,7 @@ function FocusTrapForScreen({children}: FocusTrapProps) { trapStack: sharedTrapStack, allowOutsideClick: true, fallbackFocus: document.body, - delayInitialFocus: CONST.ANIMATED_TRANSITION + CONST.ANIMATION_IN_TIMING, + delayInitialFocus: CONST.ANIMATED_TRANSITION, initialFocus: (focusTrapContainers) => { const hasFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); if (hasFocusedElementInsideContainer) { diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index b9a722471a8b..595e55cdac96 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -760,9 +760,7 @@ function MoneyRequestConfirmationList({ /> ) : ( { - inputCallbackRef(ref); - }} + buttonRef={inputCallbackRef} success pressOnEnter isDisabled={shouldDisableButton} From 367e62f3282fa3efe02de53803f62529c51e5eb1 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Tue, 16 Jul 2024 23:30:17 +0700 Subject: [PATCH 12/12] update the submit button focusing behavior --- .../ButtonWithDropdownMenu/types.ts | 2 +- .../FocusTrapForScreen/index.web.tsx | 4 ++-- .../MoneyRequestConfirmationList.tsx | 19 +++++++++++++------ src/hooks/useAutoFocusInput.ts | 9 ++++----- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/components/ButtonWithDropdownMenu/types.ts b/src/components/ButtonWithDropdownMenu/types.ts index 4902b93200f2..59621a8dd58c 100644 --- a/src/components/ButtonWithDropdownMenu/types.ts +++ b/src/components/ButtonWithDropdownMenu/types.ts @@ -66,7 +66,7 @@ type ButtonWithDropdownMenuProps = { anchorAlignment?: AnchorAlignment; /* ref for the button */ - buttonRef?: RefObject | ((ref: View) => void); + buttonRef?: RefObject; /** The priority to assign the enter key event listener to buttons. 0 is the highest priority. */ enterKeyEventListenerPriority?: number; diff --git a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx index d649c91f9c72..e7fe135c952c 100644 --- a/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx +++ b/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx @@ -42,8 +42,8 @@ function FocusTrapForScreen({children}: FocusTrapProps) { fallbackFocus: document.body, delayInitialFocus: CONST.ANIMATED_TRANSITION, initialFocus: (focusTrapContainers) => { - const hasFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); - if (hasFocusedElementInsideContainer) { + const isFocusedElementInsideContainer = focusTrapContainers?.some((container) => container.contains(document.activeElement)); + if (isFocusedElementInsideContainer) { return false; } return undefined; diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index a7452a403b56..7d65b11a6401 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -1,10 +1,9 @@ -import {useIsFocused} from '@react-navigation/native'; +import {useFocusEffect, useIsFocused} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; -import React, {memo, useCallback, useEffect, useMemo, useState} from 'react'; +import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import type {OnyxEntry} from 'react-native-onyx'; -import useAutoFocusInput from '@hooks/useAutoFocusInput'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useDebouncedState from '@hooks/useDebouncedState'; import useLocalize from '@hooks/useLocalize'; @@ -738,7 +737,16 @@ function MoneyRequestConfirmationList({ ], ); - const {inputCallbackRef} = useAutoFocusInput(); + const focusTimeoutRef = useRef(null); + const confirmButtonRef = useRef(null); + useFocusEffect( + useCallback(() => { + focusTimeoutRef.current = setTimeout(() => { + confirmButtonRef.current?.focus(); + }, CONST.ANIMATED_TRANSITION); + return () => focusTimeoutRef.current && clearTimeout(focusTimeoutRef.current); + }, []), + ); const footerContent = useMemo(() => { if (isReadOnly) { @@ -771,7 +779,7 @@ function MoneyRequestConfirmationList({ /> ) : ( void; + inputCallbackRef: (ref: TextInput | null) => void; }; export default function useAutoFocusInput(): UseAutoFocusInput { @@ -16,7 +15,7 @@ export default function useAutoFocusInput(): UseAutoFocusInput { const {isSplashHidden} = useContext(Expensify.SplashScreenHiddenContext); - const inputRef = useRef(null); + const inputRef = useRef(null); const focusTimeoutRef = useRef(null); useEffect(() => { @@ -48,7 +47,7 @@ export default function useAutoFocusInput(): UseAutoFocusInput { }, []), ); - const inputCallbackRef = (ref: FocusRef) => { + const inputCallbackRef = (ref: TextInput | null) => { inputRef.current = ref; if (isInputInitialized) { return;