Skip to content

Commit 7fddba3

Browse files
matskojamesdaily
authored andcommitted
fix($animate): skip unnecessary addClass/removeClass animations
Skip addClass animations if the element already contains the class that is being added to element. Also skip removeClass animations if the element does not contain the class that is being removed. Closes angular#4401 Closes angular#2332
1 parent 772fb61 commit 7fddba3

File tree

2 files changed

+51
-1
lines changed

2 files changed

+51
-1
lines changed

src/ngAnimate/animate.js

+10
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,16 @@ angular.module('ngAnimate', ['ng'])
541541
(ngAnimateState.done || noop)();
542542
}
543543

544+
//There is no point in perform a class-based animation if the element already contains
545+
//(on addClass) or doesn't contain (on removeClass) the className being animated.
546+
//The reason why this is being called after the previous animations are cancelled
547+
//is so that the CSS classes present on the element can be properly examined.
548+
if((event == 'addClass' && element.hasClass(className)) ||
549+
(event == 'removeClass' && !element.hasClass(className))) {
550+
onComplete && onComplete();
551+
return;
552+
}
553+
544554
element.data(NG_ANIMATE_STATE, {
545555
running:true,
546556
structural:!isClassBased,

test/ngAnimate/animateSpec.js

+41-1
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,7 @@ describe("ngAnimate", function() {
345345

346346
$animate.enabled(true);
347347

348+
element.addClass('ng-hide');
348349
$animate.removeClass(element, 'ng-hide');
349350
expect(element.text()).toBe('memento');
350351
}));
@@ -616,7 +617,7 @@ describe("ngAnimate", function() {
616617
ss.addRule('.ng-hide-add', style);
617618
ss.addRule('.ng-hide-remove', style);
618619

619-
element = $compile(html('<div>1</div>'))($rootScope);
620+
element = $compile(html('<div class="ng-hide">1</div>'))($rootScope);
620621
element.addClass('custom');
621622

622623
$animate.removeClass(element, 'ng-hide');
@@ -627,6 +628,7 @@ describe("ngAnimate", function() {
627628
expect(element.hasClass('ng-hide-remove-active')).toBe(true);
628629
}
629630

631+
element.removeClass('ng-hide');
630632
$animate.addClass(element, 'ng-hide');
631633
expect(element.hasClass('ng-hide-remove')).toBe(false); //added right away
632634

@@ -1052,21 +1054,59 @@ describe("ngAnimate", function() {
10521054
});
10531055

10541056
describe("addClass / removeClass", function() {
1057+
var captured;
10551058
beforeEach(function() {
10561059
module(function($animateProvider, $provide) {
10571060
$animateProvider.register('.klassy', function($timeout) {
10581061
return {
10591062
addClass : function(element, className, done) {
1063+
captured = 'addClass-' + className;
10601064
$timeout(done, 500, false);
10611065
},
10621066
removeClass : function(element, className, done) {
1067+
captured = 'removeClass-' + className;
10631068
$timeout(done, 3000, false);
10641069
}
10651070
}
10661071
});
10671072
});
10681073
});
10691074

1075+
it("should not perform an animation, and the followup DOM operation, if the class is " +
1076+
"already present during addClass or not present during removeClass on the element",
1077+
inject(function($animate, $rootScope, $sniffer, $rootElement, $timeout) {
1078+
1079+
var element = jqLite('<div class="klassy"></div>');
1080+
$rootElement.append(element);
1081+
body.append($rootElement);
1082+
1083+
//skipped animations
1084+
captured = 'none';
1085+
$animate.removeClass(element, 'some-class');
1086+
expect(element.hasClass('some-class')).toBe(false);
1087+
expect(captured).toBe('none');
1088+
1089+
element.addClass('some-class');
1090+
1091+
captured = 'nothing';
1092+
$animate.addClass(element, 'some-class');
1093+
expect(captured).toBe('nothing');
1094+
expect(element.hasClass('some-class')).toBe(true);
1095+
1096+
//actual animations
1097+
captured = 'none';
1098+
$animate.removeClass(element, 'some-class');
1099+
$timeout.flush();
1100+
expect(element.hasClass('some-class')).toBe(false);
1101+
expect(captured).toBe('removeClass-some-class');
1102+
1103+
captured = 'nothing';
1104+
$animate.addClass(element, 'some-class');
1105+
$timeout.flush();
1106+
expect(element.hasClass('some-class')).toBe(true);
1107+
expect(captured).toBe('addClass-some-class');
1108+
}));
1109+
10701110
it("should add and remove CSS classes after an animation even if no animation is present",
10711111
inject(function($animate, $rootScope, $sniffer, $rootElement) {
10721112

0 commit comments

Comments
 (0)