Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(select): handle corner case of adding options via a custom directive #13878

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ var SelectController =

// Tell the select control that an option, with the given value, has been added
self.addOption = function(value, element) {
// Skip comment nodes, as they only pollute the `optionsMap`
if (element[0].nodeType === NODE_TYPE_COMMENT) return;

assertNotHasOwnProperty(value, '"option value"');
if (value === '') {
self.emptyOption = element;
Expand Down Expand Up @@ -452,7 +455,6 @@ var optionDirective = ['$interpolate', function($interpolate) {
restrict: 'E',
priority: 100,
compile: function(element, attr) {

if (isDefined(attr.value)) {
// If the value attribute is defined, check if it contains an interpolation
var interpolateValueFn = $interpolate(attr.value, true);
Expand All @@ -466,7 +468,6 @@ var optionDirective = ['$interpolate', function($interpolate) {
}

return function(scope, element, attr) {

// This is an optimization over using ^^ since we don't want to have to search
// all the way to the root of the DOM for every single option element
var selectCtrlName = '$selectController',
Expand Down
29 changes: 27 additions & 2 deletions test/ng/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@ describe('select', function() {
}
};
});

$compileProvider.directive('myOptions', function() {
return {
scope: {myOptions: '='},
replace: true,
template:
'<option value="{{ option.value }}" ng-repeat="option in myOptions">' +
'{{ options.label }}' +
'</option>'
};
});
}));

beforeEach(inject(function($rootScope, _$compile_) {
Expand Down Expand Up @@ -313,7 +324,7 @@ describe('select', function() {
});


it('should cope with a dynamic empty option added to a static empty option', function() {
it('should cope with a dynamic empty option added to a static empty option', function() {
scope.dynamicOptions = [];
scope.robot = 'x';
compile('<select ng-model="robot">' +
Expand All @@ -340,7 +351,7 @@ describe('select', function() {
scope.dynamicOptions = [];
scope.$digest();
expect(element).toEqualSelect(['']);
});
});

it('should select the empty option when model is undefined', function() {
compile('<select ng-model="robot">' +
Expand Down Expand Up @@ -611,6 +622,20 @@ describe('select', function() {

});


it('should not break when adding options via a directive with `replace: true` '
+ 'and a structural directive in its template',
function() {
scope.options = [
{value: '1', label: 'Option 1'},
{value: '2', label: 'Option 2'},
{value: '3', label: 'Option 3'}
];
compile('<select ng-model="mySelect"><option my-options="options"></option></select>');

expect(element).toEqualSelect([unknownValue()], '1', '2', '3');
}
);
});


Expand Down