From e7cda24f6e8b3fb831ecbb6d3b97bc84875ae8a8 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Wed, 30 Sep 2015 11:52:24 +0200 Subject: [PATCH] wip: yet another way - noop add/removeOption in selectCtrl. Prevents options from being added before by directives that compile before select - add add/remove in select preLink - noop add/remove again in ngOptions. Prevents options from being added by directive that run postLink before ngOptions --- src/ng/directive/ngOptions.js | 3 +- src/ng/directive/select.js | 64 ++++++++++++++++-------------- test/ng/directive/ngOptionsSpec.js | 38 ++++++++++++------ 3 files changed, 61 insertions(+), 44 deletions(-) diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index 3d626706a13e..1d1b3eb64a68 100644 --- a/src/ng/directive/ngOptions.js +++ b/src/ng/directive/ngOptions.js @@ -729,7 +729,8 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { require: ['select', 'ngModel'], link: { pre: function ngOptionsPreLink(scope, selectElement, attr, ctrls) { - ctrls[0].ngModelCtrl = null; + ctrls[0].addOption = noop; + ctrls[0].removeOption = noop; }, post: ngOptionsPostLink } diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index dd468283e965..82d59c66b269 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -16,6 +16,8 @@ var SelectController = var self = this, optionsMap = new HashMap(); + self.optionsMap = optionsMap; + // If the ngModel doesn't get provided then provide a dummy noop version to prevent errors self.ngModelCtrl = noopNgModelController; @@ -68,31 +70,8 @@ var SelectController = } }; - - // Tell the select control that an option, with the given value, has been added - self.addOption = function(value, element) { - assertNotHasOwnProperty(value, '"option value"'); - if (value === '') { - self.emptyOption = element; - } - var count = optionsMap.get(value) || 0; - optionsMap.put(value, count + 1); - }; - - // Tell the select control that an option, with the given value, has been removed - self.removeOption = function(value) { - var count = optionsMap.get(value); - if (count) { - if (count === 1) { - optionsMap.remove(value); - if (value === '') { - self.emptyOption = undefined; - } - } else { - optionsMap.put(value, count - 1); - } - } - }; + self.addOption = noop; + self.removeOption = noop; // Check whether the select control has an option matching the given value self.hasOption = function(value) { @@ -324,6 +303,36 @@ var selectDirective = function() { selectCtrl.ngModelCtrl = ngModelCtrl; + // Tell the select control that an option, with the given value, has been added + selectCtrl.addOption = function(value, element) { + assertNotHasOwnProperty(value, '"option value"'); + if (value === '') { + selectCtrl.emptyOption = element; + } + var count = selectCtrl.optionsMap.get(value) || 0; + selectCtrl.optionsMap.put(value, count + 1); + + element.on('$destroy', function() { + selectCtrl.removeOption(value); + selectCtrl.ngModelCtrl.$render(); + }); + }; + + // Tell the select control that an option, with the given value, has been removed + selectCtrl.removeOption = function(value) { + var count = selectCtrl.optionsMap.get(value); + if (count) { + if (count === 1) { + selectCtrl.optionsMap.remove(value); + if (value === '') { + selectCtrl.emptyOption = undefined; + } + } else { + selectCtrl.optionsMap.put(value, count - 1); + } + } + }; + // We delegate rendering to the `writeValue` method, which can be changed // if the select can have multiple selected values or if the options are being // generated by `ngOptions` @@ -460,11 +469,6 @@ var optionDirective = ['$interpolate', function($interpolate) { // The value attribute is static addOption(attr.value); } - - element.on('$destroy', function() { - selectCtrl.removeOption(attr.value); - selectCtrl.ngModelCtrl.$render(); - }); } }; } diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index 8c2c15d7b878..fa8e21d02744 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -125,12 +125,15 @@ describe('ngOptions', function() { .directive('oCompileContents', function() { return { - link: function(scope, element) { - linkLog.push('linkCompileContents'); - $compile(element.contents())(scope); + priority: 2, + link: { + pre: function(scope, element) { + linkLog.push('linkCompileContents'); + $compile(element.contents())(scope); + } } }; - }); + }); $provide.decorator('ngOptionsDirective', function($delegate) { @@ -2170,29 +2173,38 @@ describe('ngOptions', function() { it('should not throw when a directive compiles the blank option before ngOptions is linked', function() { expect(function() { - createSelect({ - 'o-compile-contents': '', - 'name': 'select', - 'ng-model': 'value', - 'ng-options': 'item for item in items' + createSelect({ + 'o-compile-contents': '', + 'name': 'select', + 'ng-model': 'value', + 'ng-options': 'item for item in items' }, true); }).not.toThrow(); expect(linkLog).toEqual(['linkCompileContents', 'linkNgOptions']); + + var options = element.find('option'); + expect(options.length).toBe(1); + expect(options.eq(0).val()).toBe(''); + expect(options.eq(0).text()).toBe('blank'); }); it('should not throw with a directive that replaces', inject(function($templateCache, $httpBackend) { - $templateCache.put('select_template.html', ''); + $templateCache.put('select_template.html', ''); - scope.options = ['a', 'b', 'c', 'd']; + scope.options = ['a', 'b', 'c']; expect(function() { - var element = $compile('')(scope); + element = $compile('')(scope); scope.$digest(); - dealoc(element); }).not.toThrow(); + var options = element.find('option'); + expect(options.length).toBe(4); + expect(options.eq(0).val()).toBe(''); + expect(options.eq(0).text()).toBe('--select--'); + dealoc(element); })); });