Skip to content

Commit

Permalink
fix: Don't render objects/children of panels if there's a widget error
Browse files Browse the repository at this point in the history
- In that case the children are unlikely to render correctly anyway
- Have a WidgetError context that passes the widget status down
- ReactPanel checks the context and only displays the content if there's no error with the widget
  • Loading branch information
mofojed committed Jun 25, 2024
1 parent b2c44da commit 00b702b
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 85 deletions.
14 changes: 8 additions & 6 deletions plugins/ui/src/js/src/layout/ReactPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ 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 usePanelContentOverlay from './usePanelContentOverlay';
import useWidgetError from './useWidgetError';
import WidgetErrorView from '../widget/WidgetErrorView';

const log = Log.module('@deephaven/js-plugin-ui/ReactPanel');

Expand Down Expand Up @@ -168,7 +168,7 @@ function ReactPanel({
},
[parent, metadata, onOpen, panelId, title]
);
const overlayContent = usePanelContentOverlay();
const widgetError = useWidgetError();

return portal
? ReactDOM.createPortal(
Expand Down Expand Up @@ -203,14 +203,16 @@ function ReactPanel({
columnGap={columnGap}
>
{/**
* Don't render the children if there's an error overlay. If there's an error with the widget, we can assume the children won't render properly.
* 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.
*/}
{overlayContent == null && (
{widgetError != null ? (
<WidgetErrorView error={widgetError} />
) : (
<ReactPanelErrorBoundary>{children}</ReactPanelErrorBoundary>
)}
</Flex>
</View>
<ReactPanelContentOverlay />
</ReactPanelContext.Provider>,
portal,
contentKey
Expand Down
12 changes: 0 additions & 12 deletions plugins/ui/src/js/src/layout/ReactPanelContentOverlay.tsx

This file was deleted.

This file was deleted.

6 changes: 6 additions & 0 deletions plugins/ui/src/js/src/layout/WidgetErrorContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { createContext } from 'react';

/** Error status of the widget within this context */
export const WidgetErrorContext = createContext<unknown | null>(null);

export default WidgetErrorContext;
12 changes: 0 additions & 12 deletions plugins/ui/src/js/src/layout/usePanelContentOverlay.ts

This file was deleted.

12 changes: 12 additions & 0 deletions plugins/ui/src/js/src/layout/useWidgetError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { useContext } from 'react';
import { WidgetErrorContext } from './WidgetErrorContext';

/**
* Gets the overlay content from the nearest panel context.
* @returns The overlay content or null if not in a panel
*/
export function useWidgetError(): React.ReactNode | null {
return useContext(WidgetErrorContext);
}

export default useWidgetError;
21 changes: 0 additions & 21 deletions plugins/ui/src/js/src/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
32 changes: 10 additions & 22 deletions plugins/ui/src/js/src/widget/WidgetHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {
import DocumentHandler from './DocumentHandler';
import { getComponentForElement } from './WidgetUtils';
import WidgetErrorView from './WidgetErrorView';
import ReactPanelContentOverlayContext from '../layout/ReactPanelContentOverlayContext';
import WidgetErrorContext from '../layout/WidgetErrorContext';

const log = Log.module('@deephaven/js-plugin-ui/WidgetHandler');

Expand Down Expand Up @@ -311,33 +311,21 @@ function WidgetHandler({
[jsonClient, initialData, sendSetState, updateExportedObjects, widget]
);

const errorView = useMemo(() => {
if (error != null) {
return <WidgetErrorView error={error} />;
}
return null;
}, [error]);

const contentOverlay = useMemo(() => {
// We only show it as an overlay if there's already a document to show
// If there isn't, then we'll just render this as the document so it forces a panel to open
if (errorView != null && document != null) {
return errorView;
}
return null;
}, [document, errorView]);

const renderedDocument = useMemo(() => {
if (document != null) {
return document;
}
return errorView;
}, [document, errorView]);
if (error != null) {
// If there's an error and the document hasn't rendered yet, explicitly show an error view
return <WidgetErrorView error={error} />;
}
return null;
}, [document, error]);

return useMemo(
() =>
renderedDocument != null ? (
<ReactPanelContentOverlayContext.Provider value={contentOverlay}>
<WidgetErrorContext.Provider value={error}>
<DocumentHandler
widget={widgetDescriptor}
initialData={initialData}
Expand All @@ -346,10 +334,10 @@ function WidgetHandler({
>
{renderedDocument}
</DocumentHandler>
</ReactPanelContentOverlayContext.Provider>
</WidgetErrorContext.Provider>
) : null,
[
contentOverlay,
error,
widgetDescriptor,
renderedDocument,
initialData,
Expand Down
7 changes: 2 additions & 5 deletions tests/ui.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ test('boom component shows an error in a panel', async ({ page }) => {
await expect(
page.locator('.dh-react-panel').getByText('BOOM!')
).toBeVisible();
await expect(page.locator('.dh-react-panel-overlay')).not.toBeVisible();
});

test('boom counter component shows error overlay after clicking the button twice', async ({
Expand All @@ -36,10 +35,8 @@ test('boom counter component shows error overlay after clicking the button twice
await expect(btn).toBeVisible();
btn.click();

const overlayLocator = page.locator('.dh-react-panel-overlay');

await expect(
overlayLocator.getByText('ValueError', { exact: true })
panelLocator.getByText('ValueError', { exact: true })
).toBeVisible();
await expect(overlayLocator.getByText('BOOM! Value too big.')).toBeVisible();
await expect(panelLocator.getByText('BOOM! Value too big.')).toBeVisible();
});

0 comments on commit 00b702b

Please sign in to comment.