Skip to content

Commit f57baeb

Browse files
NarretzNarretz
committed
f[eat|ix](select): support values added by ngValue
select elements with ngModel will now set ngModel to option values added by ngValue. This allows setting values of any type without the use of ngOptions. Interpolations inside attributes can only be strings, but the ngValue directive uses attrs.$set, which does not follow any type restriction. Any $observe on the value attribute will therefore receive the original value (result of ngValue expression). However, when a user selects an option, the browser sets the select value to the actual option's value attribute, which is still always a string. For that reason, when option are added by ngValue, we set the hashed value of the original value in the value attribute and store the actual value in an extra map. When the select value changes, we read access the actual value via the hashed select value. Closes angular#9842 Closes angular#6297
1 parent c4e47e4 commit f57baeb

File tree

2 files changed

+165
-8
lines changed

2 files changed

+165
-8
lines changed

‎src/ng/directive/select.js

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ var SelectController =
2323
['$element', '$scope', '$attrs', function($element, $scope, $attrs) {
2424

2525
var self = this,
26-
optionsMap = new HashMap();
26+
optionsMap = new HashMap(),
27+
selectValueMap = {}; // Keys are the hashed values, values the original values
2728

2829
// If the ngModel doesn't get provided then provide a dummy noop version to prevent errors
2930
self.ngModelCtrl = noopNgModelController;
@@ -56,7 +57,9 @@ var SelectController =
5657
// upon whether the select can have multiple values and whether ngOptions is at work.
5758
self.readValue = function readSingleValue() {
5859
self.removeUnknownOption();
59-
return $element.val();
60+
var val = $element.val();
61+
// ngValue added option values are stored in the selectValueMap, normal interpolations are not
62+
return val in selectValueMap ? selectValueMap[val] : val;
6063
};
6164

6265

@@ -116,9 +119,26 @@ var SelectController =
116119

117120
self.registerOption = function(optionScope, optionElement, optionAttrs, interpolateValueFn, interpolateTextFn) {
118121

119-
if (interpolateValueFn) {
122+
if (interpolateValueFn === true) {
123+
// The value attribute is set by ngValue
124+
var oldVal, hashedVal = NaN;
125+
optionAttrs.$observe('value', function valueAttributeObserveAction(newVal) {
126+
if (isDefined(hashedVal)) {
127+
self.removeOption(oldVal);
128+
delete selectValueMap[hashedVal];
129+
}
130+
131+
hashedVal = hashKey(newVal);
132+
oldVal = newVal;
133+
self.addOption(newVal, optionElement);
134+
selectValueMap[hashedVal] = newVal;
135+
// Set the attribute directly instead of using optionAttrs.$set - this stops the observer
136+
// from firing a second time. Other $observers on value will also get the result of the
137+
// ngValue expression, not the hashed value
138+
optionElement.attr('value', hashedVal);
139+
});
140+
} else if (interpolateValueFn) {
120141
// The value attribute is interpolated
121-
var oldVal;
122142
optionAttrs.$observe('value', function valueAttributeObserveAction(newVal) {
123143
if (isDefined(oldVal)) {
124144
self.removeOption(oldVal);
@@ -455,9 +475,13 @@ var optionDirective = ['$interpolate', function($interpolate) {
455475
restrict: 'E',
456476
priority: 100,
457477
compile: function(element, attr) {
458-
if (isDefined(attr.value)) {
478+
var interpolateValueFn;
479+
480+
if (isDefined(attr.ngValue)) {
481+
interpolateValueFn = true;
482+
} else if (isDefined(attr.value)) {
459483
// If the value attribute is defined, check if it contains an interpolation
460-
var interpolateValueFn = $interpolate(attr.value, true);
484+
interpolateValueFn = $interpolate(attr.value, true);
461485
} else {
462486
// If the value attribute is not defined then we fall back to the
463487
// text content of the option element, which may be interpolated

‎test/ng/directive/selectSpec.js

Lines changed: 135 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

3-
describe('select', function() {
4-
var scope, formElement, element, $compile, ngModelCtrl, selectCtrl, renderSpy;
3+
ddescribe('select', function() {
4+
var scope, formElement, element, $compile, ngModelCtrl, selectCtrl, renderSpy, optionAttributesList = [];
55

66
function compile(html) {
77
formElement = jqLite('<form name="form">' + html + '</form>');
@@ -55,6 +55,18 @@ describe('select', function() {
5555
'</option>'
5656
};
5757
});
58+
59+
$compileProvider.directive('exposeAttributes', function() {
60+
return {
61+
require: '^^select',
62+
link: {
63+
pre: function(scope, element, attrs, ctrl) {
64+
optionAttributesList.push(attrs);
65+
}
66+
}
67+
};
68+
});
69+
5870
}));
5971

6072
beforeEach(inject(function($rootScope, _$compile_) {
@@ -1210,5 +1222,126 @@ describe('select', function() {
12101222
}).toThrowMinErr('ng','badname', 'hasOwnProperty is not a valid "option value" name');
12111223
});
12121224

1225+
describe('with ngValue', function() {
1226+
1227+
they('should set the option attribute and select it for value $prop', [
1228+
'string',
1229+
undefined,
1230+
1,
1231+
true,
1232+
null,
1233+
{prop: 'value'},
1234+
['a'],
1235+
NaN
1236+
], function(prop) {
1237+
scope.option1 = prop;
1238+
scope.selected = 'NOMATCH';
1239+
1240+
compile('<select ng-model="selected">' +
1241+
'<option ng-value="option1">{{option1}}</option>' +
1242+
'</select>');
1243+
1244+
scope.$digest();
1245+
expect(element.find('option').eq(0).val()).toBe('? string:NOMATCH ?');
1246+
1247+
scope.selected = prop;
1248+
scope.$digest();
1249+
1250+
expect(element.find('option').eq(0).val()).toBe(hashKey(prop));
1251+
1252+
// Reset
1253+
scope.selected = false;
1254+
scope.$digest();
1255+
1256+
expect(element.find('option').eq(0).val()).toBe('? boolean:false ?');
1257+
1258+
browserTrigger(element.find('option').eq(0));
1259+
if (Number.isNaN(prop)) {
1260+
expect(scope.selected).toBeNaN();
1261+
} else {
1262+
expect(scope.selected).toBe(prop);
1263+
}
1264+
});
1265+
1266+
1267+
tthey('should update the option attribute and select it for value $prop', [
1268+
'string',
1269+
// undefined,
1270+
// 1,
1271+
// true,
1272+
// null,
1273+
// {prop: 'value'},
1274+
// ['a'],
1275+
// NaN
1276+
], function(prop) {
1277+
scope.option = prop;
1278+
scope.selected = 'NOMATCH';
1279+
1280+
compile('<select ng-model="selected">' +
1281+
'<option ng-value="option">{{option}}</option>' +
1282+
'</select>');
1283+
1284+
var selectController = element.controller('select');
1285+
spyOn(selectController, 'removeOption').andCallThrough();
1286+
1287+
scope.$digest();
1288+
expect(selectController.removeOption).not.toHaveBeenCalled();
1289+
expect(element.find('option').eq(0).val()).toBe('? string:NOMATCH ?');
1290+
1291+
scope.selected = prop;
1292+
scope.$digest();
1293+
1294+
expect(element.find('option').eq(0).val()).toBe(hashKey(prop));
1295+
1296+
scope.option = 'UPDATEDVALUE';
1297+
scope.$digest();
1298+
1299+
expect(selectController.removeOption.calls.length).toBe(1);
1300+
1301+
// Updating the option value currently does not update the select model
1302+
if (Number.isNaN(prop)) {
1303+
expect(selectController.removeOption.calls[0].args[0]).toBeNaN();
1304+
expect(scope.selected).toBeNaN();
1305+
} else {
1306+
expect(selectController.removeOption.calls[0].args[0]).toBe(prop);
1307+
expect(scope.selected).toBe(prop);
1308+
}
1309+
1310+
expect(element.find('option').eq(0).prop('selected')).toBe(true);
1311+
expect(element.find('option').eq(1).val()).toBe('string:UPDATEDVALUE');
1312+
1313+
scope.selected = 'UPDATEDVALUE';
1314+
scope.$digest();
1315+
1316+
expect(element.find('option').eq(0).prop('selected')).toBe(true);
1317+
expect(element.find('option').eq(0).val()).toBe('string:UPDATEDVALUE');
1318+
});
1319+
1320+
it('should interact with custom attribute $observe and $set calls', function() {
1321+
var log = [], optionAttr;
1322+
1323+
compile('<select ng-model="selected">' +
1324+
'<option expose-attributes ng-value="option">{{option}}</option>' +
1325+
'</select>');
1326+
1327+
optionAttr = optionAttributesList[0];
1328+
optionAttr.$observe('value', function(newVal) {
1329+
log.push(newVal);
1330+
});
1331+
1332+
scope.option = 'init';
1333+
scope.$digest();
1334+
1335+
expect(log[0]).toBe('init');
1336+
expect(element.find('option').eq(1).val()).toBe('string:init');
1337+
1338+
optionAttr.$set('value', 'update');
1339+
expect(log[1]).toBe('update');
1340+
expect(element.find('option').eq(1).val()).toBe('string:update');
1341+
1342+
});
1343+
1344+
});
1345+
12131346
});
12141347
});

0 commit comments

Comments
 (0)