fix: Don't render objects/children of panels if there's a widget error (

deephaven#577)

- In that case the children are unlikely to render correctly anyway
- Have a `WidgetStatus` context that passes the widget status down
- `ReactPanel` checks the context and only displays the content if
there's no error with the widget
- Tested with DHE branch `bender_DH-16737-vplus` against dev-vplus

plugins/ui/src/js/src/layout/ReactPanel.test.tsx

@@ -1,25 +1,36 @@
 import React from 'react';
 import { render, within } from '@testing-library/react';
-import { LayoutUtils, useListener } from '@deephaven/dashboard';
+import {
+  LayoutUtils,
+  WidgetDescriptor,
+  useListener,
+} from '@deephaven/dashboard';
 import { TestUtils } from '@deephaven/utils';
 import ReactPanel from './ReactPanel';
 import {
   ReactPanelManager,
   ReactPanelManagerContext,
 } from './ReactPanelManager';
 import { ReactPanelProps } from './LayoutUtils';
-import PortalPanelManager from './PortalPanelManager';
-import PortalPanelManagerContext from './PortalPanelManagerContext';
+import PortalPanelManagerContext, {
+  PortalPanelMap,
+} from './PortalPanelManagerContext';
+import WidgetStatusContext, { WidgetStatus } from './WidgetStatusContext';
 
 const mockPanelId = 'test-panel-id';
+const defaultDescriptor = { name: 'test-name', type: 'test-type' };
+const defaultStatus: WidgetStatus = {
+  status: 'ready',
+  descriptor: defaultDescriptor,
+};
 
 beforeEach(() => {
   jest.clearAllMocks();
 });
 
 function makeReactPanelManager({
   children,
-  metadata = { name: 'test-name', type: 'test-type' },
+  metadata = defaultDescriptor,
   onClose = jest.fn(),
   onOpen = jest.fn(),
   getPanelId = jest.fn(() => mockPanelId),
@@ -41,23 +52,32 @@ function makeReactPanelManager({
 
 function makeTestComponent({
   children,
-  metadata = { name: 'test-name', type: 'test-type' },
+  metadata = defaultDescriptor,
   onClose = jest.fn(),
   onOpen = jest.fn(),
   getPanelId = jest.fn(() => mockPanelId),
+  portals = new Map(),
+  status = defaultStatus,
   title = 'test title',
-}: Partial<ReactPanelProps> & Partial<ReactPanelManager> = {}) {
+}: Partial<ReactPanelProps> &
+  Partial<ReactPanelManager> & {
+    metadata?: WidgetDescriptor;
+    portals?: PortalPanelMap;
+    status?: WidgetStatus;
+  } = {}) {
   return (
-    <PortalPanelManager>
-      {makeReactPanelManager({
-        children,
-        metadata,
-        onClose,
-        onOpen,
-        getPanelId,
-        title,
-      })}
-    </PortalPanelManager>
+    <WidgetStatusContext.Provider value={status}>
+      <PortalPanelManagerContext.Provider value={portals}>
+        {makeReactPanelManager({
+          children,
+          metadata,
+          onClose,
+          onOpen,
+          getPanelId,
+          title,
+        })}
+      </PortalPanelManagerContext.Provider>
+    </WidgetStatusContext.Provider>
   );
 }
 
@@ -181,14 +201,13 @@ it('does not call openComponent or setActiveContentItem if panel already exists
   const metadata = { type: 'bar' };
   const children = 'hello';
   const { rerender } = render(
-    <PortalPanelManagerContext.Provider value={portals}>
-      {makeReactPanelManager({
-        children,
-        onOpen,
-        onClose,
-        metadata,
-      })}
-    </PortalPanelManagerContext.Provider>
+    makeTestComponent({
+      children,
+      onOpen,
+      onClose,
+      metadata,
+      portals,
+    })
   );
   expect(LayoutUtils.openComponent).not.toHaveBeenCalled();
   expect(LayoutUtils.closeComponent).not.toHaveBeenCalled();
@@ -200,14 +219,13 @@ it('does not call openComponent or setActiveContentItem if panel already exists
 
   // Now check that it focuses it if it's called after the metadata changes
   rerender(
-    <PortalPanelManagerContext.Provider value={portals}>
-      {makeReactPanelManager({
-        children: 'world',
-        onOpen,
-        onClose,
-        metadata: { type: 'baz' },
-      })}
-    </PortalPanelManagerContext.Provider>
+    makeTestComponent({
+      children: 'world',
+      onOpen,
+      onClose,
+      metadata: { type: 'baz' },
+      portals,
+    })
   );
 
   expect(LayoutUtils.openComponent).not.toHaveBeenCalled();
@@ -274,22 +292,36 @@ it('catches an error thrown by children, renders error view', () => {
   const portals = new Map([[mockPanelId, portal]]);
 
   const { rerender } = render(
-    <PortalPanelManagerContext.Provider value={portals}>
-      {makeReactPanelManager({
-        children: <ErrorComponent />,
-      })}
-    </PortalPanelManagerContext.Provider>
+    makeTestComponent({
+      children: <ErrorComponent />,
+      portals,
+    })
   );
   const { getByText } = within(portal);
-  expect(getByText('Error: test error')).toBeDefined();
+  expect(getByText('test error')).toBeDefined();
 
   rerender(
-    <PortalPanelManagerContext.Provider value={portals}>
-      {makeReactPanelManager({
-        children: <div>Hello</div>,
-      })}
-    </PortalPanelManagerContext.Provider>
+    makeTestComponent({
+      children: <div>Hello</div>,
+      portals,
+    })
   );
 
   expect(getByText('Hello')).toBeDefined();
 });
+
+it('displays an error if the widget is in an error state', () => {
+  const error = new Error('test error');
+  const portal = document.createElement('div');
+  const portals = new Map([[mockPanelId, portal]]);
+  const status: WidgetStatus = {
+    status: 'error',
+    descriptor: defaultDescriptor,
+    error,
+  };
+
+  render(makeTestComponent({ portals, status }));
+
+  const { getByText } = within(portal);
+  expect(getByText('test error')).toBeDefined();
+});

plugins/ui/src/js/src/layout/ReactPanel.tsx

@@ -7,21 +7,17 @@ import {
   useLayoutManager,
   useListener,
 } from '@deephaven/dashboard';
-import {
-  View,
-  ViewProps,
-  Flex,
-  FlexProps,
-  ErrorBoundary,
-} from '@deephaven/components';
+import { View, ViewProps, Flex, FlexProps } from '@deephaven/components';
 import Log from '@deephaven/log';
 import PortalPanel from './PortalPanel';
 import { ReactPanelControl, useReactPanel } from './ReactPanelManager';
 import { ReactPanelProps } from './LayoutUtils';
 import { useParentItem } from './ParentItemContext';
 import { ReactPanelContext } from './ReactPanelContext';
 import { usePortalPanelManager } from './PortalPanelManagerContext';
-import ReactPanelContentOverlay from './ReactPanelContentOverlay';
+import ReactPanelErrorBoundary from './ReactPanelErrorBoundary';
+import useWidgetStatus from './useWidgetStatus';
+import WidgetErrorView from '../widget/WidgetErrorView';
 
 const log = Log.module('@deephaven/js-plugin-ui/ReactPanel');
 
@@ -172,6 +168,7 @@ function ReactPanel({
     },
     [parent, metadata, onOpen, panelId, title]
   );
+  const widgetStatus = useWidgetStatus();
 
   return portal
     ? ReactDOM.createPortal(
@@ -205,11 +202,19 @@ function ReactPanel({
               rowGap={rowGap}
               columnGap={columnGap}
             >
-              {/* Have an ErrorBoundary around the children to display an error in the panel if there's any errors thrown when rendering the children */}
-              <ErrorBoundary>{children}</ErrorBoundary>
+              <ReactPanelErrorBoundary>
+                {/**
+                 * Don't render the children if there's an error with the widget. If there's an error with the widget, we can assume the children won't render properly,
+                 * but we still want the panels to appear so things don't disappear/jump around.
+                 */}
+                {widgetStatus.status === 'error' ? (
+                  <WidgetErrorView error={widgetStatus.error} />
+                ) : (
+                  children
+                )}
+              </ReactPanelErrorBoundary>
             </Flex>
           </View>
-          <ReactPanelContentOverlay />
         </ReactPanelContext.Provider>,
         portal,
         contentKey

plugins/ui/src/js/src/layout/ReactPanelErrorBoundary.tsx

@@ -0,0 +1,59 @@
+import Log from '@deephaven/log';
+import React, { Component, ReactNode } from 'react';
+import WidgetErrorView from '../widget/WidgetErrorView';
+
+const log = Log.module('ReactPanelErrorBoundary');
+
+export interface ReactPanelErrorBoundaryProps {
+  /** Children to catch errors from. Error will reset when the children have been updated. */
+  children: ReactNode;
+}
+
+export interface ReactPanelErrorBoundaryState {
+  /** Currently displayed error. Reset when children are updated. */
+  error?: Error;
+}
+
+/**
+ * Error boundary for catching render errors in React. Displays an error message until the children have updated.
+ */
+export class ReactPanelErrorBoundary extends Component<
+  ReactPanelErrorBoundaryProps,
+  ReactPanelErrorBoundaryState
+> {
+  static getDerivedStateFromError(error: Error): ReactPanelErrorBoundaryState {
+    return { error };
+  }
+
+  constructor(props: ReactPanelErrorBoundaryProps) {
+    super(props);
+    this.state = { error: undefined };
+  }
+
+  componentDidUpdate(
+    prevProps: Readonly<ReactPanelErrorBoundaryProps>,
+    prevState: Readonly<ReactPanelErrorBoundaryState>
+  ): void {
+    const { children } = this.props;
+    if (prevProps.children !== children && prevState.error != null) {
+      log.debug(
+        'ReactPanelErrorBoundary clearing previous error',
+        prevState.error,
+        children
+      );
+      this.setState({ error: undefined });
+    }
+  }
+
+  componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void {
+    log.error('Error caught by ErrorBoundary', error, errorInfo);
+  }
+
+  render(): ReactNode {
+    const { children } = this.props;
+    const { error } = this.state;
+    return error != null ? <WidgetErrorView error={error} /> : children;
+  }
+}
+
+export default ReactPanelErrorBoundary;

plugins/ui/src/js/src/layout/WidgetStatusContext.tsx

@@ -0,0 +1,28 @@
+import { WidgetDescriptor } from '@deephaven/dashboard';
+import { createContext } from 'react';
+
+export type WidgetStatusLoading = {
+  status: 'loading';
+  descriptor: WidgetDescriptor;
+};
+
+export type WidgetStatusError = {
+  status: 'error';
+  descriptor: WidgetDescriptor;
+  error: NonNullable<unknown>;
+};
+
+export type WidgetStatusReady = {
+  status: 'ready';
+  descriptor: WidgetDescriptor;
+};
+
+export type WidgetStatus =
+  | WidgetStatusLoading
+  | WidgetStatusError
+  | WidgetStatusReady;
+
+/** Status of the widget within this context */
+export const WidgetStatusContext = createContext<WidgetStatus | null>(null);
+
+export default WidgetStatusContext;

plugins/ui/src/js/src/layout/useWidgetStatus.ts

@@ -0,0 +1,12 @@
+import { useContextOrThrow } from '@deephaven/react-hooks';
+import { WidgetStatus, WidgetStatusContext } from './WidgetStatusContext';
+
+/**
+ * Gets the widget status from the closest WidgetStatusContext.
+ * @returns Widget status or throws an error if WidgetStatusContext is not set
+ */
+export function useWidgetStatus(): WidgetStatus {
+  return useContextOrThrow(WidgetStatusContext);
+}
+
+export default useWidgetStatus;

plugins/ui/src/js/src/styles.scss

@@ -31,27 +31,6 @@
   }
 }
 
-.dh-react-panel-overlay {
-  background-color: bg-opacity(80);
-  backdrop-filter: blur(5px);
-  position: absolute;
-  top: 0;
-  left: 0;
-  right: 0;
-  bottom: 0;
-  padding: $spacer;
-  display: flex;
-  flex-direction: column;
-  justify-content: center;
-  align-items: center;
-  z-index: 1000;
-
-  .ui-widget-error-view {
-    width: 100%;
-    overflow: auto;
-  }
-}
-
 .ui-text-wrap-balance {
   text-wrap: balance;
 }