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

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 #12005 Closes #12582
angular · Aug 31, 2015 · 82b0929 · 82b0929
1 parent 7d2c6ee
commit 82b0929
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 10 deletions.
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>' +