From db1738ca3c393c895f1e8a8cb8e6ac35182d1621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 19 Nov 2015 11:01:14 -0800 Subject: [PATCH] fix(core): ensure animate runner is the same with and without animations The $$AnimateRunner class is now the same for the core $animate service and the ngAnimate $animate service. Previously, the core used a different implementation that didn't match the ngAnimate behavior with regard to callbacks. Closes #13205 Closes #13347 --- angularFiles.js | 1 + src/AngularPublic.js | 6 +- src/ng/animate.js | 30 +--- src/ng/animateCss.js | 39 +---- src/ng/animateRunner.js | 170 ++++++++++++++++++++ src/ngAnimate/animateRunner.js | 166 ------------------- src/ngAnimate/module.js | 4 - test/{ngAnimate => ng}/animateRunnerSpec.js | 7 +- 8 files changed, 188 insertions(+), 235 deletions(-) create mode 100644 src/ng/animateRunner.js delete mode 100644 src/ngAnimate/animateRunner.js rename test/{ngAnimate => ng}/animateRunnerSpec.js (99%) diff --git a/angularFiles.js b/angularFiles.js index 1e6eb6a6b77a..e3f41badc9bc 100755 --- a/angularFiles.js +++ b/angularFiles.js @@ -14,6 +14,7 @@ var angularFiles = { 'src/ng/anchorScroll.js', 'src/ng/animate.js', + 'src/ng/animateRunner.js', 'src/ng/animateCss.js', 'src/ng/browser.js', 'src/ng/cacheFactory.js', diff --git a/src/AngularPublic.js b/src/AngularPublic.js index 0de446d90085..04b2f64d886a 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -57,7 +57,8 @@ $AnimateProvider, $CoreAnimateCssProvider, $$CoreAnimateQueueProvider, - $$CoreAnimateRunnerProvider, + $$AnimateRunnerProvider, + $$AnimateAsyncRunFactoryProvider, $BrowserProvider, $CacheFactoryProvider, $ControllerProvider, @@ -217,7 +218,8 @@ function publishExternalAPI(angular) { $animate: $AnimateProvider, $animateCss: $CoreAnimateCssProvider, $$animateQueue: $$CoreAnimateQueueProvider, - $$AnimateRunner: $$CoreAnimateRunnerProvider, + $$AnimateRunner: $$AnimateRunnerFactoryProvider, + $$animateAsyncRun: $$AnimateAsyncRunFactoryProvider, $browser: $BrowserProvider, $cacheFactory: $CacheFactoryProvider, $controller: $ControllerProvider, diff --git a/src/ng/animate.js b/src/ng/animate.js index 7d04bf535fe4..6df23c4179ec 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -53,29 +53,6 @@ function prepareAnimateOptions(options) { : {}; } -var $$CoreAnimateRunnerProvider = function() { - this.$get = ['$q', '$$rAF', function($q, $$rAF) { - function AnimateRunner() {} - AnimateRunner.all = noop; - AnimateRunner.chain = noop; - AnimateRunner.prototype = { - end: noop, - cancel: noop, - resume: noop, - pause: noop, - complete: noop, - then: function(pass, fail) { - return $q(function(resolve) { - $$rAF(function() { - resolve(); - }); - }).then(pass, fail); - } - }; - return AnimateRunner; - }]; -}; - // this is prefixed with Core since it conflicts with // the animateQueueProvider defined in ngAnimate/animateQueue.js var $$CoreAnimateQueueProvider = function() { @@ -101,7 +78,12 @@ var $$CoreAnimateQueueProvider = function() { addRemoveClassesPostDigest(element, options.addClass, options.removeClass); } - return new $$AnimateRunner(); // jshint ignore:line + var runner = new $$AnimateRunner(); // jshint ignore:line + + // since there are no animations to run the runner needs to be + // notified that the animation call is complete. + runner.complete(); + return runner; } }; diff --git a/src/ng/animateCss.js b/src/ng/animateCss.js index 5aaf78bea637..62a207b18498 100644 --- a/src/ng/animateCss.js +++ b/src/ng/animateCss.js @@ -12,36 +12,7 @@ * Click here {@link ngAnimate.$animateCss to read the documentation for $animateCss}. */ var $CoreAnimateCssProvider = function() { - this.$get = ['$$rAF', '$q', function($$rAF, $q) { - - var RAFPromise = function() {}; - RAFPromise.prototype = { - done: function(cancel) { - this.defer && this.defer[cancel === true ? 'reject' : 'resolve'](); - }, - end: function() { - this.done(); - }, - cancel: function() { - this.done(true); - }, - getPromise: function() { - if (!this.defer) { - this.defer = $q.defer(); - } - return this.defer.promise; - }, - then: function(f1,f2) { - return this.getPromise().then(f1,f2); - }, - 'catch': function(f1) { - return this.getPromise()['catch'](f1); - }, - 'finally': function(f1) { - return this.getPromise()['finally'](f1); - } - }; - + this.$get = ['$$rAF', '$q', '$$AnimateRunner', function($$rAF, $q, $$AnimateRunner) { return function(element, options) { // there is no point in applying the styles since // there is no animation that goes on at all in @@ -55,7 +26,7 @@ var $CoreAnimateCssProvider = function() { options.from = null; } - var closed, runner = new RAFPromise(); + var closed, runner = new $$AnimateRunner(); return { start: run, end: run @@ -63,16 +34,16 @@ var $CoreAnimateCssProvider = function() { function run() { $$rAF(function() { - close(); + applyAnimationContents(); if (!closed) { - runner.done(); + runner.complete(); } closed = true; }); return runner; } - function close() { + function applyAnimationContents() { if (options.addClass) { element.addClass(options.addClass); options.addClass = null; diff --git a/src/ng/animateRunner.js b/src/ng/animateRunner.js new file mode 100644 index 000000000000..51701b4c16f8 --- /dev/null +++ b/src/ng/animateRunner.js @@ -0,0 +1,170 @@ +'use strict'; + +var $$AnimateAsyncRunFactoryProvider = function() { + this.$get = ['$$rAF', function($$rAF) { + var waitQueue = []; + + function waitForTick(fn) { + waitQueue.push(fn); + if (waitQueue.length > 1) return; + $$rAF(function() { + for (var i = 0; i < waitQueue.length; i++) { + waitQueue[i](); + } + waitQueue = []; + }); + } + + return function() { + var passed = false; + waitForTick(function() { + passed = true; + }); + return function(callback) { + passed ? callback() : waitForTick(callback); + }; + }; + }]; +}; + +var $$AnimateRunnerFactoryProvider = function() { + this.$get = ['$q', '$sniffer', '$$animateAsyncRun', + function($q, $sniffer, $$animateAsyncRun) { + + var INITIAL_STATE = 0; + var DONE_PENDING_STATE = 1; + var DONE_COMPLETE_STATE = 2; + + AnimateRunner.chain = function(chain, callback) { + var index = 0; + + next(); + function next() { + if (index === chain.length) { + callback(true); + return; + } + + chain[index](function(response) { + if (response === false) { + callback(false); + return; + } + index++; + next(); + }); + } + }; + + AnimateRunner.all = function(runners, callback) { + var count = 0; + var status = true; + forEach(runners, function(runner) { + runner.done(onProgress); + }); + + function onProgress(response) { + status = status && response; + if (++count === runners.length) { + callback(status); + } + } + }; + + function AnimateRunner(host) { + this.setHost(host); + + this._doneCallbacks = []; + this._runInAnimationFrame = $$animateAsyncRun(); + this._state = 0; + } + + AnimateRunner.prototype = { + setHost: function(host) { + this.host = host || {}; + }, + + done: function(fn) { + if (this._state === DONE_COMPLETE_STATE) { + fn(); + } else { + this._doneCallbacks.push(fn); + } + }, + + progress: noop, + + getPromise: function() { + if (!this.promise) { + var self = this; + this.promise = $q(function(resolve, reject) { + self.done(function(status) { + status === false ? reject() : resolve(); + }); + }); + } + return this.promise; + }, + + then: function(resolveHandler, rejectHandler) { + return this.getPromise().then(resolveHandler, rejectHandler); + }, + + 'catch': function(handler) { + return this.getPromise()['catch'](handler); + }, + + 'finally': function(handler) { + return this.getPromise()['finally'](handler); + }, + + pause: function() { + if (this.host.pause) { + this.host.pause(); + } + }, + + resume: function() { + if (this.host.resume) { + this.host.resume(); + } + }, + + end: function() { + if (this.host.end) { + this.host.end(); + } + this._resolve(true); + }, + + cancel: function() { + if (this.host.cancel) { + this.host.cancel(); + } + this._resolve(false); + }, + + complete: function(response) { + var self = this; + if (self._state === INITIAL_STATE) { + self._state = DONE_PENDING_STATE; + self._runInAnimationFrame(function() { + self._resolve(response); + }); + } + }, + + _resolve: function(response) { + if (this._state !== DONE_COMPLETE_STATE) { + forEach(this._doneCallbacks, function(fn) { + fn(response); + }); + this._doneCallbacks.length = 0; + this._state = DONE_COMPLETE_STATE; + } + } + }; + + return AnimateRunner; + }]; +}; diff --git a/src/ngAnimate/animateRunner.js b/src/ngAnimate/animateRunner.js deleted file mode 100644 index 1a2e6264a5e5..000000000000 --- a/src/ngAnimate/animateRunner.js +++ /dev/null @@ -1,166 +0,0 @@ -'use strict'; - -var $$AnimateAsyncRunFactory = ['$$rAF', function($$rAF) { - var waitQueue = []; - - function waitForTick(fn) { - waitQueue.push(fn); - if (waitQueue.length > 1) return; - $$rAF(function() { - for (var i = 0; i < waitQueue.length; i++) { - waitQueue[i](); - } - waitQueue = []; - }); - } - - return function() { - var passed = false; - waitForTick(function() { - passed = true; - }); - return function(callback) { - passed ? callback() : waitForTick(callback); - }; - }; -}]; - -var $$AnimateRunnerFactory = ['$q', '$sniffer', '$$animateAsyncRun', - function($q, $sniffer, $$animateAsyncRun) { - - var INITIAL_STATE = 0; - var DONE_PENDING_STATE = 1; - var DONE_COMPLETE_STATE = 2; - - AnimateRunner.chain = function(chain, callback) { - var index = 0; - - next(); - function next() { - if (index === chain.length) { - callback(true); - return; - } - - chain[index](function(response) { - if (response === false) { - callback(false); - return; - } - index++; - next(); - }); - } - }; - - AnimateRunner.all = function(runners, callback) { - var count = 0; - var status = true; - forEach(runners, function(runner) { - runner.done(onProgress); - }); - - function onProgress(response) { - status = status && response; - if (++count === runners.length) { - callback(status); - } - } - }; - - function AnimateRunner(host) { - this.setHost(host); - - this._doneCallbacks = []; - this._runInAnimationFrame = $$animateAsyncRun(); - this._state = 0; - } - - AnimateRunner.prototype = { - setHost: function(host) { - this.host = host || {}; - }, - - done: function(fn) { - if (this._state === DONE_COMPLETE_STATE) { - fn(); - } else { - this._doneCallbacks.push(fn); - } - }, - - progress: noop, - - getPromise: function() { - if (!this.promise) { - var self = this; - this.promise = $q(function(resolve, reject) { - self.done(function(status) { - status === false ? reject() : resolve(); - }); - }); - } - return this.promise; - }, - - then: function(resolveHandler, rejectHandler) { - return this.getPromise().then(resolveHandler, rejectHandler); - }, - - 'catch': function(handler) { - return this.getPromise()['catch'](handler); - }, - - 'finally': function(handler) { - return this.getPromise()['finally'](handler); - }, - - pause: function() { - if (this.host.pause) { - this.host.pause(); - } - }, - - resume: function() { - if (this.host.resume) { - this.host.resume(); - } - }, - - end: function() { - if (this.host.end) { - this.host.end(); - } - this._resolve(true); - }, - - cancel: function() { - if (this.host.cancel) { - this.host.cancel(); - } - this._resolve(false); - }, - - complete: function(response) { - var self = this; - if (self._state === INITIAL_STATE) { - self._state = DONE_PENDING_STATE; - self._runInAnimationFrame(function() { - self._resolve(response); - }); - } - }, - - _resolve: function(response) { - if (this._state !== DONE_COMPLETE_STATE) { - forEach(this._doneCallbacks, function(fn) { - fn(response); - }); - this._doneCallbacks.length = 0; - this._state = DONE_COMPLETE_STATE; - } - } - }; - - return AnimateRunner; -}]; diff --git a/src/ngAnimate/module.js b/src/ngAnimate/module.js index 059ebfda4229..ce4e09552cff 100644 --- a/src/ngAnimate/module.js +++ b/src/ngAnimate/module.js @@ -5,7 +5,6 @@ $$AnimateAsyncRunFactory, $$rAFSchedulerFactory, $$AnimateChildrenDirective, - $$AnimateRunnerFactory, $$AnimateQueueProvider, $$AnimationProvider, $AnimateCssProvider, @@ -741,9 +740,6 @@ angular.module('ngAnimate', []) .directive('ngAnimateChildren', $$AnimateChildrenDirective) .factory('$$rAFScheduler', $$rAFSchedulerFactory) - .factory('$$AnimateRunner', $$AnimateRunnerFactory) - .factory('$$animateAsyncRun', $$AnimateAsyncRunFactory) - .provider('$$animateQueue', $$AnimateQueueProvider) .provider('$$animation', $$AnimationProvider) diff --git a/test/ngAnimate/animateRunnerSpec.js b/test/ng/animateRunnerSpec.js similarity index 99% rename from test/ngAnimate/animateRunnerSpec.js rename to test/ng/animateRunnerSpec.js index f4526d8abb57..d6fab470e8df 100644 --- a/test/ngAnimate/animateRunnerSpec.js +++ b/test/ng/animateRunnerSpec.js @@ -1,8 +1,8 @@ 'use strict'; -describe('$$animateAsyncRun', function() { - beforeEach(module('ngAnimate')); +/* jshint newcap: false */ +describe('$$animateAsyncRun', function() { it('should fire the callback only when one or more RAFs have passed', inject(function($$animateAsyncRun, $$rAF) { @@ -32,9 +32,6 @@ describe('$$animateAsyncRun', function() { }); describe("$$AnimateRunner", function() { - - beforeEach(module('ngAnimate')); - they("should trigger the host $prop function", ['end', 'cancel', 'pause', 'resume'], function(method) {