[HOLD for payment 2024-02-07] [$500] Chat - Keyboard displayed over compose box when user returned from Distance Request Money

[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: 1.4.26-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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Start a distance money request in the workspace chat
2. Select the start and finish
3. Add stop
4. Tap on the stop
5. Tap on 3 dots button > delete waypoint
6. Tap on back button on top left corner

Expected Result:

Chat should be properly displayed

Actual Result:

Keyboard displayed over compose box when user returned from Distance Request Money

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

Bug6346204_1705567130891.RPReplay_Final1705534257.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015a375a0746825e33
• Upwork Job ID: 1747976157864992768
• Last Price Increase: 2024-01-25
• Automatic offers:
• jjcoffee | Reviewer | 28122752
• s-alves10 | Contributor | 28122753

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015a375a0746825e33

[melvin-bot]

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

[melvin-bot]

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

[s-alves10]

Proposal

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

Keyboard is opened over compose box when returning from distance request money if delete a stop waypoint

What is the root cause of that problem?

When deleting a stop waypoint, we open PopoverMenu, and selecting Delete waypoint opens the ConfirmModal.
Both PopoverMenu and ConfirmModal have shouldSetModalVisibility props with default value true. So when they are closed, ONYXKEYS.MODAL value is changed.

Please take a look at the below code snippet

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

Lines 492 to 500 in c14ca54

	if (!((willBlurTextInputOnTapOutside || shouldAutoFocus) && !isNextModalWillOpenRef.current && !modal.isVisible && isFocused && (prevIsModalVisible || !prevIsFocused))) {
	return;
	}
	if (editFocused) {
	InputFocus.inputFocusChange(false);
	return;
	}
	focus(true);

As you can see, we focus the composer when modal is invisible and the route is focused if modal is invisible or route is not focused previously.

When ConfirmModal is closed, modal.isVisible becomes false and isFocused is false. When navigating back to the chat, isFocused becomes true and so focus function is called. Route gets focused before animation is finished and focus is called while animating. This is the root cause

Note: This issue doesn't happen when not deleting a stop waypoint. Why?
When navigating back to the chat without deleting any waypoint, isFocused becomes false. And when the modal is completely hidden, modal.isVisible becomes false. So the focus function gets called after the animation is finished. This is the reason

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

I think we should not change the modal visibility while modal is opened

• Add shouldSetModalVisibility={false} props here and here
• Add shouldSetModalVisibility props to the HeaderWithBackButton component with default value true and pass it to the ThreeDotsMenu component here

This works as expected

Result

34724.mp4

What alternative solutions did you explore? (Optional)

Solution 1
We can call focus after the animation is finished
Update

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

Line 500 in c14ca54

focus(true);

to

        InteractionManager.runAfterInteractions(() => {
            focus(true);
        });

Solution 2
We can check the modal visibility on component mount and disable set modal visibility even when the shouldSetModalVisibility is true in both components. This solution looks better but we need to be more careful about if there is no other cases

[sophiepintoraetz]

Waiting on @jjcoffee to review the proposal - also surfaced it in Wave 5 channel

[jjcoffee]

Reviewing tomorrow!

[jjcoffee]

Happy with @s-alves10's proposal! We are just unnecessarily setting modal visibility, which causes focus to get called on the composer too early.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[situchan]

I think we should hold this for @ntdiary's modal refactor

[jjcoffee]

Ah thanks for flagging @situchan! Looking at it, that PR looks like it's going to take a while, so I think it's probably worth fixing this in the meantime, happy to defer to your judgement though @jasperhuangg!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

📣 @jjcoffee 🎉 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 📖

[s-alves10]

@jjcoffee
PR is ready for review #35220

[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.4.34-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: disable set modal visibility in waypoint edit page #35220

If no regressions arise, payment will be issued on 2024-02-07. 🎊

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

• @jjcoffee requires payment automatic offer (Reviewer)
• @s-alves10 requires payment automatic offer (Contributor)

[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:

• [@jjcoffee] The PR that introduced the bug has been identified. Link to the PR:
• [@jjcoffee] 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:
• [@jjcoffee] 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:
• [@jjcoffee] Determine if we should create a regression test for this bug.
• [@jjcoffee] 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.
• [@sophiepintoraetz] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - seems like this behaviour has been around for a while
• 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: N/A
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• 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.

Regression Test Proposal

1. Open ND and log in with any account
2. Start a distance money request in the workspace chat
3. Select the start and finish
4. Add stop
5. Tap on the stop
6. Tap on 3 dots button > delete waypoint
7. Tap on back button on top left corner
8. Verify that keyboard doesn't appear over composer

Do we agree 👍 or 👎

[melvin-bot]

@jjcoffee, @jasperhuangg, @adelekennedy, @s-alves10 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[adelekennedy]

regression test makes sense to me - @jasperhuangg for your thoughts as well!

[adelekennedy]

still not set on regression test steps but issuing payment

Payouts due:

Contributor: $500 @s-alves10 (Upwork)
Contributor+: $500 @jjcoffee (Upwork)

[jasperhuangg]

@adelekennedy agree! Thanks @jjcoffee for writing that up. I gave you thumbs

[melvin-bot]

@jjcoffee, @jasperhuangg, @adelekennedy, @s-alves10 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[adelekennedy]

test issue