From ca51125b3654f13350cd24997421cb1035285778 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 29 Sep 2015 19:34:19 +0200 Subject: [PATCH] fix(ngOptions): unset the select.ngModelCtrl when ngOptions is present --- src/ng/directive/ngOptions.js | 19 +++++---- src/ng/directive/select.js | 9 ++++- test/ng/directive/ngOptionsSpec.js | 65 +++++++++++++++++++++++++++--- 3 files changed, 78 insertions(+), 15 deletions(-) diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index 471f1919accc..3d626706a13e 100644 --- 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 index df2bcee32437..dd468283e965 100644 --- 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 index f80b74951e85..8c2c15d7b878 100644 --- 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('
' + html + '
'); @@ -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', ''); + + scope.options = ['a', 'b', 'c', 'd']; + + expect(function() { + var element = $compile('')(scope); + scope.$digest(); + dealoc(element); + }).not.toThrow(); + + })); + });