Skip to content

Commit

Permalink
Merge pull request #24316 from kosmydel/@swm/add-focus-trap
Browse files Browse the repository at this point in the history
Add focus trap to the RHP
  • Loading branch information
roryabraham authored Sep 13, 2023
2 parents 9675a77 + 6ad5b7c commit 1e33428
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 15 deletions.
50 changes: 50 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
"domhandler": "^4.3.0",
"expensify-common": "git+ssh://[email protected]/Expensify/expensify-common.git#35bff866a8d345b460ea6256f0a0f0a8a7f81086",
"fbjs": "^3.0.2",
"focus-trap-react": "^10.2.1",
"htmlparser2": "^7.2.0",
"idb-keyval": "^6.2.1",
"jest-when": "^3.5.2",
Expand Down
75 changes: 75 additions & 0 deletions src/components/FocusTrapView/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* The FocusTrap is only used on web and desktop
*/
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 = {
/** 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
*/
shouldEnableAutoFocus: PropTypes.bool,
};

const defaultProps = {
enabled: true,
shouldEnableAutoFocus: false,
};

function FocusTrapView({enabled, shouldEnableAutoFocus, ...props}) {
const isFocused = useIsFocused();

/**
* 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);

/**
* 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(() => {
if (!ref.current) {
return;
}
ref.current.setAttribute('tabindex', '0');
}, []);

return enabled ? (
<FocusTrap
active={isFocused}
focusTrapOptions={{
initialFocus: () => shouldEnableAutoFocus && ref.current,
fallbackFocus: () => ref.current,
clickOutsideDeactivates: true,
}}
>
<View
ref={ref}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
/>
</FocusTrap>
) : (
props.children
);
}

FocusTrapView.displayName = 'FocusTrapView';
FocusTrapView.propTypes = propTypes;
FocusTrapView.defaultProps = defaultProps;

export default FocusTrapView;
11 changes: 11 additions & 0 deletions src/components/FocusTrapView/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* The FocusTrap is only used on web and desktop
*/

function FocusTrapView({children}) {
return children;
}

FocusTrapView.displayName = 'FocusTrapView';

export default FocusTrapView;
35 changes: 21 additions & 14 deletions src/components/ScreenWrapper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React from 'react';
import _ from 'underscore';
import lodashGet from 'lodash/get';
import {PickerAvoidingView} from 'react-native-picker-select';
import FocusTrapView from '../FocusTrapView';
import KeyboardAvoidingView from '../KeyboardAvoidingView';
import CONST from '../../CONST';
import styles from '../../styles/styles';
Expand Down Expand Up @@ -124,20 +125,26 @@ class ScreenWrapper extends React.Component {
style={styles.flex1}
enabled={this.props.shouldEnablePickerAvoiding}
>
<HeaderGap styles={this.props.headerGapStyles} />
{this.props.environment === CONST.ENVIRONMENT.DEV && <TestToolsModal />}
{this.props.environment === CONST.ENVIRONMENT.DEV && <CustomDevMenu />}
{
// 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 && <OfflineIndicator style={this.props.offlineIndicatorStyle} />}
<FocusTrapView
style={[styles.flex1, styles.noSelect]}
enabled={!this.props.shouldDisableFocusTrap}
shouldEnableAutoFocus={this.props.shouldEnableAutoFocus}
>
<HeaderGap styles={this.props.headerGapStyles} />
{this.props.environment === CONST.ENVIRONMENT.DEV && <TestToolsModal />}
{this.props.environment === CONST.ENVIRONMENT.DEV && <CustomDevMenu />}
{
// 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 && <OfflineIndicator style={this.props.offlineIndicatorStyle} />}
</FocusTrapView>
</PickerAvoidingView>
</KeyboardAvoidingView>
</View>
Expand Down
8 changes: 8 additions & 0 deletions src/components/ScreenWrapper/propTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -63,6 +69,8 @@ const defaultProps = {
shouldShowOfflineIndicator: true,
offlineIndicatorStyle: [],
headerGapStyles: [],
shouldDisableFocusTrap: false,
shouldEnableAutoFocus: false,
};

export {propTypes, defaultProps};
2 changes: 1 addition & 1 deletion src/pages/ProfilePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ function ProfilePage(props) {
const navigateBackTo = lodashGet(props.route, 'params.backTo', '');

return (
<ScreenWrapper>
<ScreenWrapper shouldEnableAutoFocus>
<HeaderWithBackButton
title={props.translate('common.profile')}
onBackButtonPress={() => Navigation.goBack(navigateBackTo)}
Expand Down
1 change: 1 addition & 0 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ function ReportScreen({
<ScreenWrapper
style={screenWrapperStyle}
shouldEnableKeyboardAvoidingView={isTopMostReportId}
shouldDisableFocusTrap
>
<FullPageNotFoundView
shouldShow={(!report.reportID && !report.isLoadingReportActions && !isLoading) || shouldHideReport}
Expand Down
1 change: 1 addition & 0 deletions src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ function BaseSidebarScreen(props) {
includeSafeAreaPaddingBottom={false}
shouldEnableKeyboardAvoidingView={false}
style={[styles.sidebar, Browser.isMobile() ? styles.userSelectNone : {}]}
shouldDisableFocusTrap
>
{({insets}) => (
<>
Expand Down

0 comments on commit 1e33428

Please sign in to comment.