Skip to content

Commit

Permalink
fix: Unit tests, and pass WidgetStatus instead of just an error
Browse files Browse the repository at this point in the history
- More flexible, allowing a loading or ready status to be passed down as well
  - Could be used to show a proper loading spinner
- Fix up unit tests that were failing
  • Loading branch information
mofojed committed Jun 26, 2024
1 parent 00b702b commit 6d511e7
Show file tree
Hide file tree
Showing 11 changed files with 416 additions and 268 deletions.
434 changes: 254 additions & 180 deletions package-lock.json

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions plugins/ui/src/js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,18 @@
"@adobe/react-spectrum": "^3.34.1",
"@deephaven/chart": "^0.78.0",
"@deephaven/components": "^0.78.0",
"@deephaven/dashboard": "^0.78.0",
"@deephaven/dashboard-core-plugins": "^0.78.0",
"@deephaven/dashboard": "^0.78.2",
"@deephaven/dashboard-core-plugins": "^0.78.2",
"@deephaven/grid": "^0.78.0",
"@deephaven/icons": "^0.78.0",
"@deephaven/iris-grid": "^0.78.0",
"@deephaven/jsapi-bootstrap": "^0.78.0",
"@deephaven/jsapi-components": "^0.78.0",
"@deephaven/iris-grid": "^0.78.2",
"@deephaven/jsapi-bootstrap": "^0.78.2",
"@deephaven/jsapi-components": "^0.78.2",
"@deephaven/jsapi-types": "^1.0.0-dev0.34.2",
"@deephaven/log": "^0.78.0",
"@deephaven/plugin": "^0.78.0",
"@deephaven/plugin": "^0.78.2",
"@deephaven/react-hooks": "^0.78.0",
"@deephaven/redux": "^0.78.0",
"@deephaven/redux": "^0.78.2",
"@deephaven/utils": "^0.78.0",
"@fortawesome/react-fontawesome": "^0.2.0",
"@react-types/shared": "^3.22.0",
Expand Down
118 changes: 75 additions & 43 deletions plugins/ui/src/js/src/layout/ReactPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -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),
Expand All @@ -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>
);
}

Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
});
24 changes: 13 additions & 11 deletions plugins/ui/src/js/src/layout/ReactPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { useParentItem } from './ParentItemContext';
import { ReactPanelContext } from './ReactPanelContext';
import { usePortalPanelManager } from './PortalPanelManagerContext';
import ReactPanelErrorBoundary from './ReactPanelErrorBoundary';
import useWidgetError from './useWidgetError';
import useWidgetStatus from './useWidgetStatus';
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 widgetError = useWidgetError();
const widgetStatus = useWidgetStatus();

return portal
? ReactDOM.createPortal(
Expand Down Expand Up @@ -202,15 +202,17 @@ function ReactPanel({
rowGap={rowGap}
columnGap={columnGap}
>
{/**
* 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.
*/}
{widgetError != null ? (
<WidgetErrorView error={widgetError} />
) : (
<ReactPanelErrorBoundary>{children}</ReactPanelErrorBoundary>
)}
<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>
</ReactPanelContext.Provider>,
Expand Down
6 changes: 0 additions & 6 deletions plugins/ui/src/js/src/layout/WidgetErrorContext.tsx

This file was deleted.

28 changes: 28 additions & 0 deletions plugins/ui/src/js/src/layout/WidgetStatusContext.tsx
Original file line number Diff line number Diff line change
@@ -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;

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

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

This file was deleted.

12 changes: 12 additions & 0 deletions plugins/ui/src/js/src/layout/useWidgetStatus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { useContextOrThrow } from '@deephaven/react-hooks';
import { WidgetStatus, WidgetStatusContext } from './WidgetStatusContext';

/**
* Gets the overlay content from the nearest panel context.
* @returns The overlay content or null if not in a panel
*/
export function useWidgetStatus(): WidgetStatus {
return useContextOrThrow(WidgetStatusContext);
}

export default useWidgetStatus;
2 changes: 1 addition & 1 deletion plugins/ui/src/js/src/widget/WidgetErrorView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
} from './WidgetUtils';

/** Component that display an error message. Will automatically show a button for more info and an action button if the error has an Action defined */
function WidgetErrorView({
export function WidgetErrorView({
error,
}: {
error: NonNullable<unknown>;
Expand Down
12 changes: 9 additions & 3 deletions plugins/ui/src/js/src/widget/WidgetHandler.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from 'react';
import { act, render } from '@testing-library/react';
import { useWidget } from '@deephaven/jsapi-bootstrap';
import { dh } from '@deephaven/jsapi-types';
import { TestUtils } from '@deephaven/utils';
import WidgetHandler, { WidgetHandlerProps } from './WidgetHandler';
import { DocumentHandlerProps } from './DocumentHandler';
import {
Expand All @@ -11,10 +13,14 @@ import {
} from './WidgetTestUtils';

const mockApi = { Widget: { EVENT_MESSAGE: 'message' } };
let mockWidgetWrapper: ReturnType<typeof useWidget> = {
widget: null,
const defaultWidgetWrapper: ReturnType<typeof useWidget> = {
widget: TestUtils.createMockProxy<dh.Widget>({
getDataAsString: jest.fn(() => ''),
exportedObjects: [],
}),
error: null,
};
let mockWidgetWrapper: ReturnType<typeof useWidget> = defaultWidgetWrapper;
jest.mock('@deephaven/jsapi-bootstrap', () => ({
useApi: jest.fn(() => mockApi),
useWidget: jest.fn(() => mockWidgetWrapper),
Expand Down Expand Up @@ -43,7 +49,7 @@ function makeWidgetHandler({
}

beforeEach(() => {
mockWidgetWrapper = { widget: null, error: null };
mockWidgetWrapper = defaultWidgetWrapper;
mockDocumentHandler.mockClear();
});

Expand Down
Loading

0 comments on commit 6d511e7

Please sign in to comment.