From 822dcc8c4c7ea328724a13595d5844d8b772ce5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Tue, 13 Aug 2013 20:51:03 -0400 Subject: [PATCH 1/2] chore($rootScope): provide support to execute a function after the digest cycle is complete --- .../angular-bootstrap/bootstrap-prettify.js | 6 +- src/ng/rootScope.js | 16 ++++ test/ng/rootScopeSpec.js | 74 +++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/docs/components/angular-bootstrap/bootstrap-prettify.js b/docs/components/angular-bootstrap/bootstrap-prettify.js index 169d232b6075..68160a4a89c5 100644 --- a/docs/components/angular-bootstrap/bootstrap-prettify.js +++ b/docs/components/angular-bootstrap/bootstrap-prettify.js @@ -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; }]); diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index d94a621d94c6..0602b2a79e7d 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -119,6 +119,7 @@ function $RootScopeProvider(){ this['this'] = this.$root = this; this.$$destroyed = false; this.$$asyncQueue = []; + this.$$postDigestQueue = []; this.$$listeners = {}; this.$$isolateBindings = {}; } @@ -133,6 +134,7 @@ function $RootScopeProvider(){ Scope.prototype = { + constructor: Scope, /** * @ngdoc function * @name ng.$rootScope.Scope#$new @@ -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 @@ -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, @@ -566,6 +570,14 @@ function $RootScopeProvider(){ } while (dirty || asyncQueue.length); clearPhase(); + + while(postDigestQueue.length) { + try { + postDigestQueue.shift()(); + } catch (e) { + $exceptionHandler(e); + } + } }, @@ -683,6 +695,10 @@ function $RootScopeProvider(){ this.$$asyncQueue.push(expr); }, + $$postDigest : function(expr) { + this.$$postDigestQueue.push(expr); + }, + /** * @ngdoc function * @name ng.$rootScope.Scope#$apply diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index ddd830881d9b..ca30ddca09d9 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -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); @@ -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); + })); + + 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; From 372aa4f9af223dd982b0c8d3655f28dc4d4d098e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 14 Aug 2013 17:03:11 -0400 Subject: [PATCH 2/2] fix(ngAnimate): ensure that ngClass is always compiled before enter, leave and move animations --- src/ngAnimate/animate.js | 24 +++-- test/ng/directive/ngClassSpec.js | 131 ++++++++++++++++++++------- test/ngAnimate/animateSpec.js | 34 ++++--- test/ngRoute/directive/ngViewSpec.js | 4 +- 4 files changed, 141 insertions(+), 52 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 32bbd6d50f85..2e4ac35041e6 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -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) { @@ -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); + }); }); }, @@ -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); + }); }); }, @@ -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); + }); }); }, diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js index 54dee1420744..831bb5a68772 100644 --- a/test/ng/directive/ngClassSpec.js +++ b/test/ng/directive/ngClassSpec.js @@ -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('
'); + var body = jqLite(document.body); + body.append(element); + $compile(element)($rootScope); - var element = angular.element('
'); - 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('
'); + 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); + }); + }); }); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 75029889ae2c..8daa3b46086d 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -133,15 +133,8 @@ describe("ngAnimate", function() { expect(element.contents().length).toBe(0); $animate.enter(child, element); - $timeout.flushNext(0); - - if($sniffer.transitions) { - expect(child.hasClass('ng-enter')).toBe(true); - $timeout.flushNext(0); - expect(child.hasClass('ng-enter-active')).toBe(true); - $timeout.flushNext(1000); - } - $timeout.flush(); + $rootScope.$digest(); + $timeout.flush($sniffer.transitions ? 1 : 0); expect(element.contents().length).toBe(1); })); @@ -151,6 +144,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); $timeout.flushNext(0); @@ -174,6 +169,7 @@ describe("ngAnimate", function() { element.append(child2); expect(element.text()).toBe('12'); $animate.move(child1, element, child2); + $rootScope.$digest(); expect(element.text()).toBe('21'); })); @@ -221,6 +217,7 @@ describe("ngAnimate", function() { //enter $animate.enter(child, element); + $rootScope.$digest(); $timeout.flushNext(0); expect(child.attr('class')).toContain('ng-enter'); $timeout.flushNext(0); @@ -231,6 +228,7 @@ describe("ngAnimate", function() { //move element.append(after); $animate.move(child, element, after); + $rootScope.$digest(); $timeout.flushNext(0); expect(child.attr('class')).toContain('ng-move'); $timeout.flushNext(0); @@ -256,6 +254,7 @@ describe("ngAnimate", function() { //leave $animate.leave(child); + $rootScope.$digest(); expect(child.attr('class')).toContain('ng-leave'); $timeout.flushNext(0); expect(child.attr('class')).toContain('ng-leave-active'); @@ -296,6 +295,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 @@ -732,6 +732,7 @@ describe("ngAnimate", function() { element[0].className = 'abc'; $animate.enter(element, parent); + $rootScope.$digest(); $timeout.flushNext(0); if ($sniffer.transitions) { @@ -745,6 +746,7 @@ describe("ngAnimate", function() { element[0].className = 'xyz'; $animate.enter(element, parent); + $rootScope.$digest(); $timeout.flushNext(0); if ($sniffer.transitions) { @@ -773,6 +775,7 @@ describe("ngAnimate", function() { element.attr('class','one two'); $animate.enter(element, parent); + $rootScope.$digest(); $timeout.flushNext(0); if($sniffer.transitions) { expect(element.hasClass('one two ng-enter')).toBe(true); @@ -824,6 +827,7 @@ describe("ngAnimate", function() { $animate.enter(element, parent, null, function() { flag = true; }); + $rootScope.$digest(); $timeout.flushNext(0); //move operation callback $timeout.flushNext(0); //ngAnimate callback @@ -843,6 +847,7 @@ describe("ngAnimate", function() { $animate.leave(element, function() { flag = true; }); + $rootScope.$digest(); $timeout.flushNext(0); expect(flag).toBe(true); @@ -861,6 +866,7 @@ describe("ngAnimate", function() { $animate.move(element, parent, parent2, function() { flag = true; }); + $rootScope.$digest(); $timeout.flushNext(0); //move operation callback $timeout.flushNext(0); //ngAnimate callback @@ -1300,6 +1306,7 @@ describe("ngAnimate", function() { var child = $compile('
...
')($rootScope); $animate.enter(child, element); + $rootScope.$digest(); $timeout.flushNext(0); if($sniffer.transitions) { @@ -1323,6 +1330,7 @@ describe("ngAnimate", function() { var child = $compile('
...
')($rootScope); $animate.enter(child, element); + $rootScope.$digest(); $timeout.flushNext(0); if($sniffer.transitions) { @@ -1349,6 +1357,7 @@ describe("ngAnimate", function() { expect(child.hasClass('ng-enter')).toBe(false); $animate.enter(child, element); + $rootScope.$digest(); expect(child.hasClass('ng-enter')).toBe(false); })); @@ -1375,6 +1384,7 @@ describe("ngAnimate", function() { child.addClass('custom'); $animate.enter(child, element); + $rootScope.$digest(); $timeout.flushNext(0); if($sniffer.transitions) { @@ -1417,7 +1427,8 @@ describe("ngAnimate", function() { var child = $compile('
...
')($rootScope); $animate.enter(child, element); - $timeout.flushNext(0); + $rootScope.$digest(); + $timeout.flushNext(0); //initial callback //this is added/removed right away otherwise if($sniffer.transitions) { @@ -1428,7 +1439,7 @@ describe("ngAnimate", function() { expect(child.hasClass('this-is-mine-now')).toBe(false); child.addClass('usurper'); $animate.leave(child); - $timeout.flushNext(0); + $rootScope.$digest(); expect(child.hasClass('ng-enter')).toBe(false); expect(child.hasClass('ng-enter-active')).toBe(false); @@ -1439,6 +1450,7 @@ describe("ngAnimate", function() { $timeout.flushNext(0); } + $timeout.flushNext(0); $timeout.flushNext(55); if($sniffer.transitions) { $timeout.flushNext(2000); diff --git a/test/ngRoute/directive/ngViewSpec.js b/test/ngRoute/directive/ngViewSpec.js index 690243e7161f..bd5a4dd6a5a9 100644 --- a/test/ngRoute/directive/ngViewSpec.js +++ b/test/ngRoute/directive/ngViewSpec.js @@ -642,7 +642,8 @@ describe('ngView animations', function() { $rootScope.$digest(); $animate.flushNext('leave'); //ngView old - $timeout.flush(); + + $rootScope.$digest(); //this is required $animate.flushNext('enter'); //ngView new $timeout.flush(); @@ -652,6 +653,7 @@ describe('ngView animations', function() { $animate.flushNext('enter'); //ngRepeat 3 $animate.flushNext('enter'); //ngRepeat 4 + $rootScope.$digest(); $timeout.flush(); expect(element.text()).toEqual('34');