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

325-unauthed-experience #326

Merged
merged 2 commits into from
Nov 30, 2023
Merged

325-unauthed-experience #326

merged 2 commits into from
Nov 30, 2023

Conversation

alishaevn
Copy link
Contributor

@alishaevn alishaevn commented Nov 30, 2023

Story

correctly block unauthed users from pages that require auth

previously, a page that required authorization would show the spinning modal indefinitely, instead of showing the message that informs the user they must be signed in. this is because the variable that determined whether the spinner should show, always remained true.

this commit breaks out the logic to first check if there is a user. what renders on the page is based on that.

checking for the presence of a session has to come after all of the hooks so we don't violate the react-hooks/rules-of-hooks rule. ref: https://legacy.reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level we return the loading state in two different locations because it's rendered based on separate conditions. when isLoading is true because we don't have a user, it doesn't ever become false. that's why we were previously returning the loading spinner indefinitely. using a guard clause with an early return inside the api methods also violates the react-hooks/rules-of-hooks rule.

Screenshots / Video

Screen.Recording.2023-11-30.at.01.15.45.PM.mp4

Testing

  • visit "/requests" as a logged out user
  • verify that you see messaging about signing in, not a loading spinner (could take 3-5 seconds)

previously, a page that required authorization would show the spinning
modal indefinitely, instead of showing the message that informs the user
they must be signed in. this is because the variable that determined
whether the spinner should show, always remained true.

this commit breaks out the logic to first check if there is a user. what
renders on the page is based on that.

checking for the presence of a session has to come after all of the
hooks so we don't violate the react-hooks/rules-of-hooks
rule. ref: https://legacy.reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
we return the loading state in two different locations because it's
rendered based on separate conditions. when isLoading is true because we
don't have a user, it doesn't ever become false. that's why we were
previously returning the loading spinner indefinitely.
using a guard clause with an early return inside the api methods also
violates the react-hooks/rules-of-hooks rule.
Copy link

vercel bot commented Nov 30, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
webstore-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 30, 2023 7:54pm

@alishaevn alishaevn changed the title 325-unathed-experience 325-unauthed-experience Nov 30, 2023
@alishaevn alishaevn merged commit a463bcf into main Nov 30, 2023
3 checks passed
@alishaevn alishaevn linked an issue Nov 30, 2023 that may be closed by this pull request
1 task
@alishaevn alishaevn deleted the 325-unauthed-experience branch January 24, 2024 20:39
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.

unauthed experience
2 participants