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

Try to refactor the logic regarding refocusing when the modal is closed #29199

Closed
wants to merge 47 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
4952dce
try to refactor the modal's refocus logic
ntdiary Nov 29, 2023
7655502
fix map iterator
ntdiary Nov 29, 2023
0f1ebe1
Merge branch 'main' into fix-24452
ntdiary Dec 18, 2023
8a7ce9d
fix merge error
ntdiary Dec 18, 2023
bda18dd
Merge branch 'main' into fix-24452
ntdiary Dec 19, 2023
44b57f5
fix RHP refocus
ntdiary Dec 19, 2023
5dfca1b
fix ts check
ntdiary Dec 19, 2023
b73e737
optimize based on the comments
ntdiary Dec 19, 2023
2d85663
delete outdated file
ntdiary Dec 20, 2023
ed42722
Merge branch 'main' into fix-24452
ntdiary Dec 21, 2023
7da38ae
fix lint error
ntdiary Dec 21, 2023
a7bbc6c
Merge branch 'main' into fix-24452
ntdiary Dec 21, 2023
93972b0
add some comments
ntdiary Dec 26, 2023
f7e6697
Merge branch 'main' into fix-24452
ntdiary Jan 5, 2024
cccb86c
Merge branch 'main' into fix-24452
ntdiary Jan 5, 2024
532319f
add some missed changes
ntdiary Jan 5, 2024
89cc964
clean up
ntdiary Jan 5, 2024
1a1015d
Merge branch 'main' into fix-24452
ntdiary Jan 5, 2024
c200021
clean up
ntdiary Jan 5, 2024
7cd3b10
Merge branch 'main' into fix-24452
ntdiary Jan 5, 2024
bcb77c4
Merge branch 'main' into fix-24452
ntdiary Jan 8, 2024
016521d
clean up the console log
ntdiary Jan 8, 2024
665b0ee
Merge branch 'main' into fix-24452
ntdiary Jan 10, 2024
4da3a4d
Merge branch 'main' into fix-24452
ntdiary Jan 12, 2024
40380d1
use typescript
ntdiary Jan 12, 2024
8ad8a89
Merge branch 'main' into fix-24452
ntdiary Jan 15, 2024
716e20c
introduce businessType to remove dependence on file cancel event dete…
ntdiary Jan 15, 2024
e8cc8b1
fix lint error
ntdiary Jan 15, 2024
cce02d6
Merge branch 'main' into fix-24452
ntdiary Jan 15, 2024
e6f1f00
improve business type detection
ntdiary Jan 16, 2024
1f6b9b8
waypoint edit migration
ntdiary Jan 16, 2024
40551c0
Merge branch 'main' into fix-24452
ntdiary Jan 17, 2024
f42b9b2
fix ts check
ntdiary Jan 17, 2024
ce9f14f
Merge branch 'main' into fix-24452
ntdiary Jan 18, 2024
68e8e64
polish
ntdiary Jan 23, 2024
b450dd6
Merge branch 'main' into fix-24452
ntdiary Jan 24, 2024
5372ced
Merge branch 'main' into fix-24452
ntdiary Jan 24, 2024
8c131ed
fix conflicts
ntdiary Jan 24, 2024
4e5b69e
Merge branch 'main' into fix-24452
ntdiary Jan 25, 2024
1ddaf05
fix ts check
ntdiary Jan 25, 2024
b826eca
fix lint error
ntdiary Jan 25, 2024
51d8928
remove unused import
ntdiary Jan 25, 2024
8c73ac7
Merge branch 'main' into fix-24452
ntdiary Jan 29, 2024
cc009cc
fix app crash
ntdiary Jan 29, 2024
f853d13
fix android preview modal
ntdiary Jan 29, 2024
40ff044
fix android AddReaction button
ntdiary Jan 29, 2024
bd84c65
lint error
ntdiary Jan 29, 2024
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
9 changes: 9 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,15 @@ const CONST = {
RIGHT: 'right',
},
POPOVER_MENU_PADDING: 8,
BUSINESS_TYPE: {
DEFAULT: 'default',
ATTACHMENT: 'attachment',
},
RESTORE_FOCUS_TYPE: {
DEFAULT: 'default',
DELETE: 'delete',
PRESERVE: 'preserve',
},
},
TIMING: {
CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION: 'calc_most_recent_last_modified_action',
Expand Down
8 changes: 7 additions & 1 deletion src/components/AttachmentPicker/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ const getDataForUpload = (fileData) => {
function AttachmentPicker({type, children, shouldHideCameraOption}) {
const styles = useThemeStyles();
const [isVisible, setIsVisible] = useState(false);
const [restoreFocusType, setRestoreFocusType] = useState();

const completeAttachmentSelection = useRef();
const onModalHide = useRef();
Expand Down Expand Up @@ -305,12 +306,14 @@ function AttachmentPicker({type, children, shouldHideCameraOption}) {
return (
<>
<Popover
restoreFocusType={restoreFocusType}
onClose={() => {
close();
onCanceled.current();
}}
isVisible={isVisible}
anchorPosition={styles.createMenuPosition}
onModalShow={() => setRestoreFocusType(CONST.MODAL.RESTORE_FOCUS_TYPE.DEFAULT)}
onModalHide={onModalHide.current}
>
<View style={!isSmallScreenWidth && styles.createMenuContainer}>
Expand All @@ -319,7 +322,10 @@ function AttachmentPicker({type, children, shouldHideCameraOption}) {
key={item.textTranslationKey}
icon={item.icon}
title={translate(item.textTranslationKey)}
onPress={() => selectItem(item)}
onPress={() => {
setRestoreFocusType(CONST.MODAL.RESTORE_FOCUS_TYPE.PRESERVE);
selectItem(item);
}}
focused={focusedIndex === menuIndex}
/>
))}
Expand Down
8 changes: 0 additions & 8 deletions src/components/Composer/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type {TextInput} from 'react-native';
import {StyleSheet} from 'react-native';
import type {AnimatedTextInputRef} from '@components/RNTextInput';
import RNTextInput from '@components/RNTextInput';
import useResetComposerFocus from '@hooks/useResetComposerFocus';
import useStyleUtils from '@hooks/useStyleUtils';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
Expand All @@ -31,7 +30,6 @@ function Composer(
ref: ForwardedRef<TextInput>,
) {
const textInput = useRef<AnimatedTextInputRef | null>(null);
const {isFocused, shouldResetFocus} = useResetComposerFocus(textInput);
const theme = useTheme();
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
Expand Down Expand Up @@ -81,12 +79,6 @@ function Composer(
/* eslint-disable-next-line react/jsx-props-no-spreading */
{...props}
readOnly={isDisabled}
onBlur={(e) => {
if (!isFocused) {
shouldResetFocus.current = true; // detect the input is blurred when the page is hidden
}
props?.onBlur?.(e);
}}
/>
);
}
Expand Down
12 changes: 1 addition & 11 deletions src/components/Composer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -372,17 +372,7 @@ function Composer(
numberOfLines={numberOfLines}
disabled={isDisabled}
onKeyPress={handleKeyPress}
onFocus={(e) => {
ReportActionComposeFocusManager.onComposerFocus(() => {
if (!textInput.current) {
return;
}

textInput.current.focus();
});

props.onFocus?.(e);
}}
onFocus={(e) => props.onFocus?.(e)}
/>
{shouldCalculateCaretPosition && renderElementForCaretPosition}
</>
Expand Down
6 changes: 6 additions & 0 deletions src/components/ConfirmModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {ReactNode} from 'react';
import React from 'react';
import type {StyleProp, TextStyle, ViewStyle} from 'react-native';
import type {ValueOf} from 'type-fest';
import useWindowDimensions from '@hooks/useWindowDimensions';
import CONST from '@src/CONST';
import type IconAsset from '@src/types/utils/IconAsset';
Expand Down Expand Up @@ -64,6 +65,9 @@ type ConfirmModalProps = {

/** Whether to stack the buttons */
shouldStackButtons?: boolean;

/** How to re-focus after the modal is dismissed */
restoreFocusType?: ValueOf<typeof CONST.MODAL.RESTORE_FOCUS_TYPE>;
};

function ConfirmModal({
Expand All @@ -86,6 +90,7 @@ function ConfirmModal({
shouldStackButtons = true,
isVisible,
onConfirm,
restoreFocusType,
}: ConfirmModalProps) {
const {isSmallScreenWidth} = useWindowDimensions();

Expand All @@ -97,6 +102,7 @@ function ConfirmModal({
shouldSetModalVisibility={shouldSetModalVisibility}
onModalHide={onModalHide}
type={isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED : CONST.MODAL.MODAL_TYPE.CONFIRM}
restoreFocusType={restoreFocusType}
>
<ConfirmContent
title={title}
Expand Down
37 changes: 24 additions & 13 deletions src/components/Modal/BaseModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {forwardRef, useCallback, useEffect, useMemo, useRef} from 'react';
import React, {forwardRef, useCallback, useContext, useEffect, useMemo, useRef} from 'react';
import {View} from 'react-native';
import ReactNativeModal from 'react-native-modal';
import ColorSchemeWrapper from '@components/ColorSchemeWrapper';
Expand All @@ -14,6 +14,8 @@ import useNativeDriver from '@libs/useNativeDriver';
import variables from '@styles/variables';
import * as Modal from '@userActions/Modal';
import CONST from '@src/CONST';
import {ModalBusinessTypeContext} from './ModalBusinessTypeProvider';
import ModalContent from './ModalContent';
import type BaseModalProps from './types';

function BaseModal(
Expand All @@ -37,6 +39,8 @@ function BaseModal(
animationOutTiming,
statusBarTranslucent = true,
onLayout,
restoreFocusType,
shouldClearFocusWithType = false,
avoidKeyboard = false,
children,
shouldUseCustomBackdrop = false,
Expand All @@ -53,6 +57,13 @@ function BaseModal(
const isVisibleRef = useRef(isVisible);
const wasVisible = usePrevious(isVisible);

const modalId = useMemo(() => ComposerFocusManager.getId(), []);
const {businessType} = useContext(ModalBusinessTypeContext);
const saveFocusState = () => {
ComposerFocusManager.saveFocusState(modalId, businessType, shouldClearFocusWithType);
ComposerFocusManager.resetReadyToFocus(modalId);
};

/**
* Hides modal
* @param callHideCallback - Should we call the onModalHide callback
Expand All @@ -67,11 +78,9 @@ function BaseModal(
onModalHide();
}
Modal.onModalDidClose();
if (!fullscreen) {
ComposerFocusManager.setReadyToFocus();
}
ComposerFocusManager.tryRestoreFocusAfterClosedCompletely(modalId, businessType, restoreFocusType);
},
[shouldSetModalVisibility, onModalHide, fullscreen],
[shouldSetModalVisibility, onModalHide, businessType, restoreFocusType, modalId],
);

useEffect(() => {
Expand Down Expand Up @@ -119,7 +128,7 @@ function BaseModal(
};

const handleDismissModal = () => {
ComposerFocusManager.setReadyToFocus();
ComposerFocusManager.setReadyToFocus(modalId);
};

const {
Expand Down Expand Up @@ -181,7 +190,7 @@ function BaseModal(
onModalShow={handleShowModal}
propagateSwipe={propagateSwipe}
onModalHide={hideModal}
onModalWillShow={() => ComposerFocusManager.resetReadyToFocus()}
onModalWillShow={saveFocusState}
onDismiss={handleDismissModal}
onSwipeComplete={() => onClose?.()}
swipeDirection={swipeDirection}
Expand All @@ -205,12 +214,14 @@ function BaseModal(
avoidKeyboard={avoidKeyboard}
customBackdrop={shouldUseCustomBackdrop ? <Overlay onPress={handleBackdropPress} /> : undefined}
>
<View
style={[styles.defaultModalContainer, modalContainerStyle, modalPaddingStyles, !isVisible && styles.pointerEventsNone]}
ref={ref}
>
<ColorSchemeWrapper>{children}</ColorSchemeWrapper>
</View>
<ModalContent onDismiss={handleDismissModal}>
<View
style={[styles.defaultModalContainer, modalContainerStyle, modalPaddingStyles, !isVisible && styles.pointerEventsNone]}
ref={ref}
>
<ColorSchemeWrapper>{children}</ColorSchemeWrapper>
</View>
</ModalContent>
</ReactNativeModal>
);
}
Expand Down
20 changes: 20 additions & 0 deletions src/components/Modal/ModalBusinessTypeProvider/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React, {useMemo} from 'react';
import CONST from '@src/CONST';
import type {ModalBusinessTypeContextProps, ModalBusinessTypeContextValue} from './types';

const ModalBusinessTypeContext = React.createContext<ModalBusinessTypeContextValue>({
businessType: CONST.MODAL.BUSINESS_TYPE.DEFAULT,
});

function ModalBusinessTypeProvider({businessType, children}: ModalBusinessTypeContextProps) {
const contextValue = useMemo(
() => ({
businessType,
}),
[businessType],
);
return <ModalBusinessTypeContext.Provider value={contextValue}>{children}</ModalBusinessTypeContext.Provider>;
}

export default ModalBusinessTypeProvider;
export {ModalBusinessTypeContext};
21 changes: 21 additions & 0 deletions src/components/Modal/ModalBusinessTypeProvider/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import type {ReactNode} from 'react';
import type {ValueOf} from 'type-fest';
import type CONST from '@src/CONST';

type ModalBusinessTypeContextProps = {
/**
* So far, modern browsers only support the file cancel event in some newer versions
* (i.e., Chrome: 113+ / Firefox: 91+ / Safari 16.4+), and there is no standard feature detection method available.
* Therefore, we introduce this prop to isolate the impact of the file upload modal on the focus stack.
*/
businessType: ValueOf<typeof CONST.MODAL.BUSINESS_TYPE>;

/** Children to render */
children: ReactNode;
};

type ModalBusinessTypeContextValue = {
businessType: ValueOf<typeof CONST.MODAL.BUSINESS_TYPE>;
};

export type {ModalBusinessTypeContextProps, ModalBusinessTypeContextValue};
19 changes: 19 additions & 0 deletions src/components/Modal/ModalContent.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import type {ReactNode} from 'react';
import React from 'react';

type ModalContentProps = {
/** Modal contents */
children: ReactNode;

/** called after modal content is dismissed */
onDismiss: () => void;
};

function ModalContent({children, onDismiss = () => {}}: ModalContentProps) {
// eslint-disable-next-line react-hooks/exhaustive-deps
React.useEffect(() => () => onDismiss?.(), []);
return children;
}
ModalContent.displayName = 'ModalContent';

export default ModalContent;
10 changes: 0 additions & 10 deletions src/components/Modal/index.android.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,7 @@
import React from 'react';
import {AppState} from 'react-native';
import ComposerFocusManager from '@libs/ComposerFocusManager';
import BaseModal from './BaseModal';
import type BaseModalProps from './types';

AppState.addEventListener('focus', () => {
ComposerFocusManager.setReadyToFocus();
});

AppState.addEventListener('blur', () => {
ComposerFocusManager.resetReadyToFocus();
});

// Only want to use useNativeDriver on Android. It has strange flashes issue on IOS
// https://github.com/react-native-modal/react-native-modal#the-modal-flashes-in-a-weird-way-when-animating
function Modal({useNativeDriver = true, ...rest}: BaseModalProps) {
Expand Down
3 changes: 3 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,

/** how to re-focus after the modal is dismissed */
restoreFocusType: PropTypes.oneOf(_.values(CONST.MODAL.RESTORE_FOCUS_TYPE)),

...windowDimensionsPropTypes,
};

Expand Down
6 changes: 6 additions & 0 deletions src/components/Modal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ type BaseModalProps = Partial<ModalProps> & {

/** Should we use a custom backdrop for the modal? (This prevents focus issues on desktop) */
shouldUseCustomBackdrop?: boolean;

/** Whether the modal should clear the focus record for the current business type. */
shouldClearFocusWithType?: boolean;

/** How to re-focus after the modal is dismissed */
restoreFocusType?: ValueOf<typeof CONST.MODAL.RESTORE_FOCUS_TYPE>;
};

export default BaseModalProps;
Expand Down
16 changes: 16 additions & 0 deletions src/components/PopoverMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {RefObject} from 'react';
import React, {useRef} from 'react';
import {View} from 'react-native';
import type {ModalProps} from 'react-native-modal';
import type {ValueOf} from 'type-fest';
import useArrowKeyFocusManager from '@hooks/useArrowKeyFocusManager';
import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -50,6 +51,15 @@ type PopoverMenuProps = Partial<PopoverModalProps> & {
/** Callback method fired when the user requests to close the modal */
onClose: () => void;

/** Callback method fired when the modal is shown */
onModalShow?: () => void;

/** Whether the modal should clear the focus record for the current business type. */
shouldClearFocusWithType?: boolean;

/** How to re-focus after the modal is dismissed */
restoreFocusType?: ValueOf<typeof CONST.MODAL.RESTORE_FOCUS_TYPE>;

/** State that determines whether to display the modal or not */
isVisible: boolean;

Expand Down Expand Up @@ -91,6 +101,9 @@ function PopoverMenu({
anchorPosition,
anchorRef,
onClose,
shouldClearFocusWithType,
restoreFocusType,
onModalShow = () => {},
headerText,
fromSidebarMediumScreen,
anchorAlignment = {
Expand Down Expand Up @@ -142,6 +155,9 @@ function PopoverMenu({
anchorAlignment={anchorAlignment}
onClose={onClose}
isVisible={isVisible}
shouldClearFocusWithType={shouldClearFocusWithType}
restoreFocusType={restoreFocusType}
onModalShow={onModalShow}
onModalHide={onModalHide}
animationIn={animationIn}
animationOut={animationOut}
Expand Down
Loading
Loading