[$250] Chat - Skeleton appears behind expensify pattern on chat page

[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.23-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: https://expensify.testrail.io/index.php?/tests/view/4878182&group_by=cases:section_id&group_order=asc&group_id=229066
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. As anonymous user navigate to public room link from android/web
2. Verify the public room page opens
3. Check the skeleton position

Expected Result:

Loading skeleton should not be positioned behind expensify chat

Actual Result:

Skeleton appears behind expensify pattern on chat page

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6577958_1724232656722.Recording__3794.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d6a2ce5738f29be5
• Upwork Job ID: 1828326025582972875
• Last Price Increase: 2024-09-10
• Automatic offers:
• wildan-m | Contributor | 103914018

[melvin-bot]

Triggered auto assignment to @alexpensify (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 #vip-vsb

[IuliiaHerets]

@alexpensify 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

[daledah]

Edited by proposal-police: This proposal was edited at 2024-08-21 20:37:29 UTC.

Proposal

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

Skeleton appears behind expensify pattern on chat page

What is the root cause of that problem?

We use shouldShowSkeleton here to determine if skeleton show on the screen.

shouldShowSkeleton uses several boolean values, one of them is isLoadingApp here. When isLoadingApp is true, shouldShowSkeleton will be true.

When we open an Expensify link for the first time, the app calls OpenApp, which will set isLoadingApp data using getOnyxDataForOpenOrReconnect here, with isOpenApp set to true.

After that, every time we reload or open a link, the app calls ReconnectApp, which also calls getOnyxDataForOpenOrReconnect here, but with default openApp value, which is false.

In getOnyxDataForOpenOrReconnect, with openApp set to true, isLoadingApp will be set to false after the API call, which make the screen work as expected: skeleton doesn't show up. But when we call ReconnectApp, isLoadingApp is not updated, its value is set to true without being set back to false, hence the bug.

Another interesting part that I found in AuthScreens:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 166 to 173 in 2fd5bbe

	function handleNetworkReconnect() {
	if (isLoadingApp) {
	App.openApp();
	} else {
	Log.info('[handleNetworkReconnect] Sending ReconnectApp');
	App.reconnectApp(lastUpdateIDAppliedToClient);
	}
	}

This function is run every time we reload the app, and the above bug leads to an interesting behavior: we cycle between expected behavior and bug each time we reload the app.

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

We should call getOnyxDataForOpenOrReconnect in ReconnectApp with param openApp set to true as well.

What alternative solutions did you explore? (Optional)

[isogit123]

Edited by proposal-police: This proposal was edited at 2024-08-22 00:51:37 UTC.

Proposal

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

The skeleton appears behind the Expensify pattern on the chat page.

What is the root cause of that problem?

We show the skeleton when the app is loading by checking that isLoadingApp is true here:

App/src/pages/home/ReportScreen.tsx

Line 128 in 2fd5bbe

const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);

And using it here:

App/src/pages/home/ReportScreen.tsx

Lines 391 to 394 in 2fd5bbe

	const isLoading = isLoadingApp || !reportIDFromRoute || (!isSidebarLoaded && !isInNarrowPaneModal) || PersonalDetailsUtils.isPersonalDetailsEmpty();
	const shouldShowSkeleton =
	(isLinkingToMessage && !isLinkedMessagePageReady) || (!isLinkingToMessage && !isInitialPageReady) || isLoadingReportOnyx || !isCurrentReportLoadedFromOnyx || isLoading;

So the skeleton and the Expensify pattern appear at the same time.

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

We should avoid showing the Expensify pattern when the app is loading by showing it only when isLoadingApp becomes false.
To do this, we can edit the AnimatedEmptyStateBackground component to read the isLoadingApp by adding the following code line to the component:
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
And introduce a prop let's call it shouldHideOnLoading so we hide the pattern when the app is loading only when this prop is true. This will help avoid design regressions.
Lastly, change the return of the component to be:

    return (
        !isLoadingApp && shouldHideOnLoading && (
        <View style={StyleUtils.getReportWelcomeBackgroundContainerStyle()}>
            <Animated.Image
                source={illustrations.EmptyStateBackgroundImage}
                style={[StyleUtils.getReportWelcomeBackgroundImageStyle(shouldUseNarrowLayout), animatedStyles]}
                resizeMode={windowWidth > maxBackgroundWidth ? 'repeat' : 'cover'}
            />
        </View>
        )
    );

Below is a POC after applying the solution:

Screen_Recording_20240822_025351_Chrome.mp4

What alternative solutions did you explore? (Optional)

[isogit123]

Proposal updated
Cleaned up the proposal.

[isogit123]

Proposal updated
Update the solution to be applied by a prop.

[melvin-bot]

@alexpensify Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[melvin-bot]

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

[alexpensify]

@Ollyws - when you get a chance, please review the proposals and determine which one will fix this issue. Thanks!

---

Heads up, I will be offline until Tuesday, September 3, 2024, and will not actively watch over this GitHub during that period.

If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks!

[Ollyws]

@daledah it seems like the getOnyxDataForOpenOrReconnect function hasn't been modified for a long time, do you have any idea what caused this issue? Something on the backend possibly?

[melvin-bot]

@alexpensify, @Ollyws Whoops! This issue is 2 days overdue. Let's get this updated quick!

[daledah]

@Ollyws I tested for some times now, and it looks like the bug is not reproducible anymore on the latest main.

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

@alexpensify, @Ollyws Still overdue 6 days?! Let's take care of this!

[alexpensify]

Catching up from being OOO, @Ollyws keep us posted if you still have questions here or if we need to start a discussion.

[melvin-bot]

@alexpensify @Ollyws this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[Ollyws]

@daledah Still occuring for me:

Screen.Recording.2024-09-05.at.13.58.29.mov

[Ollyws]

Not overdue...awaiting proposals.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[wildan-m]

Proposal

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

There are two issues:

1. Skeleton behind Expensify logo on chat page
2. Infinite loading on initial public load in fresh browser session

What is the root cause of that problem?

1. The issue occurs when the last report action type is CREATED and shouldShowSkeleton in ReportScreen is still true, causing these components to be rendered:

App/src/pages/home/ReportScreen.tsx

Lines 803 to 820 in 1e8d80e

	<ReportActionsSkeletonView />
	{!!firstReportAction && (
	<ReportActionsListItemRenderer
	reportAction={firstReportAction}
	reportActions={reportActions}
	parentReportAction={parentReportAction}
	parentReportActionForTransactionThread={undefined}
	transactionThreadReport={undefined}
	index={0}
	report={report}
	displayAsGroup={false}
	shouldHideThreadDividerLine
	shouldDisplayNewMarker={false}
	shouldDisplayReplyDivider={false}
	isFirstVisibleReportAction
	shouldUseThreadDividerLine={false}
	/>
	)}

2. The second issue is the backend problem where the WRITE_COMMANDS.OPEN_APP API endpoint does not consistently provide a response.

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

The second issue requires back-end investigation, while the first can be resolved on the front-end.

The firstReportAction displays the last chat when the skeleton is loaded. https://github.com/Expensify/App/pull/47243/files

To address the first problem, we can create a new variable called shouldRenderFirstReportAction with the following conditions:

• Display only the latest report action if it's not of type CREATED
• Ensure it's not deleted to prevent the chat from showing as (edited) during loading. (new found bug)

Add this code in ReportScreen

    const firstReportAction = reportActions[0];
    const shouldRenderFirstReportAction = !!firstReportAction &&
        !ReportActionsUtils.isActionOfType(firstReportAction, CONST.REPORT.ACTIONS.TYPE.CREATED) &&
        !ReportActionsUtils.isDeletedAction(firstReportAction);

Consider renaming firstReportAction to lastReportAction for a more accurate name.

Change this code:

App/src/pages/home/ReportScreen.tsx

Lines 803 to 820 in 1e8d80e

	<ReportActionsSkeletonView />
	{!!firstReportAction && (
	<ReportActionsListItemRenderer
	reportAction={firstReportAction}
	reportActions={reportActions}
	parentReportAction={parentReportAction}
	parentReportActionForTransactionThread={undefined}
	transactionThreadReport={undefined}
	index={0}
	report={report}
	displayAsGroup={false}
	shouldHideThreadDividerLine
	shouldDisplayNewMarker={false}
	shouldDisplayReplyDivider={false}
	isFirstVisibleReportAction
	shouldUseThreadDividerLine={false}
	/>
	)}

To:

                                        {shouldRenderFirstReportAction && (
                                            <ReportActionsListItemRenderer
                                                reportAction={firstReportAction}
                                                reportActions={reportActions}
                                                parentReportAction={parentReportAction}

Branch for this solution

What alternative solutions did you explore? (Optional)

N/A

[Ollyws]

@wildan-m has the correct RCA and their solution looks good so let's go with their proposal.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[wildan-m]

@Ollyws The PR is ready. #49039

[alexpensify]

Awesome, the PR is moving along

[alexpensify]

Now we wait for this one to go through automation. It went into production three days ago.

[Ollyws]

@alexpensify Automation didn't seem to work, this one is due for payment today. Thanks!

[alexpensify]

Thanks for flagging @Ollyws. I had a note here too to prepare if there was automation failure: #47799 (comment)

Payouts due: 2024-09-24

• Contributor: $250 @wildan-m - paid via Upwork
• Contributor+: $250 @Ollyws - please submit a request via Chat

Upwork job is here.

[Ollyws]

Requested in ND.

[garrettmknight]

$250 approved for @Ollyws