savvato-software / dtim-mobile

The application we use to interact with people during a tech interview meeting.
1 stars 3 forks source link

Sign Me Up button in Im New Here is disabled perhaps confusingly #1

Closed haxwell closed 4 years ago

haxwell commented 4 years ago

In the mobile app, create a new profile by clicking I'm new here!, and then add your name, an incorrect email (like 'fdsfs@'), and a correct phone number (like 3035551212). The Sign Me Up button will be disabled, which I think is correct, but it doesn't let the user know that only an email OR a phone number is required. So if they removed their errant email address, this would be valid, and they could click the button.

So, if the phone number is filled in, it should say in the error message for the email, "Please clear this field, or enter a valid email.". In cases where the phone number is not entered, or is not valid, the error message on the email field should read "Please enter a valid email."

braydenc303 commented 4 years ago

Hi Johnathan,

I will try my hand at this one if you want to assign it to me.

Cheers,

Brayden

braydenc303 commented 4 years ago

When I ran through the app a couple of different times I did not see the issue occurring as described. I was able to sign up with either or both pieces of information. What I did notice was that I was not getting any error message when I tried to sign UP a second time. There was no success message, no error message, and there was nothing to redirect the user to the home page. Everything just stayed static leading the user to think that the app is not working. The user should get a message they they are already signed up and be redirected to login.

braydenc303 commented 4 years ago

It looks like the above behavior is due to a CORS error that I am getting. The app is attempting to redirect to login, but is being stopped by the browser.

image

haxwell commented 4 years ago

Hi Brayden, thanks for looking into this.. The issue you mentioned about signing up a second time, I think I've got that in Issue #7.. Let me know what you think..

And regarding the CORS error.. have you updated dtim-techprofile-api? That issue looks familiar, and I think I may have fixed it.. Maybe.

haxwell commented 4 years ago

Also I did reproduce this issue..

image

I edited the original description for clarity. Try reproducing it again.

haxwell commented 4 years ago

Steps to Reproduce, and Expected vs Actual Behavior. Tested with PR #19

  1. Click I'm New Here..

  2. Enter a valid name

  3. Enter email field, begin typing, notice the error message

    Expected: we should not be showing the error message until the onBlur event happens (when the cursor leaves the field). It should then validate the contents of the field.

    Actual: the validation error message does show for the email field.

  4. But ignore the email validation error message for now. Enter an invalid email, like 'bademail@'

  5. Enter the phone number field, and begin typing. Again,

    Expected: we should not be showing the error message until the onBlur event happens (when the cursor leaves the field). It should then validate the contents of the field.

    Actual: the validation error message does show for the phone number field.

  6. Finish entering a correct string of 10 digits in the phone number field.

  7. Notice the error message on the phone number field has gone away, thats good. +1

  8. The error message on the email field

    Expected: it should read "Please clear this field, or enter a valid email."

    Actual: it reads: "Please enter a valid email, OR a ten digit phone number."

Likewise, if starting from the top, you enter a valid name, and then a valid email address, as you begin typing the phone number, there should be no validation error message (wait till they exit the field to validate). If they leave the field with an invalid value, since the email address does have a valid value (in this scenario), the error on the phone number field should read "Please clear this field, or enter a valid phone number."

haxwell commented 4 years ago

Just merged #19 to address this.