Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add focus trap to the RHP (v2) #27670

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
256a839
Revert "Revert "Add focus trap to the RHP""
kosmydel Sep 18, 2023
8f1c892
fix initial state in the TwoFactorAuthSteps
kosmydel Sep 18, 2023
b8ca73d
fix focus issue
kosmydel Sep 18, 2023
550d385
Refactor
kosmydel Sep 18, 2023
857f999
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Sep 19, 2023
27bbf8b
Merge branch 'expensify-main' into @kosmydel/add-focus-trap-2
kosmydel Oct 9, 2023
9a06735
refactor calculateCurrentStep
kosmydel Oct 9, 2023
ad9911a
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Oct 10, 2023
76e9195
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Oct 10, 2023
be7a368
use useIsFocused hook
kosmydel Oct 10, 2023
be394f0
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Oct 16, 2023
9fb1ff7
enable focus trap in the ConfirmModal
kosmydel Oct 17, 2023
d9f48e2
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Oct 26, 2023
2a62369
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Nov 2, 2023
9747b20
prettier
kosmydel Nov 2, 2023
18a0aea
refactor
kosmydel Nov 6, 2023
6890531
refactor TwoFactorAuthSteps
kosmydel Nov 6, 2023
f0ec2ca
fix confirmation modal issue
kosmydel Nov 6, 2023
3f51f98
fix
kosmydel Nov 6, 2023
5462980
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Nov 7, 2023
49c8699
use tabIndex property
kosmydel Nov 7, 2023
16b8cb2
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Nov 9, 2023
7135f06
prettier
kosmydel Nov 9, 2023
528bd09
Merge branch 'Expensify:main' into @kosmydel/add-focus-trap-2
kosmydel Nov 13, 2023
317b982
Merge branch 'Expensify:main' into @kosmydel/add-focus-trap-2
kosmydel Nov 14, 2023
b78323a
Refactor FocusTrapView component
kosmydel Nov 16, 2023
0636be5
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Nov 16, 2023
d000814
fix types
kosmydel Nov 16, 2023
3a1cf2e
new ref type
kosmydel Nov 17, 2023
23c1eb6
Merge branch 'main' into @kosmydel/add-focus-trap-2
kosmydel Nov 17, 2023
4a12485
fix attachment focus trap issue
kosmydel Nov 17, 2023
2376853
address review
kosmydel Nov 20, 2023
25c5125
fix split details and details
kosmydel Nov 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -100,6 +100,7 @@
"domhandler": "^4.3.0",
"expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#886f90cbd5e83218fdfd7784d8356c308ef05791",
"fbjs": "^3.0.2",
"focus-trap-react": "^10.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use v10.2.3 to include focus-trap/focus-trap#1068

"htmlparser2": "^7.2.0",
"idb-keyval": "^6.2.1",
"jest-when": "^3.5.2",
Expand Down
1 change: 1 addition & 0 deletions src/components/ConfirmModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
>
<ConfirmContent
title={props.title}
Expand Down
12 changes: 12 additions & 0 deletions src/components/FocusTrapView/index.native.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* The FocusTrap is only used on web and desktop
*/
import FocusTrapViewProps from './types';

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

FocusTrapView.displayName = 'FocusTrapView';

export default FocusTrapView;
42 changes: 42 additions & 0 deletions src/components/FocusTrapView/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* The FocusTrap is only used on web and desktop
*/
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';

function FocusTrapView({isEnabled = true, isActive = true, shouldEnableAutoFocus = false, ...props}: FocusTrapViewProps) {
/**
* 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<HTMLDivElement>(null);

return isEnabled ? (
<FocusTrap
active={isActive}
focusTrapOptions={{
initialFocus: () => (shouldEnableAutoFocus && ref.current) ?? false,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
fallbackFocus: () => ref.current!,
clickOutsideDeactivates: true,
}}
>
<View
ref={viewRef(ref)}
tabIndex={0}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
/>
</FocusTrap>
) : (
props.children
);
}

FocusTrapView.displayName = 'FocusTrapView';

export default FocusTrapView;
21 changes: 21 additions & 0 deletions src/components/FocusTrapView/types.ts
Original file line number Diff line number Diff line change
@@ -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;
13 changes: 11 additions & 2 deletions src/components/Modal/index.tsx
Original file line number Diff line number Diff line change
@@ -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 * 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, ...rest}: BaseModalProps) {
function Modal({fullscreen = true, onModalHide = () => {}, type, onModalShow = () => {}, children, shouldEnableFocusTrap = false, ...rest}: BaseModalProps) {
const styles = useThemeStyles();
const theme = useTheme();
const [previousStatusBarColor, setPreviousStatusBarColor] = useState<string>();

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

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

...windowDimensionsPropTypes,
};

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

export {propTypes, defaultProps};
3 changes: 3 additions & 0 deletions src/components/Modal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
41 changes: 26 additions & 15 deletions src/components/ScreenWrapper/index.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -39,6 +40,8 @@ const ScreenWrapper = React.forwardRef(
shouldDismissKeyboardBeforeClose,
onEntryTransitionEnd,
testID,
shouldDisableFocusTrap,
shouldEnableAutoFocus,
},
ref,
) => {
Expand All @@ -48,6 +51,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 ? initialHeight : undefined;
Expand Down Expand Up @@ -146,20 +150,27 @@ const ScreenWrapper = React.forwardRef(
style={styles.flex1}
enabled={shouldEnablePickerAvoiding}
>
<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
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>
</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};
5 changes: 4 additions & 1 deletion src/pages/ProfilePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ function ProfilePage(props) {
}, [accountID, hasMinimumDetails]);

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

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

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