-
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
[$250] [New feature] Add loading indicator when ReconnectApp is running #46611
Comments
Triggered auto assignment to @jliexpensify ( |
|
Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify ( |
@slafortune could you assign me here too? thanks :) |
Assigned you @getusha |
Excited about this one. Let me know when there's something to look and test 😄 |
Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue. |
Current assignee @getusha is eligible for the External assigner, not assigning anyone new. |
Edited by proposal-police: This proposal was edited at 2024-08-06 15:13:28 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Add loading indicator when What is the root cause of that problem?This is a new feature What changes do you think we should make in order to solve the problem?The test branch is here: https://github.com/nkdengineer/App/tree/fix/46611 About the animation:
App/src/pages/home/ReportScreen.tsx Line 755 in 3666caa
App/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx Lines 56 to 59 in 3666caa
What alternative solutions did you explore? (Optional)NA ResultScreen.Recording.2024-09-05.at.20.16.57.movScreen.Recording.2024-09-05.at.20.17.37.mov |
ProposalPlease re-state the problem that we are trying to solve in this issue.Add loading indicator when ReconnectApp is running. What is the root cause of that problem?New feature. What changes do you think we should make in order to solve the problem?Add
Then, add the animation component at the bottom:
Example code for loading animation:
We can improve the animation. Screen.Recording.2024-08-01.at.2.19.53.PM.movScreen.Recording.2024-08-01.at.2.27.54.PM.movNote that the exact animation can be created during the PR time. |
ProposalUpdated to include videos. |
Not overdue |
The demos attached by both @ShridharGoel & @nkdengineer look way off to me... |
I modified my proposal with the expected. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.58-2 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-11-14. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
To leave a quick update, the initial PR caused multiple regressions so a new PR is being worked on. |
@nkdengineer do you have an ETA on when the new PR will be ready? |
I'm investigating to avoid the previous regression, will give an update today or tomorrow. |
@getusha @srikarparsi we have a open follow PR here |
This issue has not been updated in over 15 days. @jliexpensify, @srikarparsi, @getusha, @dubielzyk-expensify, @nkdengineer 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! |
How close are we with this one? Can we get it out before EoW? |
@srikarparsi @getusha @muttmuure - just a heads up that I am OOO from 19th - 29th December. If this needs payment, please feel free to reassign, otherwise I'll review and sort out when I'm back. |
Hey @muttmuure, we're waiting on deciding if we want the background of the loading indicator to be 1px or 2px tall and we'll make the necessary changes. I think towards the beginning of next week is probably a good estimate to get this merged. |
Hey everyone, I just stumbled upon this thread because I wanted to propose the ~same thing for a more native UX. One question, how does this loading indicator play together with the skeletons on LHN? Ideally, we could get rid of them with this feature introduced. LTM if this should be moved to Slack. |
Yeah, slack would be perfect. This was the thread this was originally discussed so that should be a good place :) |
Original post -
Sometimes, the app is loading something for a little while; as an end user, you have no clue what's happening. Then, when loading finishes, sometimes several seconds later, all of a sudden, new data pops up and takes you by surprise, resulting in a very unpleasant UX that almost feels broken.
Coming from this we've landed on showing the thin bar loader at the top of the screen when
ReconnectApp
is running.@dubielzyk-expensify created a CodePen and it's the animation called Rubber Band or #loading1 in code.
Much discussion took place here
Issue Owner
Current Issue Owner: @Issue Owner
Current Issue Owner: @Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: