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

Add HTML validation and remove jQuery validation from registration form #9244

Closed
Tracked by #9205
rebecca-shoptaw opened this issue May 9, 2024 · 0 comments · Fixed by #9245
Closed
Tracked by #9205

Add HTML validation and remove jQuery validation from registration form #9244

rebecca-shoptaw opened this issue May 9, 2024 · 0 comments · Fixed by #9245
Assignees
Labels
Lead: @rebecca-shoptaw FE: Internationalization, CSS, JS Priority: 2 Important, as time permits. [managed] registration Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] Type: Subtask of Epic A subtask that is part of the work breakdown of an epic issue (see comments). [managed]

Comments

@rebecca-shoptaw
Copy link
Collaborator

rebecca-shoptaw commented May 9, 2024

Dependency to epic #7694. Sub-task of #9205.

Describe the problem that you'd like solved

While the new real-time validation for formatting errors added in #2055 shows very helpful errors to the user, it does not actually prevent users from submitting forms with badly formatted inputs.

Proposal & Constraints

While in most cases, this problem could be solved speedily by adding relevant HTML attributes to the inputs, this particular form is generated via the forms library in form.py:
In the HTML:

<form class="olform create validate" name="signup" method="post" data-i18n="$json_encode(i18n_strings)" action="">
$if form.note:
<div class="note">$form.note</div>
$def screenname_url(): $_('Your URL'): https://openlibrary.org/people/<span id="userUrl">$(form.username.value or _('screenname'))</span>
$:field(form.email)
$:field(form.username, suffix=str(screenname_url()))
$:field(form.password)

In the Python:
class RegisterForm(Form):
INPUTS = [
Textbox(
'email',
description=_('Your email address'),
klass='required',
id='emailAddr',
validators=[
vemail,
):

However, luckily it is fairly straightforward to add any desired HTML attributes to the attribute list above, which translate to the correct attributes in the final rendered HTML.

One other issue here, which we also encountered in #8871, is that the jQuery validate plugin, called via the validate class in the form, starts adding extra error messages to the email field on key-up.

Since the validate class has no other effect on the form, as validation is handled by init_realtime_validation and forms.py validators, I propose removing it.

Additional context

Stakeholders

@cdrini

@rebecca-shoptaw rebecca-shoptaw added Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] registration Type: Subtask of Epic A subtask that is part of the work breakdown of an epic issue (see comments). [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 May 9, 2024
@rebecca-shoptaw rebecca-shoptaw self-assigned this May 9, 2024
@mekarpeles mekarpeles added Priority: 2 Important, as time permits. [managed] Lead: @rebecca-shoptaw FE: Internationalization, CSS, JS 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 May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lead: @rebecca-shoptaw FE: Internationalization, CSS, JS Priority: 2 Important, as time permits. [managed] registration Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] Type: Subtask of Epic A subtask that is part of the work breakdown of an epic issue (see comments). [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants