Skip to content

Commit df0bc97

Browse files
committed
fix(ngOptions): remove selected attribute from unselected options
When the select model changes, we add the "selected" attribute to the selected option, so that screen readers know which option is selected. Previously, we failed to remove the attribute when the model changed to match a different option, or the unknown / empty option. When using "track by", the behavior would also show when a user selected an option, and then the model was changed, because track by watches the tracked expression, and calls the $render function on change. This fix reads the current select value, finds the matching option and removes the "selected" attribute. Fixes angular#14892 Fixes angular#14419 Related angular#12731
1 parent 5fd42b6 commit df0bc97

File tree

2 files changed

+59
-0
lines changed

2 files changed

+59
-0
lines changed

src/ng/directive/ngOptions.js

+6
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,13 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
467467
if (!multiple) {
468468

469469
selectCtrl.writeValue = function writeNgOptionsValue(value) {
470+
var selectedOption = options.selectValueMap[selectElement.val()];
470471
var option = options.getOptionFromViewValue(value);
471472

473+
// Make sure to remove the selected attribute from the previously selected option
474+
// Otherwise, screen readers might get confused
475+
if (selectedOption) selectedOption.element.removeAttribute('selected');
476+
472477
if (option) {
473478
// Don't update the option when it is already selected.
474479
// For example, the browser will select the first option by default. In that case,
@@ -509,6 +514,7 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
509514

510515
// If we are using `track by` then we must watch the tracked value on the model
511516
// since ngModel only watches for object identity change
517+
// FIXME: When a user selects an option, this watch will fire needlessly
512518
if (ngOptions.trackBy) {
513519
scope.$watch(
514520
function() { return ngOptions.getTrackByValue(ngModelCtrl.$viewValue); },

test/ng/directive/ngOptionsSpec.js

+53
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,37 @@ describe('ngOptions', function() {
743743
expect(options.eq(3)).toEqualOption('c');
744744
});
745745

746+
it('should remove the "selected" attribute from the previous option when the model changes', function() {
747+
scope.values = [{id: 10, label: 'ten'}, {id:20, label: 'twenty'}];
748+
749+
createSelect({
750+
'ng-model': 'selected',
751+
'ng-options': 'item.label for item in values'
752+
}, true);
753+
754+
var resetButton = angular.element('<input type="reset" />');
755+
formElement.append(resetButton);
756+
757+
scope.selected = scope.values[0];
758+
scope.$digest();
759+
760+
var options = element.find('option');
761+
expect(options[1].getAttribute('selected')).toBe('selected');
762+
763+
scope.selected = scope.values[1];
764+
scope.$digest();
765+
766+
expect(options[1].getAttribute('selected')).toBe(null);
767+
expect(options[2].getAttribute('selected')).toBe('selected');
768+
769+
scope.selected = 'no match';
770+
scope.$digest();
771+
772+
expect(options[0].selected).toBe(true);
773+
expect(options[0].getAttribute('selected')).toBe('selected');
774+
expect(options[1].getAttribute('selected')).toBe(null);
775+
expect(options[2].getAttribute('selected')).toBe(null);
776+
});
746777

747778
describe('disableWhen expression', function() {
748779

@@ -1395,6 +1426,28 @@ describe('ngOptions', function() {
13951426
});
13961427
}).not.toThrow();
13971428
});
1429+
1430+
it('should remove the "selected" attribute when the model changes', function() {
1431+
createSelect({
1432+
'ng-model': 'selected',
1433+
'ng-options': 'item.label for item in arr track by item.id'
1434+
});
1435+
1436+
var options = element.find('option');
1437+
browserTrigger(options[2], 'click');
1438+
1439+
expect(scope.selected).toEqual(scope.arr[1]);
1440+
1441+
scope.selected = {};
1442+
scope.$digest();
1443+
1444+
options[2].selected = false;
1445+
expect(options[0].selected).toBe(true);
1446+
expect(options[0].getAttribute('selected')).toBe('selected');
1447+
expect(options[2].selected).toBe(false);
1448+
expect(options[2].getAttribute('selected')).toBe(null);
1449+
});
1450+
13981451
});
13991452

14001453

0 commit comments

Comments
 (0)