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

Make ngAnimate work as closely as possible to natural CSS transitions #5646

Closed
wants to merge 3 commits into from
Closed

Make ngAnimate work as closely as possible to natural CSS transitions #5646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22db8e2
fix($animate): avoid accidentally matching substrings when resolving …
matsko Jan 2, 2014
31c91fd
fix($animate): correctly detect and handle CSS transition changes dur…
matsko Jan 2, 2014
164b360
fix($animate): prevent race conditions for class-based animations whe…
matsko Jan 6, 2014
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
69 changes: 55 additions & 14 deletions src/ngAnimate/animate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,40 +647,52 @@ angular.module('ngAnimate', ['ng'])
return;
}

var ONE_SPACE = ' ';
//this value will be searched for class-based CSS className lookup. Therefore,
//we prefix and suffix the current className value with spaces to avoid substring
//lookups of className tokens
var futureClassName = ' ' + currentClassName + ' ';
var futureClassName = ONE_SPACE + currentClassName + ONE_SPACE;
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.closeAnimationTimeout);
cleanup(element);
cancelAnimations(ngAnimateState.animations);

//in the event that the CSS is class is quickly added and removed back
//then we don't want to wait until after the reflow to add/remove the CSS
//class since both class animations may run into a race condition.
//The code below will check to see if that is occurring and will
//immediately remove the former class before the reflow so that the
//animation can snap back to the original animation smoothly
var isFullyClassBasedAnimation = isClassBased && !ngAnimateState.structural;
var isRevertingClassAnimation = isFullyClassBasedAnimation &&
ngAnimateState.className == className &&
animationEvent != ngAnimateState.event;

//if the class is removed during the reflow then it will revert the styles temporarily
//back to the base class CSS styling causing a jump-like effect to occur. This check
//here ensures that the domOperation is only performed after the reflow has commenced
if(ngAnimateState.beforeComplete) {
if(ngAnimateState.beforeComplete || isRevertingClassAnimation) {
(ngAnimateState.done || noop)(true);
} else if(isClassBased && !ngAnimateState.structural) {
} else if(isFullyClassBasedAnimation) {
//class-based animations will compare element className values after cancelling the
//previous animation to see if the element properties already contain the final CSS
//class and if so then the animation will be skipped. Since the domOperation will
//be performed only after the reflow is complete then our element's className value
//will be invalid. Therefore the same string manipulation that would occur within the
//DOM operation will be performed below so that the class comparison is valid...
futureClassName = ngAnimateState.event == 'removeClass' ?
futureClassName.replace(ngAnimateState.className, '') :
futureClassName + ngAnimateState.className + ' ';
futureClassName.replace(ONE_SPACE + ngAnimateState.className + ONE_SPACE, ONE_SPACE) :
futureClassName + ngAnimateState.className + ONE_SPACE;
}
}

