Skip to content

Commit

Permalink
feat: enhanced error handling ui (#3021)
Browse files Browse the repository at this point in the history
* feat: better error handling

* better error handling

* try to fix netlify build

* remove new line
  • Loading branch information
sedghi authored Nov 17, 2022
1 parent 83ea228 commit eddd510
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 39 deletions.
14 changes: 1 addition & 13 deletions extensions/default/src/Panels/PanelMeasurementTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@ import { utils } from '@ohif/core';

const { downloadCSVReport } = utils;

// tools with measurements to display inside the panel
const MEASUREMENT_TOOLS = [
'EllipticalROI',
'RectangleROI',
'Length',
'Bidirectional',
];

export default function PanelMeasurementTable({
servicesManager,
commandsManager,
Expand Down Expand Up @@ -185,12 +177,8 @@ PanelMeasurementTable.propTypes = {

function _getMappedMeasurements(MeasurementService) {
const measurements = MeasurementService.getMeasurements();
// filter out measurements whose toolName is not in MEASUREMENT_TOOLS
const measurementTools = measurements.filter(measurement =>
MEASUREMENT_TOOLS.includes(measurement.toolName)
);

const mappedMeasurements = measurementTools.map((m, index) =>
const mappedMeasurements = measurements.map((m, index) =>
_mapMeasurementToDisplay(m, index, MeasurementService.VALUE_TYPES)
);

Expand Down
27 changes: 23 additions & 4 deletions extensions/default/src/ViewerLayout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,29 @@ function ViewerLayout({
};
}, []);

const getPanelData = id => {
const getComponent = id => {
const entry = extensionManager.getModuleEntry(id);
// TODO, not sure why sidepanel content has to be JSX, and not a children prop?
const content = entry.component;

if (!entry) {
throw new Error(
`${id} is not a valid entry for an extension module, please check your configuration or make sure the extension is registered.`
);
}

let content;
if (entry && entry.component) {
content = entry.component;
} else {
throw new Error(
`No component found from extension ${id}. Check the reference string to the extension in your Mode configuration`
);
}

return { entry, content };
};

const getPanelData = id => {
const { content, entry } = getComponent(id);

return {
iconName: entry.iconName,
Expand Down Expand Up @@ -156,7 +175,7 @@ function ViewerLayout({
}, [HangingProtocolService]);

const getViewportComponentData = viewportComponent => {
const entry = extensionManager.getModuleEntry(viewportComponent.namespace);
const { entry } = getComponent(viewportComponent.namespace);

return {
component: entry.component,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,7 @@ class MeasurementService {
measurement.source = source;
} catch (error) {
throw new Error(
`Failed to map '${sourceInfo}' measurement for annotationType ${annotationType}:`,
error.message
`Failed to map '${sourceInfo}' measurement for annotationType ${annotationType}: ${error.message}`
);
}

Expand Down
51 changes: 35 additions & 16 deletions platform/ui/src/components/ErrorBoundary/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,52 @@
import React, { useState } from 'react';
import { ErrorBoundary as ReactErrorBoundary } from 'react-error-boundary';
import { Icon, IconButton } from '@ohif/ui';
import PropTypes from 'prop-types';
import Modal from '../Modal';

const isProduction = process.env.NODE_ENV === 'production';

const DefaultFallback = ({ error, componentStack, context, resetErrorBoundary, fallbackRoute }) => {
const DefaultFallback = ({
error,
context,
resetErrorBoundary,
fallbackRoute,
}) => {
const [showDetails, setShowDetails] = useState(false);
const title = `Something went wrong${!isProduction && ` in ${context}`}.`;
const subtitle = `Sorry, something went wrong there. Try again.`;
return (
<div className="ErrorFallback bg-primary-dark w-full h-full" role="alert">
<p className="text-primary-light text-xl">{title}</p>
<p className="text-primary-light text-base">{subtitle}</p>
{!isProduction && (
<div className="rounded-md bg-secondary-dark p-5 mt-5">
<pre className="text-primary-light">Context: {context}</pre>
<pre className="text-primary-light">Error Message: {error.message}</pre>
<pre className="text-primary-light">Stack: {componentStack}</pre>
<div className="rounded-md bg-secondary-dark p-5 mt-5 font-mono space-y-2">
<p className="text-primary-light">Context: {context}</p>
<p className="text-primary-light">Error Message: {error.message}</p>

<IconButton
variant="contained"
color="inherit"
size="initial"
className="text-primary-active"
onClick={() => setShowDetails(!showDetails)}
>
<React.Fragment>
<div>{'Stack Trace'}</div>
<Icon width="15px" height="15px" name="chevron-down" />
</React.Fragment>
</IconButton>

{showDetails && (
<p className="px-4 text-primary-light">Stack: {error.stack}</p>
)}
</div>
)}
</div>
);
};

const noop = () => { };
const noop = () => {};

DefaultFallback.propTypes = {
error: PropTypes.object.isRequired,
Expand All @@ -32,7 +55,7 @@ DefaultFallback.propTypes = {
};

DefaultFallback.defaultProps = {
resetErrorBoundary: noop
resetErrorBoundary: noop,
};

const ErrorBoundary = ({
Expand All @@ -42,7 +65,7 @@ const ErrorBoundary = ({
fallbackComponent: FallbackComponent,
children,
fallbackRoute,
isPage
isPage,
}) => {
const [isOpen, setIsOpen] = useState(true);

Expand All @@ -53,7 +76,7 @@ const ErrorBoundary = ({

const onResetHandler = (...args) => onReset(...args);

const withModal = (Component) => props => (
const withModal = Component => props => (
<Modal
closeButton
shouldCloseOnEsc
Expand All @@ -75,11 +98,7 @@ const ErrorBoundary = ({
return (
<ReactErrorBoundary
fallbackRender={props => (
<Fallback
{...props}
context={context}
fallbackRoute={fallbackRoute}
/>
<Fallback {...props} context={context} fallbackRoute={fallbackRoute} />
)}
onReset={onResetHandler}
onError={onErrorHandler}
Expand All @@ -95,15 +114,15 @@ ErrorBoundary.propTypes = {
onError: PropTypes.func,
fallbackComponent: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
children: PropTypes.node.isRequired,
fallbackRoute: PropTypes.string
fallbackRoute: PropTypes.string,
};

ErrorBoundary.defaultProps = {
context: 'OHIF',
onReset: noop,
onError: noop,
fallbackComponent: DefaultFallback,
fallbackRoute: null
fallbackRoute: null,
};

export default ErrorBoundary;
10 changes: 9 additions & 1 deletion platform/ui/src/components/Icon/getIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,14 @@ const ICONS = {
'old-stop': oldStop,
};

function addIcon(iconName, iconSVG) {
if (ICONS[iconName]) {
console.warn(`Icon ${iconName} already exists.`);
}

ICONS[iconName] = iconSVG;
}

/**
* Return the matching SVG Icon as a React Component.
* Results in an inlined SVG Element. If there's no match,
Expand All @@ -214,4 +222,4 @@ export default function getIcon(key, props) {
return React.createElement(ICONS[key], props);
}

export { getIcon, ICONS };
export { getIcon, ICONS, addIcon };
3 changes: 2 additions & 1 deletion platform/ui/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Dialog from './Dialog';
import Dropdown from './Dropdown';
import EmptyStudies from './EmptyStudies';
import ErrorBoundary from './ErrorBoundary';
import Icon from './Icon';
import Icon, { addIcon } from './Icon';
import IconButton from './IconButton';
import Input from './Input';
import InputDateRange from './InputDateRange';
Expand Down Expand Up @@ -89,6 +89,7 @@ export {
ExpandableToolbarButton,
ListMenu,
Icon,
addIcon,
IconButton,
Input,
InputDateRange,
Expand Down
2 changes: 1 addition & 1 deletion platform/ui/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export {
} from './components';

/** These are mostly used in the docs */
export { getIcon, ICONS } from './components/Icon/getIcon';
export { getIcon, ICONS, addIcon } from './components/Icon/getIcon';
export { BackgroundColor } from './pages/Colors/BackgroundColor';
export { ModalComponent } from './contextProviders/ModalComponent';
export { Types };
2 changes: 1 addition & 1 deletion runtime.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.7
3.8

0 comments on commit eddd510

Please sign in to comment.