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

Replace all jQuery validate instances with built-in HTML or JS validation #9605

Closed
3 of 4 tasks
rebecca-shoptaw opened this issue Jul 22, 2024 · 0 comments · Fixed by #9816
Closed
3 of 4 tasks

Replace all jQuery validate instances with built-in HTML or JS validation #9605

rebecca-shoptaw opened this issue Jul 22, 2024 · 0 comments · Fixed by #9816
Assignees
Labels
Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] Priority: 3 Issues that we can consider at our leisure. [managed] Type: Epic A feature or refactor that is big enough to require subissues. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]

Comments

@rebecca-shoptaw
Copy link
Collaborator

rebecca-shoptaw commented Jul 22, 2024

Problem

As part of an overall reduction of the OL code size to improve site performance, one of the most obvious and easiest targets is jQuery validate, a form validation plugin that can be almost completely duplicated with simpler, faster built-in HTML checks.

Current behavior:

Currently, the plugin is used like so to ensure required inputs are filled, emails are correctly formatted, and the publish date is not more than a year in the future:

* To enable, add class="validate" to the form and add required validations in class to the input elements.
* Available validations are: required, email, and publish-date.
*
* Example:
* <form name="create" class="validate">
* Username: <input type="text" class="required" name="username" value=""/> <br/>
* E-mail: <input type="text" class="required email" name="username" value=""/> <br/>
* Password: <input type="text" class="required" name="username" value=""/> <br/>
* <input type="submit" name="submit" value="Register"/>
* </form>

Not only is the current implementation inefficient, but it has bizarre, non-user-friendly side-effects. For instance:

  • On the /contact page, the required attribute on the "Your Question" field, instead of displaying the desired error message if the form is submitted with an empty question, actually hides the whole question field, making the form impossible to submit:
Contact form validation
  • If a type="email" is added to an email input in a form using jQuery validate, the plugin adds non-user-friendly and visually awkward on-key-up validation as the user types
  • The error messages, such as this one in /add, are located only in the JavaScript, and cannot be translated - which you can verify by searching messages.pot for "Are you sure that"
Sample error message
  • All of these validation checks, including very important ones like "required" will not run if the user has JavaScript disabled

Proposal & Constraints

Since in practice jQuery validate is only used for 3 different checks, removing it should be very simple, and involve just:

  1. Replacing each instance of class="required" in a jQuery validate form with the HTML required attribute
  2. Replacing each instance of class="email" in a jQuery validate form with the HTML type="email" attribute
  3. For the publish-date custom validation, which is only used in add.html:
    a. Move the existing publish date validation function into the add-book.js file which handles other JS validation for the form
    b. Add the error message to the HTML so it can be correctly internationalized
    c. Ensure the function fires on the correct input on blur
  4. Clean up: Remove the validate class from each relevant form, remove the invocation from index.js, delete validate.js, and uninstall the plugin

Subtasks

Related files

validate.js
support.html
/admin/people/index.html
/admin/people/view.html
/books/add.html
/books/edit.html
/tag/add.html
/tag/edit.html
/author/edit.html

Stakeholders

@cdrini


Instructions for Contributors

  • Please run these commands to ensure your repository is up to date before creating a new branch to work on this issue and each time after pushing code to Github, because the pre-commit bot may add commits to your PRs upstream.
@rebecca-shoptaw rebecca-shoptaw added Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed] Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Jul 22, 2024
@rebecca-shoptaw rebecca-shoptaw self-assigned this Jul 22, 2024
@rebecca-shoptaw rebecca-shoptaw added the Type: Epic A feature or refactor that is big enough to require subissues. [managed] label Jul 22, 2024
@cdrini cdrini added Priority: 3 Issues that we can consider at our leisure. [managed] Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Jul 22, 2024
@RayBB RayBB added the Needs: Response Issues which require feedback from lead label Aug 10, 2024
@cdrini cdrini removed the Needs: Response Issues which require feedback from lead label Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] Priority: 3 Issues that we can consider at our leisure. [managed] Type: Epic A feature or refactor that is big enough to require subissues. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants