Skip to content

Commit e1972e1

Browse files
matskomatsko
committed
feat($animate): coalesce concurrent class-based animations within a digest loop
All class-based animation methods (addClass, removeClass and setClass) on $animate are now processed after the next digest occurs. This fix prevents any sequencing errors from occuring from excessive calls to $animate.addClass or $animate.remoteClass. BREAKING CHANGE All directive code that expects any addClass, removeClass or setClass animations to kick off immediately after being called must now be aware that the animation will only take place once the next digest has kicked off. Also note that successive calls to $animate.addClass, $animate.removeClass or $animate.setClass will be grouped together and will not cancel the former class-based animation (once the digest has passed).
1 parent 1a05daf commit e1972e1

File tree

8 files changed

+395
-68
lines changed

8 files changed

+395
-68
lines changed

‎src/ng/animate.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,8 @@ var $AnimateProvider = ['$provide', function($provide) {
234234
* CSS classes have been set on the element
235235
*/
236236
setClass : function(element, add, remove, done) {
237-
forEach(element, function (element) {
238-
jqLiteAddClass(element, add);
239-
jqLiteRemoveClass(element, remove);
240-
});
237+
this.addClass(element, add);
238+
this.removeClass(element, remove);
241239
async(done);
242240
return noop;
243241
},

‎src/ng/compile.js

+5-6
Original file line numberDiff line numberDiff line change
@@ -720,14 +720,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
720720
*/
721721
$updateClass : function(newClasses, oldClasses) {
722722
var toAdd = tokenDifference(newClasses, oldClasses);
723-
var toRemove = tokenDifference(oldClasses, newClasses);
723+
if (toAdd && toAdd.length) {
724+
$animate.addClass(this.$$element, toAdd);
725+
}
724726

725-
if(toAdd.length === 0) {
727+
var toRemove = tokenDifference(oldClasses, newClasses);
728+
if (toRemove && toRemove.length) {
726729
$animate.removeClass(this.$$element, toRemove);
727-
} else if(toRemove.length === 0) {
728-
$animate.addClass(this.$$element, toAdd);
729-
} else {
730-
$animate.setClass(this.$$element, toAdd, toRemove);
731730
}
732731
},
733732

‎src/ng/directive/ngClass.js

+5-7
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,13 @@ function classDirective(name, selector) {
5656
function updateClasses (oldClasses, newClasses) {
5757
var toAdd = arrayDifference(newClasses, oldClasses);
5858
var toRemove = arrayDifference(oldClasses, newClasses);
59-
toRemove = digestClassCounts(toRemove, -1);
6059
toAdd = digestClassCounts(toAdd, 1);
61-
62-
if (toAdd.length === 0) {
63-
$animate.removeClass(element, toRemove);
64-
} else if (toRemove.length === 0) {
60+
toRemove = digestClassCounts(toRemove, -1);
61+
if(toAdd && toAdd.length) {
6562
$animate.addClass(element, toAdd);
66-
} else {
67-
$animate.setClass(element, toAdd, toRemove);
63+
}
64+
if(toRemove && toRemove.length) {
65+
$animate.removeClass(element, toRemove);
6866
}
6967
}
7068

‎src/ngAnimate/animate.js

+134-44
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ angular.module('ngAnimate', ['ng'])
366366
var noop = angular.noop;
367367
var forEach = angular.forEach;
368368
var selectors = $animateProvider.$$selectors;
369+
var isArray = angular.isArray;
369370

370371
var ELEMENT_NODE = 1;
371372
var NG_ANIMATE_STATE = '$$ngAnimateState';
@@ -394,6 +395,10 @@ angular.module('ngAnimate', ['ng'])
394395
return extractElementNode(elm1) == extractElementNode(elm2);
395396
}
396397

398+
function isEmpty(val) {
399+
return !val && val.length === 0;
400+
}
401+
397402
$provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$$asyncCallback', '$rootScope', '$document',
398403
function($delegate, $injector, $sniffer, $rootElement, $$asyncCallback, $rootScope, $document) {
399404

@@ -419,10 +424,14 @@ angular.module('ngAnimate', ['ng'])
419424
return classNameFilter.test(className);
420425
};
421426

422-
function blockElementAnimations(element) {
427+
function classBasedAnimationsBlocked(element, setter) {
423428
var data = element.data(NG_ANIMATE_STATE) || {};
424-
data.running = true;
425-
element.data(NG_ANIMATE_STATE, data);
429+
if (setter) {
430+
data.running = true;
431+
data.structural = true;
432+
element.data(NG_ANIMATE_STATE, data);
433+
}
434+
return data.disabled || (data.running && data.structural);
426435
}
427436

428437
function runAnimationPostDigest(fn) {
@@ -435,6 +444,51 @@ angular.module('ngAnimate', ['ng'])
435444
};
436445
}
437446

447+
function resolveElementClasses(element, cache, runningAnimations) {
448+
runningAnimations = runningAnimations || {};
449+
var map = {};
450+
451+
forEach(cache.add, function(className) {
452+
if(className && className.length) {
453+
map[className] = map[className] || 0;
454+
map[className]++;
455+
}
456+
});
457+
458+
forEach(cache.remove, function(className) {
459+
if(className && className.length) {
460+
map[className] = map[className] || 0;
461+
map[className]--;
462+
}
463+
});
464+
465+
var lookup = [];
466+
forEach(runningAnimations, function(data, selector) {
467+
forEach(selector.split(' '), function(s) {
468+
lookup[s]=data;
469+
});
470+
});
471+
472+
var toAdd = [], toRemove = [];
473+
forEach(map, function(status, className) {
474+
var hasClass = element.hasClass(className);
475+
var matchingAnimation = lookup[className] || {};
476+
if (status < 0) {
477+
//does it have the class or will it have the class
478+
if(hasClass || matchingAnimation.event == 'addClass') {
479+
toRemove.push(className);
480+
}
481+
} else if (status > 0) {
482+
//is the class missing or will it be removed?
483+
if(!hasClass || matchingAnimation.event == 'removeClass') {
484+
toAdd.push(className);
485+
}
486+
}
487+
});
488+
489+
return (toAdd.length + toRemove.length) > 0 && [toAdd.join(' '), toRemove.join(' ')];
490+
}
491+
438492
function lookup(name) {
439493
if (name) {
440494
var matches = [],
@@ -473,17 +527,26 @@ angular.module('ngAnimate', ['ng'])
473527
return;
474528
}
475529

530+
var classNameAdd, classNameRemove;
531+
if(isArray(className)) {
532+
classNameAdd = className[0];
533+
classNameRemove = className[1];
534+
if (isEmpty(classNameAdd)) {
535+
className = classNameRemove;
536+
animationEvent = 'removeClass';
537+
} else if(isEmpty(classNameRemove)) {
538+
className = classNameAdd;
539+
animationEvent = 'addClass';
540+
} else {
541+
className = classNameAdd + ' ' + classNameRemove;
542+
}
543+
}
544+
476545
var isSetClassOperation = animationEvent == 'setClass';
477546
var isClassBased = isSetClassOperation ||
478547
animationEvent == 'addClass' ||
479548
animationEvent == 'removeClass';
480549

481-
var classNameAdd, classNameRemove;
482-
if(angular.isArray(className)) {
483-
classNameAdd = className[0];
484-
classNameRemove = className[1];
485-
className = classNameAdd + ' ' + classNameRemove;
486-
}
487550

488551
var currentClassName = element.attr('class');
489552
var classes = currentClassName + ' ' + className;
@@ -665,7 +728,7 @@ angular.module('ngAnimate', ['ng'])
665728
parentElement = prepareElement(parentElement);
666729
afterElement = prepareElement(afterElement);
667730

668-
blockElementAnimations(element);
731+
classBasedAnimationsBlocked(element, true);
669732
$delegate.enter(element, parentElement, afterElement);
670733
return runAnimationPostDigest(function() {
671734
return performAnimation('enter', 'ng-enter', stripCommentsFromElement(element), parentElement, afterElement, noop, doneCallback);
@@ -707,7 +770,7 @@ angular.module('ngAnimate', ['ng'])
707770
element = angular.element(element);
708771

709772
cancelChildAnimations(element);
710-
blockElementAnimations(element);
773+
classBasedAnimationsBlocked(element, true);
711774
this.enabled(false, element);
712775
return runAnimationPostDigest(function() {
713776
return performAnimation('leave', 'ng-leave', stripCommentsFromElement(element), null, null, function() {
@@ -756,7 +819,7 @@ angular.module('ngAnimate', ['ng'])
756819
afterElement = prepareElement(afterElement);
757820

758821
cancelChildAnimations(element);
759-
blockElementAnimations(element);
822+
classBasedAnimationsBlocked(element, true);
760823
$delegate.move(element, parentElement, afterElement);
761824
return runAnimationPostDigest(function() {
762825
return performAnimation('move', 'ng-move', stripCommentsFromElement(element), parentElement, afterElement, noop, doneCallback);
@@ -794,11 +857,7 @@ angular.module('ngAnimate', ['ng'])
794857
* @return {function} the animation cancellation function
795858
*/
796859
addClass : function(element, className, doneCallback) {
797-
element = angular.element(element);
798-
element = stripCommentsFromElement(element);
799-
return performAnimation('addClass', className, element, null, null, function() {
800-
$delegate.addClass(element, className);
801-
}, doneCallback);
860+
return this.setClass(element, className, [], doneCallback);
802861
},
803862

804863
/**
@@ -832,11 +891,7 @@ angular.module('ngAnimate', ['ng'])
832891
* @return {function} the animation cancellation function
833892
*/
834893
removeClass : function(element, className, doneCallback) {
835-
element = angular.element(element);
836-
element = stripCommentsFromElement(element);
837-
return performAnimation('removeClass', className, element, null, null, function() {
838-
$delegate.removeClass(element, className);
839-
}, doneCallback);
894+
return this.setClass(element, [], className, doneCallback);
840895
},
841896

842897
/**
@@ -868,11 +923,54 @@ angular.module('ngAnimate', ['ng'])
868923
* @return {function} the animation cancellation function
869924
*/
870925
setClass : function(element, add, remove, doneCallback) {
926+
var STORAGE_KEY = '$$animateClasses';
871927
element = angular.element(element);
872928
element = stripCommentsFromElement(element);
873-
return performAnimation('setClass', [add, remove], element, null, null, function() {
874-
$delegate.setClass(element, add, remove);
875-
}, doneCallback);
929+
930+
if(classBasedAnimationsBlocked(element)) {
931+
return $delegate.setClass(element, add, remove, doneCallback);
932+
}
933+
934+
add = isArray(add) ? add : add.split(' ');
935+
remove = isArray(remove) ? remove : remove.split(' ');
936+
doneCallback = doneCallback || noop;
937+
938+
var cache = element.data(STORAGE_KEY);
939+
if (cache) {
940+
cache.callbacks.push(doneCallback);
941+
cache.add = cache.add.concat(add);
942+
cache.remove = cache.remove.concat(remove);
943+
944+
//the digest cycle will combine all the animations into one function
945+
return;
946+
} else {
947+
element.data(STORAGE_KEY, cache = {
948+
callbacks : [doneCallback],
949+
add : add,
950+
remove : remove
951+
});
952+
}
953+
954+
return runAnimationPostDigest(function() {
955+
var cache = element.data(STORAGE_KEY);
956+
var callbacks = cache.callbacks;
957+
958+
element.removeData(STORAGE_KEY);
959+
960+
var state = element.data(NG_ANIMATE_STATE) || {};
961+
var classes = resolveElementClasses(element, cache, state.active);
962+
return !classes
963+
? $$asyncCallback(onComplete)
964+
: performAnimation('setClass', classes, element, null, null, function() {
965+
$delegate.setClass(element, classes[0], classes[1]);
966+
}, onComplete);
967+
968+
function onComplete() {
969+
forEach(callbacks, function(fn) {
970+
fn();
971+
});
972+
}
973+
});
876974
},
877975

878976
/**
@@ -931,6 +1029,7 @@ angular.module('ngAnimate', ['ng'])
9311029
return noopCancel;
9321030
}
9331031

1032+
animationEvent = runner.event;
9341033
className = runner.className;
9351034
var elementEvents = angular.element._data(runner.node);
9361035
elementEvents = elementEvents && elementEvents.events;
@@ -939,33 +1038,24 @@ angular.module('ngAnimate', ['ng'])
9391038
parentElement = afterElement ? afterElement.parent() : element.parent();
9401039
}
9411040

942-
var ngAnimateState = element.data(NG_ANIMATE_STATE) || {};
943-
var runningAnimations = ngAnimateState.active || {};
944-
var totalActiveAnimations = ngAnimateState.totalActive || 0;
945-
var lastAnimation = ngAnimateState.last;
946-
947-
//only allow animations if the currently running animation is not structural
948-
//or if there is no animation running at all
949-
var skipAnimations;
950-
if (runner.isClassBased) {
951-
skipAnimations = ngAnimateState.running ||
952-
ngAnimateState.disabled ||
953-
(lastAnimation && !lastAnimation.isClassBased);
954-
}
955-
9561041
//skip the animation if animations are disabled, a parent is already being animated,
9571042
//the element is not currently attached to the document body or then completely close
9581043
//the animation if any matching animations are not found at all.
9591044
//NOTE: IE8 + IE9 should close properly (run closeAnimation()) in case an animation was found.
960-
if (skipAnimations || animationsDisabled(element, parentElement)) {
1045+
if (animationsDisabled(element, parentElement)) {
9611046
fireDOMOperation();
9621047
fireBeforeCallbackAsync();
9631048
fireAfterCallbackAsync();
9641049
closeAnimation();
9651050
return noopCancel;
9661051
}
9671052

1053+
var ngAnimateState = element.data(NG_ANIMATE_STATE) || {};
1054+
var runningAnimations = ngAnimateState.active || {};
1055+
var totalActiveAnimations = ngAnimateState.totalActive || 0;
1056+
var lastAnimation = ngAnimateState.last;
9681057
var skipAnimation = false;
1058+
9691059
if(totalActiveAnimations > 0) {
9701060
var animationsToCancel = [];
9711061
if(!runner.isClassBased) {
@@ -1000,9 +1090,6 @@ angular.module('ngAnimate', ['ng'])
10001090
}
10011091
}
10021092

1003-
runningAnimations = ngAnimateState.active || {};
1004-
totalActiveAnimations = ngAnimateState.totalActive || 0;
1005-
10061093
if(runner.isClassBased && !runner.isSetClassOperation && !skipAnimation) {
10071094
skipAnimation = (animationEvent == 'addClass') == element.hasClass(className); //opposite of XOR
10081095
}
@@ -1015,6 +1102,9 @@ angular.module('ngAnimate', ['ng'])
10151102
return noopCancel;
10161103
}
10171104

1105+
runningAnimations = ngAnimateState.active || {};
1106+
totalActiveAnimations = ngAnimateState.totalActive || 0;
1107+
10181108
if(animationEvent == 'leave') {
10191109
//there's no need to ever remove the listener since the element
10201110
//will be removed (destroyed) after the leave animation ends or
@@ -1708,7 +1798,7 @@ angular.module('ngAnimate', ['ng'])
17081798

17091799
function suffixClasses(classes, suffix) {
17101800
var className = '';
1711-
classes = angular.isArray(classes) ? classes : classes.split(/\s+/);
1801+
classes = isArray(classes) ? classes : classes.split(/\s+/);
17121802
forEach(classes, function(klass, i) {
17131803
if(klass && klass.length > 0) {
17141804
className += (i > 0 ? ' ' : '') + klass + suffix;

‎test/ng/compileSpec.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -6019,9 +6019,12 @@ describe('$compile', function() {
60196019
$rootScope.$digest();
60206020

60216021
data = $animate.queue.shift();
6022-
expect(data.event).toBe('setClass');
6022+
expect(data.event).toBe('addClass');
60236023
expect(data.args[1]).toBe('dice');
6024-
expect(data.args[2]).toBe('rice');
6024+
6025+
data = $animate.queue.shift();
6026+
expect(data.event).toBe('removeClass');
6027+
expect(data.args[1]).toBe('rice');
60256028

60266029
expect(element.hasClass('ice')).toBe(true);
60276030
expect(element.hasClass('dice')).toBe(true);

0 commit comments

Comments
 (0)