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

Commit 3b05c48

Browse files
NarretzNarretz
committed
fix(ngOptions): set select value when model matches disabled option
When ngModel is set to a value that matches a disabled option, 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. A common use case for this behavior is an option that was previously valid, but has been disabled, and cannot be selected again. This commit removes a duplicate test, and all other tests that previously checked that disabled options are not set have been adjusted to the ensure the opposite. Fixes #12756
1 parent 71cf28c commit 3b05c48

File tree

2 files changed

+21
-46
lines changed

2 files changed

+21
-46
lines changed

‎src/ng/directive/ngOptions.js‎

Lines changed: 2 additions & 2 deletions
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‎

Lines changed: 19 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ describe('ngOptions', function() {
771771
});
772772

773773

774-
it('should not select disabled options when model changes', function() {
774+
it('should select disabled options when model changes', function() {
775775
scope.options = [
776776
{ name: 'white', value: '#FFFFFF' },
777777
{ name: 'one', value: 1, unavailable: true },
@@ -792,11 +792,14 @@ describe('ngOptions', function() {
792792
scope.$apply('selected = 1');
793793
options = element.find('option');
794794

795-
expect(element.val()).toEqualUnknownValue('?');
796-
expect(options.length).toEqual(5);
797-
expect(options.eq(0).prop('selected')).toEqual(true);
795+
// jQuery returns null for val() when the option is disabled, see
796+
// https://bugs.jquery.com/ticket/13097
797+
expect(element[0].value).toBe('number:1');
798+
expect(options.length).toEqual(4);
799+
expect(options.eq(0).prop('selected')).toEqual(false);
800+
expect(options.eq(1).prop('selected')).toEqual(true);
798801
expect(options.eq(2).prop('selected')).toEqual(false);
799-
expect(options.eq(4).prop('selected')).toEqual(false);
802+
expect(options.eq(3).prop('selected')).toEqual(false);
800803
});
801804

802805

@@ -816,11 +819,14 @@ describe('ngOptions', function() {
816819
scope.$apply('selected = 1');
817820
var options = element.find('option');
818821

819-
expect(element.val()).toEqualUnknownValue('?');
820-
expect(options.length).toEqual(5);
821-
expect(options.eq(0).prop('selected')).toEqual(true);
822+
// jQuery returns null for val() when the option is disabled, see
823+
// https://bugs.jquery.com/ticket/13097
824+
expect(element[0].value).toBe('number:1');
825+
expect(options.length).toEqual(4);
826+
expect(options.eq(0).prop('selected')).toEqual(false);
827+
expect(options.eq(1).prop('selected')).toEqual(true);
822828
expect(options.eq(2).prop('selected')).toEqual(false);
823-
expect(options.eq(4).prop('selected')).toEqual(false);
829+
expect(options.eq(3).prop('selected')).toEqual(false);
824830

825831
// Now enable that option
826832
scope.$apply(function() {
@@ -861,7 +867,7 @@ describe('ngOptions', function() {
861867
});
862868

863869

864-
it('should not select disabled options when model changes', function() {
870+
it('should select disabled options when model changes', function() {
865871
scope.options = [
866872
{ name: 'a', value: 0 },
867873
{ name: 'b', value: 1, unavailable: true },
@@ -886,14 +892,14 @@ describe('ngOptions', function() {
886892
scope.$apply('selected = [1,3]');
887893
options = element.find('option');
888894
expect(options.eq(0).prop('selected')).toEqual(false);
889-
expect(options.eq(1).prop('selected')).toEqual(false);
895+
expect(options.eq(1).prop('selected')).toEqual(true);
890896
expect(options.eq(2).prop('selected')).toEqual(false);
891897
expect(options.eq(3).prop('selected')).toEqual(true);
892898

893899
// Now only select the disabled option
894900
scope.$apply('selected = [1]');
895901
expect(options.eq(0).prop('selected')).toEqual(false);
896-
expect(options.eq(1).prop('selected')).toEqual(false);
902+
expect(options.eq(1).prop('selected')).toEqual(true);
897903
expect(options.eq(2).prop('selected')).toEqual(false);
898904
expect(options.eq(3).prop('selected')).toEqual(false);
899905
});
@@ -917,7 +923,7 @@ describe('ngOptions', function() {
917923
var options = element.find('option');
918924

919925
expect(options.eq(0).prop('selected')).toEqual(false);
920-
expect(options.eq(1).prop('selected')).toEqual(false);
926+
expect(options.eq(1).prop('selected')).toEqual(true);
921927
expect(options.eq(2).prop('selected')).toEqual(false);
922928
expect(options.eq(3).prop('selected')).toEqual(false);
923929

@@ -2535,37 +2541,6 @@ describe('ngOptions', function() {
25352541
expect(element.find('option')[1].selected).toBeTruthy();
25362542
});
25372543

2538-
it('should not write disabled selections from model', function() {
2539-
scope.selected = [30];
2540-
scope.options = [
2541-
{ name: 'white', value: '#FFFFFF' },
2542-
{ name: 'one', value: 1, unavailable: true },
2543-
{ name: 'notTrue', value: false },
2544-
{ name: 'thirty', value: 30, unavailable: false }
2545-
];
2546-
createSelect({
2547-
'ng-options': 'o.value as o.name disable when o.unavailable for o in options',
2548-
'ng-model': 'selected',
2549-
'multiple': true
2550-
});
2551-
2552-
var options = element.find('option');
2553-
2554-
expect(options.eq(0).prop('selected')).toEqual(false);
2555-
expect(options.eq(1).prop('selected')).toEqual(false);
2556-
expect(options.eq(2).prop('selected')).toEqual(false);
2557-
expect(options.eq(3).prop('selected')).toEqual(true);
2558-
2559-
scope.$apply(function() {
2560-
scope.selected.push(1);
2561-
});
2562-
2563-
expect(options.eq(0).prop('selected')).toEqual(false);
2564-
expect(options.eq(1).prop('selected')).toEqual(false);
2565-
expect(options.eq(2).prop('selected')).toEqual(false);
2566-
expect(options.eq(3).prop('selected')).toEqual(true);
2567-
});
2568-
25692544

25702545
it('should update model on change', function() {
25712546
createMultiSelect();

0 commit comments

Comments
 (0)