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

Implemented Promises support #143

Merged
merged 2 commits into from
Jul 8, 2014
Merged

Conversation

ydaniv
Copy link
Contributor

@ydaniv ydaniv commented Jul 2, 2014

Notice on stop I reject the promise with the chained context. I know it's contrary to what Dominic said but it seemed to be the best solution, and I couldn't find anything in the specs about the reason having to be an error.

Also, I went over the code and think I covered all places where you return from the animate method, so please check if I missed anything.

@ydaniv
Copy link
Contributor Author

ydaniv commented Jul 2, 2014

Continue from #87.

@mikaelkaron
Copy link

It is possible that @domenic will poke you with something sharp for rejecting without there actually being an error. From what I can understand that's a no-no.

I think that rejecting with a a reason that is not instanceof Error is OK (it makes life a lot easier in promise libs like when.js that can replicate try/catch logic in an async fashion, but hardly a requirement)

@ydaniv
Copy link
Contributor Author

ydaniv commented Jul 2, 2014

@mikaelkaron that's cool. My intention was to get this out there so that we can decide on the optimal solution.

How would you expect a promise to behave for a stopped animation? Fulfill it? Reject it?

@domenic
Copy link

domenic commented Jul 2, 2014

Is stopping an animation an exceptional situation? I.e. if this was a synchronous function, would it thrown an exception?

@ydaniv
Copy link
Contributor Author

ydaniv commented Jul 2, 2014

@domenic thanks for the link! this is quite helpful. Since this is an act initiated by the user I guess it won't count as an exceptional situation.

In a sync promises-less API the user expects the usual flow, however, the complete callback should not be invoked, and it does when resolving a promise.

So I guess we should resolve it, but still somehow suppress the complete callback.
Let me give it a try.

@ydaniv
Copy link
Contributor Author

ydaniv commented Jul 2, 2014

Oh! Silly me 👻 resolving will just resolve and not do anything else.

@julianshapiro
Copy link
Owner

This is about 1 day away from being implemented.

@julianshapiro julianshapiro merged commit 66f521c into julianshapiro:master Jul 8, 2014
julianshapiro added a commit that referenced this pull request Jul 8, 2014
Closes #153. Closes #110. Closes #143. Closes #95. Closes #156.
@julianshapiro
Copy link
Owner

Integrated. See http://velocityjs.org/#promises. Thank you for your help, @domenic and @mikaelkaron.

@Rycochet Rycochet mentioned this pull request Jul 8, 2014
Rycochet pushed a commit that referenced this pull request Aug 3, 2020
Closes #153. Closes #110. Closes #143. Closes #95. Closes #156.
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.

4 participants