-
Notifications
You must be signed in to change notification settings - Fork 54
feat(theme): support keyframes through animation property #414
Conversation
Codecov Report
@@ Coverage Diff @@
## master #414 +/- ##
==========================================
+ Coverage 88.38% 88.42% +0.03%
==========================================
Files 41 42 +1
Lines 1429 1451 +22
Branches 182 186 +4
==========================================
+ Hits 1263 1283 +20
- Misses 162 163 +1
- Partials 4 5 +1
Continue to review full report at Codecov.
|
let's keep: using keyframe already defined in the theme remove: using custom keyframe Only the Provider will allow defining keyframes, for now. We can iterate from there 👍 |
…g the keyframes -added option for overriding the animation props
UpdateThe theme can define various animations through the animations property in the following manner:
The usage of this animation then can be define in two manners: by specifying the name of the animation
by specifying an animation object
With the second approach the user can if necessary change some properties of the animation, without having to redefine it in the Provider's theme. |
Second approachUsing the new
Pros
Cons
|
-extracted logic for generating the animationStyle so that it can be reused in renderComponent, as well as the Transition component
docs/src/examples/components/Icon/Variations/IconAnimated.shorthand.tsx
Outdated
Show resolved
Hide resolved
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.
Merge after changes are addressed 👍
# Conflicts: # src/components/Button/Button.tsx # src/components/Icon/Icon.tsx # src/lib/mergeThemes.ts
-removed keyframeParams -added transition examples -added animation prop to all UI components
Keyframes
This PR aims to add possibility for defining and using keyframes in the theme.
API Proposal
We should add keyframes prop in the theme, that will define different keyframes in the following manner:
Then, inside the components the users can access this keyframes inside the styles, using the theme property. Something like:
Another approach that I have tried is to evaluate the keyframes inside the Provider component once and then use them like this:
Altough this API is cleaner and easier for using, we are not able to provide any props to the keyframe, which is something that is not acceptable.
Please share your thoughts on this, so that we can conclude the final API.
Update
After discussion, we decided to add animation property to the components, that would encapsulate all things regarding the keyframes and will provide easier usage. The definition of the keyframes in the theme will remain the same, but the usage will be provided in the following manner:
using keyframe already defined in the theme
using custom keyframe
The animation property is actually defining all animation-related css properties. The interface looks like this: