diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index 6d776fb87ac6..69d67927615b 100644 --- a/src/ng/directive/ngOptions.js +++ b/src/ng/directive/ngOptions.js @@ -261,9 +261,13 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { // Get the value by which we are going to track the option // if we have a trackFn then use that (passing scope and locals) // otherwise just hash the given viewValue - var getTrackByValue = trackBy ? - function(viewValue, locals) { return trackByFn(scope, locals); } : - function getHashOfValue(viewValue) { return hashKey(viewValue); }; + var getTrackByValueFn = trackBy ? + function(value, locals) { return trackByFn(scope, locals); } : + function getHashOfValue(value) { return hashKey(value); }; + var getTrackByValue = function(value, key) { + return getTrackByValueFn(value, getLocals(value, key)); + }; + var displayFn = $parse(match[2] || match[1]); var groupByFn = $parse(match[3] || ''); var disableWhenFn = $parse(match[4] || ''); @@ -290,6 +294,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { return { trackBy: trackBy, + getTrackByValue: getTrackByValue, getWatchables: $parse(valuesFn, function(values) { // Create a collection of things that we would like to watch (watchedArray) // so that they can all be watched using a single $watchCollection @@ -299,7 +304,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { Object.keys(values).forEach(function getWatchable(key) { var locals = getLocals(values[key], key); - var selectValue = getTrackByValue(values[key], locals); + var selectValue = getTrackByValueFn(values[key], locals); watchedArray.push(selectValue); // Only need to watch the displayFn if there is a specific label expression @@ -347,7 +352,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { var value = optionValues[key]; var locals = getLocals(value, key); var viewValue = viewValueFn(scope, locals); - var selectValue = getTrackByValue(viewValue, locals); + var selectValue = getTrackByValueFn(viewValue, locals); var label = displayFn(scope, locals); var group = groupByFn(scope, locals); var disabled = disableWhenFn(scope, locals); @@ -361,7 +366,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { items: optionItems, selectValueMap: selectValueMap, getOptionFromViewValue: function(value) { - return selectValueMap[getTrackByValue(value, getLocals(value))]; + return selectValueMap[getTrackByValue(value)]; }, getViewValueFromOption: function(option) { // If the viewValue could be an object that may be mutated by the application, @@ -439,44 +444,54 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { }; - selectCtrl.writeValue = function writeNgOptionsValue(value) { - var option = options.getOptionFromViewValue(value); + // Update the controller methods for multiple selectable options + if (!multiple) { - if (option && !option.disabled) { - if (selectElement[0].value !== option.selectValue) { - removeUnknownOption(); - removeEmptyOption(); + selectCtrl.writeValue = function writeNgOptionsValue(value) { + var option = options.getOptionFromViewValue(value); - selectElement[0].value = option.selectValue; - option.element.selected = true; - option.element.setAttribute('selected', 'selected'); - } - } else { - if (value === null || providedEmptyOption) { - removeUnknownOption(); - renderEmptyOption(); + if (option && !option.disabled) { + if (selectElement[0].value !== option.selectValue) { + removeUnknownOption(); + removeEmptyOption(); + + selectElement[0].value = option.selectValue; + option.element.selected = true; + option.element.setAttribute('selected', 'selected'); + } } else { - removeEmptyOption(); - renderUnknownOption(); + if (value === null || providedEmptyOption) { + removeUnknownOption(); + renderEmptyOption(); + } else { + removeEmptyOption(); + renderUnknownOption(); + } } - } - }; + }; - selectCtrl.readValue = function readNgOptionsValue() { + selectCtrl.readValue = function readNgOptionsValue() { - var selectedOption = options.selectValueMap[selectElement.val()]; + var selectedOption = options.selectValueMap[selectElement.val()]; - if (selectedOption && !selectedOption.disabled) { - removeEmptyOption(); - removeUnknownOption(); - return options.getViewValueFromOption(selectedOption); - } - return null; - }; + if (selectedOption && !selectedOption.disabled) { + removeEmptyOption(); + removeUnknownOption(); + return options.getViewValueFromOption(selectedOption); + } + return null; + }; + // If we are using `track by` then we must watch the tracked value on the model + // since ngModel only watches for object identity change + if (ngOptions.trackBy) { + scope.$watch( + function() { return ngOptions.getTrackByValue(ngModelCtrl.$viewValue); }, + function() { ngModelCtrl.$render(); } + ); + } - // Update the controller methods for multiple selectable options - if (multiple) { + } else { ngModelCtrl.$isEmpty = function(value) { return !value || value.length === 0; @@ -508,6 +523,22 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { return selections; }; + + // If we are using `track by` then we must watch these tracked values on the model + // since ngModel only watches for object identity change + if (ngOptions.trackBy) { + + scope.$watchCollection(function() { + if (isArray(ngModelCtrl.$viewValue)) { + return ngModelCtrl.$viewValue.map(function(value) { + return ngOptions.getTrackByValue(value); + }); + } + }, function() { + ngModelCtrl.$render(); + }); + + } } @@ -534,11 +565,6 @@ 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 - if (ngOptions.trackBy) { - scope.$watchCollection(attr.ngModel, function() { ngModelCtrl.$render(); }); - } // ------------------------------------------------------------------ // diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index d0edd7be8e76..499611758e07 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -988,9 +988,9 @@ describe('ngOptions', function() { expect(element.val()).toEqual(['10']); - // Update the properties on the object in the selected array, rather than replacing the whole object + // Update the tracked property on the object in the selected array, rather than replacing the whole object scope.$apply(function() { - scope.selected[0] = {id: 20, label: 'new twenty'}; + scope.selected[0].id = 20; }); // The value of the select should change since the id property changed @@ -1130,7 +1130,7 @@ describe('ngOptions', function() { }).not.toThrow(); }); - it('should re-render if a propery of the model is changed when using trackBy', function() { + it('should re-render if the tracked property of the model is changed when using trackBy', function() { createSelect({ 'ng-model': 'selected', @@ -1138,13 +1138,13 @@ describe('ngOptions', function() { }); scope.$apply(function() { - scope.selected = scope.arr[0]; + scope.selected = {id: 10, label: 'ten'}; }); spyOn(element.controller('ngModel'), '$render'); scope.$apply(function() { - scope.selected.label = 'changed'; + scope.arr[0].id = 20; }); // update render due to equality watch