From de9d272144d830637fe65b053dc1972f032a5d1c Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 25 Sep 2013 12:49:55 +0100 Subject: [PATCH] fix(input): `false` is no longer an empty value by default `checkboxInputType` and `ngList` directives need to have special logic for whether they are empty or not. Previously this had been hard coded into their own directives or the `ngRequired` directive. This made it difficult to handle these special cases. This change factors out the question of whether an input is empty into a method `$isEmpty` on the `ngModelController`. The `ngRequired` directive now uses this method when testing for validity and directives, such as `checkbox` or `ngList` can override it to apply logic specific to their needs. Closes #3490, #3658, #2594 --- src/ng/directive/input.js | 65 +++++++++++++++++++++++---------- test/ng/directive/inputSpec.js | 33 ++++++++++++++++- test/ng/directive/selectSpec.js | 25 +++++++++++++ 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 0accfb34ac14..8ea66d097468 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -384,11 +384,6 @@ var inputType = { }; -function isEmpty(value) { - return isUndefined(value) || value === '' || value === null || value !== value; -} - - function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { var listener = function() { @@ -445,7 +440,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { ctrl.$render = function() { - element.val(isEmpty(ctrl.$viewValue) ? '' : ctrl.$viewValue); + element.val(ctrl.$isEmpty(ctrl.$viewValue) ? '' : ctrl.$viewValue); }; // pattern validator @@ -454,7 +449,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { match; var validate = function(regexp, value) { - if (isEmpty(value) || regexp.test(value)) { + if (ctrl.$isEmpty(value) || regexp.test(value)) { ctrl.$setValidity('pattern', true); return value; } else { @@ -468,7 +463,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { if (match) { pattern = new RegExp(match[1], match[2]); patternValidator = function(value) { - return validate(pattern, value) + return validate(pattern, value); }; } else { patternValidator = function(value) { @@ -491,7 +486,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { if (attr.ngMinlength) { var minlength = int(attr.ngMinlength); var minLengthValidator = function(value) { - if (!isEmpty(value) && value.length < minlength) { + if (!ctrl.$isEmpty(value) && value.length < minlength) { ctrl.$setValidity('minlength', false); return undefined; } else { @@ -508,7 +503,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { if (attr.ngMaxlength) { var maxlength = int(attr.ngMaxlength); var maxLengthValidator = function(value) { - if (!isEmpty(value) && value.length > maxlength) { + if (!ctrl.$isEmpty(value) && value.length > maxlength) { ctrl.$setValidity('maxlength', false); return undefined; } else { @@ -526,7 +521,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { textInputType(scope, element, attr, ctrl, $sniffer, $browser); ctrl.$parsers.push(function(value) { - var empty = isEmpty(value); + var empty = ctrl.$isEmpty(value); if (empty || NUMBER_REGEXP.test(value)) { ctrl.$setValidity('number', true); return value === '' ? null : (empty ? value : parseFloat(value)); @@ -537,13 +532,13 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { }); ctrl.$formatters.push(function(value) { - return isEmpty(value) ? '' : '' + value; + return ctrl.$isEmpty(value) ? '' : '' + value; }); if (attr.min) { var min = parseFloat(attr.min); var minValidator = function(value) { - if (!isEmpty(value) && value < min) { + if (!ctrl.$isEmpty(value) && value < min) { ctrl.$setValidity('min', false); return undefined; } else { @@ -559,7 +554,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { if (attr.max) { var max = parseFloat(attr.max); var maxValidator = function(value) { - if (!isEmpty(value) && value > max) { + if (!ctrl.$isEmpty(value) && value > max) { ctrl.$setValidity('max', false); return undefined; } else { @@ -574,7 +569,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { ctrl.$formatters.push(function(value) { - if (isEmpty(value) || isNumber(value)) { + if (ctrl.$isEmpty(value) || isNumber(value)) { ctrl.$setValidity('number', true); return value; } else { @@ -588,7 +583,7 @@ function urlInputType(scope, element, attr, ctrl, $sniffer, $browser) { textInputType(scope, element, attr, ctrl, $sniffer, $browser); var urlValidator = function(value) { - if (isEmpty(value) || URL_REGEXP.test(value)) { + if (ctrl.$isEmpty(value) || URL_REGEXP.test(value)) { ctrl.$setValidity('url', true); return value; } else { @@ -605,7 +600,7 @@ function emailInputType(scope, element, attr, ctrl, $sniffer, $browser) { textInputType(scope, element, attr, ctrl, $sniffer, $browser); var emailValidator = function(value) { - if (isEmpty(value) || EMAIL_REGEXP.test(value)) { + if (ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value)) { ctrl.$setValidity('email', true); return value; } else { @@ -657,6 +652,11 @@ function checkboxInputType(scope, element, attr, ctrl) { element[0].checked = ctrl.$viewValue; }; + // Override the standard `$isEmpty` because a value of `false` means empty in a checkbox. + ctrl.$isEmpty = function(value) { + return value !== trueValue; + }; + ctrl.$formatters.push(function(value) { return value === trueValue; }); @@ -992,6 +992,25 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ */ this.$render = noop; + /** + * @ngdoc function + * @name { ng.directive:ngModel.NgModelController#$isEmpty + * @methodOf ng.directive:ngModel.NgModelController + * + * @description + * This is called when we need to determine if the value of the input is empty. + * + * For instance, the required directive does this to work out if the input has data or not. + * The default `$isEmpty` function checks whether the value is `undefined`, `''`, `null` or `NaN`. + * + * You can override this for input directives whose concept of being empty is different to the + * default. The `checkboxInputType` directive does this because in its case a value of `false` + * implies empty. + */ + this.$isEmpty = function(value) { + return isUndefined(value) || value === '' || value === null || value !== value; + }; + var parentForm = $element.inheritedData('$formController') || nullFormCtrl, invalidCount = 0, // used to easily determine if we are valid $error = this.$error = {}; // keep invalid keys here @@ -1266,7 +1285,7 @@ var requiredDirective = function() { attr.required = true; // force truthy in case we are on non input element var validator = function(value) { - if (attr.required && (isEmpty(value) || value === false)) { + if (attr.required && ctrl.$isEmpty(value)) { ctrl.$setValidity('required', false); return; } else { @@ -1327,7 +1346,7 @@ var requiredDirective = function() { it('should be invalid if empty', function() { input('names').enter(''); - expect(binding('names')).toEqual('[]'); + expect(binding('names')).toEqual(''); expect(binding('myForm.namesInput.$valid')).toEqual('false'); expect(element('span.error').css('display')).not().toBe('none'); }); @@ -1342,6 +1361,9 @@ var ngListDirective = function() { separator = match && new RegExp(match[1]) || attr.ngList || ','; var parse = function(viewValue) { + // If the viewValue is invalid (say required but empty) it will be `undefined` + if (isUndefined(viewValue)) return; + var list = []; if (viewValue) { @@ -1361,6 +1383,11 @@ var ngListDirective = function() { return undefined; }); + + // Override the standard $isEmpty because an empty array means the input is empty. + ctrl.$isEmpty = function(value) { + return !value || !value.length; + }; } }; }; diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index facc2b806bc9..c94eb9b898e1 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -998,6 +998,16 @@ describe('input', function() { expect(scope.list).toEqual([]); }); + it('should be invalid if required and empty', function() { + compileInput(''); + changeInputValueTo(''); + expect(scope.list).toBeUndefined(); + expect(inputElm).toBeInvalid(); + changeInputValueTo('a,b'); + expect(scope.list).toEqual(['a','b']); + expect(inputElm).toBeValid(); + }); + it('should allow custom separator', function() { compileInput(''); @@ -1090,10 +1100,29 @@ describe('input', function() { it('should set $invalid when model undefined', function() { - compileInput(''); + compileInput(''); scope.$digest(); expect(inputElm).toBeInvalid(); - }) + }); + + + it('should allow `false` as a valid value when the input type is not "checkbox"', function() { + compileInput('' + + ''); + + scope.$apply(); + expect(inputElm).toBeInvalid(); + + scope.$apply(function() { + scope.answer = true; + }); + expect(inputElm).toBeValid(); + + scope.$apply(function() { + scope.answer = false; + }); + expect(inputElm).toBeValid(); + }); }); diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 85acba1938ec..ac0cc70d758f 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -1213,6 +1213,31 @@ describe('select', function() { }); expect(element).toBeValid(); }); + + + it('should allow falsy values as values', function() { + createSelect({ + 'ng-model': 'value', + 'ng-options': 'item.value as item.name for item in values', + 'ng-required': 'required' + }, true); + + scope.$apply(function() { + scope.values = [{name: 'True', value: true}, {name: 'False', value: false}]; + scope.required = false; + }); + + element.val('1'); + browserTrigger(element, 'change'); + expect(element).toBeValid(); + expect(scope.value).toBe(false); + + scope.$apply(function() { + scope.required = true; + }); + expect(element).toBeValid(); + expect(scope.value).toBe(false); + }); }); });