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: Save/load plugin data with layout #1866

Merged
merged 6 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/code-studio/src/main/AppDashboards.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ interface AppDashboardsProps {
dashboards: {
id: string;
layoutConfig: ItemConfigType[];
key?: string;
}[];
activeDashboard: string;
onGoldenLayoutChange: (goldenLayout: LayoutManager) => void;
Expand Down Expand Up @@ -65,6 +66,7 @@ export function AppDashboards({
>
<LazyDashboard
id={d.id}
key={d.key}
isActive={d.id === activeDashboard}
emptyDashboard={
d.id === DEFAULT_DASHBOARD_ID ? (
Expand Down
61 changes: 59 additions & 2 deletions packages/code-studio/src/main/AppMainContainer.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import React from 'react';
import { Provider } from 'react-redux';
import { render, screen } from '@testing-library/react';
import { act, render, screen } from '@testing-library/react';
import { ConnectionContext } from '@deephaven/app-utils';
import { ToolType } from '@deephaven/dashboard-core-plugins';
import {
Expand Down Expand Up @@ -112,17 +112,39 @@ function renderAppMainContainer({
</Provider>
);
}

const EMPTY_LAYOUT = {
filterSets: [],
layoutConfig: [],
links: [],
version: 2,
};

let mockProp = {};
let mockId = DEFAULT_DASHBOARD_ID;
let mockIteration = 0;
jest.mock('@deephaven/dashboard', () => ({
...jest.requireActual('@deephaven/dashboard'),
__esModule: true,
LazyDashboard: jest.fn(({ hydrate }) => {
const { useMemo } = jest.requireActual('react');
// We use the `key` to determine how many times this LazyDashboard component was re-rendered with a new key
// When rendered with a new key, the `useMemo` will be useless and will return a new key
const key = useMemo(() => {
const newKey = `${mockIteration}`;
mockIteration += 1;
return newKey;
}, []);
const result = hydrate(mockProp, mockId);
if (result.fetch != null) {
result.fetch();
}
return <p>{JSON.stringify(result)}</p>;
return (
<>
<p>{JSON.stringify(result)}</p>
<p data-testid="dashboard-key">{key}</p>
</>
);
}),
default: jest.fn(),
}));
Expand All @@ -136,6 +158,8 @@ beforeEach(() => {
cb(0);
return 0;
});
mockProp = {};
mockIteration = 0;
});

afterEach(() => {
Expand Down Expand Up @@ -241,3 +265,36 @@ describe('hydrates widgets correctly', () => {
expect(objectFetcher).toHaveBeenCalled();
});
});

describe('imports layout correctly', () => {
it('uses a new key when layout is imported', async () => {
renderAppMainContainer();

expect(screen.getByText('{"localDashboardId":"default"}')).toBeTruthy();

const oldKey = screen.getByTestId('dashboard-key').textContent ?? '';
expect(oldKey.length).not.toBe(0);

await act(async () => {
const text = JSON.stringify(EMPTY_LAYOUT);
const file = TestUtils.createMockProxy<File>({
text: () => Promise.resolve(text),
name: 'layout.json',
type: 'application/json',
});

// Technically, the "Import Layout" button in the panels list is what the user clicks on to show the file picker
// However, the testing library uses the `.upload` command on the `input` element directly, which we don't display
// So just fetch it by testid and use the `.upload` command: https://testing-library.com/docs/user-event/utility/#upload
const importInput = screen.getByTestId('input-import-layout');
await userEvent.upload(importInput, file);
});

expect(screen.getByText('{"localDashboardId":"default"}')).toBeTruthy();

const newKey = screen.getByTestId('dashboard-key').textContent ?? '';

expect(newKey.length).not.toBe(0);
expect(newKey).not.toBe(oldKey);
});
});
26 changes: 15 additions & 11 deletions packages/code-studio/src/main/AppMainContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ import {
getDashboardSessionWrapper,
ControlType,
ToolType,
FilterSet,
Link,
getDashboardConnection,
NotebookPanel,
} from '@deephaven/dashboard-core-plugins';
Expand Down Expand Up @@ -155,6 +153,9 @@ interface AppMainContainerState {
widgets: DhType.ide.VariableDefinition[];
tabs: NavTabItem[];
activeTabKey: string;

// Number of times the layout has been re-initialized
layoutIteration: number;
}

export class AppMainContainer extends Component<
Expand Down Expand Up @@ -255,6 +256,7 @@ export class AppMainContainer extends Component<
title: value.title ?? 'Untitled',
})),
activeTabKey: DEFAULT_DASHBOARD_ID,
layoutIteration: 0,
};
}

