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

fix(ngAnimate): don't normalize classes on follow-up animations to jo… #13472

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 39 additions & 4 deletions src/ngAnimate/animateQueue.js
Original file line number Diff line number Diff line change
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repeated logic here could be moved into the makeTrutyMapFromKeys and perhaps be renamed to makeCssClassMap


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];
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repeated logic here could be moved into a function...

return (hasMatchingClasses(nA, cR)) || (hasMatchingClasses(nR, cA));
function hasMatchingClasses(classString, classMap) {
  if (classString && classMap) {
    return classString.split(ONE_SPACE).some(function(className) {
      return classMap[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
Original file line number Diff line number Diff line change
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