[HOLD for payment 2023-09-21] [$500] Emoji suggestion/email mentions open and close on opening assign task/request money in chat

[kavimuru]

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

---

Action Performed:

1. Open the app
2. Open any report
3. Type text to trigger emoji suggestion/email mention eg: ':te'
4. Click on plus and click assign task/request money
5. Observe that emoji suggestion/email mention reopens and closes once

Expected Result:

Emoji suggestion/email mentions should not reopen on opening RHN elements like assign task/request money

Actual Result:

Emoji suggestion/email mentions reopens on opening RHN elements like assign task/request money and closes after some time

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.61-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

suggestion.open.close.once.on.assign.task.request.money.mp4

Recording.1549.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692942214043639

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0146485d43d701eb2b
• Upwork Job ID: 1698626127471722496
• Last Price Increase: 2023-09-04
• Automatic offers:
• s77rt | Reviewer | 26606979
• s-alves10 | Contributor | 26606981
• dhanashree-sawant | Reporter | 26606984

[melvin-bot]

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

[s-alves10]

Proposal

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

Emoji/mention suggestions appears for a brief moment when opening assign task/request money in chat

What is the root cause of that problem?

When we click + button in the composer, isKeyboardVisibleWhenShowingModalRef.current becomes true by the following 2 code blocks

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Lines 204 to 209 in 28a31b8

	const onAddActionPressed = useCallback(() => {
	if (!willBlurTextInputOnTapOutside) {
	isKeyboardVisibleWhenShowingModalRef.current = composerRef.current.isFocused();
	}
	composerRef.current.blur();
	}, []);

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Lines 279 to 285 in 28a31b8

	const onBlur = useCallback((e) => {
	setIsFocused(false);
	suggestionsRef.current.resetSuggestions();
	if (e.relatedTarget && e.relatedTarget === actionButtonRef.current) {
	isKeyboardVisibleWhenShowingModalRef.current = true;
	}
	}, []);

When the popover closes, restoreKeyboardState function is called and the composer gets focused

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Lines 188 to 194 in 28a31b8

	const restoreKeyboardState = useCallback(() => {
	if (!isKeyboardVisibleWhenShowingModalRef.current) {
	return;
	}
	focus();
	isKeyboardVisibleWhenShowingModalRef.current = false;
	}, []);

If we select request money or assign task, the composer gets blurred again.

On focused, suggestions appear and disappear when blurred.
This is the root cause

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

When we select menu items, we need to inform that we're going to open modal
In order to do this, add an additional props onItemSelected to the AttachmentPickerWithMenuItems component and call it inside the onItemSelected callback of PopoverMenu

        onItemSelected={(item, index) => {
            setMenuVisibility(false);
            onItemSelected();

            ...
        }}

Define onItemSelected function in ReportActionCompose component and pass it to the AttachmentPickerWithMenuItems

    const onItemSelected = useCallback(() => {
        isKeyboardVisibleWhenShowingModalRef.current = false; // this prevents the refocusing
    }, []);

Set isKeyboardVisibleWhenShowingModalRef to true in onTriggerAttachmentPicker

    const onTriggerAttachmentPicker = useCallback(() => {
        ...
        isKeyboardVisibleWhenShowingModalRef.current = true;
    }, []);

There is another issue: sometimes suggestions window doesn't appear. This was introduced by this PR
We need to change true to false at the below line

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Line 328 in 1bdc5ac

return suggestionsRef.current.setShouldBlockSuggestionCalc(true);

This works as expected

Result

26489.mp4

What alternative solutions did you explore? (Optional)

[garrettmknight]

Confirmed!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0146485d43d701eb2b

[melvin-bot]

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

[melvin-bot]

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

[s77rt]

@s-alves10 Thanks for the proposal. Your RCA is correct. The solution makes sense but the parameter modalOpen may require some maintenance, we have to manually specially this parameter for each option, right? I have been checking the logic for the options and I think we can do the following:

