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

call onEnd with nextTick in case component is unmounted #20

Closed
wants to merge 1 commit into from
Closed

call onEnd with nextTick in case component is unmounted #20

wants to merge 1 commit into from

Conversation

pstoica
Copy link

@pstoica pstoica commented Feb 12, 2015

I had a situation where I unmounted a component in the onEnd function. I was invoking the animation from a container. Maybe weird, but it works.

Anyway, this still runs after onEnd is invoked, causing a race condition since the component is unmounted:

    this.setState({
      tweenQueue: newTweenQueue,
    });

    requestAnimationFrame(this._rafCb);

This PR should fix this!

@chenglou
Copy link
Owner

Can you give me an example snippet of your code please?

@pstoica
Copy link
Author

pstoica commented Feb 12, 2015

    var hideOverlay = () => {
      this.setState({
        isOverlayShown: false,
        isOverlayHiding: false
      });
    };   

    this.setState({ isOverlayHiding: true });

    overlay.hideAnimation(hideOverlay);

hideOverlay is the onEnd function. This is kludgy and I read your animation gist afterwards. It would probably be better to use ReactTransitionGroup + react-tween-state for now instead of managing animated children like this?

@chenglou
Copy link
Owner

@pstoica Oh geez sorry for the wait. I thought I had an email reminder for this but apparently not. #24 raised the same problem. It's now solved in a different way. You get to keep your sync callback, whatever the implications of that is.

I'll close this now. Can you try the new release (0.0.5)? If it doesn't work then reopen the issue please.

Sorry again!

@chenglou chenglou closed this Mar 17, 2015
@pstoica
Copy link
Author

pstoica commented Mar 17, 2015

No worries, that looks like a good fix. I'm pretty sure it'll work for my case. Thanks for your help!

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.

2 participants