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

Fix cannot open search page by shortcut if delete receipt confirm modal is visible #33203

Merged
merged 11 commits into from
Jan 25, 2024
2 changes: 1 addition & 1 deletion src/components/Modal/BaseModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ function BaseModal(
onBackdropPress={handleBackdropPress}
// Note: Escape key on web/desktop will trigger onBackButtonPress callback
// eslint-disable-next-line react/jsx-props-no-multi-spaces
onBackButtonPress={onClose}
onBackButtonPress={Modal.closeTop}
onModalShow={handleShowModal}
propagateSwipe={propagateSwipe}
onModalHide={hideModal}
Expand Down
1 change: 1 addition & 0 deletions src/components/ThreeDotsMenu/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ function ThreeDotsMenu({iconTooltip, icon, iconFill, iconStyles, onIconPress, me
hidePopoverMenu();
return;
}
buttonRef.current.blur();
Copy link
Contributor Author

@tsa321 tsa321 Dec 17, 2023

Choose a reason for hiding this comment

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

This is needed to prevent react native modal to show blue border box (because of focused state) of threedots button on modal hide when user close modal by pressing esc:

Video:
blue-border-d.mp4

This is because of trap focus of react native modal.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not able to reproduce. Btw, if you reproduced, can you confirm also reproducible on staging/production?
If so, no need to fix here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aimane-chnaif Because I am using onBackButtonPress={Modal.closeTop} in BaseModal.tsx to fix an issue about: When user press three dots menu on attachment modal then press esc the attachment modal is closed. The current behavior in staging:

Pressing esc on three dots menu in attachment modal:
esc-on-staging-d.mp4

After using onBackButtonPress={Modal.closeTop} :

blue border appears:
blue-border-current-branch-d.mp4

showPopoverMenu();
if (onIconPress) {
onIconPress();
Expand Down
28 changes: 23 additions & 5 deletions src/libs/actions/Modal.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import Onyx from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';

const closeModals: Array<(isNavigating: boolean) => void> = [];
const closeModals: Array<(isNavigating?: boolean) => void> = [];

let onModalClose: null | (() => void);
let isNavigate: undefined | boolean;

/**
* Allows other parts of the app to call modal close function
Expand All @@ -21,6 +22,20 @@ function setCloseModal(onClose: () => void) {
};
}

/**
* Close topmost modal
*/
function closeTop() {
if (closeModals.length === 0) {
return;
}
if (onModalClose) {
closeModals[closeModals.length - 1](isNavigate);
return;
}
closeModals[closeModals.length - 1]();
}

/**
* Close modal in other parts of the app
*/
Expand All @@ -30,15 +45,18 @@ function close(onModalCloseCallback: () => void, isNavigating = true) {
return;
}
onModalClose = onModalCloseCallback;
[...closeModals].reverse().forEach((onClose) => {
onClose(isNavigating);
});
isNavigate = isNavigating;
closeTop();
}

function onModalDidClose() {
if (!onModalClose) {
return;
}
if (closeModals.length) {
closeTop();
return;
}
onModalClose();
onModalClose = null;
}
Expand All @@ -58,4 +76,4 @@ function willAlertModalBecomeVisible(isVisible: boolean) {
Onyx.merge(ONYXKEYS.MODAL, {willAlertModalBecomeVisible: isVisible});
}

export {setCloseModal, close, onModalDidClose, setModalVisibility, willAlertModalBecomeVisible};
export {setCloseModal, close, onModalDidClose, setModalVisibility, willAlertModalBecomeVisible, closeTop};
Loading