-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add state for current user #665
Conversation
@@ -0,0 +1,12 @@ | |||
import Cookies from "js-cookie"; |
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.
this is a util file that will be used for the login / register pages
context omg amaze |
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.
Looks great! Left some comments regarding interfacing with existing api/auth functionality and regarding changes vs #664 🚀
import React, { useState, useEffect } from "react"; | ||
import { type UserPrivate } from "../utils/types"; | ||
import { AuthStateEnum, type AuthState, CurrentUserContext } from "../contexts/CurrentUserContext"; | ||
import { Api } from "../utils/api"; |
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.
After #664 is merged, this will need to change to import * as Api
👍
navigate("/login"); | ||
return null; | ||
} else { | ||
return <p>loading</p>; |
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.
Should we have a more populated loading screen here (e.g. using Spinner
)? Not necessary though, we can just go back and add one later if we need...
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.
Yup, i'll change this to a centered Spinner
in the page
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.
LGTM! 🥳
finally got around to pulling this into a separate pr from the login / register pages...
this pr adds a
CurrentUserContext
to hold the state of the current user. It also adds aPrivateRoute
component which redirects people to the login page if they are not logged upon load.