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

Routing Improvements #797

Merged
merged 3 commits into from
May 10, 2024
Merged

Routing Improvements #797

merged 3 commits into from
May 10, 2024

Conversation

lowtorola
Copy link
Contributor

Fixes some routing insecurities and improves flow of control around episode switching.

  • Episode info must be loaded before the home page renders
  • Await load episode info in the episodeLoader
  • Indicate that the episode is loading in the episode switcher while it verifies that the episode exists
  • If the episode info load fails, redirect to 404 not found page
  • If the page load fails, redirect to 404 not found page
  • Navigating to "localhost:3000" redirects to current episode home page

Closes #794 🚀

@acrantel
Copy link
Member

acrantel commented May 9, 2024

just to clarify - does "redirect to 404 not found page" mean that we should navigate to something like /notfound or does it mean that we will display the "page not found" message on the screen? currently it seems to be the "page not found" message.

(if the current behavior is the expected behavior, then this PR lgtm!)

@lowtorola
Copy link
Contributor Author

@acrantel Yes it navigates to the "Page Not Found" message screen. I thought that seemed logical since technically for those bad URLs they are trying to access a page that doesn't exist

Copy link
Member

@acrantel acrantel left a comment

Choose a reason for hiding this comment

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

image

great work!

@lowtorola
Copy link
Contributor Author

:shipit:

@lowtorola lowtorola merged commit 05b3867 into main May 10, 2024
3 checks passed
@lowtorola lowtorola deleted the routing-lockdown branch May 10, 2024 18:01
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.

Routing Lockdown
2 participants