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

Added support for numerical ngTrueValue and ngFalseValue attributes #3251

Closed
wants to merge 4 commits into from
+162 −24
Closed

Added support for numerical ngTrueValue and ngFalseValue attributes #3251

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a2df36
angular/angular.js#1199
simonsparks Jul 17, 2013
34b0195
angular/angular.js#1199
simonsparks Jul 17, 2013
89176a5
angular/angular.js#1199
simonsparks Jul 17, 2013
80a9633
Merge branch 'master' of git://github.com/angular/angular.js
simonsparks Jul 17, 2013
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
14 changes: 9 additions & 5 deletions src/ng/directive/input.js
View file
Original file line number Diff line number Diff line change
@@ -641,11 +641,8 @@ function radioInputType(scope, element, attr, ctrl) {
}

function checkboxInputType(scope, element, attr, ctrl) {
var trueValue = attr.ngTrueValue,
falseValue = attr.ngFalseValue;

if (!isString(trueValue)) trueValue = true;
if (!isString(falseValue)) falseValue = false;
var trueValue = typedValue(attr.ngTrueValue, true),
falseValue = typedValue(attr.ngFalseValue, false);

element.on('click', function() {
scope.$apply(function() {
@@ -666,6 +663,13 @@ function checkboxInputType(scope, element, attr, ctrl) {
});
}

function typedValue(val, defaultVal) {
return isString(val)
? !isNaN(parseFloat(val)) && isFinite(val)
? parseFloat(val)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be done more concisely with:

return isString(val) ?
           (isFinite(val) ? parseFloat(val) : val) :
           defaultVal;

If it is a string and finite number (isFinite parses strings and returns false for NaN) then return the parsed number. Also isFinite is more choosy - for instance parseFloat will return 9 for "9/3", whereas isFinite will return false

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach committed I've used in the past based on this SO question: http://stackoverflow.com/questions/18082/validate-numbers-in-javascript-isnumeric

But I don't personally know if ordering isFinite() first will be sufficient to catch all cases where parseFloat might fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are most, of the cases I could come up with:

isFinite(100) => true
isFinite(0) => true
isFinite(-100) => true
isFinite(1.3) => true
isFinite(1/0) => false
isFinite(NaN) => false
isFinite("100") => true
isFinite("-34.3") => true
isFinite(null) => true
isFinite(undefined) => false
isFinite("abc") => false
isFinite(1e3) => true
isFinite("1e3") => true

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking further, I noticed a suite of unit tests against various approaches: http://dl.dropboxusercontent.com/u/35146/js/tests/isNumber.html
and saw that isFinite(String(n)) passes empty strings and whitespace as valid numbers.
I'm guessing we could assume a use case where an empty string was desirable as a false value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. But if you use your original version we should cache the
parsed value rather than running parseFloat twice

On 17 July 2013 14:04, Simon Sparks notifications@github.com wrote:

In src/ng/directive/input.js:

@@ -666,6 +663,13 @@ function checkboxInputType(scope, element, attr, ctrl) {
});
}

+function typedValue(val, defaultVal) {

  • return isString(val)
  • ? !isNaN(parseFloat(val)) && isFinite(val)
  •  ? parseFloat(val)

Looking further, I noticed a suite of unit tests against various
approaches:
http://dl.dropboxusercontent.com/u/35146/js/tests/isNumber.html
and saw that isFinite(String(n)) passes empty strings and whitespace as
valid numbers.
I'm guessing we could assume a use case where an empty string was
desirable as a false value.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3251/files#r5241172
.

: val
: defaultVal;
}

/**
* @ngdoc directive
27 changes: 26 additions & 1 deletion test/ng/directive/inputSpec.js
View file
Original file line number Diff line number Diff line change
@@ -876,7 +876,7 @@ describe('input', function() {
});


it('should allow custom enumeration', function() {
it('should allow custom string enumeration', function() {
compileInput('<input type="checkbox" ng-model="name" ng-true-value="y" ' +
'ng-false-value="n">');

@@ -902,6 +902,31 @@ describe('input', function() {
expect(scope.name).toEqual('n');
});

it('should allow custom numerical enumeration', function() {
compileInput('<input type="checkbox" ng-model="name" ng-true-value="1" ' +
'ng-false-value="0">');

scope.$apply(function() {
scope.name = 1;
});
expect(inputElm[0].checked).toBe(true);

scope.$apply(function() {
scope.name = 0;
});
expect(inputElm[0].checked).toBe(false);

scope.$apply(function() {
scope.name = 'something else';
});
expect(inputElm[0].checked).toBe(false);

browserTrigger(inputElm, 'click');
expect(scope.name).toEqual(1);

browserTrigger(inputElm, 'click');
expect(scope.name).toEqual(0);
});

it('should be required if false', function() {
compileInput('<input type="checkbox" ng:model="value" required />');