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

IRO-1307 - Address UI lag in rendering Magic Link state #126

Merged
merged 40 commits into from
Dec 10, 2021

Conversation

brekk
Copy link
Contributor

@brekk brekk commented Dec 6, 2021

  • Remove LoginContext usage
  • Number explicit error boundaries
  • thread props for loginContext, coming from pages/_app directly. This ends up being both more reliable and ultimately more readable / easier to reason about.
  • refactor to use useRouter instead of Router.push

@linear
Copy link

linear bot commented Dec 6, 2021

IRO-1307 Figure out why the page renders before the magic link data hooks

Our Toast says 'welcome back' but the other parts of the UI don't know you're logged in until a refresh. It's lame.

@vercel
Copy link

vercel bot commented Dec 6, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/ironfish/website-testnet/CVXhyaV2mc8EXw4tnmDWSCMiG46S
✅ Preview: https://website-testnet-git-feature-iro-1307-magic-link-7bcac1-ironfish.vercel.app

@brekk brekk marked this pull request as ready for review December 6, 2021 23:26
@brekk brekk requested a review from dguenther December 6, 2021 23:26
Copy link
Member

@dguenther dguenther left a comment

Choose a reason for hiding this comment

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

This doesn't work as expected. When logging out, after the redirect happens, the Login page appear to be logged out, but if you click on Leaderboard, the navbar appears to be logged in again.

The source of the issue is the multiple instances of useLogin created by useProtectedRoute. The site only needs one source of truth for whether the user is logged in or not, so we can centralize that in the single useLogin in _app.tsx. useLogin should either contain all of the auth logic and export functions to control it, or export functions to control its state so that e.g. logout can clear the state and the login callback can update or trigger a reload of the state.

The use of Context is irrelevant to fixing this issue, but I'm not opposed to prop-drilling since there aren't too many components in the component tree anyway.

hooks/useProtectedRoute.ts Outdated Show resolved Hide resolved
hooks/useLogin.ts Show resolved Hide resolved
pages/logout.tsx Outdated Show resolved Hide resolved
pages/_app.tsx Show resolved Hide resolved
pages/login.tsx Outdated Show resolved Hide resolved
@brekk
Copy link
Contributor Author

brekk commented Dec 7, 2021

The source of the issue is the multiple instances of useLogin created by useProtectedRoute.

Yeah, you're right. I'm not sure when I made that mistake but that's correct. Will try to address this morning.

@brekk brekk requested a review from dguenther December 10, 2021 17:33
@dguenther dguenther merged commit 1112e56 into master Dec 10, 2021
@dguenther dguenther deleted the feature/iro-1307-magic-link-tragic-stink branch December 10, 2021 22:25
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.

2 participants