From e9696894a9160b5c31b149171a0e6a6f80d206b6 Mon Sep 17 00:00:00 2001 From: igoroctaviano Date: Fri, 26 Jun 2020 10:26:50 -0300 Subject: [PATCH 1/3] Add error boundary --- platform/ui/index.js | 1 + platform/ui/package.json | 1 + .../ErrorBoundary/ErrorBoundary.jsx | 51 ++++++++++++++++ .../ErrorBoundary/ErrorBoundary.mdx | 59 +++++++++++++++++++ .../ErrorBoundary/ErrorFallback.css | 21 +++++++ .../ui/src/components/ErrorBoundary/index.js | 2 + platform/ui/src/components/index.js | 2 + yarn.lock | 7 +++ 8 files changed, 144 insertions(+) create mode 100644 platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx create mode 100644 platform/ui/src/components/ErrorBoundary/ErrorBoundary.mdx create mode 100644 platform/ui/src/components/ErrorBoundary/ErrorFallback.css create mode 100644 platform/ui/src/components/ErrorBoundary/index.js diff --git a/platform/ui/index.js b/platform/ui/index.js index 2025cfb3b..66f9fad21 100644 --- a/platform/ui/index.js +++ b/platform/ui/index.js @@ -32,6 +32,7 @@ export { DateRange, Dialog, EmptyStudies, + ErrorBoundary, ExpandableToolbarButton, ListMenu, Icon, diff --git a/platform/ui/package.json b/platform/ui/package.json index 17619f2cf..601b7862d 100644 --- a/platform/ui/package.json +++ b/platform/ui/package.json @@ -42,6 +42,7 @@ "react-dnd": "^10.0.2", "react-dnd-html5-backend": "^10.0.2", "react-dnd-touch-backend": "^10.0.2", + "react-error-boundary": "2.2.x", "react-dom": "16.11.0", "react-modal": "^3.11.2", "react-powerplug": "1.0.0", diff --git a/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx new file mode 100644 index 000000000..b75b35710 --- /dev/null +++ b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx @@ -0,0 +1,51 @@ +import React from 'react'; +import { ErrorBoundary as ReactErrorBoundary } from 'react-error-boundary'; +import './ErrorFallback.css'; + +const isProduction = process.env.NODE_ENV === 'production'; + +const ErrorFallback = ({ error, componentStack, resetErrorBoundary }) => { + return ( +
+

Something went wrong.

+ {isProduction && ( + Sorry, something went wrong there. Try again. + )} + {!isProduction &&
{error.message}
} + {!isProduction &&
{componentStack}
} +
+ ); +}; + +ErrorFallback.propTypes = { + resetErrorBoundary: PropTypes +}; + +const ErrorBoundary = ({ + context = 'RCT', + onReset = () => { }, + onError = () => { }, + fallbackComponent, + children +}) => { + const onErrorHandler = (error, componentStack) => { + console.error(`${context} Error Boundary`, error, componentStack); + onError(error, componentStack); + }; + + const onResetHandler = () => { + onReset(); + }; + + return ( + + {children} + + ); +}; + +export default ErrorBoundary; diff --git a/platform/ui/src/components/ErrorBoundary/ErrorBoundary.mdx b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.mdx new file mode 100644 index 000000000..bb65df025 --- /dev/null +++ b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.mdx @@ -0,0 +1,59 @@ +--- +name: Error Boundary +menu: Data Display +route: components/errorBoundary +--- + +import { Playground, Props } from 'docz'; +import { ErrorBoundary } from '@ohif/ui'; + +# Error Boundary + +This component can be used to display a message or handle unhandled errors. + +## Import + +```javascript +import { ErrorBoundary } from '@ohif/ui'; +``` + +## Basic usage + + +
+ + {() => { + throw new Error('Error!'); + return

; + }} +
+
+
+ +## Custom error fallback + + + {() => { + const CustomFallback = ({ error, componentStack, resetErrorBoundary }) => { + return ( +
+

This is a custom fallback!

+
+ ); + }; + return ( +
+ + {() => { + throw new Error('Error!'); + return

; + }} +
+
+ ); + }} +
+ +## Properties + + diff --git a/platform/ui/src/components/ErrorBoundary/ErrorFallback.css b/platform/ui/src/components/ErrorBoundary/ErrorFallback.css new file mode 100644 index 000000000..34d62ccf4 --- /dev/null +++ b/platform/ui/src/components/ErrorBoundary/ErrorFallback.css @@ -0,0 +1,21 @@ +.ErrorFallback { + padding: 10px; + + &, + pre { + color: #7cc5e9; + } + + p { + font-weight: bold; + } + + span { + color: gray; + } + + pre { + background-color: black; + border: none; + } +} diff --git a/platform/ui/src/components/ErrorBoundary/index.js b/platform/ui/src/components/ErrorBoundary/index.js new file mode 100644 index 000000000..b54419282 --- /dev/null +++ b/platform/ui/src/components/ErrorBoundary/index.js @@ -0,0 +1,2 @@ +import ErrorBoundary from './ErrorBoundary'; +export default ErrorBoundary; diff --git a/platform/ui/src/components/index.js b/platform/ui/src/components/index.js index 04b5270cc..76d4f2729 100644 --- a/platform/ui/src/components/index.js +++ b/platform/ui/src/components/index.js @@ -3,6 +3,7 @@ import ButtonGroup from './ButtonGroup'; import DateRange from './DateRange'; import Dialog from './Dialog'; import EmptyStudies from './EmptyStudies'; +import ErrorBoundary from './ErrorBoundary'; import Icon from './Icon'; import IconButton from './IconButton'; import Input from './Input'; @@ -54,6 +55,7 @@ export { DateRange, Dialog, EmptyStudies, + ErrorBoundary, ExpandableToolbarButton, ListMenu, Icon, diff --git a/yarn.lock b/yarn.lock index a045e8f39..6b7ba067c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18075,6 +18075,13 @@ react-dropzone@^10.1.7: file-selector "^0.1.12" prop-types "^15.7.2" +react-error-boundary@2.2.x: + version "2.2.3" + resolved "https://registry.yarnpkg.com/react-error-boundary/-/react-error-boundary-2.2.3.tgz#34c8238012d3b4148cec47a1b3cec669d5206578" + integrity sha512-Jiaiu6CJ4ho3sMCVI7gg+O/JB5vlFFZGwlnpFBTCOSyheYRTzz+FhBMo7tfnCTB/ZR0LaMzAPGbZGrEzAOd0eg== + dependencies: + "@babel/runtime" "^7.9.6" + react-error-overlay@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/react-error-overlay/-/react-error-overlay-3.0.0.tgz#c2bc8f4d91f1375b3dad6d75265d51cd5eeaf655" From 34afb12c6f577f36ae83b2354184d9997ec12d32 Mon Sep 17 00:00:00 2001 From: igoroctaviano Date: Fri, 26 Jun 2020 12:38:10 -0300 Subject: [PATCH 2/3] Add proptypes --- platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx index b75b35710..905570910 100644 --- a/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx +++ b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx @@ -1,5 +1,6 @@ import React from 'react'; import { ErrorBoundary as ReactErrorBoundary } from 'react-error-boundary'; +import PropTypes from 'prop-types'; import './ErrorFallback.css'; const isProduction = process.env.NODE_ENV === 'production'; From b34a9880f7fba9435f9afb5cd920ab54d3f83d49 Mon Sep 17 00:00:00 2001 From: igoroctaviano Date: Fri, 26 Jun 2020 14:08:29 -0300 Subject: [PATCH 3/3] Use modal as default fallback --- extensions/default/src/ViewerLayout/index.jsx | 95 ++++++++-------- .../ErrorBoundary/ErrorBoundary.jsx | 101 ++++++++++++++---- .../ErrorBoundary/ErrorFallback.css | 21 ---- .../ViewportActionBar/ViewportActionBar.jsx | 4 +- platform/viewer/src/routes/index.js | 6 +- 5 files changed, 137 insertions(+), 90 deletions(-) delete mode 100644 platform/ui/src/components/ErrorBoundary/ErrorFallback.css diff --git a/extensions/default/src/ViewerLayout/index.jsx b/extensions/default/src/ViewerLayout/index.jsx index cf2c11826..cbf414efc 100644 --- a/extensions/default/src/ViewerLayout/index.jsx +++ b/extensions/default/src/ViewerLayout/index.jsx @@ -1,7 +1,6 @@ import React, { useEffect, useState } from 'react'; -import classnames from 'classnames'; import PropTypes from 'prop-types'; -import { SidePanel } from '@ohif/ui'; +import { SidePanel, ErrorBoundary } from '@ohif/ui'; import Header from './Header.jsx'; import NestedMenu from './ToolbarButtonNestedMenu.jsx'; @@ -93,27 +92,28 @@ function ViewerLayout({ return (
-
- {toolbars.primary.map(toolDef => { - const isNested = Array.isArray(toolDef); - - if (!isNested) { - const { id, Component, componentProps } = toolDef; - return ; - } else { - return ( - -
- {toolDef.map(x => { - const { id, Component, componentProps } = x; - return ; - })} -
-
- ); - } - })} -
+ +
+ {toolbars.primary.map(toolDef => { + const isNested = Array.isArray(toolDef); + if (!isNested) { + const { id, Component, componentProps } = toolDef; + return ; + } else { + return ( + +
+ {toolDef.map(x => { + const { id, Component, componentProps } = x; + return ; + })} +
+
+ ); + } + })} +
+
{/* LEFT SIDEPANELS */} {leftPanelComponents.length && ( - + + + )} {/* TOOLBAR + GRID */}
-
- {toolbars.secondary.map(toolDef => { - const { id, Component, componentProps } = toolDef; - - return ; - })} -
+ +
+ {toolbars.secondary.map(toolDef => { + const { id, Component, componentProps } = toolDef; + return ; + })} +
+
- + + +
{rightPanelComponents.length && ( - + + + )}
diff --git a/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx index 905570910..f6915592c 100644 --- a/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx +++ b/platform/ui/src/components/ErrorBoundary/ErrorBoundary.jsx @@ -1,46 +1,86 @@ -import React from 'react'; +import React, { useState } from 'react'; import { ErrorBoundary as ReactErrorBoundary } from 'react-error-boundary'; import PropTypes from 'prop-types'; -import './ErrorFallback.css'; +import Modal from '../Modal'; const isProduction = process.env.NODE_ENV === 'production'; -const ErrorFallback = ({ error, componentStack, resetErrorBoundary }) => { +const DefaultFallback = ({ error, componentStack, context, resetErrorBoundary, fallbackRoute }) => { + const title = `Something went wrong${!isProduction && ` in ${context}`}.`; + const subtitle = `Sorry, something went wrong there. Try again.`; return ( -
-

Something went wrong.

- {isProduction && ( - Sorry, something went wrong there. Try again. +
+

{title}

+

{subtitle}

+ {!isProduction && ( +
+
Context: {context}
+
Error Message: {error.message}
+
Stack: {componentStack}
+
)} - {!isProduction &&
{error.message}
} - {!isProduction &&
{componentStack}
}
); }; -ErrorFallback.propTypes = { - resetErrorBoundary: PropTypes +const noop = () => { }; + +DefaultFallback.propTypes = { + error: PropTypes.object.isRequired, + resetErrorBoundary: PropTypes.func, + componentStack: PropTypes.string, +}; + +DefaultFallback.defaultProps = { + resetErrorBoundary: noop }; const ErrorBoundary = ({ - context = 'RCT', - onReset = () => { }, - onError = () => { }, - fallbackComponent, - children + context, + onReset, + onError, + fallbackComponent: FallbackComponent, + children, + fallbackRoute, + isPage }) => { + const [isOpen, setIsOpen] = useState(true); + const onErrorHandler = (error, componentStack) => { - console.error(`${context} Error Boundary`, error, componentStack); - onError(error, componentStack); + console.error(`${context} Error Boundary`, error, componentStack, context); + onError(error, componentStack, context); }; - const onResetHandler = () => { - onReset(); - }; + const onResetHandler = (...args) => onReset(...args); + + const withModal = (Component) => props => ( + { + setIsOpen(false); + if (fallbackRoute) { + window.location = fallbackRoute; + } + }} + > + + + ); + + const Fallback = isPage ? FallbackComponent : withModal(FallbackComponent); return ( ( + + )} onReset={onResetHandler} onError={onErrorHandler} > @@ -49,4 +89,21 @@ const ErrorBoundary = ({ ); }; +ErrorBoundary.propTypes = { + context: PropTypes.string, + onReset: PropTypes.func, + onError: PropTypes.func, + fallbackComponent: PropTypes.oneOfType([PropTypes.node, PropTypes.func]), + children: PropTypes.node.isRequired, + fallbackRoute: PropTypes.string +}; + +ErrorBoundary.defaultProps = { + context: 'OHIF', + onReset: noop, + onError: noop, + fallbackComponent: DefaultFallback, + fallbackRoute: null +}; + export default ErrorBoundary; diff --git a/platform/ui/src/components/ErrorBoundary/ErrorFallback.css b/platform/ui/src/components/ErrorBoundary/ErrorFallback.css deleted file mode 100644 index 34d62ccf4..000000000 --- a/platform/ui/src/components/ErrorBoundary/ErrorFallback.css +++ /dev/null @@ -1,21 +0,0 @@ -.ErrorFallback { - padding: 10px; - - &, - pre { - color: #7cc5e9; - } - - p { - font-weight: bold; - } - - span { - color: gray; - } - - pre { - background-color: black; - border: none; - } -} diff --git a/platform/ui/src/components/ViewportActionBar/ViewportActionBar.jsx b/platform/ui/src/components/ViewportActionBar/ViewportActionBar.jsx index 9129d871c..12f052e76 100644 --- a/platform/ui/src/components/ViewportActionBar/ViewportActionBar.jsx +++ b/platform/ui/src/components/ViewportActionBar/ViewportActionBar.jsx @@ -136,7 +136,7 @@ const ViewportActionBar = ({
)} -
+
)} > -
+
{allRoutes.map((route, i) => { + console.log(route); return ( ( // eslint-disable-next-line react/jsx-props-no-spreading - + + + )} /> );