Skip to content

Commit 5878f07

Browse files
NarretzNarretz
committed
fix(ngOptions): select unknown option if unmatched model does not match empty option
When a regular / ngOptions select has an explicit *empty* option, this option can be selected by the user and will set the model to `null`. It is also selected when the model is set to `null` or `undefined`. When the model is set to a value that does not match any option value, and is also not `null` or `undefined`, the *unknown* option is inserted and selected - this is an explicit marker that the select is in an invalid / unknown state, which is different from an allowed empty state. Previously, regular selects followed this logic, whereas ngOptions selects selected the empty option in the case described above. This patch makes the behavior consistent between regular / ngOptions select - the latter will now insert and select the unknown option. The order of the options has been fixed to unknown -> empty -> actual options.
1 parent 8d7c7f4 commit 5878f07

File tree

5 files changed

+112
-36
lines changed

5 files changed

+112
-36
lines changed

‎src/ng/directive/ngOptions.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,8 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
473473
option.element.setAttribute('selected', 'selected');
474474
} else {
475475

476-
if (providedEmptyOption) {
476+
if (value == null && providedEmptyOption) {
477+
selectCtrl.removeUnknownOption();
477478
selectCtrl.selectEmptyOption();
478479
} else if (selectCtrl.unknownOption.parent().length) {
479480
selectCtrl.updateUnknownOption(value);
@@ -657,7 +658,12 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
657658

658659
// Ensure that the empty option is always there if it was explicitly provided
659660
if (providedEmptyOption) {
660-
selectElement.prepend(selectCtrl.emptyOption);
661+
662+
if (selectCtrl.unknownOption.parent().length) {
663+
selectCtrl.unknownOption.after(selectCtrl.emptyOption);
664+
} else {
665+
selectElement.prepend(selectCtrl.emptyOption);
666+
}
661667
}
662668

663669
options.items.forEach(function addOption(option) {

‎src/ng/directive/select.js

+7-5
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,13 @@ var SelectController =
4444
// to create it in <select> and IE barfs otherwise.
4545
self.unknownOption = jqLite(window.document.createElement('option'));
4646

47-
// The empty option is an option with the value '' that te application developer can
48-
// provide inside the select. When the model changes to a value that doesn't match an option,
49-
// it is selected - so if an empty option is provided, no unknown option is generated.
50-
// However, the empty option is not removed when the model matches an option. It is always selectable
51-
// and indicates that a "null" selection has been made.
47+
// The empty option is an option with the value '' that the application developer can
48+
// provide inside the select. It is always selectable and indicates that a "null" selection has
49+
// been made by the user.
50+
// If the select has an empty option, and the model of the select is set to "undefined" or "null",
51+
// the empty option is selected.
52+
// If the model is set to a different unmatched value, the unknown option is rendered and
53+
// selected, i.e both are present, because a "null" selection and an unknown value are different.
5254
self.hasEmptyOption = false;
5355
self.emptyOption = undefined;
5456

‎test/helpers/matchers.js

+21
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,27 @@ beforeEach(function() {
400400
return result;
401401
}
402402
};
403+
},
404+
toEqualSelect: function() {
405+
return {
406+
compare: function(actual, expected) {
407+
var actualValues = [],
408+
expectedValues = [].slice.call(arguments, 1);
409+
410+
forEach(actual.find('option'), function(option) {
411+
actualValues.push(option.selected ? [option.value] : option.value);
412+
});
413+
414+
var message = function() {
415+
return 'Expected ' + toJson(actualValues) + ' to equal ' + toJson(expectedValues) + '.';
416+
};
417+
418+
return {
419+
pass: equals(expectedValues, actualValues),
420+
message: message
421+
};
422+
}
423+
};
403424
}
404425
});
405426
});

‎test/ng/directive/ngOptionsSpec.js

+49-2
Original file line numberDiff line numberDiff line change
@@ -773,12 +773,32 @@ describe('ngOptions', function() {
773773
expect(options[1]).not.toBeMarkedAsSelected();
774774
expect(options[2]).toBeMarkedAsSelected();
775775

776-
scope.selected = 'no match';
776+
// This will select the empty option
777+
scope.selected = null;
777778
scope.$digest();
778779

779780
expect(options[0]).toBeMarkedAsSelected();
780781
expect(options[1]).not.toBeMarkedAsSelected();
781782
expect(options[2]).not.toBeMarkedAsSelected();
783+
784+
// This will add and select the unknown option
785+
scope.selected = 'unmatched value';
786+
scope.$digest();
787+
options = element.find('option');
788+
789+
expect(options[0]).toBeMarkedAsSelected();
790+
expect(options[1]).not.toBeMarkedAsSelected();
791+
expect(options[2]).not.toBeMarkedAsSelected();
792+
expect(options[3]).not.toBeMarkedAsSelected();
793+
794+
// Back to matched value
795+
scope.selected = scope.values[1];
796+
scope.$digest();
797+
options = element.find('option');
798+
799+
expect(options[0]).not.toBeMarkedAsSelected();
800+
expect(options[1]).not.toBeMarkedAsSelected();
801+
expect(options[2]).toBeMarkedAsSelected();
782802
});
783803

