Skip to content

Commit 9bb0727

Browse files
matskomatsko
committed
revert: fix(ngAnimate): ensure nested class-based animations are spaced out with a RAF
1 parent 8891893 commit 9bb0727

File tree

8 files changed

+29
-370
lines changed

8 files changed

+29
-370
lines changed

Diff for: ‎angularFiles.js

-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ var angularFiles = {
8888
'angularModules': {
8989
'ngAnimate': [
9090
'src/ngAnimate/shared.js',
91-
'src/ngAnimate/rafScheduler.js',
9291
'src/ngAnimate/animateChildrenDirective.js',
9392
'src/ngAnimate/animateCss.js',
9493
'src/ngAnimate/animateCssDriver.js',

Diff for: ‎src/ngAnimate/animateCss.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -393,9 +393,9 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
393393
var gcsStaggerLookup = createLocalCacheLookup();
394394

395395
this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout',
396-
'$document', '$sniffer', '$$rAFScheduler',
396+
'$document', '$sniffer', '$$rAF',
397397
function($window, $$jqLite, $$AnimateRunner, $timeout,
398-
$document, $sniffer, $$rAFScheduler) {
398+
$document, $sniffer, $$rAF) {
399399

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

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

455455
var bod = getDomNode($document).body;
456+
var cancelLastRAFRequest;
456457
var rafWaitQueue = [];
457458
function waitUntilQuiet(callback) {
459+
if (cancelLastRAFRequest) {
460+
cancelLastRAFRequest(); //cancels the request
461+
}
458462
rafWaitQueue.push(callback);
459-
$$rAFScheduler.waitUntilQuiet(function() {
463+
cancelLastRAFRequest = $$rAF(function() {
464+
cancelLastRAFRequest = null;
460465
gcsLookup.flush();
461466
gcsStaggerLookup.flush();
462467

Diff for: ‎src/ngAnimate/animateQueue.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -370,9 +370,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
370370
return runner;
371371
}
372372

373-
if (isStructural) {
374-
closeParentClassBasedAnimations(parent);
375-
}
373+
closeParentClassBasedAnimations(parent);
376374

377375
// the counter keeps track of cancelled animations
378376
var counter = (existingAnimation.counter || 0) + 1;
@@ -431,9 +429,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
431429
? 'setClass'
432430
: animationDetails.event;
433431

434-
if (animationDetails.structural) {
435-
closeParentClassBasedAnimations(parentElement);
436-
}
432+
closeParentClassBasedAnimations(parentElement);
437433

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

Diff for: ‎src/ngAnimate/animation.js

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

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

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

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

60-
var classBasedIndex;
61-
if (!isStructural) {
62-
classBasedIndex = totalPendingClassBasedAnimations;
63-
totalPendingClassBasedAnimations += 1;
64-
}
65-
6656
animationQueue.push({
6757
// this data is used by the postDigest code and passed into
6858
// the driver step function
6959
element: element,
7060
classes: classes,
7161
event: event,
72-
classBasedIndex: classBasedIndex,
7362
structural: isStructural,
7463
options: options,
7564
beforeStart: beforeStart,
@@ -84,10 +73,6 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
8473
if (animationQueue.length > 1) return runner;
8574

8675
$rootScope.$$postDigest(function() {
87-
totalActiveClassBasedAnimations = totalPendingClassBasedAnimations;
88-
totalPendingClassBasedAnimations = 0;
89-
classBasedAnimationsQueue.length = 0;
90-
9176
var animations = [];
9277
forEach(animationQueue, function(entry) {
9378
// the element was destroyed early on which removed the runner
@@ -102,58 +87,23 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
10287
animationQueue.length = 0;
10388

10489
forEach(groupAnimations(animations), function(animationEntry) {
105-
if (animationEntry.structural) {
106-
triggerAnimationStart();
107-
} else {
108-
classBasedAnimationsQueue.push({
109-
node: getDomNode(animationEntry.element),
110-
fn: triggerAnimationStart
111-
});
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-
}
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();
12694

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-
}
95+
var operation = invokeFirstDriver(animationEntry);
96+
var triggerAnimationStart = operation && operation.start; /// TODO(matsko): only recognize operation.start()
14797

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-
}
98+
var closeFn = animationEntry.close;
99+
if (!triggerAnimationStart) {
100+
closeFn();
101+
} else {
102+
var animationRunner = triggerAnimationStart();
103+
animationRunner.done(function(status) {
104+
closeFn(!status);
105+
});
106+
updateAnimationRunners(animationEntry, animationRunner);
157107
}
158108
});
159109
});
@@ -225,7 +175,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
225175
var lookupKey = from.animationID.toString();
226176
if (!anchorGroups[lookupKey]) {
227177
var group = anchorGroups[lookupKey] = {
228-
structural: true,
178+
// TODO(matsko): double-check this code
229179
beforeStart: function() {
230180
fromAnimation.beforeStart();
231181
toAnimation.beforeStart();

Diff for: ‎src/ngAnimate/module.js

-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
/* global angularAnimateModule: true,
44
55
$$rAFMutexFactory,
6-
$$rAFSchedulerFactory,
76
$$AnimateChildrenDirective,
87
$$AnimateRunnerFactory,
98
$$AnimateQueueProvider,
@@ -743,7 +742,6 @@ angular.module('ngAnimate', [])
743742
.directive('ngAnimateChildren', $$AnimateChildrenDirective)
744743

745744
.factory('$$rAFMutex', $$rAFMutexFactory)
746-
.factory('$$rAFScheduler', $$rAFSchedulerFactory)
747745

748746
.factory('$$AnimateRunner', $$AnimateRunnerFactory)
749747

Diff for: ‎src/ngAnimate/rafScheduler.js

-59
This file was deleted.

Diff for: ‎test/ngAnimate/animationSpec.js

+1-86
Original file line numberDiff line numberDiff line change
@@ -288,90 +288,6 @@ 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-
375291
they('should return a runner that object that contains a $prop() function',
376292
['end', 'cancel', 'then'], function(method) {
377293
inject(function($$animation) {
@@ -597,7 +513,7 @@ describe('$$animation', function() {
597513
}));
598514

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

602518
fromElement.addClass('shared-class');
603519
fromElement.attr('ng-animate-ref', '1');
@@ -612,7 +528,6 @@ describe('$$animation', function() {
612528
});
613529

614530
$rootScope.$digest();
615-
$$rAF.flush();
616531
expect(captureLog.length).toBe(2);
617532
}));
618533

0 commit comments

Comments
 (0)