fix(select): update option if interpolated value attribute changes

Narretz · Narretz · commit 82b0929e4ea0 · 2015-08-31T23:18:19.000+02:00
This is for options added without ngOptions. Previously, an option with an interpolated value attribute would not be updated if the binding changed, i.e. the select controller would not recognize the changed option. Now the value attribute will be observed if it contains an interpolation. Closes angular#12005 Closes angular#12582
diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js
@@ -401,9 +401,12 @@ var optionDirective = ['$interpolate', function($interpolate) {
     priority: 100,
     compile: function(element, attr) {
 
-      // If the value attribute is not defined then we fall back to the
-      // text content of the option element, which may be interpolated
-      if (isUndefined(attr.value)) {
+      if (isDefined(attr.value)) {
+        // If the value attribute is defined, check if it contains an interpolation
+        var valueInterpolated = $interpolate(attr.value, true);
+      } else {
+        // If the value attribute is not defined then we fall back to the
+        // text content of the option element, which may be interpolated
         var interpolateFn = $interpolate(element.text(), true);
         if (!interpolateFn) {
           attr.$set('value', element.text());
@@ -419,24 +422,38 @@ var optionDirective = ['$interpolate', function($interpolate) {
             selectCtrl = parent.data(selectCtrlName) ||
               parent.parent().data(selectCtrlName); // in case we are in optgroup
 
+        function addOption(optionValue) {
+          selectCtrl.addOption(optionValue, element);
+          selectCtrl.ngModelCtrl.$render();
+          chromeHack(element);
+        }
+
         // Only update trigger option updates if this is an option within a `select`
         // that also has `ngModel` attached
         if (selectCtrl && selectCtrl.ngModelCtrl) {
 
-          if (interpolateFn) {
+          if (valueInterpolated) {
+            // The value attribute is interpolated
+            var oldVal;
+            attr.$observe('value', function valueAttributeObserveAction(newVal) {
+              if (isDefined(oldVal)) {
+                selectCtrl.removeOption(oldVal);
+              }
+              oldVal = newVal;
+              addOption(newVal);
+            });
+          } else if (interpolateFn) {
+            // The text content is interpolated
             scope.$watch(interpolateFn, function interpolateWatchAction(newVal, oldVal) {
               attr.$set('value', newVal);
               if (oldVal !== newVal) {
                 selectCtrl.removeOption(oldVal);
               }
-              selectCtrl.addOption(newVal, element);
-              selectCtrl.ngModelCtrl.$render();
-              chromeHack(element);
+              addOption(newVal);
             });
           } else {
-            selectCtrl.addOption(attr.value, element);
-            selectCtrl.ngModelCtrl.$render();
-            chromeHack(element);
+            // The value attribute is static
+            addOption(attr.value);
           }
 
           element.on('$destroy', function() {
diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js
@@ -980,6 +980,66 @@ describe('select', function() {
       expect(element).toEqualSelect(['hello']);
     });
 
+
+    it('should add options with interpolated value attributes',
+      inject(function($rootScope, $compile) {
+        var scope = $rootScope;
+
+        scope.option1 = 'option1';
+        scope.option2 = 'option2';
+
+        var element = $compile(
+          '<select ng-model="selected">' +
+            '<option value="{{option1}}">Option 1</option>' +
+            '<option value="{{option2}}">Option 2</option>' +
+          '</div>')(scope);
+
+        scope.$digest();
+        expect(scope.selected).toBeUndefined();
+
+        browserTrigger(element.find('option').eq(0));
+        expect(scope.selected).toBe('option1');
+
+        scope.selected = 'option2';
+        scope.$digest();
+        expect(element.find('option').eq(1).prop('selected')).toBe(true);
+        expect(element.find('option').eq(1).text()).toBe('Option 2');
+      })
+    );
+
+
+    it('should update the option when the interpolated value attribute changes',
+      inject(function($rootScope, $compile) {
+        var scope = $rootScope;
+
+        scope.option1 = 'option1';
+        scope.option2 = '';
+
+        var element = $compile(
+          '<select ng-model="selected">' +
+            '<option value="{{option1}}">Option 1</option>' +
+            '<option value="{{option2}}">Option 2</option>' +
+          '</div>')(scope);
+
+        var selectCtrl = element.controller('select');
+        spyOn(selectCtrl, 'removeOption').andCallThrough();
+
+        scope.$digest();
+        expect(scope.selected).toBeUndefined();
+        expect(selectCtrl.removeOption).not.toHaveBeenCalled();
+
+        //Change value of option2
+        scope.option2 = 'option2Changed';
+        scope.selected = 'option2Changed';
+        scope.$digest();
+
+        expect(selectCtrl.removeOption).toHaveBeenCalledWith('');
+        expect(element.find('option').eq(1).prop('selected')).toBe(true);
+        expect(element.find('option').eq(1).text()).toBe('Option 2');
+      })
+    );
+
+
     it('should not blow up when option directive is found inside of a datalist',
         inject(function($compile, $rootScope) {
       var element = $compile('<div>' +
