Skip to content

Commit

Permalink
fix($animate): skip unnecessary addClass/removeClass animations
Browse files Browse the repository at this point in the history
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
  • Loading branch information
matsko committed Oct 24, 2013
1 parent faf5b98 commit 5edb98c
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
10 changes: 10 additions & 0 deletions src/ngAnimate/animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,16 @@ angular.module('ngAnimate', ['ng'])
(ngAnimateState.done || noop)();
}

//There is no point in perform a class-based animation if the element already contains
//(on addClass) or doesn't contain (on removeClass) the className being animated.
//The reason why this is being called after the previous animations are cancelled
//is so that the CSS classes present on the element can be properly examined.
if((event == 'addClass' && element.hasClass(className)) ||
(event == 'removeClass' && !element.hasClass(className))) {
onComplete && onComplete();
return;
}

element.data(NG_ANIMATE_STATE, {
running:true,
structural:!isClassBased,
Expand Down
42 changes: 41 additions & 1 deletion test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ describe("ngAnimate", function() {

$animate.enabled(true);

element.addClass('ng-hide');
$animate.removeClass(element, 'ng-hide');
expect(element.text()).toBe('memento');
}));
Expand Down Expand Up @@ -616,7 +617,7 @@ describe("ngAnimate", function() {
ss.addRule('.ng-hide-add', style);
ss.addRule('.ng-hide-remove', style);

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

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

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

Expand Down Expand Up @@ -1052,21 +1054,59 @@ describe("ngAnimate", function() {
});

describe("addClass / removeClass", function() {
var captured;
beforeEach(function() {
module(function($animateProvider, $provide) {
$animateProvider.register('.klassy', function($timeout) {
return {
addClass : function(element, className, done) {
captured = 'addClass-' + className;
$timeout(done, 500, false);
},
removeClass : function(element, className, done) {
captured = 'removeClass-' + className;
$timeout(done, 3000, false);
}
}
});
});
});

it("should not perform an animation, and the followup DOM operation, if the class is " +
"already present during addClass or not present during removeClass on the element",
inject(function($animate, $rootScope, $sniffer, $rootElement, $timeout) {

var element = jqLite('<div class="klassy"></div>');
$rootElement.append(element);
body.append($rootElement);

//skipped animations
captured = 'none';
$animate.removeClass(element, 'some-class');
expect(element.hasClass('some-class')).toBe(false);
expect(captured).toBe('none');

element.addClass('some-class');

captured = 'nothing';
$animate.addClass(element, 'some-class');
expect(captured).toBe('nothing');
expect(element.hasClass('some-class')).toBe(true);

//actual animations
captured = 'none';
$animate.removeClass(element, 'some-class');
$timeout.flush();
expect(element.hasClass('some-class')).toBe(false);
expect(captured).toBe('removeClass-some-class');

captured = 'nothing';
$animate.addClass(element, 'some-class');
$timeout.flush();
expect(element.hasClass('some-class')).toBe(true);
expect(captured).toBe('addClass-some-class');
}));

it("should add and remove CSS classes after an animation even if no animation is present",
inject(function($animate, $rootScope, $sniffer, $rootElement) {

Expand Down

0 comments on commit 5edb98c

Please sign in to comment.