fix(ngOptions): unset the select.ngModelCtrl when ngOptions is present

angular · Sep 29, 2015 · ca51125 · ca51125
1 parent c38f876
commit ca51125
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 15 deletions.
diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js
@@ -392,11 +392,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
   var optionTemplate = document.createElement('option'),
       optGroupTemplate = document.createElement('optgroup');
 
-  return {
-    restrict: 'A',
-    terminal: true,
-    require: ['select', 'ngModel'],
-    link: function(scope, selectElement, attr, ctrls) {
+    function ngOptionsPostLink(scope, selectElement, attr, ctrls) {
 
       var selectCtrl = ctrls[0];
       var ngModelCtrl = ctrls[1];
@@ -448,12 +444,11 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
         unknownOption.remove();
       };
 
-
       // Update the controller methods for multiple selectable options
       if (!multiple) {
 
         selectCtrl.writeValue = function writeNgOptionsValue(value) {
-          var option = options && options.getOptionFromViewValue(value);
+          var option = options.getOptionFromViewValue(value);
 
           if (option && !option.disabled) {
             if (selectElement[0].value !== option.selectValue) {
@@ -726,7 +721,17 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
         }
 
       }
+  }
 
+  return {
+    restrict: 'A',
+    terminal: true,
+    require: ['select', 'ngModel'],
+    link: {
+      pre: function ngOptionsPreLink(scope, selectElement, attr, ctrls) {
+        ctrls[0].ngModelCtrl = null;
+      },
+      post: ngOptionsPostLink
     }
   };
 }];
diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js
@@ -308,7 +308,13 @@ var selectDirective = function() {
     restrict: 'E',
     require: ['select', '?ngModel'],
     controller: SelectController,
-    link: function(scope, element, attr, ctrls) {
+    priority: 1,
+    link: {
+      pre: selectPreLink
+    }
+  };
+
+  function selectPreLink(scope, element, attr, ctrls) {
 
       // if ngModel is not defined, we don't need to do anything
       var ngModelCtrl = ctrls[1];
@@ -378,7 +384,6 @@ var selectDirective = function() {
 
       }
     }
-  };
 };
 
 

diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js
@@ -2,7 +2,7 @@
 
 describe('ngOptions', function() {
 
-  var scope, formElement, element, $compile;
+  var scope, formElement, element, $compile, linkLog;
 
   function compile(html) {
     formElement = jqLite('<form name="form">' + html + '</form>');
@@ -104,15 +104,51 @@ describe('ngOptions', function() {
     });
   });
 
-  beforeEach(module(function($compileProvider) {
+  beforeEach(module(function($compileProvider, $provide) {
+    linkLog = [];
+
     $compileProvider
-      .directive('compileContents', function($compile) {
+      .directive('customSelect', function() {
         return {
-          link: function(scope, element) {
+          restrict: "E",
+          replace: true,
+          scope: {
+            ngModel: '=',
+            options: '='
+          },
+          templateUrl: 'select_template.html',
+          link: function(scope, $element, attributes) {
+            scope.selectable_options = scope.options;
+          }
+        };
+      })
+
+      .directive('oCompileContents', function() {
+        return {
+          link: function(scope, element)  {
+            linkLog.push('linkCompileContents');
             $compile(element.contents())(scope);
           }
         };
     });
+
+    $provide.decorator('ngOptionsDirective', function($delegate) {
+
+      var origPreLink = $delegate[0].link.pre;
+      var origPostLink = $delegate[0].link.post;
+
+      $delegate[0].compile = function() {
+        return {
+          pre: origPreLink,
+          post: function() {
+            linkLog.push('linkNgOptions');
+            origPostLink.apply(this, arguments);
+          }
+        };
+      };
+
+      return $delegate;
+    });
   }));
 
   beforeEach(inject(function($rootScope, _$compile_) {
@@ -2135,13 +2171,30 @@ describe('ngOptions', function() {
     it('should not throw when a directive compiles the blank option before ngOptions is linked', function() {
       expect(function() {
           createSelect({
-          'compile-contents': '',
+          'o-compile-contents': '',
           'name': 'select',
           'ng-model': 'value',
-          'ng-options': 'item for item in items',
+          'ng-options': 'item for item in items'
         }, true);
       }).not.toThrow();
+
+      expect(linkLog).toEqual(['linkCompileContents', 'linkNgOptions']);
     });
+
+
+    it('should not throw with a directive that replaces', inject(function($templateCache, $httpBackend) {
+      $templateCache.put('select_template.html', '<select ng-options="option as option for option in selectable_options"> <option value="">This is a test</option> </select>');
+
+      scope.options = ['a', 'b', 'c', 'd'];
+
+      expect(function() {
+        var element = $compile('<custom-select ng-model="value" options="options"></custom-select>')(scope);
+        scope.$digest();
+        dealoc(element);
+      }).not.toThrow();
+
+    }));
+
   });