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

Commit 7ae0a5b

Browse files
NarretzNarretz
committed
feat(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 (not only strings) without the use of ngOptions. Interpolations inside attributes can only be strings, but the ngValue directive uses attrs.$set, which does not have 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 options 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. Since we only use a hashed value for ngValue, we will have extra checks for the hashed values: - when options are read, for both single and multiple select - when options are written, for multiple select I don't expect this to have a performance impact, but it should be kept in mind. Closes #9842 Closes #6297 BREAKING CHANGE: `<option>` elements added to `<select ng-model>` via `ngValue` now add their values in hash form, i.e. `<option ng-value="myString">` becomes `<option ng-value="myString" value="string:myString">`. This is done to support binding options with values of any type to selects. This should rarely affect applications, as the values of options are usually not relevant to the application logic, but it's possible that option values are checked in tests.
1 parent d7274f0 commit 7ae0a5b

File tree

2 files changed

+243
-8
lines changed

2 files changed

+243
-8
lines changed

‎src/ng/directive/select.js

+33-7
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ var SelectController =
2525
var self = this,
2626
optionsMap = new HashMap();
2727

28+
self.selectValueMap = {}; // Keys are the hashed values, values the original values
29+
2830
// If the ngModel doesn't get provided then provide a dummy noop version to prevent errors
2931
self.ngModelCtrl = noopNgModelController;
3032

@@ -56,7 +58,9 @@ var SelectController =
5658
// upon whether the select can have multiple values and whether ngOptions is at work.
5759
self.readValue = function readSingleValue() {
5860
self.removeUnknownOption();
59-
return $element.val();
61+
var val = $element.val();
62+
// ngValue added option values are stored in the selectValueMap, normal interpolations are not
63+
return val in self.selectValueMap ? self.selectValueMap[val] : val;
6064
};
6165

6266

@@ -116,9 +120,26 @@ var SelectController =
116120

117121
self.registerOption = function(optionScope, optionElement, optionAttrs, interpolateValueFn, interpolateTextFn) {
118122

119-
if (interpolateValueFn) {
123+
if (interpolateValueFn === true) {
124+
// The value attribute is set by ngValue
125+
var oldVal, hashedVal = NaN;
126+
optionAttrs.$observe('value', function valueAttributeObserveAction(newVal) {
127+
if (isDefined(hashedVal)) {
128+
self.removeOption(oldVal);
129+
delete self.selectValueMap[hashedVal];
130+
}
131+
132+
hashedVal = hashKey(newVal);
133+
oldVal = newVal;
134+
self.addOption(newVal, optionElement);
135+
self.selectValueMap[hashedVal] = newVal;
136+
// Set the attribute directly instead of using optionAttrs.$set - this stops the observer
137+
// from firing a second time. Other $observers on value will also get the result of the
138+
// ngValue expression, not the hashed value
139+
optionElement.attr('value', hashedVal);
140+
});
141+
} else if (interpolateValueFn) {
120142
// The value attribute is interpolated
121-
var oldVal;
122143
optionAttrs.$observe('value', function valueAttributeObserveAction(newVal) {
123144
if (isDefined(oldVal)) {
124145
self.removeOption(oldVal);
@@ -394,7 +415,8 @@ var selectDirective = function() {
394415
var array = [];
395416
forEach(element.find('option'), function(option) {
396417
if (option.selected) {
397-
array.push(option.value);
418+
var val = option.value;
419+
array.push(val in selectCtrl.selectValueMap ? selectCtrl.selectValueMap[val] : val);
398420
}
399421
});
400422
return array;
@@ -404,7 +426,7 @@ var selectDirective = function() {
404426
selectCtrl.writeValue = function writeMultipleValue(value) {
405427
var items = new HashMap(value);
406428
forEach(element.find('option'), function(option) {
407-
option.selected = isDefined(items.get(option.value));
429+
option.selected = isDefined(items.get(option.value)) || isDefined(items.get(selectCtrl.selectValueMap[option.value]));
408430
});
409431
};
410432

@@ -455,9 +477,13 @@ var optionDirective = ['$interpolate', function($interpolate) {
455477
restrict: 'E',
456478
priority: 100,
457479
compile: function(element, attr) {
458-
if (isDefined(attr.value)) {
480+
var interpolateValueFn;
481+
482+
if (isDefined(attr.ngValue)) {
483+
interpolateValueFn = true;
484+
} else if (isDefined(attr.value)) {
459485
// If the value attribute is defined, check if it contains an interpolation
460-
var interpolateValueFn = $interpolate(attr.value, true);
486+
interpolateValueFn = $interpolate(attr.value, true);
461487
} else {
462488
// If the value attribute is not defined then we fall back to the
463489
// text content of the option element, which may be interpolated

‎test/ng/directive/selectSpec.js

+210-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

33
describe('select', function() {
4-
var scope, formElement, element, $compile, ngModelCtrl, selectCtrl, renderSpy;
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_) {
@@ -1224,5 +1236,202 @@ describe('select', function() {
12241236
}).toThrowMinErr('ng','badname', 'hasOwnProperty is not a valid "option value" name');
12251237
});
12261238

1239+
describe('with ngValue (and non-primitive values)', function() {
1240+
1241+
they('should set the option attribute and select it for value $prop', [
1242+
'string',
1243+
undefined,
1244+
1,
1245+
true,
1246+
null,
1247+
{prop: 'value'},
1248+
['a'],
1249+
NaN
1250+
], function(prop) {
1251+
scope.option1 = prop;
1252+
scope.selected = 'NOMATCH';
1253+
1254+
compile('<select ng-model="selected">' +
1255+
'<option ng-value="option1">{{option1}}</option>' +
1256+
'</select>');
1257+
1258+
scope.$digest();
1259+
expect(element.find('option').eq(0).val()).toBe('? string:NOMATCH ?');
1260+
1261+
scope.selected = prop;
1262+
scope.$digest();
1263+
1264+
expect(element.find('option').eq(0).val()).toBe(hashKey(prop));
1265+
1266+
// Reset
1267+
scope.selected = false;
1268+
scope.$digest();
1269+
1270+
expect(element.find('option').eq(0).val()).toBe('? boolean:false ?');
1271+
1272+
browserTrigger(element.find('option').eq(0));
1273+
if (typeof prop === 'number' && isNaN(prop)) {
1274+
expect(scope.selected).toBeNaN();
1275+
} else {
1276+
expect(scope.selected).toBe(prop);
1277+
}
1278+
});
1279+
1280+
1281+
they('should update the option attribute and select it for value $prop', [
1282+
'string',
1283+
undefined,
1284+
1,
1285+
true,
1286+
null,
1287+
{prop: 'value'},
1288+
['a'],
1289+
NaN
1290+
], function(prop) {
1291+
scope.option = prop;
1292+
scope.selected = 'NOMATCH';
1293+
1294+
compile('<select ng-model="selected">' +
1295+
'<option ng-value="option">{{option}}</option>' +
1296+
'</select>');
1297+
1298+
var selectController = element.controller('select');
1299+
spyOn(selectController, 'removeOption').and.callThrough();
1300+
1301+
scope.$digest();
1302+
expect(selectController.removeOption).not.toHaveBeenCalled();
1303+
expect(element.find('option').eq(0).val()).toBe('? string:NOMATCH ?');
1304+
1305+
scope.selected = prop;
1306+
scope.$digest();
1307+
1308+
expect(element.find('option').eq(0).val()).toBe(hashKey(prop));
1309+
1310+
scope.option = 'UPDATEDVALUE';
1311+
scope.$digest();
1312+
1313+
expect(selectController.removeOption.calls.count()).toBe(1);
1314+
1315+
// Updating the option value currently does not update the select model
1316+
if (typeof prop === 'number' && isNaN(prop)) {
1317+
expect(selectController.removeOption.calls.argsFor(0)[0]).toBeNaN();
1318+
expect(scope.selected).toBeNaN();
1319+
} else {
1320+
expect(selectController.removeOption.calls.argsFor(0)[0]).toBe(prop);
1321+
expect(scope.selected).toBe(prop);
1322+
}
1323+
1324+
expect(element.find('option').eq(0).prop('selected')).toBe(true);
1325+
expect(element.find('option').eq(1).val()).toBe('string:UPDATEDVALUE');
1326+
1327+
scope.selected = 'UPDATEDVALUE';
1328+
scope.$digest();
1329+
1330+
// expect(element.find('option').eq(0).prop('selected')).toBe(true); not selected in Chrome?
1331+
expect(element.find('option').eq(0).val()).toBe('string:UPDATEDVALUE');
1332+
});
1333+
1334+
it('should interact with custom attribute $observe and $set calls', function() {
1335+
var log = [], optionAttr;
1336+
1337+
compile('<select ng-model="selected">' +
1338+
'<option expose-attributes ng-value="option">{{option}}</option>' +
1339+
'</select>');
1340+
1341+
optionAttr = optionAttributesList[0];
1342+
optionAttr.$observe('value', function(newVal) {
1343+
log.push(newVal);
1344+
});
1345+
1346+
scope.option = 'init';
1347+
scope.$digest();
1348+
1349+
expect(log[0]).toBe('init');
1350+
expect(element.find('option').eq(1).val()).toBe('string:init');
1351+
1352+
optionAttr.$set('value', 'update');
1353+
expect(log[1]).toBe('update');
1354+
expect(element.find('option').eq(1).val()).toBe('string:update');
1355+
1356+
});
1357+
1358+
describe('and select[multiple]', function() {
1359+
1360+
it('should allow multiple selection', function() {
1361+
scope.options = {
1362+
a: 'string',
1363+
b: undefined,
1364+
c: 1,
1365+
d: true,
1366+
e: null,
1367+
f: {prop: 'value'},
1368+
g: ['a'],
1369+
h: NaN
1370+
};
1371+
scope.selected = [];
1372+
1373+
compile('<select multiple ng-model="selected">' +
1374+
'<option ng-value="options.a">{{options.a}}</option>' +
1375+
'<option ng-value="options.b">{{options.b}}</option>' +
1376+
'<option ng-value="options.c">{{options.c}}</option>' +
1377+
'<option ng-value="options.d">{{options.d}}</option>' +
1378+
'<option ng-value="options.e">{{options.e}}</option>' +
1379+
'<option ng-value="options.f">{{options.f}}</option>' +
1380+
'<option ng-value="options.g">{{options.g}}</option>' +
1381+
'<option ng-value="options.h">{{options.h}}</option>' +
1382+
'</select>');
1383+
1384+
scope.$digest();
1385+
expect(element).toEqualSelect(
1386+
'string:string',
1387+
'undefined:undefined',
1388+
'number:1',
1389+
'boolean:true',
1390+
'object:null',
1391+
'object:4',
1392+
'object:5',
1393+
'number:NaN'
1394+
);
1395+
1396+
scope.selected = ['string', 1];
1397+
scope.$digest();
1398+
1399+
expect(element.find('option').eq(0).prop('selected')).toBe(true);
1400+
expect(element.find('option').eq(2).prop('selected')).toBe(true);
1401+
1402+
browserTrigger(element.find('option').eq(1));
1403+
expect(scope.selected).toEqual([undefined]);
1404+
1405+
//reset
1406+
scope.selected = [];
1407+
scope.$digest();
1408+
1409+
forEach(element.find('option'), function(option) {
1410+
// browserTrigger can't produce click + ctrl, so set selection manually
1411+
jqLite(option).prop('selected', true);
1412+
});
1413+
1414+
browserTrigger(element, 'change');
1415+
1416+
var arrayVal = ['a'];
1417+
arrayVal.$$hashKey = 'object:5';
1418+
1419+
expect(scope.selected).toEqual([
1420+
'string',
1421+
undefined,
1422+
1,
1423+
true,
1424+
null,
1425+
{prop: 'value', $$hashKey: 'object:4'},
1426+
arrayVal,
1427+
NaN
1428+
]);
1429+
});
1430+
1431+
});
1432+
1433+
1434+
});
1435+
12271436
});
12281437
});

0 commit comments

Comments
 (0)