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

Commit 4622af3

Browse files
Chad Smithpetebacondarwin
Chad Smith
authored andcommitted
fix(select): ensure empty option is not lost in IE9
Fix a check inside render for select elements with ngOptions, which compares the selected property of an element with it's desired state. Ensure the placeholder, if available, is explicitly selected if the model value can not be found in the option list. Without these fixes it's up to the browser implementation to decide which option to choose. In most browsers, this has the effect of displaying the first item in the list. In IE9 however, this causes the select to display nothing. Closes #2150, #1826
1 parent f046f6f commit 4622af3

File tree

2 files changed

+38
-8
lines changed

2 files changed

+38
-8
lines changed

src/ng/directive/select.js

+10-8
Original file line numberDiff line numberDiff line change
@@ -395,10 +395,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
395395

396396
if (multiple) {
397397
selectedSet = new HashMap(modelValue);
398-
} else if (modelValue === null || nullOption) {
399-
// if we are not multiselect, and we are null then we have to add the nullOption
400-
optionGroups[''].push({selected:modelValue === null, id:'', label:''});
401-
selectedSet = true;
402398
}
403399

404400
// We now build up the list of options we need (we merge later)
@@ -423,9 +419,14 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
423419
selected: selected // determine if we should be selected
424420
});
425421
}
426-
if (!multiple && !selectedSet) {
427-
// nothing was selected, we have to insert the undefined item
428-
optionGroups[''].unshift({id:'?', label:'', selected:true});
422+
if (!multiple) {
423+
if (nullOption || modelValue === null) {
424+
// insert null option if we have a placeholder, or the model is null
425+
optionGroups[''].unshift({id:'', label:'', selected:!selectedSet});
426+
} else if (!selectedSet) {
427+
// option could not be found, we have to insert the undefined item
428+
optionGroups[''].unshift({id:'?', label:'', selected:true});
429+
}
429430
}
430431

431432
// Now we need to update the list of DOM nodes to match the optionGroups we computed above
@@ -469,7 +470,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
469470
if (existingOption.id !== option.id) {
470471
lastElement.val(existingOption.id = option.id);
471472
}
472-
if (existingOption.element.selected !== option.selected) {
473+
// lastElement.prop('selected') provided by jQuery has side-effects
474+
if (lastElement[0].selected !== option.selected) {
473475
lastElement.prop('selected', (existingOption.selected = option.selected));
474476
}
475477
} else {

test/ng/directive/selectSpec.js

+28
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,19 @@ describe('select', function() {
977977
expect(option.attr('id')).toBe('road-runner');
978978
expect(option.attr('custom-attr')).toBe('custom-attr');
979979
});
980+
981+
it('should be selected, if it is available and no other option is selected', function() {
982+
// selectedIndex is used here because jqLite incorrectly reports element.val()
983+
scope.$apply(function() {
984+
scope.values = [{name: 'A'}];
985+
});
986+
createSingleSelect(true);
987+
// ensure the first option (the blank option) is selected
988+
expect(element[0].selectedIndex).toEqual(0);
989+
scope.$digest();
990+
// ensure the option has not changed following the digest
991+
expect(element[0].selectedIndex).toEqual(0);
992+
});
980993
});
981994

982995

@@ -1099,6 +1112,21 @@ describe('select', function() {
10991112
browserTrigger(element, 'change');
11001113
expect(scope.selected).toEqual(['0']);
11011114
});
1115+
1116+
it('should deselect all options when model is emptied', function() {
1117+
createMultiSelect();
1118+
scope.$apply(function() {
1119+
scope.values = [{name: 'A'}, {name: 'B'}];
1120+
scope.selected = [scope.values[0]];
1121+
});
1122+
expect(element.find('option')[0].selected).toEqual(true);
1123+
1124+
scope.$apply(function() {
1125+
scope.selected.pop();
1126+
});
1127+
1128+
expect(element.find('option')[0].selected).toEqual(false);
1129+
})
11021130
});
11031131

11041132

0 commit comments

Comments
 (0)