Skip to content

Commit 8724010

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. ```pre // 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 8724010

File tree

2 files changed

+84
-16
lines changed

2 files changed

+84
-16
lines changed

src/ngAnimate/animate.js

+19-16
Original file line numberDiff line numberDiff line change
@@ -471,18 +471,13 @@ angular.module('ngAnimate', ['ng'])
471471
runningAnimations = runningAnimations || {};
472472
var map = {};
473473

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-
}
474+
forEach(cache.classes, function(val) {
475+
var polarity = val.charAt(0);
476+
var className = val.substr(1);
477+
var count = map[className] = map[className] || 0;
478+
map[className] = polarity == '+'
479+
? Math.min(count + 1, 1)
480+
: Math.max(count - 1, -1);
486481
});
487482

488483
var lookup = [];
@@ -997,20 +992,28 @@ angular.module('ngAnimate', ['ng'])
997992
return $delegate.setClass(element, add, remove);
998993
}
999994

995+
// we're using a combined array for both the add and remove
996+
// operations since the ORDER OF addClass and removeClass matters
997+
var classes = [];
1000998
add = isArray(add) ? add : add.split(' ');
999+
forEach(add, function(c) {
1000+
c && c.length && classes.push('+' + c);
1001+
});
1002+
10011003
remove = isArray(remove) ? remove : remove.split(' ');
1004+
forEach(remove, function(c) {
1005+
c && c.length && classes.push('-' + c);
1006+
});
10021007

10031008
var cache = element.data(STORAGE_KEY);
10041009
if (cache) {
1005-
cache.add = cache.add.concat(add);
1006-
cache.remove = cache.remove.concat(remove);
1010+
cache.classes = cache.classes.concat(classes);
10071011

10081012
//the digest cycle will combine all the animations into one function
10091013
return cache.promise;
10101014
} else {
10111015
element.data(STORAGE_KEY, cache = {
1012-
add : add,
1013-
remove : remove
1016+
classes : classes
10141017
});
10151018
}
10161019

test/ngAnimate/animateSpec.js

+65
Original file line numberDiff line numberDiff line change
@@ -3413,6 +3413,71 @@ describe("ngAnimate", function() {
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)