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

Commit

Permalink
fix(ngAnimate): make sure that the class value passed into addClass/r…
Browse files Browse the repository at this point in the history 
…emoveClass is the base class string value
  • Loading branch information
@matsko @mhevery
matsko authored and mhevery committed Aug 9, 2013
1 parent f61ff69 commit 2430347
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 104 deletions.
209 changes: 105 additions & 104 deletions src/ngAnimate/animate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ angular.module('ngAnimate', ['ng'])
done:done
});

var baseClassName = className;
if(event == 'addClass') {
className = suffixClasses(className, '-add');
} else if(event == 'removeClass') {
Expand All @@ -504,7 +505,7 @@ angular.module('ngAnimate', ['ng'])

if(animation.start) {
if(event == 'addClass' || event == 'removeClass') {
animation.endFn = animation.start(element, className, fn);
animation.endFn = animation.start(element, baseClassName, fn);
} else {
animation.endFn = animation.start(element, fn);
}
Expand All @@ -520,17 +521,6 @@ angular.module('ngAnimate', ['ng'])
});
}

function suffixClasses(classes, suffix) {
var className = '';
classes = angular.isArray(classes) ? classes : classes.split(/\s+/);
forEach(classes, function(klass, i) {
if(klass && klass.length > 0) {
className += (i > 0 ? ' ' : '') + klass + suffix;
}
});
return className;
}

function progress(index) {
animations[index].done = true;
(animations[index].endFn || noop)();
Expand All @@ -550,117 +540,128 @@ angular.module('ngAnimate', ['ng'])
}
}
}]);
}])

