-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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] Skeleton of money request preview has no animation #40060
Comments
Triggered auto assignment to @sakluger ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.The IOU preview skeleton is displayed without animation What is the root cause of that problem?The problem is App/src/components/MoneyRequestSkeletonView.tsx Lines 16 to 17 in 8ae5491
App/src/styles/theme/themes/light.ts Lines 12 to 13 in 8ae5491
App/src/styles/theme/themes/dark.ts Lines 12 to 13 in 8ae5491
What changes do you think we should make in order to solve the problem?We should change App/src/components/MoneyRequestSkeletonView.tsx Lines 16 to 17 in 8ae5491
Or we can use App/src/components/ReportActionsSkeletonView/SkeletonViewLines.tsx Lines 21 to 22 in 8ae5491
What alternative solutions did you explore? (Optional)We can change the color of |
@sakluger I can take over this as C+ since I'm the reporter of this issue. |
Job added to Upwork: https://www.upwork.com/jobs/~01f1edc0cbcada32a1 |
Current assignee @dukenv0307 is eligible for the External assigner, not assigning anyone new. |
Thanks @dukenv0307! I assigned you as C+. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Animation doesn't show up for the money request skeleton preview. What is the root cause of that problem?We use the same color for the background and the foreground of the skeleton view as used here App/src/components/MoneyRequestSkeletonView.tsx Lines 16 to 17 in 2d19c95
and defined here App/src/styles/theme/themes/light.ts Lines 12 to 13 in 2d19c95
What changes do you think we should make in order to solve the problem?We should use the following prop values as used at other places such as here. We should also update the same for App/src/components/CurrentUserPersonalDetailsSkeletonView/index.tsx Lines 40 to 41 in 02059dd
What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
Let's go with @nkdengineer's solution. There's no problem with 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @flodnv, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
Sounds good, let's see what the design team says for which color to use. |
Triggered auto assignment to @dannymcclain ( |
I think we should do this (mentioned at the bottom of the proposal) to stay consistent with other skeleton loaders in the app. Is there any reason why we wouldn't? |
I agree with this. |
📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
This issue has not been updated in over 15 days. @flodnv, @sakluger, @dannymcclain, @dukenv0307, @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! |
@sakluger The PR was deployed last month. Can you please add |
Yes, sorry for the delay there. All paid out. |
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: 1.4.62-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1712745451877529
Action Performed:
Expected Result:
The IOU preview skeleton should have animation like other skeletons view in the App
Actual Result:
The IOU preview skeleton is displayed without animation
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Screen.Recording.2024-04-10.at.17.34.03.mov
Recording.2967.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: