Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report view header is missing skeleton while app is loading #49520

Open
2 of 6 tasks
IuliiaHerets opened this issue Sep 20, 2024 · 11 comments
Open
2 of 6 tasks

Report view header is missing skeleton while app is loading #49520

IuliiaHerets opened this issue Sep 20, 2024 · 11 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

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: 9.0.39-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go offline.
  3. Go to Account settings > Troubleshoot > Clear reset and restart.
  4. Click Reset and refresh.
  5. Go back to Inbox.
  6. Compare the header on the report view on staging and production while app is loading.

Expected Result:

The report view header will have skeleton while app is loading (production behavior).

Actual Result:

The report view header is missing skeleton while app is loading.
The issue is more obvious when the window is not in full screen mode (Mac).

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6609800_1726826572137!desktop_full_screen Bug6609800_1726826341561!desktop Bug6609800_1726826154969!Screenshot_2024-09-20_at_17 53 30
Bug6609800_1726826154949.20240920_175112.mp4

View all open jobs on GitHub

@IuliiaHerets IuliiaHerets added the Bug Something is broken. Auto assigns a BugZero manager. label Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Sep 20, 2024
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment and removed Daily KSv2 labels Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

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

Copy link
Contributor

👋 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.

@grgia
Copy link
Contributor

grgia commented Sep 20, 2024

Environment Screenshot
Staging Screenshot 2024-09-20 at 12 46 20 PM
Production Screenshot 2024-09-20 at 12 46 18 PM

@grgia
Copy link
Contributor

grgia commented Sep 20, 2024

Confirmed with test revert it was caused by #49056

@melvin-bot melvin-bot bot mentioned this issue Sep 20, 2024
50 tasks
@BhuvaneshPatil
Copy link
Contributor

Proposal

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

Report view header is missing skeleton while app is loading

What is the root cause of that problem?

In the mentioned PR #49056, we modified this lines

let headerView = report && (
<HeaderView
reportID={reportIDFromRoute}
onNavigationMenuButtonClicked={onBackButtonPress}
report={report}
parentReportAction={parentReportAction}
shouldUseNarrowLayout={shouldUseNarrowLayout}
/>
);
if (isSingleTransactionView && report) {
headerView = (
<MoneyRequestHeader
report={report}
policy={policy}
parentReportAction={parentReportAction}
shouldUseNarrowLayout={shouldUseNarrowLayout}
onBackButtonPress={onBackButtonPress}
/>
);
}

It makes header view null and it doesn't appear. Because report is null when app is loading.

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

{headerView}

We can modify this line as

{headerView??<ReportHeaderSkeletonView/>}

So in case we don't have headerView it will show skeleton

Result -

Screen.Recording.2024-09-20.at.6.28.03.PM.mov

What alternative solutions did you explore? (Optional)

@BhuvaneshPatil
Copy link
Contributor

@grgia I can raise a quick PR if you want

@daledah
Copy link
Contributor

daledah commented Sep 20, 2024

@grgia I'll raise a follow-up ASAP.

@grgia
Copy link
Contributor

grgia commented Sep 20, 2024

thank you @BhuvaneshPatil, I'm going to ask @daledah raise the PR as this bug is a regression from #49056

@BhuvaneshPatil
Copy link
Contributor

No worries. I hope the changes suggested maybe of use for resolving the bug. Thanks.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Sep 20, 2024
@daledah
Copy link
Contributor

daledah commented Sep 20, 2024

@grgia PR is up.

Additionally, there might be some other components can be rendered without report besides HeaderView that I might've missed. Please let me know if there are any so I can include the changes in this PR as well. Thanks!

@madmax330 madmax330 removed the DeployBlockerCash This issue or pull request should block deployment label Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants