-
Notifications
You must be signed in to change notification settings - Fork 1
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
[auth] Implement login via username #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! My comments are mainly styling nits (styling wasn't originally in scope but I'm excited to see you doing it!)
Outstanding items:
- Ensure that sign up can only happen if account doesn't already exist for email
- Double check that usernames are being added to the profiles table when new users sign up
I finished the rest of the review changes but I needed some help moving the login/sign up button to the bottom of the page and toggling seeing the password. @akshaynthakur |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work on the styling! Lot of small nits to fix, but overall things are looking nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Couple small changes and this should be ready to merge!
Also, just checked with Brenda and she said that the text inputs on the sign up screen should also have labels above them, she just forgot to update the designs. Make sure to update those inputs!
I think merging this also lets us merge #42 ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INCREDIBLE job on this massive PR! Big congrats on it finally being merged
What's new in this PR
Relevant Links
https://supabase.com/docs/guides/database/postgres/triggers
https://github.com/orgs/supabase/discussions/8383
Notion Sprint Task
Sprint task
Online sources
Related PRs
How to review
Next steps
None
Tests Performed, Edge Cases
See "How to Review"
Screenshots
@akshaynthakur