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

Commit 30694c8

Browse files
lgalfasopetebacondarwin
lgalfaso
authored and
petebacondarwin
committed
fix(select): fix several issues when moving options between groups
* When an option was moved to a previous group, the group that loose the option would remove the label from the controller * When an entire option group was removed, the options in the group were mot removed from the controller Closes #10166
1 parent 655ac64 commit 30694c8

File tree

2 files changed

+195
-8
lines changed

2 files changed

+195
-8
lines changed

‎src/ng/directive/select.js‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -679,18 +679,23 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
679679
updateLabelMap(labelMap, option.label, false);
680680
option.element.remove();
681681
}
682-
forEach(labelMap, function(count, label) {
683-
if (count > 0) {
684-
selectCtrl.addOption(label);
685-
} else if (count < 0) {
686-
selectCtrl.removeOption(label);
687-
}
688-
});
689682
}
690683
// remove any excessive OPTGROUPs from select
691684
while (optionGroupsCache.length > groupIndex) {
692-
optionGroupsCache.pop()[0].element.remove();
685+
// remove all the labels in the option group
686+
optionGroup = optionGroupsCache.pop();
687+
for (index = 1; index < optionGroup.length; ++index) {
688+
updateLabelMap(labelMap, optionGroup[index].label, false);
689+
}
690+
optionGroup[0].element.remove();
693691
}
692+
forEach(labelMap, function(count, label) {
693+
if (count > 0) {
694+
selectCtrl.addOption(label);
695+
} else if (count < 0) {
696+
selectCtrl.removeOption(label);
697+
}
698+
});
694699
}
695700
}
696701
}

