Skip to content

Commit

Permalink
fix(select): placeholder (empty option) is lost in IE9
Browse files Browse the repository at this point in the history
Fixes a check inside render for select elements with ngOptions, which compares the selected property of an element with it's desired state.
Ensured 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 angular#2150, angular#1826
  • Loading branch information
Chad Smith committed Mar 20, 2013
1 parent d1b49e2 commit 848944b
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 8 deletions.
18 changes: 10 additions & 8 deletions src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {

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

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

// Now we need to update the list of DOM nodes to match the optionGroups we computed above
Expand Down Expand Up @@ -468,7 +469,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
if (existingOption.id !== option.id) {
lastElement.val(existingOption.id = option.id);
}
if (existingOption.element.selected !== option.selected) {
// lastElement.prop('selected') provided by jQuery has side-effects
if (lastElement[0].selected !== option.selected) {
lastElement.prop('selected', (existingOption.selected = option.selected));
}
} else {
Expand Down
28 changes: 28 additions & 0 deletions test/ng/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,19 @@ describe('select', function() {
expect(option.attr('id')).toBe('road-runner');
expect(option.attr('custom-attr')).toBe('custom-attr');
});

it('should be selected, if it is available and no other option is selected', function() {
// selectedIndex is used here because jqLite incorrectly reports element.val()
scope.$apply(function() {
scope.values = [{name: 'A'}];
});
createSingleSelect(true);
// ensure the first option (the blank option) is selected
expect(element[0].selectedIndex).toEqual(0);
scope.$digest();
// ensure the option has not changed following the digest
expect(element[0].selectedIndex).toEqual(0);
});
});


Expand Down Expand Up @@ -1099,6 +1112,21 @@ describe('select', function() {
browserTrigger(element, 'change');
expect(scope.selected).toEqual(['0']);
});

it('should deselect all options when model is emptied', function() {
createMultiSelect();
scope.$apply(function() {
scope.values = [{name: 'A'}, {name: 'B'}];
scope.selected = [scope.values[0]];
});
expect(element.find('option')[0].selected).toEqual(true);

scope.$apply(function() {
scope.selected.pop();
});

expect(element.find('option')[0].selected).toEqual(false);
})
});


Expand Down

0 comments on commit 848944b

Please sign in to comment.