.animation('', ['$window','$sniffer', '$timeout', function($window, $sniffer, $timeout) {
var noop = angular.noop;
var forEach = angular.forEach;
function animate(element, className, done) {
if (!($sniffer.transitions || $sniffer.animations)) {
done();
} else {
var activeClassName = '';
$timeout(startAnimation, 1, false);

//this acts as the cancellation function in case
//a new animation is triggered while another animation
//is still going on (otherwise the active className
//would still hang around until the timer is complete).
return onEnd;
}
$animateProvider.register('', ['$window','$sniffer', '$timeout', function($window, $sniffer, $timeout) {
var noop = angular.noop;
var forEach = angular.forEach;
function animate(element, className, done) {
if (!($sniffer.transitions || $sniffer.animations)) {
done();
} else {
var activeClassName = '';
$timeout(startAnimation, 1, false);

//this acts as the cancellation function in case
//a new animation is triggered while another animation
//is still going on (otherwise the active className
//would still hang around until the timer is complete).
return onEnd;
}

function parseMaxTime(str) {
var total = 0, values = angular.isString(str) ? str.split(/\s*,\s*/) : [];
forEach(values, function(value) {
total = Math.max(parseFloat(value) || 0, total);
});
return total;
}
function parseMaxTime(str) {
var total = 0, values = angular.isString(str) ? str.split(/\s*,\s*/) : [];
forEach(values, function(value) {
total = Math.max(parseFloat(value) || 0, total);
});
return total;
}

function startAnimation() {
var duration = 0;
forEach(className.split(' '), function(klass, i) {
activeClassName += (i > 0 ? ' ' : '') + klass + '-active';
});
function startAnimation() {
var duration = 0;
forEach(className.split(' '), function(klass, i) {
activeClassName += (i > 0 ? ' ' : '') + klass + '-active';
});

element.addClass(activeClassName);
element.addClass(activeClassName);

//one day all browsers will have these properties
var w3cAnimationProp = 'animation';
var w3cTransitionProp = 'transition';
//one day all browsers will have these properties
var w3cAnimationProp = 'animation';
var w3cTransitionProp = 'transition';

//but some still use vendor-prefixed styles
var vendorAnimationProp = $sniffer.vendorPrefix + 'Animation';
var vendorTransitionProp = $sniffer.vendorPrefix + 'Transition';
//but some still use vendor-prefixed styles
var vendorAnimationProp = $sniffer.vendorPrefix + 'Animation';
var vendorTransitionProp = $sniffer.vendorPrefix + 'Transition';

var durationKey = 'Duration',
delayKey = 'Delay',
animationIterationCountKey = 'IterationCount';
var durationKey = 'Duration',
delayKey = 'Delay',
animationIterationCountKey = 'IterationCount';

//we want all the styles defined before and after
var ELEMENT_NODE = 1;
forEach(element, function(element) {
if (element.nodeType == ELEMENT_NODE) {
var elementStyles = $window.getComputedStyle(element) || {};
//we want all the styles defined before and after
var ELEMENT_NODE = 1;
forEach(element, function(element) {
if (element.nodeType == ELEMENT_NODE) {
var elementStyles = $window.getComputedStyle(element) || {};

var transitionDelay = Math.max(parseMaxTime(elementStyles[w3cTransitionProp + delayKey]),
parseMaxTime(elementStyles[vendorTransitionProp + delayKey]));
var transitionDelay = Math.max(parseMaxTime(elementStyles[w3cTransitionProp + delayKey]),
parseMaxTime(elementStyles[vendorTransitionProp + delayKey]));

var animationDelay = Math.max(parseMaxTime(elementStyles[w3cAnimationProp + delayKey]),
parseMaxTime(elementStyles[vendorAnimationProp + delayKey]));
var animationDelay = Math.max(parseMaxTime(elementStyles[w3cAnimationProp + delayKey]),
parseMaxTime(elementStyles[vendorAnimationProp + delayKey]));

var transitionDuration = Math.max(parseMaxTime(elementStyles[w3cTransitionProp + durationKey]),
parseMaxTime(elementStyles[vendorTransitionProp + durationKey]));
var transitionDuration = Math.max(parseMaxTime(elementStyles[w3cTransitionProp + durationKey]),
parseMaxTime(elementStyles[vendorTransitionProp + durationKey]));

var animationDuration = Math.max(parseMaxTime(elementStyles[w3cAnimationProp + durationKey]),
parseMaxTime(elementStyles[vendorAnimationProp + durationKey]));
var animationDuration = Math.max(parseMaxTime(elementStyles[w3cAnimationProp + durationKey]),
parseMaxTime(elementStyles[vendorAnimationProp + durationKey]));

if(animationDuration > 0) {
animationDuration *= Math.max(parseInt(elementStyles[w3cAnimationProp + animationIterationCountKey]) || 0,
parseInt(elementStyles[vendorAnimationProp + animationIterationCountKey]) || 0,
1);
if(animationDuration > 0) {
animationDuration *= Math.max(parseInt(elementStyles[w3cAnimationProp + animationIterationCountKey]) || 0,
parseInt(elementStyles[vendorAnimationProp + animationIterationCountKey]) || 0,
1);
}

duration = Math.max(animationDelay + animationDuration,
transitionDelay + transitionDuration,
duration);
}
});

duration = Math.max(animationDelay + animationDuration,
transitionDelay + transitionDuration,
duration);
}
});
$timeout(done, duration * 1000, false);
}

$timeout(done, duration * 1000, false);
//this will automatically be called by $animate so
//there is no need to attach this internally to the
//timeout done method
function onEnd(cancelled) {
element.removeClass(activeClassName);

//only when the animation is cancelled is the done()
//function not called for this animation therefore
//this must be also called
if(cancelled) {
done();
}
}
}

//this will automatically be called by $animate so
//there is no need to attach this internally to the
//timeout done method
function onEnd(cancelled) {
element.removeClass(activeClassName);

//only when the animation is cancelled is the done()
//function not called for this animation therefore
//this must be also called
if(cancelled) {
done();
return {
enter : function(element, done) {
return animate(element, 'ng-enter', done);
},
leave : function(element, done) {
return animate(element, 'ng-leave', done);
},
move : function(element, done) {
return animate(element, 'ng-move', done);
},
addClass : function(element, className, done) {
return animate(element, suffixClasses(className, '-add'), done);
},
removeClass : function(element, className, done) {
return animate(element, suffixClasses(className, '-remove'), done);
}
}
}
};

return {
enter : function(element, done) {
return animate(element, 'ng-enter', done);
},
leave : function(element, done) {
return animate(element, 'ng-leave', done);
},
move : function(element, done) {
return animate(element, 'ng-move', done);
},
addClass : function(element, className, done) {
return animate(element, className, done);
},
removeClass : function(element, className, done) {
return animate(element, className, done);
}
};
}]);

function suffixClasses(classes, suffix) {
var className = '';
classes = angular.isArray(classes) ? classes : classes.split(/\s+/);
forEach(classes, function(klass, i) {
if(klass && klass.length > 0) {
className += (i > 0 ? ' ' : '') + klass + suffix;
}
});
return className;
}
}]);
32 changes: 32 additions & 0 deletions test/ngAnimate/animateSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1513,4 +1513,36 @@ describe("ngAnimate", function() {
});
});

it("should provide the correct CSS class to the addClass and removeClass callbacks within a JS animation", function() {
module(function($animateProvider) {
$animateProvider.register('.classify', function($timeout) {
return {
removeClass : function(element, className, done) {
element.data('classify','remove-' + className);
done();
},
addClass : function(element, className, done) {
element.data('classify','add-' + className);
done();
}
}
});
})
inject(function($compile, $rootScope, $animate, $timeout) {
var element = html($compile('<div class="classify"></div>')($rootScope));

$animate.addClass(element, 'super');
expect(element.data('classify')).toBe('add-super');
$timeout.flush();

$animate.removeClass(element, 'super');
expect(element.data('classify')).toBe('remove-super');
$timeout.flush();

$animate.addClass(element, 'superguy');
expect(element.data('classify')).toBe('add-superguy');
$timeout.flush();
});
});

});

3 comments on commit 2430347

@damrbaby
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI I am still experiencing poor ngRepeat performance. There is flashing and it appears like there are one-too-many $digest cycles being performed (this is just a guess)? Basically sometimes I will see elements momentarily flash into view that should no longer be in the ngRepeat list -- this occurs when I change the array of items in the list to a new array. And for a list with 50 items they take a second to appear into view initially. This quirky behavior only occurs when I have ngAnimate as a dependency.

@damrbaby
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a plunkr that shows the problem clearer than my last one: http://plnkr.co/edit/udltDOfGBWBGrBP1spEK?p=preview

The above plunkr shows how re-assigning the array used for ngRepeat with ngAnimate as a dependency causes weird flashing of the ngRepeat items, which doesn't occur when you remove the ngAnimate dependency.

@damrbaby
Copy link
Contributor

Choose a reason for hiding this comment

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

@matsko @mhevery @IgorMinar Hey guys, just want to make sure you're aware of the above issue that I posted. It looks like you're gearing up for a release soon, and I don't want to see it ship with an unusable ngAnimate module that will keep me and anybody else who uses ngAnimate with ngRepeat on 1.1.5

Please sign in to comment.