784804
describe('disableWhen expression', function() {
@@ -2195,6 +2215,20 @@ describe('ngOptions', function() {
21952215
});
21962216

21972217

2218+
it('should insert and select temporary unknown option when no options-model match, empty ' +
2219+
'option is present and model is defined', function() {
2220+
scope.selected = 'C';
2221+
scope.values = [{name: 'A'}, {name: 'B'}];
2222+
createSingleSelect(true);
2223+
2224+
expect(element).toEqualSelect(['?'], '', 'object:3', 'object:4');
2225+
2226+
scope.$apply('selected = values[1]');
2227+
2228+
expect(element).toEqualSelect('', 'object:3', ['object:4']);
2229+
});
2230+
2231+
21982232
it('should select correct input if previously selected option was "?"', function() {
21992233
createSingleSelect();
22002234

@@ -2214,6 +2248,19 @@ describe('ngOptions', function() {
22142248
});
22152249

22162250

2251+
it('should remove unknown option when empty option exists and model is undefined', function() {
2252+
scope.selected = 'C';
2253+
scope.values = [{name: 'A'}, {name: 'B'}];
2254+
createSingleSelect(true);
2255+
2256+
expect(element).toEqualSelect(['?'], '', 'object:3', 'object:4');
2257+
2258+
scope.selected = undefined;
2259+
scope.$digest();
2260+
2261+
expect(element).toEqualSelect([''], 'object:3', 'object:4');
2262+
});
2263+
22172264
it('should use exact same values as values in scope with one-time bindings', function() {
22182265
scope.values = [{name: 'A'}, {name: 'B'}];
22192266
scope.selected = scope.values[0];
@@ -2955,7 +3002,7 @@ describe('ngOptions', function() {
29553002
expect(ngModelCtrl.$error.required).toBeFalsy();
29563003

29573004
// model -> view
2958-
scope.$apply('selection = "unmatched value"');
3005+
scope.$apply('selection = null');
29593006
expect(options[0]).toBeMarkedAsSelected();
29603007
expect(element).toBeInvalid();
29613008
expect(ngModelCtrl.$error.required).toBeTruthy();

‎test/ng/directive/selectSpec.js

+27-27
Original file line numberDiff line numberDiff line change
@@ -86,28 +86,6 @@ describe('select', function() {
8686

8787
beforeEach(function() {
8888
jasmine.addMatchers({
89-
toEqualSelect: function() {
90-
return {
91-
compare: function(actual, expected) {
92-
var actualValues = [],
93-
expectedValues = [].slice.call(arguments, 1);
94-
95-
forEach(actual.find('option'), function(option) {
96-
actualValues.push(option.selected ? [option.value] : option.value);
97-
});
98-
99-
var message = function() {
100-
return 'Expected ' + toJson(actualValues) + ' to equal ' + toJson(expectedValues) + '.';
101-
};
102-
103-
return {
104-
pass: equals(expectedValues, actualValues),
105-
message: message
106-
};
107-
}
108-
};
109-
},
110-
11189
toEqualSelectWithOptions: function() {
11290
return {
11391
compare: function(actual, expected) {
@@ -396,6 +374,7 @@ describe('select', function() {
396374

397375
it('should remove the "selected" attribute from the previous option when the model changes', function() {
398376
compile('<select name="select" ng-model="selected">' +
377+
'<option value="">--empty--</option>' +
399378
'<option value="a">A</option>' +
400379
'<option value="b">B</option>' +
401380
'</select>');
@@ -411,24 +390,45 @@ describe('select', function() {
411390
scope.$digest();
412391

413392
options = element.find('option');
414-
expect(options.length).toBe(2);
415-
expect(options[0]).toBeMarkedAsSelected();
416-
expect(options[1]).not.toBeMarkedAsSelected();
393+
expect(options.length).toBe(3);
394+
expect(options[0]).not.toBeMarkedAsSelected();
395+
expect(options[1]).toBeMarkedAsSelected();
396+
expect(options[2]).not.toBeMarkedAsSelected();
417397

418398
scope.selected = 'b';
419399
scope.$digest();
420400

421401
options = element.find('option');
422402
expect(options[0]).not.toBeMarkedAsSelected();
423-
expect(options[1]).toBeMarkedAsSelected();
403+
expect(options[1]).not.toBeMarkedAsSelected();
404+
expect(options[2]).toBeMarkedAsSelected();
424405

425-
scope.selected = 'no match';
406+
// This will select the empty option
407+
scope.selected = null;
426408
scope.$digest();
427409

410+
expect(options[0]).toBeMarkedAsSelected();
411+
expect(options[1]).not.toBeMarkedAsSelected();
412+
expect(options[2]).not.toBeMarkedAsSelected();
413+
414+
// This will add and select the unknown option
415+
scope.selected = 'unmatched value';
416+
scope.$digest();
428417
options = element.find('option');
418+
429419
expect(options[0]).toBeMarkedAsSelected();
430420
expect(options[1]).not.toBeMarkedAsSelected();
431421
expect(options[2]).not.toBeMarkedAsSelected();
422+
expect(options[3]).not.toBeMarkedAsSelected();
423+
424+
// Back to matched value
425+
scope.selected = 'b';
426+
scope.$digest();
427+
options = element.find('option');
428+
429+
expect(options[0]).not.toBeMarkedAsSelected();
430+
expect(options[1]).not.toBeMarkedAsSelected();
431+
expect(options[2]).toBeMarkedAsSelected();
432432
});
433433

434434
describe('empty option', function() {

0 commit comments

Comments
 (0)