Skip to content

Commit 46d17bd

Browse files
committed
fix($animate): ensure that class-based animations only consider the most recent DOM operations
Prior to this fix $animate would maintain a count of each time a class was added and removed within $animate. With this fix, $animate instead only cares about the most recent class times that the classes were added and/or removed and it will only remember a total of one addClass or removeClass operation per className. ``` // before addClass => +1 removeClass => 0 addClass => +1 addClass => +2 removeClass => +1 // this will cause an addClass animation // now addClass => +1 removeClass => 0 addClass => +1 addClass => +1 removeClass => 0 // this will cause no animation to run ``` Closes angular#8946
1 parent 613d0a3 commit 46d17bd

File tree

2 files changed

+89
-27
lines changed

2 files changed

+89
-27
lines changed

src/ngAnimate/animate.js

+23-26
Original file line numberDiff line numberDiff line change
@@ -469,31 +469,16 @@ angular.module('ngAnimate', ['ng'])
469469

470470
function resolveElementClasses(element, cache, runningAnimations) {
471471
runningAnimations = runningAnimations || {};
472-
var map = {};
473472

474-
forEach(cache.add, function(className) {
475-
if (className && className.length) {
476-
map[className] = map[className] || 0;
477-
map[className]++;
478-
}
479-
});
480-
481-
forEach(cache.remove, function(className) {
482-
if (className && className.length) {
483-
map[className] = map[className] || 0;
484-
map[className]--;
485-
}
486-
});
487-
488-
var lookup = [];
473+
var lookup = {};
489474
forEach(runningAnimations, function(data, selector) {
490475
forEach(selector.split(' '), function(s) {
491476
lookup[s]=data;
492477
});
493478
});
494479

495480
var toAdd = [], toRemove = [];
496-
forEach(map, function(status, className) {
481+
forEach(cache.classes, function(status, className) {
497482
var hasClass = angular.$$hasClass(element[0], className);
498483
var matchingAnimation = lookup[className] || {};
499484

@@ -505,12 +490,12 @@ angular.module('ngAnimate', ['ng'])
505490
// Once an animation is allowed then the code will also check to see if
506491
// there exists any on-going animation that is already adding or remvoing
507492
// the matching CSS class.
508-
if (status < 0) {
493+
if (status === false) {
509494
//does it have the class or will it have the class
510495
if (hasClass || matchingAnimation.event == 'addClass') {
511496
toRemove.push(className);
512497
}
513-
} else if (status > 0) {
498+
} else if (status === true) {
514499
//is the class missing or will it be removed?
515500
if (!hasClass || matchingAnimation.event == 'removeClass') {
516501
toAdd.push(className);
@@ -997,20 +982,32 @@ angular.module('ngAnimate', ['ng'])
997982
return $delegate.setClass(element, add, remove);
998983
}
999984

985+
// we're using a combined array for both the add and remove
986+
// operations since the ORDER OF addClass and removeClass matters
987+
var classes, cache = element.data(STORAGE_KEY);
988+
var hasCache = !!cache;
989+
if (!cache) {
990+
cache = {};
991+
cache.classes = {};
992+
}
993+
classes = cache.classes;
994+
1000995
add = isArray(add) ? add : add.split(' ');
1001-
remove = isArray(remove) ? remove : remove.split(' ');
996+
forEach(add, function(c) {
997+
c && c.length && (classes[c] = true);
998+
});
1002999

1003-
var cache = element.data(STORAGE_KEY);
1004-
if (cache) {
1005-
cache.add = cache.add.concat(add);
1006-
cache.remove = cache.remove.concat(remove);
1000+
remove = isArray(remove) ? remove : remove.split(' ');
1001+
forEach(remove, function(c) {
1002+
c && c.length && (classes[c] = false);
1003+
});
10071004

1005+
if (hasCache) {
10081006
//the digest cycle will combine all the animations into one function
10091007
return cache.promise;
10101008
} else {
10111009
element.data(STORAGE_KEY, cache = {
1012-
add : add,
1013-
remove : remove
1010+
classes : classes
10141011
});
10151012
}
10161013

test/ngAnimate/animateSpec.js

+66-1
Original file line numberDiff line numberDiff line change
@@ -3408,11 +3408,76 @@ describe("ngAnimate", function() {
34083408
$animate.triggerReflow();
34093409

34103410
expect(log.length).toBe(2);
3411-
expect(log[0]).toEqual({ name : 'addClass', className : 'one five' });
3411+
expect(log[0]).toEqual({ name : 'addClass', className : 'one four five' });
34123412
expect(log[1]).toEqual({ name : 'removeClass', className : 'three' });
34133413
});
34143414
});
34153415

3416+
it('should intelligently cancel out redundant class-based animations', function() {
3417+
var log = [];
3418+
var track = function(name) {
3419+
return function() {
3420+
log.push({ name : name, className : arguments[1] });
3421+
};
3422+
};
3423+
module(function($animateProvider) {
3424+
$animateProvider.register('.animate', function() {
3425+
return {
3426+
addClass : track('addClass'),
3427+
removeClass : track('removeClass')
3428+
};
3429+
});
3430+
});
3431+
inject(function($rootScope, $animate, $compile, $rootElement, $document) {
3432+
$animate.enabled(true);
3433+
3434+
var element = $compile('<div class="animate three four"></div>')($rootScope);
3435+
$rootElement.append(element);
3436+
angular.element($document[0].body).append($rootElement);
3437+
3438+
$animate.removeClass(element, 'one');
3439+
$rootScope.$digest();
3440+
$animate.triggerReflow();
3441+
expect(log.length).toBe(0);
3442+
$animate.triggerCallbacks();
3443+
3444+
$animate.addClass(element, 'two');
3445+
$animate.addClass(element, 'two');
3446+
$animate.removeClass(element, 'two');
3447+
$rootScope.$digest();
3448+
$animate.triggerReflow();
3449+
expect(log.length).toBe(0);
3450+
$animate.triggerCallbacks();
3451+
3452+
$animate.removeClass(element, 'three');
3453+
$animate.addClass(element, 'three');
3454+
$rootScope.$digest();
3455+
$animate.triggerReflow();
3456+
expect(log.length).toBe(0);
3457+
$animate.triggerCallbacks();
3458+
3459+
$animate.removeClass(element, 'four');
3460+
$animate.addClass(element, 'four');
3461+
$animate.removeClass(element, 'four');
3462+
$rootScope.$digest();
3463+
$animate.triggerReflow();
3464+
expect(log.length).toBe(1);
3465+
$animate.triggerCallbacks();
3466+
expect(log[0]).toEqual({ name : 'removeClass', className : 'four' });
3467+
3468+
$animate.addClass(element, 'five');
3469+
$animate.addClass(element, 'five');
3470+
$animate.addClass(element, 'five');
3471+
$animate.removeClass(element, 'five');
3472+
$animate.addClass(element, 'five');
3473+
$rootScope.$digest();
3474+
$animate.triggerReflow();
3475+
expect(log.length).toBe(2);
3476+
$animate.triggerCallbacks();
3477+
expect(log[1]).toEqual({ name : 'addClass', className : 'five' });
3478+
});
3479+
});
3480+
34163481
it('should skip class-based animations if the element is removed before the digest occurs', function() {
34173482
var spy = jasmine.createSpy();
34183483
module(function($animateProvider) {

0 commit comments

Comments
 (0)