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

Commit 2933921

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 ae9e23d commit 2933921

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);
@@ -658,7 +659,12 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
658659

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

664670
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) {
@@ -364,6 +342,7 @@ describe('select', function() {
364342

365343
it('should remove the "selected" attribute from the previous option when the model changes', function() {
366344
compile('<select name="select" ng-model="selected">' +
345+
'<option value="">--empty--</option>' +
367346
'<option value="a">A</option>' +
368347
'<option value="b">B</option>' +
369348
'</select>');
@@ -379,24 +358,45 @@ describe('select', function() {
379358
scope.$digest();
380359

381360
options = element.find('option');
382-
expect(options.length).toBe(2);
383-
expect(options[0]).toBeMarkedAsSelected();
384-
expect(options[1]).not.toBeMarkedAsSelected();
361+
expect(options.length).toBe(3);
362+
expect(options[0]).not.toBeMarkedAsSelected();
363+
expect(options[1]).toBeMarkedAsSelected();
364+
expect(options[2]).not.toBeMarkedAsSelected();
385365

386366
scope.selected = 'b';
387367
scope.$digest();
388368

389369
options = element.find('option');
390370
expect(options[0]).not.toBeMarkedAsSelected();
391-
expect(options[1]).toBeMarkedAsSelected();
371+
expect(options[1]).not.toBeMarkedAsSelected();
372+
expect(options[2]).toBeMarkedAsSelected();
392373

393-
scope.selected = 'no match';
374+
// This will select the empty option
375+
scope.selected = null;
394376
scope.$digest();
395377

378+
expect(options[0]).toBeMarkedAsSelected();
379+
expect(options[1]).not.toBeMarkedAsSelected();
380+
expect(options[2]).not.toBeMarkedAsSelected();
381+
382+
// This will add and select the unknown option
383+
scope.selected = 'unmatched value';
384+
scope.$digest();
396385
options = element.find('option');
386+
397387
expect(options[0]).toBeMarkedAsSelected();
398388
expect(options[1]).not.toBeMarkedAsSelected();
399389
expect(options[2]).not.toBeMarkedAsSelected();
390+
expect(options[3]).not.toBeMarkedAsSelected();
391+
392+
// Back to matched value
393+
scope.selected = 'b';
394+
scope.$digest();
395+
options = element.find('option');
396+
397+
expect(options[0]).not.toBeMarkedAsSelected();
398+
expect(options[1]).not.toBeMarkedAsSelected();
399+
expect(options[2]).toBeMarkedAsSelected();
400400
});
401401

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

0 commit comments

Comments
 (0)