Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] mWeb - The keyboard opens after requesting money in 1:1 DM #31588

Closed
1 of 6 tasks
kbecciv opened this issue Nov 20, 2023 · 18 comments
Closed
1 of 6 tasks

[$500] mWeb - The keyboard opens after requesting money in 1:1 DM #31588

kbecciv opened this issue Nov 20, 2023 · 18 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Nov 20, 2023

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.1.5
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. Go to https://staging.new.expensify.com/
  2. Log in with any account.
  3. Navigate to 1:1 DM.
  4. Tap on + then "Request money" Manual.
  5. Enter an amount into the BNP and press Next button.
  6. Tap on Request button.

Expected Result:

The keyboard does not open after requesting money in 1:1 DM

Actual Result:

The keyboard opens after requesting money in 1:1 DM

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

Bug6284591_1700517243513.Request.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01cf43a55cc9e8c20b
  • Upwork Job ID: 1726723624521633792
  • Last Price Increase: 2023-11-20
  • Automatic offers:
    • s77rt | Reviewer | 27889459
    • wlegolas | Contributor | 27889460
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 20, 2023
@melvin-bot melvin-bot bot changed the title mWeb - The keyboard opens after requesting money in 1:1 DM [$500] mWeb - The keyboard opens after requesting money in 1:1 DM Nov 20, 2023
Copy link

melvin-bot bot commented Nov 20, 2023

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

Copy link

melvin-bot bot commented Nov 20, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 20, 2023
Copy link

melvin-bot bot commented Nov 20, 2023

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

Copy link

melvin-bot bot commented Nov 20, 2023

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

@wlegolas
Copy link
Contributor

wlegolas commented Nov 21, 2023

Proposal

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

mWeb - The keyboard opens after requesting money in 1:1 DM

What is the root cause of that problem?

When the modal is closed there is a solution in the ComposerWithSuggestions to focus on the Compose field. In this solution, there is a condition to avoid the focus on native platforms, but the mWeb view is not present in this condition.

// We want to focus or refocus the input when a modal has been closed or the underlying screen is refocused.
// We avoid doing this on native platforms since the software keyboard popping
// open creates a jarring and broken UX.
if (!(willBlurTextInputOnTapOutside && !isNextModalWillOpenRef.current && !modal.isVisible && isFocused && (prevIsModalVisible || !prevIsFocused))) {
return;
}

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

We should replace the condition to use shouldFocusInputOnScreenFocus in place of the willBlurTextInputOnTapOutside:

For example,

useEffect(() => {
    if (modal.isVisible && !prevIsModalVisible) {
        // eslint-disable-next-line no-param-reassign
        isNextModalWillOpenRef.current = false;
    }
    // We want to focus or refocus the input when a modal has been closed or the underlying screen is refocused.
    // We avoid doing this on native platforms since the software keyboard popping
    // open creates a jarring and broken UX.
    if (!(shouldFocusInputOnScreenFocus && !isNextModalWillOpenRef.current && !modal.isVisible && isFocused && (prevIsModalVisible || !prevIsFocused))) {
        return;
    }

    if (editFocused) {
        InputFocus.inputFocusChange(false);
        return;
    }
    focus();
}, [focus, prevIsFocused, editFocused, prevIsModalVisible, isFocused, modal.isVisible, isNextModalWillOpenRef]);

What alternative solutions did you explore? (Optional)

N/A

POC

poc-issue-31588.mp4

Evidence with the focus using a Desktop Browser

Note: It's just evidence to ensure that the focus still working in the Desktop Browser

regration-test-evidence.mov

@s77rt
Copy link
Contributor

s77rt commented Nov 21, 2023

@wlegolas Thanks for the proposal. Your RCA makes sense. The solution would work but I think we can just use shouldFocusInputOnScreenFocus condition instead of willBlurTextInputOnTapOutside. If that looks good please update your proposal and tag me again.

@wlegolas
Copy link
Contributor

Hi @s77rt thank you for your suggestion.

I tested it here and works as expected, I updated my proposal to make this change.

@s77rt
Copy link
Contributor

s77rt commented Nov 22, 2023

@wlegolas Thanks for the confirmation. Let's get this fixed 🚀

🎀 👀 🎀 C+ reviewed
Link to proposal

Copy link

melvin-bot bot commented Nov 22, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Nov 24, 2023
@s77rt
Copy link
Contributor

s77rt commented Nov 24, 2023

Not overdue. We have a proposal. Waiting assignment

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 24, 2023
@s77rt
Copy link
Contributor

s77rt commented Nov 27, 2023

@jasperhuangg #31588 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Nov 27, 2023
@melvin-bot melvin-bot bot added Overdue and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Nov 29, 2023
Copy link

melvin-bot bot commented Nov 29, 2023

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

Copy link

melvin-bot bot commented Nov 29, 2023

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

@strepanier03
Copy link
Contributor

We're good here Melvin, assignment has been completed.

@melvin-bot melvin-bot bot removed the Overdue label Nov 30, 2023
@0xmiros
Copy link
Contributor

0xmiros commented Nov 30, 2023

Sorry for late. This was dupe of #27540.
I didn't approve same proposal because it still didn't meet the expected behavior.
We should fix refocus behavior globally, not only native vs mWeb inconsistency.
Also, I suggest to hold this for #29199

@wlegolas
Copy link
Contributor

Hi @0xmiroslav and @s77rt do I need to stop the changes that I'm working on?

I almost finished the changes and I'm just trying to generate the last test evidence using the Android Native. For all the other platforms I created the test evidence.

@0xmiros
Copy link
Contributor

0xmiros commented Nov 30, 2023

I think so. As you see, that code part is being fully removed here - https://github.com/Expensify/App/pull/29199/files#diff-4e5fc12f136b6de8d193f9da851a61da009309c463307e00f444aaf0cd55fdd6L491-L510

@jasperhuangg
Copy link
Contributor

Got it, sorry for the confusioin @wlegolas, let's go ahead and close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

6 participants