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

Commit 171e397

Browse files
matskomatsko
committed
revert: fix(ngAnimate): ensure nested class-based animations are spaced out with a RAF
1 parent 2d66c44 commit 171e397

File tree

8 files changed

+31
-371
lines changed

8 files changed

+31
-371
lines changed

‎angularFiles.js‎

Lines changed: 0 additions & 1 deletion
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',

‎src/ngAnimate/animateCss.js‎

Lines changed: 8 additions & 3 deletions
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

‎src/ngAnimate/animateQueue.js‎

Lines changed: 2 additions & 6 deletions
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);

‎src/ngAnimate/animation.js‎

Lines changed: 20 additions & 69 deletions
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,16 +73,13 @@ 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
9479
// form its storage. This means we can't animate this element
9580
// at all and it already has been closed due to destruction.
96-
if (getRunner(entry.element)) {
81+
var elm = entry.element;
82+
if (getRunner(elm) && getDomNode(elm).parentNode) {
9783
animations.push(entry);
9884
}
9985
});
@@ -102,58 +88,23 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
10288
animationQueue.length = 0;
10389

10490
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-
}
91+
// it's important that we apply the `ng-animate` CSS class and the
92+
// temporary classes before we do any driver invoking since these
93+
// CSS classes may be required for proper CSS detection.
94+
animationEntry.beforeStart();
12695

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) && getDomNode(targetElement).parentNode) {
142-
var operation = invokeFirstDriver(animationEntry);
143-
if (operation) {
144-
startAnimationFn = operation.start;
145-
}
146-
}
96+
var operation = invokeFirstDriver(animationEntry);
97+
var triggerAnimationStart = operation && operation.start; /// TODO(matsko): only recognize operation.start()
14798

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-
}
99+
var closeFn = animationEntry.close;
100+
if (!triggerAnimationStart) {
101+
closeFn();
102+
} else {
103+
var animationRunner = triggerAnimationStart();
104+
animationRunner.done(function(status) {
105+
closeFn(!status);
106+
});
107+
updateAnimationRunners(animationEntry, animationRunner);
157108
}
158109
});
159110
});
@@ -225,7 +176,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
225176
var lookupKey = from.animationID.toString();
226177
if (!anchorGroups[lookupKey]) {
227178
var group = anchorGroups[lookupKey] = {
228-
structural: true,
179+
// TODO(matsko): double-check this code
229180
beforeStart: function() {
230181
fromAnimation.beforeStart();
231182
toAnimation.beforeStart();

‎src/ngAnimate/module.js‎

Lines changed: 0 additions & 2 deletions
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,
@@ -744,7 +743,6 @@ angular.module('ngAnimate', [])
744743
.directive('ngAnimateChildren', $$AnimateChildrenDirective)
745744

746745
.factory('$$rAFMutex', $$rAFMutexFactory)
747-
.factory('$$rAFScheduler', $$rAFSchedulerFactory)
748746

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

‎src/ngAnimate/rafScheduler.js‎

Lines changed: 0 additions & 59 deletions
This file was deleted.

‎test/ngAnimate/animationSpec.js‎

Lines changed: 1 addition & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -296,90 +296,6 @@ describe('$$animation', function() {
296296
};
297297
}));
298298

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

607523
it("should not group animations into an anchored animation if enter/leave events are NOT used",
608-
inject(function($$animation, $rootScope, $$rAF) {
524+
inject(function($$animation, $rootScope) {
609525

610526
fromElement.addClass('shared-class');
611527
fromElement.attr('ng-animate-ref', '1');
@@ -620,7 +536,6 @@ describe('$$animation', function() {
620536
});
621537

622538
$rootScope.$digest();
623-
$$rAF.flush();
624539
expect(captureLog.length).toBe(2);
625540
}));
626541

0 commit comments

Comments
 (0)