fix(input): false is no longer an empty value by default

petebacondarwin · vojtajina · commit b56b21a898b3 · 2013-10-07T16:47:51.000-07:00
`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 angular#3490, angular#3658, angular#2594
diff --git 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
@@ -998,6 +998,16 @@ describe('input', function() {
       expect(scope.list).toEqual([]);
     });
 
+    it('should be invalid if required and empty', function() {
+      compileInput('<input type="text" ng-list ng-model="list" required>');
+      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('<input type="text" ng-model="list" ng-list=":" />');
@@ -1090,10 +1100,29 @@ describe('input', function() {
 
 
     it('should set $invalid when model undefined', function() {
-      compileInput('<input type="text" ng-model="notDefiend" required />');
+      compileInput('<input type="text" ng-model="notDefined" required />');
       scope.$digest();
       expect(inputElm).toBeInvalid();
-    })
+    });
+
+
+    it('should allow `false` as a valid value when the input type is not "checkbox"', function() {
+      compileInput('<input type="radio" ng-value="true" ng-model="answer" required />' +
+        '<input type="radio" ng-value="false" ng-model="answer" required />');
+
+      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
@@ -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);
+      });
     });
   });
 
