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 #3 and #4 (KN-44 KN-70) Login Refactor #164

Merged

Conversation

agrimes23
Copy link
Collaborator

@agrimes23 agrimes23 commented Dec 14, 2024

This includes the login modal and refactored login page (if user tries to access dashboard but is not logged in) according to the figma wireframes. I used @JPEscobari's FormInput component and worked with him to make it reusable between both signup and login. The login modal uses this component as well, and uses the reusable modal in ui-components/Modal.

Notes:

  • Form Input errors for sign up only show when the user clicks away from the input and there is an error. Error does go away when the input is correct and the user clicks out of the input. This can be changed if another behavior is preferred.
  • Made sure that input error messages do not move other elements when they appear. The exception being the password validation since it's a deliberate behavior according to the wireframes.
  • Login Page and modal have styling for mobile, but login modal on mobile is a bit too long on mobile screens.

Some of @JPEscobari 's work with the signup is in this PR as well. Mainly updating the password validation. He will submit a PR for the rest of signup changes after this is merged with this user-auth branch!

AbigailDawson and others added 30 commits October 29, 2024 20:33
Signed-off-by: abigaildawson <[email protected]>
… bio photos that are no longer being used

Signed-off-by: abigaildawson <[email protected]>
Add JP to Core Contributors, update Past Core Contributors and remove…
Signed-off-by: abigaildawson <[email protected]>
Signed-off-by: abigaildawson <[email protected]>
…ization

JWT Authorization Tasks Complete
@agrimes23
Copy link
Collaborator Author

agrimes23 commented Dec 14, 2024

Looks like there's a lot of commits, but this includes code updated from develop and main that's not yet on this user-auth branch. Let me know if you'd like to discuss or would like for me to do something different

Copy link

Sorry! I’ll get to this soon! Been busy lately with life happenings!

Copy link

No worries all good! 🙌

AbigailDawson and others added 6 commits December 20, 2024 09:33
Signed-off-by: abigaildawson <[email protected]>
Signed-off-by: abigaildawson <[email protected]>
Signed-off-by: abigaildawson <[email protected]>
refactored DemoStudyText and DemoTranslateText from css to scss files
Copy link
Collaborator

@psantos2107 psantos2107 left a comment

Choose a reason for hiding this comment

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

Hey Alex! This looks amazing! (and luckily no merge conflicts). I love the flow! If possible, can you or JP make sure that the sign-up form is also a pop up, just like the login form is?

Copy link

Yes, JP and I will look into this!

@agrimes23
Copy link
Collaborator Author

@psantos2107 Just spoke with JP, and mentioned it would be easier to merge these login changes with user-auth first, and then he can work on the rest of the signup code tasks on a new branch. This PR is mainly for the login flow anyway, but does have some signup related code due to the reusable components between the two. Would it be okay to merge as is?

Copy link

yeah! That’s not a problem at all! I’ll merge it right now

@psantos2107 psantos2107 merged commit 0aa79b5 into user-authentication-authorization Jan 14, 2025
Copy link

huly-for-github bot commented Jan 14, 2025

Okay! It’s merged! 😊

Copy link

🙌 Awesome, thank you!

@agrimes23 agrimes23 deleted the alexg/KN-44-login-refactor branch January 14, 2025 20:51
Copy link
Collaborator

The Signup modal is done!

I’ve sent a pull request to user-authentication-authorization that should be an easy merge.

Copy link

Thanks JP! I’ll check it out later today

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.

7 participants