Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Changed the getValue utility function not to return null when given '' #51

Merged
merged 2 commits into from

3 participants

@willcumbie
Collaborator

No description provided.

@luizfar

Maybe we should have an unit tests for that?

@jpbochi

I'm glad there's one test that failed because of the change. I'm not sure hidden inputs should have a null value, though.

PhantomJS 1.9.2 (Linux) ko.validation.utilities getValue for hidden inputs returns null when no value is set FAILED
Expected '' to be null.
@luizfar

:point_up: That's a good point. I believe there may be places where we purposely treat '' differently than null. Maybe we could have getValue explicitly check for ''? Would that work?

@willcumbie
Collaborator

@luizfar I'm not sure what you're suggesting. What should we return in cases of '', other than '' itself?

@luizfar

I mean, return '' only if value is '' (may want to trim and stuff), otherwise return null.

@willcumbie
Collaborator

I'm not sure the text input itself will ever be equal to null or undefined. Where are you seeing those cases?

@luizfar

My comment was regarding other types of inputs that are not covered here. Returning '' is probably fine as I think that's anyways what the browser gives us. I just worry that we may want to distinguish '' from null in some places.
Either way, it's probably fine :)
:thumbsup:

@willcumbie willcumbie merged commit 0ec3fe4 into master

1 check passed

Details default The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 21, 2014
  1. @willcumbie

    Changed getValue to return null for empty hidden inputs but '' for no…

    willcumbie authored
    …rmal empty text inputs; updated tests
  2. @willcumbie
This page is out of date. Refresh to see the latest.
View
5 spec/validators/utilities-spec.js
@@ -127,9 +127,10 @@ describe('ko.validation.utilities', function () {
expect(utilities.getValue($('#test-input')[0])).toBe('x-wing');
});
- it('returns null when no value is set', function () {
+ it('returns empty string when the input has no text', function () {
setFixtures('<input type="text" value="" id="test-input"/>');
- expect(utilities.getValue($('#test-input')[0])).toBe(null);
+ $('#test-input').val('');
+ expect(utilities.getValue($('#test-input')[0])).toBe('');
});
});
View
4 src/validators/utilities.js
@@ -66,8 +66,10 @@ ko.validators.utilities = (function () {
return getSelectSingleValue(element);
case 'select-multiple':
return getSelectMultipleValue(element);
- default:
+ case 'hidden':
return element.value || null;
+ default:
+ return (element.value !== undefined) ? element.value : '';
}
};
Something went wrong with that request. Please try again.