From d9a3a89a62ac0a72953c6df0bab3322c286923fb Mon Sep 17 00:00:00 2001 From: Lucas Galfaso Date: Sat, 22 Nov 2014 01:33:26 +0100 Subject: [PATCH] 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 --- src/ng/directive/select.js | 21 ++-- test/ng/directive/selectSpec.js | 182 ++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 8 deletions(-) diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index df2359d12294..ed372c26e31d 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -679,18 +679,23 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { updateLabelMap(labelMap, option.label, false); option.element.remove(); } - forEach(labelMap, function(count, label) { - if (count > 0) { - selectCtrl.addOption(label); - } else if (count < 0) { - selectCtrl.removeOption(label); - } - }); } // remove any excessive OPTGROUPs from select while (optionGroupsCache.length > groupIndex) { - optionGroupsCache.pop()[0].element.remove(); + // remove all the labels in the option group + optionGroup = optionGroupsCache.pop(); + for (index = 1; index < optionGroup.length; ++index) { + updateLabelMap(labelMap, optionGroup[index].label, false); + } + optionGroup[0].element.remove(); } + forEach(labelMap, function(count, label) { + if (count > 0) { + selectCtrl.addOption(label); + } else if (count < 0) { + selectCtrl.removeOption(label); + } + }); } } } diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 4abde899325a..aae1a3aade01 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -40,6 +40,23 @@ describe('select', function() { return equals(expectedValues, actualValues); }, + toEqualSelectWithOptions: function(expected) { + var actualValues = {}; + var optionGroup; + + forEach(this.actual.find('option'), function(option) { + optionGroup = option.parentNode.label || ''; + actualValues[optionGroup] = actualValues[optionGroup] || []; + actualValues[optionGroup].push(option.label); + }); + + this.message = function() { + return 'Expected ' + toJson(actualValues) + ' to equal ' + toJson(expected) + '.'; + }; + + return equals(expected, actualValues); + }, + toEqualOption: function(value, text, label) { var errors = []; if (this.actual.attr('value') !== value) { @@ -447,6 +464,7 @@ describe('select', function() { expect(selectCtrl.hasOption('r2d2')).toBe(true); }); + it('should return false for options popped via ngOptions', function() { scope.robots = [ {value: 1, label: 'c3p0'}, @@ -467,6 +485,7 @@ describe('select', function() { expect(selectCtrl.hasOption('r2d2')).toBe(false); }); + it('should return true for options added via ngOptions', function() { scope.robots = [ {value: 2, label: 'r2d2'} @@ -485,6 +504,169 @@ describe('select', function() { expect(selectCtrl.hasOption('c3p0')).toBe(true); expect(selectCtrl.hasOption('r2d2')).toBe(true); }); + + + it('should keep all the options when changing the model', function() { + compile(''); + var selectCtrl = element.data().$selectController; + scope.$apply(function() { + scope.mySelect = 'C'; + }); + expect(selectCtrl.hasOption('A')).toBe(true); + expect(selectCtrl.hasOption('B')).toBe(true); + expect(selectCtrl.hasOption('C')).toBe(true); + expect(element).toEqualSelectWithOptions({'': ['A', 'B', 'C']}); + }); + + + it('should be able to detect when elements move from a previous group', function() { + scope.values = [ + {name: 'A'}, + {name: 'B', group: 'first'}, + {name: 'C', group: 'first'}, + {name: 'D', group: 'first'}, + {name: 'E', group: 'second'} + ]; + + compile(''); + var selectCtrl = element.data().$selectController; + + scope.$apply(function() { + scope.values[3] = {name: 'D', group: 'second'}; + scope.values.shift(); + }); + expect(selectCtrl.hasOption('A')).toBe(false); + expect(selectCtrl.hasOption('B')).toBe(true); + expect(selectCtrl.hasOption('C')).toBe(true); + expect(selectCtrl.hasOption('D')).toBe(true); + expect(selectCtrl.hasOption('E')).toBe(true); + expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C'], 'second': ['D', 'E']}); + }); + + + it('should be able to detect when elements move from a following group', function() { + scope.values = [ + {name: 'A'}, + {name: 'B', group: 'first'}, + {name: 'C', group: 'first'}, + {name: 'D', group: 'second'}, + {name: 'E', group: 'second'} + ]; + + compile(''); + var selectCtrl = element.data().$selectController; + + scope.$apply(function() { + scope.values[3].group = 'first'; + scope.values.shift(); + }); + expect(selectCtrl.hasOption('A')).toBe(false); + expect(selectCtrl.hasOption('B')).toBe(true); + expect(selectCtrl.hasOption('C')).toBe(true); + expect(selectCtrl.hasOption('D')).toBe(true); + expect(selectCtrl.hasOption('E')).toBe(true); + expect(element).toEqualSelectWithOptions({'': [''], 'first':['B', 'C', 'D'], 'second': ['E']}); + }); + + + it('should be able to detect when an element is replaced with an element from a previous group', function() { + scope.values = [ + {name: 'A'}, + {name: 'B', group: 'first'}, + {name: 'C', group: 'first'}, + {name: 'D', group: 'first'}, + {name: 'E', group: 'second'}, + {name: 'F', group: 'second'} + ]; + + compile(''); + var selectCtrl = element.data().$selectController; + + scope.$apply(function() { + scope.values[3].group = 'second'; + scope.values.pop(); + }); + expect(selectCtrl.hasOption('A')).toBe(true); + expect(selectCtrl.hasOption('B')).toBe(true); + expect(selectCtrl.hasOption('C')).toBe(true); + expect(selectCtrl.hasOption('D')).toBe(true); + expect(selectCtrl.hasOption('E')).toBe(true); + expect(selectCtrl.hasOption('F')).toBe(false); + expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['D', 'E']}); + }); + + + it('should be able to detect when element is replaced with an element from a following group', function() { + scope.values = [ + {name: 'A'}, + {name: 'B', group: 'first'}, + {name: 'C', group: 'first'}, + {name: 'D', group: 'second'}, + {name: 'E', group: 'second'} + ]; + + compile(''); + var selectCtrl = element.data().$selectController; + + scope.$apply(function() { + scope.values[3].group = 'first'; + scope.values.splice(2, 1); + }); + expect(selectCtrl.hasOption('A')).toBe(true); + expect(selectCtrl.hasOption('B')).toBe(true); + expect(selectCtrl.hasOption('C')).toBe(false); + expect(selectCtrl.hasOption('D')).toBe(true); + expect(selectCtrl.hasOption('E')).toBe(true); + expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'D'], 'second': ['E']}); + }); + + + it('should be able to detect when an element is removed', function() { + scope.values = [ + {name: 'A'}, + {name: 'B', group: 'first'}, + {name: 'C', group: 'first'}, + {name: 'D', group: 'second'}, + {name: 'E', group: 'second'} + ]; + + compile(''); + var selectCtrl = element.data().$selectController; + + scope.$apply(function() { + scope.values.splice(3, 1); + }); + expect(selectCtrl.hasOption('A')).toBe(true); + expect(selectCtrl.hasOption('B')).toBe(true); + expect(selectCtrl.hasOption('C')).toBe(true); + expect(selectCtrl.hasOption('D')).toBe(false); + expect(selectCtrl.hasOption('E')).toBe(true); + expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C'], 'second': ['E']}); + }); + + + it('should be able to detect when a group is removed', function() { + scope.values = [ + {name: 'A'}, + {name: 'B', group: 'first'}, + {name: 'C', group: 'first'}, + {name: 'D', group: 'second'}, + {name: 'E', group: 'second'} + ]; + + compile(''); + var selectCtrl = element.data().$selectController; + + scope.$apply(function() { + scope.values.splice(3, 2); + }); + expect(selectCtrl.hasOption('A')).toBe(true); + expect(selectCtrl.hasOption('B')).toBe(true); + expect(selectCtrl.hasOption('C')).toBe(true); + expect(selectCtrl.hasOption('D')).toBe(false); + expect(selectCtrl.hasOption('E')).toBe(false); + expect(element).toEqualSelectWithOptions({'': ['', 'A'], 'first':['B', 'C']}); + }); }); }); });