-
Notifications
You must be signed in to change notification settings - Fork 56
Request animation frame #91
Request animation frame #91
Conversation
* `requestAnimationFrame :: forall eff. Eff (dom :: DOM | eff) Unit -> Window -> Eff (dom :: DOM | eff ) RequestAnimationFrameId` * `cancelAnimationFrame :: forall eff. RequestAnimationFrameId -> Window -> Eff (dom :: DOM | eff) Unit`
* `requestIdleCallback :: forall eff. Eff (dom :: DOM | eff) Unit -> { timeout :: Int } -> Window -> Eff (dom :: DOM | eff ) RequestIdleCallbackId` * `cancelIdleCallback :: forall eff. RequestIdleCallbackId -> { timeout :: Int } -> Window -> Eff (dom :: DOM | eff) Unit` In the web api `timeout` option can be skipped, but one can always set it to `0` to get the same behaviour (or just pass `unsafeCoerce unit`).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just a couple of things that could use tweaking / discussion.
src/DOM/HTML/Window.js
Outdated
|
||
exports._cancelAnimationFrame = function(id) { | ||
return function(window) { | ||
return window.cancelAnimationFrame(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need an extra function() {
wrapping the inner thing here as we're Eff
-typing these things.
src/DOM/HTML/Window.js
Outdated
|
||
exports._cancelIdleCallback = function(id) { | ||
return function(window) { | ||
return window.cancelIdleCallback(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with this one!
|
||
newtype RequestAnimationFrameId = RequestAnimationFrameId Int | ||
|
||
derive instance newtypeRequestAnimationFrameId :: Newtype RequestAnimationFrameId _ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you derive Eq
and Ord
for both of the newtypes introduced in here please?
src/DOM/HTML/Window.purs
Outdated
|
||
foreign import _requestIdleCallback :: forall eff. Eff (dom :: DOM | eff) Unit -> { timeout :: Int } -> Window -> Eff (dom :: DOM | eff) Int | ||
|
||
requestIdleCallback :: forall eff. Eff (dom :: DOM | eff) Unit -> { timeout :: Int } -> Window -> Eff (dom :: DOM | eff ) RequestIdleCallbackId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should put the options first here? Seems like a more natural ordering for currying perhaps.
Also, is timeout: 0
different from not specifying a value at all? If so, I guess we'll have to provide two varieties of requestIdleCallback
to either accept options or not / have Maybe
properties or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec says that the timeout is only taken into account if it is a positive integer.
Thanks for making those changes, and adding the comment about 0 timeout. Just need those additional updates to the JS (adding the thunk for |
* thunks for EFF * swap arguments of cancelIdleCallback
Ups, I forgot to commit them. Here there are :) |
Thanks! |
Completes #90.