diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 505fecfb410c..fdef4d779245 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -356,7 +356,8 @@ var selectDirective = function() { controller: SelectController, priority: 1, link: { - pre: selectPreLink + pre: selectPreLink, + post: selectPostLink } }; @@ -370,13 +371,6 @@ var selectDirective = function() { selectCtrl.ngModelCtrl = ngModelCtrl; - // 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` - ngModelCtrl.$render = function() { - selectCtrl.writeValue(ngModelCtrl.$viewValue); - }; - // When the selected item(s) changes we delegate getting the value of the select control // to the `readValue` method, which can be changed if the select can have multiple // selected values or if the options are being generated by `ngOptions` @@ -430,6 +424,23 @@ var selectDirective = function() { } } + + function selectPostLink(scope, element, attrs, ctrls) { + // if ngModel is not defined, we don't need to do anything + var ngModelCtrl = ctrls[1]; + if (!ngModelCtrl) return; + + var selectCtrl = ctrls[0]; + + // 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`. + // This must be done in the postLink fn to prevent $render to be called before + // all nodes have been linked correctly. + ngModelCtrl.$render = function() { + selectCtrl.writeValue(ngModelCtrl.$viewValue); + }; + } }; diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 7be4cceb19f8..23a961611cce 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -1,7 +1,7 @@ 'use strict'; describe('select', function() { - var scope, formElement, element, $compile; + var scope, formElement, element, $compile, ngModelCtrl, selectCtrl, renderSpy; function compile(html) { formElement = jqLite('
'); @@ -10,10 +10,42 @@ describe('select', function() { scope.$apply(); } + function compileRepeatedOptions() { + compile(''); + } + + function compileGroupedOptions() { + compile( + ''); + } + function unknownValue(value) { return '? ' + hashKey(value) + ' ?'; } + beforeEach(module(function($compileProvider) { + $compileProvider.directive('spyOnWriteValue', function() { + return { + require: 'select', + link: { + pre: function(scope, element, attrs, ctrl) { + selectCtrl = ctrl; + renderSpy = jasmine.createSpy('renderSpy'); + selectCtrl.ngModelCtrl.$render = renderSpy.andCallFake(selectCtrl.ngModelCtrl.$render); + spyOn(selectCtrl, 'writeValue').andCallThrough(); + } + } + }; + }); + })); + beforeEach(inject(function($rootScope, _$compile_) { scope = $rootScope.$new(); //create a child scope because the root scope can't be $destroy-ed $compile = _$compile_; @@ -47,12 +79,14 @@ describe('select', function() { toEqualSelectWithOptions: function(expected) { var actualValues = {}; var optionGroup; + var optionValue; forEach(this.actual.find('option'), function(option) { optionGroup = option.parentNode.label || ''; actualValues[optionGroup] = actualValues[optionGroup] || []; // IE9 doesn't populate the label property from the text property like other browsers - actualValues[optionGroup].push(option.label || option.text); + optionValue = option.label || option.text; + actualValues[optionGroup].push(option.selected ? [optionValue] : optionValue); }); this.message = function() { @@ -198,6 +232,50 @@ describe('select', function() { }); + it('should select options in a group when there is a linebreak before an option', function() { + scope.mySelect = 'B'; + scope.$apply(); + + var select = jqLite( + ''); + + $compile(select)(scope); + scope.$apply(); + + expect(select).toEqualSelectWithOptions({'first':['A'], 'second': [['B']]}); + dealoc(select); + }); + + + it('should only call selectCtrl.writeValue after a digest has occured', function() { + scope.mySelect = 'B'; + scope.$apply(); + + var select = jqLite( + ''); + + $compile(select)(scope); + expect(selectCtrl.writeValue).not.toHaveBeenCalled(); + + scope.$digest(); + expect(selectCtrl.writeValue).toHaveBeenCalledOnce(); + dealoc(select); + }); + describe('empty option', function() { it('should allow empty option to be added and removed dynamically', function() { @@ -538,22 +616,6 @@ describe('select', function() { describe('selectController.hasOption', function() { - function compileRepeatedOptions() { - compile(''); - } - - function compileGroupedOptions() { - compile( - ''); - } - describe('flat options', function() { it('should return false for options shifted via ngRepeat', function() { scope.robots = [ @@ -624,7 +686,7 @@ describe('select', function() { expect(selectCtrl.hasOption('A')).toBe(true); expect(selectCtrl.hasOption('B')).toBe(true); expect(selectCtrl.hasOption('C')).toBe(true); - expect(element).toEqualSelectWithOptions({'': ['A', 'B', 'C']}); + expect(element).toEqualSelectWithOptions({'': ['A', 'B', ['C']]}); }); }); @@ -665,7 +727,7 @@ describe('select', function() { expect(selectCtrl.hasOption('C')).toBe(true); expect(selectCtrl.hasOption('D')).toBe(true); expect(selectCtrl.hasOption('E')).toBe(true); - expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C'], 'second': ['D', 'E']}); + expect(element).toEqualSelectWithOptions({'': [['']], 'first':['B', 'C'], 'second': ['D', 'E']}); }); @@ -702,7 +764,7 @@ describe('select', function() { expect(selectCtrl.hasOption('C')).toBe(true); expect(selectCtrl.hasOption('D')).toBe(true); expect(selectCtrl.hasOption('E')).toBe(true); - expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C', 'D'], 'second': ['E']}); + expect(element).toEqualSelectWithOptions({'': [['']], 'first':['B', 'C', 'D'], 'second': ['E']}); }); @@ -741,7 +803,7 @@ describe('select', function() { expect(selectCtrl.hasOption('D')).toBe(true); expect(selectCtrl.hasOption('E')).toBe(true); expect(selectCtrl.hasOption('F')).toBe(false); - expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['D', 'E']}); + expect(element).toEqualSelectWithOptions({'': [[''], 'A'], 'first':['B', 'C'], 'second': ['D', 'E']}); }); @@ -778,7 +840,7 @@ describe('select', function() { expect(selectCtrl.hasOption('C')).toBe(false); expect(selectCtrl.hasOption('D')).toBe(true); expect(selectCtrl.hasOption('E')).toBe(true); - expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'D'], 'second': ['E']}); + expect(element).toEqualSelectWithOptions({'': [[''], 'A'], 'first':['B', 'D'], 'second': ['E']}); }); @@ -813,7 +875,7 @@ describe('select', function() { expect(selectCtrl.hasOption('C')).toBe(true); expect(selectCtrl.hasOption('D')).toBe(false); expect(selectCtrl.hasOption('E')).toBe(true); - expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['E']}); + expect(element).toEqualSelectWithOptions({'': [[''], 'A'], 'first':['B', 'C'], 'second': ['E']}); }); @@ -848,7 +910,7 @@ describe('select', function() { expect(selectCtrl.hasOption('C')).toBe(true); expect(selectCtrl.hasOption('D')).toBe(false); expect(selectCtrl.hasOption('E')).toBe(false); - expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C']}); + expect(element).toEqualSelectWithOptions({'': [[''], 'A'], 'first':['B', 'C']}); }); }); });