-
Notifications
You must be signed in to change notification settings - Fork 5k
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
feat: Add animation on the Smart Transaction status page #24389
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
background-position: 0 0; | ||
|
||
&--top { | ||
width: 1634px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a future PR:
Where do these constants come from? Should we use computed values or constants?
cc @georgewrmarshall from DS team.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tag @dbrans! I tend to think arbitrary values in styles are code smells, but sometimes they are necessary. It would be great to see a screencast of these changes in the PR description so I can review without having to checkout the branch and locate where these changes are implemented @dan437 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This width is twice the size of the transaction-background-top.svg image (817px) and I believe it was used in the past to make the animation work properly. We just reused the same code for animation from Smart Swaps as a short term solution. We will be replacing it with a long term solution once it's designed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #24389 +/- ##
===========================================
- Coverage 67.37% 67.34% -0.03%
===========================================
Files 1278 1282 +4
Lines 49881 50065 +184
Branches 12944 12986 +42
===========================================
+ Hits 33605 33716 +111
- Misses 16276 16349 +73 ☔ View full report in Codecov by Sentry. |
Builds ready [1e16638]
Page Load Metrics (185 ± 252 ms)
Bundle size diffs
|
Missing release label release-11.15.3 on PR. Adding release label release-11.15.3 on PR and removing other release labels(release-11.18.0), as PR was cherry-picked in branch 11.15.3. |
Description
Add animation on the Smart Transaction status page. We reused the animation from Smart Swaps.
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Pre-merge author checklist
Pre-merge reviewer checklist