-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2024-04-09] [$500] [MEDIUM] IOU - Money request page displays skeleton in online #26424
Comments
Triggered auto assignment to @trjExpensify ( |
Bug0 Triage Checklist (Main S/O)
|
When I come back online, they eventually load and the skeleton UI goes away, so that's contrary to the bug report. When we can't load the content, the skeleton UI is the correct pattern to use. RPReplay_Final1693568080.MP4I'm curious why the report total extended header in the iouReport isn't available to view offline in the first place though, and whether we can improve that in the spirit of offline-first? |
It seems like a regression. I agree they should be working offline. |
Yep I think in offline it can work like that, online we probably still need to show the skeleton |
Can you expand on that, @mountiny? Why do we need to show the skeleton for the report total when online? |
No we dont have to, sorry we were probably talking about different things |
Cool, so we agree with the below? Then next question, is this going to need backend changes? Expected results Actual Results |
What if you have exisitng IOU report and you sign out sign back in, you dont go to the IOU report so you dont have the report actions but you have the total. You are offline, You request more, the total is updated and you see that offline made request but you dont see the previosu messages and there is no indication the report has more report actions while total is more. Is that fine as a feedback to the user while offline? |
You might have to type that out again in steps for me to follow it completely sorry man. Let's park this and focus on the deploy for a sec. |
Okaaaay. @mountiny, circling back here. Mind rephrasing? 😄 |
If there is no skeleton, then it might be confusing to the user as there is only one report action and they cannot see the others |
Why wouldn't it just work like this as it does now? 😕 yfMvCuNr58.mp4Interestingly, the report total updates as well. Perhaps that should be at 50% opacity as well though, as it's pending the update? CC: @JmillsExpensify @shawnborton |
Yeah, I think that makes sense to me. |
@trjExpensify In this case you have visited the report before you went offline so you had the £50 report action. You could not have that if you would sig in and not open the report and went offline and then go to the report. Its edge case but thats what we trying to cover here |
I don't think I did visit the iouReport, no. Here's another one: ofdi8s8pOz.mp4 |
@trjExpensify yeah but you just created it locally so the request is there. You would have to create the request and report, then sign out and sign back in, request from global create from the same person, the historical report actions will be missing I believe because they do not load during openApp |
@trjExpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@trjExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Going to chat with Vit about this on Monday. |
@abdulrahuman5196 The PR is ready for review. |
Reassigning to @mountiny since he has a lot more context of this |
PR in active review. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 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-04-09. 🎊 For reference, here are some details about the assignees on 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:
|
$500 to @dukenv0307 and to @abdulrahuman5196 |
👋 checklist time please, @abdulrahuman5196! In the meantime, payment summary will be as follows:
|
Not a regression.
Yes.
@trjExpensify Added checklist. And I am still getting paid via upwork. Kindly process on the same. |
@trjExpensify, @mountiny, @abdulrahuman5196, @dukenv0307 Huh... This is 4 days overdue. Who can take care of this? |
Paid you both! Closing! |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Expected Result:
Output of @mountiny & I discussing this one here for reference.
-The iouActions we can't load would still show with a skeleton UI as we do today (we agreed just the one request preview component appearing as a skeleton is fine).
When a new request is added to the request when offline and appears at 50% opacity in the pending create state, so should the report total in the expanded header area.
Actual Result:
In online, in IOU report for request money, the page keeps on loading
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.60-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
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
Notes/Photos/Videos: Any additional supporting documentation
Bug6184136_loading_redmi_secondaey_receivrr.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @trjExpensifyThe text was updated successfully, but these errors were encountered: