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

[WIP] Validity checks for signin. #4444

Closed
wants to merge 2 commits into from

Conversation

kevinzluo
Copy link
Collaborator

Fixes #3439 (<=== Add issue number here)

Continued from #3984 . Just rebased, still a work in progress.

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • PR is descriptively titled 📑
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

Fix issue with password confirmation field

Add comments and fix scrollup

add spamaway completion check and possible check for google recaptcha

Move scripts to external file

allow submit on second, fix code
@ghost ghost assigned kevinzluo Dec 29, 2018
@ghost ghost added the in progress label Dec 29, 2018
@SidharthBansal SidharthBansal added this to the OAuth milestone Dec 29, 2018
@plotsbot
Copy link
Collaborator

2 Messages
📖 @kevinzluo Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖 Pull Request is marked as Work in Progress

Generated by 🚫 Danger

@SidharthBansal
Copy link
Member

@kevinzluo can you please paste the screenshots/gifs here?

@kevinzluo
Copy link
Collaborator Author

@SidharthBansal I am still working on it. The new changes might have caused some errors. I will post screenshots as soon as I feel it is working 👍 .

@SidharthBansal
Copy link
Member

ok lets finish #4438 first.

@kevinzluo
Copy link
Collaborator Author

There are some issues with this:
Because /signup and the signup modal use the same form, there are many repeated id's. This prevents the script from working at /signup. It only works on the modal. We need to either disable the modal at /signin or maybe remove /signup altogether (which is only possible after the error messages can be rendered on the modal). @SidharthBansal, @gauravano what do you think? Should we wait for @jywarren for this?

Until then I will work on my other issues.

@SidharthBansal
Copy link
Member

SidharthBansal commented Dec 31, 2018 via email

@jywarren
Copy link
Member

jywarren commented Jan 2, 2019

Could you put a container div around the re-used portion, which has a distinct classname, and then be referencing specific fields only with that nested reference, like:

$('.containerDiv input.username')...

What do you think? Thanks for being persistent and great with this one! I'm sure we'll be able to merge it soon and it will be great.

@SidharthBansal
Copy link
Member

@kevinzluo can you please resolve conflicts of this PR also?

@SidharthBansal
Copy link
Member

Closing this PR as Kevin is offline for many months. Kevin feel free to reopen it when you want to work on it again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants