From 904b69c745ea4afc1d6ecd2a5f3138c6f947b157 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 18 Apr 2012 00:48:25 -0700 Subject: [PATCH] fix(select): properly handle empty & unknown options without ngOptions Previously only when ngOptions was used, we correctly handled situations when model was set to an unknown value. With this change, we'll add/remove extra unknown option or reuse an existing empty option (option with value set to "") when model is undefined. --- src/ng/directive/select.js | 204 ++++++++++++++++------ test/ng/directive/selectSpec.js | 295 +++++++++++++++++++++++++++++--- 2 files changed, 425 insertions(+), 74 deletions(-) diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 49137b33c3ce..aa540828030d 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -22,16 +22,10 @@ * be nested into the `
Color (null allowed): -
+ -

+
Color grouped by shade: and IE barfs otherwise. + optionTemplate = jqLite(document.createElement('option')), + optGroupTemplate =jqLite(document.createElement('optgroup')), + unknownOption = optionTemplate.clone(); + + // find "null" option + for(var i = 0, children = element.children(), ii = children.length; i < ii; i++) { + if (children[i].value == '') { + emptyOption = nullOption = children.eq(i); + break; + } + } + + selectCtrl.init(ngModelCtrl, nullOption, unknownOption); // required validator if (multiple && (attr.required || attr.ngRequired)) { var requiredValidator = function(value) { - ctrl.$setValidity('required', !attr.required || (value && value.length)); + ngModelCtrl.$setValidity('required', !attr.required || (value && value.length)); return value; }; - ctrl.$parsers.push(requiredValidator); - ctrl.$formatters.unshift(requiredValidator); + ngModelCtrl.$parsers.push(requiredValidator); + ngModelCtrl.$formatters.unshift(requiredValidator); attr.$observe('required', function() { - requiredValidator(ctrl.$viewValue); + requiredValidator(ngModelCtrl.$viewValue); }); } - if (optionsExp) Options(scope, element, ctrl); - else if (multiple) Multiple(scope, element, ctrl); - else Single(scope, element, ctrl); + if (optionsExp) Options(scope, element, ngModelCtrl); + else if (multiple) Multiple(scope, element, ngModelCtrl); + else Single(scope, element, ngModelCtrl, selectCtrl); //////////////////////////// - function Single(scope, selectElement, ctrl) { - ctrl.$render = function() { - selectElement.val(ctrl.$viewValue); + function Single(scope, selectElement, ngModelCtrl, selectCtrl) { + ngModelCtrl.$render = function() { + var viewValue = ngModelCtrl.$viewValue; + + if (selectCtrl.hasOption(viewValue)) { + if (unknownOption.parent()) unknownOption.remove(); + selectElement.val(viewValue); + if (viewValue === '') emptyOption.prop('selected', true); // to make IE9 happy + } else { + if (isUndefined(viewValue) && emptyOption) { + selectElement.val(''); + } else { + selectCtrl.renderUnknownOption(viewValue); + } + } }; selectElement.bind('change', function() { scope.$apply(function() { - ctrl.$setViewValue(selectElement.val()); + if (unknownOption.parent()) unknownOption.remove(); + ngModelCtrl.$setViewValue(selectElement.val()); }); }); } @@ -219,26 +300,26 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { groupByFn = $parse(match[3] || ''), valueFn = $parse(match[2] ? match[1] : valueName), valuesFn = $parse(match[7]), - // we can't just jqLite('' + + ''); + expect(element).toEqualSelect('c3p0', ['r2d2']); + + browserTrigger(element.find('option').eq(0)); + expect(element).toEqualSelect(['c3p0'], 'r2d2'); + expect(scope.robot).toBe('c3p0'); + + scope.$apply(function() { + scope.robots.unshift('wallee'); + }); + expect(element).toEqualSelect('wallee', ['c3p0'], 'r2d2'); + expect(scope.robot).toBe('c3p0'); + + scope.$apply(function() { + scope.robots = ['c3p0+', 'r2d2+']; + scope.robot = 'r2d2+'; + }); + expect(element).toEqualSelect('c3p0+', ['r2d2+']); + expect(scope.robot).toBe('r2d2+'); + }); + + + describe('empty option', function() { + + it('should select the empty option when model is undefined', function() { + compile(''); + + expect(element).toEqualSelect([''], 'x', 'y'); + }); + + + it('should support defining an empty option anywhere in the option list', function() { + compile(''); + + expect(element).toEqualSelect('x', [''], 'y'); + }); + + + it('should set the model to empty string when empty option is selected', function() { + scope.robot = 'x'; + compile(''); + expect(element).toEqualSelect('', ['x'], 'y'); + + browserTrigger(element.find('option').eq(0)); + expect(element).toEqualSelect([''], 'x', 'y'); + expect(scope.robot).toBe(''); + }); + + + describe('interactions with repeated options', function() { + + it('should select empty option when model is undefined', function() { + scope.robots = ['c3p0', 'r2d2']; + compile(''); + expect(element).toEqualSelect([''], 'c3p0', 'r2d2'); + }); + + + it('should set model to empty string when selected', function() { + scope.robots = ['c3p0', 'r2d2']; + compile(''); + + browserTrigger(element.find('option').eq(1)); + expect(element).toEqualSelect('', ['c3p0'], 'r2d2'); + expect(scope.robot).toBe('c3p0'); + + browserTrigger(element.find('option').eq(0)); + expect(element).toEqualSelect([''], 'c3p0', 'r2d2'); + expect(scope.robot).toBe(''); + }); + + + it('should not break if both the select and repeater models change at once', function() { + scope.robots = ['c3p0', 'r2d2']; + scope.robot = 'c3p0' + compile(''); + expect(element).toEqualSelect('', ['c3p0'], 'r2d2'); + + scope.$apply(function() { + scope.robots = ['wallee']; + scope.robot = ''; + }); + + expect(element).toEqualSelect([''], 'wallee'); + }); + }); + }); + + + describe('unknown option', function() { + + it("should insert&select temporary unknown option when no options-model match", function() { + compile(''); + + expect(element).toEqualSelect(['? undefined:undefined ?'], 'c3p0', 'r2d2'); + + scope.$apply(function() { + scope.robot = 'r2d2'; + }); + expect(element).toEqualSelect('c3p0', ['r2d2']); + + + scope.$apply(function() { + scope.robot = "wallee"; + }); + expect(element).toEqualSelect(['? string:wallee ?'], 'c3p0', 'r2d2'); + }); + + + it("should NOT insert temporary unknown option when model is undefined and empty options " + + "is present", function() { + compile(''); + + expect(element).toEqualSelect([''], 'c3p0', 'r2d2'); + expect(scope.robot).toBeUndefined(); + + scope.$apply(function() { + scope.robot = null; + }); + expect(element).toEqualSelect(['? object:null ?'], '', 'c3p0', 'r2d2'); + + scope.$apply(function() { + scope.robot = 'r2d2'; + }); + expect(element).toEqualSelect('', 'c3p0', ['r2d2']); + + scope.$apply(function() { + delete scope.robot; + }); + expect(element).toEqualSelect([''], 'c3p0', 'r2d2'); + }); + + + it("should insert&select temporary unknown option when no options-model match, empty " + + "option is present and model is defined", function() { + scope.robot = 'wallee'; + compile(''); + + expect(element).toEqualSelect(['? string:wallee ?'], '', 'c3p0', 'r2d2'); + + scope.$apply(function() { + scope.robot = 'r2d2'; + }); + expect(element).toEqualSelect('', 'c3p0', ['r2d2']); + }); + + + describe('interactions with repeated options', function() { + + it('should work with repeated options', function() { + compile(''); + expect(element).toEqualSelect(['? undefined:undefined ?']); + expect(scope.robot).toBeUndefined(); + + scope.$apply(function() { + scope.robot = 'r2d2'; + }); + expect(element).toEqualSelect(['? string:r2d2 ?']); + expect(scope.robot).toBe('r2d2'); + + scope.$apply(function() { + scope.robots = ['c3p0', 'r2d2']; + }); + expect(element).toEqualSelect('c3p0', ['r2d2']); + expect(scope.robot).toBe('r2d2'); + }); + + + it('should work with empty option and repeated options', function() { + compile(''); + expect(element).toEqualSelect(['']); + expect(scope.robot).toBeUndefined(); + + scope.$apply(function() { + scope.robot = 'r2d2'; + }); + expect(element).toEqualSelect(['? string:r2d2 ?'], ''); + expect(scope.robot).toBe('r2d2'); + + scope.$apply(function() { + scope.robots = ['c3p0', 'r2d2']; + }); + expect(element).toEqualSelect('', 'c3p0', ['r2d2']); + expect(scope.robot).toBe('r2d2'); + }); + + + it('should insert unknown element when repeater shrinks and selected option is unavailable', + function() { + scope.robots = ['c3p0', 'r2d2']; + scope.robot = 'r2d2'; + compile(''); + expect(element).toEqualSelect('c3p0', ['r2d2']); + expect(scope.robot).toBe('r2d2'); + + scope.$apply(function() { + scope.robots.pop(); + }); + expect(element).toEqualSelect(['? string:r2d2 ?'], 'c3p0'); + expect(scope.robot).toBe('r2d2'); + + scope.$apply(function() { + scope.robots.unshift('r2d2'); + }); + expect(element).toEqualSelect(['r2d2'], 'c3p0'); + expect(scope.robot).toBe('r2d2'); + + scope.$apply(function() { + delete scope.robots; + }); + expect(element).toEqualSelect(['? string:r2d2 ?']); + expect(scope.robot).toBe('r2d2'); + }); + }); + }); }); @@ -840,24 +1101,18 @@ describe('select', function() { it('should populate value attribute on OPTION', function() { compile(''); - expect(element).toEqualSelect('abc'); + expect(element).toEqualSelect(['? undefined:undefined ?'], 'abc'); }); it('should ignore value if already exists', function() { compile(''); - expect(element).toEqualSelect('abc'); - }); - - it('should set value even if newlines present', function() { - compile(''); - expect(element).toEqualSelect('\nabc\n'); + expect(element).toEqualSelect(['? undefined:undefined ?'], 'abc'); }); it('should set value even if self closing HTML', function() { - // IE removes the \n from option, which makes this test pointless - if (msie) return; - compile(''); - expect(element).toEqualSelect('\n'); + scope.x = 'hello' + compile(''); + expect(element).toEqualSelect(['hello']); }); }); });