1. onItemSelected: set isKeyboardVisibleWhenShowingModalRef to false
2. onTriggerAttachmentPicker: set isKeyboardVisibleWhenShowingModalRef to true

Anytime a menu item is selected, we don't want the focus to trigger as we always navigate to some page, except for the attachment picker. Let me know if this would work.

Also, why you are changing setShouldBlockSuggestionCalc in onItemSelected? Is this required?

[s-alves10]

@s77rt

Yeah, that would work. And setShouldBlockSuggestionCalc isn't needed.

[s-alves10]

Proposal

Updated

[s77rt]

@s-alves10 Thanks for the update. That looks good to me, except the last part: useEffect - suggestionsRef.current.setShouldBlockSuggestionCalc I don't see why this is needed. Is it?

[s-alves10]

@s77rt

I added that for showing suggestions when modal is closed, but I don't think this is needed

Proposal

Updated

[s77rt]

@s-alves10 Upon further testing I noticed the suggestion box does not show up reliably. Can you please check the root cause behind that?

Screen.Recording.2023-09-05.at.5.21.09.PM.mov

[s-alves10]

@s77rt

We set the shouldBlockSuggestionCalc flag to true when we click the composer as you can see below

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Line 474 in 1bdc5ac

onClick={setShouldBlockSuggestionCalc}

This was introduced by this PR.
We need to change true to false at the below line

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Line 328 in 1bdc5ac

return suggestionsRef.current.setShouldBlockSuggestionCalc(true);

[s77rt]

@s-alves10 I don't think we want to unblock that. It's done in purpose. I think in most of the cases the suggestion box will show up if it was shown before but won't show up if it was not. Can you please confirm if that's the case or am I missing something?

[s-alves10]

@s77rt
Please check the PR. I'm sure the above change is an mistake during the refactoring

[s77rt]

@s-alves10 Right, previously we used to actually unblock on click, now we block on click. I asked here if it's intended.

[s77rt]

@s-alves10 Looks like that change was mostly unintended. Can you please update your proposal to include this fix #26489 (comment).

[s-alves10]

Proposal

Updated

[s77rt]

@s-alves10 Thanks for the update. This looks good to me 👍 .

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @s-alves10 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[s-alves10]

@s77rt

PR is ready for review

[melvin-bot]

🎯 ⚡️ Woah @s77rt / @s-alves10, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @s-alves10 got assigned: 2023-09-11 09:38:30 Z
• when the PR got merged: 2023-09-13 02:47:13 UTC

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.69-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: don't focus on composer when open right modal #27155

If no regressions arise, payment will be issued on 2023-09-21. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment offer (Reviewer)
• @s-alves10 requires payment offer (Contributor)
• @dhanashree-sawant requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@garrettmknight] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: fix keyboard flashing while clicking "Add attachment" #23994
• The offending PR has been commented on: fix keyboard flashing while clicking "Add attachment" #23994 (comment)
• A discussion in #expensify-bugs has been started: Not needed. Bug could have been caught during testing
• Determine if we should create a regression test for this bug: No

[ntdiary]

Hi, isKeyboardVisibleWhenShowingModalRef represents the keyboard state before showing the modal, which could be visible or hidden. If we set it to true here, it will lose its original meaning.

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Line 277 in 9b302c9

isKeyboardVisibleWhenShowingModalRef.current = true;

Actually, BaseModal's onClose is not triggered when clicking on the modal options, but for web platform's PopoverWithoutOverlay, onClose is triggered. I think what we need to do here is to standardize the modal callback instead of changing the value of isKeyboardVisibleWhenShowingModalRef.

[garrettmknight]

Summary of payments for this issue:

• Urgency bonus? yes
• Reporter: @dhairyasenjaliya $50 paid via Upwork
• Contributor: @s-alves10 $750 paid via Upwork
• Contributor+: @s77rt $750 paid via Upwork

Upwork job: https://www.upwork.com/jobs/~0146485d43d701eb2b

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.