-
Notifications
You must be signed in to change notification settings - Fork 11
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(BrandLoadingScreen): improve lottie animations #1235
Conversation
Size stats
|
Accessibility report ℹ️ You can run this locally by executing |
Deploy preview for mistica-web ready! ✅ Preview Built with commit 6dadf8f. |
src/lottie/index.tsx
Outdated
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.
Copied from https://github.com/Gamote/lottie-react/blob/main/src/components/Lottie.ts and modified in order to fix some errors and adapt to our use cases
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.
Used https://www.lottiemizer.com/ to compress the JSON files
src/lottie/hooks/use-lottie.tsx
Outdated
const deregisterList = listeners.map((listener: Listener) => { | ||
// In Mistica we only use these two events. If we try to add the event listener for all the other | ||
// events from the listeners array, some runtime errors appear (and TS complains about wrong types). | ||
const misticaAnimationEvents: Array<AnimationEventName> = ['complete', 'loopComplete']; |
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.
I had to add the listeners conditionally only for these two events because I was getting runtime (and TS) errors otherwise.
I've analyzed this and we should be fine by adding only these two events, because we never use the other ones in Mistica
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.
I've tested rendering the BrandLoadingScreen in the Mistica storybook, and I can verify the improvements I've measured locally with bundle analyzer. As you can see, the total size of mistica bundles appears to have been reduced by around 170kb when using the BrandLoadingScreen (over a 10% improvement in the total bundle size for this page).
Also, notice how the bundle that contains the main.
prefix is faster now (567ms vs 186ms).
Before:
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.
Using yarn patch
to import the light version of lottie-web
instead of the default one that the library uses. This is the main improvement
# [16.1.0](v16.0.0...v16.1.0) (2024-09-27) ### Features * **BrandLoadingScreen:** improve lottie animations ([#1235](#1235)) ([e7dc87f](e7dc87f)), closes [/github.com/airbnb/lottie-web/issues/1184#issuecomment-411586909](https://github.com//github.com/airbnb/lottie-web/issues/1184/issues/issuecomment-411586909) * **Form fields:** allow blocking copy/cut ([#1251](#1251)) ([8fd2838](8fd2838)) * **Form fields:** improve accesibility of errors ([#1246](#1246)) ([e35a99e](e35a99e)) * **PhoneNumberField:** Custom formatter support + lazy load libphonenumber on demand ([#1244](#1244)) ([2ee88e9](2ee88e9)) * **PosterCard, DisplayMediaCard:** allow using srcSet for backgroundImage ([#1253](#1253)) ([3b3d85f](3b3d85f)) * **Rating, InfoRating:** new components ([#1196](#1196)) ([02c91f6](02c91f6)) * **SearchField, TextField:** support inputMode prop ([#1249](#1249)) ([fe31eca](fe31eca)) * **Sheet:** lazy load sheet implementations ([#1250](#1250)) ([40fecdd](40fecdd))
🎉 This PR is included in version 16.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
In order to verify the improvements, I've modified locally the size-stats example and used bundle analizer to compare the results.
Used an online tool to compress the lottie JSON files for Vivo-new logo. This improved in 20% the total size of the lotties:
Before:
After:
I noticed that the main bottleneck was using
lottie-react
to render the lotties. This lib importslottie-web
, and that library is quiet heavy. Some people recommended importinglottie-light
fromlottie-web
and using it as replacement for the default one.What I've done is copying a minimized version of (lottie-react into our repository and adapted it to use
lottie-light
instead oflottie-web
. This caused a pretty big improvement (over 50% of the total size of the lib).Before:
After:
In total, by using the exact same example in size-stats, we can see that the bundles size has been reduced by over 160kb:
Before:
After:
🗒️ Note
The size stats comment in this PR only shows the improvements caused by compressing the vivo lotties. The ones obtained by replacing the lottie library are not visible because the example page in the size-stats doesn't use a BrandLoadingScreen component (and the lotties are imported lazily). Refer to the images I've attached in this description to see the impact.