Danielle/homepage-flickers-as-it-checks-authentication #654
Conversation
…ct if user is logged in
|
Gridiron Survivor Application
Project name: Gridiron Survivor Application
Only deployments on the production branch are activated automatically. If you'd like to activate this deployment, navigate to your deployments. Learn more about Appwrite Function deployments.
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…t displaying login page
… login status (null, true, false)
…n-survivor into danielle/635-bug-homepage-flickers-as-it-checks-authentication
Danielle254
changed the title
app/(main)/login/page.test.tsx
Outdated
| login: jest.Mock; | ||
| } | ||
|
|
||
| let mockUseAuthContext: MockUseAuthContext = { |
There was a problem hiding this comment.
Nit: The original const was fine since I don't think we plan on reassigning a new object to mockUseAuthContext.
There was a problem hiding this comment.
@CorinaMurg good point! Changed back to const.
| </p> | ||
| <p className="hidden leading-7 xl:block">Jimmy Fallon</p> | ||
| <section className={`grid ${isSignedIn === null ? '' : 'xl:grid-cols-2'} xl:grid-rows-none`}> | ||
| {(isSignedIn === null || isSignedIn === true) && |
There was a problem hiding this comment.
I wonder if we can make these conditional statements more cleaner.
Ex: {isSignedIn !== false ? (<GlobalSpinner />) : (<> Rest of code </>)}
Since null and true aren't false
There was a problem hiding this comment.
@alexappleget I like the explicit nature of these calls from a clarity of code perspective.
change let to const for mockUseAuthContext object
| @@ -23,7 +23,7 @@ type AuthContextType = { | |||
| getUser: () => Promise<IUser | undefined>; | |||
| login: (user: UserCredentials) => Promise<void | Error>; // eslint-disable-line no-unused-vars | |||
| logoutAccount: () => Promise<void | Error>; | |||
| isSignedIn: boolean; | |||
| isSignedIn: boolean | null; | |||
There was a problem hiding this comment.
When is isSignedIn null?
| @@ -39,7 +39,7 @@ export const AuthContextProvider = ({ | |||
| }: { | |||
| children: React.ReactNode; | |||
| }): JSX.Element => { | |||
| const [isSignedIn, setIsSignedIn] = useState<boolean>(false); | |||
| const [isSignedIn, setIsSignedIn] = useState<boolean | null>(null); | |||
There was a problem hiding this comment.
Why is the default null and not false?
|
|
||
| render(<Login />); | ||
|
|
||
| expect(screen.getByTestId('global-spinner')).toBeInTheDocument(); |
There was a problem hiding this comment.
Need to test that global-spinner is not in the document as well when isSignedIn is either false or true.
…n-survivor into danielle/635-bug-homepage-flickers-as-it-checks-authentication
…ication' of https://github.com/LetsGetTechnical/gridiron-survivor into danielle/635-bug-homepage-flickers-as-it-checks-authentication
…uthentication is complete
Danielle254
deleted the
danielle/635-bug-homepage-flickers-as-it-checks-authentication
branch
Closes #635
Issue:
If user is logged in and goes to gridironsurvivor.com or gridironsurvivor.com/login, the login page displays briefly before the user is redirected to the leagues landing page.
Desired behavior:
A user who is already logged in should see the global spinner (pulsing logo) while authentication takes place, then is taken straight to leagues landing page. Login page should not display at all.
For a user who is not logged in, the global spinner should display while authentication is checked, then see the login page.
Solution:
isSignedInvariable to have 3 states instead of the previous 2 - null, true, false. Null will satisfy the condition where authentication is being checked. If the user is logged in, state goes to true. If not logged in, state goes to false.GlobalSpinnerand added conditional rendering so that, if the user is logged in, they only see the spinner followed by the page redirect