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

Add NotFound component and 404 redirect #440

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gabalafou
Copy link
Contributor

@gabalafou gabalafou commented Oct 29, 2024

Related to conda-incubator/conda-store#899

Description

The basic idea of this pull request is that if the app loads and the user is at some unknown route such as /conda-store/foo/bar, the app will forcefully redirect them to /conda-store/not-found. This redirect should hit the server, which should return this React app along with a 404 HTTP status code. The app will in turn execute at this URL and render a "not found" page.

This pull request:

  • Does a client-side, hard redirect to {urlBasename}/not-found if no route matches. By hard, I mean it's not just a state transition via history.pushState() but an actual refresh of the browser page using window.location.replace()
  • Adds a NotFound component to be rendered when the {urlBasename}/not-found route matches.
screenshot of new 404 not found page

Pull request checklist

  • Did you test this change locally?
  • n/a Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

@gabalafou
Copy link
Contributor Author

I made this a draft PR for now until we have server-side support for the {urlBasename}/not-found route.

On the other hand, if we don't mind showing the server-controlled 404 page...

server 404 is a small json dictionary with key detail, value not found

@gabalafou
Copy link
Contributor Author

Since there is now a companion server pull request, I will take this out of draft.

@gabalafou gabalafou marked this pull request as ready for review October 29, 2024 16:16
@peytondmurray peytondmurray self-requested a review October 29, 2024 22:23
Copy link
Contributor

@peytondmurray peytondmurray left a comment

Choose a reason for hiding this comment

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

I'm curious about whether this works with the MemoryRouter, since you're directly modifying window.location. Otherwise seems good.

height: "100%"
}}
>
<Typography variant="h1" sx={{ fontSize: "18px", color: "#333" }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if now is the time to think about this, but do we have some global theme somewhere where these colors are stored? In the future we may want to support theming (so that we can integrate nicely with JupyterLab, among other reasons).

// useHref takes into account React Router basename option. So if basename is
// set to "/conda-store", the hook will return "/conda-store/not-found"
const url = useHref("/not-found");
window.location.replace(url);
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work when using MemoryRouter?

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