Skip to content

Commit 73e73d0

Browse files
siddiitbosch
authored andcommitted
fix(ngSelect): Support not selected option on multi select with ngOptions
Shows the static option for a not selected value in a select with ngOption also when the multiple attribute is used. Closes angular#4325 Closes angular#5938
1 parent bf4b0db commit 73e73d0

File tree

2 files changed

+134
-17
lines changed

2 files changed

+134
-17
lines changed

src/ng/directive/select.js

+17-17
Original file line numberDiff line numberDiff line change
@@ -359,17 +359,19 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
359359

360360
for(index = 1, length = optionGroup.length; index < length; index++) {
361361
if ((optionElement = optionGroup[index].element)[0].selected) {
362-
key = optionElement.val();
363-
if (keyName) locals[keyName] = key;
364-
if (trackFn) {
365-
for (trackIndex = 0; trackIndex < collection.length; trackIndex++) {
366-
locals[valueName] = collection[trackIndex];
367-
if (trackFn(scope, locals) == key) break;
362+
if (optionElement[0] !== nullOption[0]) {
363+
key = optionElement.val();
364+
if (keyName) locals[keyName] = key;
365+
if (trackFn) {
366+
for (trackIndex = 0; trackIndex < collection.length; trackIndex++) {
367+
locals[valueName] = collection[trackIndex];
368+
if (trackFn(scope, locals) == key) break;
369+
}
370+
} else {
371+
locals[valueName] = collection[key];
368372
}
369-
} else {
370-
locals[valueName] = collection[key];
373+
value.push(valueFn(scope, locals));
371374
}
372-
value.push(valueFn(scope, locals));
373375
}
374376
}
375377
}
@@ -479,14 +481,12 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
479481
selected: selected // determine if we should be selected
480482
});
481483
}
482-
if (!multiple) {
483-
if (nullOption || modelValue === null) {
484-
// insert null option if we have a placeholder, or the model is null
485-
optionGroups[''].unshift({id:'', label:'', selected:!selectedSet});
486-
} else if (!selectedSet) {
487-
// option could not be found, we have to insert the undefined item
488-
optionGroups[''].unshift({id:'?', label:'', selected:true});
489-
}
484+
if (nullOption || modelValue === null) {
485+
// insert null option if we have a placeholder, or the model is null
486+
optionGroups[''].unshift({id:'', label:'', selected:!selectedSet});
487+
} else if (!selectedSet) {
488+
// option could not be found, we have to insert the undefined item
489+
optionGroups[''].unshift({id:'?', label:'', selected:true});
490490
}
491491

492492
// Now we need to update the list of DOM nodes to match the optionGroups we computed above

test/ng/directive/selectSpec.js

+117
Original file line numberDiff line numberDiff line change
@@ -1148,6 +1148,123 @@ describe('select', function() {
11481148
});
11491149

11501150

1151+
describe('blank option', function () {
1152+
1153+
it('should be compiled as template, be watched and updated', function () {
1154+
var option;
1155+
createMultiSelect('<option value="">blank is {{blankVal}}</option>');
1156+
1157+
scope.$apply(function() {
1158+
scope.blankVal = 'so blank';
1159+
scope.values = [{name: 'A'}];
1160+
});
1161+
1162+
// check blank option is first and is compiled
1163+
expect(element.find('option').length).toBe(2);
1164+
option = element.find('option').eq(0);
1165+
expect(option.val()).toBe('');
1166+
expect(option.text()).toBe('blank is so blank');
1167+
1168+
scope.$apply(function() {
1169+
scope.blankVal = 'not so blank';
1170+
});
1171+
1172+
// check blank option is first and is compiled
1173+
expect(element.find('option').length).toBe(2);
1174+
option = element.find('option').eq(0);
1175+
expect(option.val()).toBe('');
1176+
expect(option.text()).toBe('blank is not so blank');
1177+
});
1178+
1179+
1180+
it('should support binding via ngBindTemplate directive', function () {
1181+
var option;
1182+
createMultiSelect('<option value="" ng-bind-template="blank is {{blankVal}}"></option>');
1183+
1184+
scope.$apply(function() {
1185+
scope.blankVal = 'so blank';
1186+
scope.values = [{name: 'A'}];
1187+
});
1188+
1189+
// check blank option is first and is compiled
1190+
expect(element.find('option').length).toBe(2);
1191+
option = element.find('option').eq(0);
1192+
expect(option.val()).toBe('');
1193+
expect(option.text()).toBe('blank is so blank');
1194+
});
1195+
1196+
1197+
it('should support binding via ngBind attribute', function () {
1198+
var option;
1199+
createMultiSelect('<option value="" ng-bind="blankVal"></option>');
1200+
1201+
scope.$apply(function() {
1202+
scope.blankVal = 'is blank';
1203+
scope.values = [{name: 'A'}];
1204+
});
1205+
1206+
// check blank option is first and is compiled
1207+
expect(element.find('option').length).toBe(2);
1208+
option = element.find('option').eq(0);
1209+
expect(option.val()).toBe('');
1210+
expect(option.text()).toBe('is blank');
1211+
});
1212+
1213+
1214+
it('should be rendered with the attributes preserved', function () {
1215+
var option;
1216+
createMultiSelect('<option value="" class="coyote" id="road-runner" ' +
1217+
'custom-attr="custom-attr">{{blankVal}}</option>');
1218+
1219+
scope.$apply(function() {
1220+
scope.blankVal = 'is blank';
1221+
});
1222+
1223+
// check blank option is first and is compiled
1224+
option = element.find('option').eq(0);
1225+
expect(option.hasClass('coyote')).toBeTruthy();
1226+
expect(option.attr('id')).toBe('road-runner');
1227+
expect(option.attr('custom-attr')).toBe('custom-attr');
1228+
});
1229+
1230+
it('should not be selected, if it is available and no other option is selected', function() {
1231+
// selectedIndex is used here because jqLite incorrectly reports element.val()
1232+
scope.$apply(function() {
1233+
scope.values = [{name: 'A'}];
1234+
});
1235+
createMultiSelect(true);
1236+
// ensure the first option (the blank option) is selected
1237+
expect(element[0].selectedIndex).toEqual(-1);
1238+
scope.$digest();
1239+
// ensure the option has not changed following the digest
1240+
expect(element[0].selectedIndex).toEqual(-1);
1241+
});
1242+
1243+
it('should unselect itself when selected', function() {
1244+
var option;
1245+
createMultiSelect(true);
1246+
scope.$apply(function() {
1247+
scope.values = [{name: 'A'}];
1248+
});
1249+
option = element.find('option').eq(0);
1250+
option.prop('selected', true);
1251+
browserTrigger(element, 'change');
1252+
expect(option.prop('selected')).toBe(false);
1253+
});
1254+
1255+
it('should not add the null value to the result when selected', function() {
1256+
var option;
1257+
createMultiSelect(true);
1258+
scope.$apply(function() {
1259+
scope.values = [{name: 'A'}];
1260+
});
1261+
option = element.find('option').eq(0);
1262+
option.prop('selected', true);
1263+
browserTrigger(element, 'change');
1264+
expect(scope.selected).toEqual([]);
1265+
});
1266+
});
1267+
11511268
it('should update model on change', function() {
11521269
createMultiSelect();
11531270

0 commit comments

Comments
 (0)