Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.49k stars 2.85k forks source link

[HOLD for E/App #49802] "Release options" modal ( CTRL+D) does not appear when image is open #50114

Open lanitochka17 opened 2 weeks ago

lanitochka17 commented 2 weeks ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.43-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): gocemate+a2386@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to any chat
  2. Open a image from chat history
  3. Press Ctrl+D to open "Release options" modal

Expected Result:

"Release options" modal should appear

Actual Result:

"Release options" modal ( CTRL+D) does not appear when image is open

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/68fc9a78-59f5-4c6d-b955-684c52fc289e

View all open jobs on GitHub

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @alexpensify (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.

lanitochka17 commented 2 weeks ago

@alexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

Nodebrute commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2024-10-03 00:58:22 UTC.

Proposal

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

Modal ( CTRL+D) does not appear when image is open

What is the root cause of that problem?

We don't close the previous modals when we open Debug modal using shortcut key CMD + D https://github.com/Expensify/App/blob/b52f1d4a0d9c7b58e5fd698ddb4cdb6c3975cba8/src/libs/Navigation/AppNavigator/AuthScreens.tsx#L379-L383

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

We can use Modal.close here https://github.com/Expensify/App/blob/b52f1d4a0d9c7b58e5fd698ddb4cdb6c3975cba8/src/libs/Navigation/AppNavigator/AuthScreens.tsx#L379-L383 We can do something like this Pseudo code

 const unsubscribeDebugShortcut = KeyboardShortcut.subscribe(
            debugShortcutConfig.shortcutKey,
            () => {
                  Modal.close(toggleTestToolsModal);
            },
            debugShortcutConfig.descriptionKey,
            debugShortcutConfig.modifiers,
            true,
        );

https://github.com/user-attachments/assets/451679c5-ff65-4e32-991b-30a42acd1569

This will also fix the modal issue in other parts of app

Before https://github.com/user-attachments/assets/76b6061c-ae44-4e87-9691-9b28c2cc449b
After https://github.com/user-attachments/assets/b9ffe5b2-08eb-40ad-beae-1bde91a1aa93

What alternative solutions did you explore? (Optional)

Optionally we can also use Session.checkIfActionIsAllowed along with above solution

bernhardoj commented 2 weeks ago

Same solution as https://github.com/Expensify/App/issues/49802

alexpensify commented 2 weeks ago

@bernhardoj do you think we should close this one or put it on hold for https://github.com/Expensify/App/issues/49802

bernhardoj commented 2 weeks ago

Let's hold it for https://github.com/Expensify/App/issues/49802

alexpensify commented 2 weeks ago

Thanks for the feedback!

alexpensify commented 1 week ago

Weekly Update: On hold for https://github.com/Expensify/App/issues/49802

alexpensify commented 6 days ago

Weekly Update: The PR for https://github.com/Expensify/App/issues/49802 is under review