From 767ee85e0eab4ad5d9f837170528241728e614b0 Mon Sep 17 00:00:00 2001 From: Gonzalo Ruiz de Villa Date: Tue, 3 Sep 2013 17:12:09 -0500 Subject: [PATCH 1/2] fix(rootScope): $watchCollection returns in oldCollection a copy of the former array data related to #1751 When watching arrays, $watchCollection returned the new data both in the newCollection and the oldCollection arguments of the listener --- src/ng/rootScope.js | 6 +++- test/ng/rootScopeSpec.js | 59 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 8259bf881bd9..95be978e6950 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -415,6 +415,7 @@ function $RootScopeProvider(){ $watchCollection: function(obj, listener) { var self = this; var oldValue; + var oldArray; var newValue; var changeDetected = 0; var objGetter = $parse(obj); @@ -424,6 +425,7 @@ function $RootScopeProvider(){ function $watchCollectionWatch() { newValue = objGetter(self); + oldArray = null; var newLength, key; if (!isObject(newValue)) { @@ -439,6 +441,8 @@ function $RootScopeProvider(){ changeDetected++; } + oldArray = oldValue.length > 0 ? Array.prototype.slice.call(oldValue, 0) : []; + newLength = newValue.length; if (oldLength !== newLength) { @@ -492,7 +496,7 @@ function $RootScopeProvider(){ } function $watchCollectionAction() { - listener(newValue, oldValue, self); + listener(newValue, oldArray || oldValue, self); } return this.$watch($watchCollectionWatch, $watchCollectionAction); diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index e47111e2d0eb..c765640a99e7 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -572,6 +572,65 @@ describe('Scope', function() { $rootScope.$digest(); expect(arrayLikelog).toEqual(['x', 'y']); }); + + it('should return a new array with old values', function(){ + var watchArgs; + $rootScope.$watchCollection('obj', function (newValues, oldValues) { + watchArgs = { + newValues: newValues, + oldValues: oldValues + }; + }); + + $rootScope.obj = ['a']; + $rootScope.$digest(); + + expect(watchArgs.newValues).toEqual($rootScope.obj); + expect(watchArgs.oldValues).toEqual([]); + + $rootScope.obj.push('b'); + $rootScope.$digest(); + + expect(watchArgs.newValues).toEqual(['a', 'b']); + expect(watchArgs.oldValues).toEqual(['a']); + }); + + it('should return a new array with old values from array like objects', function(){ + var watchArgs; + $rootScope.$watchCollection('arrayLikeObject', function (newValues, oldValues) { + watchArgs = { + newValues: [], + oldValues: [] + }; + forEach(newValues, function (element){ + watchArgs.newValues.push(element.name); + }); + forEach(oldValues, function (element){ + watchArgs.oldValues.push(element.name); + }); + }); + + document.body.innerHTML = "

" + + "a" + + "b" + + "c" + + "

"; + + $rootScope.arrayLikeObject = document.getElementsByTagName('a'); + $rootScope.$digest(); + + document.body.innerHTML = "

" + + "a" + + "b" + + "

"; + + $rootScope.arrayLikeObject.length = 2; + $rootScope.$digest(); + + expect(watchArgs.newValues).toEqual(['x', 'y']); + expect(watchArgs.oldValues).toEqual(['x', 'y', 'z']); + }); + }); From 091228fd853a83e70ac1a4bf714166dff7b69a89 Mon Sep 17 00:00:00 2001 From: Gonzalo Ruiz de Villa Date: Tue, 3 Sep 2013 19:16:53 -0500 Subject: [PATCH 2/2] fix(input): ngList is updated when array model values are changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes #1751 When the model referenced the same same array and the array values where changed, the list wasn't updated. Now watchCollection is used to detect changes in NgModelController. Changes in input.js breaked tests: — select select-multiple should require — select ngOptions ngRequired should treat an empty array as invalid when `multiple` attribute used Changes in select.js fixed them again: changes in the collections should trigger the formatters and render again. BEHAVIOUR CHANGE There is a change in the behaviour of ngList when typing a list. When “a , b” is typed is automatically changed to “a, b”. --- src/ng/directive/input.js | 4 ++-- src/ng/directive/select.js | 30 +++++++++++++++++++++++------- test/ng/directive/inputSpec.js | 24 +++++++++++++++++++++++- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 4e77397878dd..ec1c795a5d83 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1097,11 +1097,11 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ // model -> value var ctrl = this; - $scope.$watch(function ngModelWatch() { + $scope.$watchCollection($attr.ngModel, function ngModelWatch(newValue, oldValue) { var value = ngModelGet($scope); // if scope model value and ngModel value are out of sync - if (ctrl.$modelValue !== value) { + if (!equals(ctrl.$modelValue, oldValue)) { var formatters = ctrl.$formatters, idx = formatters.length; diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 3dbb5b8e52d5..4df51ecd0efa 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -262,7 +262,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { } function setupAsMultiple(scope, selectElement, ctrl) { - var lastView; ctrl.$render = function() { var items = new HashMap(ctrl.$viewValue); forEach(selectElement.find('option'), function(option) { @@ -270,13 +269,17 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { }); }; - // we have to do it on each watch since ngModel watches reference, but - // we need to work of an array, so we need to see if anything was inserted/removed - scope.$watch(function selectMultipleWatch() { - if (!equals(lastView, ctrl.$viewValue)) { - lastView = copy(ctrl.$viewValue); - ctrl.$render(); + scope.$watchCollection(attr.ngModel, function selectMultipleWatch(newValue, oldValue) { + var formatters = ctrl.$formatters, + idx = formatters.length, + value = newValue; + + ctrl.$modelValue = value; + while(idx--) { + value = formatters[idx](value); } + ctrl.$viewValue = value; + ctrl.$render(); }); selectElement.on('change', function() { @@ -395,6 +398,19 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { // TODO(vojta): can't we optimize this ? scope.$watch(render); + scope.$watchCollection(attr.ngModel, function selectMultipleWatch(newValue, oldValue) { + var formatters = ctrl.$formatters, + idx = formatters.length, + value = newValue; + + ctrl.$modelValue = value; + while(idx--) { + value = formatters[idx](value); + } + ctrl.$viewValue = value; + render(); + }); + function render() { // Temporary location for the option groups before we render them var optionGroups = {'':[]}, diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 26abceae1cfe..f1734083bae4 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -1181,7 +1181,7 @@ describe('input', function() { expect(scope.list).toEqual(['a']); changeInputValueTo('a , b'); - expect(inputElm.val()).toEqual('a , b'); + expect(inputElm.val()).toEqual('a, b'); expect(scope.list).toEqual(['a', 'b']); }); @@ -1224,6 +1224,28 @@ describe('input', function() { changeInputValueTo('a,b: c'); expect(scope.list).toEqual(['a', 'b', 'c']); }); + + it("should detect changes in the values of an array", function () { + var list = ['x', 'y', 'z']; + compileInput(''); + scope.$apply(function() { + scope.list = list; + }); + expect(inputElm.val()).toBe('x, y, z'); + scope.$apply(function() { + list.unshift('w'); + }); + expect(inputElm.val()).toBe('w, x, y, z'); + }); + + it('should be invalid if empty', function() { + compileInput(''); + changeInputValueTo('a'); + expect(inputElm).toBeValid(); + changeInputValueTo(''); + expect(inputElm).toBeInvalid(); + }); + }); describe('required', function() {