[HOLD for payment 2024-10-29] [HOLD for payment 2024-10-25] [$250] Android - IOS - It flickers when I open the manual submit expense twice or more

[IuliiaHerets]

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: 9.0.43-1
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #49007
Issue reported by: Applause Internal Team

Action Performed:

1. Open the app
2. Log in with a Gmail account
3. Navigate to FAB - Submit expense - Manual
4. Navigate back to the LHN
5. Navigate to FAB - Submit expense

Expected Result:

It shouldn't flicker.

Actual Result:

It flickers when I open the manual submit expense twice or more.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6622398_1727890757439.flicker.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021843392417403890622
• Upwork Job ID: 1843392417403890622
• Last Price Increase: 2024-10-14
• Automatic offers:
• rayane-djouah | Reviewer | 104420246

Issue Owner

Current Issue Owner: @slafortune

[melvin-bot]

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

We think that this bug might be related to #wave-collect - Release 1

[IuliiaHerets]

@slafortune FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[mkzie2]

Proposal

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

• It flickers when I open the manual submit expense twice or more.

What is the root cause of that problem?

• In IOURequestStartPage, we display the loading indicator if the transaction data is not available:

App/src/pages/iou/request/IOURequestStartPage.tsx

Line 101 in e02df29

return <FullScreenLoadingIndicator />;

• If the transaction data is loaded successfully, we display the tab navigator:

App/src/pages/iou/request/IOURequestStartPage.tsx

Line 137 in e02df29

<OnyxTabNavigator

• In the tab navigator, we also have logic that will return null if the selectedTab data is loading:

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 106 to 108 in e02df29

	if (isLoadingOnyxValue(selectedTabResult)) {
	return null;
	}

• That leads to is flicker effect. The order of displayed components: FullScreenLoadingIndicator > Navigator with 3 tab (Manual | Scan | Distance) > null > Navigator with 3 tab (Manual | Scan | Distance).

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

• We should display the loading indicator instead of just returning null in:

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 106 to 108 in e02df29

	if (isLoadingOnyxValue(selectedTabResult)) {
	return null;
	}

Now, the flicker effect still appears. The order of displayed components: FullScreenLoadingIndicator > Navigator with 3 tab (Manual | Scan | Distance) > FullScreenLoadingIndicator > Navigator with 3 tab (Manual | Scan | Distance).

• We should change the order to: FullScreenLoadingIndicator > Navigator with 3 tab (Manual | Scan | Distance). To do it, we need to remove:

App/src/pages/iou/request/IOURequestStartPage.tsx

Lines 98 to 102 in e02df29

	if (!transaction?.transactionID) {
	// The draft transaction is initialized only after the component is mounted,
	// which will lead to briefly displaying the Not Found page without this loader.
	return <FullScreenLoadingIndicator />;
	}

and pass isLoading={!transaction?.transactionID} when using OnyxTabNavigator here.

• In OnyxTabNavigator, update:

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 106 to 108 in e02df29

	if (isLoadingOnyxValue(selectedTabResult)) {
	return null;
	}

    if (isLoadingOnyxValue(selectedTabResult) || isLoading) {
        return <FullScreenLoadingIndicator />;
    }

• Beside the above main change, we need to make sure the changes do not break the rest logic in IOURequestStartPage:

App/src/pages/iou/request/IOURequestStartPage.tsx

Lines 173 to 181 in e02df29

	<FocusTrapContainerElement
	onContainerElementChanged={setActiveTabContainerElement}
	style={[styles.flexColumn, styles.flex1]}
	>
	<IOURequestStepAmount
	route={route}
	shouldKeepUserInput
	/>
	</FocusTrapContainerElement>

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

The submit expense page flickers when open it.

What is the root cause of that problem?

In submit expense page, we use an OnyxTabNavigator. Inside the component, we return null if the selectedTab onyx data is loading.

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 106 to 108 in e61ee7f

	if (isLoadingOnyxValue(selectedTabResult)) {
	return null;
	}

If we already previously loaded the data before, then the tab navigator will be shown immediately. But the navigator has a state listener which is triggered on mount which updates the selectedTab onyx with the same value.

App/src/libs/Navigation/OnyxTabNavigator.tsx

Lines 120 to 126 in e61ee7f

	screenListeners={{
	state: (e) => {
	const event = e as unknown as EventMapCore<NavigationState>['state'];
	const state = event.data.state;
	const index = state.index;
	const routeNames = state.routeNames;
	Tab.setSelectedTab(id, routeNames.at(index) as SelectedTabRequest);

It will make the status becomes loading again because now we have a pending merge.
https://github.com/Expensify/react-native-onyx/blob/22b2717e4b6a21b444efe59048ead7cb795b8671/lib/useOnyx.ts#L177-L179

loaded -> loading -> loaded

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

Don't update the onyx if the selected tab is the same.

const newSelectedTab = routeNames.at(index);
if (selectedTab !== newSelectedTab) {
    Tab.setSelectedTab(id, newSelectedTab as SelectedTabRequest);
    onTabSelected(newSelectedTab as IOURequestType);
}

[slafortune]

Yep - tested and get the same results 👍

[melvin-bot]

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

[melvin-bot]

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

[rayane-djouah]

Reviewing 👀

[rayane-djouah]

@bernhardoj's proposal looks good to me

🎀👀🎀 C+ reviewed

[melvin-bot]

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

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

📣 @rayane-djouah 🎉 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]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.50-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix money request page flickers when open #50772
• Fix money request is initialized with the wrong information #51040
• [CP Staging] Fix missing merchant and date field in invoice confirm page #51083

If no regressions arise, payment will be issued on 2024-10-25. 🎊

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

• @bernhardoj requires payment through NewDot Manual Requests
• @rayane-djouah requires payment automatic offer (Reviewer)

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

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

[rayane-djouah]

BugZero Checklist

• The PR that introduced the bug has been identified. Link to the PR: fix: default the selected tab to scan #47310
• 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/47310/files#r1807441500
• 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. Navigate to FAB > Submit Expense > Manual.
2. Navigate back.
3. Navigate to FAB > Submit Expense again.
4. Expected Result:
5. Verify that the page does not flicker when opening the manual submit expense option multiple times.

Do we agree 👍 or 👎

[marcaaron]

LGTM

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.51-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix money request is initialized with the wrong information #51040
• [CP Staging] Fix missing merchant and date field in invoice confirm page #51083

If no regressions arise, payment will be issued on 2024-10-29. 🎊

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

• @bernhardoj requires payment through NewDot Manual Requests
• @rayane-djouah requires payment automatic offer (Reviewer)

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

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

[rayane-djouah]

The checklist has already been completed #50098 (comment)

[melvin-bot]

@slafortune, @marcaaron, @bernhardoj, @rayane-djouah Whoops! This issue is 2 days overdue. Let's get this updated quick!

[marcaaron]

Are we still waiting for payment here?

[bernhardoj]

Yes.

[slafortune]

Payment is due today -

[slafortune]

@bernhardoj Role Contributor - requires payment through NewDot Manual Requests - Due $250
@rayane-djouah Role C+ requires payment via Upworks - Paid $250

[melvin-bot]

@slafortune @marcaaron Be sure to fill out the Contact List!

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj