From 677f10bac1c3a854d2768d3454b2f15cf76077f9 Mon Sep 17 00:00:00 2001 From: Karl Seamon Date: Tue, 3 Dec 2013 18:13:39 -0500 Subject: [PATCH 01/19] chore($resource): Use shallow copy instead of angular.copy Replace calls to angular.copy with calls to a new function, shallowClearAndCopy. Add calls to copy for cache access in $http in order to prevent modification of cached data. Results in a measurable improvement to the startup time of complex apps within Google. --- src/ng/http.js | 6 +++--- src/ngResource/resource.js | 25 ++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index e7bbb8bc6e93..af2b3c4fd2e9 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -951,9 +951,9 @@ function $HttpProvider() { } else { // serving from cache if (isArray(cachedResp)) { - resolvePromise(cachedResp[1], cachedResp[0], copy(cachedResp[2])); + resolvePromise(copy(cachedResp[1]), cachedResp[0], copy(cachedResp[2])); } else { - resolvePromise(cachedResp, 200, {}); + resolvePromise(copy(cachedResp), 200, {}); } } } else { @@ -980,7 +980,7 @@ function $HttpProvider() { function done(status, response, headersString) { if (cache) { if (isSuccess(status)) { - cache.put(url, [status, response, parseHeaders(headersString)]); + cache.put(url, [status, copy(response), parseHeaders(headersString)]); } else { // remove promise from the cache cache.remove(url); diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index e2499864ff6f..6d7deafd19cc 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -24,6 +24,25 @@ function lookupDottedPath(obj, path) { return obj; } +/** + * Create a shallow copy of an object and clear other fields from the destination + */ +function shallowClearAndCopy(src, dst) { + dst = dst || {}; + + angular.forEach(dst, function(value, key){ + delete dst[key]; + }); + + for (var key in src) { + if (src.hasOwnProperty(key) && key.substr(0, 2) !== '$$') { + dst[key] = src[key]; + } + } + + return dst; +} + /** * @ngdoc overview * @name ngResource @@ -393,7 +412,7 @@ angular.module('ngResource', ['ng']). } function Resource(value){ - copy(value || {}, this); + shallowClearAndCopy(value || {}, this); } forEach(actions, function(action, name) { @@ -465,7 +484,7 @@ angular.module('ngResource', ['ng']). if (data) { // Need to convert action.isArray to boolean in case it is undefined // jshint -W018 - if ( angular.isArray(data) !== (!!action.isArray) ) { + if (angular.isArray(data) !== (!!action.isArray) ) { throw $resourceMinErr('badcfg', 'Error in resource configuration. Expected ' + 'response to contain an {0} but got an {1}', action.isArray?'array':'object', angular.isArray(data)?'array':'object'); @@ -477,7 +496,7 @@ angular.module('ngResource', ['ng']). value.push(new Resource(item)); }); } else { - copy(data, value); + shallowClearAndCopy(data, value); value.$promise = promise; } } From 4797ba74d16603e8fd7ded8e0f1818f711340c0f Mon Sep 17 00:00:00 2001 From: Karl Seamon Date: Wed, 4 Dec 2013 15:11:09 -0500 Subject: [PATCH 02/19] Reverts http.js --- src/ng/http.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index af2b3c4fd2e9..e7bbb8bc6e93 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -951,9 +951,9 @@ function $HttpProvider() { } else { // serving from cache if (isArray(cachedResp)) { - resolvePromise(copy(cachedResp[1]), cachedResp[0], copy(cachedResp[2])); + resolvePromise(cachedResp[1], cachedResp[0], copy(cachedResp[2])); } else { - resolvePromise(copy(cachedResp), 200, {}); + resolvePromise(cachedResp, 200, {}); } } } else { @@ -980,7 +980,7 @@ function $HttpProvider() { function done(status, response, headersString) { if (cache) { if (isSuccess(status)) { - cache.put(url, [status, copy(response), parseHeaders(headersString)]); + cache.put(url, [status, response, parseHeaders(headersString)]); } else { // remove promise from the cache cache.remove(url); From e210f9139956e3da15ee6b0e7b00d0ed10571434 Mon Sep 17 00:00:00 2001 From: Karl Seamon Date: Wed, 4 Dec 2013 15:44:32 -0500 Subject: [PATCH 03/19] Removes a white space, per review. --- src/ngResource/resource.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 6d7deafd19cc..8e63c4e8f4cd 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -484,7 +484,7 @@ angular.module('ngResource', ['ng']). if (data) { // Need to convert action.isArray to boolean in case it is undefined // jshint -W018 - if (angular.isArray(data) !== (!!action.isArray) ) { + if (angular.isArray(data) !== (!!action.isArray)) { throw $resourceMinErr('badcfg', 'Error in resource configuration. Expected ' + 'response to contain an {0} but got an {1}', action.isArray?'array':'object', angular.isArray(data)?'array':'object'); From fbc5cf514bea49f12c1ff1ff331c7cf54e92b670 Mon Sep 17 00:00:00 2001 From: Elwin Arens Date: Tue, 26 Nov 2013 22:54:38 +0100 Subject: [PATCH 04/19] docs(tutorial/step-12): fix refernce to incorrect jquery version Closes #5156 --- docs/content/tutorial/step_12.ngdoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/content/tutorial/step_12.ngdoc b/docs/content/tutorial/step_12.ngdoc index bd4333e4c012..1fabfd1c1b8c 100644 --- a/docs/content/tutorial/step_12.ngdoc +++ b/docs/content/tutorial/step_12.ngdoc @@ -43,7 +43,7 @@ __`app/index.html`.__ 
 ...
   
-  
+  
 
   
   
@@ -56,6 +56,10 @@ __`app/index.html`.__
 ...
+
+ **Important:** Be sure to use jQuery version `1.10.x`. AngularJS does not yet support jQuery `2.x`. +
+ Animations can now be created within the CSS code (`animations.css`) as well as the JavaScript code (`animations.js`). But before we start, let's create a new module which uses the ngAnimate module as a dependency just like we did before with `ngResource`. @@ -383,10 +387,6 @@ isn't required to do JavaScript animations with AngularJS, but we're going to us your own JavaScript animation library is beyond the scope of this tutorial. For more on `jQuery.animate`, see the {@link http://api.jquery.com/animate/ jQuery documentation}. -
- **Important:** Be sure to use jQuery version `1.10.x`. AngularJS does not yet support jQuery `2.x`. -
- The `addClass` and `removeClass` callback functions are called whenever an a class is added or removed on the element that contains the class we registered, which is in this case `.phone`. When the `.active` class is added to the element (via the `ng-class` directive) the `addClass` JavaScript callback will From 280b5ce3c069dc55d2c101a533390a1cdaddf493 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 27 Nov 2013 10:58:20 -0600 Subject: [PATCH 05/19] chore(closure): add `$routeProvider#redirectTo` function parameters Closes #5173 --- closure/angular.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/closure/angular.js b/closure/angular.js index 0e520c535aca..d39b2ab876f3 100644 --- a/closure/angular.js +++ b/closure/angular.js @@ -1709,7 +1709,8 @@ angular.$routeProvider.when = function(path, route) {}; * resolve: (Object.|angular.$q.Promise * )>|undefined), - * redirectTo: (string|function()|undefined), + * redirectTo: ( + * string|function(Object., string, Object): string|undefined), * reloadOnSearch: (boolean|undefined) * }} */ @@ -1732,7 +1733,7 @@ angular.$routeProvider.Params.templateUrl; */ angular.$routeProvider.Params.resolve; -/** @type {string|function()} */ +/** @type {string|function(Object., string, Object): string} */ angular.$routeProvider.Params.redirectTo; /** @type {boolean} */ From 1e7675ad4c6dd472755be65fb21c1788afec5856 Mon Sep 17 00:00:00 2001 From: Julien Bouquillon Date: Thu, 28 Nov 2013 00:38:47 +0100 Subject: [PATCH 06/19] docs(input): remove deprecated isolated scope pitfall The 1.2 release fixed the documented pitfall at 909cabd36d779598763cc358979ecd85bb40d4d7 by isolating only the isolated directive's scope. Closes #5179 --- src/ng/directive/input.js | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 8307f7e816e8..1d8d14eef6e5 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -951,39 +951,6 @@ var VALID_CLASS = 'ng-valid', * * - * ## Isolated Scope Pitfall - * - * Note that if you have a directive with an isolated scope, you cannot require `ngModel` - * since the model value will be looked up on the isolated scope rather than the outer scope. - * When the directive updates the model value, calling `ngModel.$setViewValue()` the property - * on the outer scope will not be updated. However you can get around this by using $parent. - * - * Here is an example of this situation. You'll notice that the first div is not updating the input. - * However the second div can update the input properly. - * - * - - angular.module('badIsolatedDirective', []).directive('isolate', function() { - return { - require: 'ngModel', - scope: { }, - template: '', - link: function(scope, element, attrs, ngModel) { - scope.$watch('innerModel', function(value) { - console.log(value); - ngModel.$setViewValue(value); - }); - } - }; - }); - - - -
-
- - * - * * */ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$parse', From b38a2287f239f6b31a7476e6767bc66dafb2a693 Mon Sep 17 00:00:00 2001 From: Iwona Lalik Date: Thu, 28 Nov 2013 10:24:41 +0100 Subject: [PATCH 07/19] docs(tutorial/step-3): add module to `ng-app` directive in code sample Closes #5184 --- docs/content/tutorial/step_03.ngdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/tutorial/step_03.ngdoc b/docs/content/tutorial/step_03.ngdoc index a26a43a189b9..618d2e0682f7 100644 --- a/docs/content/tutorial/step_03.ngdoc +++ b/docs/content/tutorial/step_03.ngdoc @@ -154,7 +154,7 @@ really is that easy to set up any functional, readable, end-to-end test. `ngController` declaration to the HTML element because it is the common parent of both the body and title elements: - + Be sure to __remove__ the `ng-controller` declaration from the body element. From e8f4305e9d1587512dbef44733fd162caa1cef59 Mon Sep 17 00:00:00 2001 From: Sorin Gitlan Date: Thu, 28 Nov 2013 12:14:51 +0200 Subject: [PATCH 08/19] docs($interpolate): demonstrate a filter in the interpolated expression Closes #5186 --- src/ng/interpolate.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index 43593f91fc14..b78f44a4c9c5 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -103,8 +103,8 @@ function $InterpolateProvider() { * 
          var $interpolate = ...; // injected
-         var exp = $interpolate('Hello {{name}}!');
-         expect(exp({name:'Angular'}).toEqual('Hello Angular!');
+         var exp = $interpolate('Hello {{name | uppercase}}!');
+         expect(exp({name:'Angular'}).toEqual('Hello ANGULAR!');
* * From d802ed1b3680cfc1751777fac465b92ee29944dc Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Wed, 4 Dec 2013 14:33:58 -0800 Subject: [PATCH 09/19] fix($rootScope): broadcast $destroy event on $rootScope Fixes #5169 --- src/ng/rootScope.js | 3 ++- test/ng/rootScopeSpec.js | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 13ee4c70a0b1..a54fdc98fc1e 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -670,11 +670,12 @@ function $RootScopeProvider(){ */ $destroy: function() { // we can't destroy the root scope or a scope that has been already destroyed - if ($rootScope == this || this.$$destroyed) return; + if (this.$$destroyed) return; var parent = this.$parent; this.$broadcast('$destroy'); this.$$destroyed = true; + if (this === $rootScope) return; if (parent.$$childHead == this) parent.$$childHead = this.$$nextSibling; if (parent.$$childTail == this) parent.$$childTail = this.$$prevSibling; diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 69447a3c20b9..287b535634da 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -599,10 +599,14 @@ describe('Scope', function() { })); - it('should ignore remove on root', inject(function($rootScope) { + it('should broadcast $destroy on rootScope', inject(function($rootScope) { + var spy = spyOn(angular, 'noop'); + $rootScope.$on('$destroy', angular.noop); $rootScope.$destroy(); $rootScope.$digest(); expect(log).toEqual('123'); + expect(spy).toHaveBeenCalled(); + expect($rootScope.$$destroyed).toBe(true); })); From 93901bdde4bb9f0ba114ebb33b8885808e1823e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 27 Nov 2013 23:18:51 -0500 Subject: [PATCH 10/19] fix($animate): ensure ms durations are properly rounded Closes #5113 Closes #5162 --- src/ngAnimate/animate.js | 8 +++++++- test/ngAnimate/animateSpec.js | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index bbe056ed45fa..18796ba9f216 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -865,6 +865,7 @@ angular.module('ngAnimate', ['ng']) var NG_ANIMATE_CSS_DATA_KEY = '$$ngAnimateCSS3Data'; var NG_ANIMATE_FALLBACK_CLASS_NAME = 'ng-animate-start'; var NG_ANIMATE_FALLBACK_ACTIVE_CLASS_NAME = 'ng-animate-active'; + var ELAPSED_TIME_MAX_DECIMAL_PLACES = 3; var lookupCache = {}; var parentCounter = 0; @@ -1118,6 +1119,11 @@ angular.module('ngAnimate', ['ng']) event.stopPropagation(); var ev = event.originalEvent || event; var timeStamp = ev.$manualTimeStamp || ev.timeStamp || Date.now(); + + /* Firefox (or possibly just Gecko) likes to not round values up + * when a ms measurement is used for the animation */ + var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES)); + /* $manualTimeStamp is a mocked timeStamp value which is set * within browserTrigger(). This is only here so that tests can * mock animations properly. Real events fallback to event.timeStamp, @@ -1125,7 +1131,7 @@ angular.module('ngAnimate', ['ng']) * We're checking to see if the timeStamp surpasses the expected delay, * but we're using elapsedTime instead of the timeStamp on the 2nd * pre-condition since animations sometimes close off early */ - if(Math.max(timeStamp - startTime, 0) >= maxDelayTime && ev.elapsedTime >= maxDuration) { + if(Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) { activeAnimationComplete(); } } diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index b4635bd0907c..b3068470e3ed 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -2852,5 +2852,26 @@ describe("ngAnimate", function() { $timeout.flush(); }); }); + + it('should round up long elapsedTime values to close off a CSS3 animation', + inject(function($rootScope, $compile, $rootElement, $document, $animate, $sniffer, $timeout, $window) { + if (!$sniffer.animations) return; + + ss.addRule('.millisecond-transition.ng-leave', '-webkit-transition:510ms linear all;' + + 'transition:510ms linear all;'); + + var element = $compile('
')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + $animate.leave(element); + $rootScope.$digest(); + + $timeout.flush(); + + browserTrigger(element, 'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 0.50999999991 }); + + expect($rootElement.children().length).toBe(0); + })); }); }); From b6d5439343b9801f7f2a009d0de09cba9aa21a1d Mon Sep 17 00:00:00 2001 From: Daniel Tabuenca Date: Wed, 4 Dec 2013 16:25:49 -0800 Subject: [PATCH 11/19] fix(input): ensure ngModelWatch() triggers second digest pass when appropriate Due to an earlier change, ngModelWatch() no longer returns a value to the caller. This means the digest loop has no way to tell if the watch actually modified anything and so can not schedule another pass. This means any watches that watch form or model controller changes (e.g. watches on form.$valid) that are scheduled prior to an ngModelWatch() will not be able to see any changes made therin. This commit fixes this behavior by returning the latest evaluated ng-model value. Closes #5258 Closes #5282 --- src/ng/directive/input.js | 4 +++- test/ng/directive/inputSpec.js | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 1d8d14eef6e5..9ff50364ac97 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1097,7 +1097,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * It will update the $viewValue, then pass this value through each of the functions in `$parsers`, * which includes any validators. The value that comes out of this `$parsers` pipeline, be applied to * `$modelValue` and the **expression** specified in the `ng-model` attribute. - * + * * Lastly, all the registered change listeners, in the `$viewChangeListeners` list, are called. * * Note that calling this function does not trigger a `$digest`. @@ -1154,6 +1154,8 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ ctrl.$render(); } } + + return value; }); }]; diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 892c1b7f534d..c568e807d112 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -383,6 +383,29 @@ describe('ngModel', function() { dealoc(element); }); }); + + it('should keep previously defined watches consistent when changes in validity are made', + inject(function($compile, $rootScope) { + + var isFormValid; + $rootScope.$watch('myForm.$valid', function(value) { isFormValid = value; }); + + var element = $compile('
' + + '' + + '
')($rootScope); + + $rootScope.$apply(); + expect(isFormValid).toBe(false); + expect($rootScope.myForm.$valid).toBe(false); + + $rootScope.value='value'; + $rootScope.$apply(); + expect(isFormValid).toBe(true); + expect($rootScope.myForm.$valid).toBe(true); + + dealoc(element); + })); + }); From 21e48abbc1e75ace1b6b99c7b19f538d4e792068 Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Wed, 4 Dec 2013 22:53:28 -0800 Subject: [PATCH 12/19] chore(travis): move checks from before_scripts to scripts If jshint (or any other ci-check) fails, Travis marks the build as "Errored" which I don't think is desider: https://travis-ci.org/angular/angular.js/builds/14938896 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index b8d925b15d37..38ac9be188db 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,10 +18,10 @@ before_script: - grunt bower - grunt bower - grunt package-without-bower - - grunt ci-checks - ./lib/sauce/sauce_connect_block.sh script: + - grunt ci-checks - ./travis_build.sh after_script: From 0e50810c53428f4c1f5bfdba9599df54cb7a6c6e Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Fri, 29 Nov 2013 21:02:00 -0500 Subject: [PATCH 13/19] fix(ngInit): evaluate ngInit before ngInclude The priority of ngInit is adjusted to occur before ngInclude, and after ngController. This enables ngInit to initiallize values in a controller's scope, and also to initiallize values before ngInclude executes. Closes #5167 Closes #5208 --- src/ng/directive/ngInit.js | 3 +++ test/ng/directive/ngInitSpec.js | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/ng/directive/ngInit.js b/src/ng/directive/ngInit.js index d3d0f3c1931c..934b31cca1e2 100644 --- a/src/ng/directive/ngInit.js +++ b/src/ng/directive/ngInit.js @@ -16,6 +16,8 @@ * to initialize values on a scope. * * + * @priority 450 + * * @element ANY * @param {expression} ngInit {@link guide/expression Expression} to eval. * @@ -47,6 +49,7 @@ */ var ngInitDirective = ngDirective({ + priority: 450, compile: function() { return { pre: function(scope, element, attrs) { diff --git a/test/ng/directive/ngInitSpec.js b/test/ng/directive/ngInitSpec.js index 00038621a495..9ed930ad39eb 100644 --- a/test/ng/directive/ngInitSpec.js +++ b/test/ng/directive/ngInitSpec.js @@ -13,4 +13,30 @@ describe('ngInit', function() { element = $compile('
')($rootScope); expect($rootScope.a).toEqual(123); })); + + + it("should be evaluated before ngInclude", inject(function($rootScope, $templateCache, $compile) { + $templateCache.put('template1.tpl', '1'); + $templateCache.put('template2.tpl', '2'); + $rootScope.template = 'template1.tpl'; + element = $compile('
')($rootScope); + $rootScope.$digest(); + expect($rootScope.template).toEqual('template2.tpl'); + expect(element.find('span').text()).toEqual('2'); + })); + + + it("should be evaluated after ngController", function() { + module(function($controllerProvider) { + $controllerProvider.register('TestCtrl', function($scope) {}); + }); + inject(function($rootScope, $compile) { + element = $compile('
')($rootScope); + $rootScope.$digest(); + expect($rootScope.test).toBeUndefined(); + expect(element.children('div').scope().test).toEqual(123); + }); + }); }); From 958d3d56b1899a2cfc7b18c0292e5a1d8c64d0a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 4 Dec 2013 12:49:02 -0500 Subject: [PATCH 14/19] fix($animate): ensure animations work with directives that share a transclusion Closes #4716 Closes #4871 Closes #5021 Closes #5278 --- src/ngAnimate/animate.js | 61 ++++++++++++++++++++++++----------- test/ngAnimate/animateSpec.js | 53 ++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 19 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 18796ba9f216..8ff7b4298725 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -258,6 +258,19 @@ angular.module('ngAnimate', ['ng']) var NG_ANIMATE_CLASS_NAME = 'ng-animate'; var rootAnimateState = {running: true}; + function extractElementNode(element) { + for(var i = 0; i < element.length; i++) { + var elm = element[i]; + if(elm.nodeType == ELEMENT_NODE) { + return elm; + } + } + } + + function isMatchingElement(elm1, elm2) { + return extractElementNode(elm1) == extractElementNode(elm2); + } + $provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout', '$rootScope', '$document', function($delegate, $injector, $sniffer, $rootElement, $timeout, $rootScope, $document) { @@ -556,7 +569,16 @@ angular.module('ngAnimate', ['ng']) and the onComplete callback will be fired once the animation is fully complete. */ function performAnimation(animationEvent, className, element, parentElement, afterElement, domOperation, doneCallback) { - var currentClassName = element.attr('class') || ''; + var node = extractElementNode(element); + //transcluded directives may sometimes fire an animation using only comment nodes + //best to catch this early on to prevent any animation operations from occurring + if(!node) { + fireDOMOperation(); + closeAnimation(); + return; + } + + var currentClassName = node.className; var classes = currentClassName + ' ' + className; var animationLookup = (' ' + classes).replace(/\s+/g,'.'); if (!parentElement) { @@ -760,11 +782,7 @@ angular.module('ngAnimate', ['ng']) } function cancelChildAnimations(element) { - var node = element[0]; - if(node.nodeType != ELEMENT_NODE) { - return; - } - + var node = extractElementNode(element); forEach(node.querySelectorAll('.' + NG_ANIMATE_CLASS_NAME), function(element) { element = angular.element(element); var data = element.data(NG_ANIMATE_STATE); @@ -788,7 +806,7 @@ angular.module('ngAnimate', ['ng']) } function cleanup(element) { - if(element[0] == $rootElement[0]) { + if(isMatchingElement(element, $rootElement)) { if(!rootAnimateState.disabled) { rootAnimateState.running = false; rootAnimateState.structural = false; @@ -802,7 +820,7 @@ angular.module('ngAnimate', ['ng']) function animationsDisabled(element, parentElement) { if (rootAnimateState.disabled) return true; - if(element[0] == $rootElement[0]) { + if(isMatchingElement(element, $rootElement)) { return rootAnimateState.disabled || rootAnimateState.running; } @@ -812,7 +830,7 @@ angular.module('ngAnimate', ['ng']) //any animations on it if(parentElement.length === 0) break; - var isRoot = parentElement[0] == $rootElement[0]; + var isRoot = isMatchingElement(parentElement, $rootElement); var state = isRoot ? rootAnimateState : parentElement.data(NG_ANIMATE_STATE); var result = state && (!!state.disabled || !!state.running); if(isRoot || result) { @@ -960,7 +978,7 @@ angular.module('ngAnimate', ['ng']) parentElement.data(NG_ANIMATE_PARENT_KEY, ++parentCounter); parentID = parentCounter; } - return parentID + '-' + element[0].className; + return parentID + '-' + extractElementNode(element).className; } function animateSetup(element, className) { @@ -995,7 +1013,6 @@ angular.module('ngAnimate', ['ng']) return false; } - 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). var activeClassName = ''; @@ -1025,35 +1042,37 @@ angular.module('ngAnimate', ['ng']) } function blockTransitions(element) { - element[0].style[TRANSITION_PROP + PROPERTY_KEY] = 'none'; + extractElementNode(element).style[TRANSITION_PROP + PROPERTY_KEY] = 'none'; } function blockKeyframeAnimations(element) { - element[0].style[ANIMATION_PROP] = 'none 0s'; + extractElementNode(element).style[ANIMATION_PROP] = 'none 0s'; } function unblockTransitions(element) { - var node = element[0], prop = TRANSITION_PROP + PROPERTY_KEY; + var prop = TRANSITION_PROP + PROPERTY_KEY; + var node = extractElementNode(element); if(node.style[prop] && node.style[prop].length > 0) { node.style[prop] = ''; } } function unblockKeyframeAnimations(element) { - var node = element[0], prop = ANIMATION_PROP; + var prop = ANIMATION_PROP; + var node = extractElementNode(element); if(node.style[prop] && node.style[prop].length > 0) { - element[0].style[prop] = ''; + node.style[prop] = ''; } } function animateRun(element, className, activeAnimationComplete) { var data = element.data(NG_ANIMATE_CSS_DATA_KEY); - if(!element.hasClass(className) || !data) { + var node = extractElementNode(element); + if(node.className.indexOf(className) == -1 || !data) { activeAnimationComplete(); return; } - var node = element[0]; var timings = data.timings; var stagger = data.stagger; var maxDuration = data.maxDuration; @@ -1096,6 +1115,9 @@ angular.module('ngAnimate', ['ng']) } if(appliedStyles.length > 0) { + //the element being animated may sometimes contain comment nodes in + //the jqLite object, so we're safe to use a single variable to house + //the styles since there is always only one element being animated var oldStyle = node.getAttribute('style') || ''; node.setAttribute('style', oldStyle + ' ' + style); } @@ -1110,6 +1132,7 @@ angular.module('ngAnimate', ['ng']) element.off(css3AnimationEvents, onAnimationProgress); element.removeClass(activeClassName); animateClose(element, className); + var node = extractElementNode(element); for (var i in appliedStyles) { node.style.removeProperty(appliedStyles[i]); } @@ -1209,7 +1232,7 @@ angular.module('ngAnimate', ['ng']) } var parentElement = element.parent(); - var clone = angular.element(element[0].cloneNode()); + var clone = angular.element(extractElementNode(element).cloneNode()); //make the element super hidden and override any CSS style values clone.attr('style','position:absolute; top:-9999px; left:-9999px'); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index b3068470e3ed..44b623b43ef1 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -2873,5 +2873,58 @@ describe("ngAnimate", function() { expect($rootElement.children().length).toBe(0); })); + + it('should properly animate elements with compound directives', function() { + var capturedAnimation; + module(function($animateProvider) { + $animateProvider.register('.special', function() { + return { + enter : function(element, done) { + capturedAnimation = 'enter'; + done(); + }, + leave : function(element, done) { + capturedAnimation = 'leave'; + done(); + } + } + }); + }); + inject(function($rootScope, $compile, $rootElement, $document, $timeout, $templateCache, $sniffer) { + if(!$sniffer.transitions) return; + + $templateCache.put('item-template', 'item: #{{ item }} '); + var element = $compile('
' + + '
' + + '
')($rootScope); + + ss.addRule('.special', '-webkit-transition:1s linear all;' + + 'transition:1s linear all;'); + + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + $rootScope.tpl = 'item-template'; + $rootScope.items = [1,2,3]; + $rootScope.$digest(); + $timeout.flush(); + + expect(capturedAnimation).toBe('enter'); + expect(element.text()).toContain('item: #1'); + + forEach(element.children(), function(kid) { + browserTrigger(kid, 'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1 }); + }); + $timeout.flush(); + + $rootScope.items = []; + $rootScope.$digest(); + $timeout.flush(); + + expect(capturedAnimation).toBe('leave'); + }); + }); }); }); From 04a570d31cf05731e44a8bd9bde0fcc9c0fbff86 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Wed, 4 Dec 2013 17:52:17 -0800 Subject: [PATCH 15/19] docs(TRIAGING): Initial doc about triaging issues in Angular --- TRIAGING.md | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 TRIAGING.md diff --git a/TRIAGING.md b/TRIAGING.md new file mode 100644 index 000000000000..ec7ac6e89a95 --- /dev/null +++ b/TRIAGING.md @@ -0,0 +1,55 @@ +# Triage new issues/PRs on github + +This document shows the steps the Angular team is using to triage issues. +The labels are used later on for planning releases. + +## Tips ## + +* install [github pr helper extension](https://github.com/petebacondarwin/github-pr-helper) and become 356% more productive +* Label "resolution:*" + * these tags can be used for labeling a closed issue/PR with a reason why it was closed. (we can add reasons as we need them, right there are only a few rejection reasons. it doesn't make sense to label issues that were fixed or prs that were merged) + + +## Process ## + +1. Open list of [non triaged issues](https://github.com/angular/angular.js/issues?direction=desc&milestone=none&page=1&sort=created&state=open) +1. Assign yourself: Pick an issue that is not assigned to anyone and assign it to you +1. Assign milestone: + * "Docs only" milestone - for documentation PR -> **Done**. + * Current/next milestone - regressions + * 1.2.x - everything else +1. Label "GH: *" (to be automated via Mary Poppins) + * PR - issue is a PR + * issue - otherwise +1. Bugs: + * Label "Type: Bug" + * Label "Type: Regression" - if the bug is a regression + * Duplicate? - Check if there are comments pointing out that this is a dupe, if they do exist verify that this is indeed a dupe and close it and go to the last step + * Reproducible? - Steps to reproduce the bug are clear, if not ask for clarification (ideally plunker or fiddle) + * Reproducible on master? - http://code.angularjs.org/snapshot/ + +1. Non bugs: + * Label "Type: Feature" or "Type: Chore" + * Label "needs: breaking change" - if needed + * Understandable? - verify if the description of the request is clear. if not ask for clarification + * Goals of angular core? - Often new features should be implemented as a third-party module rather than an addition to the core. + +1. Label "component: *" + * In rare cases, it's ok to have multiple components. +1. Label "impact: *" + * small - obscure issue affecting one or handful of developers + * medium - impacts some usage patterns + * large - impacts most or all of angular apps +1. Label "complexity: *" + * small - trivial change + * medium - non-trivial but straightforward change + * large - changes to many components in angular or any changes to $compile, ngRepeat or other "fun" components +1. Label "PRs welcome" for "GH: issue" + * if complexity is small or medium and the problem as well as solution are well captured in the issue +1. Label "cla: yes" for "GH: PR": + * otherwise prompt the contributor to sign the CLA +1. Label "origin: google" for issues from Google +1. Label "high priority" for security issues, major performance regressions or memory leaks + +1. Unassign yourself from the issue + From 39c5ffb2a61c607810beed8cbb0f8dc19745045f Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 5 Dec 2013 22:06:28 +0000 Subject: [PATCH 16/19] docs(tutorial/step-2): remember to install karma plugins --- docs/content/tutorial/step_02.ngdoc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/content/tutorial/step_02.ngdoc b/docs/content/tutorial/step_02.ngdoc index f1906018046e..2489162c6ad3 100644 --- a/docs/content/tutorial/step_02.ngdoc +++ b/docs/content/tutorial/step_02.ngdoc @@ -184,7 +184,11 @@ http://pivotal.github.com/jasmine/ Jasmine home page} and at the {@link http://pivotal.github.io/jasmine/ Jasmine docs}. The angular-seed project is pre-configured to run all unit tests using {@link -http://karma-runner.github.io/ Karma}. To run the test, do the following: +http://karma-runner.github.io/ Karma}. Ensure that the necessary karma plugins are installed. +You can do this by issuing `npm install` into your terminal. + + +To run the test, do the following: 1. In a _separate_ terminal window or tab, go to the `angular-phonecat` directory and run `./scripts/test.sh` to start the Karma server (the config file necessary to start the server From 2adbcf189b692bfcfa9f99760f2ba9a84eee1e0f Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 5 Dec 2013 22:06:38 +0000 Subject: [PATCH 17/19] docs(tutorial/step-3): remember to install karma plugins --- docs/content/tutorial/step_03.ngdoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/tutorial/step_03.ngdoc b/docs/content/tutorial/step_03.ngdoc index 618d2e0682f7..686f0854c25b 100644 --- a/docs/content/tutorial/step_03.ngdoc +++ b/docs/content/tutorial/step_03.ngdoc @@ -127,8 +127,8 @@ end-to-end tests! Use `./scripts/e2e-test.sh` script for that. End-to-end tests with unit tests, Karma will exit after the test run and will not automatically rerun the test suite on every file change. To rerun the test suite, execute the `e2e-test.sh` script again. -Note: You must ensure you've installed karma-ng-scenario prior to running the `e2e-test.sh` script. -You can do this by issuing `npm install karma-ng-scenario` into your terminal. +Note: You must ensure you've installed the karma-ng-scenario framework plugin prior to running the +`e2e-test.sh` script. You can do this by issuing `npm install` into your terminal. This test verifies that the search box and the repeater are correctly wired together. Notice how easy it is to write end-to-end tests in Angular. Although this example is for a simple test, it From 09648e4888896948b733febb9c8652a36f8a22dd Mon Sep 17 00:00:00 2001 From: Andres Kalle Date: Fri, 29 Nov 2013 13:40:43 +0200 Subject: [PATCH 18/19] docs(tutorial/step-6): remove unused `class="diagram"` Closes #5197 --- docs/content/tutorial/step_06.ngdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/tutorial/step_06.ngdoc b/docs/content/tutorial/step_06.ngdoc index 40e22dfce4cc..eaf0ad3cbbf7 100644 --- a/docs/content/tutorial/step_06.ngdoc +++ b/docs/content/tutorial/step_06.ngdoc @@ -64,7 +64,7 @@ We also added phone images next to each record using an image tag with the {@lin api/ng.directive:ngSrc ngSrc} directive. That directive prevents the browser from treating the angular `{{ expression }}` markup literally, and initiating a request to invalid url `http://localhost:8000/app/{{phone.imageUrl}}`, which it would have done if we had only -specified an attribute binding in a regular `src` attribute (``). +specified an attribute binding in a regular `src` attribute (``). Using the `ngSrc` directive prevents the browser from making an http request to an invalid location. From 1722574f4298559562d7d4977131167f2d760e04 Mon Sep 17 00:00:00 2001 From: Karl Seamon Date: Tue, 3 Dec 2013 18:13:39 -0500 Subject: [PATCH 19/19] chore($resource): Use shallow copy instead of angular.copy Replace calls to angular.copy with calls to a new function, shallowClearAndCopy. Add calls to copy for cache access in $http in order to prevent modification of cached data. Results in a measurable improvement to the startup time of complex apps within Google. --- src/ngResource/resource.js | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index e2499864ff6f..8e63c4e8f4cd 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -24,6 +24,25 @@ function lookupDottedPath(obj, path) { return obj; } +/** + * Create a shallow copy of an object and clear other fields from the destination + */ +function shallowClearAndCopy(src, dst) { + dst = dst || {}; + + angular.forEach(dst, function(value, key){ + delete dst[key]; + }); + + for (var key in src) { + if (src.hasOwnProperty(key) && key.substr(0, 2) !== '$$') { + dst[key] = src[key]; + } + } + + return dst; +} + /** * @ngdoc overview * @name ngResource @@ -393,7 +412,7 @@ angular.module('ngResource', ['ng']). } function Resource(value){ - copy(value || {}, this); + shallowClearAndCopy(value || {}, this); } forEach(actions, function(action, name) { @@ -465,7 +484,7 @@ angular.module('ngResource', ['ng']). if (data) { // Need to convert action.isArray to boolean in case it is undefined // jshint -W018 - if ( angular.isArray(data) !== (!!action.isArray) ) { + if (angular.isArray(data) !== (!!action.isArray)) { throw $resourceMinErr('badcfg', 'Error in resource configuration. Expected ' + 'response to contain an {0} but got an {1}', action.isArray?'array':'object', angular.isArray(data)?'array':'object'); @@ -477,7 +496,7 @@ angular.module('ngResource', ['ng']). value.push(new Resource(item)); }); } else { - copy(data, value); + shallowClearAndCopy(data, value); value.$promise = promise; } }