search

netceteragroup / valdr

A model centric approach to AngularJS form validation
http://netceteragroup.github.io/valdr/
MIT License
153 stars 43 forks source link

fix(sizeValidator):changes sizeValidator to accept numbers #106

Closed lonnelars closed 8 years ago

lonnelars commented 8 years ago

We're using valdr in our project, and I came across the bug that is described in issue #59, where an input field with type="number" is always in error. This PR solves it by converting the input value in sizeValidator to a string before checking the length.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling a587c297386ea18f5a19b939891a78ecf859386c on lonnelars:issue#59 into ec235ce5c36306ec0d3f82436808a12ca52c69df on netceteragroup:master.

marcelstoer commented 8 years ago

The fix is fine but I question the initial bug report. If you define a size constraint for a field that uses type="number" in the markup hasn't something else gone wrong fundamentally? Why should valdr try to silently cope with this?

lonnelars commented 8 years ago

Hm, you might be right. I just saw the open bug report, and figured I'd send in a PR. However, on some occasions I think it makes sense to speak of the length of numbers. In our case, we need to validate a postal code (zip code). To me it makes more sense to say that the postal code should be four digits, than saying that it should be less than 10000.

marcelstoer commented 8 years ago

Be that as it may, size is defined to constrain the length of strings: https://github.com/netceteragroup/valdr#size. I really don't see a compelling reason to merge this and unless you find a convincing argument (who knows, there may be one 😉 ) I'm also gonna close the initial bug report.

lonnelars commented 8 years ago

Sounds good to me :ok_hand: