[HOLD for payment 2023-10-02] [$500] Request money - No autofocus blink cursor shown in Request money page

[izarutskaya]

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. Go to fab button -> request money ->Manual
2. Notice that there is no blinking cursor beside the currency symbol .
   ( however if the request money page was previously set to scan or distance pages, such behaviour doesn't appear. Thus make sure after clicking request money, manual section is opened automatically)
3. Close the RHN and again go to fab button -> split bill
4. Notice that there appears blinking cursor beside currency symbol

Expected Result:

Consistency in pages in split money and request money ( Manual)

Actual Result:

No autofocus blink cursor shown in Request money page

Workaround:

Unknown

Platforms:

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

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

Version Number: v1.3.65-6

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

Screen.Recording.2023-09-03.At.5.33.37.Pm.mp4

2023-09-07.22.44.53.mp4

Expensify/Expensify Issue URL:

Issue reported by: @ashimsharma10

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693742251468319

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d0c4dd10133dafa2
• Upwork Job ID: 1701221973754032128
• Last Price Increase: 2023-09-11
• Automatic offers:
• fedirjh | Reviewer | 26670391
• Krishna2323 | Contributor | 26670393
• ashimsharma10 | Reporter | 26670394

[melvin-bot]

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

[Krishna2323]

Proposal

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

Request money - No autofocus blink cursor shown in Request money page

What is the root cause of that problem?

Safari's WebKit rendering engine interacts differently with the JavaScript event queue compared to other browsers. This divergence is particularly evident when programmatically focusing an input element. While InteractionManager.runAfterInteractions() is designed to execute actions after any active animations, its interaction with Safari results in differing behavior. Specifically, Safari may require direct user actions for certain tasks, including focusing on an input, leading to timing inconsistencies in script execution and due to this onBlur event is being triggered immediately after onFocus, and the focus is shifting to the body element, it indicates that there is some external factor causing the TextInput to lose its focus.

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

To resolve this, we should modify the focusTextInput method by introducing an internal setTimeout function with a delay of 0ms. This ensures the focus command is repositioned to the end of the event queue, effectively navigating around the idiosyncrasies of Safari.

Reference Code:
Current Implementation

Updated Code:

const focusTextInput = () => {
    // Component may not be initialized due to navigation transitions
    // Wait until interactions are complete before trying to focus
    InteractionManager.runAfterInteractions(() => {
        // Focus text input
        if (!textInput.current) {
            return;
        }

        textInput.current.focus();

        setTimeout(() => {
            if (!textInput.current) {
                return;
            }

            textInput.current.focus();
        }, 0);
    });
};

Result

Screen.Recording.2023-09-08.at.7.21.09.AM.mp4

Explanation

Using InteractionManager.runAfterInteractions() inside React Native (or in environments where it's supported) ensures that any work wrapped inside it gets executed after all animations and interactions have finished.

However, "finished" doesn't always mean "rendered to the screen," especially on some browsers or environments. There might be slight differences in how the browser's rendering engine processes updates to the DOM and how it queues JavaScript events.

When we're using InteractionManager.runAfterInteractions(), the intended behavior is to delay JavaScript execution until animations have completed, but this does not necessarily guarantee that the browser has completed its rendering process. In Safari's case, the rendering and JavaScript event queues might interact a bit differently than in other browsers.

By adding the setTimeout with a delay of 0ms, we're essentially placing the focus event at the end of the current JavaScript event queue. Even though the delay is 0ms, it doesn't execute immediately; instead, it waits for the current queue to be emptied. This slight delay, even though minuscule, gives Safari the time it needs to complete the rendering process, ensuring that the focus event works correctly.

To summarize, while InteractionManager.runAfterInteractions() ensures that JS execution is deferred until animations are done, it doesn't account for potential idiosyncrasies in browser rendering. The nested setTimeout, even with a 0ms delay, provides that extra assurance that the focus will be executed after the rendering process has completed in the Safari browser.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[tienifr]

Proposal

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

Request money - No autofocus blink cursor shown in Request money page

What is the root cause of that problem?

In

App/src/pages/iou/MoneyRequestSelectorPage.js

Line 73 in 5a153a3

<ScreenWrapper

we're already using ScreenWrapper, and we can get this.props.navigation in ScreenWrapper.

In

App/src/pages/iou/steps/NewRequestAmountPage.js

Line 175 in 5a153a3

<ScreenWrapper

we're still using ScreenWrapper, but there's no such thing as the this.props.navigation, that why onEntryTransitionEnd function is never called

=> focusTextInput is not called when the transition is ended

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

In MoneyRequestSelectorPage we create new state transitionEnded and set it to true in onEntryTransitionEnd function

        <ScreenWrapper
       ...
            onEntryTransitionEnd={()=>{
                setTransitionEnded(true)
            }}
        >

and pass transitionEnded to NewRequestAmountPage

In NewRequestAmountPage we focus the input when transitionEnded is true

    useEffect(()=>{
        if(transitionEnded){
            focusTextInput()
        }
    },[transitionEnded])

Result

Screen.Recording.2023-09-12.at.17.23.24.mov

[melvin-bot]

@NicMendonca, @fedirjh Eep! 4 days overdue now. Issues have feelings too...

[fedirjh]

it indicates that there is some external factor causing the TextInput to lose its focus.

@Krishna2323 What's the external factor you referring to? why does the split bill page focus the input correctly while the request money page doesn’t?

---

@tienifr The root cause is not clear to me can you elaborate more?

=> focusTextInput is not called when the transition is ended

I don't think this is correct. The input appears to be initially focused correctly, but it loses focus instantly. Please check the video below. I added a console log inside the focus function, and it seems that the focus works as expected. However, it promptly loses focus, which is clearly noticeable.

CleanShot.2023-09-13.at.10.17.05.mp4

[tienifr]

@fedirjh As we can see there're 2 places that call focusTextInput, focusTextInput is called when we open the page (useFocusEffect), but it is not called when the transition is ended (onEntryTransitionEnd)

[tienifr]

After the input is focused (in useFocusEffect), there're some transitions of navigation (this causes the input blur), that why we need to wait for them end before focusing on the input

[fedirjh]

why does the split bill page focus the input correctly? It has the same transition of navigation.

[tienifr]

Ah, because in Request money page, we use TopTab, and it requires some extra transitions.

[tienifr]

run focusTextInput in useFocusEffect can not make sure it runs after the transition is end.

[Krishna2323]

Proposal (Updated)

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

Request money - No autofocus blink cursor shown in Request money page.

What is the root cause of that problem?

Upon investigation, the root cause appears to be the default behavior of the TopTab.Navigator. Without explicit configuration, the navigator might interfere with the keyboard's display settings, leading to its unintended dismissal or erratic behavior, especially when switching tabs or interacting with input fields.

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

To counteract this issue, we should modify our TopTab.Navigator configuration by setting the keyboardDismissMode property to "none". This would ensure that the keyboard remains consistent in its behavior, regardless of interactions within the navigator.

By applying this change, we will override the default behavior and prevent the keyboard from dismissing due to tab interactions.

We just need to pass keyboardDismissMode={"none"} here.

App/src/libs/Navigation/OnyxTabNavigator.js

Lines 30 to 47 in d68263a

	<TopTab.Navigator
	/* eslint-disable-next-line react/jsx-props-no-spreading */
	{...rest}
	id={id}
	initialRouteName={selectedTab}
	backBehavior="initialRoute"
	screenListeners={{
	state: (event) => {
	const state = event.data.state;
	const index = state.index;
	const routeNames = state.routeNames;
	Tab.setSelectedTab(id, routeNames[index]);
	},
	...(rest.screenListeners || {}),
	}}
	>
	{children}
	</TopTab.Navigator>

Result

Screen.Recording.2023-09-13.at.4.27.18.PM.mov

[Krishna2323]

@fedirjh, pls review updated proposal.

[fedirjh]

@Krishna2323 That makes sense to me. I believe it will provide us with greater flexibility in managing the keyboard.

This proposal from @Krishna2323 looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[Krishna2323]

@fedirjh, PR is ready for review.

[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 @Krishna2323 got assigned: 2023-09-14 09:26:04 Z
• when the PR got merged: 2023-09-21 08:53:22 UTC
• days elapsed: 4

On to the next one 🚀

[Krishna2323]

@MariaHCD, considering we also addressed two additional issues and successfully resolved one of them, would this qualify for a bonus?

[MariaHCD]

@Krishna2323 I believe the additional issue identified had the same root cause so I am not sure this would be eligible for a bonus. We normally pay out bonuses if the issue/solution becomes more complicated/challenging and the contributor has to invest more time/effort into the issue.

However, I will defer to @NicMendonca

@Krishna2323 could you can explain why you feel your work on this issue deserves a bonus?

[Krishna2323]

@MariaHCD , the PR was ready to be merged quite early. However, we tested this issue afterward, which added an additional time.

I'm inquiring about the urgency bonus and kindly asking if we're eligible.

[MariaHCD]

Hmm, so we tried to fix the same issue on iOS native but that led to a bit of a delay in getting the PR merged. I think it's all part of the review process and you did handle addressing the reviews with urgency so I think paying out the bonus in this case is fine. Regardless, I'll leave the final call to @NicMendonca

[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: Request money - No autofocus blink cursor shown in Request money page #27475

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

• @fedirjh requires payment offer (Reviewer)
• @Krishna2323 requires payment offer (Contributor)
• @ashimsharma10 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:

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

[NicMendonca]

@fedirjh bump on the BZ issue ^^

[NicMendonca]

Reporter: @ashimsharma10 - $50
Contributor: @Krishna2323 - $750
C+ @fedirjh - $750

[NicMendonca]

@Krishna2323 @ashimsharma10 - you've both been paid 🎉

@fedirjh can you please accept the offer in Upwork?

[Krishna2323]

@NicMendonca Thank you very much for the bonus, I appreciate it.

[NicMendonca]

@fedirjh bump bump on BZ checklist

[fedirjh]

BugZero Checklist:

• Link to the PR: Add ability to Take/Upload Receipts #23046
• Link to comment: Add ability to Take/Upload Receipts #23046 (comment)
• Link to discussion: N/A Regression test should help prevent this bug in the future
• Determine if we should create a regression test for this bug: 🟢

---

Regression Test Proposal

1. Go to fab button -> Request money
2. Switch to the manual tab
3. Verify that the amount input is focused and the cursor is blinking.

• Do we agree 👍 or 👎

[NicMendonca]

@fedirjh thanks! You've been paid via Upwork🎉

All set here ✅