-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Adding real time validation to signup #3452
Adding real time validation to signup #3452
Conversation
Let's not do this on keyup; it creates a bit of a disruptive experience to get errors thrown at you are you're typing. Let's do this on blur instead. I checked Google, Apple, Adobe, Amazon, Spotify; none of them give feedback as you type. Also validate before the form is submitted. |
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.
Happy to review any patches that pull out the JS from the template separately!
@cdrini was planning to add throttling/debouncing like we have in search, I have a few issues with blur which we can discuss on the call :) |
👍 Let's talk about it tomorrow! |
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Michael E. Karpeles <michael.karpeles@gmail.com>
Co-authored-by: Michael E. Karpeles <michael.karpeles@gmail.com>
Co-authored-by: Michael E. Karpeles <michael.karpeles@gmail.com>
Co-authored-by: Michael E. Karpeles <michael.karpeles@gmail.com>
Co-authored-by: Michael E. Karpeles <michael.karpeles@gmail.com>
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.
Let's open a new issue to sweep over this file and make sure it has the right ES6 syntax (e.g. const v. var / let).
openlibrary/plugins/openlibrary/js/realtime_account_validation.js
Outdated
Show resolved
Hide resolved
var value_email = $(this).val(); | ||
if (!value_email=='') { | ||
$.ajax({ | ||
url: `/account/validate?email=${value_email}`, |
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.
We may have to urlencode +
for the email. Otherwise, the API call will treat it as a space. This would means we would also have to urlencode the email on the other side.
encodeURIComponent
from js (before we do the ajax request)
unquote_plus
on the backend (within API)
openlibrary/plugins/openlibrary/js/realtime_account_validation.js
Outdated
Show resolved
Hide resolved
def validate_username(username): | ||
if not 3 <= len(username) <= 20: | ||
return _('Username must be between 3-20 characters') | ||
if not re.match('^[A-Za-z0-9-_]{3,20}$', username): |
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.
Suggestion: We should try to DRY this regex since we're now using it in 2 different places.
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.
Turns out, we only needed to uriencode on the frontend and then everything worked.
Nice! |
Issue: #3256
Closes #1433
Todo: