Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Add option to run $timeout as an requestAnimationFrame callback #4278

Closed
cburgdorf opened this issue Oct 4, 2013 · 14 comments
Closed

Add option to run $timeout as an requestAnimationFrame callback #4278

cburgdorf opened this issue Oct 4, 2013 · 14 comments

Comments

@cburgdorf
Copy link
Contributor

I think it's been claimed by a bunch of smart folks that you should avoid using setTimeout at all costs and replace it with requestAnimationFrame.

Even if you know that you want to run some code in 1000ms it's still smarter to schedule the code with setTimeout but then call requestAnimationFrame to actually do the work. At least if your work is rendering related.

To let code speak, this:

setTimeout(function(){
    requestAnimationFrame(function(){
        doWork();
    });
},1000);

So in terms of Angular this could look like this:

//I would advise changing the last parameter to be an option object 
//because currently it's used for invokeApply
$timeout(doWork, 1000, { invokeApply: false, requestAnimationFrame: true});

I wonder if you guys agree that it makes sense to have it baked into the framework. After all, it's just a couple of lines of code for the framework that makes life easier for the developer because it takes care of $apply and returning you a promise etc.

If you think it's a good idea, I'll craft a PR for it.

@petebacondarwin
Copy link
Member

I think @matsko is on this?

@matsko
Copy link
Contributor

matsko commented Oct 5, 2013

Yeah I have it in a commit offline. In the process of doing a huge refactoring on $animate to make this work better. I'll close this issue once I have the commit pushed up which should be in the next couple days or so.

@cburgdorf
Copy link
Contributor Author

Ah, great to hear!

@matsko
Copy link
Contributor

matsko commented Oct 15, 2013

@cburgdorf looks like we don't need this feature. The way that ngAnimate works now is it uses a global timeout (just one) to manage all concurrent animations. So if you have a repeater with 1000 items then everything will be grouped together. I tried doing this with requestAnimationFrame() but the cancellation wasn't fast enough to group everything together. Also requestAnimationFrame() isn't supported by android devices and overall the purpose of the feature is to make incremental JavaScript animations more performant. Which isn't what we're trying to do with CSS3 transitions. So looks like we won't be using this for now.

@matsko matsko closed this as completed Oct 15, 2013
@cburgdorf
Copy link
Contributor Author

Well, not for ngAnimate but I thought it's quite useful for angular in general. Don't you think?

@c0bra
Copy link

c0bra commented Jan 10, 2014

Agreed. It would be a handy shortcut for those of us using JS to do things that can cause jank.

@matsko
Copy link
Contributor

matsko commented Jan 10, 2014

Going to see if this works with our new setup on 1.2.x

@matsko
Copy link
Contributor

matsko commented Jan 12, 2014

Good news. This works great with the latest ngAnimate code. And it fixes a bunch of flicker-related bugs.

This will surface in 1.2.9.

@matsko matsko reopened this Jan 12, 2014
matsko added a commit to matsko/angular.js that referenced this issue Jan 12, 2014
matsko added a commit to matsko/angular.js that referenced this issue Jan 12, 2014
matsko added a commit to matsko/angular.js that referenced this issue Jan 13, 2014
matsko added a commit to matsko/angular.js that referenced this issue Jan 13, 2014
matsko added a commit to matsko/angular.js that referenced this issue Jan 14, 2014
matsko added a commit to matsko/angular.js that referenced this issue Jan 14, 2014
IgorMinar pushed a commit to IgorMinar/angular.js that referenced this issue Jan 14, 2014
matsko added a commit to matsko/angular.js that referenced this issue Jan 14, 2014
@matsko matsko closed this as completed in 4ae3184 Jan 14, 2014
@0x-r4bbit
Copy link
Contributor

@matsko great work!

@revolunet
Copy link
Contributor

awesome thanks

@matsko
Copy link
Contributor

matsko commented Jan 14, 2014

@c0bra there is a hidden service for this called $$animateReflow (just don't tell anyone ;)).

@cburgdorf
Copy link
Contributor Author

Great to see this made it into core. I think $requestAnimationFrame or $rAF would have been a better name though for three reasons:

  • it follows the rule of least astonishment for the developer
  • it's more symmetric to things like $timeout and $interval
  • the word reflow might be a bit misleading as a function scheduled with the service doesn't necessarily triggers a reflow

There's also a more robust polyfill which was initially created by an Opera guy. Paul Irish blogged about it:

http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/

@0x-r4bbit
Copy link
Contributor

Agreeing with @cburgdorf

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@WillsB3
Copy link

WillsB3 commented Nov 24, 2014

Why not expose (and document) $$RAFProvider?

There are legitimate cases when we need to use a RAF ourselves for things which ng-animate isn't suitable for...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants