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

Commit ea8016c

Browse files
matskomatsko
committed
fix(ngAnimate): use requestAnimationFrame to space out child animations
This reverts the previous behaviour of using foreced reflows to deal with preparation classes in favour of a system that uses requestAnimationFrame (RAF). Closes #12669 Closes #12594 Closes #12655 Closes #12631 Closes #12612 Closes #12187
1 parent c3d5e33 commit ea8016c

13 files changed

+374
-194
lines changed

‎angularFiles.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ var angularFiles = {
9393
'ngAnimate': [
9494
'src/ngAnimate/shared.js',
9595
'src/ngAnimate/body.js',
96+
'src/ngAnimate/rafScheduler.js',
9697
'src/ngAnimate/animateChildrenDirective.js',
9798
'src/ngAnimate/animateCss.js',
9899
'src/ngAnimate/animateCssDriver.js',

‎src/ngAnimate/animateCss.js

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
328328
var gcsLookup = createLocalCacheLookup();
329329
var gcsStaggerLookup = createLocalCacheLookup();
330330

331-
this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout', '$$forceReflow', '$sniffer', '$$rAF', '$animate',
332-
function($window, $$jqLite, $$AnimateRunner, $timeout, $$forceReflow, $sniffer, $$rAF, $animate) {
331+
this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout',
332+
'$$forceReflow', '$sniffer', '$$rAFScheduler', '$animate',
333+
function($window, $$jqLite, $$AnimateRunner, $timeout,
334+
$$forceReflow, $sniffer, $$rAFScheduler, $animate) {
333335

334336
var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
335337

@@ -389,12 +391,8 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
389391
var cancelLastRAFRequest;
390392
var rafWaitQueue = [];
391393
function waitUntilQuiet(callback) {
392-
if (cancelLastRAFRequest) {
393-
cancelLastRAFRequest(); //cancels the request
394-
}
395394
rafWaitQueue.push(callback);
396-
cancelLastRAFRequest = $$rAF(function() {
397-
cancelLastRAFRequest = null;
395+
$$rAFScheduler.waitUntilQuiet(function() {
398396
gcsLookup.flush();
399397
gcsStaggerLookup.flush();
400398

@@ -485,7 +483,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
485483
// there actually is a detected transition or keyframe animation
486484
if (options.applyClassesEarly && addRemoveClassName.length) {
487485
applyAnimationClasses(element, options);
488-
addRemoveClassName = '';
489486
}
490487

491488
var preparationClasses = [structuralClassName, addRemoveClassName].join(' ').trim();

‎src/ngAnimate/animateCssDriver.js

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro
2222

2323
var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
2424

25-
return function initDriverFn(animationDetails, onBeforeClassesAppliedCb) {
25+
return function initDriverFn(animationDetails) {
2626
return animationDetails.from && animationDetails.to
2727
? prepareFromToAnchorAnimation(animationDetails.from,
2828
animationDetails.to,
2929
animationDetails.classes,
3030
animationDetails.anchors)
31-
: prepareRegularAnimation(animationDetails, onBeforeClassesAppliedCb);
31+
: prepareRegularAnimation(animationDetails);
3232
};
3333

3434
function filterCssClasses(classes) {
@@ -224,21 +224,14 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro
224224
};
225225
}
226226

227-
function prepareRegularAnimation(animationDetails, onBeforeClassesAppliedCb) {
227+
function prepareRegularAnimation(animationDetails) {
228228
var element = animationDetails.element;
229229
var options = animationDetails.options || {};
230230

231-
// since the ng-EVENT, class-ADD and class-REMOVE classes are applied inside
232-
// of the animateQueue pre and postDigest stages then there is no need to add
233-
// then them here as well.
234-
options.$$skipPreparationClasses = true;
235-
236-
// during the pre/post digest stages inside of animateQueue we also performed
237-
// the blocking (transition:-9999s) so there is no point in doing that again.
238-
options.skipBlocking = true;
239-
240231
if (animationDetails.structural) {
241232
options.event = animationDetails.event;
233+
options.structural = true;
234+
options.applyClassesEarly = true;
242235

243236
// we special case the leave animation since we want to ensure that
244237
// the element is removed as soon as the animation is over. Otherwise
@@ -248,11 +241,6 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro
248241
}
249242
}
250243

251-
// we apply the classes right away since the pre-digest took care of the
252-
// preparation classes.
253-
onBeforeClassesAppliedCb(element);
254-
applyAnimationClasses(element, options);
255-
256244
// We assign the preparationClasses as the actual animation event since
257245
// the internals of $animateCss will just suffix the event token values
258246
// with `-active` to trigger the animation.

‎src/ngAnimate/animateQueue.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,6 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
381381
return runner;
382382
}
383383

384-
applyGeneratedPreparationClasses(element, isStructural ? event : null, options);
385-
blockTransitions(node, SAFE_FAST_FORWARD_DURATION_VALUE);
386-
387384
// the counter keeps track of cancelled animations
388385
var counter = (existingAnimation.counter || 0) + 1;
389386
newAnimation.counter = counter;
@@ -442,10 +439,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
442439
: animationDetails.event;
443440

444441
markElementAnimationState(element, RUNNING_STATE);
445-
var realRunner = $$animation(element, event, animationDetails.options, function(e) {
446-
$$forceReflow();
447-
blockTransitions(getDomNode(e), false);
448-
});
442+
var realRunner = $$animation(element, event, animationDetails.options);
449443

450444
realRunner.done(function(status) {
451445
close(!status);

‎src/ngAnimate/animation.js

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
1919
return element.data(RUNNER_STORAGE_KEY);
2020
}
2121

22-
this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$HashMap',
23-
function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$HashMap) {
22+
this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$HashMap', '$$rAFScheduler',
23+
function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$HashMap, $$rAFScheduler) {
2424

2525
var animationQueue = [];
2626
var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
@@ -88,26 +88,27 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
8888
if (remainingLevelEntries <= 0) {
8989
remainingLevelEntries = nextLevelEntries;
9090
nextLevelEntries = 0;
91-
result = result.concat(row);
91+
result.push(row);
9292
row = [];
9393
}
9494
row.push(entry.fn);
95-
forEach(entry.children, function(childEntry) {
95+
entry.children.forEach(function(childEntry) {
9696
nextLevelEntries++;
9797
queue.push(childEntry);
9898
});
9999
remainingLevelEntries--;
100100
}
101101

102102
if (row.length) {
103-
result = result.concat(row);
103+
result.push(row);
104104
}
105+
105106
return result;
106107
}
107108
}
108109

109110
// TODO(matsko): document the signature in a better way
110-
return function(element, event, options, onBeforeClassesAppliedCb) {
111+
return function(element, event, options) {
111112
options = prepareAnimationOptions(options);
112113
var isStructural = ['enter', 'move', 'leave'].indexOf(event) >= 0;
113114

@@ -159,8 +160,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
159160
// the element was destroyed early on which removed the runner
160161
// form its storage. This means we can't animate this element
161162
// at all and it already has been closed due to destruction.
162-
var elm = entry.element;
163-
if (getRunner(elm) && getDomNode(elm).parentNode) {
163+
if (getRunner(entry.element)) {
164164
animations.push(entry);
165165
} else {
166166
entry.close();
@@ -191,7 +191,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
191191
: animationEntry.element;
192192

193193
if (getRunner(targetElement)) {
194-
var operation = invokeFirstDriver(animationEntry, onBeforeClassesAppliedCb);
194+
var operation = invokeFirstDriver(animationEntry);
195195
if (operation) {
196196
startAnimationFn = operation.start;
197197
}
@@ -211,11 +211,9 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
211211
});
212212

213213
// we need to sort each of the animations in order of parent to child
214-
// relationships. This ensures that the parent to child classes are
215-
// applied at the right time.
216-
forEach(sortAnimations(toBeSortedAnimations), function(triggerAnimation) {
217-
triggerAnimation();
218-
});
214+
// relationships. This ensures that the child classes are applied at the
215+
// right time.
216+
$$rAFScheduler(sortAnimations(toBeSortedAnimations));
219217
});
220218

221219
return runner;
@@ -285,7 +283,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
285283
var lookupKey = from.animationID.toString();
286284
if (!anchorGroups[lookupKey]) {
287285
var group = anchorGroups[lookupKey] = {
288-
// TODO(matsko): double-check this code
286+
structural: true,
289287
beforeStart: function() {
290288
fromAnimation.beforeStart();
291289
toAnimation.beforeStart();
@@ -339,15 +337,15 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
339337
return matches.join(' ');
340338
}
341339

342-
function invokeFirstDriver(animationDetails, onBeforeClassesAppliedCb) {
340+
function invokeFirstDriver(animationDetails) {
343341
// we loop in reverse order since the more general drivers (like CSS and JS)
344342
// may attempt more elements, but custom drivers are more particular
345343
for (var i = drivers.length - 1; i >= 0; i--) {
346344
var driverName = drivers[i];
347345
if (!$injector.has(driverName)) continue; // TODO(matsko): remove this check
348346

349347
var factory = $injector.get(driverName);
350-
var driver = factory(animationDetails, onBeforeClassesAppliedCb);
348+
var driver = factory(animationDetails);
351349
if (driver) {
352350
return driver;
353351
}

‎src/ngAnimate/module.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
55
$$BodyProvider,
66
$$AnimateAsyncRunFactory,
7+
$$rAFSchedulerFactory,
78
$$AnimateChildrenDirective,
89
$$AnimateRunnerFactory,
910
$$AnimateQueueProvider,
@@ -744,6 +745,7 @@ angular.module('ngAnimate', [])
744745
.provider('$$body', $$BodyProvider)
745746

746747
.directive('ngAnimateChildren', $$AnimateChildrenDirective)
748+
.factory('$$rAFScheduler', $$rAFSchedulerFactory)
747749

748750
.factory('$$AnimateRunner', $$AnimateRunnerFactory)
749751
.factory('$$animateAsyncRun', $$AnimateAsyncRunFactory)

‎src/ngAnimate/rafScheduler.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict';
2+
3+
var $$rAFSchedulerFactory = ['$$rAF', function($$rAF) {
4+
var queue, cancelFn;
5+
6+
function scheduler(tasks) {
7+
// we make a copy since RAFScheduler mutates the state
8+
// of the passed in array variable and this would be difficult
9+
// to track down on the outside code
10+
queue = queue.concat(tasks);
11+
nextTick();
12+
}
13+
14+
queue = scheduler.queue = [];
15+
16+
/* waitUntilQuiet does two things:
17+
* 1. It will run the FINAL `fn` value only when an uncancelled RAF has passed through
18+
* 2. It will delay the next wave of tasks from running until the quiet `fn` has run.
19+
*
20+
* The motivation here is that animation code can request more time from the scheduler
21+
* before the next wave runs. This allows for certain DOM properties such as classes to
22+
* be resolved in time for the next animation to run.
23+
*/
24+
scheduler.waitUntilQuiet = function(fn) {
25+
if (cancelFn) cancelFn();
26+
27+
cancelFn = $$rAF(function() {
28+
cancelFn = null;
29+
fn();
30+
nextTick();
31+
});
32+
};
33+
34+
return scheduler;
35+
36+
function nextTick() {
37+
if (!queue.length) return;
38+
39+
var items = queue.shift();
40+
for (var i = 0; i < items.length; i++) {
41+
items[i]();
42+
}
43+
44+
if (!cancelFn) {
45+
$$rAF(function() {
46+
if (!cancelFn) nextTick();
47+
});
48+
}
49+
}
50+
}];

‎test/ngAnimate/animateCssDriverSpec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe("ngAnimate $$animateCssDriver", function() {
105105
expect(capturedAnimation[1].applyClassesEarly).toBeFalsy();
106106

107107
driver({ element: element, structural: true });
108-
expect(capturedAnimation[1].applyClassesEarly).toBeFalsy();
108+
expect(capturedAnimation[1].applyClassesEarly).toBeTruthy();
109109
}));
110110

111111
it("should only set the event value if the animation is structural", inject(function() {

‎test/ngAnimate/animateSpec.js

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -492,74 +492,6 @@ describe("animations", function() {
492492
expect(element).not.toHaveClass('green');
493493
}));
494494

495-
they('should apply the $prop CSS class to the element before digest for the given event and remove when complete',
496-
{'ng-enter': 'enter', 'ng-leave': 'leave', 'ng-move': 'move'}, function(event) {
497-
498-
inject(function($animate, $rootScope, $document, $rootElement) {
499-
$animate.enabled(true);
500-
501-
var element = jqLite('<div></div>');
502-
var parent = jqLite('<div></div>');
503-
504-
$rootElement.append(parent);
505-
jqLite($document[0].body).append($rootElement);
506-
507-
var runner;
508-
if (event === 'leave') {
509-
parent.append(element);
510-
runner = $animate[event](element);
511-
} else {
512-
runner = $animate[event](element, parent);
513-
}
514-
515-
var expectedClassName = 'ng-' + event;
516-
517-
expect(element).toHaveClass(expectedClassName);
518-
519-
$rootScope.$digest();
520-
expect(element).toHaveClass(expectedClassName);
521-
522-
runner.end();
523-
expect(element).not.toHaveClass(expectedClassName);
524-
525-
dealoc(parent);
526-
});
527-
});
528-
529-
they('should add CSS classes with the $prop suffix when depending on the event and remove when complete',
530-
{'-add': 'add', '-remove': 'remove'}, function(event) {
531-
532-
inject(function($animate, $rootScope, $document, $rootElement) {
533-
$animate.enabled(true);
534-
535-
var element = jqLite('<div></div>');
536-
537-
$rootElement.append(element);
538-
jqLite($document[0].body).append($rootElement);
539-
540-
var classes = 'one two';
541-
var expectedClasses = ['one-',event,' ','two-', event].join('');
542-
543-
var runner;
544-
if (event === 'add') {
545-
runner = $animate.addClass(element, classes);
546-
} else {
547-
element.addClass(classes);
548-
runner = $animate.removeClass(element, classes);
549-
}
550-
551-
expect(element).toHaveClass(expectedClasses);
552-
553-
$rootScope.$digest();
554-
expect(element).toHaveClass(expectedClasses);
555-
556-
runner.end();
557-
expect(element).not.toHaveClass(expectedClasses);
558-
559-
dealoc(element);
560-
});
561-
});
562-
563495
they('$prop() should operate using a native DOM element',
564496
['enter', 'move', 'leave', 'addClass', 'removeClass', 'setClass', 'animate'], function(event) {
565497

0 commit comments

Comments
 (0)