Skip to content

Commit

Permalink
fix($animate): only cancel class-based animations if the follow-up cl…
Browse files Browse the repository at this point in the history
…ass contains CSS transition/keyframe animation code

Closes angular#4463
Closes angular#3784
  • Loading branch information
matsko committed Oct 22, 2013
1 parent 280e33d commit d0ed528
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 22 deletions.
86 changes: 66 additions & 20 deletions src/ngAnimate/animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -489,35 +489,47 @@ angular.module('ngAnimate', ['ng'])
*/
function performAnimation(event, className, element, parent, after, onComplete) {
var classes = (element.attr('class') || '') + ' ' + className;
var animationLookup = (' ' + classes).replace(/\s+/g,'.'),
animations = [];
forEach(lookup(animationLookup), function(animation, index) {
animations.push({
start : animation[event]
});
});

var animationLookup = (' ' + classes).replace(/\s+/g,'.');
if (!parent) {
parent = after ? after.parent() : element.parent();
}

var disabledAnimation = { running : true };
var matches = lookup(animationLookup);
var isClassBased = event == 'addClass' || event == 'removeClass';
var ngAnimateState = element.data(NG_ANIMATE_STATE) || {};

//skip the animation if animations are disabled, a parent is already being animated
//or the element is not currently attached to the document body.
if ((parent.inheritedData(NG_ANIMATE_STATE) || disabledAnimation).running || animations.length == 0) {
//skip the animation if animations are disabled, a parent is already being animated,
//the element is not currently attached to the document body or then completely close
//the animation if any matching animations are not found at all.
//NOTE: IE8 + IE9 should close properly (run done()) in case a NO animation is not found.
if ((parent.inheritedData(NG_ANIMATE_STATE) || disabledAnimation).running || matches.length == 0) {
done();
return;
}

var ngAnimateState = element.data(NG_ANIMATE_STATE) || {};
var animations = [];
//only add animations if the currently running animation is not structural
//or if there is no animation running at all
if(!ngAnimateState.running || !(isClassBased && ngAnimateState.structural)) {
forEach(matches, function(animation) {
//add the animation to the queue to if it is allowed to be cancelled
if(!animation.allowCancel || animation.allowCancel(element, event, className)) {
animations.push({
start : animation[event]
});
}
});
}

var isClassBased = event == 'addClass' || event == 'removeClass';
if(ngAnimateState.running) {
if(isClassBased && ngAnimateState.structural) {
onComplete && onComplete();
return;
}
//this would mean that an animation was not allowed so let the existing
//animation do it's thing and close this one early
if(animations.length == 0) {
onComplete && onComplete();
return;
}

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.flagTimer);
Expand Down Expand Up @@ -643,6 +655,7 @@ angular.module('ngAnimate', ['ng'])
ELEMENT_NODE = 1;

var NG_ANIMATE_PARENT_KEY = '$ngAnimateKey';
var NG_ANIMATE_CLASS_KEY = '$$ngAnimateClasses';
var lookupCache = {};
var parentCounter = 0;

Expand All @@ -661,7 +674,7 @@ angular.module('ngAnimate', ['ng'])
}

function getElementAnimationDetails(element, cacheKey, onlyCheckTransition) {
var data = lookupCache[cacheKey];
var data = cacheKey ? lookupCache[cacheKey] : null;
if(!data) {
var transitionDuration = 0, transitionDelay = 0,
animationDuration = 0, animationDelay = 0;
Expand Down Expand Up @@ -694,7 +707,9 @@ angular.module('ngAnimate', ['ng'])
transitionDuration : transitionDuration,
animationDuration : animationDuration
};
lookupCache[cacheKey] = data;
if(cacheKey) {
lookupCache[cacheKey] = data;
}
}
return data;
}
Expand Down Expand Up @@ -761,6 +776,7 @@ angular.module('ngAnimate', ['ng'])
element.addClass(activeClassName);
});

element.data(NG_ANIMATE_CLASS_KEY, className + ' ' + activeClassName);
element.on(css3AnimationEvents, onAnimationProgress);

// This will automatically be called by $animate so
Expand All @@ -770,6 +786,7 @@ angular.module('ngAnimate', ['ng'])
element.off(css3AnimationEvents, onAnimationProgress);
element.removeClass(className);
element.removeClass(activeClassName);
element.removeData(NG_ANIMATE_CLASS_KEY);

// Only when the animation is cancelled is the done()
// function not called for this animation therefore
Expand Down Expand Up @@ -803,6 +820,35 @@ angular.module('ngAnimate', ['ng'])
}

return {
allowCancel : function(element, event, className) {
//always cancel the current animation if it is a
//structural animation
var oldClasses = element.data(NG_ANIMATE_CLASS_KEY);
if(!oldClasses || ['enter','leave','move'].indexOf(event) >= 0) {
return true;
}

var parent = element.parent();
var clone = angular.element(element[0].cloneNode());

//make the element super hidden and override any CSS style values
clone.attr('style','position:absolute; top:-9999px; left:-9999px');
clone.removeAttr('id');
clone.html('');

angular.forEach(oldClasses.split(' '), function(klass) {
clone.removeClass(klass);
});

var suffix = event == 'addClass' ? '-add' : '-remove';
clone.addClass(suffixClasses(className, suffix));
parent.append(clone);

var timings = getElementAnimationDetails(clone);
clone.remove();

return Math.max(timings.transitionDuration, timings.animationDuration) > 0;
},
enter : function(element, done) {
return animate(element, 'ng-enter', done);
},
Expand Down
40 changes: 38 additions & 2 deletions test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,8 @@ describe("ngAnimate", function() {
expect(element.hasClass('ng-enter')).toBe(true);
expect(element.hasClass('ng-enter-active')).toBe(true);
browserTrigger(element,'transitionend', { timeStamp: Date.now() + 22000, elapsedTime: 22000 });
$timeout.flush();
}
$timeout.flush();
expect(element.hasClass('abc')).toBe(true);

$rootScope.klass = 'xyz';
Expand All @@ -760,8 +760,8 @@ describe("ngAnimate", function() {
expect(element.hasClass('ng-enter')).toBe(true);
expect(element.hasClass('ng-enter-active')).toBe(true);
browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 });
$timeout.flush();
}
$timeout.flush();
expect(element.hasClass('xyz')).toBe(true);
}));

Expand Down Expand Up @@ -1920,4 +1920,40 @@ describe("ngAnimate", function() {
expect(count).toBe(40);
});
});

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

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);
}));
});

0 comments on commit d0ed528

Please sign in to comment.