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

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 46 additions & 14 deletions src/ng/directive/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,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
Expand All @@ -454,7 +454,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 {
Expand All @@ -468,7 +468,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) {
Expand All @@ -491,7 +491,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 {
Expand All @@ -508,7 +508,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 {
Expand All @@ -526,7 +526,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));
Expand All @@ -537,13 +537,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 {
Expand All @@ -559,7 +559,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 {
Expand All @@ -574,7 +574,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 {
Expand All @@ -588,7 +588,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 {
Expand All @@ -605,7 +605,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 {
Expand Down Expand Up @@ -657,6 +657,12 @@ 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.
var _$isEmpty = ctrl.$isEmpty;
ctrl.$isEmpty = function(value) {
return _$isEmpty(value) || value === false;
};

ctrl.$formatters.push(function(value) {
return value === trueValue;
});
Expand Down Expand Up @@ -992,6 +998,23 @@ 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 = isEmpty;

var parentForm = $element.inheritedData('$formController') || nullFormCtrl,
invalidCount = 0, // used to easily determine if we are valid
$error = this.$error = {}; // keep invalid keys here
Expand Down Expand Up @@ -1266,7 +1289,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 {
Expand Down Expand Up @@ -1326,7 +1349,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');
});
Expand All @@ -1341,6 +1364,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) {
Expand All @@ -1360,6 +1386,12 @@ var ngListDirective = function() {

return undefined;
});

// Override the standard $isEmpty because an empty array means the input is empty
var _$isEmpty = ctrl.$isEmpty;
ctrl.$isEmpty = function(value) {
return _$isEmpty(value) || value.length === 0;
};
}
};
};
Expand Down
33 changes: 31 additions & 2 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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=":" />');
Expand Down Expand Up @@ -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();
});
});


Expand Down
25 changes: 25 additions & 0 deletions test/ng/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});

Expand Down