Expand Down Expand Up @@ -577,13 +579,7 @@ export class AppMainContainer extends Component<
try {
const { workspace } = this.props;
const { data } = workspace;
const exportedConfig = UserLayoutUtils.exportLayout(
data as {
filterSets: FilterSet[];
links: Link[];
layoutConfig: ItemConfigType[];
}
);
const exportedConfig = UserLayoutUtils.exportLayout(data);

log.info('handleExportLayoutClick exportedConfig', exportedConfig);

Expand Down Expand Up @@ -658,14 +654,18 @@ export class AppMainContainer extends Component<
const { updateDashboardData, updateWorkspaceData } = this.props;
const fileText = await file.text();
const exportedLayout = JSON.parse(fileText);
const { filterSets, layoutConfig, links } =
const { filterSets, layoutConfig, links, pluginDataMap } =
UserLayoutUtils.normalizeLayout(exportedLayout);

updateWorkspaceData({ layoutConfig });
updateDashboardData(DEFAULT_DASHBOARD_ID, {
filterSets,
links,
pluginDataMap,
});
this.setState(({ layoutIteration }) => ({
layoutIteration: layoutIteration + 1,
}));
} catch (e) {
log.error('Unable to import layout', e);
}
Expand Down Expand Up @@ -832,8 +832,9 @@ export class AppMainContainer extends Component<
getDashboards(): {
id: string;
layoutConfig: ItemConfigType[];
key?: string;
}[] {
const { tabs } = this.state;
const { layoutIteration, tabs } = this.state;
const { allDashboardData, workspace } = this.props;
const { data: workspaceData } = workspace;
const { layoutConfig } = workspaceData;
Expand All @@ -842,11 +843,13 @@ export class AppMainContainer extends Component<
{
id: DEFAULT_DASHBOARD_ID,
layoutConfig: layoutConfig as ItemConfigType[],
key: `${DEFAULT_DASHBOARD_ID}-${layoutIteration}`,
},
...tabs.map(tab => ({
mofojed marked this conversation as resolved.
Show resolved Hide resolved
id: tab.key,
layoutConfig: (allDashboardData[tab.key]?.layoutConfig ??
EMPTY_ARRAY) as ItemConfigType[],
key: `${tab.key}-${layoutIteration}`,
})),
];
}
Expand Down Expand Up @@ -1019,6 +1022,7 @@ export class AppMainContainer extends Component<
accept=".json"
style={{ display: 'none' }}
onChange={this.handleImportLayoutFiles}
data-testid="input-import-layout"
/>
<DebouncedModal
isOpen={isDisconnected && !isAuthFailed}
Expand Down
18 changes: 9 additions & 9 deletions packages/code-studio/src/main/UserLayoutUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import {
CommandHistoryPanel,
ConsolePanel,
FileExplorerPanel,
FilterSet,
Link,
LogPanel,
} from '@deephaven/dashboard-core-plugins';
import type { ItemConfigType } from '@deephaven/golden-layout';
import Log from '@deephaven/log';
import { CustomizableWorkspaceData } from '@deephaven/redux';
import LayoutStorage, {
ExportedLayout,
ExportedLayoutV2,
Expand Down Expand Up @@ -137,16 +135,18 @@ export async function getDefaultLayout(
: DEFAULT_LAYOUT_CONFIG_NO_CONSOLE;
}

export function exportLayout(data: {
filterSets: FilterSet[];
links: Link[];
layoutConfig: ItemConfigType[];
}): ExportedLayoutV2 {
const { filterSets, layoutConfig, links } = data;
export function exportLayout(
data: CustomizableWorkspaceData
): ExportedLayoutV2 {
const { filterSets, layoutConfig, links, pluginDataMap } = data as Omit<
ExportedLayoutV2,
'version'
>;
Comment on lines +141 to +144
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this cast is because the actual data types are part of the dashboard package, but the redux package just marks them as unknown. We should probably clean that up at some point. Related #730

const exportedLayout: ExportedLayoutV2 = {
filterSets,
layoutConfig,
links,
pluginDataMap,
version: 2,
};
return exportedLayout;
Expand Down
2 changes: 2 additions & 0 deletions packages/code-studio/src/storage/LayoutStorage.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ItemConfigType } from '@deephaven/golden-layout';
import { FilterSet, Link } from '@deephaven/dashboard-core-plugins';
import { PluginDataMap } from '@deephaven/redux';

/**
* Have a different version to support legacy layout exports
Expand All @@ -10,6 +11,7 @@ export type ExportedLayoutV2 = {
filterSets: FilterSet[];
links: Link[];
layoutConfig: ItemConfigType[];
pluginDataMap?: PluginDataMap;
version: 2;
};

Expand Down
Loading