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

Commit

Permalink
fix($animate): ensure $animate doesn't break natural CSS transitions
Browse files Browse the repository at this point in the history
BREAKING CHANGE: ngClass and {{ class }} will now call the `setClass`
animation callback instead of addClass / removeClass when both a
addClass/removeClass operation is being executed on the element during the animation.

Please include the setClass animation callback as well as addClass and removeClass within
your JS animations to work with ngClass and {{ class }} directives.

Closes #6019
  • Loading branch information
matsko authored and IgorMinar committed Feb 15, 2014
1 parent cf5e463 commit 4f84f6b
Show file tree
Hide file tree
Showing 9 changed files with 344 additions and 280 deletions.
5 changes: 5 additions & 0 deletions css/angular.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@
ng\:form {
display: block;
}

.ng-animate-block-transitions {
transition:0s all!important;
-webkit-transition:0s all!important;
}
23 changes: 23 additions & 0 deletions src/ng/animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,29 @@ var $AnimateProvider = ['$provide', function($provide) {
done && $timeout(done, 0, false);
},

/**
*
* @ngdoc function
* @name ng.$animate#setClass
* @methodOf ng.$animate
* @function
* @description Adds and/or removes the given CSS classes to and from the element.
* Once complete, the done() callback will be fired (if provided).
* @param {jQuery/jqLite element} element the element which will it's CSS classes changed
* removed from it
* @param {string} add the CSS classes which will be added to the element
* @param {string} remove the CSS class which will be removed from the element
* @param {function=} done the callback function (if provided) that will be fired after the
* CSS classes have been set on the element
*/
setClass : function(element, add, remove, done) {
forEach(element, function (element) {
jqLiteAddClass(element, add);
jqLiteRemoveClass(element, remove);
});
done && $timeout(done, 0, false);
},

enabled : noop
};
}];
Expand Down
12 changes: 10 additions & 2 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -690,8 +690,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
* @param {string} oldClasses The former CSS className value
*/
$updateClass : function(newClasses, oldClasses) {
this.$removeClass(tokenDifference(oldClasses, newClasses));
this.$addClass(tokenDifference(newClasses, oldClasses));
var toAdd = tokenDifference(newClasses, oldClasses);
var toRemove = tokenDifference(oldClasses, newClasses);

if(toAdd.length === 0) {
$animate.removeClass(this.$$element, toRemove);
} else if(toRemove.length === 0) {
$animate.addClass(this.$$element, toAdd);
} else {
$animate.setClass(this.$$element, toAdd, toRemove);
}
},

