-
Notifications
You must be signed in to change notification settings - Fork 837
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
[CSS-in-JS] Convert EuiErrorBoundary
#6053
[CSS-in-JS] Convert EuiErrorBoundary
#6053
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_6053/ |
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! 🎉
I just have a few questions and suggestions. Tested in Chrome, Safari, Edge, and Firefox.
EuiErrorBoundaryProps, | ||
type EuiErrorBoundaryExtendedProps = EuiErrorBoundaryProps & WithEuiThemeProps; | ||
|
||
export class _EuiErrorBoundary extends Component< |
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 this component be converted to a function component?
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.
Unfortunately it's not yet possible: https://reactjs.org/docs/hooks-faq.html#do-hooks-cover-all-use-cases-for-classes
We need componentDidCatch
for this to function and there is no way to achieve that without a class component.
Preview documentation changes for this PR: https://eui.elastic.co/pr_6053/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_6053/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_6053/ |
@thompsongl it's looking good in Safari 15.4! 🎉 |
Summary
Converts
EuiErrorBoundary
to@emotion
styling.Note that if you run this locally, the error overlay on the Error Boundary docs page is expected.
Checklist