Skip to content
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

Add styling to JSON animation #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions README.MD
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,12 @@ private _onPressToReset = (_: GestureResponderEvent) => {
```

### AnimationWrapperView props
| Prop name | Prop Type | Description |
| --- | --- | --- |
| Prop name | Prop Type | Description |
| --- | --- |--------------------------------------------------------------------------------------------------------------------------------------------------------|
| animationConfig | BaseAnimation | Object which will contain all optional and non-optional parameters needed to render the animation, including AnimationType, AnimationTriggerType, etc. |
| onAnimationFinish | () => void | (optional) Callback function, if provided will be invoked once animation is finished. |
| onAnimationStart | () => void | (optional) Callback function, if provided will be invoked when the animation is triggered. |
| onAnimationFinish | () => void | (optional) Callback function, if provided will be invoked once animation is finished. |
| onAnimationStart | () => void | (optional) Callback function, if provided will be invoked when the animation is triggered. |
| animationWrapperStyles | StyleProp<ViewStyle> | (optional) Styling, if provided will be applied to the root element. |


## Types of supported Animations
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "animation-wrapper-view",
"description": "Create declarative animations for React Native",
"version": "1.1.2",
"version": "1.1.3",
"repository": {
"type": "git",
"url": "git+https://github.com/flipkart-incubator/animation-wrapper-view.git"
Expand Down
2 changes: 2 additions & 0 deletions src/core/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { RippleAnimationProps } from "./components/wrapper/RippleAnimationWrappe
import { ScaleAnimationProps } from "./components/wrapper/ScaleAnimationWrapper";
import { WiggleAnimationProps } from "./components/wrapper/WiggleAnimationWrapper";
import BaseAnimationConfig from "./data/BaseAnimationConfig";
import { StyleProp, ViewStyle } from "react-native";

/**
* Append the React.ComponentClass<T extends AnimationProps> for each class extending from BaseAnimationWrapper
Expand All @@ -23,6 +24,7 @@ type AnimationWrapperProps = {
animationConfig: BaseAnimationConfig;
onAnimationFinish?: (animationConfig?: BaseAnimationConfig) => void;
onAnimationStart?: (animationConfig?: BaseAnimationConfig) => void;
animationWrapperStyles?: StyleProp<ViewStyle>;
}

type SlideAnimationProps = AnimationWrapperProps;
Expand Down
5 changes: 3 additions & 2 deletions src/core/components/AnimationWrapperView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ export default class AnimationWrapperView extends React.Component<AnimationWrapp

public render(): React.ReactNode | undefined {
this._assertChildType();
const { animationConfig, children, onAnimationFinish, onAnimationStart } = this.props;
const { animationConfig, children, onAnimationFinish, onAnimationStart, animationWrapperStyles } = this.props;
if (this._animationComponentClass && children) {
return (
<this._animationComponentClass
ref={this._setRef}
animationConfig={animationConfig as any}
onAnimationFinish={onAnimationFinish}
onAnimationStart={onAnimationStart}>
onAnimationStart={onAnimationStart}
animationWrapperStyles={animationWrapperStyles}>
doomtrooper marked this conversation as resolved.
Show resolved Hide resolved
{children}
</this._animationComponentClass>
);
Expand Down
4 changes: 2 additions & 2 deletions src/core/components/wrapper/BaseAnimationWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ export abstract class BaseAnimationWrapper<P extends AnimationWrapperProps> exte
const content = this.props.children;
if (this.props.animationConfig?.triggerType === AnimationTriggerType.ON_CLICK) {
return (
<TouchableWithoutFeedback onPress={this._onPress}>
<TouchableWithoutFeedback onPress={this._onPress} style={this.props?.animationWrapperStyles}>
{this.renderAnimation(content)}
</TouchableWithoutFeedback>
);
} else {
return (
<View>
<View style={this.props?.animationWrapperStyles}>
{this.renderAnimation(content)}
</View>
)
Expand Down
3 changes: 2 additions & 1 deletion src/core/components/wrapper/JsonAnimationWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ export class JsonAnimationWrapper extends BaseAnimationWrapper<JsonAnimationProp
}
}

//For now, we are only adding the stying passed as a prop for JsonAnimationWrapper.
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]}>
doomtrooper marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have already passed the styles to BaseAnimationWrapper, do we still need to explicitly pass it to this container as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, looks fishy, will have to test this. I will check this and then update.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we can pass different styleProps to BaseAnimationWrapper and JsonAnimationWrapper.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{content}
</Animated.View>
);
Expand Down