Skip to content

Commit 213c2a7

Browse files
matskomatsko
committed
fix(ngAnimate): ensure nested class-based animations are spaced out with a RAF
Prior to this fix any nested class-based animations (animations that are triggered with addClass/removeClass or ngClass) would cancel each other out when nested in DOM structure. This fix ensures that the nested animations are spaced out with sequenced RAFs so that parent CSS classes are applied prior to any ancestor animations that are scheduled to run. Closes angular#11812
1 parent 3545abf commit 213c2a7

File tree

8 files changed

+370
-29
lines changed

8 files changed

+370
-29
lines changed

‎angularFiles.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ var angularFiles = {
8888
'angularModules': {
8989
'ngAnimate': [
9090
'src/ngAnimate/shared.js',
91+
'src/ngAnimate/rafScheduler.js',
9192
'src/ngAnimate/animateChildrenDirective.js',
9293
'src/ngAnimate/animateCss.js',
9394
'src/ngAnimate/animateCssDriver.js',

‎src/ngAnimate/animateCss.js‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,9 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
392392
var gcsStaggerLookup = createLocalCacheLookup();
393393

394394
this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout',
395-
'$document', '$sniffer', '$$rAF',
395+
'$document', '$sniffer', '$$rAFScheduler',
396396
function($window, $$jqLite, $$AnimateRunner, $timeout,
397-
$document, $sniffer, $$rAF) {
397+
$document, $sniffer, $$rAFScheduler) {
398398

399399
var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
400400

@@ -452,15 +452,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
452452
}
453453

454454
var bod = getDomNode($document).body;
455-
var cancelLastRAFRequest;
456455
var rafWaitQueue = [];
457456
function waitUntilQuiet(callback) {
458-
if (cancelLastRAFRequest) {
459-
cancelLastRAFRequest(); //cancels the request
460-
}
461457
rafWaitQueue.push(callback);
462-
cancelLastRAFRequest = $$rAF(function() {
463-
cancelLastRAFRequest = null;
458+
$$rAFScheduler.waitUntilQuiet(function() {
464459
gcsLookup.flush();
465460
gcsStaggerLookup.flush();
466461

‎src/ngAnimate/animateQueue.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
361361
return runner;
362362
}
363363

364-
closeParentClassBasedAnimations(parent);
364+
if (isStructural) {
365+
closeParentClassBasedAnimations(parent);
366+
}
365367

366368
// the counter keeps track of cancelled animations
367369
var counter = (existingAnimation.counter || 0) + 1;
@@ -420,7 +422,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
420422
? 'setClass'
421423
: animationDetails.event;
422424

423-
closeParentClassBasedAnimations(parentElement);
425+
if (animationDetails.structural) {
426+
closeParentClassBasedAnimations(parentElement);
427+
}
424428

425429
markElementAnimationState(element, RUNNING_STATE);
426430
var realRunner = $$animation(element, event, animationDetails.options);

‎src/ngAnimate/animation.js‎

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

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

2525
var animationQueue = [];
2626
var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
2727

28+
var totalPendingClassBasedAnimations = 0;
29+
var totalActiveClassBasedAnimations = 0;
30+
var classBasedAnimationsQueue = [];
31+
2832
// TODO(matsko): document the signature in a better way
2933
return function(element, event, options) {
3034
options = prepareAnimationOptions(options);
@@ -53,12 +57,19 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
5357
options.tempClasses = null;
5458
}
5559

60+
var classBasedIndex;
61+
if (!isStructural) {
62+
classBasedIndex = totalPendingClassBasedAnimations;
63+
totalPendingClassBasedAnimations += 1;
64+
}
65+
5666
animationQueue.push({
5767
// this data is used by the postDigest code and passed into
5868
// the driver step function
5969
element: element,
6070
classes: classes,
6171
event: event,
72+
classBasedIndex: classBasedIndex,
6273
structural: isStructural,
6374
options: options,
6475
beforeStart: beforeStart,
@@ -73,6 +84,10 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
7384
if (animationQueue.length > 1) return runner;
7485

7586
$rootScope.$$postDigest(function() {
87+
totalActiveClassBasedAnimations = totalPendingClassBasedAnimations;
88+
totalPendingClassBasedAnimations = 0;
89+
classBasedAnimationsQueue.length = 0;
90+
7691
var animations = [];
7792
forEach(animationQueue, function(entry) {
7893
// the element was destroyed early on which removed the runner
@@ -87,23 +102,58 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
87102
animationQueue.length = 0;
88103

89104
forEach(groupAnimations(animations), function(animationEntry) {
90-
// it's important that we apply the `ng-animate` CSS class and the
91-
// temporary classes before we do any driver invoking since these
92-
// CSS classes may be required for proper CSS detection.
93-
animationEntry.beforeStart();
94-
95-
var operation = invokeFirstDriver(animationEntry);
96-
var triggerAnimationStart = operation && operation.start; /// TODO(matsko): only recognize operation.start()
97-
98-
var closeFn = animationEntry.close;
99-
if (!triggerAnimationStart) {
100-
closeFn();
105+
if (animationEntry.structural) {
106+
triggerAnimationStart();
101107
} else {
102-
var animationRunner = triggerAnimationStart();
103-
animationRunner.done(function(status) {
104-
closeFn(!status);
108+
classBasedAnimationsQueue.push({
109+
node: getDomNode(animationEntry.element),
110+
fn: triggerAnimationStart
105111
});
106-
updateAnimationRunners(animationEntry, animationRunner);
112+
113+
if (animationEntry.classBasedIndex === totalActiveClassBasedAnimations - 1) {
114+
// we need to sort each of the animations in order of parent to child
115+
// relationships. This ensures that the child classes are applied at the
116+
// right time.
117+
classBasedAnimationsQueue = classBasedAnimationsQueue.sort(function(a,b) {
118+
return b.node.contains(a.node);
119+
}).map(function(entry) {
120+
return entry.fn;
121+
});
122+
123+
$$rAFScheduler(classBasedAnimationsQueue);
124+
}
125+
}
126+
127+
function triggerAnimationStart() {
128+
// it's important that we apply the `ng-animate` CSS class and the
129+
// temporary classes before we do any driver invoking since these
130+
// CSS classes may be required for proper CSS detection.
131+
animationEntry.beforeStart();
132+
133+
var startAnimationFn, closeFn = animationEntry.close;
134+
135+
// in the event that the element was removed before the digest runs or
136+
// during the RAF sequencing then we should not trigger the animation.
137+
var targetElement = animationEntry.anchors
138+
? (animationEntry.from.element || animationEntry.to.element)
139+
: animationEntry.element;
140+
141+
if (getRunner(targetElement)) {
142+
var operation = invokeFirstDriver(animationEntry);
143+
if (operation) {
144+
startAnimationFn = operation.start;
145+
}
146+
}
147+
148+
if (!startAnimationFn) {
149+
closeFn();
150+
} else {
151+
var animationRunner = startAnimationFn();
152+
animationRunner.done(function(status) {
153+
closeFn(!status);
154+
});
155+
updateAnimationRunners(animationEntry, animationRunner);
156+
}
107157
}
108158
});
109159
});
@@ -175,7 +225,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
175225
var lookupKey = from.animationID.toString();
176226
if (!anchorGroups[lookupKey]) {
177227
var group = anchorGroups[lookupKey] = {
178-
// TODO(matsko): double-check this code
228+
structural: true,
179229
beforeStart: function() {
180230
fromAnimation.beforeStart();
181231
toAnimation.beforeStart();

‎src/ngAnimate/module.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
/* global angularAnimateModule: true,
44
55
$$rAFMutexFactory,
6+
$$rAFSchedulerFactory,
67
$$AnimateChildrenDirective,
78
$$AnimateRunnerFactory,
89
$$AnimateQueueProvider,
@@ -742,6 +743,7 @@ angular.module('ngAnimate', [])
742743
.directive('ngAnimateChildren', $$AnimateChildrenDirective)
743744

744745
.factory('$$rAFMutex', $$rAFMutexFactory)
746+
.factory('$$rAFScheduler', $$rAFSchedulerFactory)
745747

746748
.factory('$$AnimateRunner', $$AnimateRunnerFactory)
747749

‎src/ngAnimate/rafScheduler.js‎

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

‎test/ngAnimate/animationSpec.js‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,90 @@ describe('$$animation', function() {
288288
};
289289
}));
290290

291+
it('should space out multiple ancestorial class-based animations with a RAF in between',
292+
inject(function($rootScope, $$animation, $$rAF) {
293+
294+
var parent = element;
295+
element = jqLite('<div></div>');
296+
parent.append(element);
297+
298+
var child = jqLite('<div></div>');
299+
element.append(child);
300+
301+
$$animation(parent, 'addClass', { addClass: 'blue' });
302+
$$animation(element, 'addClass', { addClass: 'red' });
303+
$$animation(child, 'addClass', { addClass: 'green' });
304+
305+
$rootScope.$digest();
306+
307+
expect(captureLog.length).toBe(1);
308+
expect(capturedAnimation.options.addClass).toBe('blue');
309+
310+
$$rAF.flush();
311+
expect(captureLog.length).toBe(2);
312+
expect(capturedAnimation.options.addClass).toBe('red');
313+
314+
$$rAF.flush();
315+
expect(captureLog.length).toBe(3);
316+
expect(capturedAnimation.options.addClass).toBe('green');
317+
}));
318+
319+
it('should properly cancel out pending animations that are spaced with a RAF request before the digest completes',
320+
inject(function($rootScope, $$animation, $$rAF) {
321+
322+
var parent = element;
323+
element = jqLite('<div></div>');
324+
parent.append(element);
325+
326+
var child = jqLite('<div></div>');
327+
element.append(child);
328+
329+
var r1 = $$animation(parent, 'addClass', { addClass: 'blue' });
330+
var r2 = $$animation(element, 'addClass', { addClass: 'red' });
331+
var r3 = $$animation(child, 'addClass', { addClass: 'green' });
332+
333+
r2.end();
334+
335+
$rootScope.$digest();
336+
337+
expect(captureLog.length).toBe(1);
338+
expect(capturedAnimation.options.addClass).toBe('blue');
339+
340+
$$rAF.flush();
341+
342+
expect(captureLog.length).toBe(2);
343+
expect(capturedAnimation.options.addClass).toBe('green');
344+
}));
345+
346+
it('should properly cancel out pending animations that are spaced with a RAF request after the digest completes',
347+
inject(function($rootScope, $$animation, $$rAF) {
348+
349+
var parent = element;
350+
element = jqLite('<div></div>');
351+
parent.append(element);
352+
353+
var child = jqLite('<div></div>');
354+
element.append(child);
355+
356+
var r1 = $$animation(parent, 'addClass', { addClass: 'blue' });
357+
var r2 = $$animation(element, 'addClass', { addClass: 'red' });
358+
var r3 = $$animation(child, 'addClass', { addClass: 'green' });
359+
360+
$rootScope.$digest();
361+
362+
r2.end();
363+
364+
expect(captureLog.length).toBe(1);
365+
expect(capturedAnimation.options.addClass).toBe('blue');
366+
367+
$$rAF.flush();
368+
expect(captureLog.length).toBe(1);
369+
370+
$$rAF.flush();
371+
expect(captureLog.length).toBe(2);
372+
expect(capturedAnimation.options.addClass).toBe('green');
373+
}));
374+
291375
they('should return a runner that object that contains a $prop() function',
292376
['end', 'cancel', 'then'], function(method) {
293377
inject(function($$animation) {
@@ -513,7 +597,7 @@ describe('$$animation', function() {
513597
}));
514598

515599
it("should not group animations into an anchored animation if enter/leave events are NOT used",
516-
inject(function($$animation, $rootScope) {
600+
inject(function($$animation, $rootScope, $$rAF) {
517601

518602
fromElement.addClass('shared-class');
519603
fromElement.attr('ng-animate-ref', '1');
@@ -528,6 +612,7 @@ describe('$$animation', function() {
528612
});
529613

530614
$rootScope.$digest();
615+
$$rAF.flush();
531616
expect(captureLog.length).toBe(2);
532617
}));
533618

0 commit comments

Comments
 (0)