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

ngAnimate ngClass digest fixes #3717

Closed
wants to merge 2 commits 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
6 changes: 5 additions & 1 deletion docs/components/angular-bootstrap/bootstrap-prettify.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,13 @@ directive.ngEmbedApp = ['$templateCache', '$browser', '$rootScope', '$location',
}]);
$provide.decorator('$rootScope', ['$delegate', function($delegate) {
embedRootScope = $delegate;
var embedded$digest = embedRootScope.$digest;
deregisterEmbedRootScope = docsRootScope.$watch(function embedRootScopeDigestWatch() {
embedRootScope.$digest();
embedded$digest.call(embedRootScope);
});
embedRootScope.constructor.prototype.$digest = function() {
return docsRootScope.$digest();
};

return embedRootScope;
}]);
Expand Down
16 changes: 16 additions & 0 deletions src/ng/rootScope.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ function $RootScopeProvider(){
this['this'] = this.$root = this;
this.$$destroyed = false;
this.$$asyncQueue = [];
this.$$postDigestQueue = [];
this.$$listeners = {};
this.$$isolateBindings = {};
}
Expand All @@ -133,6 +134,7 @@ function $RootScopeProvider(){


Scope.prototype = {
constructor: Scope,
/**
* @ngdoc function
* @name ng.$rootScope.Scope#$new
Expand Down Expand Up @@ -167,6 +169,7 @@ function $RootScopeProvider(){
child.$root = this.$root;
// ensure that there is just one async queue per $rootScope and it's children
child.$$asyncQueue = this.$$asyncQueue;
child.$$postDigestQueue = this.$$postDigestQueue;
} else {
Child = function() {}; // should be anonymous; This is so that when the minifier munges
// the name it does not become random set of chars. These will then show up as class
Expand Down Expand Up @@ -494,6 +497,7 @@ function $RootScopeProvider(){
var watch, value, last,
watchers,
asyncQueue = this.$$asyncQueue,
postDigestQueue = this.$$postDigestQueue,
length,
dirty, ttl = TTL,
next, current, target = this,
Expand Down Expand Up @@ -566,6 +570,14 @@ function $RootScopeProvider(){
} while (dirty || asyncQueue.length);

clearPhase();

while(postDigestQueue.length) {
try {
postDigestQueue.shift()();
} catch (e) {
$exceptionHandler(e);
}
}
},


Expand Down Expand Up @@ -683,6 +695,10 @@ function $RootScopeProvider(){
this.$$asyncQueue.push(expr);
},

$$postDigest : function(expr) {
this.$$postDigestQueue.push(expr);
},

/**
* @ngdoc function
* @name ng.$rootScope.Scope#$apply
Expand Down
24 changes: 15 additions & 9 deletions src/ngAnimate/animate.js
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() {
$timeout(done || noop, 0, false);
$rootScope.$$postDigest(function() {
performAnimation('enter', 'ng-enter', element, parent, after, function() {
$timeout(done || noop, 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() {
$timeout(done || noop, 0, false);
$rootScope.$$postDigest(function() {
performAnimation('move', 'ng-move', element, null, null, function() {
$timeout(done || noop, 0, false);
});
});
},

Expand Down
131 changes: 100 additions & 31 deletions test/ng/directive/ngClassSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,42 +308,111 @@ 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');
$timeout.flush();
expect($animate.queue.length).toBe(0);

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

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

$rootScope.val = 'one';
$rootScope.$digest();
$animate.flushNext('addClass');
$timeout.flush();
expect($animate.queue.length).toBe(0);
$rootScope.val = 'two';
$rootScope.$digest();
$animate.flushNext('removeClass');
$animate.flushNext('addClass');
$timeout.flush();
expect($animate.queue.length).toBe(0);
});
});

$rootScope.val = 'two';
$rootScope.$digest();
$animate.flushNext('removeClass');
$animate.flushNext('addClass');
$timeout.flush();
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) {

//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);
});
});
});
74 changes: 74 additions & 0 deletions test/ng/rootScopeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ describe('Scope', function() {
}));


it('should expose the constructor', inject(function($rootScope) {
if (msie) return;
expect($rootScope.__proto__).toBe($rootScope.constructor.prototype);
}));


it('should not have $root on children, but should inherit', inject(function($rootScope) {
var child = $rootScope.$new();
expect(child.$root).toEqual($rootScope);
Expand Down Expand Up @@ -672,6 +678,74 @@ describe('Scope', function() {
expect(log).toEqual('parent.async;child.async;parent.$digest;child.$digest;');
}));

it('should not run another digest for an $$postDigest call', inject(function($rootScope) {
var internalWatchCount = 0;
var externalWatchCount = 0;

$rootScope.internalCount = 0;
$rootScope.externalCount = 0;

$rootScope.$evalAsync(function(scope) {
$rootScope.internalCount++;
});

$rootScope.$$postDigest(function(scope) {
$rootScope.externalCount++;
});

$rootScope.$watch('internalCount', function(value) {
internalWatchCount = value;
});
$rootScope.$watch('externalCount', function(value) {
externalWatchCount = value;
});

$rootScope.$digest();

expect(internalWatchCount).toEqual(1);
expect(externalWatchCount).toEqual(0);
}));

it('should run a $$postDigest call on all child scopes when a parent scope is digested', inject(function($rootScope) {
var parent = $rootScope.$new(),
child = parent.$new(),
count = 0;

$rootScope.$$postDigest(function() {
count++;
});

parent.$$postDigest(function() {
count++;
});

child.$$postDigest(function() {
count++;
});

expect(count).toBe(0);
$rootScope.$digest();
expect(count).toBe(3);
}));
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a new spec for the case when a child is an isolate scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


it('should run a $$postDigest call even if the child scope is isolated', inject(function($rootScope) {
var parent = $rootScope.$new(),
child = parent.$new(true),
signature = '';

parent.$$postDigest(function() {
signature += 'A';
});

child.$$postDigest(function() {
signature += 'B';
});

expect(signature).toBe('');
$rootScope.$digest();
expect(signature).toBe('AB');
}));

it('should cause a $digest rerun', inject(function($rootScope) {
$rootScope.log = '';
$rootScope.value = 0;
Expand Down
Loading