-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Our415-45] Page Not Found page and component #274
Conversation
app/pages/OrganizationDetailPage.tsx
Outdated
if ("message" in org) { | ||
throw new Error(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: @adriencyberspace I'm not sure this check will work as you might think? The fetchOrganization
function does not return an object with a message property currently. (Or does it????) Were you looking at fetchService
? You may need to replicate the response handling and return values I added in that file a few months ago for this check to work similarly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have time to check the app this sec but I can perhaps a bit later or tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! I was referencing the ServiceDetailPage logic and then didn't check that part. Removed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I add error handling logic to Organization.ts
instead?
app/components/ui/PageNotFound.tsx
Outdated
We’re sorry, but the page you’re looking for can’t be found. The URL may | ||
be misspelled or the page you’re looking for is no longer available. | ||
</p> | ||
<Button href="/">Go to the homepage →</Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: Not your fault, but bummer this isn't a react router dom <Link to...>
so we wouldn't have to do a whole page reload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is my fault! I built the built the Button. Also realized there's an arrowVariant prop I can use instead of this
🎫: 404 page / page not found
This creates the
PageNotFound
component and page per mockupAdds component to:
ServiceDetailPage
OrganizationDetailPage
NOTE: This keeps the back page and also makes it easy to pass in different messages as props if we decide to do so in the future
Adds
PageNotFoundPage
toRouter
as a catch-allScreenshots
Firefox 132.0.2 (aarch64)