Corrected Signup form validation #5116

ananyaarun · 2019-03-16T22:43:04Z

PR is descriptively titled 📑 and links the original issue above 🔗
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 📁
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

Thanks!

ananyaarun · 2019-03-16T22:44:50Z

@gauravano , Form doesn't accept null values anymore during signup. Also i have made only username , email. password and repeat password compulsory. Since image and Bio is up to user if they want to add or not.

plotsbot · 2019-03-16T22:55:42Z

	1 Message
📖	@ananyaarun 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.

Generated by 🚫 Danger

grvsachdeva

the sign-up button is not coming back so the user has to refresh their browser tab and fill the form again. the ideal behavior would be to validate all fields on pressing Sign-up button and if fields are incorrect/empty then focus on those fields or messages?

ananyaarun · 2019-03-17T15:18:33Z

@gauravano , The thing as of now I have used the same method of form validation in Both login and signup. But signup uses a rotating wheel fontawesome icon. ie upon one click the icon appears and signup option disappears as you said. Upon removing this (like how it isnt there for login) this is how it looks.

So now even if there is an error while filling there is a prompt to refill but the signup icon doesnt appear.
Thoughts ?

grvsachdeva · 2019-03-18T17:55:44Z

Can't we show that progress icon only when JavaScript validation is passed and in other cases we can just focus on the fields with error messages along with each field?

ananyaarun · 2019-03-18T18:46:43Z

@gauravano , Hmm I just looked into this. So the method of using required: true is basically form validation in ruby. Whereas this button is a JS element which always executes whenever the signup button is clicked.
I'm unsure of how to integrate the ruby validation along with JS spinning button script. I tried adding if conditions but the ruby form elements arent accessible outside the form template where the script is defined.
A solution would be to implement this just like login which doesnt have a spinning button at the moment.
I'll look more into this though and get back to you if something works ?

ananyaarun · 2019-03-19T15:04:10Z

@gauravano , So far i can get 2 solutions to this issue.

The signup from works and ensures it gets submitted only when all fields are filled. But the spinning wheel never appears.(Just like how login is implemented presently)
When i try to integrate the spinning wheel icon with ruby form validation, it appears on click no matter which fields are unfilled/filled.

Any thoughts on what can be done ?

grvsachdeva · 2019-03-20T20:22:00Z

How about this - "User fills the form partially and goes on to submit the form by pressing sign-up and we can show a warning on hovering over the sign-up button only or maybe disable it too?"

ananyaarun · 2019-03-20T20:44:50Z

So basically there are 4 compulsory fields a user needs to fill. The user may not fill this in any permutation or combination.
Right now what happens according to the changes i made is that whichever field isnt filled a "pls fill this " msg appears below it when the user clicks signup. So in a way it is disabled until all fields are filled by the user.

grvsachdeva · 2019-03-20T21:00:41Z

But, user need to refresh the screen with your approach right?

ananyaarun · 2019-03-20T21:04:42Z

No not any more, I made a slight change. I'll attach a gif in 10 minutes.

ananyaarun · 2019-03-20T21:14:02Z

@gauravano Here is the gif, User need not refresh after each "Pls fill this field" msg. Basically until all fields are filled this msg comes and form is submitted on one click after that.

grvsachdeva · 2019-03-20T21:56:33Z

app/views/users/_create_form.html.erb

          <label for="username"><%= t('user_sessions.new.username') %></label>
-          <%= f.text_field :username, { tabindex: 1, placeholder: "Username", class: 'form-control', id: 'username-signup' } %>
+          <%= f.text_field :username, { tabindex: 1, placeholder: "Username", class: 'form-control', id: 'username-signup', required:true } %>
+


Suggested change

Let's remove this empty space

grvsachdeva · 2019-03-20T21:56:45Z

app/views/users/_create_form.html.erb

          <label for="email"><%= t('users._form.email') %></label>
-          <%= f.text_field :email, { tabindex: 3, placeholder: "you@email.com", class: 'form-control', id: 'email' } %>
+          <%= f.text_field :email, { tabindex: 3, placeholder: "you@email.com", class: 'form-control', id: 'email', required: true } %>
+


Suggested change

grvsachdeva · 2019-03-20T21:59:43Z

app/views/users/_create_form.html.erb

-                   .html("<i class='fa fa-spinner fa-spin'></i>"); // make a spinner that spins when clicked
-          })
-        })();
-      </script>


Instead of removing this, we can disable the button only when all the required fields are filled i.e., spinner will show only when all required fields are filled otherwise focus on the fields?

Yes that would be ideal. Do you have any idea how this can be done ?. Because the problem is since button is a JS element and form validation happens in ruby, i wasn't able to find a way to check if all fields are filled before displaying the button. The fields i try to access show "Cant be accessed error on console". I'll keep trying other ways to do this in the meantime maybe i was wrong, Thanks @gauravano :)

