[HOLD for payment 2024-06-06] [$500] Android - Blank screen appears when transitioning between pages after relaunching app

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

---

Version Number: 1.4-37.0
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. Launch New Expensify app
2. Kill the app
3. Relaunch New Expensify app
4. Open chat with IOU
5. Tap on the preview to go to report

Expected Result:

There will be no blank page when transitioning between pages

Actual Result:

In Step 4 and 5, there is a blank screen when transitioning between pages

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

Bug6369376_1707228967576.Screen_Recording_20240206_094858_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01902a14c0b46d4b01
• Upwork Job ID: 1754876717376974848
• Last Price Increase: 2024-02-06
• Automatic offers:
• ntdiary | Reviewer | 0
• bernhardoj | Contributor | 0

Issue Owner

Current Issue Owner: @kevinksullivan

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[bernhardoj]

Proposal

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

A black screen is showing when opening a report screen for the first time.

What is the root cause of that problem?

The black screen is because withOnyx returns null while some of the onyx data is unavailable yet and we are loading it.
https://github.com/Expensify/react-native-onyx/blob/13936aee27bb0cb8c98cb6bf3941044f3d5958ce/lib/withOnyx.js#L315-L317

The data that are unavailable yet are isComposerFullSize, accountManagerReportID, and userLeavingStatus, but we already set the initialValue for these data, so it should be available immediately.

App/src/pages/home/ReportScreen.js

Lines 623 to 626 in 13f832c

	isComposerFullSize: {
	key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${getReportID(route)}`,
	initialValue: false,
	},

App/src/pages/home/ReportScreen.js

Lines 634 to 637 in 13f832c

	accountManagerReportID: {
	key: ONYXKEYS.ACCOUNT_MANAGER_REPORT_ID,
	initialValue: null,
	},

App/src/pages/home/ReportScreen.js

Lines 638 to 641 in 13f832c

	userLeavingStatus: {
	key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${getReportID(route)}`,
	initialValue: false,
	},

However, we only set the initial value if the value is truthy and both null and false are not.
https://github.com/Expensify/react-native-onyx/blob/13936aee27bb0cb8c98cb6bf3941044f3d5958ce/lib/withOnyx.js#L61-L64

if (!value && mapping.initialValue) {
    value = mapping.initialValue;
}

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

Update react-native-onyx to apply the initial state if it's not undefined.

if (!value && mapping.initialValue !== undefined) {
    value = mapping.initialValue;
}

Notice that the value could be applied to the initial value if the onyx value is falsey, for example, if we have an onyx data that stores a boolean and has a false value, and we set an initial value of 1, then the value will be 1 instead of false. We can apply the same fix by checking whether it's undefined or not.

We can even simplify the above condition to:

if (value === undefined) {
    value = mapping.initialValue;
}

We already guard the undefined value here, so I think we don't need it twice.

[ntdiary]

Under review.

[ntdiary]

I think @bernhardoj's proposal makes sense. Additionally, this change will also affect other components, so we need to test them to avoid regressions. :)

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

@yuwenmemon, @ntdiary, @kevinksullivan Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[yuwenmemon]

Thanks @ntdiary !

[melvin-bot]

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

[yuwenmemon]

@bernhardoj Let us know when we can expect to see a PR

[bernhardoj]

Will open the PR today or tomorrow

[bernhardoj]

The onyx PR is here

cc: @ntdiary

[melvin-bot]

@yuwenmemon, @ntdiary, @kevinksullivan, @bernhardoj Huh... This is 4 days overdue. Who can take care of this?

[bernhardoj]

App PR is ready for review

cc: @ntdiary

[melvin-bot]

This issue has not been updated in over 15 days. @yuwenmemon, @ntdiary, @kevinksullivan, @bernhardoj eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[ntdiary]

This issue has not been updated in over 15 days. @yuwenmemon, @ntdiary, @kevinksullivan, @bernhardoj eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

Oops, missed this issue, it seemed a bit challenging at the time, and the solution requires further investigation on my part. Expect to provide an update by tomorrow.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[bernhardoj]

false alarm

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

• Fix blank screen is shown when opening a report screen after relaunching app #37596

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

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

• @ntdiary requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

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

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

[melvin-bot]

@yuwenmemon, @ntdiary, @kevinksullivan, @bernhardoj Huh... This is 4 days overdue. Who can take care of this?

[bernhardoj]

This is ready for payment @kevinksullivan

[ntdiary]

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:

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

When the initialValue is not set, our HOC component will display nothing before it finishes loading (i.e., a black screen), and I believe we will replace them with useOnyx hooks in the futureso I marked the PR as N/A.

Regression Test Proposal

1. Login with a High Traffic Account.
2. Kill the app and relaunch it.
3. Open a report with a lot of chat history.
4. Verify the chat screen/skeleton is shown instead of blank screen for a brief moment.

[kevinksullivan]

Summary:

• @ntdiary C+: $500 (newdot)
• @bernhardoj Contributor : $500

[kevinksullivan]

All set - regression test created and paid @bernhardoj

[JmillsExpensify]

$500 approved for @ntdiary