Skip to content

Commit 164b360

Browse files
committed
fix($animate): prevent race conditions for class-based animations when animating on the same CSS class
Closes angular#5588
1 parent 31c91fd commit 164b360

File tree

2 files changed

+70
-5
lines changed

2 files changed

+70
-5
lines changed

src/ngAnimate/animate.js

+15-4
Original file line numberDiff line numberDiff line change
@@ -659,12 +659,23 @@ angular.module('ngAnimate', ['ng'])
659659
cleanup(element);
660660
cancelAnimations(ngAnimateState.animations);
661661

662+
//in the event that the CSS is class is quickly added and removed back
663+
//then we don't want to wait until after the reflow to add/remove the CSS
664+
//class since both class animations may run into a race condition.
665+
//The code below will check to see if that is occurring and will
666+
//immediately remove the former class before the reflow so that the
667+
//animation can snap back to the original animation smoothly
668+
var isFullyClassBasedAnimation = isClassBased && !ngAnimateState.structural;
669+
var isRevertingClassAnimation = isFullyClassBasedAnimation &&
670+
ngAnimateState.className == className &&
671+
animationEvent != ngAnimateState.event;
672+
662673
//if the class is removed during the reflow then it will revert the styles temporarily
663674
//back to the base class CSS styling causing a jump-like effect to occur. This check
664675
//here ensures that the domOperation is only performed after the reflow has commenced
665-
if(ngAnimateState.beforeComplete) {
676+
if(ngAnimateState.beforeComplete || isRevertingClassAnimation) {
666677
(ngAnimateState.done || noop)(true);
667-
} else if(isClassBased && !ngAnimateState.structural) {
678+
} else if(isFullyClassBasedAnimation) {
668679
//class-based animations will compare element className values after cancelling the
669680
//previous animation to see if the element properties already contain the final CSS
670681
//class and if so then the animation will be skipped. Since the domOperation will
@@ -812,10 +823,10 @@ angular.module('ngAnimate', ['ng'])
812823
function cancelAnimations(animations) {
813824
var isCancelledFlag = true;
814825
forEach(animations, function(animation) {
815-
if(!animations.beforeComplete) {
826+
if(!animation.beforeComplete) {
816827
(animation.beforeEnd || noop)(isCancelledFlag);
817828
}
818-
if(!animations.afterComplete) {
829+
if(!animation.afterComplete) {
819830
(animation.afterEnd || noop)(isCancelledFlag);
820831
}
821832
});

test/ngAnimate/animateSpec.js

+55-1
Original file line numberDiff line numberDiff line change
@@ -2675,10 +2675,16 @@ describe("ngAnimate", function() {
26752675
beforeAddClass : function(element, className, done) {
26762676
currentAnimation = 'addClass';
26772677
currentFn = done;
2678+
return function(cancelled) {
2679+
currentAnimation = cancelled ? null : currentAnimation;
2680+
}
26782681
},
26792682
beforeRemoveClass : function(element, className, done) {
26802683
currentAnimation = 'removeClass';
26812684
currentFn = done;
2685+
return function(cancelled) {
2686+
currentAnimation = cancelled ? null : currentAnimation;
2687+
}
26822688
}
26832689
};
26842690
});
@@ -2692,10 +2698,12 @@ describe("ngAnimate", function() {
26922698
expect(currentAnimation).toBe('addClass');
26932699
currentFn();
26942700

2701+
currentAnimation = null;
2702+
26952703
$animate.removeClass(element, 'on');
26962704
$animate.addClass(element, 'on');
26972705

2698-
expect(currentAnimation).toBe('addClass');
2706+
expect(currentAnimation).toBe(null);
26992707
});
27002708
});
27012709

@@ -3115,5 +3123,51 @@ describe("ngAnimate", function() {
31153123
$timeout.flush(1);
31163124
expect(ready).toBe(true);
31173125
}));
3126+
3127+
it('should avoid skip animations if the same CSS class is added / removed synchronously before the reflow kicks in',
3128+
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {
3129+
3130+
if (!$sniffer.transitions) return;
3131+
3132+
ss.addRule('.water-class', '-webkit-transition:2s linear all;' +
3133+
'transition:2s linear all;');
3134+
3135+
$animate.enabled(true);
3136+
3137+
var element = $compile('<div class="water-class on"></div>')($rootScope);
3138+
$rootElement.append(element);
3139+
jqLite($document[0].body).append($rootElement);
3140+
3141+
var signature = '';
3142+
$animate.removeClass(element, 'on', function() {
3143+
signature += 'A';
3144+
});
3145+
$animate.addClass(element, 'on', function() {
3146+
signature += 'B';
3147+
});
3148+
3149+
$timeout.flush(1);
3150+
expect(signature).toBe('AB');
3151+
3152+
signature = '';
3153+
$animate.removeClass(element, 'on', function() {
3154+
signature += 'A';
3155+
});
3156+
$animate.addClass(element, 'on', function() {
3157+
signature += 'B';
3158+
});
3159+
$animate.removeClass(element, 'on', function() {
3160+
signature += 'C';
3161+
});
3162+
3163+
$timeout.flush(1);
3164+
expect(signature).toBe('AB');
3165+
3166+
$timeout.flush(10);
3167+
browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 2000 });
3168+
$timeout.flush(1);
3169+
3170+
expect(signature).toBe('ABC');
3171+
}));
31183172
});
31193173
});

0 commit comments

Comments
 (0)