refactor: error boundary #8
Conversation
| const ErrorFallbackDefault = ({ intl }) => ( | ||
| <div> | ||
| <h2> | ||
| {intl.formatMessage(messages.unexpectedError)} | ||
| </h2> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
I changed the errorFallBackProp to ErrorFallbackComponent. It is now capitalized to signify that it is a React component. The reason why I wanted to make this clear is because the <ErrorBoundary /> component from frontend-platform wouldn't render anything unless it was provided as a React Component (ie. <ErrorFallback />). It was picking up on the prop but wasn't rendering it to the page unless it was provided this way.
| @@ -71,7 +71,7 @@ const PluginContainerIframe = ({ | |||
| scrolling={scrolling} | |||
| referrerPolicy="origin" // The sent referrer will be limited to the origin of the referring page: its scheme, host, and port. | |||
| className={classNames( | |||
| 'border border-0', | |||
| 'border border-0 w-100', | |||
There was a problem hiding this comment.
This sets the width: 100%; CSS rule that enables the iframe to stretch its width to available space. The width is currently being set (I believe) by the useElementSize() hook which sets the width to a fixed number determined by the elements that are wrapped by the plugin
| const defaultLoadingFallback = ( | ||
| <div className={classNames(pluginProps.className, 'd-flex justify-content-center align-items-center')}> | ||
| <Spinner animation="border" screenReaderText={intl.formatMessage(messages.loading)} /> | ||
| </div> | ||
| ); | ||
| if (fallback !== undefined) { | ||
| finalFallback = fallback; | ||
| } | ||
| const finalLoadingFallback = loadingFallback !== undefined ? loadingFallback : defaultLoadingFallback; |
There was a problem hiding this comment.
This is now loadingFallback to better show what it is used for. This fallback will be rendered the page while the PLUGIN_READY value is false. The PLUGIN_READY event is fired when the Plugin component mounts. This event will never fire if there is no Plugin being wrapped by the PluginSlot . And thus the spinner would remain on the screen with no indication to the viewer.
This is important to note when creating Plugin — perhaps a PluginWrapper component should be encouraged? Similar to what is used in the test files? This ensures that alerts, or any other fallback components for the Plugin, are wrapped and thus will fire the "ready" event which will clear the loading spinner and render whatever fallback is intended. Happy to elaborate more on this
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #8 +/- ##
==========================================
+ Coverage 75.00% 75.19% +0.19%
==========================================
Files 10 10
Lines 132 129 -3
Branches 19 19
==========================================
- Hits 99 97 -2
+ Misses 31 30 -1
Partials 2 2 ☔ View full report in Codecov by Sentry. |
This refactors the ErrorBoundary for the Plugin wrapper component to use the
ErrorBoundaryfromfrontend-platform. ThisErrorBoundarycomponent takes in an optional prop for a custom ErrorFallback component.There is also the
loadingFallbackoptional prop that is passed topluginPropsof thePluginSlotcomponent. Some wording around theloadingFallbackhas been changed to make it more clear what the intention of this particular fallback component is so as not to confuse it will the ErrorFallback component.