Skip to content

Commit

Permalink
fix($animate): ensure addClass/removeClass animations do not snap dur…
Browse files Browse the repository at this point in the history
…ing reflow

Closes angular#4892
  • Loading branch information
matsko authored and jamesdaily committed Jan 27, 2014
1 parent d6b5092 commit 7d8d0a4
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
31 changes: 27 additions & 4 deletions src/ngAnimate/animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,8 @@ angular.module('ngAnimate', ['ng'])
and the onComplete callback will be fired once the animation is fully complete.
*/
function performAnimation(animationEvent, className, element, parentElement, afterElement, domOperation, doneCallback) {
var classes = (element.attr('class') || '') + ' ' + className;
var currentClassName = element.attr('class') || '';
var classes = currentClassName + ' ' + className;
var animationLookup = (' ' + classes).replace(/\s+/g,'.');
if (!parentElement) {
parentElement = afterElement ? afterElement.parent() : element.parent();
Expand Down Expand Up @@ -602,21 +603,42 @@ angular.module('ngAnimate', ['ng'])
return;
}

//this value will be searched for class-based CSS className lookup. Therefore,
//we prefix and suffix the current className value with spaces to avoid substring
//lookups of className tokens
var futureClassName = ' ' + currentClassName + ' ';
if(ngAnimateState.running) {
//if an animation is currently running on the element then lets take the steps
//to cancel that animation and fire any required callbacks
$timeout.cancel(ngAnimateState.closeAnimationTimeout);
cleanup(element);
cancelAnimations(ngAnimateState.animations);
(ngAnimateState.done || noop)(true);

//if the class is removed during the reflow then it will revert the styles temporarily
//back to the base class CSS styling causing a jump-like effect to occur. This check
//here ensures that the domOperation is only performed after the reflow has commenced
if(ngAnimateState.beforeComplete) {
(ngAnimateState.done || noop)(true);
} else if(isClassBased && !ngAnimateState.structural) {
//class-based animations will compare element className values after cancelling the
//previous animation to see if the element properties already contain the final CSS
//class and if so then the animation will be skipped. Since the domOperation will
//be performed only after the reflow is complete then our element's className value
//will be invalid. Therefore the same string manipulation that would occur within the
//DOM operation will be performed below so that the class comparison is valid...
futureClassName = ngAnimateState.event == 'removeClass' ?
futureClassName.replace(ngAnimateState.className, '') :
futureClassName + ngAnimateState.className + ' ';
}
}

//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((animationEvent == 'addClass' && element.hasClass(className)) ||
(animationEvent == 'removeClass' && !element.hasClass(className))) {
var classNameToken = ' ' + className + ' ';
if((animationEvent == 'addClass' && futureClassName.indexOf(classNameToken) >= 0) ||
(animationEvent == 'removeClass' && futureClassName.indexOf(classNameToken) == -1)) {
fireDOMOperation();
fireDoneCallbackAsync();
return;
Expand All @@ -628,6 +650,7 @@ angular.module('ngAnimate', ['ng'])

element.data(NG_ANIMATE_STATE, {
running:true,
event:animationEvent,
className:className,
structural:!isClassBased,
animations:animations,
Expand Down
30 changes: 30 additions & 0 deletions test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2515,6 +2515,36 @@ describe("ngAnimate", function() {
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) {

if (!$sniffer.transitions) return;

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

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

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, 'red');
expect(element.hasClass('red-add')).toBe(true);

expect(element.hasClass('green')).toBe(false);
expect(element.hasClass('red')).toBe(false);

$timeout.flush();

expect(element.hasClass('green')).toBe(true);
expect(element.hasClass('red')).toBe(true);
}));

it('should enable and disable animations properly on the root element', function() {
var count = 0;
module(function($animateProvider) {
Expand Down

0 comments on commit 7d8d0a4

Please sign in to comment.