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

Login and Registration Pages #11

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

Conversation

ShivaniVora
Copy link

@ShivaniVora ShivaniVora commented Jan 28, 2025

Login and Registration Pages

Issue Number(s): #3

  • Added in Login and Registration pages at routes /login and /register

Checklist

  • Login and signup pages utilize the form HTML tags, with proper submission handling. For now, simply printing the form data will suffice.
  • All states in the login / sign up flow (error or success) should be implemented and simulated with tests.
  • Write basic form validation logic, and display error states based on the validation. Assume that if data validation passes (for now), the state will be success. This will change with the status codes from the backend.
  • Routes: /login and /register. This is subject to change

@michaelhyi
Copy link
Contributor

michaelhyi commented Jan 29, 2025

Forget about Zod for now, just do some basic JS validation logic as discussed :)

Updated ticket requirements to reflect this

Overall, great work! Keep it up

@ShivaniVora ShivaniVora marked this pull request as ready for review January 31, 2025 04:30
Copy link
Contributor

@michaelhyi michaelhyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! This looks amazing and works well. I'd like to request some slight changes to the code, as you'll see in the comments. Mostly design related choices just for maintainability. In addition to the comments, could also consider:

  • The login and register modals are not the same width. Some padding between the elements in both modals aren't exactly equivalent either. Could we try to make this a bit more consistent?
  • Could we try indenting a bit more between JS logic? Not the biggest deal in the world, just makes it easier to read in the future.
  • Try to abstract some HTML into React components, especially if code is repeated! Not a strict requirement though, I'd say

Thanks for all your hard work! This is incredible

src/pages/login/index.tsx Show resolved Hide resolved
src/pages/register/index.tsx Outdated Show resolved Hide resolved
src/pages/login/index.tsx Outdated Show resolved Hide resolved
src/pages/register/index.tsx Outdated Show resolved Hide resolved
src/pages/register/index.tsx Show resolved Hide resolved
@mngo39
Copy link

mngo39 commented Feb 2, 2025

YAY! THANKS FOR ALL YOUR HARD WORK!!! It looks great, im super happy with how it turned out!
Login Pages:

  • currently can see the overflow scroll bars on my own full-screen view, but this might be a problem with my own laptop
  • would we be able to reduce the spacing between password input box and "forget password?" I think the spacing might be there to have the error message have room to appear on the incorrect screen, so would it be possible to reduce the spacing on the non-error regular landing page?
  • can we capitalize "Log In" and "Sign Up" where the second word might not be capitalized?
  • what's the current size of the font being used for "Forget Password?" "don't have an account" and error messages? I know we were having trouble with the font for those messages, so that might have played a part, but could we try making the font a few points bigger for better readability?

Sign Up Pages

  • could we have the error message under "Confirm Password" input box say "passwords don't match"?
  • same thing about the font size as the login pages
  • reducing the space between "Creating Account" and the loading icon

Stretch Goal

  • this is completely my fault for not clarifying earlier or leaving a comment in the Figma file, SO IT'S ON ME AND YOU DON'T HAVE TO DO THIS IF IT TAKES TOO MUCH TIME but maybe in a future sprint, having the loading icon be animated? or even replacing the icon with something/another icon that does animate to show loading.

@cynthiazhang755
Copy link

Hey Shivani,
Great job on the ticket! Thanks so much for your hard work!
Login page notes:

  1. I think all the components squish together a little and a scroll bar appears whenever the window isn't completely full screened. I think most users will probably be using the web app without full screening their browser, would it be possible to adjust the components to account for that?
Screenshot 2025-02-01 at 9 26 39 PM
  1. After entering an invalid email and password, clicking login, and then re-entering a valid email and password, the text is still red when they are typing in a new email. Can the text be changed back to grey when users are attempting to enter a valid email/password after inputting an invalid email/password?
Screenshot 2025-02-01 at 9 24 35 PM

Sign-up page notes:

  1. When I enter a valid password and then enter a different invalid password for the confirmation password field, the app still lets me create an account.

  2. When I enter an invalid password for both the password and confirm password fields, the app says that the password is invalid for both instead of saying that the passwords don't match for the confirm password field.

Screenshot 2025-02-01 at 9 33 11 PM
  1. Is there a way to check that the email account entered is a valid email that exists?

@ShivaniVora
Copy link
Author

I should have addressed all review comments:

  • Matched the width of register/login modals
  • Made error box component to abstract some HTML out of the pages
  • Abstracted validation logic into src/utils/validation.ts
  • in submit logic:
    • else for clearing error messages is there
    • returns errorDetected
  • Made login/register modals wider on narrower screens to help with overflow issues
  • Capitalization on Log in and Sign up is fixed
  • Spacing between password and forgot password is less
  • Forgot password is 20pt font and error messages are 19pt font (20pt font for both does not fit on the same line, so I did it this way for now. I think a different font will fix it though).
  • Space reduced between Create Account and loading icon
  • Loading icon is now animated
  • Text changes back to grey and error message disappears when a user goes to "fix an error"
  • Zod is now used for email validation
  • When an invalid password is entered and the same password is entered in the confirm password box, the error message shown is passwords don't match, even though the passwords match.

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.

4 participants