/**
Expand Down
491 changes: 286 additions & 205 deletions src/ngAnimate/animate.js

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,8 @@ angular.mock.animate = angular.module('ngAnimateMock', ['ng'])
}
};

angular.forEach(['enter','leave','move','addClass','removeClass'], function(method) {
angular.forEach(
['enter','leave','move','addClass','removeClass','setClass'], function(method) {
animate[method] = function() {
animate.queue.push({
event : method,
Expand Down
6 changes: 2 additions & 4 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4666,11 +4666,9 @@ describe('$compile', function() {
$rootScope.$digest();

data = $animate.queue.shift();
expect(data.event).toBe('removeClass');
expect(data.args[1]).toBe('rice');
data = $animate.queue.shift();
expect(data.event).toBe('addClass');
expect(data.event).toBe('setClass');
expect(data.args[1]).toBe('dice');
expect(data.args[2]).toBe('rice');

expect(element.hasClass('ice')).toBe(true);
expect(element.hasClass('dice')).toBe(true);
Expand Down
10 changes: 3 additions & 7 deletions test/ng/directive/ngClassSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,7 @@ describe('ngClass animations', function() {

$rootScope.val = 'two';
$rootScope.$digest();
expect($animate.queue.shift().event).toBe('removeClass');
expect($animate.queue.shift().event).toBe('addClass');
expect($animate.queue.shift().event).toBe('setClass');
expect($animate.queue.length).toBe(0);
});
});
Expand Down Expand Up @@ -450,12 +449,9 @@ describe('ngClass animations', function() {
$rootScope.$digest();

item = $animate.queue.shift();
expect(item.event).toBe('removeClass');
expect(item.args[1]).toBe('two');

item = $animate.queue.shift();
expect(item.event).toBe('addClass');
expect(item.event).toBe('setClass');
expect(item.args[1]).toBe('three');
expect(item.args[2]).toBe('two');

expect($animate.queue.length).toBe(0);
});
Expand Down
71 changes: 12 additions & 59 deletions test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ describe("ngAnimate", function() {
$animate.triggerReflow();

//this is to verify that the existing style is appended with a semicolon automatically
expect(child.attr('style')).toMatch(/width: 20px;.+?/i);
expect(child.attr('style')).toMatch(/width: 20px;.*?/i);
browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1 });
}

Expand Down Expand Up @@ -564,15 +564,15 @@ describe("ngAnimate", function() {
});
});

it("should fire the cancel/end function with the correct flag in the parameters",
it("should not apply a cancellation when addClass is done multiple times",
inject(function($animate, $rootScope, $sniffer, $timeout) {

element.append(child);

$animate.addClass(child, 'custom-delay');
$animate.addClass(child, 'custom-long-delay');

expect(child.hasClass('animation-cancelled')).toBe(true);
expect(child.hasClass('animation-cancelled')).toBe(false);
expect(child.hasClass('animation-ended')).toBe(false);

$timeout.flush();
Expand Down Expand Up @@ -764,7 +764,6 @@ describe("ngAnimate", function() {
$animate.addClass(element, 'ng-hide');
expect(element.hasClass('ng-hide-remove')).toBe(false); //added right away


if($sniffer.animations) { //cleanup some pending animations
$animate.triggerReflow();
expect(element.hasClass('ng-hide-add')).toBe(true);
Expand Down Expand Up @@ -1472,6 +1471,8 @@ describe("ngAnimate", function() {

expect(flag).toBe(true);
expect(element.parent().id).toBe(parent2.id);

dealoc(element);
}));


Expand Down Expand Up @@ -1620,11 +1621,12 @@ describe("ngAnimate", function() {
var element = parent.find('span');

var flag = false;
$animate.removeClass(element, 'ng-hide', function() {
$animate.addClass(element, 'ng-hide', function() {
flag = true;
});

if($sniffer.transitions) {
$animate.triggerReflow();
browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1 });
}
$timeout.flush();
Expand Down Expand Up @@ -2734,42 +2736,6 @@ describe("ngAnimate", function() {
});


it("should cancel an ongoing class-based animation only if the new class contains transition/animation CSS code",
inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {

if (!$sniffer.transitions) return;

ss.addRule('.green-add', '-webkit-transition:1s linear all;' +
'transition:1s linear all;');

ss.addRule('.blue-add', 'background:blue;');

ss.addRule('.red-add', '-webkit-transition:1s linear all;' +
'transition:1s linear all;');

ss.addRule('.yellow-add', '-webkit-animation: some_animation 4s linear 1s 2 alternate;' +
'animation: some_animation 4s linear 1s 2 alternate;');

var element = $compile('<div></div>')($rootScope);
$rootElement.append(element);
jqLite($document[0].body).append($rootElement);

$animate.addClass(element, 'green');
expect(element.hasClass('green-add')).toBe(true);

$animate.addClass(element, 'blue');
expect(element.hasClass('blue')).toBe(true);
expect(element.hasClass('green-add')).toBe(true); //not cancelled

$animate.addClass(element, 'red');
expect(element.hasClass('green-add')).toBe(false);
expect(element.hasClass('red-add')).toBe(true);

$animate.addClass(element, 'yellow');
expect(element.hasClass('red-add')).toBe(false);
expect(element.hasClass('yellow-add')).toBe(true);
}));


it("should cancel and perform the dom operation only after the reflow has run",
inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {
Expand Down Expand Up @@ -2837,7 +2803,7 @@ describe("ngAnimate", function() {
$animate.removeClass(element, 'on');
$animate.addClass(element, 'on');

expect(currentAnimation).toBe(null);
expect(currentAnimation).toBe('addClass');
});
});

Expand Down Expand Up @@ -3259,7 +3225,7 @@ describe("ngAnimate", function() {
expect(ready).toBe(true);
}));

it('should avoid skip animations if the same CSS class is added / removed synchronously before the reflow kicks in',
it('should immediately close the former animation if the same CSS class is added/removed',
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {

if (!$sniffer.transitions) return;
Expand All @@ -3281,28 +3247,15 @@ describe("ngAnimate", function() {
signature += 'B';
});

$timeout.flush(1);
expect(signature).toBe('AB');

signature = '';
$animate.removeClass(element, 'on', function() {
signature += 'A';
});
$animate.addClass(element, 'on', function() {
signature += 'B';
});
$animate.removeClass(element, 'on', function() {
signature += 'C';
});
$animate.triggerReflow();

$timeout.flush(1);
expect(signature).toBe('AB');
expect(signature).toBe('A');

$animate.triggerReflow();
browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 2000 });
$timeout.flush(1);

expect(signature).toBe('ABC');
expect(signature).toBe('AB');
}));
});
});
3 changes: 1 addition & 2 deletions test/ngRoute/directive/ngViewSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -767,8 +767,7 @@ describe('ngView animations', function() {
$rootScope.klass = 'boring';
$rootScope.$digest();

expect($animate.queue.shift().event).toBe('removeClass');
expect($animate.queue.shift().event).toBe('addClass');
expect($animate.queue.shift().event).toBe('setClass');

expect(item.hasClass('classy')).toBe(false);
expect(item.hasClass('boring')).toBe(true);
Expand Down

0 comments on commit 4f84f6b

Please sign in to comment.