You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2024-10-17] [$250] Book travel - Book travel animation becomes blank while RHP is dismissed #49744
Version Number: 9.0.40-0 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers):applausetester+kh230901@applause.expensifail.com Issue reported by: Applause - Internal Team
Action Performed:
Go to staging.new.expensify.com
Open FAB
Click Book travel
Dismiss the RHP
Expected Result:
Book travel animation will not become blank while RHP is dismissed
Actual Result:
Book travel animation becomes blank while RHP is dismissed
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
👋 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:
Identify the pull request that introduced this issue and revert it.
Please re-state the problem that we are trying to solve in this issue.
Default name in name field displays user/report ID instead of user/report name
What is the root cause of that problem?
This happens as we render an empty view here if the user has navigated away. The issue we set hasNavigatedAway quickly before the RHP animation has been completed. This then shows the user an empty view while RHP is dismissing.
What changes do you think we should make in order to solve the problem?
We will change the state change here: to run after interactions, which will make it wait for the animation to be completed:
// If the page navigates to another screen, the image fails to load, app is in background state, animation file isn't ready, or the splash screen isn't hidden yet,
// we'll just render an empty view as the fallback to prevent
// 1. memory leak, see issue: https://github.com/Expensify/App/issues/36645
// 2. heavy rendering, see issues: https://github.com/Expensify/App/issues/34696 and https://github.com/Expensify/App/issues/47273
// 3. lag on react navigation transitions, see issue: https://github.com/Expensify/App/issues/44812
// If the page navigates to another screen, the image fails to load, app is in background state, animation file isn't ready, or the splash screen isn't hidden yet,
// we'll just render an empty view as the fallback to prevent
// 1. memory leak, see issue: https://github.com/Expensify/App/issues/36645
// 2. heavy rendering, see issues: https://github.com/Expensify/App/issues/34696 and https://github.com/Expensify/App/issues/47273
// 3. lag on react navigation transitions, see issue: https://github.com/Expensify/App/issues/44812
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.47-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
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:
[@suneox] The PR that introduced the bug has been identified. Link to the PR:
[@suneox] 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:
[@suneox] 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:
[@suneox] Determine if we should create a regression test for this bug.
[@suneox] 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.
[@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
[@suneox] 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
[@suneox] 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
[@suneox] Determine if we should create a regression test for this bug: N/A
[@suneox] 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. N/A
Activity
melvin-bot commentedon Sep 25, 2024
Triggered auto assignment to @MonilBhavsar (
DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.github-actions commentedon Sep 25, 2024
👋 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:
rushatgabhane commentedon Sep 25, 2024
don't know what caused it, but def NOT A BLOCKER
Nodebrute commentedon Sep 25, 2024
This happens with all the animations, like on the wallet, security, and troubleshoot pages.
rushatgabhane commentedon Sep 25, 2024
let's close 😄
MonilBhavsar commentedon Sep 25, 2024
Thanks for looking! Demoting from blocker, but let's try if we can fix it
neonbhai commentedon Sep 25, 2024
Proposal
Please re-state the problem that we are trying to solve in this issue.
Default name in name field displays user/report ID instead of user/report name
What is the root cause of that problem?
This happens as we render an empty view here if the user has navigated away. The issue we set hasNavigatedAway quickly before the RHP animation has been completed. This then shows the user an empty view while RHP is dismissing.
What changes do you think we should make in order to solve the problem?
We will change the state change here: to run after interactions, which will make it wait for the animation to be completed:
App/src/components/Lottie/index.tsx
Lines 82 to 88 in a4a344b
Result
Screen.Recording.2024-09-26.at.2.36.22.AM.mov
rushatgabhane commentedon Sep 25, 2024
@MonilBhavsar @neonbhai the reasons mentioned here suggest that we should do nothing. Or it may cause issues 1, 2, 3
But I guess that is only the case when the image hasn't loaded
App/src/components/Lottie/index.tsx
Lines 82 to 88 in a4a344b
47 remaining items
melvin-bot commentedon Oct 10, 2024
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon Oct 10, 2024
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.47-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2024-10-17. 🎊
For reference, here are some details about the assignees on this issue:
melvin-bot commentedon Oct 10, 2024
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:
sakluger commentedon Oct 17, 2024
Summarizing payment on this issue:
Contributor: @layacat $250, please apply to the Upwork job or comment with your Upwork profile
Contributor+: @suneox $250, paid via Upwork
@suneox please complete the BZ checklist.
layacat commentedon Oct 18, 2024
Hi @sakluger I submitted work on Upwork, please check
suneox commentedon Oct 18, 2024
Checklist
sakluger commentedon Oct 18, 2024
@layacat thanks! I didn't realize you had already applied. I sent an offer via Upwork.
layacat commentedon Oct 21, 2024
I just submitted work for payment on Upwork. Please check
sakluger commentedon Oct 21, 2024
Perfect, thanks! All contracts have been paid out now.