The form/input fields we create in rails also result in the generation of HTML form only and so can be assessed just like that, open the sign-up form of publiclab.org and try $('#username-signup').val() in your console and you'll get the value of username field.

Thanks, I'll try this :)

grvsachdeva

Give a try to make changes in the script so we don't need to remove the spinner

ananyaarun · 2019-03-23T17:41:54Z

@gauravano , I got an idea how to implement this keeping the spinner button, Sorry have been a little busy with assignments, exams and writing my proposal lately. Will work on this as soon as i am done with these.

grvsachdeva · 2019-03-23T19:00:11Z

No issue! And, thanks for working on it.

SidharthBansal · 2019-04-29T19:21:52Z

There are some conflicts in your pr. It will be great if you can rebase your pr.
Thanks for the work done here at PL

grvsachdeva · 2019-05-31T23:04:19Z

Hi @ananyaarun, please update this PR too whenever you get time. Thanks!

ananyaarun · 2019-06-01T21:02:41Z

Sure, I have resolved the conflicts.
Still trying ways to keep the spinner. As of now validation works but I have removed the font awesome spinner icon.

SidharthBansal

Hi, changes looks good to me. Thanks for your work.
@jywarren @gauravano can you please review this and merge this?

jywarren · 2019-06-05T22:48:49Z

OK, just because this being signup, it's a very critical system, would you mind sharing a gif of the current latest functionality just to confirm, @ananyaarun ? Thank you!!!!

ananyaarun · 2019-06-06T12:01:41Z

@jywarren here
Basically won't allow a user signup without entering username and password.
Also other fields are optional (ie not 'required') as of now, but not entering those gives a spam detection msg as expected.

jywarren · 2019-06-06T14:28:23Z

Ok, would you mind testing this on unstable to be sure the recaptcha version of anti spam system works? I believe there's a PR open with new documentation on unstable server usage? Thanks!!!

…

On Thu, Jun 6, 2019, 5:02 AM Ananya Arun ***@***.***> wrote: @jywarren <https://github.com/jywarren> here Basically won't allow a user signup without entering username and password. Also other fields are optional (ie not 'required') as of now, but not entering those gives a spam detection msg as expected. [image: signup1 1] <https://user-images.githubusercontent.com/32260628/59031298-e9ba4180-8880-11e9-8a46-e4b0683c12aa.gif> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5116?email_source=notifications&email_token=AAAF6JZDUFOMRYVOTC45TADPZD4CNA5CNFSM4G7AI4LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXCTYYI#issuecomment-499465313>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6JZ5DFSQB5DC4ONUPCTPZD4CNANCNFSM4G7AI4LA> .

ananyaarun · 2019-06-06T14:50:24Z

Can I test this on unstable myself ? Are there permissions required ?
@gauravano @jywarren ?
Thanks!

ananyaarun · 2019-06-26T11:48:00Z

@gauravano @jywarren I dont have permission to push to unstable to test.
Can someone who has permission test and let me know if there are further changes to be made ?
Thanks !!

sashadev-sky · 2019-09-13T03:15:09Z

@gauravano I think this one might be stale, what do you think? The issue with it is it should be reopened perhaps as a general frontend validation feature in addition to the work that needs to be done on #3340.

If we can salvage a part of it and merge then lets do that, otherwise lets close it.
@ananyaarun what are your thoughts

SidharthBansal · 2019-09-13T08:24:36Z

I agree! For authentication issues please add my name also. Please feel free to close this issue and open a new issue as suggested above.

…

On Fri, Sep 13, 2019, 8:45 AM Sasha Boginsky ***@***.***> wrote: @gauravano <https://github.com/gauravano> I think this one might be stale, what do you think? The issue with it is it should be reopened perhaps as a general frontend validation feature in addition to the work that needs to be done on #3340 <#3340>. If we can salvage a part of it and merge then lets do that, otherwise lets close it. @ananyaarun <https://github.com/ananyaarun> what are your thoughts — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5116?email_source=notifications&email_token=AFAAEQ6WJ6ADJNXP2XICOLLQJMAUDA5CNFSM4G7AI4LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6T23IY#issuecomment-531082659>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFAAEQ54KVSG3SIMVI6XFNTQJMAUDANCNFSM4G7AI4LA> .

ananyaarun · 2019-09-13T10:46:03Z

Sure at @sashadev-sky !
Closing this issue would make sense. This has been open for quite some time, We don't want to confuse newcomers / GCI students with stale/ invalid issues.

Will u be opening another issue for this ?

cesswairimu · 2019-12-23T18:28:53Z

Hey @ananyaarun , seems the issue you were fixed in this PR was closed..Closing this. Thanks

ananyaarun mentioned this pull request Mar 17, 2019

Wrong error msg during signup #5115

Closed

grvsachdeva reviewed Mar 17, 2019

View reviewed changes

Fixed signup form validation

6bd0064

ananyaarun force-pushed the issue-3340 branch from fc77804 to 6bd0064 Compare March 19, 2019 15:17

