[$500] Android - IOU-Tapping hold request, cursor is shown but keypad is not opened

[izarutskaya]

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

---

Version Number: 1.4.44
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team

Action Performed:

1. Launch app
2. Tap 1:1 report
3. Create request money
4. Tap IOU to open details page
5. Tap 3 dots on top
6. Tap hold request

Expected Result:

Tapping hold request, cursor is shown similarly keypad must be opened.

Actual Result:

Tapping hold request, cursor is shown but keypad is not opened.

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

Bug6393692_1709018301629.hold.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012dbcf06cfb05e557
• Upwork Job ID: 1762440810179457024
• Last Price Increase: 2024-02-27

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~012dbcf06cfb05e557

[melvin-bot]

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

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @rlinoz (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[izarutskaya]

We think that this bug might be related to #vip-bills
CC @davidcardoza

[neonbhai]

Proposal

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

Tapping hold request, cursor is shown but keypad is not opened

What is the root cause of that problem?

We are focusing using the autofocus prop on this page which tends to be unreliable

App/src/pages/iou/HoldReasonPage.tsx

Line 89 in 668df99

autoFocus

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

instead of passing prop here we should use the useAutoFocusInput hook to focus on this page like we do in other areas of the app

const {inputCallbackRef} = useAutoFocusInput();

ref={inputCallbackRef}

Alternatively

We can use the useFocusEffect logic

Code

Following the logic in TaskDescriptionPage:

const focusTimeoutRef = useRef(null);

useFocusEffect(
    useCallback(() => {
        focusTimeoutRef.current = setTimeout(() => {
            if (inputRef.current) {
                inputRef.current.focus();
            }
            return () => {
                if (!focusTimeoutRef.current) {
                    return;
                }
                clearTimeout(focusTimeoutRef.current);
            };
        }, CONST.ANIMATED_TRANSITION);
    }, []),
);

[ishpaul777]

@neonbhai Can you point to offending PR ?

[ishpaul777]

I can reproduce this in production so this can be dealt as normal bug, no need to block deploy for this one 🙃

[situchan]

@izarutskaya can you please test again on production android app with the same android device where you reproduced bug on staging build?

[ishpaul777]

Record_2024-02-27-18-58-33.mp4

[ikevin127]

Proposal

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

When the HoldReasonPage opens, the input is looks like it's focused but the keyboard doesn't show up.

What is the root cause of that problem?

I was not able to find any offending PR within the deploy checklist, so I'm assuming this was not caused by the latest deploy.

This being said, the root cause of why the keyboard does not open on Android even though the input looks like it's focused (we see focused styles and the cursor) is because the focus is called when the component is not completely ready, most likely because of the navigation transition. I can confirm this since in rare cases, the keyboard does show-up (randomly) and I think this varies due to individual device thread load.

This is the logic that handles the autofocus on mount on native platforms, see code block.

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

Within the native specific code block mentioned above we can observe that we already have logic implemented to delay the focusing on mount via shouldDelayFocus.

So within the HoldReasonPage we should pass shouldDelayFocus as true to the input component here, under / above the existing autoFocus prop.

Since this alone would still not show the keyboard 100% of the time (depending on individual device thread load), I suggest wrapping the focus call from within the existing shouldDelayFocus timeout with InteractionManager.runAfterInteractions - this way besides the timeout's 300ms delay, we add the interactions delay too. This ensures the keyboard will show 100% of the time.

Changed code:

if (shouldDelayFocus) {
    const focusTimeout = setTimeout(() => {
        InteractionManager.runAfterInteractions(() => input.current?.focus()); // <- this line
    }, CONST.ANIMATED_TRANSITION);
    return () => clearTimeout(focusTimeout);
}

Videos

Android: Native

Before	After
before.mov	after.mov

[rlinoz]

Even if we cannot reproduce this in prod, since we still can get the keyboard to show up, because of this I will remove the deploy blocker label.

[ishpaul777]

Thanks for your proposals @ikevin127 @neonbhai, both of the given solutions works, @ikevin127 has better explaination for the root cause but i prefer the solution the given by @neonbhai becuase that is sort of what preferred in docs https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#how-do-i-auto-focus-a-textinput-using-usefocuseffect, useAutoFocusInput is just more optimized hook designed following the same approach in our docs

@neonbhai proposal looks good to me.
🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @rlinoz is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[Krishna2323]

@ishpaul777, this is a dupe of #36948 with some different issue but the solution is the same. I think we should hold here.

[situchan]

yup, @Krishna2323 proposed first here

[ishpaul777]

@situchan if you dont mind can we move @Krishna2323 proposal here i assume you haven't started reviewing till now 🙇‍♂️

[situchan]

Ah sorry I just completed review. It was weekly and @Krishna2323 bumped me on slack so I already planned review today

[situchan]

I understand why this was marked as deploy blocker.
On both staging and prod, sometimes keyboard shows and sometimes not, even on same device.
Mostly auto focus doesn't work in android without timeout. Maybe it accidentally worked when QA team tested on production app.

[ikevin127]

I agree that @neonbhai's solution goes by the docs, but in this case the TextInput shouldDelayFocus logic doesn't work as expected because of the navigation transition delay and I think fixing that as suggested by my proposal here is a more holistic approach instead of using the useAutoFocusInput hook in every component where the issue occurs.

What do you guys think ?

cc @rlinoz @ishpaul777 @situchan

[situchan]

@ikevin127 useAutoFocusInput is not using TextInput shouldDelayFocus logic at all.
The solution is actually combined with autoFocus prop removal.

[situchan]

useFocusEffect approach is what we already discussed and landed on - https://expensify.slack.com/archives/C01GTK53T8Q/p1694660990479979

If you have better approach, please raise discussion on slack and get votes.

[ikevin127]

@situchan I understand how useAutoFocusInput works, my thinking here was to fix the TextInput shouldDelayFocus logic and just pass shouldDelayFocus besides the already existing autoFocus prop within the components where this issue occurs, instead of adding the hook and its logic to every component which has this issue -> as this would just mean duplicated code.

While the issue can be fixed from the root and just use the shouldDelayFocus wherever we encounter the issue.

Just my 2 cents ✌️

I'll comment on Slack and see what others think 🙏

Edit:
I put it up to vote on Slack: https://expensify.enterprise.slack.com/archives/C01GTK53T8Q/p1709053753771649

[rlinoz]

I think we should hold this in favor of #36948 since the proposal was first posted there.

Do you agree @situchan @ishpaul777 ?

[izarutskaya]

Production video

-4947531272222306079no56788.mp4

[rlinoz]

Moving to weekly since it is on HOLD

[rlinoz]

PR of the other issue is in draft.

[rlinoz]

PR of the other issue is in draft.

[rlinoz]

Other PR is deployed and fixed this one as expected, so closing.