From 11d2242df65b2ade0dabe366a0c42963b6d37df5 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Wed, 24 Sep 2014 15:33:35 -0700 Subject: [PATCH] fix(select): make ctrl.hasOption method consistent Prior to this fix, options added to a select by ngOptions would not cause `selectCtrl.hasOption` to return `true` Closes #8761 --- src/ng/directive/select.js | 5 ++- test/ng/directive/selectSpec.js | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 3d2fe7d8..2c4754fc 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -589,6 +589,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { id: option.id, selected: option.selected }); + selectCtrl.addOption(option.label, element); if (lastElement) { lastElement.after(element); } else { @@ -600,7 +601,9 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { // remove any excessive OPTIONs in a group index++; // increment since the existingOptions[0] is parent element not OPTION while(existingOptions.length > index) { - existingOptions.pop().element.remove(); + option = existingOptions.pop(); + selectCtrl.removeOption(option.label); + option.element.remove(); } } // remove any excessive OPTGROUPs from select diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 5bda1248..bf16744c 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -378,6 +378,39 @@ describe('select', function() { expect(element).toEqualSelect(['? string:r2d2 ?']); expect(scope.robot).toBe('r2d2'); }); + + describe('selectController.hasOption', function() { + it('should return true for options added via ngOptions', function() { + scope.robots = [ + {key: 1, value: 'c3p0'}, + {key: 2, value: 'r2d2'} + ]; + scope.robot = 'r2d2'; + + compile(''); + + var selectCtrl = element.data().$selectController; + + expect(selectCtrl.hasOption('c3p0')).toBe(true); + expect(selectCtrl.hasOption('r2d2')).toBe(true); + + scope.$apply(function() { + scope.robots.pop(); + }); + + expect(selectCtrl.hasOption('c3p0')).toBe(true); + expect(selectCtrl.hasOption('r2d2')).toBe(false); + + scope.$apply(function() { + scope.robots.push({key: 2, value: 'r2d2'}); + }); + + expect(selectCtrl.hasOption('c3p0')).toBe(true); + expect(selectCtrl.hasOption('r2d2')).toBe(true); + }); + }); }); }); }); @@ -454,6 +487,39 @@ describe('select', function() { expect(element).toBeValid(); expect(element).toBeDirty(); }); + + describe('selectController.hasOption', function() { + it('should return true for options added via ngOptions', function() { + scope.robots = [ + {key: 1, value: 'c3p0'}, + {key: 2, value: 'r2d2'} + ]; + scope.robot = 'r2d2'; + + compile(''); + + var selectCtrl = element.data().$selectController; + + expect(selectCtrl.hasOption('c3p0')).toBe(true); + expect(selectCtrl.hasOption('r2d2')).toBe(true); + + scope.$apply(function() { + scope.robots.pop(); + }); + + expect(selectCtrl.hasOption('c3p0')).toBe(true); + expect(selectCtrl.hasOption('r2d2')).toBe(false); + + scope.$apply(function() { + scope.robots.push({key: 2, value: 'r2d2'}); + }); + + expect(selectCtrl.hasOption('c3p0')).toBe(true); + expect(selectCtrl.hasOption('r2d2')).toBe(true); + }); + }); });