[PAID] [$250] Chat - When opening a PDF and clicking 'Ctrl + K,' the search opens but the page scrolls to the next

[lanitochka17]

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.13-3
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):**betlihemasfaw14@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to any chat and click the plus sign
2. Select "Attachment" and send the Pdf attachment
3. Open the attachment and click 'Ctrl + K
   4, click on 'esc' to close

Expected Result:

When we open the PDF and click 'Ctrl + K,' the search should open, and the page should not scroll to the next page

Actual Result:

When we open the PDF and click 'Ctrl + K,' the search opens, but the page scrolls to the next page

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

Bug6550529_1721740884041.Screen_Recording_2024-07-23_at_6.19.05_AM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b20e5826b61d01de
• Upwork Job ID: 1820952988321413899
• Last Price Increase: 2024-08-13
• Automatic offers:
• allgandalf | Reviewer | 103557309
• dominictb | Contributor | 103557313

Issue Owner

Current Issue Owner: @

[melvin-bot]

Triggered auto assignment to @strepanier03 (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]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@strepanier03 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

[dominictb]

Proposal

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

Reproduction step:

• Upload a pdf in a chat (chat 1) and then open another chat (chat 2)
• Open chat 1 again using LHN
• Open the pdf attachment
• Click Cmd + K

Instead of closing the pdf attachment and show the chat 1 in the background while the chat search left modal open, it shows the chat 2 in the background instead

What is the root cause of that problem?

In here, when calling Modal.close, we are actually calling the Modal's onClose callback more than once

• The first time is in here, we called closeTop from Modal.close() function directly
• The second time is in here, the react-native-modal library will trigger the onModalHide, and in that function we also call Modal.onModalDidClose, which in turn calls closeTop accordingly
• The third time is in when the modal is being unmounted, and like the second time, it triggers Modal.closeTop. However, at this point in time, we have already removed the onClose listener here, hence in here the callback will be triggered, i.e: opening the chat search page.

So, this function will be triggered twice, and because the navigation is not an synchronous operation, in here, we can see that during both triggers on dismissModal function, the lastRoute is still the chat attachment modal, however, we are dispatching pop action twice, hence we can see that the navigation will look like this Chat attachment modal > Chat 1 report page > Chat 2 report page > Chat search page modal

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

The idea is to make sure that the onClose callback of each modal is idempotent, i.e: If we have already triggered the callback, triggering it again shouldn't cause any other side effects. We can choose to fix it for just ReportAttachments or fix it globally.

To fix it for ReportAttachments, simply add a ref const modalDismissed = useRef(false) , and in here lock the action Navigation.dismissModal

if(!modalDismissed.current) {
      Navigation.dismissModal();
      modalDismissed.current = true;
 }

To fix it globally for all modal onClose callbacks (in case those callbacks are not idempotent in principle), we can keep track the number of trigger for each onClose callbacks in here, and prevent the callback to be called again if it has been called before

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

function setCloseModal(onClose: () => void) {
    if (!closeModals.includes(onClose)) {
        closeModals.push([onClose, 0]); // initially, the number of execution is 0
    }
    return () => {
        const index = closeModals.findIndex([func, counter] => func === onClose);
        if (index === -1) {
            return;
        }
        closeModals.splice(index, 1);
    };
}

function closeTop() {
    if (closeModals.length === 0) {
        return;
    }
    const [onClose, executionCounter] = closeModals[closeModals.length - 1]
    if(executionCounter > 0) { // prevent multiple triggers
        return;
    }
    closeModals[closeModals.length - 1][1] += 1

    if (onModalClose) {
        onClose(isNavigate)
        return;
    }
   onClose()
}

What alternative solutions did you explore? (Optional)

[melvin-bot]

@strepanier03 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@strepanier03 Still overdue 6 days?! Let's take care of this!

[strepanier03]

Testing now.