//There is no point in perform a class-based animation if the element already contains
//(on addClass) or doesn't contain (on removeClass) the className being animated.
//The reason why this is being called after the previous animations are cancelled
//is so that the CSS classes present on the element can be properly examined.
var classNameToken = ' ' + className + ' ';
var classNameToken = ONE_SPACE + className + ONE_SPACE;
if((animationEvent == 'addClass' && futureClassName.indexOf(classNameToken) >= 0) ||
(animationEvent == 'removeClass' && futureClassName.indexOf(classNameToken) == -1)) {
fireDOMOperation();
Expand Down Expand Up @@ -811,10 +823,10 @@ angular.module('ngAnimate', ['ng'])
function cancelAnimations(animations) {
var isCancelledFlag = true;
forEach(animations, function(animation) {
if(!animations.beforeComplete) {
if(!animation.beforeComplete) {
(animation.beforeEnd || noop)(isCancelledFlag);
}
if(!animations.afterComplete) {
if(!animation.afterComplete) {
(animation.afterEnd || noop)(isCancelledFlag);
}
});
Expand Down Expand Up @@ -1042,7 +1054,7 @@ angular.module('ngAnimate', ['ng'])
return parentID + '-' + extractElementNode(element).className;
}

function animateSetup(element, className) {
function animateSetup(element, className, calculationDecorator) {
var cacheKey = getCacheKey(element);
var eventCacheKey = cacheKey + ' ' + className;
var stagger = {};
Expand All @@ -1060,9 +1072,16 @@ angular.module('ngAnimate', ['ng'])
applyClasses && element.removeClass(staggerClassName);
}

/* the animation itself may need to add/remove special CSS classes
* before calculating the anmation styles */
calculationDecorator = calculationDecorator ||
function(fn) { return fn(); };

element.addClass(className);

var timings = getElementAnimationDetails(element, eventCacheKey);
var timings = calculationDecorator(function() {
return getElementAnimationDetails(element, eventCacheKey);
});

/* there is no point in performing a reflow if the animation
timeout is empty (this would cause a flicker bug normally
Expand Down Expand Up @@ -1227,8 +1246,8 @@ angular.module('ngAnimate', ['ng'])
return style;
}

function animateBefore(element, className) {
if(animateSetup(element, className)) {
function animateBefore(element, className, calculationDecorator) {
if(animateSetup(element, className, calculationDecorator)) {
return function(cancelled) {
cancelled && animateClose(element, className);
};
Expand Down Expand Up @@ -1323,7 +1342,18 @@ angular.module('ngAnimate', ['ng'])
},

beforeAddClass : function(element, className, animationCompleted) {
var cancellationMethod = animateBefore(element, suffixClasses(className, '-add'));
var cancellationMethod = animateBefore(element, suffixClasses(className, '-add'), function(fn) {

/* when a CSS class is added to an element then the transition style that
* is applied is the transition defined on the element when the CSS class
* is added at the time of the animation. This is how CSS3 functions
* outside of ngAnimate. */
element.addClass(className);
var timings = fn();
element.removeClass(className);
return timings;
});

if(cancellationMethod) {
afterReflow(element, function() {
unblockTransitions(element);
Expand All @@ -1340,7 +1370,18 @@ angular.module('ngAnimate', ['ng'])
},

beforeRemoveClass : function(element, className, animationCompleted) {
var cancellationMethod = animateBefore(element, suffixClasses(className, '-remove'));
var cancellationMethod = animateBefore(element, suffixClasses(className, '-remove'), function(fn) {
/* when classes are removed from an element then the transition style
* that is applied is the transition defined on the element without the
* CSS class being there. This is how CSS3 functions outside of ngAnimate.
* http://plnkr.co/edit/j8OzgTNxHTb4n3zLyjGW?p=preview */
var klass = element.attr('class');
element.removeClass(className);
var timings = fn();
element.attr('class', klass);
return timings;
});

if(cancellationMethod) {
afterReflow(element, function() {
unblockTransitions(element);
Expand Down
154 changes: 152 additions & 2 deletions test/ngAnimate/animateSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2667,6 +2667,45 @@ describe("ngAnimate", function() {
expect(element.hasClass('red')).toBe(true);
}));

it("should avoid mixing up substring classes during add and remove operations", function() {
var currentAnimation, currentFn;
module(function($animateProvider) {
$animateProvider.register('.on', function() {
return {
beforeAddClass : function(element, className, done) {
currentAnimation = 'addClass';
currentFn = done;
return function(cancelled) {
currentAnimation = cancelled ? null : currentAnimation;
}
},
beforeRemoveClass : function(element, className, done) {
currentAnimation = 'removeClass';
currentFn = done;
return function(cancelled) {
currentAnimation = cancelled ? null : currentAnimation;
}
}
};
});
});
inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {
var element = $compile('<div class="animation-enabled only"></div>')($rootScope);
$rootElement.append(element);
jqLite($document[0].body).append($rootElement);

$animate.addClass(element, 'on');
expect(currentAnimation).toBe('addClass');
currentFn();

currentAnimation = null;

$animate.removeClass(element, 'on');
$animate.addClass(element, 'on');

expect(currentAnimation).toBe(null);
});
});

it('should enable and disable animations properly on the root element', function() {
var count = 0;
Expand Down Expand Up @@ -2772,14 +2811,14 @@ describe("ngAnimate", function() {
$animate.removeClass(element, 'base-class one two');

//still true since we're before the reflow
expect(element.hasClass('base-class')).toBe(true);
expect(element.hasClass('base-class')).toBe(false);

//this will cancel the remove animation
$animate.addClass(element, 'base-class one two');

//the cancellation was a success and the class was added right away
//since there was no successive animation for the after animation
expect(element.hasClass('base-class')).toBe(true);
expect(element.hasClass('base-class')).toBe(false);

//the reflow...
$timeout.flush();
Expand Down Expand Up @@ -3019,5 +3058,116 @@ describe("ngAnimate", function() {
expect(leaveDone).toBe(true);
});
});

it('should respect the most relevant CSS transition property if defined in multiple classes',
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {

if (!$sniffer.transitions) return;

ss.addRule('.base-class', '-webkit-transition:1s linear all;' +
'transition:1s linear all;');

ss.addRule('.base-class.on', '-webkit-transition:5s linear all;' +
'transition:5s linear all;');

$animate.enabled(true);

var element = $compile('<div class="base-class"></div>')($rootScope);
$rootElement.append(element);
jqLite($document[0].body).append($rootElement);

var ready = false;
$animate.addClass(element, 'on', function() {
ready = true;
});

$timeout.flush(10);
browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 1 });
$timeout.flush(1);
expect(ready).toBe(false);

browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 5 });
$timeout.flush(1);
expect(ready).toBe(true);

ready = false;
$animate.removeClass(element, 'on', function() {
ready = true;
});

$timeout.flush(10);
browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 1 });
$timeout.flush(1);
expect(ready).toBe(true);
}));

it('should not apply a transition upon removal of a class that has a transition',
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {

if (!$sniffer.transitions) return;

ss.addRule('.base-class.on', '-webkit-transition:5s linear all;' +
'transition:5s linear all;');

$animate.enabled(true);

var element = $compile('<div class="base-class on"></div>')($rootScope);
$rootElement.append(element);
jqLite($document[0].body).append($rootElement);

var ready = false;
$animate.removeClass(element, 'on', function() {
ready = true;
});

$timeout.flush(1);
expect(ready).toBe(true);
}));

it('should avoid skip animations if the same CSS class is added / removed synchronously before the reflow kicks in',
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {

if (!$sniffer.transitions) return;

ss.addRule('.water-class', '-webkit-transition:2s linear all;' +
'transition:2s linear all;');

$animate.enabled(true);

var element = $compile('<div class="water-class on"></div>')($rootScope);
$rootElement.append(element);
jqLite($document[0].body).append($rootElement);

var signature = '';
$animate.removeClass(element, 'on', function() {
signature += 'A';
});
$animate.addClass(element, 'on', function() {
signature += 'B';
});

$timeout.flush(1);
expect(signature).toBe('AB');

signature = '';
$animate.removeClass(element, 'on', function() {
signature += 'A';
});
$animate.addClass(element, 'on', function() {
signature += 'B';
});
$animate.removeClass(element, 'on', function() {
signature += 'C';
});

$timeout.flush(1);
expect(signature).toBe('AB');

$timeout.flush(10);
browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 2000 });
$timeout.flush(1);

expect(signature).toBe('ABC');
}));
});
});