[HOLD for payment 2023-10-02] [$500] IOU - Losing focus on the amount input field when switching between apps

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

---

Action Performed:

1. Open app in fullscreen view
2. Tap on FAB
3. Tap on Request money or Split bill
4. Switch to other app by swiping with 3 fingers gesture (do not use comant +tab)
5. Open back the New Expensify app by swiping with 3 fingers gesture (do not use comant +tab)
6. Press any number keys on the keyboard

Expected Result:

Amount input label should be focused when user switched back to the app

Actual Result:

Losing focus on the amount input field for Request Money/Split bills when switching between apps by 3 fingers swipe gesture when app in fullscreen view

Workaround:

Unknown

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?: Yes

Reproducible in production?: Yes

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

Bug6185121_Recording__279.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f419594d979a6b3d
• Upwork Job ID: 1700176267856764928
• Last Price Increase: 2023-09-15
• Automatic offers:
• tienifr | Contributor | 26746042

[ishpaul777]

Proposal

Problem

IOU - Losing focus on the amount input field when switching between apps

Root Cause

When user is switch window the focus is lost from text input and when user is back to app input is not focussed

Chnages

we can use visibilitylistner to focus back on textInput in MoneyRequestAmountForm when user switch back to app.

useEffect(() => {
        const unsubscribeVisibilityListener = Visibility.onVisibilityChange(() => {
            // focus the text input when the app is back in the foreground
            if (Visibility.isVisible() && textInput.current) {
                textInput.current.focus();
            }
        });
        return () => {
            unsubscribeVisibilityListener();
        };
    }, []);

Without Changes

Screen.Recording.2023-09-02.at.3.33.58.PM.mov

With Changes

Screen.Recording.2023-09-02.at.3.37.11.PM.mov

[melvin-bot]

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

[ishpaul777]

@lanitochka17 No one is assigned to the issue, looks like automation failed.

[melvin-bot]

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

[CortneyOfstad]

Getting eyes on this 👍

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[pradeepmdk]

Proposal

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

Losing focus on the amount of input field when switching between apps

What is the root cause of that problem?

App/src/components/TextInput/index.js

Line 25 in 153ab36

