LF-3946 Show a fallback screen instead of white screen #3110
Conversation
Ran i18n script which also fixed some alphabetical ordering
kathyavini
requested review from
antsgar and
SayakaOno
and removed request for
a team
packages/webapp/src/stories/ReactErrorFallback/ReactErrorFallback.stories.tsx
Show resolved
Hide resolved
packages/webapp/src/components/ErrorHandler/PureReactErrorFallback/index.tsx
Show resolved
Hide resolved
|
@loicsans tagging you in this one for French translations |
| }: PureReactErrorFallbackProps) => { | ||
| const { t } = useTranslation(); | ||
|
|
||
| const isDesktop = useIsAboveBreakpoint('(min-width: 600px)'); |
There was a problem hiding this comment.
Lately we've been using MUI's breakpoints and their hook, in this case like this
const isDesktop = useMediaQuery(theme.breakpoints.down('sm'));
That way we always use the same breakpoints instead of typing in the number in the components. I think there's only one remaining component using useIsAboveBreakpoint which could be refactored as well and then we could remove the custom hook
There was a problem hiding this comment.
No issues with this component, but for refactoring the last component that uses useIsAboveBreakpoint... the breakpoint is 856px. Is it safe to round up to 900px (MUI breakpoint md)? @SayakaOno it is your component so I want to defer to you! This is the floating button on finances.
I think it looks okay at 900 but I want to make sure I'm not missing something 🙂
There was a problem hiding this comment.
900px looks fine to me too. It looks like it was originally 768px, and I thought I just had followed Loïc's design, but I couldn't find any breakpoints in the mockups... also, it looks like Anto updated it to 856px here.
Setting a higher breakpoint seems safe...
| } | ||
|
|
||
| .title { | ||
| color: #16423d; /* Figma's teal 900 on this page is not is not the app's teal 900 -- this should be checked */ |
There was a problem hiding this comment.
The app's var is close enough that we could probably use that one?
There was a problem hiding this comment.
(Discussed today in tech daily but just noting here for GitHub too)...
This colour is part of the new design system; Loïc provided this really helpful Figma link. Since we're going to move towards using CSS variables according to this convention, we'll eventually go the other way and migrate the old teal over when ready.
antsgar
deleted the
LF-3946-show-a-fallback-screen-instead-of-white-screen
branch
…een-instead-of-white-screen LF-3946 Show a fallback screen instead of white screen
Description
fallback_user_action; see example event here)src/assets/colors.scssNote about the
Failed to import dynamic moduleerror:Sadly I think the "TypeError: Failed to fetch dynamically imported module" error will not trigger the error boundary. I thought this would be considered an error in the
<Routes />component, but you can see looking at this Sentry error that it is instead a system exception. If you scroll to the bottom and expand the Event Grouping Information, theBy In-Appsection (which contains the Component Stack for an error within the React component tree), is empty.Jira link: https://lite-farm.atlassian.net/browse/LF-3946
Type of change
How Has This Been Tested?
I triggered what would normally be a whitescreen type error crash in two ways:
litefarm_langlocal storage value and click the LF logo (inspired by our recent bug bash!)I also tried to recreate the
Failed to fetch dynamically imported moduleerror, and as mentioned above, could not trigger the boundary that way. If you can think of any other kinds of known TypeError to test on, that would be great 🙏Checklist: