Skip to content

Commit 6adbc1e

Browse files
author
Erin Altenhof-Long
committed
fix(select): alter detection of unnecessary selections
A previous patch attempted to avoid unnecessary selections on hover in Firefox by skipping selection changes when no change event was fired. However, the alteration in "lastElement[0].selected" to "existingOption.selected" effectively checked incorrect values. This caused other select issues, particularly with the disabled attribute. angular#7715
1 parent d89bacd commit 6adbc1e

File tree

2 files changed

+39
-4
lines changed

2 files changed

+39
-4
lines changed

src/ng/directive/select.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
537537
lastElement.val(existingOption.id = option.id);
538538
}
539539
// lastElement.prop('selected') provided by jQuery has side-effects
540-
if (existingOption.selected !== option.selected) {
540+
if (lastElement[0].selected !== option.selected) {
541541
lastElement.prop('selected', (existingOption.selected = option.selected));
542542
}
543543
} else {

test/ng/directive/selectSpec.js

+38-3
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ describe('select', function() {
458458

459459

460460
describe('ngOptions', function() {
461-
function createSelect(attrs, blank, unknown) {
461+
function createSelect(attrs, blank, unknown, disabled) {
462462
var html = '<select';
463463
forEach(attrs, function(value, key) {
464464
if (isBoolean(value)) {
@@ -470,6 +470,7 @@ describe('select', function() {
470470
html += '>' +
471471
(blank ? (isString(blank) ? blank : '<option value="">blank</option>') : '') +
472472
(unknown ? (isString(unknown) ? unknown : '<option value="?">unknown</option>') : '') +
473+
(disabled ? (isString(disabled) ? blank : '<option value="" disabled="true">blank</option>') : '') +
473474
'</select>';
474475

475476
compile(html);
@@ -744,14 +745,48 @@ describe('select', function() {
744745

745746
var options = element.find('option');
746747
var optionToSelect = options.eq(1);
747-
748748
expect(optionToSelect.text()).toBe('B');
749749

750750
optionToSelect.prop('selected', true);
751751
scope.$digest();
752+
expect(optionToSelect.prop('selected')).not.toBe(true);
753+
expect(scope.selected).toBe(scope.values[0]);
754+
});
752755

756+
it('should allow the initial selection of a disabled element',
757+
function() {
758+
//Create a select element with a disabled option
759+
createSelect({
760+
'ng-model':'selected',
761+
'ng-options':'value.name for value in values',
762+
'ng-required': 'required'
763+
}, false, false, true);
764+
765+
//Add options 'A' and 'B'
766+
//Set the select to be HTML5 'required'
767+
scope.$apply(function() {
768+
scope.values = [{name: 'A'}, {name: 'B'}];
769+
scope.required = true;
770+
});
771+
772+
//Find the disabled option and select it
773+
var options = element.find('option');
774+
var optionToSelect = options.eq(0);
775+
optionToSelect.prop('selected', true);
776+
777+
//Expect that the disabled item can be selected
753778
expect(optionToSelect.prop('selected')).toBe(true);
754-
expect(scope.selected).toBe(scope.values[0]);
779+
//Expect that the browser won't automatically pick a non-disabled item
780+
expect(options.eq(1).prop('selected')).not.toBe(true);
781+
//Expect that with the disabled option selected, validity will be false
782+
expect(element[0].validity.valid).toBe(false);
783+
784+
//Select a non-disabled option
785+
options.eq(1).prop('selected', true);
786+
//Expect it to be selected
787+
expect(options.eq(1).prop('selected')).toBe(true);
788+
//Expect the validity to be true
789+
expect(element[0].validity.valid).toBe(true);
755790
});
756791

757792
describe('binding', function() {

0 commit comments

Comments
 (0)