search

jeremyckahn / shifty

The fastest TypeScript animation engine on the web
https://jeremyckahn.github.io/shifty/doc/
MIT License
1.54k stars 88 forks source link

Improve error message when stop is called. #115

Closed wparad closed 4 years ago

wparad commented 4 years ago

Spent way to long trying to figure why errors were being thrown. Also helps the myriad of other libraries using shifty:

https://github.com/kimmobrunfeldt/progressbar.js/issues/255

jeremyckahn commented 4 years ago

Hi @wparad, sorry for all the trouble you had tracking this down. Thank you for this PR! This all looks pretty good to me. I will test it out and get it released just as soon as I have the time to!

wparad commented 4 years ago

Hi @wparad, sorry for all the trouble you had tracking this down. Thank you for this PR! This all looks pretty good to me. I will test it out and get it released just as soon as I have the time to!

Awesome to hear. I actually wasn't sure about the _attachment property being passed to the resolve and reject functions as a second parameter. I'm not aware of the native promises or promise libraries doing anything with that. But perhaps I am missing something.

jeremyckahn commented 4 years ago

Awesome to hear. I actually wasn't sure about the _attachment property being passed to the resolve and reject functions as a second parameter. I'm not aware of the native promises or promise libraries doing anything with that. But perhaps I am missing something.

Oh, I just realized that this is going to change how tween promises get rejected a tiny bit. I'm okay with that, but I think it warrants a minor version bump when I release this!

In that case, I'd like to roll the _attachment data into the first parameter. I can take care of that after merging this!

jeremyckahn commented 4 years ago

I had to make a few changes to the PR, but I will merge this now. Thank you for the fix @wparad!

jeremyckahn commented 4 years ago

This is in 2.9.0.

kimmobrunfeldt commented 4 years ago

This improved error message is great, but I thought it would be perfectly fine to call the stop method while an animation is running without throwing an error.

jeremyckahn commented 4 years ago

You raise a good point @kimmobrunfeldt. It's fair to say that it's not an error to prematurely stop a tween. I like the catch/reject Promise semantics to differentiate between "tween completed" and "tween canceled," though I agree that calling all canceled tweens an error might be a little heavy-handed.

I don't have much time to do significant development on Shifty at the moment, but I'll think on a more robust API for this and roll that into 2.10.0 when I can!

jeremyckahn commented 4 years ago

FYI @kimmobrunfeldt @wparad, I just released Shifty 2.10.0 which removes the reject logic entirely (https://github.com/jeremyckahn/shifty/commit/d18b2780af584060539529bf80649ed0a0646d5d). After giving it some thought, it didn't seem necessary and was just creating problems. Hopefully this makes Shifty easier to use in your projects!