We handle client-side validation in a piecemeal fashion. It would simplify things for the user and developers if we standardized on one JavaScript library for handling all client-side form validation. I propose we use this one: http://bassistance.de/jquery-plugins/jquery-plugin-validation/ Syntax is pretty simple: In the JavaScript: $(document).ready(function(){ $("#commentForm").validate(); }); In the markup: <form id="commentForm" method="get" action=""> <input id="cname" name="name" size="25" class="required" minlength="2" /> "class='required'" tells the script to validate that field as required. Other options include email, url, date, number, specific length, etc.
Owen, this idea sounds interesting. But what about CSS3 and html5 and advanced form validation ? won't your proposal result in some work to be done again once browser accept those advanced new features ?
I'm taking this up again today and doing tests with the plugin. In the release notes for the latest version (http://bassistance.de/2011/10/07/release-validation-plugin-1-9-0/): "Most notable is heavily improved compability with HTML5 controls: You can apply validation rules to input types like number, email or url, it’ll get picked up by the plugin if the type matches a rule, and a required attribute (with the argument) also works with both jQuery 1.6+ (prop) and previous versions (attr)." I will keep this in mind as I do my tests.
Created attachment 8474 [details] [review] Bug 6209 - Standardize on one plugin for client-side form validation This patch adds the jQuery "Validation" plugin (http://docs.jquery.com/Plugins/Validation) and includes it by default in the staff client, adding one default configuration: A translatable string to be used for a default "required" message. For a simple example I have modified the library entry form to mark required fields as required (something which had not been done before). Each required field has been given a class, "required" and the HTML5 attribute "required." The former adds the hook for the plugin, the latter adds browser-based validation in recent browsers (and which will be superceded by the plugin if JavaScript is enabled). In the template the form which should be validated is initialized like so: $("#FormID").validate(); This will check for inputs classed "required" without any additional configuration. More complex forms (with specific data types, for instance) will require more inline JavaScript configuration. To test, go to Administration -> Libraries and Groups and choose "New library." Submit the form without entering a library code or name. You should be prevented from submitting the form and be shown an error message by each field. To test browser-based validation, disable JavaScript and follow the same procedure.
Created attachment 8520 [details] [review] Another form validation plugin (Ketchup) I just wanted to propose another form validation plugin, Ketchup. It seems that both can achieve the same job (they're both translatable and highly customizable), but I don't like the way we have to declare "validation instructions" in HTML tags with Validation plugin: some of them can be declared in class attribute (which is designed for styling), and others have to be declared in separate non-standard attributes (eg: minlength="2") (by the way, how do we declare "instructions" with more than 1 parameter? the documentation is not clear about this). To avoid this, Validation plugin provide a way to declare these "instructions" in another attribute, like 'data-validate' for example, but looking at the code it seems that it requires Metadata plugin which has been integrated in jQuery since 1.4 (and ($.metadata) returns "undefined" with jQuery 1.7.1) With Ketchup, "validations instructions" are declared like this: <input type="text" name="foo" data-validate="validate(required,minlength(3))" /> Documentation and demos can be found here: http://demos.usejquery.com/ketchup-plugin/index.html Source is here: https://github.com/mustardamus/ketchup-plugin Patch attached shows a demo on the same page that previous patch for Validation (admin/branches.pl) It includes Ketchup plugin, a CSS file, and the latest version of jQuery (Ketchup doesn't work with jQuery 1.3.2 :/)
Note about the patch above: branchcode and branchname have been marked as 'required', Email as 'email', and url as 'url'. Submitting a form with errors inside causes window to scroll to the top of the page and all errors are displayed in front of corresponding inputs.
(In reply to comment #4) > I just wanted to propose another form validation plugin, Ketchup. I have no opinion on which one is the best, but, as you're in the same room, maybe it's worth a small discussion to share arguments!
> I have no opinion on which one is the best, but, as you're in the same room, > maybe it's worth a small discussion to share arguments! We did, and I asked Julian to submit a simple patch demonstrating his version so that I could test and so that others could give their opinions. I'd be very interested in hearing what others think.
I'm still trying to make a decision on this, and at the moment I'm still on the side of Jörn Zaefferer's Validation plugin if only because it's is under more active development. Points also go to Zaefferer for creating the original tabs plugin which was incorporated into jQueryUI. However, I want to fully understand these concerns: (In reply to comment #4) > > but I don't like the way we have to declare > "validation instructions" in HTML tags with Validation plugin: some of them > can be declared in class attribute (which is designed for styling) I don't think we should worry about "abusing" the class attribute. It's perfectly reasonable to use the class attribute for semantically-meaningful purposes. We use class selectors in many places in Koha jQuery scripts. > others have to be declared in separate non-standard attributes (eg: > minlength="2") (by the way, how do we declare "instructions" with more than > 1 parameter? the documentation is not clear about this). The Validation plugin lets you define multiple parameters if you're doing the configuration in your JavaScript. It sounds like Ketchup would give us the benefit of being able to define more complex validation parameters directly in HTML tag attributes. Validation would require that we do so in JavaScript. Does that get to the essence of the difference?
Created attachment 15017 [details] [review] Bug 6209 - Standardize on one plugin for client-side form validation This patch adds the jQuery "Validation" plugin (http://docs.jquery.com/Plugins/Validation) and includes it by default in the staff client along with an include file containing translatable strings for all of the plugin's possible messages. The staff client's main JavaScript file has been modified to perform automatic validation of any form with the class "validated." For a simple example I have modified the library entry form to mark required fields as required (something which had not been done before). Each required field has been given a class, "required" and the HTML5 attribute "required." The former adds the hook for the plugin, the latter adds browser-based validation in recent browsers (and which will be superceded by the plugin if JavaScript is enabled). This will check for inputs classed "required" without any additional configuration. More complex forms (with specific data types, for instance) will require more inline JavaScript configuration. $("#FormID").validate({ /* configuration details */ }); Also added to the library entry form are checks on the email and URL fields via classes ("email" and "url", respectively). This demonstrates validation of content without forcing the field to be required. To test, go to Administration -> Libraries and Groups and choose "New library." Submit the form without entering a library code or name. You should be prevented from submitting the form and be shown an error message by each field. Perform the same check with an existing entry. To test browser-based validation, disable JavaScript and follow the same procedure.
Looking good overall, but I've got a nit to pick: --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt @@ -31,6 +31,7 @@ tinyMCE.init({ height : "300", width : "700" }); + //]]> </script> That added line doesn't appear to correspond to a //<![CDATA[ That aside, I'm close to being ready to sign off on this.
Created attachment 15141 [details] [review] Bug 6209 [Revised] Standardize on one plugin for client-side form validation This patch adds the jQuery "Validation" plugin (http://docs.jquery.com/Plugins/Validation) and includes it by default in the staff client along with an include file containing translatable strings for all of the plugin's possible messages. The staff client's main JavaScript file has been modified to perform automatic validation of any form with the class "validated." For a simple example I have modified the library entry form to mark required fields as required (something which had not been done before). Each required field has been given a class, "required" and the HTML5 attribute "required." The former adds the hook for the plugin, the latter adds browser-based validation in recent browsers (and which will be superceded by the plugin if JavaScript is enabled). This will check for inputs classed "required" without any additional configuration. More complex forms (with specific data types, for instance) will require more inline JavaScript configuration. $("#FormID").validate({ /* configuration details */ }); Also added to the library entry form are checks on the email and URL fields via classes ("email" and "url", respectively). This demonstrates validation of content without forcing the field to be required. To test, go to Administration -> Libraries and Groups and choose "New library." Submit the form without entering a library code or name. You should be prevented from submitting the form and be shown an error message by each field. Perform the same check with an existing entry. To test browser-based validation, disable JavaScript and follow the same procedure. Revision rebases the patch against current master and adds missing "//<![CDATA["
Created attachment 15802 [details] [review] Bug 6209 [Revised] Standardize on one plugin for client-side form validation This patch adds the jQuery "Validation" plugin (http://docs.jquery.com/Plugins/Validation) and includes it by default in the staff client along with an include file containing translatable strings for all of the plugin's possible messages. The staff client's main JavaScript file has been modified to perform automatic validation of any form with the class "validated." For a simple example I have modified the library entry form to mark required fields as required (something which had not been done before). Each required field has been given a class, "required" and the HTML5 attribute "required." The former adds the hook for the plugin, the latter adds browser-based validation in recent browsers (and which will be superceded by the plugin if JavaScript is enabled). This will check for inputs classed "required" without any additional configuration. More complex forms (with specific data types, for instance) will require more inline JavaScript configuration. $("#FormID").validate({ /* configuration details */ }); Also added to the library entry form are checks on the email and URL fields via classes ("email" and "url", respectively). This demonstrates validation of content without forcing the field to be required. To test, go to Administration -> Libraries and Groups and choose "New library." Submit the form without entering a library code or name. You should be prevented from submitting the form and be shown an error message by each field. Perform the same check with an existing entry. To test browser-based validation, disable JavaScript and follow the same procedure. Revision rebases the patch against current master and adds missing "//<![CDATA["
Created attachment 15816 [details] [review] [SIGNED-OFF] Bug 6209 [Revised] Standardize on one plugin for client-side form validation This patch adds the jQuery "Validation" plugin (http://docs.jquery.com/Plugins/Validation) and includes it by default in the staff client along with an include file containing translatable strings for all of the plugin's possible messages. The staff client's main JavaScript file has been modified to perform automatic validation of any form with the class "validated." For a simple example I have modified the library entry form to mark required fields as required (something which had not been done before). Each required field has been given a class, "required" and the HTML5 attribute "required." The former adds the hook for the plugin, the latter adds browser-based validation in recent browsers (and which will be superceded by the plugin if JavaScript is enabled). This will check for inputs classed "required" without any additional configuration. More complex forms (with specific data types, for instance) will require more inline JavaScript configuration. $("#FormID").validate({ /* configuration details */ }); Also added to the library entry form are checks on the email and URL fields via classes ("email" and "url", respectively). This demonstrates validation of content without forcing the field to be required. To test, go to Administration -> Libraries and Groups and choose "New library." Submit the form without entering a library code or name. You should be prevented from submitting the form and be shown an error message by each field. Perform the same check with an existing entry. To test browser-based validation, disable JavaScript and follow the same procedure. Revision rebases the patch against current master and adds missing "//<![CDATA[" Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Work as described with new or existing library. No errors.
Sorry owen, but this patch does not apply anymore and it's out of my skills to fix the conflict in branches.tt Please rebase & set back to "signed off", i'll take care (hopefully tomorrow if you rebase during my european night ;-) )
Created attachment 16701 [details] [review] [SIGNED-OFF] Bug 6209 [Revised] Standardize on one plugin for client-side form validation This patch adds the jQuery "Validation" plugin (http://docs.jquery.com/Plugins/Validation) and includes it by default in the staff client along with an include file containing translatable strings for all of the plugin's possible messages. The staff client's main JavaScript file has been modified to perform automatic validation of any form with the class "validated." For a simple example I have modified the library entry form to mark required fields as required (something which had not been done before). Each required field has been given a class, "required" and the HTML5 attribute "required." The former adds the hook for the plugin, the latter adds browser-based validation in recent browsers (and which will be superceded by the plugin if JavaScript is enabled). This will check for inputs classed "required" without any additional configuration. More complex forms (with specific data types, for instance) will require more inline JavaScript configuration. $("#FormID").validate({ /* configuration details */ }); Also added to the library entry form are checks on the email and URL fields via classes ("email" and "url", respectively). This demonstrates validation of content without forcing the field to be required. To test, go to Administration -> Libraries and Groups and choose "New library." Submit the form without entering a library code or name. You should be prevented from submitting the form and be shown an error message by each field. Perform the same check with an existing entry. To test browser-based validation, disable JavaScript and follow the same procedure. Revision rebases the patch against current master and adds missing "//<![CDATA[" Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Work as described with new or existing library. No errors.
QA comment: * passes koha-qa.pl * work fine BUT : it seems the strings can't be translated. I've updated the french po, and strings like "This field is required." are not in the .po file. I've made a follow-up to fix this mistake, the feature work with it and the strings appear in the .po file passed QA
Created attachment 16771 [details] [review] [PASSED QA] Bug 6209 follow-up removing tabs when applying this patch, koha-qa.pl script complain about some tabs added instead of 4 spaces This patch replaces tabs to make koha-qa.pl happy & use our community rule everywhere Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 16772 [details] [review] [PASSED QA] Bug 6209 follow-up, make strings translatable Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
This patch has been pushed to master. I did not push the tab follow-up because it did not apply. As the tabs were in Javascript, I didn't care so much.