-
Notifications
You must be signed in to change notification settings - Fork 188
feat: css animation polyfill #428
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
base: main
Are you sure you want to change the base?
Conversation
We might have some logic in the CSS transitions polyfill to prevent this happening for that API polyfill.
Sounds like this might be a problem with the implementation or tests
That's ok, we don't support the shorthand animation property
Yes, an array of keyframes
That would be cool, I had the same thought. However, we might be better off trying to accelerate this being added to React Native. Totally ok not to include it here |
|
@necolas thanks for taking a look I'll be away during holidays, so the PR may remain open for a while, but would like to summarize the next steps to move forward. Must have to merge
Out of scope or possible follow up PRs
Nice to have, but not a blockerThe API and the implementation is very close to the new CSS based API of reanimated (see: https://github.com/software-mansion/react-native-reanimated/tree/main/packages/react-native-reanimated/src/css ). While for RSD it makes more sense to use Animated and rely on it being optimized on RN core side, it would be nice to be able to share some of the logic and evaluate if at least the parsing logic could be extracted (or even more, but not sure to what extent Animated and Reanimated are exchangeable API wise) cc @MatiPl01 @tomekzaw It would also be great if while waiting for the C++ version of Animated to become available, consumers could swap Animated with Reanimated (for both css animations and transitions) by using custom node resolution (assuming the API is the same, but realistically may be hard) |
|
Sounds good! The C++ Animated will available in OSS RN early next year. We should then push for CSS transitions/animations implementation to be built into RN itself so we can delete these polyfills altogether. |
6eeb8d9 to
25994e5
Compare
25994e5 to
199a23f
Compare
|
cc @necolas I'm back. I did some cleanup and added the support for multiple animation (following stylex syntax with strings rather than array). Have a look when you have some time, should be good to go and seems to work fine on my device, but an extra pair of eyes are very much needed. AI helped a lot with the logic for animation-composition and fill-mode and it gets a bit complex. Also is a bit tricky to correctly create/dispose the animation controller (needs to be a fresh one when changing certain animation styles, but it needs to stay stable when changing animation state for correct play/pause). The CSS specs for animation-composition are also not too intuitive and had to compare what different browser were doing... I would aim to merge this one, then there can be a follow up to support the JS API (or at least a subset), that should not be overly complex as we can probably expose the controller methods to the ref, but would need to check the exact browser API to see how compatible it is beyond the simple play/pause |
Close #3
This is a POC to polyfill CSS animations using css.keyframes.
I used the implementation of transitions as a rough guideline, as well as trying to adhere to (part) of the CSS specs. LLMs were extensively used to fill the gaps and add extensive testing, I went trough many iterations until the code worked correctly and looked good to me in terms of structure and overall architecture, but it definitely needs some extra pairs of eyes.
In particular:
On a high level the polyfill works like this:
What is covered
I added some examples in the platform-tests. Currently I was only able to test on android.
What is missing
Sorry for the size and if I missed any LLM's slop while reviewing it and iterating on it. I mostly wanted to validate the approach and get an early feedback.