-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 C+ Payment] [$1000] Slow animation on sign in input #23137
Comments
Triggered auto assignment to @CortneyOfstad ( |
Bug0 Triage Checklist (Main S/O)
|
Was able to recreate this, so getting eyes on it |
Job added to Upwork: https://www.upwork.com/jobs/~0177b62cd207274e58 |
Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Slow animation on sign in input What is the root cause of that problem?RCA for this issue is with the background SVG we're rendering and the position of it. If we observe the svg that is on background it is covered into whole screen whereas it should be covered fully into the scrollview only. Not sure why having an SVG into background would interrupt animations only in safari :) What changes do you think we should make in order to solve the problem?We would need to add
and pass styles.signInBackgroundDesktop to style prop here
would solve the issue. What alternative solutions did you explore? (Optional)NA, I feel this is the only solution to this issue. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Slow animation on the input labels of the login page in Safari. There is also laggy in the Lottie animation on the right side mentioned at #22994 What is the root cause of that problem?The issue is caused by the home-background--desktop.svg. It is not well-optimized, which is causing a widespread performance problem on the page. By fixing it, both issues are resolved: the input label animation and the animation on the right side of the screen, as can be seen in the video below: Screen.Recording.2023-07-20.at.14.59.18.movWhat changes do you think we should make in order to solve the problem?Optimize SVG file. What alternative solutions did you explore? (Optional)Not applicable. |
📣 @andreyazevedo! 📣
|
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@rushatgabhane any thoughts on the proposals above? |
ProposalPlease re-state the problem that we are trying to solve in this issue.Slow/Laggy animations on sign in page in Safari. What is the root cause of that problem?home-background--desktop.svg does not have position: absolute causing a ton of unnecessary recalculations. What changes do you think we should make in order to solve the problem?Styles exist for both mobile and desktop in styles.js: Lines 1136 to 1148 in b7d17e0
However they are only passed to the mobile version: src/pages/signin/SignInPageLayout/index.js: App/src/pages/signin/SignInPageLayout/index.js Lines 90 to 94 in b7d17e0
App/src/pages/signin/SignInPageLayout/index.js Lines 125 to 130 in b7d17e0
src/pages/signin/SignInPageLayout/BackgroundImage/index.js: Since the styles are the same for both platforms, I suggest merging them into one "signInBackground" in styles.js, getting rid of the style prop passed from SignInPageLayout to BackgroundImage and importing it directly from styles here. expensify_laggy_animations_fix.mov |
📣 @dantastisk! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@rushatgabhane Any thoughts on the proposals? |
ProposalPlease re-state the problem that we are trying to solve in this issue.The problem we are trying to solve is that there are slow and laggy animations on the sign-in page in Safari. What is the root cause of that problem?The root cause of this problem is that the "home-background--desktop.svg" does not have a "position: absolute" property, which leads to a large number of unnecessary recalculations during animations. What changes do you think we should make in order to solve the problem?To solve the problem, we should make the following changes: In the "styles.js" file, merge the styles for both mobile and desktop versions of the "signInBackground" into one style with the "position: absolute" property.
In the "SignInPageLayout/index.js" file, remove the "style" prop passed to the "BackgroundImage" component and import the "signInBackground" style directly from the "styles.js" file.
By making these changes, we ensure that the "home-background--desktop.svg" is positioned absolutely, eliminating unnecessary recalculations and improving the performance of animations on the sign-in page in Safari. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.54-13 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 2023-08-24. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External 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:
|
@rushatgabhane Any update on the checklist above? TIA! |
I'm waiting for the existing manual payment requests to be approved. So maybe 2 days?? I'll then complete all pending checklists and request payment on new dot. |
Sounds good @rushatgabhane — payments have been confirmed and contracts closed for @spacexdragonpie and @dantastisk so they are all set! |
@CortneyOfstad, @joelbettner, @rushatgabhane, @sophiepintoraetz, @dantastisk Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@rushatgabhane Just checking to see where we're at — thanks! |
Bump @rushatgabhane ^^^ |
still waiting for payment to be made |
@rushatgabhane Okay, if you need me to make a bump internally, just let me know 👍 |
@CortneyOfstad - make sure you're leaving a summary to this effect and assigning @JmillsExpensify (there's a TE snippet Payouts due: Contributor+: $1500 @rushatgabhane Eligible for 50% #urgency bonus? Y |
|
Created a manual request here - https://staging.new.expensify.com/r/7881967223525419 this issue can be closed |
$1,500 payment approved for @rushatgabhane based on BZ summary. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Expected that on Safari, the animation of labels is similar to speed as on Chrome.
Actual Result:
Slow animation on Safari
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number:
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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
Screen.Recording.2023-07-17.at.6.51.40.PM.mp4
Expensify/Expensify Issue URL:
Issue reported by: @spacexdragonpie
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689595055183879
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: