-
Notifications
You must be signed in to change notification settings - Fork 192
592: Tell users if they enter an invalid age #709
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
I signed it! |
nworden
left a comment
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.
Thanks for doing this!
app/resources/static/fixed/forms.js
Outdated
|
|
||
| if (age_value != "") { | ||
| var curated_age_value = age_value.replace(/\s/g,''); | ||
| var valid_age_re = /^(\d+-?)+\d+$/; |
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 think the question mark should go on the outside of the parens, and the plus sign there now can be dropped, like this:
^(\d+-)?\d+$
app/resources/static/fixed/forms.js
Outdated
|
|
||
| if (age_value != "") { | ||
| var curated_age_value = age_value.replace(/\s/g,''); | ||
| var valid_age_re = /^(\d+-?)+\d+$/; |
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.
It might be worth making this RE a constant outside the function, though it's ok here too.
app/resources/static/fixed/forms.js
Outdated
| var isAgeFormatValid = valid_age_re.test(curated_age_value); | ||
|
|
||
| if (!isAgeFormatValid) { | ||
| alert("The age entered is not in a valid format."); |
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'd like to be consistent about how we display errors, so could you please handle this like the others. The improper email address one is probably a good example you could follow: it's displayed with code a little bit above in this file, and the HTML is in app/resources/errors.html.template.
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
@googlebot I fixed it. |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
@googlebot I fixed it. |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
Worked on #592 ;
It is its most simplest validation form (which doesn't consider the CJK or if the order of the ages is proper (i.e. "34-0" might be considered invalid if the database doesn't handle that)).