Skip to content

Commit

Permalink
feat: Add render blocking errors to Chart (#2255)
Browse files Browse the repository at this point in the history
Adds new events that enable render-blocking overlays to the chart.
The current error pop-up is not sufficient because it still allows the
chart to render. This is problematic in the case of webgl because
disabling webgl means we don't want the chart rendering at all in case
the computer can't handle it.
  • Loading branch information
jnumainville authored and mofojed committed Jan 8, 2025
1 parent 003583a commit f729841
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 22 deletions.
78 changes: 56 additions & 22 deletions packages/chart/src/Chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,14 @@ interface ChartState {
isDownsampleInProgress: boolean;
isDownsamplingDisabled: boolean;

/** Any other kind of error */
/** Any other kind of error that doesn't completely block the chart from rendering */
error: unknown;
shownError: string | null;
layout: Partial<Layout>;
revision: number;

/** A message that blocks the chart from rendering. It can be bypassed by the user to continue rendering. */
shownBlocker: string | null;
}

class Chart extends Component<ChartProps, ChartState> {
Expand Down Expand Up @@ -180,6 +183,7 @@ class Chart extends Component<ChartProps, ChartState> {
datarevision: 0,
},
revision: 0,
shownBlocker: null,
};
}

Expand Down Expand Up @@ -512,6 +516,15 @@ class Chart extends Component<ChartProps, ChartState> {
onError(new Error(error));
break;
}
case ChartModel.EVENT_BLOCKER: {
const blocker = `${detail}`;
this.setState({ shownBlocker: blocker });
break;
}
case ChartModel.EVENT_BLOCKER_CLEAR: {
this.setState({ shownBlocker: null });
break;
}
default:
log.debug('Unknown event type', type, event);
}
Expand Down Expand Up @@ -705,6 +718,7 @@ class Chart extends Component<ChartProps, ChartState> {
shownError,
layout,
revision,
shownBlocker,
} = this.state;
const config = this.getCachedConfig(
downsamplingError,
Expand All @@ -714,7 +728,46 @@ class Chart extends Component<ChartProps, ChartState> {
data ?? [],
error
);
const isPlotShown = data != null;
const { model } = this.props;
const isPlotShown = data != null && shownBlocker == null;

let errorOverlay: React.ReactNode = null;
if (shownBlocker != null) {
errorOverlay = (
<ChartErrorOverlay
errorMessage={`${shownBlocker}`}
onConfirm={() => {
model.fireBlockerClear();
}}
/>
);
} else if (shownError != null) {
errorOverlay = (
<ChartErrorOverlay
errorMessage={`${downsamplingError}`}
onDiscard={() => {
this.handleDownsampleErrorClose();
}}
onConfirm={() => {
this.handleDownsampleErrorClose();
this.handleDownsampleClick();
}}
/>
);
} else if (downsamplingError != null) {
errorOverlay = (
<ChartErrorOverlay
errorMessage={`${downsamplingError}`}
onDiscard={() => {
this.handleDownsampleErrorClose();
}}
onConfirm={() => {
this.handleDownsampleErrorClose();
this.handleDownsampleClick();
}}
/>
);
}

return (
<div className="h-100 w-100 chart-wrapper" ref={this.plotWrapperMerged}>
Expand All @@ -735,26 +788,7 @@ class Chart extends Component<ChartProps, ChartState> {
style={{ height: '100%', width: '100%' }}
/>
)}
{downsamplingError != null && shownError == null && (
<ChartErrorOverlay
errorMessage={`${downsamplingError}`}
onDiscard={() => {
this.handleDownsampleErrorClose();
}}
onConfirm={() => {
this.handleDownsampleErrorClose();
this.handleDownsampleClick();
}}
/>
)}
{shownError != null && (
<ChartErrorOverlay
errorMessage={`${shownError}`}
onDiscard={() => {
this.handleErrorClose();
}}
/>
)}
{errorOverlay}
</div>
);
}
Expand Down
12 changes: 12 additions & 0 deletions packages/chart/src/ChartModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class ChartModel {

static EVENT_ERROR = 'ChartModel.EVENT_ERROR';

static EVENT_BLOCKER = 'ChartModel.EVENT_BLOCKER';

static EVENT_BLOCKER_CLEAR = 'ChartModel.EVENT_BLOCKER_CLEAR';

constructor(dh: typeof DhType) {
this.dh = dh;
this.listeners = [];
Expand Down Expand Up @@ -184,6 +188,14 @@ class ChartModel {
fireError(detail: string[]): void {
this.fireEvent(new CustomEvent(ChartModel.EVENT_ERROR, { detail }));
}

fireBlocker(detail: string[]): void {
this.fireEvent(new CustomEvent(ChartModel.EVENT_BLOCKER, { detail }));
}

fireBlockerClear(): void {
this.fireEvent(new CustomEvent(ChartModel.EVENT_BLOCKER_CLEAR));
}
}

export default ChartModel;

0 comments on commit f729841

Please sign in to comment.