azazdeaz / react-gsap-enhancer

Use the full power of React and GSAP together
MIT License
726 stars 38 forks source link

Incremental calls to `onComplete` #11

Closed ryanblakeley closed 8 years ago

ryanblakeley commented 8 years ago

I'll preface by saying that I'm new to GSAP so I may be using their API incorrectly. But I've done my best to copy the demo patterns.

animationSource

function createScaleDown ({target, options}) {
  return TweenMax.to(options.toggleIcon, 0.1, {
    scale: 0.65,
    onComplete: () => {
      console.log('complete - scale down')}
  });
}

addAnimation()

_handleScaleDown = (event) => {
  console.log('add - scale down');
  this.addAnimation(createScaleDown, {toggleIcon: this['toggle-icon']});
}

I have other animations in the file which follow a similar pattern. The first time this executes, there is one of each console statement, as expected. Subsequent executions incrementally repeat the complete - scale down statement while only logging add - scale down once.
At the top of my file I have:

import TweenMax from 'gsap';
import GSAP from 'react-gsap-enhancer';

Edit: This is the file where this occurs.

azazdeaz commented 8 years ago

Hi @rblakeley,

Cool animation!! Thanks for the helpful explanation of the problem, i see it but i don't know what is the best way to resolve it. I will investigate it as soon as i can. Until that if you call controller.kill() after the animation completed that resolves the problem.

ryanblakeley commented 8 years ago

@azazdeaz Thanks. The way I'm seeing it, when I call controller.kill() after the animation completes, I lose the styles from the end of the animation. So it sort of jumps to the wrong style.

Adding some insight: the calls to onComplete are doubling with each event increment with each event by the total number of TweenMax animations which get created.

ryanblakeley commented 8 years ago

IMO, this error could be expected because of the way the API for react-gsap-enhancer is designed. If my handler is calling addAnimation every time it handles an event, I would expect a new animation is going to be created. This would explain the existence of duplicates unless you destroy each one when it completes.

Your demos don't seem to encounter that problem though. So it seems more likely that something is uniquely wrong about the way I'm using it. Just intuitively though, the error isn't that surprising.

Edit: I would like to dig into your source to debug how animations are cached. But there isn't any documentation on contributing or building.

ryanblakeley commented 8 years ago

I was able to reproduce this same incrementing error with one of your demos.

Go to line 11 and add onComplete: () => { console.log('complete - anim') }.

createAnim

function createAnim(utils) {
  var box = utils.target.find({name: 'box'})

  return new TimelineMax()
    .to(box, 0.44, {
      x: utils.options.x,
      y: utils.options.y,
      ease: Elastic.easeOut,
      onComplete: () => {console.log('complete - anim')}, // <-- add this
    })
}

Open the console and interact with the demo. You'll notice that complete - anim gets logged incrementally with each event.

Edit: Working through the source code, I'm noticing that a new controller will be added to this.__runningAnimations when a controller with the same animationSource, target, and options has already been added. So a refactor of controller caching might be needed.

azazdeaz commented 8 years ago

Thanks a lot for your great help to find and fix this bug @rblakeley!

The fix is published on npm @0.2.1

Hope everything will work find for you too. The problem was that it invalidates and restarts all the added animations on every react update to pick up all the changes that can affect the animations. GSAP has an option to suppress the events while restarting the animation and setting the time again but i forgot that i heva to use it here. Thanks again for reporting this bug and helping to fix it!

I also added a minimal CONTRIBITING.md and updated this demo.

ryanblakeley commented 8 years ago

Thanks for solving that so quickly. It's always nice when it's literally a one-word patch.