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

Commit f87382e

Browse files
petebacondarwinpetebacondarwin
committed
fix(select): ensure SelectController.hasOption copes with options moving between groups
1 parent 22dcb0e commit f87382e

File tree

2 files changed

+329
-4
lines changed

2 files changed

+329
-4
lines changed

‎src/ng/directive/select.js

+9-3
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,20 @@ var SelectController =
8484
// Tell the select control that an option, with the given value, has been added
8585
self.addOption = function(value) {
8686
assertNotHasOwnProperty(value, '"option value"');
87-
optionsMap.put(value, true);
87+
var count = optionsMap.get(value) || 0;
88+
optionsMap.put(value, count + 1);
8889
self.ngModelCtrl.$render();
8990
};
9091

9192
// Tell the select control that an option, with the given value, has been removed
9293
self.removeOption = function(value) {
93-
if (this.hasOption(value)) {
94-
optionsMap.remove(value);
94+
var count = optionsMap.get(value);
95+
if (count) {
96+
if (count == 1) {
97+
optionsMap.remove(value);
98+
} else {
99+
optionsMap.put(value, count - 1);
100+
}
95101
self.ngModelCtrl.$render();
96102
}
97103
};

‎test/ng/directive/selectSpec.js

+320-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ describe('select', function() {
5151
forEach(this.actual.find('option'), function(option) {
5252
optionGroup = option.parentNode.label || '';
5353
actualValues[optionGroup] = actualValues[optionGroup] || [];
54-
actualValues[optionGroup].push(option.label);
54+
// IE9 doesn't populate the label property from the text property like other browsers
55+
actualValues[optionGroup].push(option.label || option.text);
5556
});
5657

5758
this.message = function() {
@@ -433,6 +434,324 @@ describe('select', function() {
433434

434435
});
435436

437+
438+
describe('selectController.hasOption', function() {
439+
440+
function compileRepeatedOptions() {
441+
compile('<select ng-model="robot">' +
442+
'<option value="{{item.value}}" ng-repeat="item in robots">{{item.label}}</option>' +
443+
'</select>');
444+
}
445+
446+
function compileGroupedOptions() {
447+
compile(
448+
'<select ng-model="mySelect">' +
449+
'<option ng-repeat="item in values">{{item.name}}</option>' +
450+
'<optgroup ng-repeat="group in groups" label="{{group.name}}">' +
451+
'<option ng-repeat="item in group.values">{{item.name}}</option>' +
452+
'</optgroup>' +
453+
'</select>');
454+
}
455+
456+
describe('flat options', function() {
457+
it('should return false for options shifted via ngRepeat', function() {
458+
scope.robots = [
459+
{value: 1, label: 'c3p0'},
460+
{value: 2, label: 'r2d2'}
461+
];
462+
463+
compileRepeatedOptions();
464+
465+
var selectCtrl = element.controller('select');
466+
467+
scope.$apply(function() {
468+
scope.robots.shift();
469+
});
470+
471+
expect(selectCtrl.hasOption('1')).toBe(false);
472+
expect(selectCtrl.hasOption('2')).toBe(true);
473+
});
474+
475+
476+
it('should return false for options popped via ngRepeat', function() {
477+
scope.robots = [
478+
{value: 1, label: 'c3p0'},
479+
{value: 2, label: 'r2d2'}
480+
];
481+
482+
compileRepeatedOptions();
483+
484+
var selectCtrl = element.controller('select');
485+
486+
scope.$apply(function() {
487+
scope.robots.pop();
488+
});
489+
490+
expect(selectCtrl.hasOption('1')).toBe(true);
491+
expect(selectCtrl.hasOption('2')).toBe(false);
492+
});
493+
494+
495+
it('should return true for options added via ngRepeat', function() {
496+
scope.robots = [
497+
{value: 2, label: 'r2d2'}
498+
];
499+
500+
compileRepeatedOptions();
501+
502+
var selectCtrl = element.controller('select');
503+
504+
scope.$apply(function() {
505+
scope.robots.unshift({value: 1, label: 'c3p0'});
506+
});
507+
508+
expect(selectCtrl.hasOption('1')).toBe(true);
509+
expect(selectCtrl.hasOption('2')).toBe(true);
510+
});
511+
512+
513+
it('should keep all the options when changing the model', function() {
514+
515+
compile('<select ng-model="mySelect"><option ng-repeat="o in [\'A\',\'B\',\'C\']">{{o}}</option></select>');
516+
517+
var selectCtrl = element.controller('select');
518+
519+
scope.$apply(function() {
520+
scope.mySelect = 'C';
521+
});
522+
523+
expect(selectCtrl.hasOption('A')).toBe(true);
524+
expect(selectCtrl.hasOption('B')).toBe(true);
525+
expect(selectCtrl.hasOption('C')).toBe(true);
526+
expect(element).toEqualSelectWithOptions({'': ['A', 'B', 'C']});
527+
});
528+
});
529+
530+
531+
describe('grouped options', function() {
532+
533+
it('should be able to detect when elements move from a previous group', function() {
534+
scope.values = [{name: 'A'}];
535+
scope.groups = [
536+
{
537+
name: 'first',
538+
values: [
539+
{name: 'B'},
540+
{name: 'C'},
541+
{name: 'D'}
542+
]
543+
},
544+
{
545+
name: 'second',
546+
values: [
547+
{name: 'E'}
548+
]
549+
}
550+
];
551+
552+
compileGroupedOptions();
553+
554+
var selectCtrl = element.controller('select');
555+
556+
scope.$apply(function() {
557+
var itemD = scope.groups[0].values.pop();
558+
scope.groups[1].values.unshift(itemD);
559+
scope.values.shift();
560+
});
561+
562+
expect(selectCtrl.hasOption('A')).toBe(false);
563+
expect(selectCtrl.hasOption('B')).toBe(true);
564+
expect(selectCtrl.hasOption('C')).toBe(true);
565+
expect(selectCtrl.hasOption('D')).toBe(true);
566+
expect(selectCtrl.hasOption('E')).toBe(true);
567+
expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C'], 'second': ['D', 'E']});
568+
});
569+
570+
571+
it('should be able to detect when elements move from a following group', function() {
572+
scope.values = [{name: 'A'}];
573+
scope.groups = [
574+
{
575+
name: 'first',
576+
values: [
577+
{name: 'B'},
578+
{name: 'C'}
579+
]
580+
},
581+
{
582+
name: 'second',
583+
values: [
584+
{name: 'D'},
585+
{name: 'E'}
586+
]
587+
}
588+
];
589+
590+
compileGroupedOptions();
591+
592+
var selectCtrl = element.controller('select');
593+
594+
scope.$apply(function() {
595+
var itemD = scope.groups[1].values.shift();
596+
scope.groups[0].values.push(itemD);
597+
scope.values.shift();
598+
});
599+
expect(selectCtrl.hasOption('A')).toBe(false);
600+
expect(selectCtrl.hasOption('B')).toBe(true);
601+
expect(selectCtrl.hasOption('C')).toBe(true);
602+
expect(selectCtrl.hasOption('D')).toBe(true);
603+
expect(selectCtrl.hasOption('E')).toBe(true);
604+
expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C', 'D'], 'second': ['E']});
605+
});
606+
607+
608+
it('should be able to detect when an element is replaced with an element from a previous group', function() {
609+
scope.values = [{name: 'A'}];
610+
scope.groups = [
611+
{
612+
name: 'first',
613+
values: [
614+
{name: 'B'},
615+
{name: 'C'},
616+
{name: 'D'}
617+
]
618+
},
619+
{
620+
name: 'second',
621+
values: [
622+
{name: 'E'},
623+
{name: 'F'}
624+
]
625+
}
626+
];
627+
628+
compileGroupedOptions();
629+
630+
var selectCtrl = element.controller('select');
631+
632+
scope.$apply(function() {
633+
var itemD = scope.groups[0].values.pop();
634+
scope.groups[1].values.unshift(itemD);
635+
scope.groups[1].values.pop();
636+
});
637+
expect(selectCtrl.hasOption('A')).toBe(true);
638+
expect(selectCtrl.hasOption('B')).toBe(true);
639+
expect(selectCtrl.hasOption('C')).toBe(true);
640+
expect(selectCtrl.hasOption('D')).toBe(true);
641+
expect(selectCtrl.hasOption('E')).toBe(true);
642+
expect(selectCtrl.hasOption('F')).toBe(false);
643+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['D', 'E']});
644+
});
645+
646+
647+
it('should be able to detect when element is replaced with an element from a following group', function() {
648+
scope.values = [{name: 'A'}];
649+
scope.groups = [
650+
{
651+
name: 'first',
652+
values: [
653+
{name: 'B'},
654+
{name: 'C'}
655+
]
656+
},
657+
{
658+
name: 'second',
659+
values: [
660+
{name: 'D'},
661+
{name: 'E'}
662+
]
663+
}
664+
];
665+
666+
compileGroupedOptions();
667+
668+
var selectCtrl = element.controller('select');
669+
670+
scope.$apply(function() {
671+
scope.groups[0].values.pop();
672+
var itemD = scope.groups[1].values.shift();
673+
scope.groups[0].values.push(itemD);
674+
});
675+
expect(selectCtrl.hasOption('A')).toBe(true);
676+
expect(selectCtrl.hasOption('B')).toBe(true);
677+
expect(selectCtrl.hasOption('C')).toBe(false);
678+
expect(selectCtrl.hasOption('D')).toBe(true);
679+
expect(selectCtrl.hasOption('E')).toBe(true);
680+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'D'], 'second': ['E']});
681+
});
682+
683+
684+
it('should be able to detect when an element is removed', function() {
685+
scope.values = [{name: 'A'}];
686+
scope.groups = [
687+
{
688+
name: 'first',
689+
values: [
690+
{name: 'B'},
691+
{name: 'C'}
692+
]
693+
},
694+
{
695+
name: 'second',
696+
values: [
697+
{name: 'D'},
698+
{name: 'E'}
699+
]
700+
}
701+
];
702+
703+
compileGroupedOptions();
704+
705+
var selectCtrl = element.controller('select');
706+
707+
scope.$apply(function() {
708+
scope.groups[1].values.shift();
709+
});
710+
expect(selectCtrl.hasOption('A')).toBe(true);
711+
expect(selectCtrl.hasOption('B')).toBe(true);
712+
expect(selectCtrl.hasOption('C')).toBe(true);
713+
expect(selectCtrl.hasOption('D')).toBe(false);
714+
expect(selectCtrl.hasOption('E')).toBe(true);
715+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['E']});
716+
});
717+
718+
719+
it('should be able to detect when a group is removed', function() {
720+
scope.values = [{name: 'A'}];
721+
scope.groups = [
722+
{
723+
name: 'first',
724+
values: [
725+
{name: 'B'},
726+
{name: 'C'}
727+
]
728+
},
729+
{
730+
name: 'second',
731+
values: [
732+
{name: 'D'},
733+
{name: 'E'}
734+
]
735+
}
736+
];
737+
738+
compileGroupedOptions();
739+
740+
var selectCtrl = element.controller('select');
741+
742+
scope.$apply(function() {
743+
scope.groups.pop();
744+
});
745+
expect(selectCtrl.hasOption('A')).toBe(true);
746+
expect(selectCtrl.hasOption('B')).toBe(true);
747+
expect(selectCtrl.hasOption('C')).toBe(true);
748+
expect(selectCtrl.hasOption('D')).toBe(false);
749+
expect(selectCtrl.hasOption('E')).toBe(false);
750+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C']});
751+
});
752+
});
753+
});
754+
436755
describe('select-multiple', function() {
437756

438757
it('should support type="select-multiple"', function() {

0 commit comments

Comments
 (0)