-
Notifications
You must be signed in to change notification settings - Fork 0
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 component added #16
base: main
Are you sure you want to change the base?
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!
|
||
try { | ||
const response = await fetch( | ||
"http://localhost:8000/api/v1/auth/login", |
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.
Same comment I added to Beth's PR, let's not hard-code a URL in here for the backend. Instead, let's load it as an environment variable from the .env
file so it's easy to swap between the production and a locally-running instance of the backend.
i.e. process.meta.env + "/api/v1/auth/login"
|
||
if (response.ok) { | ||
const data = await response.json(); | ||
localStorage.setItem("token", data.token); |
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.
Doesn't have to be in this MR, but I think it would be great to set up a React context that holds this token and can call the login and register routes. That way, we can easily access the token from anywhere in our site by using the context.
This might be a good read for what I'm describing here: https://dev.to/miracool/how-to-manage-user-authentication-with-react-js-3ic5. I don't think we need to go crazy and have authenticated routes on our frontend, but the context itself would be really nice rather than loading from localstorage everywhere.
const [showPassword, setShowPassword] = useState(false); | ||
const navigate = useNavigate(); | ||
|
||
const validateEmail = (email) => { |
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.
I don't think we need to validate an email in the login form. It will either match something in the database or it won't.
I changed the login components as suggested by Eli.