From 9de54e53b62b777069c031d3f80345f5e4d427cd Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:19:44 +0100 Subject: [PATCH 1/7] Revert "Revert "Add focus trap to the RHP (v2)"" This reverts commit d24deb586d8ba72ac921db69b8e0b451d39ff29a. --- package-lock.json | 50 +++++++++++++++++++ package.json | 1 + src/components/ConfirmModal.js | 1 + src/components/FocusTrapView/index.native.tsx | 12 +++++ src/components/FocusTrapView/index.tsx | 42 ++++++++++++++++ src/components/FocusTrapView/types.ts | 21 ++++++++ src/components/Modal/index.tsx | 13 ++++- src/components/Modal/modalPropTypes.js | 4 ++ src/components/Modal/types.ts | 3 ++ src/components/ScreenWrapper/index.js | 41 +++++++++------ src/components/ScreenWrapper/propTypes.js | 8 +++ src/pages/ProfilePage.js | 5 +- src/pages/ReportDetailsPage.js | 5 +- src/pages/home/ReportScreen.js | 1 + .../SidebarScreen/BaseSidebarScreen.js | 1 + src/pages/iou/SplitBillDetailsPage.js | 5 +- .../TwoFactorAuth/TwoFactorAuthSteps.js | 24 +++------ 17 files changed, 200 insertions(+), 37 deletions(-) create mode 100644 src/components/FocusTrapView/index.native.tsx create mode 100644 src/components/FocusTrapView/index.tsx create mode 100644 src/components/FocusTrapView/types.ts diff --git a/package-lock.json b/package-lock.json index 6d125e2f6b2..65dee8cde80 100644 --- a/package-lock.json +++ b/package-lock.json @@ -52,6 +52,7 @@ "domhandler": "^4.3.0", "expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#ee14b3255da33d2b6924c357f43393251b6dc6d2", "fbjs": "^3.0.2", + "focus-trap-react": "^10.2.1", "htmlparser2": "^7.2.0", "idb-keyval": "^6.2.1", "jest-when": "^3.5.2", @@ -30776,6 +30777,28 @@ "readable-stream": "^2.3.6" } }, + "node_modules/focus-trap": { + "version": "7.5.2", + "resolved": "https://registry.npmjs.org/focus-trap/-/focus-trap-7.5.2.tgz", + "integrity": "sha512-p6vGNNWLDGwJCiEjkSK6oERj/hEyI9ITsSwIUICBoKLlWiTWXJRfQibCwcoi50rTZdbi87qDtUlMCmQwsGSgPw==", + "dependencies": { + "tabbable": "^6.2.0" + } + }, + "node_modules/focus-trap-react": { + "version": "10.2.1", + "resolved": "https://registry.npmjs.org/focus-trap-react/-/focus-trap-react-10.2.1.tgz", + "integrity": "sha512-UrAKOn52lvfHF6lkUMfFhlQxFgahyNW5i6FpHWkDxAeD4FSk3iwx9n4UEA4Sims0G5WiGIi0fAyoq3/UVeNCYA==", + "dependencies": { + "focus-trap": "^7.5.2", + "tabbable": "^6.2.0" + }, + "peerDependencies": { + "prop-types": "^15.8.1", + "react": ">=16.3.0", + "react-dom": ">=16.3.0" + } + }, "node_modules/follow-redirects": { "version": "1.15.3", "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.3.tgz", @@ -49038,6 +49061,11 @@ "dev": true, "license": "BSD-3-Clause" }, + "node_modules/tabbable": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/tabbable/-/tabbable-6.2.0.tgz", + "integrity": "sha512-Cat63mxsVJlzYvN51JmVXIgNoUokrIaT2zLclCXjRd8boZ0004U4KCs/sToJ75C6sdlByWxpYnb5Boif1VSFew==" + }, "node_modules/table": { "version": "6.8.1", "resolved": "https://registry.npmjs.org/table/-/table-6.8.1.tgz", @@ -75061,6 +75089,23 @@ "readable-stream": "^2.3.6" } }, + "focus-trap": { + "version": "7.5.2", + "resolved": "https://registry.npmjs.org/focus-trap/-/focus-trap-7.5.2.tgz", + "integrity": "sha512-p6vGNNWLDGwJCiEjkSK6oERj/hEyI9ITsSwIUICBoKLlWiTWXJRfQibCwcoi50rTZdbi87qDtUlMCmQwsGSgPw==", + "requires": { + "tabbable": "^6.2.0" + } + }, + "focus-trap-react": { + "version": "10.2.1", + "resolved": "https://registry.npmjs.org/focus-trap-react/-/focus-trap-react-10.2.1.tgz", + "integrity": "sha512-UrAKOn52lvfHF6lkUMfFhlQxFgahyNW5i6FpHWkDxAeD4FSk3iwx9n4UEA4Sims0G5WiGIi0fAyoq3/UVeNCYA==", + "requires": { + "focus-trap": "^7.5.2", + "tabbable": "^6.2.0" + } + }, "follow-redirects": { "version": "1.15.3", "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.3.tgz", @@ -88057,6 +88102,11 @@ "version": "2.0.15", "dev": true }, + "tabbable": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/tabbable/-/tabbable-6.2.0.tgz", + "integrity": "sha512-Cat63mxsVJlzYvN51JmVXIgNoUokrIaT2zLclCXjRd8boZ0004U4KCs/sToJ75C6sdlByWxpYnb5Boif1VSFew==" + }, "table": { "version": "6.8.1", "resolved": "https://registry.npmjs.org/table/-/table-6.8.1.tgz", diff --git a/package.json b/package.json index dc1964eac40..d75d7c96487 100644 --- a/package.json +++ b/package.json @@ -100,6 +100,7 @@ "domhandler": "^4.3.0", "expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#ee14b3255da33d2b6924c357f43393251b6dc6d2", "fbjs": "^3.0.2", + "focus-trap-react": "^10.2.1", "htmlparser2": "^7.2.0", "idb-keyval": "^6.2.1", "jest-when": "^3.5.2", diff --git a/src/components/ConfirmModal.js b/src/components/ConfirmModal.js index 3fe3838c8c8..7c720c4bd68 100755 --- a/src/components/ConfirmModal.js +++ b/src/components/ConfirmModal.js @@ -98,6 +98,7 @@ function ConfirmModal(props) { shouldSetModalVisibility={props.shouldSetModalVisibility} onModalHide={props.onModalHide} type={props.isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED : CONST.MODAL.MODAL_TYPE.CONFIRM} + shouldEnableFocusTrap > (null); + + return isEnabled ? ( + (shouldEnableAutoFocus && ref.current) ?? false, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + fallbackFocus: () => ref.current!, + clickOutsideDeactivates: true, + }} + > + + + ) : ( + props.children + ); +} + +FocusTrapView.displayName = 'FocusTrapView'; + +export default FocusTrapView; diff --git a/src/components/FocusTrapView/types.ts b/src/components/FocusTrapView/types.ts new file mode 100644 index 00000000000..500b4b4315d --- /dev/null +++ b/src/components/FocusTrapView/types.ts @@ -0,0 +1,21 @@ +import {ViewProps} from 'react-native'; +import ChildrenProps from '@src/types/utils/ChildrenProps'; + +type FocusTrapViewProps = ChildrenProps & { + /** + * Whether to enable the FocusTrap. + * If the FocusTrap is disabled, we just pass the children through. + */ + isEnabled?: boolean; + + /** + * Whether to disable auto focus + * It is used when the component inside the FocusTrap have their own auto focus logic + */ + shouldEnableAutoFocus?: boolean; + + /** Whether the FocusTrap is active (listening for events) */ + isActive?: boolean; +} & ViewProps; + +export default FocusTrapViewProps; diff --git a/src/components/Modal/index.tsx b/src/components/Modal/index.tsx index bfc899f9c27..f9bd7d21283 100644 --- a/src/components/Modal/index.tsx +++ b/src/components/Modal/index.tsx @@ -1,13 +1,16 @@ import React, {useState} from 'react'; +import FocusTrapView from '@components/FocusTrapView'; import withWindowDimensions from '@components/withWindowDimensions'; import StatusBar from '@libs/StatusBar'; import useTheme from '@styles/themes/useTheme'; import useStyleUtils from '@styles/useStyleUtils'; +import useThemeStyles from '@styles/useThemeStyles'; import CONST from '@src/CONST'; import BaseModal from './BaseModal'; import BaseModalProps from './types'; -function Modal({fullscreen = true, onModalHide = () => {}, type, onModalShow = () => {}, children, ...rest}: BaseModalProps) { +function Modal({fullscreen = true, onModalHide = () => {}, type, onModalShow = () => {}, children, shouldEnableFocusTrap = false, ...rest}: BaseModalProps) { + const styles = useThemeStyles(); const theme = useTheme(); const StyleUtils = useStyleUtils(); const [previousStatusBarColor, setPreviousStatusBarColor] = useState(); @@ -49,7 +52,13 @@ function Modal({fullscreen = true, onModalHide = () => {}, type, onModalShow = ( fullscreen={fullscreen} type={type} > - {children} + + {children} + ); } diff --git a/src/components/Modal/modalPropTypes.js b/src/components/Modal/modalPropTypes.js index 84e610b694e..7465e28b28a 100644 --- a/src/components/Modal/modalPropTypes.js +++ b/src/components/Modal/modalPropTypes.js @@ -66,6 +66,9 @@ const propTypes = { * */ hideModalContentWhileAnimating: PropTypes.bool, + /** Should the modal use custom focus trap logic */ + shouldEnableFocusTrap: PropTypes.bool, + ...windowDimensionsPropTypes, }; @@ -84,6 +87,7 @@ const defaultProps = { statusBarTranslucent: true, avoidKeyboard: false, hideModalContentWhileAnimating: false, + shouldEnableFocusTrap: false, }; export {propTypes, defaultProps}; diff --git a/src/components/Modal/types.ts b/src/components/Modal/types.ts index 172965b45fb..609cefc16bf 100644 --- a/src/components/Modal/types.ts +++ b/src/components/Modal/types.ts @@ -61,6 +61,9 @@ type BaseModalProps = WindowDimensionsProps & * See: https://github.com/react-native-modal/react-native-modal/pull/116 * */ hideModalContentWhileAnimating?: boolean; + + /** Whether the modal should use focus trap */ + shouldEnableFocusTrap?: boolean; }; export default BaseModalProps; diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index 6af67c51ffa..2e504c6fb8f 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -1,10 +1,11 @@ -import {useNavigation} from '@react-navigation/native'; +import {useIsFocused, useNavigation} from '@react-navigation/native'; import lodashGet from 'lodash/get'; import React, {useEffect, useRef, useState} from 'react'; import {Keyboard, PanResponder, View} from 'react-native'; import {PickerAvoidingView} from 'react-native-picker-select'; import _ from 'underscore'; import CustomDevMenu from '@components/CustomDevMenu'; +import FocusTrapView from '@components/FocusTrapView'; import HeaderGap from '@components/HeaderGap'; import KeyboardAvoidingView from '@components/KeyboardAvoidingView'; import OfflineIndicator from '@components/OfflineIndicator'; @@ -39,6 +40,8 @@ const ScreenWrapper = React.forwardRef( shouldDismissKeyboardBeforeClose, onEntryTransitionEnd, testID, + shouldDisableFocusTrap, + shouldEnableAutoFocus, }, ref, ) => { @@ -49,6 +52,7 @@ const ScreenWrapper = React.forwardRef( const {isDevelopment} = useEnvironment(); const {isOffline} = useNetwork(); const navigation = useNavigation(); + const isFocused = useIsFocused(); const [didScreenTransitionEnd, setDidScreenTransitionEnd] = useState(false); const maxHeight = shouldEnableMaxHeight ? windowHeight : undefined; const minHeight = shouldEnableMinHeight && !Browser.isSafari() ? initialHeight : undefined; @@ -147,20 +151,27 @@ const ScreenWrapper = React.forwardRef( style={styles.flex1} enabled={shouldEnablePickerAvoiding} > - - {isDevelopment && } - {isDevelopment && } - { - // If props.children is a function, call it to provide the insets to the children. - _.isFunction(children) - ? children({ - insets, - safeAreaPaddingBottomStyle, - didScreenTransitionEnd, - }) - : children - } - {isSmallScreenWidth && shouldShowOfflineIndicator && } + + + {isDevelopment && } + {isDevelopment && } + { + // If props.children is a function, call it to provide the insets to the children. + _.isFunction(children) + ? children({ + insets, + safeAreaPaddingBottomStyle, + didScreenTransitionEnd, + }) + : children + } + {isSmallScreenWidth && shouldShowOfflineIndicator && } + diff --git a/src/components/ScreenWrapper/propTypes.js b/src/components/ScreenWrapper/propTypes.js index c98968bb112..8984c860a15 100644 --- a/src/components/ScreenWrapper/propTypes.js +++ b/src/components/ScreenWrapper/propTypes.js @@ -48,6 +48,12 @@ const propTypes = { /** Styles for the offline indicator */ offlineIndicatorStyle: stylePropTypes, + + /** Whether to disable the focus trap */ + shouldDisableFocusTrap: PropTypes.bool, + + /** Whether to disable auto focus of the focus trap */ + shouldEnableAutoFocus: PropTypes.bool, }; const defaultProps = { @@ -63,6 +69,8 @@ const defaultProps = { shouldShowOfflineIndicator: true, offlineIndicatorStyle: [], headerGapStyles: [], + shouldDisableFocusTrap: false, + shouldEnableAutoFocus: false, }; export {propTypes, defaultProps}; diff --git a/src/pages/ProfilePage.js b/src/pages/ProfilePage.js index 97ec3f99da3..2e2c5d1b2b2 100755 --- a/src/pages/ProfilePage.js +++ b/src/pages/ProfilePage.js @@ -147,7 +147,10 @@ function ProfilePage(props) { }, [accountID, hasMinimumDetails]); return ( - + Navigation.goBack(navigateBackTo)} diff --git a/src/pages/ReportDetailsPage.js b/src/pages/ReportDetailsPage.js index c5e6bf43fa1..3dd3da02b21 100644 --- a/src/pages/ReportDetailsPage.js +++ b/src/pages/ReportDetailsPage.js @@ -175,7 +175,10 @@ function ReportDetailsPage(props) { ) : null; return ( - + {({insets}) => ( <> diff --git a/src/pages/iou/SplitBillDetailsPage.js b/src/pages/iou/SplitBillDetailsPage.js index 65bf43500f8..661410483d3 100644 --- a/src/pages/iou/SplitBillDetailsPage.js +++ b/src/pages/iou/SplitBillDetailsPage.js @@ -110,7 +110,10 @@ function SplitBillDetailsPage(props) { ); return ( - + diff --git a/src/pages/settings/Security/TwoFactorAuth/TwoFactorAuthSteps.js b/src/pages/settings/Security/TwoFactorAuth/TwoFactorAuthSteps.js index 720c2e02b3c..61d7c69ba19 100644 --- a/src/pages/settings/Security/TwoFactorAuth/TwoFactorAuthSteps.js +++ b/src/pages/settings/Security/TwoFactorAuth/TwoFactorAuthSteps.js @@ -1,6 +1,6 @@ import {useRoute} from '@react-navigation/native'; import lodashGet from 'lodash/get'; -import React, {useCallback, useEffect, useMemo, useState} from 'react'; +import React, {useCallback, useEffect, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import useAnimatedStepContext from '@components/AnimatedStep/useAnimatedStepContext'; import * as TwoFactorAuthActions from '@userActions/TwoFactorAuthActions'; @@ -17,30 +17,20 @@ import {defaultAccount, TwoFactorAuthPropTypes} from './TwoFactorAuthPropTypes'; function TwoFactorAuthSteps({account = defaultAccount}) { const route = useRoute(); const backTo = lodashGet(route.params, 'backTo', ''); - const [currentStep, setCurrentStep] = useState(CONST.TWO_FACTOR_AUTH_STEPS.CODES); - - const {setAnimationDirection} = useAnimatedStepContext(); - - useEffect(() => () => TwoFactorAuthActions.clearTwoFactorAuthData(), []); - - useEffect(() => { + const currentStep = useMemo(() => { if (account.twoFactorAuthStep) { - setCurrentStep(account.twoFactorAuthStep); - return; - } - - if (account.requiresTwoFactorAuth) { - setCurrentStep(CONST.TWO_FACTOR_AUTH_STEPS.ENABLED); - } else { - setCurrentStep(CONST.TWO_FACTOR_AUTH_STEPS.CODES); + return account.twoFactorAuthStep; } + return account.requiresTwoFactorAuth ? CONST.TWO_FACTOR_AUTH_STEPS.ENABLED : CONST.TWO_FACTOR_AUTH_STEPS.CODES; }, [account.requiresTwoFactorAuth, account.twoFactorAuthStep]); + const {setAnimationDirection} = useAnimatedStepContext(); + + useEffect(() => () => TwoFactorAuthActions.clearTwoFactorAuthData(), []); const handleSetStep = useCallback( (step, animationDirection = CONST.ANIMATION_DIRECTION.IN) => { setAnimationDirection(animationDirection); TwoFactorAuthActions.setTwoFactorAuthStep(step); - setCurrentStep(step); }, [setAnimationDirection], ); From dff444b078a722ed13dd3e028d2c49f7535602f6 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:20:25 +0100 Subject: [PATCH 2/7] update focus-trap-react --- package-lock.json | 30 +++++++++++++++--------------- package.json | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/package-lock.json b/package-lock.json index 65dee8cde80..ccc2ac561ee 100644 --- a/package-lock.json +++ b/package-lock.json @@ -52,7 +52,7 @@ "domhandler": "^4.3.0", "expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#ee14b3255da33d2b6924c357f43393251b6dc6d2", "fbjs": "^3.0.2", - "focus-trap-react": "^10.2.1", + "focus-trap-react": "^10.2.3", "htmlparser2": "^7.2.0", "idb-keyval": "^6.2.1", "jest-when": "^3.5.2", @@ -30778,19 +30778,19 @@ } }, "node_modules/focus-trap": { - "version": "7.5.2", - "resolved": "https://registry.npmjs.org/focus-trap/-/focus-trap-7.5.2.tgz", - "integrity": "sha512-p6vGNNWLDGwJCiEjkSK6oERj/hEyI9ITsSwIUICBoKLlWiTWXJRfQibCwcoi50rTZdbi87qDtUlMCmQwsGSgPw==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/focus-trap/-/focus-trap-7.5.4.tgz", + "integrity": "sha512-N7kHdlgsO/v+iD/dMoJKtsSqs5Dz/dXZVebRgJw23LDk+jMi/974zyiOYDziY2JPp8xivq9BmUGwIJMiuSBi7w==", "dependencies": { "tabbable": "^6.2.0" } }, "node_modules/focus-trap-react": { - "version": "10.2.1", - "resolved": "https://registry.npmjs.org/focus-trap-react/-/focus-trap-react-10.2.1.tgz", - "integrity": "sha512-UrAKOn52lvfHF6lkUMfFhlQxFgahyNW5i6FpHWkDxAeD4FSk3iwx9n4UEA4Sims0G5WiGIi0fAyoq3/UVeNCYA==", + "version": "10.2.3", + "resolved": "https://registry.npmjs.org/focus-trap-react/-/focus-trap-react-10.2.3.tgz", + "integrity": "sha512-YXBpFu/hIeSu6NnmV2xlXzOYxuWkoOtar9jzgp3lOmjWLWY59C/b8DtDHEAV4SPU07Nd/t+nS/SBNGkhUBFmEw==", "dependencies": { - "focus-trap": "^7.5.2", + "focus-trap": "^7.5.4", "tabbable": "^6.2.0" }, "peerDependencies": { @@ -75090,19 +75090,19 @@ } }, "focus-trap": { - "version": "7.5.2", - "resolved": "https://registry.npmjs.org/focus-trap/-/focus-trap-7.5.2.tgz", - "integrity": "sha512-p6vGNNWLDGwJCiEjkSK6oERj/hEyI9ITsSwIUICBoKLlWiTWXJRfQibCwcoi50rTZdbi87qDtUlMCmQwsGSgPw==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/focus-trap/-/focus-trap-7.5.4.tgz", + "integrity": "sha512-N7kHdlgsO/v+iD/dMoJKtsSqs5Dz/dXZVebRgJw23LDk+jMi/974zyiOYDziY2JPp8xivq9BmUGwIJMiuSBi7w==", "requires": { "tabbable": "^6.2.0" } }, "focus-trap-react": { - "version": "10.2.1", - "resolved": "https://registry.npmjs.org/focus-trap-react/-/focus-trap-react-10.2.1.tgz", - "integrity": "sha512-UrAKOn52lvfHF6lkUMfFhlQxFgahyNW5i6FpHWkDxAeD4FSk3iwx9n4UEA4Sims0G5WiGIi0fAyoq3/UVeNCYA==", + "version": "10.2.3", + "resolved": "https://registry.npmjs.org/focus-trap-react/-/focus-trap-react-10.2.3.tgz", + "integrity": "sha512-YXBpFu/hIeSu6NnmV2xlXzOYxuWkoOtar9jzgp3lOmjWLWY59C/b8DtDHEAV4SPU07Nd/t+nS/SBNGkhUBFmEw==", "requires": { - "focus-trap": "^7.5.2", + "focus-trap": "^7.5.4", "tabbable": "^6.2.0" } }, diff --git a/package.json b/package.json index d75d7c96487..385dd887db2 100644 --- a/package.json +++ b/package.json @@ -100,7 +100,7 @@ "domhandler": "^4.3.0", "expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#ee14b3255da33d2b6924c357f43393251b6dc6d2", "fbjs": "^3.0.2", - "focus-trap-react": "^10.2.1", + "focus-trap-react": "^10.2.3", "htmlparser2": "^7.2.0", "idb-keyval": "^6.2.1", "jest-when": "^3.5.2", From e1691d103751dd5e96f3ca37a2c4a72c25d38ab5 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:39:42 +0100 Subject: [PATCH 3/7] fix regressions --- src/components/FocusTrapView/index.tsx | 3 ++- src/components/FocusTrapView/types.ts | 6 ++++++ src/components/ScreenWrapper/index.js | 2 ++ src/components/ScreenWrapper/propTypes.js | 4 ++++ src/pages/iou/IOUCurrencySelection.js | 1 + src/pages/workspace/WorkspacePageWithSections.js | 1 + 6 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/components/FocusTrapView/index.tsx b/src/components/FocusTrapView/index.tsx index 6b52512c2e6..ba7f774dc65 100644 --- a/src/components/FocusTrapView/index.tsx +++ b/src/components/FocusTrapView/index.tsx @@ -7,7 +7,7 @@ import {View} from 'react-native'; import viewRef from '@src/types/utils/viewRef'; import FocusTrapViewProps from './types'; -function FocusTrapView({isEnabled = true, isActive = true, shouldEnableAutoFocus = false, ...props}: FocusTrapViewProps) { +function FocusTrapView({isEnabled = true, isActive = true, shouldEnableAutoFocus = false, shouldReturnFocusOnDeactivate = true, ...props}: FocusTrapViewProps) { /** * Focus trap always needs a focusable element. * In case that we don't have any focusable elements in the modal, @@ -23,6 +23,7 @@ function FocusTrapView({isEnabled = true, isActive = true, shouldEnableAutoFocus // eslint-disable-next-line @typescript-eslint/no-non-null-assertion fallbackFocus: () => ref.current!, clickOutsideDeactivates: true, + returnFocusOnDeactivate: shouldReturnFocusOnDeactivate, }} > { @@ -155,6 +156,7 @@ const ScreenWrapper = React.forwardRef( style={[styles.flex1, styles.noSelect]} isEnabled={!shouldDisableFocusTrap} shouldEnableAutoFocus={shouldEnableAutoFocus} + shouldReturnFocusOnDeactivate={shouldReturnFocusOnDeactivate} isActive={isFocused} > diff --git a/src/components/ScreenWrapper/propTypes.js b/src/components/ScreenWrapper/propTypes.js index 8984c860a15..46da8c55ee7 100644 --- a/src/components/ScreenWrapper/propTypes.js +++ b/src/components/ScreenWrapper/propTypes.js @@ -54,6 +54,9 @@ const propTypes = { /** Whether to disable auto focus of the focus trap */ shouldEnableAutoFocus: PropTypes.bool, + + /** Whether to return focus on deactivate of the focus trap */ + shouldReturnFocusOnDeactivate: PropTypes.bool, }; const defaultProps = { @@ -71,6 +74,7 @@ const defaultProps = { headerGapStyles: [], shouldDisableFocusTrap: false, shouldEnableAutoFocus: false, + shouldReturnFocusOnDeactivate: true, }; export {propTypes, defaultProps}; diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index eab9ab5a751..b6004a7c500 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -156,6 +156,7 @@ function IOUCurrencySelection(props) { includeSafeAreaPaddingBottom={false} onEntryTransitionEnd={() => optionsSelectorRef.current && optionsSelectorRef.current.focus()} testID={IOUCurrencySelection.displayName} + shouldReturnFocusOnDeactivate={false} > {({didScreenTransitionEnd}) => ( <> diff --git a/src/pages/workspace/WorkspacePageWithSections.js b/src/pages/workspace/WorkspacePageWithSections.js index bea94ed3ef4..b77f1b9f06c 100644 --- a/src/pages/workspace/WorkspacePageWithSections.js +++ b/src/pages/workspace/WorkspacePageWithSections.js @@ -104,6 +104,7 @@ function WorkspacePageWithSections({backButtonRoute, children, footer, guidesCal shouldEnablePickerAvoiding={false} shouldEnableMaxHeight testID={WorkspacePageWithSections.displayName} + shouldReturnFocusOnDeactivate={false} > Navigation.goBack(ROUTES.SETTINGS_WORKSPACES)} From f3817cfbd8a0bb68f4217af3852134e29c48cf7f Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 18 Dec 2023 11:00:48 +0100 Subject: [PATCH 4/7] fix another issues --- src/pages/SearchPage.js | 1 + src/pages/iou/request/step/IOURequestStepCurrency.js | 1 + src/pages/iou/request/step/StepScreenWrapper.js | 7 ++++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index 5d111e7c181..0f712a18216 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -183,6 +183,7 @@ class SearchPage extends Component { includeSafeAreaPaddingBottom={false} testID={SearchPage.displayName} onEntryTransitionEnd={this.updateOptions} + shouldReturnFocusOnDeactivate={false} > {({didScreenTransitionEnd, safeAreaPaddingBottomStyle}) => ( <> diff --git a/src/pages/iou/request/step/IOURequestStepCurrency.js b/src/pages/iou/request/step/IOURequestStepCurrency.js index b4281de4d16..275538adcd3 100644 --- a/src/pages/iou/request/step/IOURequestStepCurrency.js +++ b/src/pages/iou/request/step/IOURequestStepCurrency.js @@ -128,6 +128,7 @@ function IOURequestStepCurrency({ onEntryTransitionEnd={() => optionsSelectorRef.current && optionsSelectorRef.current.focus()} shouldShowWrapper testID={IOURequestStepCurrency.displayName} + shouldReturnFocusOnDeactivate={false} > {({didScreenTransitionEnd}) => ( {}, includeSafeAreaPaddingBottom: false, + shouldReturnFocusOnDeactivate: true, }; -function StepScreenWrapper({testID, headerTitle, onBackButtonPress, onEntryTransitionEnd, children, shouldShowWrapper, includeSafeAreaPaddingBottom}) { +function StepScreenWrapper({testID, headerTitle, onBackButtonPress, onEntryTransitionEnd, children, shouldShowWrapper, includeSafeAreaPaddingBottom, shouldReturnFocusOnDeactivate}) { const styles = useThemeStyles(); if (!shouldShowWrapper) { @@ -48,6 +52,7 @@ function StepScreenWrapper({testID, headerTitle, onBackButtonPress, onEntryTrans onEntryTransitionEnd={onEntryTransitionEnd} testID={testID} shouldEnableMaxHeight={DeviceCapabilities.canUseTouchScreen()} + shouldReturnFocusOnDeactivate={shouldReturnFocusOnDeactivate} > {({insets, safeAreaPaddingBottomStyle, didScreenTransitionEnd}) => ( From 469188d5c40771b8fca8b796f7ca1b55186f894a Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 8 Jan 2024 17:08:09 +0100 Subject: [PATCH 5/7] fix types --- src/components/FocusTrapView/index.native.tsx | 4 ++-- src/components/FocusTrapView/index.tsx | 2 +- src/components/FocusTrapView/types.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/FocusTrapView/index.native.tsx b/src/components/FocusTrapView/index.native.tsx index 1190cfda415..4c0b704e275 100644 --- a/src/components/FocusTrapView/index.native.tsx +++ b/src/components/FocusTrapView/index.native.tsx @@ -1,8 +1,8 @@ +import type FocusTrapViewProps from './types'; + /* * The FocusTrap is only used on web and desktop */ -import FocusTrapViewProps from './types'; - function FocusTrapView({children}: FocusTrapViewProps) { return children; } diff --git a/src/components/FocusTrapView/index.tsx b/src/components/FocusTrapView/index.tsx index ba7f774dc65..eef367aa063 100644 --- a/src/components/FocusTrapView/index.tsx +++ b/src/components/FocusTrapView/index.tsx @@ -5,7 +5,7 @@ import FocusTrap from 'focus-trap-react'; import React, {useRef} from 'react'; import {View} from 'react-native'; import viewRef from '@src/types/utils/viewRef'; -import FocusTrapViewProps from './types'; +import type FocusTrapViewProps from './types'; function FocusTrapView({isEnabled = true, isActive = true, shouldEnableAutoFocus = false, shouldReturnFocusOnDeactivate = true, ...props}: FocusTrapViewProps) { /** diff --git a/src/components/FocusTrapView/types.ts b/src/components/FocusTrapView/types.ts index 7a8f5b04698..a3eb55a5294 100644 --- a/src/components/FocusTrapView/types.ts +++ b/src/components/FocusTrapView/types.ts @@ -1,5 +1,5 @@ -import {ViewProps} from 'react-native'; -import ChildrenProps from '@src/types/utils/ChildrenProps'; +import type {ViewProps} from 'react-native'; +import type ChildrenProps from '@src/types/utils/ChildrenProps'; type FocusTrapViewProps = ChildrenProps & { /** From b9ec1e8280032ba0da74d00df658f965b2625750 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 10 Jan 2024 15:55:00 +0100 Subject: [PATCH 6/7] address review --- src/components/ScreenWrapper.tsx | 6 +++--- src/pages/home/ReportScreen.js | 2 +- src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/ScreenWrapper.tsx b/src/components/ScreenWrapper.tsx index 6cf47a51a6f..caa8c189e53 100644 --- a/src/components/ScreenWrapper.tsx +++ b/src/components/ScreenWrapper.tsx @@ -89,7 +89,7 @@ type ScreenWrapperProps = { navigation?: StackNavigationProp; /** Whether to disable the focus trap */ - shouldDisableFocusTrap?: boolean; + shouldEnableFocusTrap?: boolean; /** Whether to disable auto focus of the focus trap */ shouldEnableAutoFocus?: boolean; @@ -116,7 +116,7 @@ function ScreenWrapper( onEntryTransitionEnd, testID, navigation: navigationProp, - shouldDisableFocusTrap = false, + shouldEnableFocusTrap = true, shouldEnableAutoFocus = false, shouldReturnFocusOnDeactivate = true, }: ScreenWrapperProps, @@ -240,7 +240,7 @@ function ScreenWrapper( > {({insets}) => ( <> From 14799fcdaf490e4c267e5fa2b3afab93c8ef6ec9 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Fri, 2 Feb 2024 11:20:52 +0100 Subject: [PATCH 7/7] fix merge --- src/components/ScreenWrapper.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/components/ScreenWrapper.tsx b/src/components/ScreenWrapper.tsx index 6310037a5a7..6f106c50859 100644 --- a/src/components/ScreenWrapper.tsx +++ b/src/components/ScreenWrapper.tsx @@ -263,6 +263,12 @@ function ScreenWrapper( : children } {isSmallScreenWidth && shouldShowOfflineIndicator && } + {!isSmallScreenWidth && shouldShowOfflineIndicatorInWideScreen && ( + + )}