search

mozilla / reflex

Functional reactive UI library
MIT License
367 stars 20 forks source link

Version 0.4 #47

Closed Gozala closed 8 years ago

Gozala commented 8 years ago

Overview of the changes

  1. Fix regression introduced by 4e13b24 & ba21bbb that cause misbehaviors on synchronous tasks.
  2. Factor out requestAnimationFrame scheduler from Effects library and expose it via tasks instead. Having it part of Effects did not really made much sense.
  3. Renamed Effects.task(task) to Effects.perform(task) as former was pretty confusing.
  4. Changed API for creating cancellable tasks so they won't perform extra allocations on execution. Most relevant for requestAnimationFrame task that are used a lot during animations.
  5. Change class hierarchies to avoid passing own methods to super calls.
  6. Add Task.prototype.recover API so that task failures can more efficiently & intuitively be translated to error actions.
Gozala commented 8 years ago

@tschneidereit r?

tschneidereit commented 8 years ago

Looks good with feedback addressed. I'm not too happy with a new requestAnimationFrame function that works differently from the builtin version, but I'm also not sure what would be a better name. Perhaps something like queueAnimationFrameCallback and removeAnimationFrameCallback or something along those lines.

Gozala commented 8 years ago

Looks good with feedback addressed. I'm not too happy with a new requestAnimationFrame function that works differently from the builtin version, but I'm also not sure what would be a better name. Perhaps something like queueAnimationFrameCallback and removeAnimationFrameCallback or something along those lines.

@tschneidereit Is that in reference to exports of preemptive-animation-frame module ?

tschneidereit commented 8 years ago

@tschneidereit Is that in reference to exports of preemptive-animation-frame module ?

It is, yes.

Gozala commented 8 years ago

@tschneidereit Is that in reference to exports of preemptive-animation-frame module ?

It is, yes.

I'm afraid I can't think of better name maybe requestPreemptiveAnimationFrame / cancelPreemptiveAnimationFrame ? But then module name kind of already implies that no ? Only difference between native and this implementation is that this one is preemptive, meaning it ensures you don't miss a frame regardless when you requested it.

Given that it's just an internal module (that I actually want to factor out into separate package) I'll leave it as is for 0.4 as I would not like to block a release for this. My interpretation of your comment on this was that it was "nice to have" vs "must have".

tschneidereit commented 8 years ago

Given that it's just an internal module (that I actually want to factor out into separate package) I'll leave it as is for 0.4 as I would not like to block a release for this. My interpretation of your comment on this was that it was "nice to have" vs "must have".

That makes sense.