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

Commit 2f4437b

Browse files
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, $animate.remoteClass or $animate.setClass. BREAKING CHANGE $animate.addClass, $animate.removeClass and $animate.setClass will no longer start the animation right after being called in the directive code. The animation will only commence once a digest has passed. This means that all animation-related testing code requires an extra digest to kick off the animation. ```js //before this fix $animate.addClass(element, 'super'); expect(element).toHaveClass('super'); //now $animate.addClass(element, 'super'); $rootScope.$digest(); expect(element).toHaveClass('super'); ``` $animate will also tally the amount of times classes are added and removed and only animate the left over classes once the digest kicks in. This means that for any directive code that adds and removes the same CSS class on the same element then this may result in no animation being triggered at all. ```js $animate.addClass(element, 'klass'); $animate.removeClass(element, 'klass'); $rootScope.$digest(); //nothing happens... ```
1 parent d0b4189 commit 2f4437b

File tree

8 files changed

+400
-69
lines changed

8 files changed

+400
-69
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
@@ -742,14 +742,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
742742
*/
743743
$updateClass : function(newClasses, oldClasses) {
744744
var toAdd = tokenDifference(newClasses, oldClasses);
745-
var toRemove = tokenDifference(oldClasses, newClasses);
745+
if (toAdd && toAdd.length) {
746+
$animate.addClass(this.$$element, toAdd);
747+
}
746748

747-
if(toAdd.length === 0) {
749+
var toRemove = tokenDifference(oldClasses, newClasses);
750+
if (toRemove && toRemove.length) {
748751
$animate.removeClass(this.$$element, toRemove);
749-
} else if(toRemove.length === 0) {
750-
$animate.addClass(this.$$element, toAdd);
751-
} else {
752-
$animate.setClass(this.$$element, toAdd, toRemove);
753752
}
754753
},
755754

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

+139-45
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';
@@ -419,10 +420,14 @@ angular.module('ngAnimate', ['ng'])
419420
return classNameFilter.test(className);
420421
};
421422

422-
function blockElementAnimations(element) {
423+
function classBasedAnimationsBlocked(element, setter) {
423424
var data = element.data(NG_ANIMATE_STATE) || {};
424-
data.running = true;
425-
element.data(NG_ANIMATE_STATE, data);
425+
if (setter) {
426+
data.running = true;
427+
data.structural = true;
428+
element.data(NG_ANIMATE_STATE, data);
429+
}
430+
return data.disabled || (data.running && data.structural);
426431
}
427432

428433
function runAnimationPostDigest(fn) {
@@ -435,6 +440,60 @@ angular.module('ngAnimate', ['ng'])
435440
};
436441
}
437442

