google-fabric / velocity-react

React components for Velocity.js
MIT License
2.94k stars 168 forks source link

slideUp and removing children nodes error #109

Closed rnsloan closed 8 years ago

rnsloan commented 8 years ago

If you look at this modification of the scrolling-group example https://github.com/rnsloan/velocity-react/commit/bd1a8064aa180865e13e6fd5fd4d3faaa9d4a8d9 a remove all button has been added and the animations have been changed to slideDown and slideUp.

Clicking on the remove all button triggers a react error Cannot read property 'componentDidLeave' of undefined (which comes from ReactTransitionGroup.js). If the animations are left as Animations.In and Animations.Out it works fine.

To get a better understanding of this library, could anyone explain why using slideUp with an action that removes nodes errors but Animations.Out is fine.

fionawhim commented 8 years ago

Curious. The slideUp/slideDown animations are handled a little bit specially by Velocity; I don't know if that affects the way that the complete callback is called.

Looking at where the error might happen in ReactTransitionGroup, it's something around the doneFn callback being called but the child element it was originally bound to is no longer present. Might be worth checking to see if somehow slideUp is calling its complete handler twice or something.

I think that removeAll may be aggravating this because that example as originally written was being sloppy about keys. They've been based on array position, rather than something more intrinsic. That may have masked this bug because the doneFn lookup by key that happens in ReactTransitionGroup would have succeeded accidentally if the key were re-used.

rnsloan commented 8 years ago

Thanks. I will take a bit of a look into the slide animations in velocity to get a better understanding, but feel free to close this issue as the solution to not use them solves my issue for now.

rnsloan commented 8 years ago

Related React error https://github.com/facebook/react/issues/4876

hassankhan commented 8 years ago

Hi, I seem to be getting this issue, along with what seems to be #79 as well. I followed the crossfade example as best as I could.

<VelocityTransitionGroup component="ul"
          enter={{animation: 'slideDown', duration: this.props.duration}}
          leave={{animation: 'slideUp', duration: this.props.duration}}>
          {this.state.isActive ? this.props.children : null}
</VelocityTransitionGroup>

When isActive is false, I get the Cannot read property 'componentDidLeave' of undefined errors.

As an aside, if I add a className, then the leave transition stops working.

ssilve1989 commented 8 years ago

I to have this problem following a very similar approach as @hassankhan

mattjfong commented 7 years ago

There's a commit to velocity to fix this issue for slideUp/slideDown animations. Added after the 1.3.1 release.