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

onEnd called forever #1803

Closed
4 tasks done
dmwyatt opened this issue Mar 10, 2021 · 5 comments
Closed
4 tasks done

onEnd called forever #1803

dmwyatt opened this issue Mar 10, 2021 · 5 comments

Comments

@dmwyatt
Copy link

dmwyatt commented Mar 10, 2021

Bugs and Questions

Checklist

  • This is not a victory-native specific issue. (Issues that only appear in victory-native should be opened here)

  • I have read through the FAQ and Guides before asking a question

  • I am using the latest version of Victory

  • I've searched open issues to make sure I'm not opening a duplicate issue

The Problem

I'm fairly sure I'm just not understanding animation's onEnd, but maybe there's a bug?

Why does onEnd get called forever?

I'm just trying to get notified when the animation is done...

Reproduction

https://codesandbox.io/s/victory-on-end-forever-nq09z?file=/src/App.js

@dmwyatt
Copy link
Author

dmwyatt commented Mar 10, 2021

I haven't quite grokked how everything fits together, but it looks like maybe there should be a check here to see if onEnd has been called already? Though, I'm still not quite clear on where and when traverseQueue could be called.

@boygirl
Copy link
Contributor

boygirl commented Mar 18, 2021

Oh, interesting! Thank you for the minimal reproduction, that's super helpful. I'll take a look.

@dmwyatt
Copy link
Author

dmwyatt commented Mar 18, 2021

Of note, if you change the animation duration there's a corresponding change of the rate that the number increments, so I guess that might tell you...something?

@boygirl
Copy link
Contributor

boygirl commented Mar 22, 2021

Hmm... It also looks like onEnd is only getting called repeatedly if it includes a something that sets state. In your example, the whole tree that contains the chart is being rerendered after your onEnd sets the count state.

@becca-bailey
Copy link
Contributor

Closing this issue in favor of #2104

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

No branches or pull requests

3 participants