Skip to content

Commit

Permalink
Revert "Add focus trap to the RHP (v2)"
Browse files Browse the repository at this point in the history
  • Loading branch information
thienlnam authored Nov 22, 2023
1 parent a273354 commit d24deb5
Show file tree
Hide file tree
Showing 18 changed files with 37 additions and 205 deletions.
50 changes: 0 additions & 50 deletions package-lock.json

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

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@
"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",
Expand Down
1 change: 0 additions & 1 deletion src/components/ConfirmModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ function ConfirmModal(props) {
shouldSetModalVisibility={props.shouldSetModalVisibility}
onModalHide={props.onModalHide}
type={props.isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED : CONST.MODAL.MODAL_TYPE.CONFIRM}
shouldEnableFocusTrap
>
<ConfirmContent
title={props.title}
Expand Down
12 changes: 0 additions & 12 deletions src/components/FocusTrapView/index.native.tsx

This file was deleted.

42 changes: 0 additions & 42 deletions src/components/FocusTrapView/index.tsx

This file was deleted.

21 changes: 0 additions & 21 deletions src/components/FocusTrapView/types.ts

This file was deleted.

13 changes: 2 additions & 11 deletions src/components/Modal/index.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
import React, {useState} from 'react';
import FocusTrapView from '@components/FocusTrapView';
import withWindowDimensions from '@components/withWindowDimensions';
import StatusBar from '@libs/StatusBar';
import * as StyleUtils from '@styles/StyleUtils';
import useTheme from '@styles/themes/useTheme';
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, shouldEnableFocusTrap = false, ...rest}: BaseModalProps) {
const styles = useThemeStyles();
function Modal({fullscreen = true, onModalHide = () => {}, type, onModalShow = () => {}, children, ...rest}: BaseModalProps) {
const theme = useTheme();
const [previousStatusBarColor, setPreviousStatusBarColor] = useState<string>();

Expand Down Expand Up @@ -51,13 +48,7 @@ function Modal({fullscreen = true, onModalHide = () => {}, type, onModalShow = (
fullscreen={fullscreen}
type={type}
>
<FocusTrapView
isEnabled={shouldEnableFocusTrap}
isActive
style={styles.noSelect}
>
{children}
</FocusTrapView>
{children}
</BaseModal>
);
}
Expand Down
4 changes: 0 additions & 4 deletions src/components/Modal/modalPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ const propTypes = {
* */
hideModalContentWhileAnimating: PropTypes.bool,

/** Should the modal use custom focus trap logic */
shouldEnableFocusTrap: PropTypes.bool,

...windowDimensionsPropTypes,
};

Expand All @@ -87,7 +84,6 @@ const defaultProps = {
statusBarTranslucent: true,
avoidKeyboard: false,
hideModalContentWhileAnimating: false,
shouldEnableFocusTrap: false,
};

export {propTypes, defaultProps};
3 changes: 0 additions & 3 deletions src/components/Modal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ 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;
41 changes: 15 additions & 26 deletions src/components/ScreenWrapper/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import {useIsFocused, useNavigation} from '@react-navigation/native';
import {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';
Expand Down Expand Up @@ -40,8 +39,6 @@ const ScreenWrapper = React.forwardRef(
shouldDismissKeyboardBeforeClose,
onEntryTransitionEnd,
testID,
shouldDisableFocusTrap,
shouldEnableAutoFocus,
},
ref,
) => {
Expand All @@ -51,7 +48,6 @@ 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 ? initialHeight : undefined;
Expand Down Expand Up @@ -150,27 +146,20 @@ const ScreenWrapper = React.forwardRef(
style={styles.flex1}
enabled={shouldEnablePickerAvoiding}
>
<FocusTrapView
style={[styles.flex1, styles.noSelect]}
isEnabled={!shouldDisableFocusTrap}
shouldEnableAutoFocus={shouldEnableAutoFocus}
isActive={isFocused}
>
<HeaderGap styles={headerGapStyles} />
{isDevelopment && <TestToolsModal />}
{isDevelopment && <CustomDevMenu />}
{
// If props.children is a function, call it to provide the insets to the children.
_.isFunction(children)
? children({
insets,
safeAreaPaddingBottomStyle,
didScreenTransitionEnd,
})
: children
}
{isSmallScreenWidth && shouldShowOfflineIndicator && <OfflineIndicator style={offlineIndicatorStyle} />}
</FocusTrapView>
<HeaderGap styles={headerGapStyles} />
{isDevelopment && <TestToolsModal />}
{isDevelopment && <CustomDevMenu />}
{
// If props.children is a function, call it to provide the insets to the children.
_.isFunction(children)
? children({
insets,
safeAreaPaddingBottomStyle,
didScreenTransitionEnd,
})
: children
}
{isSmallScreenWidth && shouldShowOfflineIndicator && <OfflineIndicator style={offlineIndicatorStyle} />}
</PickerAvoidingView>
</KeyboardAvoidingView>
</View>
Expand Down
8 changes: 0 additions & 8 deletions src/components/ScreenWrapper/propTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,6 @@ 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 @@ -69,8 +63,6 @@ const defaultProps = {
shouldShowOfflineIndicator: true,
offlineIndicatorStyle: [],
headerGapStyles: [],
shouldDisableFocusTrap: false,
shouldEnableAutoFocus: false,
};

export {propTypes, defaultProps};
5 changes: 1 addition & 4 deletions src/pages/ProfilePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,7 @@ function ProfilePage(props) {
}, [accountID, hasMinimumDetails]);

return (
<ScreenWrapper
testID={ProfilePage.displayName}
shouldEnableAutoFocus
>
<ScreenWrapper testID={ProfilePage.displayName}>
<HeaderWithBackButton
title={props.translate('common.profile')}
onBackButtonPress={() => Navigation.goBack(navigateBackTo)}
Expand Down
5 changes: 1 addition & 4 deletions src/pages/ReportDetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,7 @@ function ReportDetailsPage(props) {
) : null;

return (
<ScreenWrapper
testID={ReportDetailsPage.displayName}
shouldEnableAutoFocus
>
<ScreenWrapper testID={ReportDetailsPage.displayName}>
<FullPageNotFoundView shouldShow={_.isEmpty(props.report)}>
<HeaderWithBackButton
title={props.translate('common.details')}
Expand Down
1 change: 0 additions & 1 deletion src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,6 @@ function ReportScreen({
style={screenWrapperStyle}
shouldEnableKeyboardAvoidingView={isTopMostReportId}
testID={ReportScreen.displayName}
shouldDisableFocusTrap
>
<FullPageNotFoundView
shouldShow={shouldShowNotFoundPage}
Expand Down
1 change: 0 additions & 1 deletion src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ function BaseSidebarScreen(props) {
shouldEnableKeyboardAvoidingView={false}
style={[styles.sidebar, Browser.isMobile() ? styles.userSelectNone : {}]}
testID={BaseSidebarScreen.displayName}
shouldDisableFocusTrap
>
{({insets}) => (
<>
Expand Down
5 changes: 1 addition & 4 deletions src/pages/iou/SplitBillDetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,7 @@ function SplitBillDetailsPage(props) {
);

return (
<ScreenWrapper
testID={SplitBillDetailsPage.displayName}
shouldEnableAutoFocus
>
<ScreenWrapper testID={SplitBillDetailsPage.displayName}>
<FullPageNotFoundView shouldShow={_.isEmpty(reportID) || _.isEmpty(reportAction) || _.isEmpty(props.transaction)}>
<HeaderWithBackButton title={translate('common.details')} />
<View style={[styles.containerWithSpaceBetween, styles.pointerEventsBoxNone]}>
Expand Down
Loading

0 comments on commit d24deb5

Please sign in to comment.