Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
Core: Add greaterThan and lessThan methods.#2061
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Core: Add greaterThan and lessThan methods.#2061
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for the PR. Looks good to me. Can you also unbind the event and remove the class name in Lines 1121 to 1123 in539fa4d
As for testing the above request, you can do the same as the PR#1709 |
@Arkni So did I get it, or do I need to do some more studying of this repo? |
| $.validator.addMethod( "greaterThan", function( value, element, param ) { | ||
| var target = $( param ); | ||
| if ( this.settings.onfocusout && target.not( ".validate-greaterThan-blur" ).length ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why do we need this focus handling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
My motivation was to follow the pattern of theequalTo method, and this was introduced there two years ago inCore: Bind theblur event just once inequalTo rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just a clarification, the focus handling was added +5 years ago in this commitaca144b#diff-a692042a498ec7e201f38855d169e7b1R1163. The only thing that was added in the commit linked by@RobJohnston is binding the blur event one time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to me.
@RobJohnston do you have a actual use case for this newly added methods? I am fine with adding them in case there is a need for it. |
Yes I do. I have an application where people must record the inventory that they use, and each item in inventory has a serial number. At the end of the day, they have a range of consumed inventory, and this validation would ensure that the start serial number is less than the end serial number (or end is greater than start). If they only consumed one piece of inventory, then the end serial number has to be greater than or equal to the start serial number. I thought it would be an easy PR to accept due to the previous discussion in#458 (comment). |
Thx for your detailed feedback |
Fixes#2030.
Tests are limited to integers, but I think that other data types would work too.