[HOLD for payment] [$2000] Wrong chat composer highlighted after clicking out of emoji modal

[kavimuru]

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

---

Action Performed:

1. Open app, access a report
2. Press the edit message icon
3. Press the emoji icon in the edit composer to open the emoji modal
4. Exit the emoji model by pressing outside

Expected Result:

The main composer is not highlighted, and the keyboard is focused on the editing composer

Actual Result:

The main composer is highlighted, and the keyboard is not focused on the editing composer

Workaround:

unknown

Platforms:

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

• iOS / Safari

Version Number: 1.2.65-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

original-50D4B522-84CD-4952-97DE-ED6DF955B90D.mp4

RPReplay-Final1675655017.MP4

Expensify/Expensify Issue URL:
Issue reported by: @tienifr

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1675481157349539

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01aa19f44ae1aa2e68
• Upwork Job ID: 1623817661403475968
• Last Price Increase: 2023-02-16

[melvin-bot]

Triggered auto assignment to @davidcardoza (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

@davidcardoza Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[davidcardoza]

I was able to reproduce the issue. Going to bring this out to a contributor.

[MelvinBot]

Triggered auto assignment to @NikkiWines (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01aa19f44ae1aa2e68

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

[MelvinBot]

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

[cristipaval]

Why did Melvin assign both me and @NikkiWines ? 👀

[allroundexperts]

Proposal

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

When the emoji selector popup is opened from the edit message composer and then closed, the main chat composer gets the focus instead of the edit message composer.

What is the root cause of that problem?

The main composer has autoFocus property partially set to true. That is, whenever the main composer mounts, it gets focused due to the presence of this property.

On smaller screens, when the second composer (one which comes up when you edit the message) gets the focus, we're un-mounting the main composer. When the secondary composer looses focus (by a click outside), we then mount the main composer again. Since the main composer has auto-focus property setup, the main composer gets focused. This can be seen in the ReportActionItemMessageEdit file. The function used to show or hide the main composer is toggleReportActionComposeView.

When the emoji popup gets closed by clicking outside the secondary composer, it gets blur for a split second. At the same instance, the emoji popup also calls onModalHide since it has been closed. The two events happen almost at the same time. The first event (secondary composer blur) causes the primary composer to show up and be focused. The second event (onModalHide) causes the secondary composer to focus. The first event takes more time to complete (since it mounts a whole component) and thus, causes the main composer to be in a focused state.

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

We can do the following:

1. We can delay the callback executed in the onModalHide function in the emoji picker popup. This will make sure that the secondary composer focus happens AFTER the main composer has finished its focus. For an example, we can use setTimeout function for this.

Alternate Solutions

• We could probably remove this functionality of hiding the main composer when the secondary composer gets focus on small screens altogether. It's more of a design / UX decision though.
• We can also remove autofocus property of the main composer. This will cause the onModalHide event to always execute which is what we want in this case.

[tienifr]

Proposal

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

After closing the emoji picker when editing message, the wrong composer appeared and was focused instead of the edit composer

What is the root cause of that problem?

After the emoji picker is closed, the focus was returned to the body element, at the same time we try to execute focus on the edit composer. This causes a race condition, when the event focus was returned to the body element happens after the edit composer is focused, it causes the edit composer to immediately be blurred and triggered the main composer to appear and gain focus.

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

In src/components/EmojiPicker/EmojiPicker.js, we need to add a new function (only for web) for onModalHide that wraps the function passed in to the EmojiPicker. This new function will create a one-time listener for the focusin event of the body element in the DOM and only run the original onModalHide after the body is already focused, thus preventing the race condition.

Here's the pseudo code of the wrapped function:

wrappedOnModalHide(onModalHide) {
        return () => {
            if (document) {
                document.addEventListener('focusin', (event) => {
                    if (event.target.localName === 'body') {
                        onModalHide();
                    }
                    
                }, { once: true });
            }
        }
    }

We can optionally consider putting that same logic down to the Modal component instead if we want this to apply to all modals.

What alternative solutions did you explore? (Optional)

We can use timeout but generally if there's a working solution that doesn't use timeout, we should prioritize that solution.

Result

Working well after the fix:

Screen.Recording.2023-02-11.at.14.50.10.mov

[s77rt]

Proposal

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

Closing the emoji modal does not focus the corresponding composer.

What is the root cause of that problem?

The onModalHide callback is being called too early (i.e. before actually hiding the modal). This is a bug in react-native-modal. Focusing the composer before hiding the modal is illegal where focus trap kicks in and shifts the focus back to the modal.

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

• Fix the bug upstream: Fixed onModalHide being called early react-native-modal/react-native-modal#733

What alternative solutions did you explore? (Optional)

• Reach out to RN to implement a onHide prop and wait for RNW to adopt it, then use that new prop on react-native-modal to determine when to call the onModalHide callback.

[allroundexperts]

@s77rt Is this really a bug upstream? Aren't you just delaying the callback as well but in a different way? (Rendering twice vs using setTimeout)? Also, aren't we not allowed to create a PR without being assigned the issue?

[cristipaval]

This is off hold! @ntdiary @mollfpr could you please check this one again?

[ntdiary]

This is off hold! @ntdiary @mollfpr could you please check this one again?

@cristipaval, eh, it's been so long, this issue has gone away. :)

test.mp4

[mollfpr]

I can't reproduce it either.

[cristipaval]

Ok, according to this comment, I am proposing to pay @mollfpr and @ntdiary for the long investigation that was done several months ago. I'll assign a bugzero member.

[melvin-bot]

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[cristipaval]

@sonialiap, could you please help me with this?

[sonialiap]

@cristipaval we typically pay around 25% of the issue price in these cases. Split between the two, does $250 each sound reasonable to you?

[cristipaval]

@cristipaval we typically pay around 25% of the issue price in these cases. Split between the two, does $250 each sound reasonable to you?

yes, thank you @sonialiap! 🙏

[sonialiap]

@mollfpr $250 - offer sent please request in NewDot
@ntdiary $250 - offer sent - paid ✔️

[cristipaval]

What's the payment status here? Can we close the issue?

[ntdiary]

What's the payment status here? Can we close the issue?

awaiting payment. :)

[mollfpr]

@sonialiap I'll do manual request on NewDot, thank you!

[sonialiap]

@mollfpr sounds good, withdrawing the Upwork offer
@ntdiary payment completed 💰

[JmillsExpensify]

$250 payment approved for @mollfpr based on this comment.