Skip to content

Commit

Permalink
fix(ngAnimate): ensure that ngClass is always compiled before enter, …
Browse files Browse the repository at this point in the history 
…leave and move animations

Closes angular#3727
Closes angular#3603
  • Loading branch information
@matsko @jamesdaily
matsko authored and jamesdaily committed Sep 25, 2013
1 parent c6c70d4 commit c81264d
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 37 deletions.
34 changes: 25 additions & 9 deletions src/ngAnimate/animate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ angular.module('ngAnimate', ['ng'])

var NG_ANIMATE_STATE = '$$ngAnimateState';
var rootAnimateState = {running:true};
$provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout',
function($delegate, $injector, $sniffer, $rootElement, $timeout) {

$provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout', '$rootScope',
function($delegate, $injector, $sniffer, $rootElement, $timeout, $rootScope) {
$rootElement.data(NG_ANIMATE_STATE, rootAnimateState);

function lookup(name) {
Expand Down Expand Up @@ -282,8 +282,10 @@ angular.module('ngAnimate', ['ng'])
*/
enter : function(element, parent, after, done) {
$delegate.enter(element, parent, after);
performAnimation('enter', 'ng-enter', element, parent, after, function() {
done && $timeout(done, 0, false);
$rootScope.$$postDigest(function() {
performAnimation('enter', 'ng-enter', element, parent, after, function() {
done && $timeout(done, 0, false);
});
});
},

Expand Down Expand Up @@ -315,8 +317,10 @@ angular.module('ngAnimate', ['ng'])
* @param {function()=} done callback function that will be called once the animation is complete
*/
leave : function(element, done) {
performAnimation('leave', 'ng-leave', element, null, null, function() {
$delegate.leave(element, done);
$rootScope.$$postDigest(function() {
performAnimation('leave', 'ng-leave', element, null, null, function() {
$delegate.leave(element, done);
});
});
},

Expand Down Expand Up @@ -352,8 +356,10 @@ angular.module('ngAnimate', ['ng'])
*/
move : function(element, parent, after, done) {
$delegate.move(element, parent, after);
performAnimation('move', 'ng-move', element, null, null, function() {
done && $timeout(done, 0, false);
$rootScope.$$postDigest(function() {
performAnimation('move', 'ng-move', element, null, null, function() {
done && $timeout(done, 0, false);
});
});
},

Expand Down Expand Up @@ -550,6 +556,7 @@ angular.module('ngAnimate', ['ng'])

var durationKey = 'Duration',
delayKey = 'Delay',
propertyKey = 'Property',
animationIterationCountKey = 'IterationCount',
ELEMENT_NODE = 1;

Expand Down Expand Up @@ -610,13 +617,22 @@ angular.module('ngAnimate', ['ng'])
timeout is empty (this would cause a flicker bug normally
in the page */
if(duration > 0) {
var node = element[0];

//temporarily disable the transition so that the enter styles
//don't animate twice (this is here to avoid a bug in Chrome/FF).
node.style[w3cTransitionProp + propertyKey] = 'none';
node.style[vendorTransitionProp + propertyKey] = 'none';

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

//this triggers a reflow which allows for the transition animation to kick in
element.prop('clientWidth');
node.style[w3cTransitionProp + propertyKey] = '';
node.style[vendorTransitionProp + propertyKey] = '';
element.addClass(activeClassName);

$timeout(done, duration * 1000, false);
Expand Down
123 changes: 95 additions & 28 deletions test/ng/directive/ngClassSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,40 +308,107 @@ describe('ngClass', function() {
describe('ngClass animations', function() {
var body, element, $rootElement;

beforeEach(module('mock.animate'));

it("should avoid calling addClass accidentally when removeClass is going on",
it("should avoid calling addClass accidentally when removeClass is going on", function() {
module('mock.animate');
inject(function($compile, $rootScope, $animate, $timeout) {
var element = angular.element('<div ng-class="val"></div>');
var body = jqLite(document.body);
body.append(element);
$compile(element)($rootScope);

var element = angular.element('<div ng-class="val"></div>');
var body = jqLite(document.body);
body.append(element);
$compile(element)($rootScope);
expect($animate.queue.length).toBe(0);

expect($animate.queue.length).toBe(0);
$rootScope.val = 'one';
$rootScope.$digest();
$animate.flushNext('addClass');
$animate.flushNext('addClass');
expect($animate.queue.length).toBe(0);

$rootScope.val = 'one';
$timeout.flush();
$rootScope.val = '';
$rootScope.$digest();
$animate.flushNext('removeClass'); //only removeClass is called
expect($animate.queue.length).toBe(0);

$rootScope.$digest();
$animate.flushNext('addClass');
$animate.flushNext('addClass');
expect($animate.queue.length).toBe(0);
$rootScope.val = 'one';
$rootScope.$digest();
$animate.flushNext('addClass');
expect($animate.queue.length).toBe(0);

$rootScope.val = '';
$rootScope.$digest();
$animate.flushNext('removeClass'); //only removeClass is called
expect($animate.queue.length).toBe(0);
$rootScope.val = 'two';
$rootScope.$digest();
$animate.flushNext('removeClass');
$animate.flushNext('addClass');
expect($animate.queue.length).toBe(0);
});
});

$rootScope.val = 'one';
$rootScope.$digest();
$animate.flushNext('addClass');
expect($animate.queue.length).toBe(0);
it("should consider the ngClass expression evaluation before performing an animation", function() {

//mocks are not used since the enter delegation method is called before addClass and
//it makes it impossible to test to see that addClass is called first
module('ngAnimate');

var digestQueue = [];
module(function($animateProvider) {
$animateProvider.register('.crazy', function() {
return {
enter : function(element, done) {
element.data('state', 'crazy-enter');
done();
}
};
});

return function($rootScope) {
var before = $rootScope.$$postDigest;
$rootScope.$$postDigest = function() {
var args = arguments;
digestQueue.push(function() {
before.apply($rootScope, args);
});
};
};
});
inject(function($compile, $rootScope, $rootElement, $animate, $timeout, $document) {

$rootScope.val = 'two';
$rootScope.$digest();
$animate.flushNext('removeClass');
$animate.flushNext('addClass');
expect($animate.queue.length).toBe(0);
}));
//since we skip animations upon first digest, this needs to be set to true
$animate.enabled(true);

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

$compile(element)($rootScope);

var enterComplete = false;
$animate.enter(element, $rootElement, null, function() {
enterComplete = true;
});

//jquery doesn't compare both elements properly so let's use the nodes
expect(element.parent()[0]).toEqual($rootElement[0]);
expect(element.hasClass('crazy')).toBe(false);
expect(enterComplete).toBe(false);

expect(digestQueue.length).toBe(1);
$rootScope.$digest();

$timeout.flush();

expect(element.hasClass('crazy')).toBe(true);
expect(enterComplete).toBe(false);

digestQueue.shift()(); //enter
expect(digestQueue.length).toBe(0);

//we don't normally need this, but since the timing between digests
//is spaced-out then it is required so that the original digestion
//is kicked into gear
$rootScope.$digest();
$timeout.flush();

expect(element.data('state')).toBe('crazy-enter');
expect(enterComplete).toBe(true);
});
});
});
22 changes: 22 additions & 0 deletions test/ngAnimate/animateSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ describe("ngAnimate", function() {

expect(element.contents().length).toBe(0);
$animate.enter(child, element);
$rootScope.$digest();

if($sniffer.transitions) {
expect(child.hasClass('ng-enter')).toBe(true);
Expand All @@ -148,6 +149,8 @@ describe("ngAnimate", function() {

expect(element.contents().length).toBe(1);
$animate.leave(child);
$rootScope.$digest();

if($sniffer.transitions) {
expect(child.hasClass('ng-leave')).toBe(true);
expect(child.hasClass('ng-leave-active')).toBe(true);
Expand All @@ -169,6 +172,7 @@ describe("ngAnimate", function() {
element.append(child2);
expect(element.text()).toBe('12');
$animate.move(child1, element, child2);
$rootScope.$digest();
expect(element.text()).toBe('21');
}));

Expand Down Expand Up @@ -213,13 +217,17 @@ describe("ngAnimate", function() {

//enter
$animate.enter(child, element);
$rootScope.$digest();

expect(child.attr('class')).toContain('ng-enter');
expect(child.attr('class')).toContain('ng-enter-active');
$timeout.flushNext(1000);

//move
element.append(after);
$animate.move(child, element, after);
$rootScope.$digest();

expect(child.attr('class')).toContain('ng-move');
expect(child.attr('class')).toContain('ng-move-active');
$timeout.flushNext(1000);
Expand All @@ -238,6 +246,7 @@ describe("ngAnimate", function() {

//leave
$animate.leave(child);
$rootScope.$digest();
expect(child.attr('class')).toContain('ng-leave');
expect(child.attr('class')).toContain('ng-leave-active');
$timeout.flushNext(1000);
Expand Down Expand Up @@ -274,6 +283,7 @@ describe("ngAnimate", function() {
expect(child).toBeShown();

$animate.leave(child);
$rootScope.$digest();
expect(child).toBeHidden(); //hides instantly

//lets change this to prove that done doesn't fire anymore for the previous hide() operation
Expand Down Expand Up @@ -682,6 +692,7 @@ describe("ngAnimate", function() {

element[0].className = 'abc';
$animate.enter(element, parent);
$rootScope.$digest();

if ($sniffer.transitions) {
expect(element.hasClass('abc ng-enter')).toBe(true);
Expand All @@ -692,6 +703,7 @@ describe("ngAnimate", function() {

element[0].className = 'xyz';
$animate.enter(element, parent);
$rootScope.$digest();

if ($sniffer.transitions) {
expect(element.hasClass('xyz')).toBe(true);
Expand All @@ -717,6 +729,7 @@ describe("ngAnimate", function() {
element.attr('class','one two');

$animate.enter(element, parent);
$rootScope.$digest();
if($sniffer.transitions) {
expect(element.hasClass('one two ng-enter')).toBe(true);
expect(element.hasClass('one two ng-enter ng-enter-active')).toBe(true);
Expand Down Expand Up @@ -766,6 +779,7 @@ describe("ngAnimate", function() {
$animate.enter(element, parent, null, function() {
flag = true;
});
$rootScope.$digest();

$timeout.flush();

Expand All @@ -784,6 +798,7 @@ describe("ngAnimate", function() {
$animate.leave(element, function() {
flag = true;
});
$rootScope.$digest();

$timeout.flush();

Expand All @@ -803,6 +818,7 @@ describe("ngAnimate", function() {
$animate.move(element, parent, parent2, function() {
flag = true;
});
$rootScope.$digest();

$timeout.flush();

Expand Down Expand Up @@ -1212,6 +1228,7 @@ describe("ngAnimate", function() {
var child = $compile('<div>...</div>')($rootScope);

$animate.enter(child, element);
$rootScope.$digest();

if($sniffer.transitions) {
expect(child.hasClass('ng-enter')).toBe(true);
Expand All @@ -1233,6 +1250,7 @@ describe("ngAnimate", function() {
var child = $compile('<div>...</div>')($rootScope);

$animate.enter(child, element);
$rootScope.$digest();

if($sniffer.transitions) {
expect(child.hasClass('ng-enter')).toBe(true);
Expand All @@ -1257,6 +1275,7 @@ describe("ngAnimate", function() {

expect(child.hasClass('ng-enter')).toBe(false);
$animate.enter(child, element);
$rootScope.$digest();
expect(child.hasClass('ng-enter')).toBe(false);
}));

Expand All @@ -1283,6 +1302,7 @@ describe("ngAnimate", function() {

child.addClass('custom');
$animate.enter(child, element);
$rootScope.$digest();

$timeout.flushNext(10);

Expand Down Expand Up @@ -1315,6 +1335,7 @@ describe("ngAnimate", function() {
var child = $compile('<div>...</div>')($rootScope);

$animate.enter(child, element);
$rootScope.$digest();

//this is added/removed right away otherwise
if($sniffer.transitions) {
Expand All @@ -1325,6 +1346,7 @@ describe("ngAnimate", function() {
expect(child.hasClass('this-is-mine-now')).toBe(false);
child.addClass('usurper');
$animate.leave(child);
$rootScope.$digest();

expect(child.hasClass('ng-enter')).toBe(false);
expect(child.hasClass('ng-enter-active')).toBe(false);
Expand Down
5 changes: 5 additions & 0 deletions test/ngRoute/directive/ngViewSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,13 +637,18 @@ describe('ngView animations', function() {
$rootScope.$digest();

$animate.flushNext('leave'); //ngView old

$rootScope.$digest();

$animate.flushNext('enter'); //ngView new

expect(n(element.text())).toEqual(''); //this is midway during the animation

$animate.flushNext('enter'); //ngRepeat 3
$animate.flushNext('enter'); //ngRepeat 4

$rootScope.$digest();

expect(element.text()).toEqual('34');

function n(text) {
Expand Down

0 comments on commit c81264d

Please sign in to comment.