-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: error page when service worker not available #283
feat: error page when service worker not available #283
Conversation
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.
self review
src/index.tsx
Outdated
let ErrorPage: null | React.LazyExoticComponent<() => React.JSX.Element> = null | ||
if (navigator.serviceWorker == null) { | ||
ErrorPage = LazyServiceWorkerErrorPage | ||
} | ||
|
||
const routes: Route[] = [ | ||
{ default: true, component: LazyHelperUi }, | ||
{ default: true, component: ErrorPage ?? LazyHelperUi }, |
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.
gives us the ability to easily set different error pages for other conditions, without fronting the loading of all the errors for all users.
Updated UI to include the header & about components, and red error display with |
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.
reviewed latest changes
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.
LGTM.
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.
@SgtPooki thank you, lgtm.
Small suggestions/asks inline (feel free to merge once addressed)
Co-authored-by: Marcin Rataj <[email protected]>
Co-authored-by: Marcin Rataj <[email protected]>
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.
@lidel what do you think about a link to ff discussion
Co-authored-by: Russell Dempsey <[email protected]>
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.
yay, CI finally passed. merging. |
Title
feat: error page when service worker not available
Description
Basic error page implementation for users who don't have serviceWorker available.
Fixes #272
Notes & open questions
Change checklist