if (!Browser.isMobileChrome() || !Visibility.isVisible() || !textInputRef.current || DomUtils.getActiveElement() !== textInputRef.current) {

here two is issues First we allow only mobile chrome and DomUtils.getActiveElement() when the app is switched between apps focused on the body tag.

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

we can use new props isVisibleFoucs on TextInput we will pass in the param from MoneyRequestAmountForm (these props pass all child components) If isVisibleFoucs props are true means we set the focus.

[tienifr]

Proposal

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

Losing focus on the amount input field for Request Money/Split bills when switching between apps by 3 fingers swipe gesture when app in fullscreen view

What is the root cause of that problem?

This issue only happens for the amount input, no other inputs have the issue (it always regains focus after the app is foregrounded).

It's because we disableKeyboard on the amount input here. When that flag is true, we have this logic to call Keyboard.dismiss whenever the app is backgrounded. When Keyboard.dismiss is called, the current focused input will be blurred, so when the app is foregrounded it will not refocus on the input. It will only return focus if the input loses focus due to the backgrounding, not when we triggered it ourselves by clicking out of the input, or using Keyboard.dismiss.

That Keyboard.dismiss was added as a fix this RN inconsistency where in native, showSoftInputOnFocus does not work if the app is backgrounded, then foregrounded.

That inconsistency only occurs on Android, and according to "What do I think?" here, it's not really a bug, just the way it works in that platform. And according to point 5, we don't have to refocus the input in that case because the user can already immediately type on the numbers in the screen.

So the problem here is, that consistency fix is affecting other platforms (other than Android).

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

We move this useEffect block to a platform specific file (.android.js, if we want iOS to also dismiss the keyboard, .native.js). That is ok because in Android/iOS, the user can already immediately type the numbers in our own defined keyboard, so there's no need for the input to regain focus.

So that inconsistency fix won't have any negative impact on other platforms like web/mWeb/Desktop. Especially in web/Desktop, we need the refocusing behavior for the users to type the value.

What alternative solutions did you explore? (Optional)

We could try to fix the inconsistency in the RN repo, but as mentioned above, it might not be accepted because it might be how it works in the platform. Also, it doesn't bring much value since refocusing is not needed for Android/iOS.

[CortneyOfstad]

@eVoloshchak any thoughts on the proposals above?

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @tienifr got assigned: 2023-09-19 04:05:42 Z
• when the PR got merged: 2023-09-22 08:08:07 UTC
• days elapsed: 3

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

• fix: 26505 Losing focus on the amount input field when switching between apps #27749

If no regressions arise, payment will be issued on 2023-10-02. 🎊

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 — Applause, so does not require payment
• Contributor that fixed the issue — @tienifr paid **$500 (**does not qualify to the bonus based)
• Contributor+ that helped on the issue and/or PR — @eVoloshchak paid via NewDot ($500)

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

• @eVoloshchak does not require payment (Eligible for Manual Requests)
• @tienifr requires payment offer (Contributor)

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:

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

[CortneyOfstad]

Payment Breakdown

• Reporter — Applause, so no payment
• Contributor — @tienifr, paid $500 as it did not qualify for bonus
• C+ — @eVoloshchak to be paid in NewDot ($500)

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: Changed the Amount Text to TextInput #6811
• 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: https://github.com/Expensify/App/pull/6811/files#r1345751207
• 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: I don't think an additional discussion is needed, this type of bug is easy to catch when testing, the only complication here is that it's specific to only a single platform (Desktop)
• Determine if we should create a regression test for this bug. While the bug doesn't impact user experience, there is a probability of it re-appearing with a RN/Electron/MacOS update, all of which are outside of our control. I think we might want to implement a regression test

[eVoloshchak]

Regression Test Proposal
web/mweb/desktop

1. Open the app in fullscreen view
2. Tap on FAB
3. Tap on Request money or Split bill
4. Switch to another app by swiping with 3 fingers gesture (do not use command+tab)
5. Open back the New Expensify app by swiping with 3 fingers gesture (do not use command+tab)
6. Verify that the amount input is focused when you switched back to the app

Do we agree 👍 or 👎

[eVoloshchak]

when @tienifr got assigned: 2023-09-19 04:05:42 Z
when the PR got merged: 2023-09-22 08:08:07 UTC
the pull request did not get merged within 3 working days of assignment.

I don't think that's correct, the PR was merged on 22th, which isn't a working day, so it was merged in 2 business days

@CortneyOfstad, could you double-check that please?

[CortneyOfstad]

@thienlnam Does that sound correct to you ^^^

Just want a second pair of eyes on this

[thienlnam]

Why isn't the 22nd a working day? Granted the PR was approved and required no updates from me on the 21st, so we can give the bonus here

[eVoloshchak]

Why isn't the 22nd a working day?

Oh my bad, I was looking at the calendar for October by mistake
Sorry for the confusion😅

[CortneyOfstad]

No worries @eVoloshchak! I sent you a proposal in Upwork for the additional $250 bonus amount. Please let me know once you approve it!

Creating the regression test GH now 👍

[CortneyOfstad]

Checklist above updated with Regression Test link 👍

[eVoloshchak]

No need for Upwork, I'll just send the payment request via NewDot

[CortneyOfstad]

Sounds good! Closing!

[CortneyOfstad]

Payment Breakdown:

External issue reporter — Applause, so does not require payment
Contributor that fixed the issue — @tienifr paid **$500 (**does not qualify to the bonus based)
Contributor+ that helped on the issue and/or PR — @eVoloshchak paid via NewDot ($500) — additional $250 to be paid via NewDot

[JmillsExpensify]

$750 payment approved for @eVoloshchak based on summary immediately above.