-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Core: move message processing from formatAndAdd
to defaultMessage
#1644
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
Conversation
@@ -213,6 +213,9 @@ $.validator.format = function( source, params ) { | |||
return $.validator.format.apply( this, args ); | |||
}; | |||
} | |||
if ( params === undefined ) { |
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.
Unrelated change?
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.
I did it on purpose because of the line 1013 in this PR.
Should I omit it and change the line 1013 to the following:
message: this.defaultMessage( element, { method: "remote", parameters: "" } )
thanks! |
"<strong>Warning: No message defined for " + element.name + "</strong>" | ||
); | ||
}, | ||
defaultMessage: function( element, 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.
This is a BC Break. Previusly the second argument was an string and now its an object.
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.
if (typeof rule === 'string') {
rule = {method: 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.
You're absolutely right!
But just a note, undocumented methods are, in the first place, used internally, and only a bunch o methods are intended to be used elsewhere:
The validate method returns a Validator object that has a few public methods that you can use to trigger validation programmatically or change the contents of the form. The validator object has more methods, but only those documented here are intended for usage.
- Validator.form() – Validates the form.
- Validator.element() – Validates a single element.
- Validator.resetForm() – Resets the controlled form.
- Validator.showErrors() – Show the specified messages.
- Validator.numberOfInvalids() – Returns the number of invalid fields.
There are a few static methods on the validator object:
- jQuery.validator.addMethod() – Add a custom validation method.
- jQuery.validator.format() – Replaces {n} placeholders with arguments.
- jQuery.validator.setDefaults() – Modify default settings for validation.
- jQuery.validator.addClassRules() – Add a compound class method.
...
But I can fix that in a followup proposal to maintain backward compatibility with code that still relaying on the old behavior.
Thanks for the note!
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.
How can we write custom validation rules then?
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.
Like any of the additional methos are written
https://github.com/jzaefferer/jquery-validation/tree/master/src/additional
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.
None of them use a remote validation.
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.
I'll create a PR soon with the following patch:
src/core.js | 8 +++++++-
test/test.js | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/core.js b/src/core.js
index 743f028..b497c4f 100644
--- a/src/core.js
+++ b/src/core.js
@@ -803,6 +803,10 @@ $.extend( $.validator, {
},
defaultMessage: function( element, rule ) {
+ if ( typeof rule === "string" ) {
+ rule = { method: rule };
+ }
+
var message = this.findDefined(
this.customMessage( element.name, rule.method ),
this.customDataMessage( element, rule.method ),
@@ -1067,10 +1071,12 @@ $.extend( $.validator, {
},
previousValue: function( element, method ) {
+ method = typeof method === "string" && method || "remote";
+
return $.data( element, "previousValue" ) || $.data( element, "previousValue", {
old: null,
valid: true,
- message: this.defaultMessage( element, { method: method } )
+ message: this.defaultMessage( element, method )
} );
},
diff --git a/test/test.js b/test/test.js
index 12541c2..05ec156 100644
--- a/test/test.js
+++ b/test/test.js
@@ -678,7 +678,7 @@ test( "option: errorClass with multiple classes", function() {
test( "defaultMessage(), empty title is ignored", function() {
var v = $( "#userForm" ).validate();
- equal( v.defaultMessage( $( "#username" )[ 0 ], { method: "required", parameters: true } ), "This field is required." );
+ equal( v.defaultMessage( $( "#username" )[ 0 ], "required" ), "This field is required." );
} );
test( "#741: move message processing from formatAndAdd to defaultMessage", function() {
If there are other things I should take care off, please mention them and I will update before sending the PR
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.
I don't see why change message: this.defaultMessage( element, { method: method } )
May add some kind of deprecation comment about the old parameter type so it could be removed on next major version
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.
Make sense.
I'll do the update soon.
Thanks!
As of now the signature of `validator.defaultMessage( element, string )` is deprecated. Use `validator.defaultMessage( element, object )` instead where objet is defined as ``` object = { method: "method name", parameters: "the given method parameters" } ```
Fixes #741