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

Commit a4f953f

Browse files
committed
fix(select): update option if interpolated value attribute changes
Previously, the option would only update if the fallback (the interpolated option text) was used. Now the value attribute will be observed if it contains an interpolation. Closes #12005
1 parent cf28c1a commit a4f953f

File tree

2 files changed

+87
-12
lines changed

2 files changed

+87
-12
lines changed

src/ng/directive/select.js

+27-10
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,12 @@ var optionDirective = ['$interpolate', function($interpolate) {
270270
priority: 100,
271271
compile: function(element, attr) {
272272

273-
// If the value attribute is not defined then we fall back to the
274-
// text content of the option element, which may be interpolated
275-
if (isUndefined(attr.value)) {
273+
if (isDefined(attr.value)) {
274+
// If the value attribute is defined, check if it contains an interpolation
275+
var valueInterpolated = $interpolate(attr.value, true);
276+
} else {
277+
// If the value attribute is not defined then we fall back to the
278+
// text content of the option element, which may be interpolated
276279
var interpolateFn = $interpolate(element.text(), true);
277280
if (!interpolateFn) {
278281
attr.$set('value', element.text());
@@ -288,24 +291,38 @@ var optionDirective = ['$interpolate', function($interpolate) {
288291
selectCtrl = parent.data(selectCtrlName) ||
289292
parent.parent().data(selectCtrlName); // in case we are in optgroup
290293

294+
function addOption(optionValue) {
295+
selectCtrl.addOption(optionValue, element);
296+
selectCtrl.ngModelCtrl.$render();
297+
chromeHack(element);
298+
}
299+
291300
// Only update trigger option updates if this is an option within a `select`
292301
// that also has `ngModel` attached
293302
if (selectCtrl && selectCtrl.ngModelCtrl) {
294303

295-
if (interpolateFn) {
304+
if (valueInterpolated) {
305+
// The value attribute is interpolated
306+
var oldVal;
307+
attr.$observe('value', function valueAttributeObserveAction(newVal) {
308+
if (oldVal && oldVal !== newVal) {
309+
selectCtrl.removeOption(oldVal);
310+
}
311+
oldVal = newVal;
312+
addOption(newVal);
313+
});
314+
} else if (interpolateFn) {
315+
// The text content is interpolated
296316
scope.$watch(interpolateFn, function interpolateWatchAction(newVal, oldVal) {
297317
attr.$set('value', newVal);
298318
if (oldVal !== newVal) {
299319
selectCtrl.removeOption(oldVal);
300320
}
301-
selectCtrl.addOption(newVal, element);
302-
selectCtrl.ngModelCtrl.$render();
303-
chromeHack(element);
321+
addOption(newVal);
304322
});
305323
} else {
306-
selectCtrl.addOption(attr.value, element);
307-
selectCtrl.ngModelCtrl.$render();
308-
chromeHack(element);
324+
// Either the value attribute or the text content is static
325+
addOption(attr.value);
309326
}
310327

311328
element.on('$destroy', function() {

test/ng/directive/selectSpec.js

+60-2
Original file line numberDiff line numberDiff line change
@@ -964,22 +964,79 @@ describe('select', function() {
964964

965965
describe('option', function() {
966966

967-
it('should populate value attribute on OPTION', function() {
967+
it('should populate a missing value attribute with the option text', function() {
968968
compile('<select ng-model="x"><option selected>abc</option></select>');
969969
expect(element).toEqualSelect([unknownValue(undefined)], 'abc');
970970
});
971971

972-
it('should ignore value if already exists', function() {
972+
973+
it('should ignore the option text if the value attribute exists', function() {
973974
compile('<select ng-model="x"><option value="abc">xyz</option></select>');
974975
expect(element).toEqualSelect([unknownValue(undefined)], 'abc');
975976
});
976977

978+
977979
it('should set value even if self closing HTML', function() {
978980
scope.x = 'hello';
979981
compile('<select ng-model="x"><option>hello</select>');
980982
expect(element).toEqualSelect(['hello']);
981983
});
982984

985+
986+
it('should add options with interpolated value attributes',
987+
inject(function($rootScope, $compile) {
988+
var scope = $rootScope;
989+
990+
scope.option1 = 'option1';
991+
scope.option2 = 'option2';
992+
993+
var element = $compile(
994+
'<select ng-model="selected">' +
995+
'<option value="{{option1}}">Option 1</option>' +
996+
'<option value="{{option2}}">Option 2</option>' +
997+
'</div>')(scope);
998+
999+
scope.$digest();
1000+
expect(scope.selected).toBeUndefined();
1001+
1002+
browserTrigger(element.find('option').eq(0));
1003+
expect(scope.selected).toBe('option1');
1004+
1005+
scope.selected = 'option2';
1006+
scope.$digest();
1007+
expect(element.find('option').eq(1).prop('selected')).toBe(true);
1008+
expect(element.find('option').eq(1).text()).toBe('Option 2');
1009+
})
1010+
);
1011+
1012+
1013+
it('should update the option when the interpolated value attribute changes',
1014+
inject(function($rootScope, $compile) {
1015+
var scope = $rootScope;
1016+
1017+
scope.option1 = 'option1';
1018+
scope.option2 = 'option2';
1019+
1020+
var element = $compile(
1021+
'<select ng-model="selected">' +
1022+
'<option value="{{option1}}">Option 1</option>' +
1023+
'<option value="{{option2}}">Option 2</option>' +
1024+
'</div>')(scope);
1025+
1026+
scope.$digest();
1027+
expect(scope.selected).toBeUndefined();
1028+
1029+
//Change value of option2
1030+
scope.option2 = 'option2Changed';
1031+
1032+
scope.selected = 'option2Changed';
1033+
scope.$digest();
1034+
expect(element.find('option').eq(1).prop('selected')).toBe(true);
1035+
expect(element.find('option').eq(1).text()).toBe('Option 2');
1036+
})
1037+
);
1038+
1039+
9831040
it('should not blow up when option directive is found inside of a datalist',
9841041
inject(function($compile, $rootScope) {
9851042
var element = $compile('<div>' +
@@ -993,6 +1050,7 @@ describe('select', function() {
9931050
dealoc(element);
9941051
}));
9951052

1053+
9961054
it('should throw an exception if an option value interpolates to "hasOwnProperty"', function() {
9971055
scope.hasOwnPropertyOption = "hasOwnProperty";
9981056
expect(function() {

0 commit comments

Comments
 (0)