443+
function resolveElementClasses(element, cache, runningAnimations) {
444+
runningAnimations = runningAnimations || {};
445+
var map = {};
446+
447+
forEach(cache.add, function(className) {
448+
if (className && className.length) {
449+
map[className] = map[className] || 0;
450+
map[className]++;
451+
}
452+
});
453+
454+
forEach(cache.remove, function(className) {
455+
if (className && className.length) {
456+
map[className] = map[className] || 0;
457+
map[className]--;
458+
}
459+
});
460+
461+
var lookup = [];
462+
forEach(runningAnimations, function(data, selector) {
463+
forEach(selector.split(' '), function(s) {
464+
lookup[s]=data;
465+
});
466+
});
467+
468+
var toAdd = [], toRemove = [];
469+
forEach(map, function(status, className) {
470+
var hasClass = element.hasClass(className);
471+
var matchingAnimation = lookup[className] || {};
472+
473+
// When addClass and removeClass is called then $animate will check to
474+
// see if addClass and removeClass cancel each other out. When there are
475+
// more calls to removeClass than addClass then the count falls below 0
476+
// and then the removeClass animation will be allowed. Otherwise if the
477+
// count is above 0 then that means an addClass animation will commence.
478+
// Once an animation is allowed then the code will also check to see if
479+
// there exists any on-going animation that is already adding or remvoing
480+
// the matching CSS class.
481+
if (status < 0) {
482+
//does it have the class or will it have the class
483+
if(hasClass || matchingAnimation.event == 'addClass') {
484+
toRemove.push(className);
485+
}
486+
} else if (status > 0) {
487+
//is the class missing or will it be removed?
488+
if(!hasClass || matchingAnimation.event == 'removeClass') {
489+
toAdd.push(className);
490+
}
491+
}
492+
});
493+
494+
return (toAdd.length + toRemove.length) > 0 && [toAdd.join(' '), toRemove.join(' ')];
495+
}
496+
438497
function lookup(name) {
439498
if (name) {
440499
var matches = [],
@@ -473,18 +532,27 @@ angular.module('ngAnimate', ['ng'])
473532
return;
474533
}
475534

535+
var classNameAdd;
536+
var classNameRemove;
537+
if (isArray(className)) {
538+
classNameAdd = className[0];
539+
classNameRemove = className[1];
540+
if (!classNameAdd) {
541+
className = classNameRemove;
542+
animationEvent = 'removeClass';
543+
} else if(!classNameRemove) {
544+
className = classNameAdd;
545+
animationEvent = 'addClass';
546+
} else {
547+
className = classNameAdd + ' ' + classNameRemove;
548+
}
549+
}
550+
476551
var isSetClassOperation = animationEvent == 'setClass';
477552
var isClassBased = isSetClassOperation ||
478553
animationEvent == 'addClass' ||
479554
animationEvent == 'removeClass';
480555

481-
var classNameAdd, classNameRemove;
482-
if (angular.isArray(className)) {
483-
classNameAdd = className[0];
484-
classNameRemove = className[1];
485-
className = classNameAdd + ' ' + classNameRemove;
486-
}
487-
488556
var currentClassName = element.attr('class');
489557
var classes = currentClassName + ' ' + className;
490558
if (!isAnimatableClassName(classes)) {
@@ -665,7 +733,7 @@ angular.module('ngAnimate', ['ng'])
665733
parentElement = prepareElement(parentElement);
666734
afterElement = prepareElement(afterElement);
667735

668-
blockElementAnimations(element);
736+
classBasedAnimationsBlocked(element, true);
669737
$delegate.enter(element, parentElement, afterElement);
670738
return runAnimationPostDigest(function() {
671739
return performAnimation('enter', 'ng-enter', stripCommentsFromElement(element), parentElement, afterElement, noop, doneCallback);
@@ -707,7 +775,7 @@ angular.module('ngAnimate', ['ng'])
707775
element = angular.element(element);
708776

709777
cancelChildAnimations(element);
710-
blockElementAnimations(element);
778+
classBasedAnimationsBlocked(element, true);
711779
this.enabled(false, element);
712780
return runAnimationPostDigest(function() {
713781
return performAnimation('leave', 'ng-leave', stripCommentsFromElement(element), null, null, function() {
@@ -756,7 +824,7 @@ angular.module('ngAnimate', ['ng'])
756824
afterElement = prepareElement(afterElement);
757825

758826
cancelChildAnimations(element);
759-
blockElementAnimations(element);
827+
classBasedAnimationsBlocked(element, true);
760828
$delegate.move(element, parentElement, afterElement);
761829
return runAnimationPostDigest(function() {
762830
return performAnimation('move', 'ng-move', stripCommentsFromElement(element), parentElement, afterElement, noop, doneCallback);
@@ -794,11 +862,7 @@ angular.module('ngAnimate', ['ng'])
794862
* @return {function} the animation cancellation function
795863
*/
796864
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);
865+
return this.setClass(element, className, [], doneCallback);
802866
},
803867

804868
/**
@@ -832,11 +896,7 @@ angular.module('ngAnimate', ['ng'])
832896
* @return {function} the animation cancellation function
833897
*/
834898
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);
899+
return this.setClass(element, [], className, doneCallback);
840900
},
841901

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

878981
/**
@@ -931,6 +1034,7 @@ angular.module('ngAnimate', ['ng'])
9311034
return noopCancel;
9321035
}
9331036

1037+
animationEvent = runner.event;
9341038
className = runner.className;
9351039
var elementEvents = angular.element._data(runner.node);
9361040
elementEvents = elementEvents && elementEvents.events;
@@ -939,32 +1043,22 @@ angular.module('ngAnimate', ['ng'])
9391043
parentElement = afterElement ? afterElement.parent() : element.parent();
9401044
}
9411045

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-
9561046
//skip the animation if animations are disabled, a parent is already being animated,
9571047
//the element is not currently attached to the document body or then completely close
9581048
//the animation if any matching animations are not found at all.
9591049
//NOTE: IE8 + IE9 should close properly (run closeAnimation()) in case an animation was found.
960-
if (skipAnimations || animationsDisabled(element, parentElement)) {
1050+
if (animationsDisabled(element, parentElement)) {
9611051
fireDOMOperation();
9621052
fireBeforeCallbackAsync();
9631053
fireAfterCallbackAsync();
9641054
closeAnimation();
9651055
return noopCancel;
9661056
}
9671057

1058+
var ngAnimateState = element.data(NG_ANIMATE_STATE) || {};
1059+
var runningAnimations = ngAnimateState.active || {};
1060+
var totalActiveAnimations = ngAnimateState.totalActive || 0;
1061+
var lastAnimation = ngAnimateState.last;
9681062
var skipAnimation = false;
9691063
if (totalActiveAnimations > 0) {
9701064
var animationsToCancel = [];
@@ -1000,9 +1094,6 @@ angular.module('ngAnimate', ['ng'])
10001094
}
10011095
}
10021096

1003-
runningAnimations = ngAnimateState.active || {};
1004-
totalActiveAnimations = ngAnimateState.totalActive || 0;
1005-
10061097
if (runner.isClassBased && !runner.isSetClassOperation && !skipAnimation) {
10071098
skipAnimation = (animationEvent == 'addClass') == element.hasClass(className); //opposite of XOR
10081099
}
@@ -1015,6 +1106,9 @@ angular.module('ngAnimate', ['ng'])
10151106
return noopCancel;
10161107
}
10171108

1109+
runningAnimations = ngAnimateState.active || {};
1110+
totalActiveAnimations = ngAnimateState.totalActive || 0;
1111+
10181112
if (animationEvent == 'leave') {
10191113
//there's no need to ever remove the listener since the element
10201114
//will be removed (destroyed) after the leave animation ends or
@@ -1708,7 +1802,7 @@ angular.module('ngAnimate', ['ng'])
17081802

17091803
function suffixClasses(classes, suffix) {
17101804
var className = '';
1711-
classes = angular.isArray(classes) ? classes : classes.split(/\s+/);
1805+
classes = isArray(classes) ? classes : classes.split(/\s+/);
17121806
forEach(classes, function(klass, i) {
17131807
if (klass && klass.length > 0) {
17141808
className += (i > 0 ? ' ' : '') + klass + suffix;

test/ng/compileSpec.js

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

61966196
data = $animate.queue.shift();
6197-
expect(data.event).toBe('setClass');
6197+
expect(data.event).toBe('addClass');
61986198
expect(data.args[1]).toBe('dice');
6199-
expect(data.args[2]).toBe('rice');
6199+
6200+
data = $animate.queue.shift();
6201+
expect(data.event).toBe('removeClass');
6202+
expect(data.args[1]).toBe('rice');
62006203

62016204
expect(element.hasClass('ice')).toBe(true);
62026205
expect(element.hasClass('dice')).toBe(true);

0 commit comments

Comments
 (0)