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

feat: fe error boundary #391

Merged
merged 1 commit into from
Jul 8, 2022

Conversation

tomaszantas
Copy link

Description

ErrorBoundary intercepts the app exceptions, and shows the error message to the user.

TODO in next PR(s):

  • ask design team about current UI implementation

Motivation and Context

Show error to the user instead of crashing the app.

How Has This Been Tested?

image

image

@tomaszantas tomaszantas requested review from corquaid and tarnas14 July 8, 2022 12:17
/**
* Catches the app exceptions.
*
* This implementation tries to NOT depend on anything, ie. styled-components and themes.
Copy link

@corquaid corquaid Jul 8, 2022

Choose a reason for hiding this comment

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

How does work for locales / i18n? Just hard-coded English text for now?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I made this decision to minimise the number of dependencies. Now, not using locales is not a real case for us, because it's just plain object, but if any i18n will be added, then I think it's still good to not be dependent on specific library.

Copy link

@corquaid corquaid left a comment

Choose a reason for hiding this comment

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

LGTM!

@tomaszantas tomaszantas merged commit 4b18b14 into launchpad_such_wow Jul 8, 2022
@tomaszantas tomaszantas deleted the launchpad/feature/error-boundary branch July 8, 2022 13:04
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.

3 participants