Skip to content

Commit 21a6be3

Browse files
fix(select): correctly handle ngOptions and ngMultiple together
Previously, <select ngMultiple='...'> would only work with hard-coded <option> elements underneath, not with an ngOptions attribute. This is because the boolean ng-multiple attribute is only actually applied inside a scope.$watch, which happens after the link function for select is called. The fix is to scope.$watch on the multiple attribute in the select directive, and refresh the directive when it changes. Closes angular#2113
1 parent a170fc1 commit 21a6be3

File tree

2 files changed

+48
-1
lines changed

2 files changed

+48
-1
lines changed

src/ng/directive/select.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,15 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
231231
});
232232
}
233233

234-
if (optionsExp) Options(scope, element, ngModelCtrl);
234+
235+
if (optionsExp) {
236+
scope.$watch(function() {
237+
return jqLite(element).attr('multiple');
238+
}, function(newMultiple) {
239+
multiple = newMultiple;
240+
});
241+
Options(scope, element, ngModelCtrl);
242+
}
235243
else if (multiple) Multiple(scope, element, ngModelCtrl);
236244
else Single(scope, element, ngModelCtrl, selectCtrl);
237245

test/ng/directive/selectSpec.js

+39
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,14 @@ describe('select', function() {
490490
}, blank, unknown);
491491
}
492492

493+
function createNgMultiSelect(blank, unknown) {
494+
createSelect({
495+
'ng-model':'selected',
496+
'ng-multiple':'multi',
497+
'ng-options':'value.id as value.name for value in values'
498+
}, blank, unknown);
499+
}
500+
493501

494502
it('should throw when not formated "? for ? in ?"', function() {
495503
expect(function() {
@@ -1107,6 +1115,37 @@ describe('select', function() {
11071115
expect(element.find('option')[1].selected).toBeTruthy();
11081116
});
11091117

1118+
it('should work with ng-multiple and ng-options', function(){
1119+
createNgMultiSelect();
1120+
1121+
scope.$apply(function(){
1122+
scope.values = [{name: 'A', id: 'A'}, {name: 'B', id: 'A'}];
1123+
scope.selected = [];
1124+
scope.multi = false;
1125+
});
1126+
1127+
expect(element.attr('multiple')).toBeFalsy();
1128+
1129+
scope.$apply(function(){
1130+
scope.multi = true;
1131+
});
1132+
1133+
expect(element.attr('multiple')).toBeTruthy();
1134+
1135+
element.find('option')[0].selected = true;
1136+
element.find('option')[1].selected = true;
1137+
browserTrigger(element, 'change');
1138+
1139+
expect(scope.selected.length).toBe(2);
1140+
1141+
scope.$apply(function(){
1142+
scope.selected = ['A'];
1143+
});
1144+
1145+
expect(element.find('option')[0].selected).toBeTruthy();
1146+
expect(element.find('option')[1].selected).toBeFalsy();
1147+
});
1148+
11101149

11111150
it('should update model on change', function() {
11121151
createMultiSelect();

0 commit comments

Comments
 (0)