Skip to content

Commit c83b4ba

Browse files
committed
fix(ngOptions): select disabled option when set from model
When a disabled option is set as the ngModel, ngOptions will now select the option in the select element, which will set select.val() to the option hash value and visually show the option value / label as selected in the select box. Previously, disabled options forced the unknown value. The previous behavior is inconsistent with both default HTML behavior and select with ngModel but without ngOptions. Both allow disabled values to be selected programmatically. Common use cases for this behavior include options that where previously valid, but have been disabled, and cannot be selected again. This commit removes a duplicate tests, all other tests that previously checked that disabled options are not set have been adjusted to the ensure the opposite. Fixes angular#12756
1 parent 7ecfa5d commit c83b4ba

File tree

2 files changed

+17
-46
lines changed

2 files changed

+17
-46
lines changed

src/ng/directive/ngOptions.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
467467
selectCtrl.writeValue = function writeNgOptionsValue(value) {
468468
var option = options.getOptionFromViewValue(value);
469469

470-
if (option && !option.disabled) {
470+
if (option) {
471471
// Don't update the option when it is already selected.
472472
// For example, the browser will select the first option by default. In that case,
473473
// most properties are set automatically - except the `selected` attribute, which we
@@ -529,7 +529,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
529529
if (value) {
530530
value.forEach(function(item) {
531531
var option = options.getOptionFromViewValue(item);
532-
if (option && !option.disabled) option.element.selected = true;
532+
if (option) option.element.selected = true;
533533
});
534534
}
535535
};

test/ng/directive/ngOptionsSpec.js

+15-44
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,7 @@ describe('ngOptions', function() {
751751
});
752752

753753

754-
it('should not select disabled options when model changes', function() {
754+
it('should select disabled options when model changes', function() {
755755
scope.options = [
756756
{ name: 'white', value: '#FFFFFF' },
757757
{ name: 'one', value: 1, unavailable: true },
@@ -772,11 +772,12 @@ describe('ngOptions', function() {
772772
scope.$apply('selected = 1');
773773
options = element.find('option');
774774

775-
expect(element.val()).toEqualUnknownValue('?');
776-
expect(options.length).toEqual(5);
777-
expect(options.eq(0).prop('selected')).toEqual(true);
775+
expect(element.val()).toBe('number:1');
776+
expect(options.length).toEqual(4);
777+
expect(options.eq(0).prop('selected')).toEqual(false);
778+
expect(options.eq(1).prop('selected')).toEqual(true);
778779
expect(options.eq(2).prop('selected')).toEqual(false);
779-
expect(options.eq(4).prop('selected')).toEqual(false);
780+
expect(options.eq(3).prop('selected')).toEqual(false);
780781
});
781782

782783

@@ -796,11 +797,12 @@ describe('ngOptions', function() {
796797
scope.$apply('selected = 1');
797798
var options = element.find('option');
798799

799-
expect(element.val()).toEqualUnknownValue('?');
800-
expect(options.length).toEqual(5);
801-
expect(options.eq(0).prop('selected')).toEqual(true);
800+
expect(element.val()).toBe('number:1');
801+
expect(options.length).toEqual(4);
802+
expect(options.eq(0).prop('selected')).toEqual(false);
803+
expect(options.eq(1).prop('selected')).toEqual(true);
802804
expect(options.eq(2).prop('selected')).toEqual(false);
803-
expect(options.eq(4).prop('selected')).toEqual(false);
805+
expect(options.eq(3).prop('selected')).toEqual(false);
804806

805807
// Now enable that option
806808
scope.$apply(function() {
@@ -841,7 +843,7 @@ describe('ngOptions', function() {
841843
});
842844

843845

844-
it('should not select disabled options when model changes', function() {
846+
it('should select disabled options when model changes', function() {
845847
scope.options = [
846848
{ name: 'a', value: 0 },
847849
{ name: 'b', value: 1, unavailable: true },
@@ -866,14 +868,14 @@ describe('ngOptions', function() {
866868
scope.$apply('selected = [1,3]');
867869
options = element.find('option');
868870
expect(options.eq(0).prop('selected')).toEqual(false);
869-
expect(options.eq(1).prop('selected')).toEqual(false);
871+
expect(options.eq(1).prop('selected')).toEqual(true);
870872
expect(options.eq(2).prop('selected')).toEqual(false);
871873
expect(options.eq(3).prop('selected')).toEqual(true);
872874

873875
// Now only select the disabled option
874876
scope.$apply('selected = [1]');
875877
expect(options.eq(0).prop('selected')).toEqual(false);
876-
expect(options.eq(1).prop('selected')).toEqual(false);
878+
expect(options.eq(1).prop('selected')).toEqual(true);
877879
expect(options.eq(2).prop('selected')).toEqual(false);
878880
expect(options.eq(3).prop('selected')).toEqual(false);
879881
});
@@ -897,7 +899,7 @@ describe('ngOptions', function() {
897899
var options = element.find('option');
898900

899901
expect(options.eq(0).prop('selected')).toEqual(false);
900-
expect(options.eq(1).prop('selected')).toEqual(false);
902+
expect(options.eq(1).prop('selected')).toEqual(true);
901903
expect(options.eq(2).prop('selected')).toEqual(false);
902904
expect(options.eq(3).prop('selected')).toEqual(false);
903905

@@ -2515,37 +2517,6 @@ describe('ngOptions', function() {
25152517
expect(element.find('option')[1].selected).toBeTruthy();
25162518
});
25172519

2518-
it('should not write disabled selections from model', function() {
2519-
scope.selected = [30];
2520-
scope.options = [
2521-
{ name: 'white', value: '#FFFFFF' },
2522-
{ name: 'one', value: 1, unavailable: true },
2523-
{ name: 'notTrue', value: false },
2524-
{ name: 'thirty', value: 30, unavailable: false }
2525-
];
2526-
createSelect({
2527-
'ng-options': 'o.value as o.name disable when o.unavailable for o in options',
2528-
'ng-model': 'selected',
2529-
'multiple': true
2530-
});
2531-
2532-
var options = element.find('option');
2533-
2534-
expect(options.eq(0).prop('selected')).toEqual(false);
2535-
expect(options.eq(1).prop('selected')).toEqual(false);
2536-
expect(options.eq(2).prop('selected')).toEqual(false);
2537-
expect(options.eq(3).prop('selected')).toEqual(true);
2538-
2539-
scope.$apply(function() {
2540-
scope.selected.push(1);
2541-
});
2542-
2543-
expect(options.eq(0).prop('selected')).toEqual(false);
2544-
expect(options.eq(1).prop('selected')).toEqual(false);
2545-
expect(options.eq(2).prop('selected')).toEqual(false);
2546-
expect(options.eq(3).prop('selected')).toEqual(true);
2547-
});
2548-
25492520

25502521
it('should update model on change', function() {
25512522
createMultiSelect();

0 commit comments

Comments
 (0)