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

Feature: added iterationPause property #169

Merged
merged 5 commits into from
May 31, 2018
Merged

Conversation

almostintuitive
Copy link
Contributor

@almostintuitive almostintuitive commented Dec 21, 2017

A new, optional property to insert a pause between two iterations.

We're happy to update the README to include this, if everything is fine with this PR.

Closes #168

@@ -291,11 +291,12 @@ export default function createAnimatableComponent(WrappedComponent) {
delay,
onAnimationBegin,
onAnimationEnd,
iterationPause,
Copy link
Owner

Choose a reason for hiding this comment

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

Would prefer to name this iterationDelay instead to keep it consistent.

@@ -349,10 +350,10 @@ export default function createAnimatableComponent(WrappedComponent) {
this.setState({ animationStyle, compiledAnimation }, callback);
}

animate(animation, duration) {
animate(animation, duration, iterationCount) {
Copy link
Owner

Choose a reason for hiding this comment

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

This should be iterationDelay no?

@@ -389,21 +390,24 @@ export default function createAnimatableComponent(WrappedComponent) {
if (reversed) {
easing = Easing.out(easing);
}

Animated.timing(animationValue, {
const delay = iterationPause ? { delay: iterationPause } : {}
Copy link
Owner

Choose a reason for hiding this comment

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

Not a fan of this style, can't you just do it inline with a default to 0 like the others?

Copy link
Owner

@oblador oblador left a comment

Choose a reason for hiding this comment

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

Cool! Some stylistic changes and a nit about naming otherwise good job!

@oblador
Copy link
Owner

oblador commented Apr 4, 2018

@itchingpixels Thanks again for your PR. Are you planning on addressing the comments?

@ohtangza
Copy link

Can't wait using it!

@almostintuitive
Copy link
Contributor Author

I'll get this done in 24 hours :)

@almostintuitive
Copy link
Contributor Author

done!:)

@oblador oblador merged commit 34e0ef0 into oblador:master May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants