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

Trigger onChange and onComplete with the right values #5813

Merged
merged 2 commits into from Jul 21, 2019

Conversation

asturur
Copy link
Member

@asturur asturur commented Jul 20, 2019

Close #5700

Trigger onChange and onComplete with values as accurate as possible.

@asturur asturur merged commit 84c725a into master Jul 21, 2019
@asturur asturur mentioned this pull request Aug 19, 2019
@idelsink
Copy link

Would this change not fall under the 'incompatible API changes' section of semver? And this should cause the version to not be 3.4.0 but 4.x.x?

@asturur
Copy link
Member Author

asturur commented Aug 26, 2019

the difference is minimal.
You get the arguments on onComplete, exactly as you would get on onComplete when called by onAbort.
You get the onChange arguments capped at max value if the time is bigger than finish time.

Did you encounter a breaking change?

@idelsink
Copy link

idelsink commented Aug 27, 2019

I did encounter a breaking change but that was mostly my 'incompetence' (for lack of a better word) :-) for depending on the undefined value of the onComplete function argument. I wanted to know if another animation was running in the onComplete method so that I can hide something when it was not moving. I reworked it by using the onChange percentage and the abort method. This seems to work as expected now.

@asturur
Copy link
Member Author

asturur commented Aug 27, 2019

the fabricjs animation system is not that great. sometimes i wonder if i should remove and just get some examples with other animation libraries.

@asturur asturur deleted the fix-animation-onComplete-values branch August 27, 2019 09:47
@idelsink
Copy link

You mean like incorporate some external library or take inspiration from another library? I do think that as a whole animation support is a real strong feature of FabricJS

@asturur
Copy link
Member Author

asturur commented Aug 27, 2019

i was meaning that any animation library can animate a fabricjs object.
like tween.js or others ( that i do not know of ).
I m not sure it make sense to have our animation code.

thiagocunha pushed a commit to thiagocunha/fabric.js that referenced this pull request Nov 18, 2019
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.

Animate Calculation Issue in 3.0.0
2 participants