grvsachdeva reviewed Mar 20, 2019

View reviewed changes

CleverFool77 mentioned this pull request Apr 7, 2019

Add error message when email is nil or of a different pattern #5398

Merged

5 tasks

grvsachdeva added the in progress label May 31, 2019

ananyaarun added 2 commits June 1, 2019 22:18

resolving conflicts

30b2cf8

signup validation without spin icon

89c20d7

ananyaarun mentioned this pull request Jun 4, 2019

Weekly Community Check-In #24 (Learning from Public Lab) #5828

Closed

SidharthBansal approved these changes Jun 5, 2019

View reviewed changes

cesswairimu closed this Dec 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Corrected Signup form validation #5116

Corrected Signup form validation #5116

ananyaarun commented Mar 16, 2019 •

edited

Loading

ananyaarun commented Mar 16, 2019

plotsbot commented Mar 16, 2019 •

edited

Loading

grvsachdeva left a comment

ananyaarun commented Mar 17, 2019 •

edited

Loading

grvsachdeva commented Mar 18, 2019 •

edited

Loading

ananyaarun commented Mar 18, 2019

ananyaarun commented Mar 19, 2019

grvsachdeva commented Mar 20, 2019

ananyaarun commented Mar 20, 2019

grvsachdeva commented Mar 20, 2019 •

edited

Loading

ananyaarun commented Mar 20, 2019 •

edited

Loading

ananyaarun commented Mar 20, 2019

grvsachdeva Mar 20, 2019

grvsachdeva Mar 20, 2019

grvsachdeva Mar 20, 2019

ananyaarun Mar 21, 2019

grvsachdeva Mar 21, 2019

ananyaarun Mar 21, 2019

grvsachdeva left a comment

ananyaarun commented Mar 23, 2019

grvsachdeva commented Mar 23, 2019

SidharthBansal commented Apr 29, 2019

grvsachdeva commented May 31, 2019

ananyaarun commented Jun 1, 2019

SidharthBansal left a comment

jywarren commented Jun 5, 2019

ananyaarun commented Jun 6, 2019

jywarren commented Jun 6, 2019 via email

ananyaarun commented Jun 6, 2019 •

edited

Loading

ananyaarun commented Jun 26, 2019

sashadev-sky commented Sep 13, 2019

SidharthBansal commented Sep 13, 2019 via email

ananyaarun commented Sep 13, 2019

cesswairimu commented Dec 23, 2019

Corrected Signup form validation #5116

Corrected Signup form validation #5116

Conversation

ananyaarun commented Mar 16, 2019 • edited Loading

ananyaarun commented Mar 16, 2019

plotsbot commented Mar 16, 2019 • edited Loading

grvsachdeva left a comment

Choose a reason for hiding this comment

ananyaarun commented Mar 17, 2019 • edited Loading

grvsachdeva commented Mar 18, 2019 • edited Loading

ananyaarun commented Mar 18, 2019

ananyaarun commented Mar 19, 2019

grvsachdeva commented Mar 20, 2019

ananyaarun commented Mar 20, 2019

grvsachdeva commented Mar 20, 2019 • edited Loading

ananyaarun commented Mar 20, 2019 • edited Loading

ananyaarun commented Mar 20, 2019

grvsachdeva Mar 20, 2019

Choose a reason for hiding this comment

grvsachdeva Mar 20, 2019

Choose a reason for hiding this comment

grvsachdeva Mar 20, 2019

Choose a reason for hiding this comment

ananyaarun Mar 21, 2019

Choose a reason for hiding this comment

grvsachdeva Mar 21, 2019

Choose a reason for hiding this comment

ananyaarun Mar 21, 2019

Choose a reason for hiding this comment

grvsachdeva left a comment

Choose a reason for hiding this comment

ananyaarun commented Mar 23, 2019

grvsachdeva commented Mar 23, 2019

SidharthBansal commented Apr 29, 2019

grvsachdeva commented May 31, 2019

ananyaarun commented Jun 1, 2019

SidharthBansal left a comment

Choose a reason for hiding this comment

jywarren commented Jun 5, 2019

ananyaarun commented Jun 6, 2019

jywarren commented Jun 6, 2019 via email

ananyaarun commented Jun 6, 2019 • edited Loading

ananyaarun commented Jun 26, 2019

sashadev-sky commented Sep 13, 2019

SidharthBansal commented Sep 13, 2019 via email

ananyaarun commented Sep 13, 2019

cesswairimu commented Dec 23, 2019

ananyaarun commented Mar 16, 2019 •

edited

Loading

plotsbot commented Mar 16, 2019 •

edited

Loading

ananyaarun commented Mar 17, 2019 •

edited

Loading

grvsachdeva commented Mar 18, 2019 •

edited

Loading

grvsachdeva commented Mar 20, 2019 •

edited

Loading

ananyaarun commented Mar 20, 2019 •

edited

Loading

ananyaarun commented Jun 6, 2019 •

edited

Loading