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

Commit c75698d

Browse files
committed
fix(select): add/remove selected attribute for selected/unselected options
1 parent 2785ad7 commit c75698d

File tree

3 files changed

+88
-12
lines changed

3 files changed

+88
-12
lines changed

src/ng/directive/ngOptions.js

-5
Original file line numberDiff line numberDiff line change
@@ -504,11 +504,6 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
504504

505505
} else {
506506

507-
ngModelCtrl.$isEmpty = function(value) {
508-
return !value || value.length === 0;
509-
};
510-
511-
512507
selectCtrl.writeValue = function writeNgOptionsMultiple(value) {
513508
options.items.forEach(function(option) {
514509
option.element.selected = false;

src/ng/directive/select.js

+16-7
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,14 @@ var SelectController =
3636
var unknownVal = self.generateUnknownOptionValue(val);
3737
self.unknownOption.val(unknownVal);
3838
$element.prepend(self.unknownOption);
39-
self.unknownOption.prop('selected', true); // needed for IE
40-
self.unknownOption.attr('selected', true);
39+
setOptionAsSelected(self.unknownOption);
4140
$element.val(unknownVal);
4241
};
4342

4443
self.updateUnknownOption = function(val) {
4544
var unknownVal = self.generateUnknownOptionValue(val);
4645
self.unknownOption.val(unknownVal);
47-
self.unknownOption.prop('selected', true); // needed for IE
48-
self.unknownOption.attr('selected', true);
46+
setOptionAsSelected(self.unknownOption);
4947
$element.val(unknownVal);
5048
};
5149

@@ -67,8 +65,7 @@ var SelectController =
6765
self.selectEmptyOption = function() {
6866
if (self.emptyOption) {
6967
$element.val('');
70-
self.emptyOption.prop('selected', true); // needed for IE
71-
self.emptyOption.attr('selected', true);
68+
setOptionAsSelected(self.emptyOption);
7269
}
7370
};
7471

@@ -101,13 +98,20 @@ var SelectController =
10198
// Write the value to the select control, the implementation of this changes depending
10299
// upon whether the select can have multiple values and whether ngOptions is at work.
103100
self.writeValue = function writeSingleValue(value) {
101+
// Make sure to remove the selected attribute from the previously selected option
102+
// Otherwise, screen readers might get confused
103+
var currentlySelectedOption = $element[0].options[$element[0].selectedIndex];
104+
if (currentlySelectedOption) currentlySelectedOption.removeAttribute('selected');
105+
104106
if (self.hasOption(value)) {
105107
self.removeUnknownOption();
106108

107109
var hashedVal = hashKey(value);
108110
$element.val(hashedVal in self.selectValueMap ? hashedVal : value);
109111

110-
if (value === '') self.emptyOption.prop('selected', true); // to make IE9 happy
112+
// Set selected attribute and property on selected option for screen readers
113+
var selectedOption = $element[0].options[$element[0].selectedIndex];
114+
setOptionAsSelected(jqLite(selectedOption));
111115
} else {
112116
if (value == null && self.emptyOption) {
113117
self.removeUnknownOption();
@@ -283,6 +287,11 @@ var SelectController =
283287
}
284288
});
285289
};
290+
291+
function setOptionAsSelected(optionEl) {
292+
optionEl.prop('selected', true); // needed for IE
293+
optionEl.attr('selected', true);
294+
}
286295
}];
287296

288297
/**

test/ng/directive/selectSpec.js

+72
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,44 @@ describe('select', function() {
337337
dealoc(select);
338338
});
339339

340+
341+
it('should remove the "selected" attribute from the previous option when the model changes', function() {
342+
compile('<select name="select" ng-model="selected">' +
343+
'<option value="a">A</option>' +
344+
'<option value="b">B</option>' +
345+
'</select>');
346+
347+
scope.$digest();
348+
349+
var options = element.find('option');
350+
expect(options[0]).toBeMarkedAsSelected();
351+
expect(options[1]).not.toBeMarkedAsSelected();
352+
expect(options[2]).not.toBeMarkedAsSelected();
353+
354+
scope.selected = 'a';
355+
scope.$digest();
356+
357+
options = element.find('option');
358+
expect(options.length).toBe(2);
359+
expect(options[0]).toBeMarkedAsSelected();
360+
expect(options[1]).not.toBeMarkedAsSelected();
361+
362+
scope.selected = 'b';
363+
scope.$digest();
364+
365+
options = element.find('option');
366+
expect(options[0]).not.toBeMarkedAsSelected();
367+
expect(options[1]).toBeMarkedAsSelected();
368+
369+
scope.selected = 'no match';
370+
scope.$digest();
371+
372+
options = element.find('option');
373+
expect(options[0]).toBeMarkedAsSelected();
374+
expect(options[1]).not.toBeMarkedAsSelected();
375+
expect(options[2]).not.toBeMarkedAsSelected();
376+
});
377+
340378
describe('empty option', function() {
341379

342380
it('should allow empty option to be added and removed dynamically', function() {
@@ -525,6 +563,40 @@ describe('select', function() {
525563
expect(element).toEqualSelect([''], 'wallee');
526564
});
527565
});
566+
567+
it('should add/remove the "selected" attribute when the empty option is selected/unselected', function() {
568+
compile('<select name="select" ng-model="selected">' +
569+
'<option value="">--select--</option>' +
570+
'<option value="a">A</option>' +
571+
'<option value="b">B</option>' +
572+
'</select>');
573+
574+
scope.$digest();
575+
576+
var options = element.find('option');
577+
expect(options.length).toBe(3);
578+
expect(options[0]).toBeMarkedAsSelected();
579+
expect(options[1]).not.toBeMarkedAsSelected();
580+
expect(options[2]).not.toBeMarkedAsSelected();
581+
582+
scope.selected = 'a';
583+
scope.$digest();
584+
585+
options = element.find('option');
586+
expect(options.length).toBe(3);
587+
expect(options[0]).not.toBeMarkedAsSelected();
588+
expect(options[1]).toBeMarkedAsSelected();
589+
expect(options[2]).not.toBeMarkedAsSelected();
590+
591+
scope.selected = 'no match';
592+
scope.$digest();
593+
594+
options = element.find('option');
595+
expect(options[0]).toBeMarkedAsSelected();
596+
expect(options[1]).not.toBeMarkedAsSelected();
597+
expect(options[2]).not.toBeMarkedAsSelected();
598+
});
599+
528600
});
529601

530602

0 commit comments

Comments
 (0)