‎test/ng/directive/selectSpec.js‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,23 @@ describe('select', function() {
4040
return equals(expectedValues, actualValues);
4141
},
4242

43+
toEqualSelectWithOptions: function(expected) {
44+
var actualValues = {};
45+
var optionGroup;
46+
47+
forEach(this.actual.find('option'), function(option) {
48+
optionGroup = option.parentNode.label || '';
49+
actualValues[optionGroup] = actualValues[optionGroup] || [];
50+
actualValues[optionGroup].push(option.label);
51+
});
52+
53+
this.message = function() {
54+
return 'Expected ' + toJson(actualValues) + ' to equal ' + toJson(expected) + '.';
55+
};
56+
57+
return equals(expected, actualValues);
58+
},
59+
4360
toEqualOption: function(value, text, label) {
4461
var errors = [];
4562
if (this.actual.attr('value') !== value) {
@@ -447,6 +464,7 @@ describe('select', function() {
447464
expect(selectCtrl.hasOption('r2d2')).toBe(true);
448465
});
449466

467+
450468
it('should return false for options popped via ngOptions', function() {
451469
scope.robots = [
452470
{value: 1, label: 'c3p0'},
@@ -467,6 +485,7 @@ describe('select', function() {
467485
expect(selectCtrl.hasOption('r2d2')).toBe(false);
468486
});
469487

488+
470489
it('should return true for options added via ngOptions', function() {
471490
scope.robots = [
472491
{value: 2, label: 'r2d2'}
@@ -485,6 +504,169 @@ describe('select', function() {
485504
expect(selectCtrl.hasOption('c3p0')).toBe(true);
486505
expect(selectCtrl.hasOption('r2d2')).toBe(true);
487506
});
507+
508+
509+
it('should keep all the options when changing the model', function() {
510+
compile('<select ng-model="mySelect" ng-options="o for o in [\'A\',\'B\',\'C\']"></select>');
511+
var selectCtrl = element.controller('select');
512+
scope.$apply(function() {
513+
scope.mySelect = 'C';
514+
});
515+
expect(selectCtrl.hasOption('A')).toBe(true);
516+
expect(selectCtrl.hasOption('B')).toBe(true);
517+
expect(selectCtrl.hasOption('C')).toBe(true);
518+
expect(element).toEqualSelectWithOptions({'': ['A', 'B', 'C']});
519+
});
520+
521+
522+
it('should be able to detect when elements move from a previous group', function() {
523+
scope.values = [
524+
{name: 'A'},
525+
{name: 'B', group: 'first'},
526+
{name: 'C', group: 'first'},
527+
{name: 'D', group: 'first'},
528+
{name: 'E', group: 'second'}
529+
];
530+
531+
compile('<select ng-model="mySelect" ng-options="item.name group by item.group for item in values"></select>');
532+
var selectCtrl = element.data().$selectController;
533+
534+
scope.$apply(function() {
535+
scope.values[3] = {name: 'D', group: 'second'};
536+
scope.values.shift();
537+
});
538+
expect(selectCtrl.hasOption('A')).toBe(false);
539+
expect(selectCtrl.hasOption('B')).toBe(true);
540+
expect(selectCtrl.hasOption('C')).toBe(true);
541+
expect(selectCtrl.hasOption('D')).toBe(true);
542+
expect(selectCtrl.hasOption('E')).toBe(true);
543+
expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C'], 'second': ['D', 'E']});
544+
});
545+
546+
547+
it('should be able to detect when elements move from a following group', function() {
548+
scope.values = [
549+
{name: 'A'},
550+
{name: 'B', group: 'first'},
551+
{name: 'C', group: 'first'},
552+
{name: 'D', group: 'second'},
553+
{name: 'E', group: 'second'}
554+
];
555+
556+
compile('<select ng-model="mySelect" ng-options="item.name group by item.group for item in values"></select>');
557+
var selectCtrl = element.data().$selectController;
558+
559+
scope.$apply(function() {
560+
scope.values[3].group = 'first';
561+
scope.values.shift();
562+
});
563+
expect(selectCtrl.hasOption('A')).toBe(false);
564+
expect(selectCtrl.hasOption('B')).toBe(true);
565+
expect(selectCtrl.hasOption('C')).toBe(true);
566+
expect(selectCtrl.hasOption('D')).toBe(true);
567+
expect(selectCtrl.hasOption('E')).toBe(true);
568+
expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C', 'D'], 'second': ['E']});
569+
});
570+
571+
572+
it('should be able to detect when an element is replaced with an element from a previous group', function() {
573+
scope.values = [
574+
{name: 'A'},
575+
{name: 'B', group: 'first'},
576+
{name: 'C', group: 'first'},
577+
{name: 'D', group: 'first'},
578+
{name: 'E', group: 'second'},
579+
{name: 'F', group: 'second'}
580+
];
581+
582+
compile('<select ng-model="mySelect" ng-options="item.name group by item.group for item in values"></select>');
583+
var selectCtrl = element.data().$selectController;
584+
585+
scope.$apply(function() {
586+
scope.values[3].group = 'second';
587+
scope.values.pop();
588+
});
589+
expect(selectCtrl.hasOption('A')).toBe(true);
590+
expect(selectCtrl.hasOption('B')).toBe(true);
591+
expect(selectCtrl.hasOption('C')).toBe(true);
592+
expect(selectCtrl.hasOption('D')).toBe(true);
593+
expect(selectCtrl.hasOption('E')).toBe(true);
594+
expect(selectCtrl.hasOption('F')).toBe(false);
595+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['D', 'E']});
596+
});
597+
598+
599+
it('should be able to detect when element is replaced with an element from a following group', function() {
600+
scope.values = [
601+
{name: 'A'},
602+
{name: 'B', group: 'first'},
603+
{name: 'C', group: 'first'},
604+
{name: 'D', group: 'second'},
605+
{name: 'E', group: 'second'}
606+
];
607+
608+
compile('<select ng-model="mySelect" ng-options="item.name group by item.group for item in values"></select>');
609+
var selectCtrl = element.data().$selectController;
610+
611+
scope.$apply(function() {
612+
scope.values[3].group = 'first';
613+
scope.values.splice(2, 1);
614+
});
615+
expect(selectCtrl.hasOption('A')).toBe(true);
616+
expect(selectCtrl.hasOption('B')).toBe(true);
617+
expect(selectCtrl.hasOption('C')).toBe(false);
618+
expect(selectCtrl.hasOption('D')).toBe(true);
619+
expect(selectCtrl.hasOption('E')).toBe(true);
620+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'D'], 'second': ['E']});
621+
});
622+
623+
624+
it('should be able to detect when an element is removed', function() {
625+
scope.values = [
626+
{name: 'A'},
627+
{name: 'B', group: 'first'},
628+
{name: 'C', group: 'first'},
629+
{name: 'D', group: 'second'},
630+
{name: 'E', group: 'second'}
631+
];
632+
633+
compile('<select ng-model="mySelect" ng-options="item.name group by item.group for item in values"></select>');
634+
var selectCtrl = element.data().$selectController;
635+
636+
scope.$apply(function() {
637+
scope.values.splice(3, 1);
638+
});
639+
expect(selectCtrl.hasOption('A')).toBe(true);
640+
expect(selectCtrl.hasOption('B')).toBe(true);
641+
expect(selectCtrl.hasOption('C')).toBe(true);
642+
expect(selectCtrl.hasOption('D')).toBe(false);
643+
expect(selectCtrl.hasOption('E')).toBe(true);
644+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['E']});
645+
});
646+
647+
648+
it('should be able to detect when a group is removed', function() {
649+
scope.values = [
650+
{name: 'A'},
651+
{name: 'B', group: 'first'},
652+
{name: 'C', group: 'first'},
653+
{name: 'D', group: 'second'},
654+
{name: 'E', group: 'second'}
655+
];
656+
657+
compile('<select ng-model="mySelect" ng-options="item.name group by item.group for item in values"></select>');
658+
var selectCtrl = element.data().$selectController;
659+
660+
scope.$apply(function() {
661+
scope.values.splice(3, 2);
662+
});
663+
expect(selectCtrl.hasOption('A')).toBe(true);
664+
expect(selectCtrl.hasOption('B')).toBe(true);
665+
expect(selectCtrl.hasOption('C')).toBe(true);
666+
expect(selectCtrl.hasOption('D')).toBe(false);
667+
expect(selectCtrl.hasOption('E')).toBe(false);
668+
expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C']});
669+
});
488670
});
489671
});
490672
});

0 commit comments

Comments
 (0)