Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 14f8ce3

Browse files
committedFeb 5, 2017
fix(select): add attribute "selected" for select[multiple]
This helps screen readers identify the selected options, see angular#14419
1 parent 1078ec3 commit 14f8ce3

File tree

2 files changed

+25
-10
lines changed

2 files changed

+25
-10
lines changed
 

‎src/ng/directive/select.js

+17-10
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44

55
var noopNgModelController = { $setViewValue: noop, $render: noop };
66

7+
function setOptionAsSelected(optionEl) {
8+
optionEl.prop('selected', true); // needed for IE
9+
optionEl.attr('selected', true);
10+
}
11+
712
/**
813
* @ngdoc type
914
* @name select.SelectController
@@ -292,11 +297,6 @@ var SelectController =
292297
}
293298
});
294299
};
295-
296-
function setOptionAsSelected(optionEl) {
297-
optionEl.prop('selected', true); // needed for IE
298-
optionEl.attr('selected', true);
299-
}
300300
}];
301301

302302
/**
@@ -611,11 +611,18 @@ var selectDirective = function() {
611611
includes(value, selectCtrl.selectValueMap[option.value]));
612612
var currentlySelected = option.selected;
613613

614-
// IE and Edge will de-select selected options when you set the selected property again, e.g.
615-
// when you add to the selection via shift+click/UP/DOWN
616-
// Therefore, only set it if necessary
617-
if ((shouldBeSelected && !currentlySelected) || (!shouldBeSelected && currentlySelected)) {
618-
option.selected = shouldBeSelected;
614+
// IE and Edge, adding options to the selection via shift+click/UP/DOWN,
615+
// will de-select already selected options if "selected" on those options was set
616+
// more than once (i.e. when the options were already selected)
617+
// So we only modify the selected property if neccessary.
618+
// Note: this behavior cannot be replicated via unit tests because it only shows in the
619+
// actual user interface.
620+
if ((shouldBeSelected && !currentlySelected)) {
621+
setOptionAsSelected(jqLite(option));
622+
} else if ((!shouldBeSelected && currentlySelected)) {
623+
option.selected = shouldBeSelected;
624+
// Remove attribute to not confuse screen readers
625+
option.removeAttribute('selected');
619626
}
620627

621628
});

‎test/ng/directive/selectSpec.js

+8
Original file line numberDiff line numberDiff line change
@@ -1098,13 +1098,21 @@ describe('select', function() {
10981098
scope.selection = ['A'];
10991099
});
11001100

1101+
var optionElements = element.find('option');
1102+
11011103
expect(element).toEqualSelect(['A'], 'B');
1104+
expect(optionElements[0]).toBeMarkedAsSelected();
1105+
expect(optionElements[1]).not.toBeMarkedAsSelected();
11021106

11031107
scope.$apply(function() {
11041108
scope.selection.push('B');
11051109
});
11061110

1111+
optionElements = element.find('option');
1112+
11071113
expect(element).toEqualSelect(['A'], ['B']);
1114+
expect(optionElements[0]).toBeMarkedAsSelected();
1115+
expect(optionElements[1]).toBeMarkedAsSelected();
11081116
});
11091117

11101118
it('should work with optgroups', function() {

0 commit comments

Comments
 (0)
Please sign in to comment.