diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index 85a725069ee0..476da491d755 100644 --- a/src/ng/directive/ngOptions.js +++ b/src/ng/directive/ngOptions.js @@ -335,6 +335,11 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { selectValueMap: selectValueMap, getOptionFromViewValue: function(value) { return selectValueMap[getTrackByValue(value, getLocals(value))]; + }, + getViewValueFromOption: function(option) { + // If the viewValue could be an object that may be mutated by the application, + // we need to make a copy and not return the reference to the value on the option. + return trackBy ? angular.copy(option.viewValue) : option.viewValue; } }; } @@ -428,7 +433,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { if (selectedOption && !selectedOption.disabled) { removeEmptyOption(); removeUnknownOption(); - return selectedOption.viewValue; + return options.getViewValueFromOption(selectedOption); } return null; }; @@ -462,7 +467,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { forEach(selectedValues, function(value) { var option = options.selectValueMap[value]; - if (!option.disabled) selections.push(option.viewValue); + if (!option.disabled) selections.push(options.getViewValueFromOption(option)); }); return selections; @@ -493,6 +498,10 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { // We will re-render the option elements if the option values or labels change scope.$watchCollection(ngOptions.getWatchables, updateOptions); + // We also need to watch to see if the internals of the model changes, since + // ngModel only watches for object identity change + scope.$watch(attr.ngModel, function() { ngModelCtrl.$render(); }, true); + // ------------------------------------------------------------------ // diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index 62b3be328734..ceb3e312f494 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -362,6 +362,7 @@ describe('ngOptions', function() { expect(options.eq(2)).toEqualOption(scope.values[2], 'D'); }); + it('should preserve pre-existing empty option', function() { createSingleSelect(true); @@ -855,6 +856,87 @@ describe('ngOptions', function() { expect(options.eq(2)).toEqualTrackedOption(20, 'twenty'); }); + + it('should update the selected option even if only the tracked property on the selected object changes (single)', function() { + createSelect({ + 'ng-model': 'selected', + 'ng-options': 'item.label for item in arr track by item.id' + }); + + scope.$apply(function() { + scope.selected = {id: 10, label: 'ten'}; + }); + + expect(element.val()).toEqual('10'); + + // Update the properties on the selected object, rather than replacing the whole object + scope.$apply(function() { + scope.selected.id = 20; + scope.selected.label = 'new twenty'; + }); + + // The value of the select should change since the id property changed + expect(element.val()).toEqual('20'); + + // But the label of the selected option does not change + var option = element.find('option').eq(1); + expect(option.prop('selected')).toEqual(true); + expect(option.text()).toEqual('twenty'); // not 'new twenty' + }); + + + it('should update the selected options even if only the tracked properties on the objects in the ' + + 'selected collection change (multi)', function() { + createSelect({ + 'ng-model': 'selected', + 'multiple': true, + 'ng-options': 'item.label for item in arr track by item.id' + }); + + scope.$apply(function() { + scope.selected = [{id: 10, label: 'ten'}]; + }); + + expect(element.val()).toEqual(['10']); + + // Update the properties on the object in the selected array, rather than replacing the whole object + scope.$apply(function() { + scope.selected[0].id = 20; + scope.selected[0].label = 'new twenty'; + }); + + // The value of the select should change since the id property changed + expect(element.val()).toEqual(['20']); + + // But the label of the selected option does not change + var option = element.find('option').eq(1); + expect(option.prop('selected')).toEqual(true); + expect(option.text()).toEqual('twenty'); // not 'new twenty' + }); + + + it('should prevent changes to the selected object from modifying the options objects (single)', function() { + + createSelect({ + 'ng-model': 'selected', + 'ng-options': 'item.label for item in arr track by item.id' + }); + + element.val('10'); + browserTrigger(element, 'change'); + + expect(scope.selected).toEqual(scope.arr[0]); + + scope.$apply(function() { + scope.selected.id = 20; + }); + + expect(scope.selected).not.toEqual(scope.arr[0]); + expect(element.val()).toEqual('20'); + expect(scope.arr).toEqual([{id: 10, label: 'ten'}, {id:20, label: 'twenty'}]); + }); + + it('should preserve value even when reference has changed (single&array)', function() { createSelect({ 'ng-model': 'selected', @@ -917,7 +999,7 @@ describe('ngOptions', function() { expect(element.val()).toBe('10'); setSelectValue(element, 1); - expect(scope.selected).toBe(scope.obj['2']); + expect(scope.selected).toEqual(scope.obj['2']); }); @@ -996,7 +1078,7 @@ describe('ngOptions', function() { element.val('10'); browserTrigger(element, 'change'); - expect(scope.selected).toBe(scope.arr[0].subItem); + expect(scope.selected).toEqual(scope.arr[0].subItem); // Now reload the array scope.$apply(function() { @@ -1575,7 +1657,7 @@ describe('ngOptions', function() { scope.values.pop(); }); - expect(element.val()).toEqualUnknownValue(); + expect(element.val()).toEqual(''); expect(scope.selected).toEqual(null); // Check after model change @@ -1589,7 +1671,7 @@ describe('ngOptions', function() { scope.values.pop(); }); - expect(element.val()).toEqualUnknownValue(); + expect(element.val()).toEqual(''); expect(scope.selected).toEqual(null); });