Skip to content

Commit

Permalink
fix(ngAnimate): don't normalize classes on follow-up animations to jo…
Browse files Browse the repository at this point in the history
…ined animations

This allows follow-up animations to remove a class that is currently
being added.

Fixes angular#13339
Fixes angular#13380
Closes angular#13414
  • Loading branch information
Narretz authored and matsko committed Dec 9, 2015
1 parent 023b777 commit dd45a48
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 4 deletions.
43 changes: 39 additions & 4 deletions src/ngAnimate/animateQueue.js
Expand Up @@ -12,6 +12,14 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
join: []
};

function makeTruthyMapFromKeys(keys) {
var map = Object.create(null);
forEach(keys, function(key) {
map[key] = true;
});
return map;
}

function isAllowed(ruleType, element, currentAnimation, previousAnimation) {
return rules[ruleType].some(function(fn) {
return fn(element, currentAnimation, previousAnimation);
Expand Down Expand Up @@ -59,11 +67,38 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
});

rules.cancel.push(function(element, newAnimation, currentAnimation) {
var nO = newAnimation.options;
var cO = currentAnimation.options;
var ONE_SPACE = ' ';

var nA = newAnimation.options.addClass;
var nR = newAnimation.options.removeClass;
var cA = currentAnimation.options.addClass;
var cR = currentAnimation.options.removeClass;

// early detection to save the global CPU shortage :)
if ((!isDefined(nA) && !isDefined(nR)) || (!isDefined(cA) && !isDefined(cR))) {
return false;
}

var cancelSomething = false;

cA = cA ? makeTruthyMapFromKeys(cA.split(ONE_SPACE)) : null;
cR = cR ? makeTruthyMapFromKeys(cR.split(ONE_SPACE)) : null;

if (cR && nA) {
cancelSomething = nA.split(ONE_SPACE).some(function(className) {
return cR[className];
});

if (cancelSomething) return true;
}

if (cA && nR) {
cancelSomething = nR.split(ONE_SPACE).some(function(className) {
return cA[className];
});
}

// if the exact same CSS class is added/removed then it's safe to cancel it
return (nO.addClass && nO.addClass === cO.removeClass) || (nO.removeClass && nO.removeClass === cO.addClass);
return cancelSomething;
});

this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$HashMap',
Expand Down
39 changes: 39 additions & 0 deletions test/ngAnimate/integrationSpec.js
Expand Up @@ -351,6 +351,45 @@ describe('ngAnimate integration tests', function() {

dealoc(element);
}));


it("should not normalize classes in a follow-up animation to a joined class-based animation",
inject(function($animate, $animateCss, $rootScope, $document, $rootElement, $$rAF) {

ss.addRule('.hide', 'opacity: 0');
ss.addRule('.hide-add, .hide-remove', 'transition: 1s linear all');

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

// These animations will be joined together
$animate.addClass(element, 'red');
$animate.addClass(element, 'hide');
$rootScope.$digest();

expect(element).toHaveClass('red-add');
expect(element).toHaveClass('hide-add');

// When a digest has passed, but no $rAF has been issued yet, .hide hasn't been added to
// the element, which means class normalization does not allow .hide to get removed
$animate.removeClass(element, 'hide');
$rootScope.$digest();
$$rAF.flush();

expect(element).not.toHaveClass('hide-add hide-add-active');
expect(element).toHaveClass('hide-remove hide-remove-active');

//End the animation process
browserTrigger(element, 'transitionend',
{ timeStamp: Date.now() + 1000, elapsedTime: 2 });
$animate.flush();

expect(element).not.toHaveClass('hide-add-active red-add-active');
expect(element).toHaveClass('red');
expect(element).not.toHaveClass('hide');
}));

});

describe('JS animations', function() {
Expand Down

0 comments on commit dd45a48

Please sign in to comment.