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

fix(ngAnimate): ensure nested class-based animations are spaced out with a RAF #11907

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions angularFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ var angularFiles = {
'angularModules': {
'ngAnimate': [
'src/ngAnimate/shared.js',
'src/ngAnimate/rafScheduler.js',
'src/ngAnimate/animateChildrenDirective.js',
'src/ngAnimate/animateCss.js',
'src/ngAnimate/animateCssDriver.js',
Expand Down
11 changes: 3 additions & 8 deletions src/ngAnimate/animateCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,9 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
var gcsStaggerLookup = createLocalCacheLookup();

this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout',
'$document', '$sniffer', '$$rAF',
'$document', '$sniffer', '$$rAFScheduler',
function($window, $$jqLite, $$AnimateRunner, $timeout,
$document, $sniffer, $$rAF) {
$document, $sniffer, $$rAFScheduler) {

var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);

Expand Down Expand Up @@ -452,15 +452,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
}

var bod = getDomNode($document).body;
var cancelLastRAFRequest;
var rafWaitQueue = [];
function waitUntilQuiet(callback) {
if (cancelLastRAFRequest) {
cancelLastRAFRequest(); //cancels the request
}
rafWaitQueue.push(callback);
cancelLastRAFRequest = $$rAF(function() {
cancelLastRAFRequest = null;
$$rAFScheduler.waitUntilQuiet(function() {
gcsLookup.flush();
gcsStaggerLookup.flush();

Expand Down
8 changes: 6 additions & 2 deletions src/ngAnimate/animateQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
return runner;
}

closeParentClassBasedAnimations(parent);
if (isStructural) {
closeParentClassBasedAnimations(parent);
}

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

closeParentClassBasedAnimations(parentElement);
if (animationDetails.structural) {
closeParentClassBasedAnimations(parentElement);
}

markElementAnimationState(element, RUNNING_STATE);
var realRunner = $$animation(element, event, animationDetails.options);
Expand Down
86 changes: 68 additions & 18 deletions src/ngAnimate/animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,16 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
return element.data(RUNNER_STORAGE_KEY);
}

this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner',
function($$jqLite, $rootScope, $injector, $$AnimateRunner) {
this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$rAFScheduler',
function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$rAFScheduler) {

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

var totalPendingClassBasedAnimations = 0;
var totalActiveClassBasedAnimations = 0;
var classBasedAnimationsQueue = [];

// TODO(matsko): document the signature in a better way
return function(element, event, options) {
options = prepareAnimationOptions(options);
Expand Down Expand Up @@ -53,12 +57,19 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
options.tempClasses = null;
}

var classBasedIndex;
if (!isStructural) {
classBasedIndex = totalPendingClassBasedAnimations;
totalPendingClassBasedAnimations += 1;
}

animationQueue.push({
// this data is used by the postDigest code and passed into
// the driver step function
element: element,
classes: classes,
event: event,
classBasedIndex: classBasedIndex,
structural: isStructural,
options: options,
beforeStart: beforeStart,
Expand All @@ -73,6 +84,10 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
if (animationQueue.length > 1) return runner;

$rootScope.$$postDigest(function() {
totalActiveClassBasedAnimations = totalPendingClassBasedAnimations;
totalPendingClassBasedAnimations = 0;
classBasedAnimationsQueue.length = 0;

var animations = [];
forEach(animationQueue, function(entry) {
// the element was destroyed early on which removed the runner
Expand All @@ -87,23 +102,58 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
animationQueue.length = 0;

forEach(groupAnimations(animations), function(animationEntry) {
// it's important that we apply the `ng-animate` CSS class and the
// temporary classes before we do any driver invoking since these
// CSS classes may be required for proper CSS detection.
animationEntry.beforeStart();

var operation = invokeFirstDriver(animationEntry);
var triggerAnimationStart = operation && operation.start; /// TODO(matsko): only recognize operation.start()

var closeFn = animationEntry.close;
if (!triggerAnimationStart) {
closeFn();
if (animationEntry.structural) {
triggerAnimationStart();
} else {
var animationRunner = triggerAnimationStart();
animationRunner.done(function(status) {
closeFn(!status);
classBasedAnimationsQueue.push({
node: getDomNode(animationEntry.element),
fn: triggerAnimationStart
});
updateAnimationRunners(animationEntry, animationRunner);

if (animationEntry.classBasedIndex === totalActiveClassBasedAnimations - 1) {
// we need to sort each of the animations in order of parent to child
// relationships. This ensures that the child classes are applied at the
// right time.
classBasedAnimationsQueue = classBasedAnimationsQueue.sort(function(a,b) {
return b.node.contains(a.node);
}).map(function(entry) {
return entry.fn;
});

$$rAFScheduler(classBasedAnimationsQueue);
}
}

function triggerAnimationStart() {
// it's important that we apply the `ng-animate` CSS class and the
// temporary classes before we do any driver invoking since these
// CSS classes may be required for proper CSS detection.
animationEntry.beforeStart();

var startAnimationFn, closeFn = animationEntry.close;

// in the event that the element was removed before the digest runs or
// during the RAF sequencing then we should not trigger the animation.
var targetElement = animationEntry.anchors
? (animationEntry.from.element || animationEntry.to.element)
: animationEntry.element;

if (getRunner(targetElement)) {
var operation = invokeFirstDriver(animationEntry);
if (operation) {
startAnimationFn = operation.start;
}
}

if (!startAnimationFn) {
closeFn();
} else {
var animationRunner = startAnimationFn();
animationRunner.done(function(status) {
closeFn(!status);
});
updateAnimationRunners(animationEntry, animationRunner);
}
}
});
});
Expand Down Expand Up @@ -175,7 +225,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
var lookupKey = from.animationID.toString();
if (!anchorGroups[lookupKey]) {
var group = anchorGroups[lookupKey] = {
// TODO(matsko): double-check this code
structural: true,
beforeStart: function() {
fromAnimation.beforeStart();
toAnimation.beforeStart();
Expand Down
2 changes: 2 additions & 0 deletions src/ngAnimate/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/* global angularAnimateModule: true,

$$rAFMutexFactory,
$$rAFSchedulerFactory,
$$AnimateChildrenDirective,
$$AnimateRunnerFactory,
$$AnimateQueueProvider,
Expand Down Expand Up @@ -742,6 +743,7 @@ angular.module('ngAnimate', [])
.directive('ngAnimateChildren', $$AnimateChildrenDirective)

.factory('$$rAFMutex', $$rAFMutexFactory)
.factory('$$rAFScheduler', $$rAFSchedulerFactory)

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

Expand Down
51 changes: 51 additions & 0 deletions src/ngAnimate/rafScheduler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
'use strict';

var $$rAFSchedulerFactory = ['$$rAF', function($$rAF) {
var tickQueue = [];
var cancelFn;

function scheduler(tasks) {
// we make a copy since RAFScheduler mutates the state
// of the passed in array variable and this would be difficult
// to track down on the outside code
tickQueue.push([].concat(tasks));
nextTick();
}

scheduler.waitUntilQuiet = function(fn) {
if (cancelFn) cancelFn();

cancelFn = $$rAF(function() {
cancelFn = null;
fn();
nextTick();
});
};

return scheduler;

function nextTick() {
if (!tickQueue.length) return;

var updatedQueue = [];
for (var i = 0; i < tickQueue.length; i++) {
var innerQueue = tickQueue[i];
runNextTask(innerQueue);
if (innerQueue.length) {
updatedQueue.push(innerQueue);
}
}
tickQueue = updatedQueue;

if (!cancelFn) {
$$rAF(function() {
if (!cancelFn) nextTick();
});
}
}

function runNextTask(tasks) {
var nextTask = tasks.shift();
nextTask();
}
}];
87 changes: 86 additions & 1 deletion test/ngAnimate/animationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,90 @@ describe('$$animation', function() {
};
}));

it('should space out multiple ancestorial class-based animations with a RAF in between',
inject(function($rootScope, $$animation, $$rAF) {

var parent = element;
element = jqLite('<div></div>');
parent.append(element);

var child = jqLite('<div></div>');
element.append(child);

$$animation(parent, 'addClass', { addClass: 'blue' });
$$animation(element, 'addClass', { addClass: 'red' });
$$animation(child, 'addClass', { addClass: 'green' });

$rootScope.$digest();

expect(captureLog.length).toBe(1);
expect(capturedAnimation.options.addClass).toBe('blue');

$$rAF.flush();
expect(captureLog.length).toBe(2);
expect(capturedAnimation.options.addClass).toBe('red');

$$rAF.flush();
expect(captureLog.length).toBe(3);
expect(capturedAnimation.options.addClass).toBe('green');
}));

it('should properly cancel out pending animations that are spaced with a RAF request before the digest completes',
inject(function($rootScope, $$animation, $$rAF) {

var parent = element;
element = jqLite('<div></div>');
parent.append(element);

var child = jqLite('<div></div>');
element.append(child);

var r1 = $$animation(parent, 'addClass', { addClass: 'blue' });
var r2 = $$animation(element, 'addClass', { addClass: 'red' });
var r3 = $$animation(child, 'addClass', { addClass: 'green' });

r2.end();

$rootScope.$digest();

expect(captureLog.length).toBe(1);
expect(capturedAnimation.options.addClass).toBe('blue');

$$rAF.flush();

expect(captureLog.length).toBe(2);
expect(capturedAnimation.options.addClass).toBe('green');
}));

it('should properly cancel out pending animations that are spaced with a RAF request after the digest completes',
inject(function($rootScope, $$animation, $$rAF) {

var parent = element;
element = jqLite('<div></div>');
parent.append(element);

var child = jqLite('<div></div>');
element.append(child);

var r1 = $$animation(parent, 'addClass', { addClass: 'blue' });
var r2 = $$animation(element, 'addClass', { addClass: 'red' });
var r3 = $$animation(child, 'addClass', { addClass: 'green' });

$rootScope.$digest();

r2.end();

expect(captureLog.length).toBe(1);
expect(capturedAnimation.options.addClass).toBe('blue');

$$rAF.flush();
expect(captureLog.length).toBe(1);

$$rAF.flush();
expect(captureLog.length).toBe(2);
expect(capturedAnimation.options.addClass).toBe('green');
}));

they('should return a runner that object that contains a $prop() function',
['end', 'cancel', 'then'], function(method) {
inject(function($$animation) {
Expand Down Expand Up @@ -513,7 +597,7 @@ describe('$$animation', function() {
}));

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

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

$rootScope.$digest();
$$rAF.flush();
expect(captureLog.length).toBe(2);
}));

Expand Down
Loading