SlideUp and SlideDown break when switching too fast

[Robinfr]

If you switch the animation prop between slideUp and slideDown too fast, the animation kinda breaks, as in, it becomes stuck in this in between state where it has almost no height.

It seems that this is caused by the fact that both of these animations take the current height as the starting height instead of the original height. This is causing problems if you switch between the animations in the middle of the animation, e.g. by clicking on a Read more/Read less button really fast.

[fionawhim]

This is likely a consequence of VelocityComponent using stop when switching animations. You could add a config property to change that to finish or allow it to queue, the way that the runAnimation method takes options.

See the demo for how these behaviors differ when calling runAnimation.

[Robinfr]

Ah I see how the demo does it. But this feels more like a workaround than the way it should be to be honest.. This forces me to directly use the Velocity API instead of using this wrapper in a nice way..

Edit: OK so runAnimation is not Velocity API. My bad. Still, it feels a bit weird to work this way..

Especially because the docs say

Useful for when you want an animation that corresponds to an event but not a particular model state change (e.g. a "bump" when a click occurs).

But this change is happening in response to a state change.

[fionawhim]

The same sort of configuration that is used in `runAnimation` could be added to the props for VelocityComponent, if you're interested in sending in a PR. It's all a workaround for slideUp/slideDown's implementation.

…

On Tue, Apr 18, 2017 at 9:35 AM, Robin ***@***.***> wrote: Ah I see how the demo does it. But this feels more like a workaround than the way it should be to be honest.. This forces me to directly use the Velocity API instead of using this wrapper in a nice way.. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#185 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAP65Xqb1JCOAGk0UE_zFiDvMaLVfim-ks5rxLwrgaJpZM4NAEoA> .

[Robinfr]

@finneganh wouldn't it be better to add a prop to the velocity component? That way in componentWillUpdate we can check for that prop. If prop==='stop' then stopAnimation, otherwise finishAnimation.

What do you think?

[fionawhim]

Yep, I think we're on the same page.

…

On Tue, Apr 18, 2017 at 10:01 AM, Robin ***@***.***> wrote: @finneganh <https://github.com/finneganh> wouldn't it be better to add a prop to the velocity component? That way in componentWillUpdate we can check for that prop. If prop==='stop' then stopAnimation, otherwise finishAnimation. What do you think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#185 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAP65aPsI1m_ax7XF0Li_LQL9mgCs4AJks5rxMJLgaJpZM4NAEoA> .

[Robinfr]

I'll make a PR in that case.

[Robinfr]

Added the PR. Let me know if you agree or if you need changes.

P.S.: I also tried updating the demo, but in the demo the animation prop never changes so that wasn't possible with this new prop. It might be doable if you also check if the children prop changes in componentWillUpdate but I see that as a different issue.

[fionawhim]

I'm working on updating the demo to be hosted on Glitch. Will see if there's a good place to put it in then.

[fionawhim]

Cool, this is now live.

[Robinfr]

@finneganh I tried it out but it still doesn't seem to have the desired effect. I will check it out once more once I have some time.

[Robinfr]

I think I figured out why this is happening. Velocity is using Promises and returns a Promise for most functions. Yet in velocity-component.js you're calling the functions as if they are synchronous functions. I believe this might be causing issues with race conditions. I would suggest rewriting parts of the velocity component implementing promises to extend what Velocity is doing.. @finneganh

[fionawhim]

Interesting. I believe that the Promise bits have been added since velocity-react was written.

I can take a look. This will probably help if things like stop / finish are actually asynchronous, but not if the Promises are just an alternative to the complete callback for animations.

[Antho2407]

@finneganh Any updates on that ? Having the same issue, but can't control anything if I use the VelocityTransitionGroup component 😢

[markjaquith]

I ran into this issue today, and had an idea.

Velocity accepts a begin option that runs when an animation is starting. Since the issue seemed to be that the partial height from a previous interruption gets "stuck" into the style and used as the new height, I figured I could use begin to reset the element's height to null before the animation begins.

And that works! slideDown no longer gets stuck at a previous interrupt point. But it's annoying to have to manually pass a begin option for every call to VelocityComponent, so I made a wrapper that automatically adds it for any slideDown animations.

class MyVelocity extends React.Component {
  constructor(props) {
    super(props);
    // https://reactjs.org/docs/handling-events.html
    this.begin = this.begin.bind(this);
  }

  begin(elements) {
    if ( 'slideDown' === this.props.animation ) {
      elements.forEach((el) => {
        if ( 'none' === el.style.display ) {
          el.style.height = null;
        }
      });
    }
  }

  render() {
    const props = Object.assign({}, {begin: this.begin}, this.props);
    return (
      <VelocityComponent {...props}>
        {this.props.children}
      </VelocityComponent>
    );
  }
}

Now you just use <MyVelocity /> the same way you'd use <VelocityComponent />, and it will keep your slideDown animations from getting stuck.

It's a bandaid, but for anyone tearing their hair out about this, I hope this helps.