Workspace - Esc key does not close the opened window when 3 dot menu is open

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.40-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Log in to any account
2. Go to setting> Workspace> Select a workspace > Members
3. Click on the 3 dot menu and hover over it.
4. Press "Ctrl + D"
5. Press "esc" key
   You might need to repeat step 3-5

Expected Result:

Opened window is dismissed

Actual Result:

Opened window is not dismissed. And page is navigated to workspace page with out dismissing the window

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6615895_1727344310082.Screen_Recording_2024-09-26_at_12.29.17_in_the_afternoon.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @

[melvin-bot]

Triggered auto assignment to @OfstadC (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[codewaseem]

I could not reproduce this on the latest main branch.

[rijusougata13]

This issue has been resolved. Please close this one.

[melvin-bot]

📣 @rijusougata13! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[rijusougata13]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link:

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

ESC shortcut doesn't close the Debug modal/page.

What is the root cause of that problem?

Every modal when mounted will set a "close modal" function that can be used to close the modal.

App/src/components/Modal/BaseModal.tsx

Lines 107 to 116 in c7ee85b

	removeOnCloseListener = Modal.setCloseModal(onClose);
	}
	return () => {
	if (!removeOnCloseListener) {
	return;
	}
	removeOnCloseListener();
	};
	}, [isVisible, wasVisible, onClose, type]);

If we press ESC, Modal.closeTop will be called which will call the last/top "close modal" function if exists and then remove it from the list. But sometimes, the "close modal" is empty, thus the Debug modal is never closed.

App/src/libs/actions/Modal.ts

Lines 29 to 40 in c7ee85b

	function closeTop() {
	if (closeModals.length === 0) {
	return;
	}
	if (onModalClose) {
	closeModals[closeModals.length - 1](isNavigate);
	closeModals.pop();
	return;
	}
	closeModals[closeModals.length - 1]();
	closeModals.pop();
	}

Another way for the "close modal" function to be removed is when the modal is unmounted which is shown by the first permalink above.

App/src/libs/actions/Modal.ts

Lines 13 to 24 in c7ee85b

	function setCloseModal(onClose: () => void) {
	if (!closeModals.includes(onClose)) {
	closeModals.push(onClose);
	}
	return () => {
	const index = closeModals.indexOf(onClose);
	if (index === -1) {
	return;
	}
	closeModals.splice(index, 1);
	};
	}

The Debug modal is placed inside a ScreenWrapper, which means every screen has the modal mounted.

App/src/components/ScreenWrapper.tsx

Line 275 in c7ee85b

<TestToolsModal />

In this issue, when we press ESC, the workspace members list page is closed because the dismissModal here is called which closes a FullScreenNavigator. However, dismissModal shouldn't be called here because a modal is visible.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 318 to 328 in 28bc3cd

	() => {
	if (modal.current.willAlertModalBecomeVisible) {
	return;
	}
	if (modal.current.disableDismissOnEscape) {
	return;
	}
	Navigation.dismissModal();
	},

But in our case, willAlertModalBecomeVisible is false even though the modal is visible. willAlertModalBecomeVisible is updated every time a modal/popover is shown/hidden.

App/src/components/Modal/BaseModal.tsx

Lines 84 to 87 in 28bc3cd

	const hideModal = useCallback(
	(callHideCallback = true) => {
	if (Modal.areAllModalsHidden()) {
	Modal.willAlertModalBecomeVisible(false);

App/src/components/Modal/BaseModal.tsx

Lines 104 to 105 in 28bc3cd

	if (isVisible) {
	Modal.willAlertModalBecomeVisible(true, type === CONST.MODAL.MODAL_TYPE.POPOVER || type === CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED);

App/src/components/PopoverWithoutOverlay/index.tsx

Line 63 in 28bc3cd

Modal.willAlertModalBecomeVisible(isVisible, true);

In this issue, we first open a popover of the member list and then press the Debug modal shortcut. Sometimes, the Debug modal is shown first which sets the willAlertModalBecomeVisible and then followed by the popover hide which sets the willAlertModalBecomeVisible to false.

Because of that, pressing the ESC will close the workspace members page which unmounts the Debug modal which removes the "close modal" function from the list, so the ESC shortcut doesn't close the Debug modal because the "close modal" list is now empty.

What changes do you think we should make in order to solve the problem?

To fix that, we need to wait for any modal to close first before showing the debug modal.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 378 to 382 in c7ee85b

	const unsubscribeDebugShortcut = KeyboardShortcut.subscribe(
	debugShortcutConfig.shortcutKey,
	() => {
	toggleTestToolsModal();
	},

Modal.close(toggleTestToolsModal);

Also, we can consider moving the Debug modal (TestToolsModal) from ScreenWrapper to AuthScreens. This way, we won't render the modal on every screen.

Before:

After:

[OfstadC]

This is no longer reproducible - closing

[bernhardoj]

@OfstadC this is still reproducible

[OfstadC]

@bernhardoj

Esc is working for me

[bernhardoj]

@OfstadC you missed this step.

Click on the 3 dot menu and hover over it.

web.mp4

[OfstadC]

@OfstadC you missed this step.

🤦‍♀️

[OfstadC]

Thanks @bernhardoj !

[parasharrajat]

We are discussing to solve this issue with #49976. Its not yet decided yet. In that case, @OfstadC could you please assign me as C+ here?

[parasharrajat]

Ok. Thanks. @bernhardoj's proposal looks good to me. let's make that change. This proposal will also solve #49976

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @marcochavezf, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[parasharrajat]

@OfstadC We are missing the help wanted label. Let's apply that before hiring anyone for automation sake. Thanks.

[melvin-bot]

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

[melvin-bot]

Current assignee @parasharrajat is eligible for the External assigner, not assigning anyone new.

[marcochavezf]

Thanks for the review @parasharrajat, assigning @bernhardoj 🚀

[bernhardoj]

PR is ready
cc: @parasharrajat