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

fix($animate): ensure that class-based animations only consider the most recent DOM operations #9458

Closed
wants to merge 1 commit into from
Closed

fix($animate): ensure that class-based animations only consider the most recent DOM operations #9458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 27 additions & 26 deletions src/ngAnimate/animate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,31 +469,16 @@ angular.module('ngAnimate', ['ng'])

function resolveElementClasses(element, cache, runningAnimations) {
runningAnimations = runningAnimations || {};
var map = {};

forEach(cache.add, function(className) {
if (className && className.length) {
map[className] = map[className] || 0;
map[className]++;
}
});

forEach(cache.remove, function(className) {
if (className && className.length) {
map[className] = map[className] || 0;
map[className]--;
}
});

var lookup = [];
var lookup = {};
forEach(runningAnimations, function(data, selector) {
forEach(selector.split(' '), function(s) {
lookup[s]=data;
});
});

var toAdd = [], toRemove = [];
forEach(map, function(status, className) {
forEach(cache.classes, function(status, className) {
var hasClass = angular.$$hasClass(element[0], className);
var matchingAnimation = lookup[className] || {};

Expand All @@ -505,12 +490,12 @@ angular.module('ngAnimate', ['ng'])
// Once an animation is allowed then the code will also check to see if
// there exists any on-going animation that is already adding or remvoing
// the matching CSS class.
if (status < 0) {
if (status === false) {
//does it have the class or will it have the class
if (hasClass || matchingAnimation.event == 'addClass') {
toRemove.push(className);
}
} else if (status > 0) {
} else if (status === true) {
//is the class missing or will it be removed?
if (!hasClass || matchingAnimation.event == 'removeClass') {
toAdd.push(className);
Expand Down Expand Up @@ -997,20 +982,36 @@ angular.module('ngAnimate', ['ng'])
return $delegate.setClass(element, add, remove);
}

// we're using a combined array for both the add and remove
// operations since the ORDER OF addClass and removeClass matters
var classes, cache = element.data(STORAGE_KEY);
var hasCache = !!cache;
if (!cache) {
cache = {};
cache.classes = {};
}
classes = cache.classes;

add = isArray(add) ? add : add.split(' ');
remove = isArray(remove) ? remove : remove.split(' ');
forEach(add, function(c) {
if (c && c.length) {
classes[c] = true;
}
});

var cache = element.data(STORAGE_KEY);
if (cache) {
cache.add = cache.add.concat(add);
cache.remove = cache.remove.concat(remove);
remove = isArray(remove) ? remove : remove.split(' ');
forEach(remove, function(c) {
if (c && c.length) {
classes[c] = false;
}
});

if (hasCache) {
//the digest cycle will combine all the animations into one function
return cache.promise;
} else {
element.data(STORAGE_KEY, cache = {
add : add,
remove : remove
classes : classes
});
}

Expand Down
67 changes: 66 additions & 1 deletion test/ngAnimate/animateSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3408,11 +3408,76 @@ describe("ngAnimate", function() {
$animate.triggerReflow();

expect(log.length).toBe(2);
expect(log[0]).toEqual({ name : 'addClass', className : 'one five' });
expect(log[0]).toEqual({ name : 'addClass', className : 'one four five' });
expect(log[1]).toEqual({ name : 'removeClass', className : 'three' });
});
});

it('should intelligently cancel out redundant class-based animations', function() {
var log = [];
var track = function(name) {
return function() {
log.push({ name : name, className : arguments[1] });
};
};
module(function($animateProvider) {
$animateProvider.register('.animate', function() {
return {
addClass : track('addClass'),
removeClass : track('removeClass')
};
});
});
inject(function($rootScope, $animate, $compile, $rootElement, $document) {
$animate.enabled(true);

var element = $compile('<div class="animate three four"></div>')($rootScope);
$rootElement.append(element);
angular.element($document[0].body).append($rootElement);

$animate.removeClass(element, 'one');
$rootScope.$digest();
$animate.triggerReflow();
expect(log.length).toBe(0);
$animate.triggerCallbacks();

$animate.addClass(element, 'two');
$animate.addClass(element, 'two');
$animate.removeClass(element, 'two');
$rootScope.$digest();
$animate.triggerReflow();
expect(log.length).toBe(0);
$animate.triggerCallbacks();

$animate.removeClass(element, 'three');
$animate.addClass(element, 'three');
$rootScope.$digest();
$animate.triggerReflow();
expect(log.length).toBe(0);
$animate.triggerCallbacks();

$animate.removeClass(element, 'four');
$animate.addClass(element, 'four');
$animate.removeClass(element, 'four');
$rootScope.$digest();
$animate.triggerReflow();
expect(log.length).toBe(1);
$animate.triggerCallbacks();
expect(log[0]).toEqual({ name : 'removeClass', className : 'four' });

$animate.addClass(element, 'five');
$animate.addClass(element, 'five');
$animate.addClass(element, 'five');
$animate.removeClass(element, 'five');
$animate.addClass(element, 'five');
$rootScope.$digest();
$animate.triggerReflow();
expect(log.length).toBe(2);
$animate.triggerCallbacks();
expect(log[1]).toEqual({ name : 'addClass', className : 'five' });
});
});

it('should skip class-based animations if the element is removed before the digest occurs', function() {
var spy = jasmine.createSpy();
module(function($animateProvider) {
Expand Down