Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit b56b21a

Browse files
petebacondarwinvojtajina
petebacondarwin
authored and
vojtajina
committed
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
1 parent 2b5ce84 commit b56b21a

File tree

3 files changed

+102
-21
lines changed

3 files changed

+102
-21
lines changed

‎src/ng/directive/input.js

+46-19
Original file line numberDiff line numberDiff line change
@@ -384,11 +384,6 @@ var inputType = {
384384
};
385385

386386

387-
function isEmpty(value) {
388-
return isUndefined(value) || value === '' || value === null || value !== value;
389-
}
390-
391-
392387
function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
393388

394389
var listener = function() {
@@ -445,7 +440,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
445440

446441

447442
ctrl.$render = function() {
448-
element.val(isEmpty(ctrl.$viewValue) ? '' : ctrl.$viewValue);
443+
element.val(ctrl.$isEmpty(ctrl.$viewValue) ? '' : ctrl.$viewValue);
449444
};
450445

451446
// pattern validator
@@ -454,7 +449,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
454449
match;
455450

456451
var validate = function(regexp, value) {
457-
if (isEmpty(value) || regexp.test(value)) {
452+
if (ctrl.$isEmpty(value) || regexp.test(value)) {
458453
ctrl.$setValidity('pattern', true);
459454
return value;
460455
} else {
@@ -468,7 +463,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
468463
if (match) {
469464
pattern = new RegExp(match[1], match[2]);
470465
patternValidator = function(value) {
471-
return validate(pattern, value)
466+
return validate(pattern, value);
472467
};
473468
} else {
474469
patternValidator = function(value) {
@@ -491,7 +486,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
491486
if (attr.ngMinlength) {
492487
var minlength = int(attr.ngMinlength);
493488
var minLengthValidator = function(value) {
494-
if (!isEmpty(value) && value.length < minlength) {
489+
if (!ctrl.$isEmpty(value) && value.length < minlength) {
495490
ctrl.$setValidity('minlength', false);
496491
return undefined;
497492
} else {
@@ -508,7 +503,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
508503
if (attr.ngMaxlength) {
509504
var maxlength = int(attr.ngMaxlength);
510505
var maxLengthValidator = function(value) {
511-
if (!isEmpty(value) && value.length > maxlength) {
506+
if (!ctrl.$isEmpty(value) && value.length > maxlength) {
512507
ctrl.$setValidity('maxlength', false);
513508
return undefined;
514509
} else {
@@ -526,7 +521,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
526521
textInputType(scope, element, attr, ctrl, $sniffer, $browser);
527522

528523
ctrl.$parsers.push(function(value) {
529-
var empty = isEmpty(value);
524+
var empty = ctrl.$isEmpty(value);
530525
if (empty || NUMBER_REGEXP.test(value)) {
531526
ctrl.$setValidity('number', true);
532527
return value === '' ? null : (empty ? value : parseFloat(value));
@@ -537,13 +532,13 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
537532
});
538533

539534
ctrl.$formatters.push(function(value) {
540-
return isEmpty(value) ? '' : '' + value;
535+
return ctrl.$isEmpty(value) ? '' : '' + value;
541536
});
542537

543538
if (attr.min) {
544539
var min = parseFloat(attr.min);
545540
var minValidator = function(value) {
546-
if (!isEmpty(value) && value < min) {
541+
if (!ctrl.$isEmpty(value) && value < min) {
547542
ctrl.$setValidity('min', false);
548543
return undefined;
549544
} else {
@@ -559,7 +554,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
559554
if (attr.max) {
560555
var max = parseFloat(attr.max);
561556
var maxValidator = function(value) {
562-
if (!isEmpty(value) && value > max) {
557+
if (!ctrl.$isEmpty(value) && value > max) {
563558
ctrl.$setValidity('max', false);
564559
return undefined;
565560
} else {
@@ -574,7 +569,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
574569

575570
ctrl.$formatters.push(function(value) {
576571

577-
if (isEmpty(value) || isNumber(value)) {
572+
if (ctrl.$isEmpty(value) || isNumber(value)) {
578573
ctrl.$setValidity('number', true);
579574
return value;
580575
} else {
@@ -588,7 +583,7 @@ function urlInputType(scope, element, attr, ctrl, $sniffer, $browser) {
588583
textInputType(scope, element, attr, ctrl, $sniffer, $browser);
589584

590585
var urlValidator = function(value) {
591-
if (isEmpty(value) || URL_REGEXP.test(value)) {
586+
if (ctrl.$isEmpty(value) || URL_REGEXP.test(value)) {
592587
ctrl.$setValidity('url', true);
593588
return value;
594589
} else {
@@ -605,7 +600,7 @@ function emailInputType(scope, element, attr, ctrl, $sniffer, $browser) {
605600
textInputType(scope, element, attr, ctrl, $sniffer, $browser);
606601

607602
var emailValidator = function(value) {
608-
if (isEmpty(value) || EMAIL_REGEXP.test(value)) {
603+
if (ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value)) {
609604
ctrl.$setValidity('email', true);
610605
return value;
611606
} else {
@@ -657,6 +652,11 @@ function checkboxInputType(scope, element, attr, ctrl) {
657652
element[0].checked = ctrl.$viewValue;
658653
};
659654

655+
// Override the standard `$isEmpty` because a value of `false` means empty in a checkbox.
656+
ctrl.$isEmpty = function(value) {
657+
return value !== trueValue;
658+
};
659+
660660
ctrl.$formatters.push(function(value) {
661661
return value === trueValue;
662662
});
@@ -992,6 +992,25 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
992992
*/
993993
this.$render = noop;
994994

995+
/**
996+
* @ngdoc function
997+
* @name { ng.directive:ngModel.NgModelController#$isEmpty
998+
* @methodOf ng.directive:ngModel.NgModelController
999+
*
1000+
* @description
1001+
* This is called when we need to determine if the value of the input is empty.
1002+
*
1003+
* For instance, the required directive does this to work out if the input has data or not.
1004+
* The default `$isEmpty` function checks whether the value is `undefined`, `''`, `null` or `NaN`.
1005+
*
1006+
* You can override this for input directives whose concept of being empty is different to the
1007+
* default. The `checkboxInputType` directive does this because in its case a value of `false`
1008+
* implies empty.
1009+
*/
1010+
this.$isEmpty = function(value) {
1011+
return isUndefined(value) || value === '' || value === null || value !== value;
1012+
};
1013+
9951014
var parentForm = $element.inheritedData('$formController') || nullFormCtrl,
9961015
invalidCount = 0, // used to easily determine if we are valid
9971016
$error = this.$error = {}; // keep invalid keys here
@@ -1266,7 +1285,7 @@ var requiredDirective = function() {
12661285
attr.required = true; // force truthy in case we are on non input element
12671286

12681287
var validator = function(value) {
1269-
if (attr.required && (isEmpty(value) || value === false)) {
1288+
if (attr.required && ctrl.$isEmpty(value)) {
12701289
ctrl.$setValidity('required', false);
12711290
return;
12721291
} else {
@@ -1327,7 +1346,7 @@ var requiredDirective = function() {
13271346
13281347
it('should be invalid if empty', function() {
13291348
input('names').enter('');
1330-
expect(binding('names')).toEqual('[]');
1349+
expect(binding('names')).toEqual('');
13311350
expect(binding('myForm.namesInput.$valid')).toEqual('false');
13321351
expect(element('span.error').css('display')).not().toBe('none');
13331352
});
@@ -1342,6 +1361,9 @@ var ngListDirective = function() {
13421361
separator = match && new RegExp(match[1]) || attr.ngList || ',';
13431362

13441363
var parse = function(viewValue) {
1364+
// If the viewValue is invalid (say required but empty) it will be `undefined`
1365+
if (isUndefined(viewValue)) return;
1366+
13451367
var list = [];
13461368

13471369
if (viewValue) {
@@ -1361,6 +1383,11 @@ var ngListDirective = function() {
13611383

13621384
return undefined;
13631385
});
1386+
1387+
// Override the standard $isEmpty because an empty array means the input is empty.
1388+
ctrl.$isEmpty = function(value) {
1389+
return !value || !value.length;
1390+
};
13641391
}
13651392
};
13661393
};

‎test/ng/directive/inputSpec.js

+31-2
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,16 @@ describe('input', function() {
998998
expect(scope.list).toEqual([]);
999999
});
10001000

1001+
it('should be invalid if required and empty', function() {
1002+
compileInput('<input type="text" ng-list ng-model="list" required>');
1003+
changeInputValueTo('');
1004+
expect(scope.list).toBeUndefined();
1005+
expect(inputElm).toBeInvalid();
1006+
changeInputValueTo('a,b');
1007+
expect(scope.list).toEqual(['a','b']);
1008+
expect(inputElm).toBeValid();
1009+
});
1010+
10011011

10021012
it('should allow custom separator', function() {
10031013
compileInput('<input type="text" ng-model="list" ng-list=":" />');
@@ -1090,10 +1100,29 @@ describe('input', function() {
10901100

10911101

10921102
it('should set $invalid when model undefined', function() {
1093-
compileInput('<input type="text" ng-model="notDefiend" required />');
1103+
compileInput('<input type="text" ng-model="notDefined" required />');
10941104
scope.$digest();
10951105
expect(inputElm).toBeInvalid();
1096-
})
1106+
});
1107+
1108+
1109+
it('should allow `false` as a valid value when the input type is not "checkbox"', function() {
1110+
compileInput('<input type="radio" ng-value="true" ng-model="answer" required />' +
1111+
'<input type="radio" ng-value="false" ng-model="answer" required />');
1112+
1113+
scope.$apply();
1114+
expect(inputElm).toBeInvalid();
1115+
1116+
scope.$apply(function() {
1117+
scope.answer = true;
1118+
});
1119+
expect(inputElm).toBeValid();
1120+
1121+
scope.$apply(function() {
1122+
scope.answer = false;
1123+
});
1124+
expect(inputElm).toBeValid();
1125+
});
10971126
});
10981127

10991128

‎test/ng/directive/selectSpec.js

+25
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,31 @@ describe('select', function() {
12131213
});
12141214
expect(element).toBeValid();
12151215
});
1216+
1217+
1218+
it('should allow falsy values as values', function() {
1219+
createSelect({
1220+
'ng-model': 'value',
1221+
'ng-options': 'item.value as item.name for item in values',
1222+
'ng-required': 'required'
1223+
}, true);
1224+
1225+
scope.$apply(function() {
1226+
scope.values = [{name: 'True', value: true}, {name: 'False', value: false}];
1227+
scope.required = false;
1228+
});
1229+
1230+
element.val('1');
1231+
browserTrigger(element, 'change');
1232+
expect(element).toBeValid();
1233+
expect(scope.value).toBe(false);
1234+
1235+
scope.$apply(function() {
1236+
scope.required = true;
1237+
});
1238+
expect(element).toBeValid();
1239+
expect(scope.value).toBe(false);
1240+
});
12161241
});
12171242
});
12181243

0 commit comments

Comments
 (0)