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

Generate the 404 page using React on the frontend. #2168

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

lindapaiste
Copy link
Collaborator

@lindapaiste lindapaiste commented Mar 20, 2023

Fixes #2159
Fixes #2256

Screenshot 2023-03-19 190246

Done:

  • Copy the HTML and CSS from the server 404 page to the frontend.
  • Support translating the page texts (needs the translations).
  • Fetch a random sketch from user p5.
  • Display sketch using existing EmbedFrame component.
  • Disable the portions of the EmbedFrame code for sending errors to the IDE console from when it is used on the 404 page (via a new sendMessages prop).
  • Adjust the canvas size to fill the area below the message only.
  • Add a link to the sketch.
  • Show the toolbar at the top of the 404 page.
  • Remove malformed assets URL replacement. (Improper loading of static files on 404 pages #2256)
  • Add a catch-all react-router route to show the 404 page when no route matches.
  • Change the catch-all server route to return the renderIndex() content with a 404 status.
  • Cleanup server "object exists" checks with async/await syntax.
  • Show the frontend 404 page when a matching user/collection/sketch object is not found by passing a variable window.__SERVER_404__ from server to client.

To Do:

  • Figure out the correct settings for the textOutput gridOutput and basePath props.
  • (maybe?) Create an API endpoint to return a single random sketch, instead of fetching all.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste lindapaiste marked this pull request as ready for review June 19, 2023 17:31
@lindapaiste lindapaiste requested a review from raclim June 19, 2023 17:31
@lindapaiste
Copy link
Collaborator Author

lindapaiste commented Jun 19, 2023

@raclim I've got this to a point where it's working and I feel that it's a major improvement. Let me now if you have any questions about the code or any other feedback.

We will eventually want to add translations of the 404 page text for all languages.

There are some minor TODOs remaining. The missing env/config vars "PREVIEW_SCRIPTS_URL" and "EDITOR_URL" cause warnings in the console but no other problems. From what I can tell these vars are used for dispatching messages from the embedded sketch to the editor so that they can be displayed in the console. That behavior is not needed here. Ideally I would figure out how to avoid executing those parts of the code. done.

@lindapaiste lindapaiste changed the title [WIP] Generate the 404 page using React on the frontend. Generate the 404 page using React on the frontend. Aug 3, 2023
@lindapaiste
Copy link
Collaborator Author

@raclim I removed the refactoring of userExists etc. to make this clearer. I will submit those changes in a separate PR.

@lindapaiste
Copy link
Collaborator Author

ugh I need to fix the import of the Nav now that we refactored it and moved things around.

@raclim
Copy link
Collaborator

raclim commented Sep 8, 2023

Thanks so much for keeping up and working on all of these different components and changes so far!

Copy link
Contributor

@PiyushChandra17 PiyushChandra17 left a comment

Choose a reason for hiding this comment

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

Nicely Done 👍

@KrishavRajSingh KrishavRajSingh mentioned this pull request Dec 23, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PRs to Review
Development

Successfully merging this pull request may close these issues.

Improper loading of static files on 404 pages Discussion: Why is the 404 page rendered by the server?
3 participants