From c2de4f0677d2ebfb3f5d348a5cade98b566902c3 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 9 Aug 2023 09:20:17 +0200 Subject: [PATCH 01/16] Add focus-trap-react library --- package-lock.json | 50 +++++++++++++++++++++++++++++++++++++++++++++++ package.json | 5 +++-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index df746a65f3ae..7162d7cfdc9b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44,6 +44,7 @@ "domhandler": "^4.3.0", "expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#9940dd127c2d44809c98ee628a8057f08c93bfc9", "fbjs": "^3.0.2", + "focus-trap-react": "^10.2.1", "htmlparser2": "^7.2.0", "jest-when": "^3.5.2", "localforage": "^1.10.0", @@ -29994,6 +29995,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.1", "dev": true, @@ -46743,6 +46766,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.0", "dev": true, @@ -70539,6 +70567,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.1", "dev": true @@ -81799,6 +81844,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.0", "dev": true, diff --git a/package.json b/package.json index 56ac7926f0e5..47733b5500c1 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "domhandler": "^4.3.0", "expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#9940dd127c2d44809c98ee628a8057f08c93bfc9", "fbjs": "^3.0.2", + "focus-trap-react": "^10.2.1", "htmlparser2": "^7.2.0", "jest-when": "^3.5.2", "localforage": "^1.10.0", @@ -156,8 +157,8 @@ "@babel/preset-env": "^7.20.0", "@babel/preset-flow": "^7.12.13", "@babel/preset-react": "^7.10.4", - "@babel/runtime": "^7.20.0", "@babel/preset-typescript": "^7.21.5", + "@babel/runtime": "^7.20.0", "@electron/notarize": "^1.2.3", "@jest/globals": "^29.5.0", "@octokit/core": "4.0.4", @@ -177,12 +178,12 @@ "@svgr/webpack": "^6.0.0", "@testing-library/jest-native": "5.4.1", "@testing-library/react-native": "11.5.1", - "@types/metro-config": "^0.76.3", "@types/concurrently": "^7.0.0", "@types/jest": "^29.5.2", "@types/jest-when": "^3.5.2", "@types/js-yaml": "^4.0.5", "@types/lodash": "^4.14.195", + "@types/metro-config": "^0.76.3", "@types/mock-fs": "^4.13.1", "@types/pusher-js": "^5.1.0", "@types/react": "^18.2.12", From ca6bfc4a2fd2537ebcd32a2b5820df7a03fc64b0 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 9 Aug 2023 09:29:37 +0200 Subject: [PATCH 02/16] Add focus trap to Screen Wrapper --- src/components/ScreenWrapper/index.js | 68 +++++++++++++---------- src/components/ScreenWrapper/propTypes.js | 4 ++ 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index 8cd4206886f8..6fa0f7e254a6 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -3,6 +3,7 @@ import React from 'react'; import _ from 'underscore'; import lodashGet from 'lodash/get'; import {PickerAvoidingView} from 'react-native-picker-select'; +import FocusTrap from 'focus-trap-react'; import KeyboardAvoidingView from '../KeyboardAvoidingView'; import CONST from '../../CONST'; import styles from '../../styles/styles'; @@ -32,6 +33,8 @@ class ScreenWrapper extends React.Component { this.state = { didScreenTransitionEnd: false, }; + + this.focusRef = React.createRef(); } componentDidMount() { @@ -95,37 +98,46 @@ class ScreenWrapper extends React.Component { } return ( - this.focusRef.current, + clickOutsideDeactivates: true, + }} > - - - - {this.props.environment === CONST.ENVIRONMENT.DEV && } - {this.props.environment === CONST.ENVIRONMENT.DEV && } - { - // If props.children is a function, call it to provide the insets to the children. - _.isFunction(this.props.children) - ? this.props.children({ - insets, - safeAreaPaddingBottomStyle, - didScreenTransitionEnd: this.state.didScreenTransitionEnd, - }) - : this.props.children - } - {this.props.isSmallScreenWidth && this.props.shouldShowOfflineIndicator && } - - - + + + {this.props.environment === CONST.ENVIRONMENT.DEV && } + {this.props.environment === CONST.ENVIRONMENT.DEV && } + { + // If props.children is a function, call it to provide the insets to the children. + _.isFunction(this.props.children) + ? this.props.children({ + insets, + safeAreaPaddingBottomStyle, + didScreenTransitionEnd: this.state.didScreenTransitionEnd, + }) + : this.props.children + } + {this.props.isSmallScreenWidth && this.props.shouldShowOfflineIndicator && } + + + + ); }} diff --git a/src/components/ScreenWrapper/propTypes.js b/src/components/ScreenWrapper/propTypes.js index 7162ca074f43..ce6368056d47 100644 --- a/src/components/ScreenWrapper/propTypes.js +++ b/src/components/ScreenWrapper/propTypes.js @@ -45,6 +45,9 @@ const propTypes = { /** Styles for the offline indicator */ offlineIndicatorStyle: stylePropTypes, + + /** Whether to disable focus trap */ + shouldDisableFocusTrap: PropTypes.bool, }; const defaultProps = { @@ -59,6 +62,7 @@ const defaultProps = { shouldEnablePickerAvoiding: true, shouldShowOfflineIndicator: true, offlineIndicatorStyle: [], + shouldDisableFocusTrap: false, }; export {propTypes, defaultProps}; From 9e629a673c45291342d6d6cdef9b8dc165dbfecc Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 9 Aug 2023 11:14:45 +0200 Subject: [PATCH 03/16] Disable focus-trap where it isn't needed --- src/pages/home/ReportScreen.js | 1 + src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js | 1 + 2 files changed, 2 insertions(+) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 94fe5e2beb00..15603f013aea 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -313,6 +313,7 @@ class ReportScreen extends React.Component { {({insets}) => ( <> From 1d401a513e57a234dccd66b36722f26fc56d9443 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 9 Aug 2023 12:01:50 +0200 Subject: [PATCH 04/16] Create FocusTrapView, to make it work on the native --- src/components/FocusTrapView/index.js | 42 +++++++++++++++++++ src/components/FocusTrapView/index.native.js | 18 ++++++++ src/components/ScreenWrapper/index.js | 43 ++++++++------------ 3 files changed, 77 insertions(+), 26 deletions(-) create mode 100644 src/components/FocusTrapView/index.js create mode 100644 src/components/FocusTrapView/index.native.js diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js new file mode 100644 index 000000000000..1e36550fcf46 --- /dev/null +++ b/src/components/FocusTrapView/index.js @@ -0,0 +1,42 @@ +/* + * The FocusTrap is only used on web and desktop + */ +import React, {useRef} from 'react'; +import FocusTrap from 'focus-trap-react'; +import {View} from 'react-native'; +import { PropTypes } from 'prop-types'; + +const propTypes = { + /** Whether or not to disable the FocusTrap */ + shouldDisableFocusTrap: PropTypes.bool, +}; + +const defaultProps = { + shouldDisableFocusTrap: false, +}; + +function FocusTrapView({shouldDisableFocusTrap, ...props}) { + const ref = useRef(null); + + return ( + ref.current, + clickOutsideDeactivates: true, + }} + > + + + ); +} + +FocusTrapView.displayName = 'FocusTrapView'; +FocusTrapView.propTypes = propTypes; +FocusTrapView.defaultProps = defaultProps; + +export default FocusTrapView; diff --git a/src/components/FocusTrapView/index.native.js b/src/components/FocusTrapView/index.native.js new file mode 100644 index 000000000000..cd9647808141 --- /dev/null +++ b/src/components/FocusTrapView/index.native.js @@ -0,0 +1,18 @@ +/* + * The FocusTrap is only used on web and desktop + */ +import React from 'react'; +import {View} from 'react-native'; +import _ from 'underscore'; + +function FocusTrapView(props) { + const viewProps = _.omit(props, ['shouldDisableFocusTrap']); + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + + ); +} + +FocusTrapView.displayName = 'FocusTrapView'; + +export default FocusTrapView; diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index 6fa0f7e254a6..0b1f8a7ceada 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -3,7 +3,7 @@ import React from 'react'; import _ from 'underscore'; import lodashGet from 'lodash/get'; import {PickerAvoidingView} from 'react-native-picker-select'; -import FocusTrap from 'focus-trap-react'; +import FocusTrapView from '../FocusTrapView'; import KeyboardAvoidingView from '../KeyboardAvoidingView'; import CONST from '../../CONST'; import styles from '../../styles/styles'; @@ -33,8 +33,6 @@ class ScreenWrapper extends React.Component { this.state = { didScreenTransitionEnd: false, }; - - this.focusRef = React.createRef(); } componentDidMount() { @@ -98,28 +96,21 @@ class ScreenWrapper extends React.Component { } return ( - this.focusRef.current, - clickOutsideDeactivates: true, - }} + - - - + {this.props.environment === CONST.ENVIRONMENT.DEV && } {this.props.environment === CONST.ENVIRONMENT.DEV && } @@ -134,10 +125,10 @@ class ScreenWrapper extends React.Component { : this.props.children } {this.props.isSmallScreenWidth && this.props.shouldShowOfflineIndicator && } - - - - + + + + ); }} From bb3450bf26020021f5db0220796c0049a54a462d Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 9 Aug 2023 12:28:24 +0200 Subject: [PATCH 05/16] Disable initial focus & refactor --- src/components/FocusTrapView/index.js | 9 +++++---- src/components/FocusTrapView/index.native.js | 2 +- src/components/ScreenWrapper/index.js | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index 1e36550fcf46..f60f9c8078d4 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -8,20 +8,21 @@ import { PropTypes } from 'prop-types'; const propTypes = { /** Whether or not to disable the FocusTrap */ - shouldDisableFocusTrap: PropTypes.bool, + enabled: PropTypes.bool, }; const defaultProps = { - shouldDisableFocusTrap: false, + enabled: true, }; -function FocusTrapView({shouldDisableFocusTrap, ...props}) { +function FocusTrapView({enabled, ...props}) { const ref = useRef(null); return ( ref.current, clickOutsideDeactivates: true, }} diff --git a/src/components/FocusTrapView/index.native.js b/src/components/FocusTrapView/index.native.js index cd9647808141..62a7591f0a08 100644 --- a/src/components/FocusTrapView/index.native.js +++ b/src/components/FocusTrapView/index.native.js @@ -6,7 +6,7 @@ import {View} from 'react-native'; import _ from 'underscore'; function FocusTrapView(props) { - const viewProps = _.omit(props, ['shouldDisableFocusTrap']); + const viewProps = _.omit(props, ['enabled']); return ( // eslint-disable-next-line react/jsx-props-no-spreading diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index 0b1f8a7ceada..1a43c6348531 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -110,7 +110,7 @@ class ScreenWrapper extends React.Component { style={styles.flex1} enabled={this.props.shouldEnablePickerAvoiding} > - + {this.props.environment === CONST.ENVIRONMENT.DEV && } {this.props.environment === CONST.ENVIRONMENT.DEV && } From b2958f55ebf7547a28a895f82353d5652646a69c Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 9 Aug 2023 14:19:29 +0200 Subject: [PATCH 06/16] Refactor, add comments --- src/components/FocusTrapView/index.js | 9 +++++++-- src/components/ScreenWrapper/index.js | 5 ++++- src/components/ScreenWrapper/propTypes.js | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index f60f9c8078d4..f9077d636be6 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -4,10 +4,10 @@ import React, {useRef} from 'react'; import FocusTrap from 'focus-trap-react'; import {View} from 'react-native'; -import { PropTypes } from 'prop-types'; +import {PropTypes} from 'prop-types'; const propTypes = { - /** Whether or not to disable the FocusTrap */ + /** Whether to enable the FocusTrap */ enabled: PropTypes.bool, }; @@ -16,6 +16,11 @@ const defaultProps = { }; function FocusTrapView({enabled, ...props}) { + /** + * Focus trap always needs a focusable element. + * In case that we don't have any focusable elements in the modal, + * the FocusTrap will use fallback View element using this ref. + */ const ref = useRef(null); return ( diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index 1a43c6348531..aa0aaf0d8988 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -110,7 +110,10 @@ class ScreenWrapper extends React.Component { style={styles.flex1} enabled={this.props.shouldEnablePickerAvoiding} > - + {this.props.environment === CONST.ENVIRONMENT.DEV && } {this.props.environment === CONST.ENVIRONMENT.DEV && } diff --git a/src/components/ScreenWrapper/propTypes.js b/src/components/ScreenWrapper/propTypes.js index ce6368056d47..6b000da8a22b 100644 --- a/src/components/ScreenWrapper/propTypes.js +++ b/src/components/ScreenWrapper/propTypes.js @@ -46,7 +46,7 @@ const propTypes = { /** Styles for the offline indicator */ offlineIndicatorStyle: stylePropTypes, - /** Whether to disable focus trap */ + /** Whether to disable the focus trap */ shouldDisableFocusTrap: PropTypes.bool, }; From 222d706184297364b74d5ecc21ccf3d8cbc3a8e7 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:20:59 +0200 Subject: [PATCH 07/16] Temporarily turn on the initial focus --- src/components/FocusTrapView/index.js | 2 +- src/components/HeaderWithBackButton/index.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index f9077d636be6..35285b570215 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -27,7 +27,7 @@ function FocusTrapView({enabled, ...props}) { ref.current, clickOutsideDeactivates: true, }} diff --git a/src/components/HeaderWithBackButton/index.js b/src/components/HeaderWithBackButton/index.js index 6e6164f4c4fc..52880a81dba0 100755 --- a/src/components/HeaderWithBackButton/index.js +++ b/src/components/HeaderWithBackButton/index.js @@ -64,7 +64,8 @@ function HeaderWithBackButton({ onBackButtonPress(); }} style={[styles.touchableButtonImage]} - accessibilityRole="button" + // Temporary solution to prevent Space from closing the RHP. + // accessibilityRole="button" accessibilityLabel={translate('common.back')} > Date: Thu, 10 Aug 2023 09:46:55 +0200 Subject: [PATCH 08/16] Update package.lock --- package-lock.json | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/package-lock.json b/package-lock.json index c6adf7c192b6..041343a19b1c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -144,6 +144,7 @@ "@types/jest-when": "^3.5.2", "@types/js-yaml": "^4.0.5", "@types/lodash": "^4.14.195", + "@types/metro-config": "^0.76.3", "@types/mock-fs": "^4.13.1", "@types/pusher-js": "^5.1.0", "@types/react": "^18.2.12", @@ -20107,6 +20108,16 @@ "integrity": "sha512-76CqzuD6Q7LC+AtbPqrvD9AqsN0k8bsYo2bM2J8pmNldP1aIPAbzUQ7QbobyXL4eLr1wK5x8FZFe8eF/ubRuBg==", "dev": true }, + "node_modules/@types/metro-config": { + "version": "0.76.3", + "resolved": "https://registry.npmjs.org/@types/metro-config/-/metro-config-0.76.3.tgz", + "integrity": "sha512-tq9IyKm1JzNRpM7V7LALxXiAyTlV5eZwoqntpnRIbnqDsIyNlnPkWQEoX77f40pzcUuAH4CiHRDxW5VskPlgtQ==", + "deprecated": "This is a stub types definition. metro-config provides its own type definitions, so you do not need this installed.", + "dev": true, + "dependencies": { + "metro-config": "*" + } + }, "node_modules/@types/mime": { "version": "3.0.1", "dev": true, @@ -63759,6 +63770,15 @@ "integrity": "sha512-76CqzuD6Q7LC+AtbPqrvD9AqsN0k8bsYo2bM2J8pmNldP1aIPAbzUQ7QbobyXL4eLr1wK5x8FZFe8eF/ubRuBg==", "dev": true }, + "@types/metro-config": { + "version": "0.76.3", + "resolved": "https://registry.npmjs.org/@types/metro-config/-/metro-config-0.76.3.tgz", + "integrity": "sha512-tq9IyKm1JzNRpM7V7LALxXiAyTlV5eZwoqntpnRIbnqDsIyNlnPkWQEoX77f40pzcUuAH4CiHRDxW5VskPlgtQ==", + "dev": true, + "requires": { + "metro-config": "*" + } + }, "@types/mime": { "version": "3.0.1", "dev": true From db68328827935ab857db9be69965ea9b891451f4 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Thu, 10 Aug 2023 10:33:49 +0200 Subject: [PATCH 09/16] Add tabindex property --- src/components/FocusTrapView/index.js | 14 +++++++++++--- src/components/HeaderWithBackButton/index.js | 3 +-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index 35285b570215..96ab17fa068c 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -1,7 +1,7 @@ /* * The FocusTrap is only used on web and desktop */ -import React, {useRef} from 'react'; +import React, {useEffect, useRef} from 'react'; import FocusTrap from 'focus-trap-react'; import {View} from 'react-native'; import {PropTypes} from 'prop-types'; @@ -23,12 +23,20 @@ function FocusTrapView({enabled, ...props}) { */ const ref = useRef(null); + /** + * We have to set the 'tabindex' attribute to 0 to make the View focusable. + * Currently, it is not possible to set this through props. + * After the upgrade of 'react-native-web' to version 0.19 we can use 'tabIndex={0}' prop instead. + */ + useEffect(() => { + ref.current.setAttribute('tabindex', '0'); + }, []); + return ( ref.current, + initialFocus: () => ref.current, clickOutsideDeactivates: true, }} > diff --git a/src/components/HeaderWithBackButton/index.js b/src/components/HeaderWithBackButton/index.js index 52880a81dba0..6e6164f4c4fc 100755 --- a/src/components/HeaderWithBackButton/index.js +++ b/src/components/HeaderWithBackButton/index.js @@ -64,8 +64,7 @@ function HeaderWithBackButton({ onBackButtonPress(); }} style={[styles.touchableButtonImage]} - // Temporary solution to prevent Space from closing the RHP. - // accessibilityRole="button" + accessibilityRole="button" accessibilityLabel={translate('common.back')} > Date: Thu, 10 Aug 2023 12:04:32 +0200 Subject: [PATCH 10/16] Add shouldDisableAutoFocus prop to FocusTrapView --- src/components/FocusTrapView/index.js | 11 +++++++++-- src/components/FocusTrapView/index.native.js | 2 +- src/components/ScreenWrapper/index.js | 1 + src/components/ScreenWrapper/propTypes.js | 4 ++++ src/pages/SearchPage.js | 5 ++++- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index 96ab17fa068c..4ab5228e303a 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -9,13 +9,19 @@ import {PropTypes} from 'prop-types'; const propTypes = { /** Whether to enable the FocusTrap */ enabled: PropTypes.bool, + + /** Whether to disable auto focus + * It is used when the component inside the FocusTrap have their own auto focus logic + */ + shouldDisableAutoFocus: PropTypes.bool, }; const defaultProps = { enabled: true, + shouldDisableAutoFocus: false, }; -function FocusTrapView({enabled, ...props}) { +function FocusTrapView({enabled, shouldDisableAutoFocus, ...props}) { /** * Focus trap always needs a focusable element. * In case that we don't have any focusable elements in the modal, @@ -36,7 +42,8 @@ function FocusTrapView({enabled, ...props}) { ref.current, + initialFocus: () => !shouldDisableAutoFocus && ref.current, + fallbackFocus: () => ref.current, clickOutsideDeactivates: true, }} > diff --git a/src/components/FocusTrapView/index.native.js b/src/components/FocusTrapView/index.native.js index 62a7591f0a08..2e02edeadb72 100644 --- a/src/components/FocusTrapView/index.native.js +++ b/src/components/FocusTrapView/index.native.js @@ -6,7 +6,7 @@ import {View} from 'react-native'; import _ from 'underscore'; function FocusTrapView(props) { - const viewProps = _.omit(props, ['enabled']); + const viewProps = _.omit(props, ['enabled', 'shouldDisableAutoFocus']); return ( // eslint-disable-next-line react/jsx-props-no-spreading diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index b66bf7fb4dc5..71c3659f73fa 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -128,6 +128,7 @@ class ScreenWrapper extends React.Component { {this.props.environment === CONST.ENVIRONMENT.DEV && } diff --git a/src/components/ScreenWrapper/propTypes.js b/src/components/ScreenWrapper/propTypes.js index 6b000da8a22b..6f2502935246 100644 --- a/src/components/ScreenWrapper/propTypes.js +++ b/src/components/ScreenWrapper/propTypes.js @@ -48,6 +48,9 @@ const propTypes = { /** Whether to disable the focus trap */ shouldDisableFocusTrap: PropTypes.bool, + + /** Whether to disable auto focus of the focus trap */ + shouldDisableAutoFocus: PropTypes.bool, }; const defaultProps = { @@ -63,6 +66,7 @@ const defaultProps = { shouldShowOfflineIndicator: true, offlineIndicatorStyle: [], shouldDisableFocusTrap: false, + shouldDisableAutoFocus: false, }; export {propTypes, defaultProps}; diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index 3e32922ebdd5..8a65cfdc0cab 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -169,7 +169,10 @@ class SearchPage extends Component { ); return ( - + {({didScreenTransitionEnd, safeAreaPaddingBottomStyle}) => ( <> From f46797cd002c512f739275582bedf62c6e821c6a Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Thu, 10 Aug 2023 14:08:40 +0200 Subject: [PATCH 11/16] Check isFocused prop --- src/components/FocusTrapView/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index 4ab5228e303a..fc194455f8fb 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -5,6 +5,7 @@ import React, {useEffect, useRef} from 'react'; import FocusTrap from 'focus-trap-react'; import {View} from 'react-native'; import {PropTypes} from 'prop-types'; +import {useIsFocused} from '@react-navigation/native'; const propTypes = { /** Whether to enable the FocusTrap */ @@ -22,6 +23,8 @@ const defaultProps = { }; function FocusTrapView({enabled, shouldDisableAutoFocus, ...props}) { + const isFocused = useIsFocused(); + /** * Focus trap always needs a focusable element. * In case that we don't have any focusable elements in the modal, @@ -40,7 +43,7 @@ function FocusTrapView({enabled, shouldDisableAutoFocus, ...props}) { return ( !shouldDisableAutoFocus && ref.current, fallbackFocus: () => ref.current, From 6e880a8fea277734dd55eecdee90ef7696264943 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Fri, 18 Aug 2023 09:44:43 +0200 Subject: [PATCH 12/16] Remove redundant library --- package-lock.json | 20 -------------------- package.json | 1 - 2 files changed, 21 deletions(-) diff --git a/package-lock.json b/package-lock.json index 041343a19b1c..c6adf7c192b6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -144,7 +144,6 @@ "@types/jest-when": "^3.5.2", "@types/js-yaml": "^4.0.5", "@types/lodash": "^4.14.195", - "@types/metro-config": "^0.76.3", "@types/mock-fs": "^4.13.1", "@types/pusher-js": "^5.1.0", "@types/react": "^18.2.12", @@ -20108,16 +20107,6 @@ "integrity": "sha512-76CqzuD6Q7LC+AtbPqrvD9AqsN0k8bsYo2bM2J8pmNldP1aIPAbzUQ7QbobyXL4eLr1wK5x8FZFe8eF/ubRuBg==", "dev": true }, - "node_modules/@types/metro-config": { - "version": "0.76.3", - "resolved": "https://registry.npmjs.org/@types/metro-config/-/metro-config-0.76.3.tgz", - "integrity": "sha512-tq9IyKm1JzNRpM7V7LALxXiAyTlV5eZwoqntpnRIbnqDsIyNlnPkWQEoX77f40pzcUuAH4CiHRDxW5VskPlgtQ==", - "deprecated": "This is a stub types definition. metro-config provides its own type definitions, so you do not need this installed.", - "dev": true, - "dependencies": { - "metro-config": "*" - } - }, "node_modules/@types/mime": { "version": "3.0.1", "dev": true, @@ -63770,15 +63759,6 @@ "integrity": "sha512-76CqzuD6Q7LC+AtbPqrvD9AqsN0k8bsYo2bM2J8pmNldP1aIPAbzUQ7QbobyXL4eLr1wK5x8FZFe8eF/ubRuBg==", "dev": true }, - "@types/metro-config": { - "version": "0.76.3", - "resolved": "https://registry.npmjs.org/@types/metro-config/-/metro-config-0.76.3.tgz", - "integrity": "sha512-tq9IyKm1JzNRpM7V7LALxXiAyTlV5eZwoqntpnRIbnqDsIyNlnPkWQEoX77f40pzcUuAH4CiHRDxW5VskPlgtQ==", - "dev": true, - "requires": { - "metro-config": "*" - } - }, "@types/mime": { "version": "3.0.1", "dev": true diff --git a/package.json b/package.json index 2c8bc20506cc..d34eccde5417 100644 --- a/package.json +++ b/package.json @@ -183,7 +183,6 @@ "@types/jest-when": "^3.5.2", "@types/js-yaml": "^4.0.5", "@types/lodash": "^4.14.195", - "@types/metro-config": "^0.76.3", "@types/mock-fs": "^4.13.1", "@types/pusher-js": "^5.1.0", "@types/react": "^18.2.12", From 9b02fce21bff4a698a6a5c06d71809c28d0caa90 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 21 Aug 2023 09:16:04 +0200 Subject: [PATCH 13/16] Remove outline --- src/components/ScreenWrapper/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index d25180736986..73f0e4b9d4b8 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -126,7 +126,7 @@ class ScreenWrapper extends React.Component { enabled={this.props.shouldEnablePickerAvoiding} > From 354abe4ba73492d6ba5e0e9fed8c24350404b1ac Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 21 Aug 2023 09:48:42 +0200 Subject: [PATCH 14/16] Refactor shouldDisableAutoFocus to shouldEnableAutoFocus --- src/components/FocusTrapView/index.js | 8 ++++---- src/components/FocusTrapView/index.native.js | 2 +- src/components/ScreenWrapper/index.js | 2 +- src/components/ScreenWrapper/propTypes.js | 4 ++-- src/pages/ProfilePage.js | 2 +- src/pages/SearchPage.js | 5 +---- 6 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index fc194455f8fb..dc429a8cbe49 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -14,15 +14,15 @@ const propTypes = { /** Whether to disable auto focus * It is used when the component inside the FocusTrap have their own auto focus logic */ - shouldDisableAutoFocus: PropTypes.bool, + shouldEnableAutoFocus: PropTypes.bool, }; const defaultProps = { enabled: true, - shouldDisableAutoFocus: false, + shouldEnableAutoFocus: false, }; -function FocusTrapView({enabled, shouldDisableAutoFocus, ...props}) { +function FocusTrapView({enabled, shouldEnableAutoFocus, ...props}) { const isFocused = useIsFocused(); /** @@ -45,7 +45,7 @@ function FocusTrapView({enabled, shouldDisableAutoFocus, ...props}) { !shouldDisableAutoFocus && ref.current, + initialFocus: () => shouldEnableAutoFocus && ref.current, fallbackFocus: () => ref.current, clickOutsideDeactivates: true, }} diff --git a/src/components/FocusTrapView/index.native.js b/src/components/FocusTrapView/index.native.js index 2e02edeadb72..1c077229760c 100644 --- a/src/components/FocusTrapView/index.native.js +++ b/src/components/FocusTrapView/index.native.js @@ -6,7 +6,7 @@ import {View} from 'react-native'; import _ from 'underscore'; function FocusTrapView(props) { - const viewProps = _.omit(props, ['enabled', 'shouldDisableAutoFocus']); + const viewProps = _.omit(props, ['enabled', 'shouldEnableAutoFocus']); return ( // eslint-disable-next-line react/jsx-props-no-spreading diff --git a/src/components/ScreenWrapper/index.js b/src/components/ScreenWrapper/index.js index 73f0e4b9d4b8..581b4338bcf2 100644 --- a/src/components/ScreenWrapper/index.js +++ b/src/components/ScreenWrapper/index.js @@ -128,7 +128,7 @@ class ScreenWrapper extends React.Component { {this.props.environment === CONST.ENVIRONMENT.DEV && } diff --git a/src/components/ScreenWrapper/propTypes.js b/src/components/ScreenWrapper/propTypes.js index 6f2502935246..617f3ea844ca 100644 --- a/src/components/ScreenWrapper/propTypes.js +++ b/src/components/ScreenWrapper/propTypes.js @@ -50,7 +50,7 @@ const propTypes = { shouldDisableFocusTrap: PropTypes.bool, /** Whether to disable auto focus of the focus trap */ - shouldDisableAutoFocus: PropTypes.bool, + shouldEnableAutoFocus: PropTypes.bool, }; const defaultProps = { @@ -66,7 +66,7 @@ const defaultProps = { shouldShowOfflineIndicator: true, offlineIndicatorStyle: [], shouldDisableFocusTrap: false, - shouldDisableAutoFocus: false, + shouldEnableAutoFocus: false, }; export {propTypes, defaultProps}; diff --git a/src/pages/ProfilePage.js b/src/pages/ProfilePage.js index 11e006c8d8fa..37d4e3a25d4c 100755 --- a/src/pages/ProfilePage.js +++ b/src/pages/ProfilePage.js @@ -140,7 +140,7 @@ function ProfilePage(props) { const statusContent = `${statusEmojiCode} ${statusText}`; return ( - + Navigation.goBack(ROUTES.HOME)} diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index 6753fb23267d..6505b604b614 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -169,10 +169,7 @@ class SearchPage extends Component { ); return ( - + {({didScreenTransitionEnd, safeAreaPaddingBottomStyle}) => ( <> From 18a3047984a588f104eb1fa438b8dbd8c19114c8 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 21 Aug 2023 10:03:26 +0200 Subject: [PATCH 15/16] Refactor --- src/components/FocusTrapView/index.js | 16 ++++++++++++---- src/components/FocusTrapView/index.native.js | 11 ++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index dc429a8cbe49..63380dd71d87 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -8,12 +8,14 @@ import {PropTypes} from 'prop-types'; import {useIsFocused} from '@react-navigation/native'; const propTypes = { + /** Children to wrap with FocusTrap */ + children: PropTypes.node.isRequired, + /** Whether to enable the FocusTrap */ enabled: PropTypes.bool, /** Whether to disable auto focus - * It is used when the component inside the FocusTrap have their own auto focus logic - */ + * It is used when the component inside the FocusTrap have their own auto focus logic */ shouldEnableAutoFocus: PropTypes.bool, }; @@ -38,12 +40,15 @@ function FocusTrapView({enabled, shouldEnableAutoFocus, ...props}) { * After the upgrade of 'react-native-web' to version 0.19 we can use 'tabIndex={0}' prop instead. */ useEffect(() => { + if (!ref.current) { + return; + } ref.current.setAttribute('tabindex', '0'); }, []); - return ( + return enabled ? ( shouldEnableAutoFocus && ref.current, fallbackFocus: () => ref.current, @@ -56,6 +61,9 @@ function FocusTrapView({enabled, shouldEnableAutoFocus, ...props}) { {...props} /> + ) : ( + // eslint-disable-next-line react/jsx-props-no-spreading + props.children ); } diff --git a/src/components/FocusTrapView/index.native.js b/src/components/FocusTrapView/index.native.js index 1c077229760c..5720601f5a2b 100644 --- a/src/components/FocusTrapView/index.native.js +++ b/src/components/FocusTrapView/index.native.js @@ -1,16 +1,9 @@ /* * The FocusTrap is only used on web and desktop */ -import React from 'react'; -import {View} from 'react-native'; -import _ from 'underscore'; -function FocusTrapView(props) { - const viewProps = _.omit(props, ['enabled', 'shouldEnableAutoFocus']); - return ( - // eslint-disable-next-line react/jsx-props-no-spreading - - ); +function FocusTrapView({children}) { + return children; } FocusTrapView.displayName = 'FocusTrapView'; From ce8cf2979e68cac5ccffba2908341490b1fce663 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 21 Aug 2023 10:08:58 +0200 Subject: [PATCH 16/16] Reformat --- src/components/FocusTrapView/index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/FocusTrapView/index.js b/src/components/FocusTrapView/index.js index 63380dd71d87..2dcab7b9d998 100644 --- a/src/components/FocusTrapView/index.js +++ b/src/components/FocusTrapView/index.js @@ -14,8 +14,10 @@ const propTypes = { /** Whether to enable the FocusTrap */ enabled: PropTypes.bool, - /** Whether to disable auto focus - * It is used when the component inside the FocusTrap have their own auto focus logic */ + /** + * Whether to disable auto focus + * It is used when the component inside the FocusTrap have their own auto focus logic + */ shouldEnableAutoFocus: PropTypes.bool, }; @@ -62,7 +64,6 @@ function FocusTrapView({enabled, shouldEnableAutoFocus, ...props}) { /> ) : ( - // eslint-disable-next-line react/jsx-props-no-spreading props.children ); }