Migrate animations to app assets and .lottie files#29975
Conversation
updating the expensify card admin settings and features help doc per this issue here https://github.com/Expensify/Expensify/issues/311376
updating the formatting to include numbered steps for certain sections
updating the formatting
|
How did you convert these to |
roryabraham
left a comment
There was a problem hiding this comment.
As suggested in slack, let's split this up into two:
- One PR to upgrade lottie to 6.4.0, stop using react-native-web-lottie, and delete our patch in react-native-web-lottie
- Another to switch from .json to .lottie
Hey,
Sure, I will prepare two separate PRs. However, if there are any problems with the JSON animations library (as the |
|
Sorry for the delay, I can't get the optimized Lottie assets to you just now as LottieFiles has been down for a few days, presumably due to the ongoing Cloudflare outage |
|
@kosmydel, it turns out that some of the "optimized" DotLottie file formats are bigger than the "un-optimized" DotLottie. Also seems like LottieFiles isn't really working as expected right now, so let's not block on that. |
|
Sure! I've pulled the main, quickly tested it, and it is ready for review. Keep in mind, that since we are moving the animations to the assets, animations are |
|
Awaiting C+ review from @mananjadhav |
|
Half way through the code, will finish this today. |
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid-ios-animations.movAndroid: mWeb Chromemweb-chrome-animations.moviOS: Nativeios-animations.moviOS: mWeb Safarimweb-safari-animations.movMacOS: Chrome / Safariweb-lottie-animations-5.movweb-lottie-animations-4.movweb-lottie-animations-3.movweb-lottie-animations-2.movweb-lottie-animations-1.movMacOS: Desktopdesktop-lottie-animations.mov |
|
I am testing each of the lottie animations. I could test all but one |
|
@roryabraham Except for one animation, I was able to test all. |
|
@kosmydel I think you need to merge main here. I think the conflicts will merge cleanly because the changes in |
@roryabraham I've resolved conflicts and quickly tested a few animations. The conflicts were with this PR fixing a bug in the |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 1.3.98-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.98-5 🚀
|
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 1.3.99-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.99-0 🚀
|
Details
Fixed Issues
$ #28160
$ #26857
PROPOSAL: N/A
Tests
Offline tests
Offline test
Screen.Recording.2023-11-06.at.10.43.46.mov
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native / iOS: Native
ios_android.mov
Android: mWeb Chrome / iOS: mWeb Safari
mweb.mov
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov