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

Bry/error alerts #82

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Bry/error alerts #82

wants to merge 6 commits into from

Conversation

bry-gavino
Copy link
Collaborator

✨ New in this PR ✨

One liner: what did you do? 🚨

Added alerts to user when certain TextFields are inputted incorrectly.

Coverage 🙆‍♀️

Alerts in these files:

  • RegisterScreen
    • all TextInputs must be filled
    • email must be formatted correctly
    • password must be >= 6 chars, have 1 uppercase char, have 1 lowercase char
    • passwords must match
  • LoginScreen
    • email & password must both exist and be correct
  • SettingsScreen
    • new email must be formatted correctly
    • new password must be formatted correctly
    • old password must be correct
    • tells you if a value has been updated
  • GeneralQuestionManager
    • if you try to continue to the specific caseType questions, it will alert you if you already filled out a form for it

How can the reviewer test your code? 👩‍💻

Go through all the cases as listed above in Coverage^^^

Any bugs you encountered or still having trouble with? 🐛

Had issue with using Text() from ContextProvider -- it was returning an Object instead of a String.

  • made a function to return String:

Screen Shot 2022-05-28 at 8 19 46 PM

🧜‍♀️ cc: @phoebeli23

@bry-gavino bry-gavino requested a review from shannonbonet May 29, 2022 03:26
@shannonbonet
Copy link
Member

Missing cases (WIP)

  • user tries logging in with a pre-existing email
  • no alert shows up if I do "shanshanlola@gmail
  • no password verification happens when resetting password

@gregoriiaaa
Copy link
Collaborator

Suggestions

  • On the Register screen: small description under of what a valid password looks like
  • On the Register screen: alert message for an invalid password doesn't describe all the issues with the password; for example, when I put in "password", I get an error message saying I need to include a capital letter, then I put in "Password", which then gives me an error message that I need to include a number, and so on; therefore, maybe it would be helpful to list all the reasons why a password is invalid at once
  • On the Settings Screen: alert message doesn't say why it could not update a field, could be helpful to add why (e.g., the password was missing a number)

Bugs

  • On the Settings screen: there is no error message for changing my email to one that is already linked to a different account and it also allows the change (also noticed that there are two different email fields for the clients on the firebase that do not correspond to each other; not sure if this is causing the mentioned bug)

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

Successfully merging this pull request may close these issues.

3 participants