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

Feature: 404 page for invalid routes #2289

Merged
merged 16 commits into from
Dec 7, 2021

Conversation

gutsyphilip
Copy link
Contributor

@gutsyphilip gutsyphilip commented Dec 2, 2021

Added a custom 404 page to the wallet codebase. Currently, we don’t have any 404 handling, so any invalid routes always result in the user being re-directed back to the main dashboard page.

Extra

  • Alphabetized the translations for ease of reference

image

How it works:

When an invalid route is visited:

  • If no valid account is found, the user sees the ActivateAccountWithRouter component.
  • Otherwise, the new PageNotFound component is shown.

@netlify
Copy link

netlify bot commented Dec 2, 2021

👷 Deploy request for near-wallet-staging pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: 9cae3f9

@gutsyphilip gutsyphilip marked this pull request as ready for review December 2, 2021 18:18
@gutsyphilip gutsyphilip marked this pull request as draft December 2, 2021 18:19
@gutsyphilip gutsyphilip marked this pull request as ready for review December 2, 2021 18:27
@heycorwin
Copy link
Contributor

Lookin snazzy @worldclassdev

Copy link
Contributor

@Patrick1904 Patrick1904 left a comment

Choose a reason for hiding this comment

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

Thanks for your submission Philip! This is looking great. I added a few comments/suggestions above. How much time do you think you spent on putting this PR together?

@Patrick1904
Copy link
Contributor

@worldclassdev thanks for alphabetizing the translations. Did you use some type of tool for this or did you write a script? Thinking that if you have a script, we'd want to run this on file save so that we can keep this pattern going forward :)

@gutsyphilip
Copy link
Contributor Author

Thanks for your submission Philip! This is looking great. I added a few comments/suggestions above. How much time do you think you spent on putting this PR together?

It's slightly difficult to say exactly. But I had the original draft in like 30 minutes(I had already gone through the codebase in the last week, so I utilized styling from the guest landing page). And then worked with Corwin for an extra say 45-60 minutes to iterate through the design a little bit more.

@gutsyphilip
Copy link
Contributor Author

@worldclassdev thanks for alphabetizing the translations. Did you use some type of tool for this or did you write a script? Thinking that if you have a script, we'd want to run this on file save so that we can keep this pattern going forward :)

I used a vs-code extension called Sort JSON Objects.
https://marketplace.visualstudio.com/items?itemName=richie5um2.vscode-sort-json

I thought about having a script, but I felt once it was alphabetized, everyone should be able to follow the format and in cases where it isn't, it'd get caught in review. But for automation, I was thinking of using a pre-commit hook.

That fixes two things:

  • We can automatically lint all changed files and prevent committing until lint errors are fixed(This is something we don't have at the moment, but I think could be useful). Our current setup allows you to commit and push unused variables et al.
  • We can automatically sort the translation files.

@Patrick1904
Copy link
Contributor

@worldclassdev thanks for alphabetizing the translations. Did you use some type of tool for this or did you write a script? Thinking that if you have a script, we'd want to run this on file save so that we can keep this pattern going forward :)

I used a vs-code extension called Sort JSON Objects. https://marketplace.visualstudio.com/items?itemName=richie5um2.vscode-sort-json

I thought about having a script, but I felt once it was alphabetized, everyone should be able to follow the format and in cases where it isn't, it'd get caught in review. But for automation, I was thinking of using a pre-commit hook.

That fixes two things:

  • We can automatically lint all changed files and prevent committing until lint errors are fixed(This is something we don't have at the moment, but I think could be useful). Our current setup allows you to commit and push unused variables et al.
  • We can automatically sort the translation files.

Great idea to use a pre-commit hook! Is this something you think you'd be able to add with the remaining ~2 out of 4 total hours? No worries if not.

@gutsyphilip
Copy link
Contributor Author

gutsyphilip commented Dec 3, 2021

@worldclassdev thanks for alphabetizing the translations. Did you use some type of tool for this or did you write a script? Thinking that if you have a script, we'd want to run this on file save so that we can keep this pattern going forward :)

I used a vs-code extension called Sort JSON Objects. https://marketplace.visualstudio.com/items?itemName=richie5um2.vscode-sort-json
I thought about having a script, but I felt once it was alphabetized, everyone should be able to follow the format and in cases where it isn't, it'd get caught in review. But for automation, I was thinking of using a pre-commit hook.
That fixes two things:

  • We can automatically lint all changed files and prevent committing until lint errors are fixed(This is something we don't have at the moment, but I think could be useful). Our current setup allows you to commit and push unused variables et al.
  • We can automatically sort the translation files.

Great idea to use a pre-commit hook! Is this something you think you'd be able to add with the remaining ~2 out of 4 total hours? No worries if not.

Sure. I started working on it already on a separate branch.
PR: #2294

@Patrick1904
Copy link
Contributor

Sure. I started working on it already on a separate branch. I'd make a PR soon.

Awesome! 🙏

Copy link
Contributor

@Patrick1904 Patrick1904 left a comment

Choose a reason for hiding this comment

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

Great job with this! 💯

Copy link
Contributor

@MaximusHaximus MaximusHaximus left a comment

Choose a reason for hiding this comment

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

👍 This looks great! Nice work. :)

packages/frontend/yarn.lock Outdated Show resolved Hide resolved
@Patrick1904 Patrick1904 merged commit 401fdab into near:master Dec 7, 2021
@Patrick1904 Patrick1904 linked an issue Dec 7, 2021 that may be closed by this pull request
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.

404 page should rendering on non existing page
4 participants