Skip to content

Commit

Permalink
fix(select): allow to select first option with value undefined
Browse files Browse the repository at this point in the history
Previously, the value observer incorrectly assumed a value had changed even if
it was the first time it was set, which caused it to remove an option with
the value `undefined` from the internal option map.

Fixes angular#16653
Closes angular#16656
  • Loading branch information
Narretz authored and Kevin Feinberg committed Aug 14, 2018
1 parent 44e15a6 commit 4309063
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ var SelectController =

if (optionAttrs.$attr.ngValue) {
// The value attribute is set by ngValue
var oldVal, hashedVal = NaN;
var oldVal, hashedVal;
optionAttrs.$observe('value', function valueAttributeObserveAction(newVal) {

var removal;
Expand Down
53 changes: 38 additions & 15 deletions test/ng/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1530,11 +1530,14 @@ describe('select', function() {
['a'],
NaN
], function(prop) {

scope.option1 = prop;
scope.option2 = 'red';
scope.selected = 'NOMATCH';

compile('<select ng-model="selected">' +
'<option ng-value="option1">{{option1}}</option>' +
'<option ng-value="option2">{{option2}}</option>' +
'</select>');

scope.$digest();
Expand Down Expand Up @@ -1571,10 +1574,12 @@ describe('select', function() {
NaN
], function(prop) {
scope.option = prop;
scope.option2 = 'red';
scope.selected = 'NOMATCH';

compile('<select ng-model="selected">' +
'<option ng-value="option">{{option}}</option>' +
'<option ng-value="option2">{{option2}}</option>' +
'</select>');

var selectController = element.controller('select');
Expand Down Expand Up @@ -1604,7 +1609,7 @@ describe('select', function() {

expect(scope.selected).toBe(null);
expect(element[0].selectedIndex).toBe(0);
expect(element.find('option').length).toBe(2);
expect(element.find('option').length).toBe(3);
expect(element.find('option').eq(0).prop('selected')).toBe(true);
expect(element.find('option').eq(0).val()).toBe(unknownValue(prop));
expect(element.find('option').eq(1).prop('selected')).toBe(false);
Expand All @@ -1617,6 +1622,7 @@ describe('select', function() {
expect(element.find('option').eq(0).val()).toBe('string:UPDATEDVALUE');
});


it('should interact with custom attribute $observe and $set calls', function() {
var log = [], optionAttr;

Expand All @@ -1638,26 +1644,43 @@ describe('select', function() {
optionAttr.$set('value', 'update');
expect(log[1]).toBe('update');
expect(element.find('option').eq(1).val()).toBe('string:update');
});


it('should ignore the option text / value attribute if the ngValue attribute exists', function() {
scope.ngvalue = 'abc';
scope.value = 'def';
scope.textvalue = 'ghi';

compile('<select ng-model="x"><option ng-value="ngvalue" value="{{value}}">{{textvalue}}</option></select>');
expect(element).toEqualSelect([unknownValue(undefined)], 'string:abc');
});

it('should ignore the option text / value attribute if the ngValue attribute exists', function() {
scope.ngvalue = 'abc';
scope.value = 'def';
scope.textvalue = 'ghi';

compile('<select ng-model="x"><option ng-value="ngvalue" value="{{value}}">{{textvalue}}</option></select>');
expect(element).toEqualSelect([unknownValue(undefined)], 'string:abc');
});
it('should ignore option text with multiple interpolations if the ngValue attribute exists', function() {
scope.ngvalue = 'abc';
scope.textvalue = 'def';
scope.textvalue2 = 'ghi';

it('should ignore option text with multiple interpolations if the ngValue attribute exists', function() {
scope.ngvalue = 'abc';
scope.textvalue = 'def';
scope.textvalue2 = 'ghi';
compile('<select ng-model="x"><option ng-value="ngvalue">{{textvalue}} {{textvalue2}}</option></select>');
expect(element).toEqualSelect([unknownValue(undefined)], 'string:abc');
});


it('should select the first option if it is `undefined`', function() {
scope.selected = undefined;

scope.option1 = undefined;
scope.option2 = 'red';

compile('<select ng-model="selected">' +
'<option ng-value="option1">{{option1}}</option>' +
'<option ng-value="option2">{{option2}}</option>' +
'</select>');

expect(element).toEqualSelect(['undefined:undefined'], 'string:red');
});

compile('<select ng-model="x"><option ng-value="ngvalue">{{textvalue}} {{textvalue2}}</option></select>');
expect(element).toEqualSelect([unknownValue(undefined)], 'string:abc');
});

describe('and select[multiple]', function() {

Expand Down

0 comments on commit 4309063

Please sign in to comment.