Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

United client- and server-side validation of float (closes #1430) #1462

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@redhead
Copy link
Contributor

commented Mar 21, 2014

This pull request unites client-side validation of the 'float' rule with the server-side counterpart.

The problem was in failing validation when a comma was used as a decimal point and/or spaces used as thousand separators. Both are handled on the server side by stripping spaces and replacing comma with period.

@pavelkouril

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2014

Hmmm, so it will fail on numbers written in 1,999.99 format?

@redhead

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2014

Yes. Just like the server-side validation does from the beginning (see docs).

Of course it would be better to implement some kind of locale support in Nette. This pull request, however, just fixes a filed issue.

@dg

This comment has been minimized.

Copy link
Member

commented Mar 22, 2014

This is more a workaround than a solution. On the server side validator float changes the value, so the following rules are already working with the changed value. Thus not only the range.

@redhead

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2014

Not sure what you mean. What would be your proposed solution?

@dg

This comment has been minimized.

Copy link
Member

commented Mar 23, 2014

You don't have to fix validator 'range', just 'float'.

@redhead

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2014

This would mean bigger refactoring since the validators on the client-side can't change values in the current implementation.

@dg

This comment has been minimized.

Copy link
Member

commented Mar 25, 2014

Exactly.

@redhead

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2014

I found out pretty questionable feature - server-side range validator can handle string values as well (all comparable types in fact), eg. array('a', 'g'), however, javascript range validator forces the value to be parsed as float. The current inconsistency makes it harder to unite the two.

It is evident in the description of bug #1430, where you need to add rule float so the comma and thousand separators are handled and value is changed. But if you omit the float rule, server-side validator just compares the given values as they are, but the client side parses it as float (no matter if the float rule was added).

@redhead

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2014

Bump

@dg

This comment has been minimized.

Copy link
Member

commented Apr 13, 2014

Yes, parseFloat should be removed from JS function range.

@janedbal

This comment has been minimized.

Copy link

commented Jul 11, 2014

What is missing for completing this issue?

@dg

This comment has been minimized.

Copy link
Member

commented Jul 11, 2014

@janedbal

This comment has been minimized.

Copy link

commented Jul 17, 2014

Thanks, is there any workaround? Ideally without modifying original script.

@dg dg force-pushed the nette:master branch 3 times, most recently from 991ba1a to e23de7a Aug 26, 2014

@dg dg force-pushed the nette:master branch from 872d693 to 1467a8d Jan 31, 2015

dg added a commit to nette/forms that referenced this pull request Feb 17, 2015

@janedbal

This comment has been minimized.

Copy link

commented Feb 17, 2015

@dg Great, thanks for resolving!

dg added a commit to nette/forms that referenced this pull request Feb 18, 2015

dg added a commit to nette/forms that referenced this pull request Feb 18, 2015

dg added a commit to nette/forms that referenced this pull request Feb 19, 2015

dg added a commit to nette/forms that referenced this pull request Feb 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.