Add styling to JSON animation #6
Conversation
|
@swapnil1104 @DevAhamed Can we merge this to master. |
|
@swapnil1104 Can we merge this? |
| protected renderAnimation(content: React.ReactNode): React.ReactNode { | ||
| const transformArray = this._getTransformArray(); | ||
| const animations: ViewStyle[] = this._getViewStyleAnimationArray(); | ||
| return ( | ||
| <Animated.View style={[{ transform: transformArray }, animations]}> | ||
| <Animated.View style={[{transform: transformArray}, animations, this.props?.animationWrapperStyles]}> |
There was a problem hiding this comment.
If we have already passed the styles to BaseAnimationWrapper, do we still need to explicitly pass it to this container as well?
There was a problem hiding this comment.
yeah, looks fishy, will have to test this. I will check this and then update.
There was a problem hiding this comment.
@swapnil1104 We are using RLV, which requires all its parent element to have fixed height or flex.
That's why the style prop is passed to container also.
There was a problem hiding this comment.
We are consuming StyleProp<ViewStyles> and not just a height value, that would mean any other additional style will also be applied to both BaseAnimationWrapper and JsonAnimationWrapper.
Have we verified that this won't cause any side-effects, by passing styles other than flex and fixed height to this prop?
There was a problem hiding this comment.
Then we can pass different styleProps to BaseAnimationWrapper and JsonAnimationWrapper.
There was a problem hiding this comment.
Can you verify the repercussions of sending the same style to the containers?
We can send different style props to the different containers, but that will pollute the public API of AnimationWrapperView, should carefully consider before adding additional props just to satisfy one condition.
We can explore extracting only height out of the style props and pass it to the JsonAnimationWrapper.
There was a problem hiding this comment.
@swapnil1104 created new PR for this: https://github.com/flipkart-incubator/animation-wrapper-view/pull/7/files
No description provided.