Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ChartBuilderPlugin fixes for charts built from PPQs in Enterprise #2167

Merged
merged 9 commits into from
Sep 11, 2024
8 changes: 2 additions & 6 deletions packages/app-utils/src/components/AppDashboards.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import {
DehydratedDashboardPanelProps,
LazyDashboard,
} from '@deephaven/dashboard';
import {
sanitizeVariableDescriptor,
useObjectFetcher,
} from '@deephaven/jsapi-bootstrap';
import { useObjectFetcher } from '@deephaven/jsapi-bootstrap';
import LayoutManager, {
ItemConfig,
Settings as LayoutSettings,
Expand Down Expand Up @@ -44,9 +41,8 @@ export function AppDashboards({
const { metadata } = hydrateProps;
try {
if (metadata != null) {
const widget = sanitizeVariableDescriptor(metadata);
return {
fetch: async () => fetchObject(widget),
fetch: async () => fetchObject(metadata),
...hydrateProps,
localDashboardId: id,
};
Expand Down
16 changes: 4 additions & 12 deletions packages/dashboard-core-plugins/src/ChartBuilderPlugin.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
import { useCallback } from 'react';
import {
ChartModel,
ChartModelFactory,
ChartModelSettings,
ChartUtils,
} from '@deephaven/chart';
import { ChartModelSettings, ChartUtils } from '@deephaven/chart';
import {
assertIsDashboardPluginProps,
DashboardPluginComponentProps,
LayoutUtils,
useListener,
} from '@deephaven/dashboard';
import { useApi } from '@deephaven/jsapi-bootstrap';
import type { dh } from '@deephaven/jsapi-types';
import { nanoid } from 'nanoid';
import { IrisGridEvent } from './events';
Expand All @@ -28,7 +22,6 @@ export function ChartBuilderPlugin(
): JSX.Element | null {
assertIsDashboardPluginProps(props);
const { id, layout } = props;
const dh = useApi();

const handleCreateChart = useCallback(
({
Expand All @@ -45,8 +38,7 @@ export function ChartBuilderPlugin(
table: dh.Table;
}) => {
const { settings } = metadata;
const makeModel = (): Promise<ChartModel> =>
ChartModelFactory.makeModelFromSettings(dh, settings, table);
const fetchTable = async () => table;
const title = ChartUtils.titleFromSettings(settings);

const config = {
Expand All @@ -56,7 +48,7 @@ export function ChartBuilderPlugin(
localDashboardId: id,
id: panelId,
metadata,
makeModel,
fetch: fetchTable,
},
title,
id: panelId,
Expand All @@ -65,7 +57,7 @@ export function ChartBuilderPlugin(
const { root } = layout;
LayoutUtils.openComponent({ root, config });
},
[dh, id, layout]
[id, layout]
);

useListener(layout.eventHub, IrisGridEvent.CREATE_CHART, handleCreateChart);
Expand Down
133 changes: 46 additions & 87 deletions packages/dashboard-core-plugins/src/ChartPanelPlugin.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import { forwardRef, useMemo } from 'react';
import {
ObjectFetcher,
useApi,
useObjectFetcher,
} from '@deephaven/jsapi-bootstrap';
import { forwardRef, useCallback } from 'react';
import { useDeferredApi } from '@deephaven/jsapi-bootstrap';
import { ChartModel, ChartModelFactory } from '@deephaven/chart';
import type { dh as DhType } from '@deephaven/jsapi-types';
import { IrisGridUtils } from '@deephaven/iris-grid';
import { getTimeZone, store } from '@deephaven/redux';
import { WidgetPanelProps } from '@deephaven/plugin';
import { assertNotNull } from '@deephaven/utils';
import {
ChartPanelMetadata,
GLChartPanelState,
isChartPanelDehydratedProps,
isChartPanelFigureMetadata,
isChartPanelTableMetadata,
} from './panels';
import ConnectedChartPanel, {
Expand All @@ -23,28 +19,18 @@ import ConnectedChartPanel, {

async function createChartModel(
dh: typeof DhType,
fetchObject: ObjectFetcher,
metadata: ChartPanelMetadata,
fetchFigure: () => Promise<DhType.plot.Figure>,
panelFetch: () => Promise<DhType.plot.Figure | DhType.Table>,
panelState?: GLChartPanelState
): Promise<ChartModel> {
let settings;
let tableName;
let figureName;
let tableSettings;
let settings = {};
let tableName = '';
let tableSettings = {};

if (isChartPanelTableMetadata(metadata)) {
settings = metadata.settings;
tableName = metadata.table;
figureName = undefined;
tableSettings = metadata.tableSettings;
} else {
settings = {};
tableName = '';
figureName = isChartPanelFigureMetadata(metadata)
? metadata.figure
: metadata.name;
tableSettings = {};
}
if (panelState != null) {
if (panelState.tableSettings != null) {
Expand All @@ -53,9 +39,6 @@ async function createChartModel(
if (panelState.table != null) {
tableName = panelState.table;
}
if (panelState.figure != null) {
figureName = panelState.figure;
}
if (panelState.settings != null) {
settings = {
...settings,
Expand All @@ -64,83 +47,59 @@ async function createChartModel(
}
}

if (figureName == null && tableName == null) {
const figure = await fetchFigure();

return ChartModelFactory.makeModel(dh, settings, figure);
}

if (figureName != null) {
let figure: DhType.plot.Figure;

if (metadata.type === dh.VariableType.FIGURE) {
const descriptor = {
...metadata,
name: figureName,
type: dh.VariableType.FIGURE,
};
figure = await fetchObject<DhType.plot.Figure>(descriptor);
} else {
figure = await fetchFigure();
}
if (tableName != null && tableName !== '') {
const table = (await panelFetch()) as DhType.Table;
new IrisGridUtils(dh).applyTableSettings(
table,
tableSettings,
getTimeZone(store.getState())
);

return ChartModelFactory.makeModel(dh, settings, figure);
return ChartModelFactory.makeModelFromSettings(dh, settings, table);
}

const descriptor = {
...metadata,
name: tableName,
type: dh.VariableType.TABLE,
};
const table = await fetchObject<DhType.Table>(descriptor);
new IrisGridUtils(dh).applyTableSettings(
table,
tableSettings,
getTimeZone(store.getState())
);
const figure = (await panelFetch()) as DhType.plot.Figure;

return ChartModelFactory.makeModelFromSettings(
dh,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
settings as any,
table
);
return ChartModelFactory.makeModel(dh, settings, figure);
}

export const ChartPanelPlugin = forwardRef(
(props: WidgetPanelProps<DhType.plot.Figure>, ref: React.Ref<ChartPanel>) => {
const dh = useApi();
const fetchObject = useObjectFetcher();

const panelState = isChartPanelDehydratedProps(props)
? (props as unknown as ChartPanelProps).panelState
: undefined;

const { fetch: panelFetch, metadata, localDashboardId } = props;

const hydratedProps = useMemo(
() => ({
metadata: metadata as ChartPanelMetadata,
localDashboardId,
makeModel: () => {
if (metadata == null) {
throw new Error('Metadata is required for chart panel');
}

return createChartModel(
dh,
fetchObject,
metadata as ChartPanelMetadata,
panelFetch,
panelState
);
},
}),
[metadata, localDashboardId, dh, fetchObject, panelFetch, panelState]
assertNotNull(metadata);
const [dh, error] = useDeferredApi(metadata);

const makeModel = useCallback(async () => {
if (error != null) {
throw error;
}
if (dh == null) {
mofojed marked this conversation as resolved.
Show resolved Hide resolved
return new Promise<ChartModel>(() => {
// We don't have the API yet, just return an unresolved promise so it shows as loading
});
}
return createChartModel(
dh,
metadata as ChartPanelMetadata,
panelFetch,
panelState
);
}, [dh, error, metadata, panelFetch, panelState]);

return (
<ConnectedChartPanel
ref={ref}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
metadata={metadata}
localDashboardId={localDashboardId}
makeModel={makeModel}
/>
);

// eslint-disable-next-line react/jsx-props-no-spreading
return <ConnectedChartPanel ref={ref} {...props} {...hydratedProps} />;
}
);

Expand Down
20 changes: 9 additions & 11 deletions packages/dashboard-core-plugins/src/panels/ChartPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import debounce from 'lodash.debounce';
import {
Chart,
ChartModel,
ChartModelFactory,
ChartModelSettings,
ChartUtils,
FilterMap,
isFigureChartModel,
} from '@deephaven/chart';
Expand Down Expand Up @@ -286,14 +286,14 @@ export class ChartPanel extends Component<ChartPanelProps, ChartPanelState> {
const { columnMap, model, filterMap, filterValueMap, isLinked, settings } =
this.state;

if (!model) {
return;
}

if (makeModel !== prevProps.makeModel) {
this.initModel();
}

if (model == null) {
return;
}

if (columnMap !== prevState.columnMap) {
this.pruneFilterMaps();
}
Expand Down Expand Up @@ -352,6 +352,8 @@ export class ChartPanel extends Component<ChartPanelProps, ChartPanelState> {

const { makeModel } = this.props;

this.pending.cancel();

this.pending
.add(makeModel(), resolved => {
resolved.close();
Expand Down Expand Up @@ -651,12 +653,8 @@ export class ChartPanel extends Component<ChartPanelProps, ChartPanelState> {
const { settings } = metadata;
this.pending
.add(
dh.plot.Figure.create(
new ChartUtils(dh).makeFigureSettings(
settings,
source
) as unknown as dh.plot.FigureDescriptor
)
ChartModelFactory.makeFigureFromSettings(dh, settings, source),
resolved => resolved.close()
)
.then(figure => {
if (isFigureChartModel(model)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/dashboard-core-plugins/src/panels/WidgetPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React, { PureComponent, ReactElement, ReactNode } from 'react';
import classNames from 'classnames';
import memoize from 'memoize-one';
import { ContextActions, createXComponent } from '@deephaven/components';
import { PanelComponent } from '@deephaven/dashboard';
import type { Container, EventEmitter } from '@deephaven/golden-layout';
import { ContextActions, createXComponent } from '@deephaven/components';
import { copyToClipboard } from '@deephaven/utils';
import Panel from './Panel';
import WidgetPanelTooltip from './WidgetPanelTooltip';
Expand Down
11 changes: 10 additions & 1 deletion packages/jsapi-bootstrap/src/useDeferredApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ describe('useDeferredApi', () => {
expect(result.current).toEqual([dh1, null]);

const { result: result2 } = renderHook(() =>
useDeferredApi({ type: 'foo', foo: 'bar' })
useDeferredApi({
type: 'foo',
foo: 'bar',
} as DhType.ide.VariableDescriptor)
);
expect(result2.current).toEqual([dh1, null]);
});
Expand Down Expand Up @@ -68,4 +71,10 @@ describe('useDeferredApi', () => {
const { result } = renderHook(() => useDeferredApi(objectMetadata));
expect(result.current).toEqual([null, expect.any(Error)]);
});

it('returns an error if the metadata is null', async () => {
asMock(useContext).mockReturnValue(dh1);
const { result } = renderHook(() => useDeferredApi(null));
expect(result.current).toEqual([null, expect.any(Error)]);
});
});
11 changes: 8 additions & 3 deletions packages/jsapi-bootstrap/src/useDeferredApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ export const DeferredApiContext = createContext<
* @returns A tuple with the API instance, and an error if one occurred.
*/
export function useDeferredApi(
widget: DhType.ide.VariableDescriptor
): [typeof DhType | null, unknown | null] {
widget: DhType.ide.VariableDescriptor | null
): [dh: typeof DhType | null, error: unknown | null] {
mofojed marked this conversation as resolved.
Show resolved Hide resolved
const [api, setApi] = useState<typeof DhType | null>(null);
const [error, setError] = useState<unknown | null>(null);
const deferredApi = useContext(DeferredApiContext);
Expand All @@ -49,7 +49,12 @@ export function useDeferredApi(
let isCancelled = false;

async function loadApi() {
if (typeof deferredApi === 'function') {
if (widget == null) {
if (!isCancelled) {
setApi(null);
setError(new Error('No widget provided to useDeferredApi'));
}
} else if (typeof deferredApi === 'function') {
try {
const newApi = await deferredApi(widget);
if (!isCancelled) {
Expand Down
Loading