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: Reuse dashboard tabs when reassigning the variable #2243

Merged
merged 2 commits into from
Oct 2, 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
76 changes: 55 additions & 21 deletions packages/code-studio/src/main/AppMainContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ import {
getAllDashboardsData,
getDashboardData,
listenForCreateDashboard,
listenForCloseDashboard,
PanelEvent,
setDashboardData as setDashboardDataAction,
setDashboardPluginData as setDashboardPluginDataAction,
stopListenForCreateDashboard,
updateDashboardData as updateDashboardDataAction,
} from '@deephaven/dashboard';
import {
Expand Down Expand Up @@ -189,6 +189,8 @@ export class AppMainContainer extends Component<
const { allDashboardData } = this.props;

this.dashboardLayouts = new Map();
this.createDashboardListenerRemovers = new Map();
this.closeDashboardListenerRemovers = new Map();

this.state = {
contextActions: [
Expand Down Expand Up @@ -275,9 +277,8 @@ export class AppMainContainer extends Component<
componentWillUnmount(): void {
this.deinitWidgets();

this.dashboardLayouts.forEach(layout => {
stopListenForCreateDashboard(layout.eventHub, this.handleCreateDashboard);
});
this.createDashboardListenerRemovers.forEach(rm => rm());
this.closeDashboardListenerRemovers.forEach(rm => rm());

window.removeEventListener(
'beforeunload',
Expand All @@ -288,6 +289,12 @@ export class AppMainContainer extends Component<
/** Map from the dashboard ID to the GoldenLayout instance for that dashboard */
dashboardLayouts: Map<string, GoldenLayout>;

/** Map from dashboard ID to remover functions for create dashboard listener */
createDashboardListenerRemovers: Map<string, () => void>;

/** Map from dashboard ID to remover functions for close dashboard listener */
closeDashboardListenerRemovers: Map<string, () => void>;

importElement: RefObject<HTMLInputElement>;

widgetListenerRemover?: () => void;
Expand Down Expand Up @@ -504,16 +511,21 @@ export class AppMainContainer extends Component<
const oldLayout = this.dashboardLayouts.get(activeTabKey);
if (oldLayout === newLayout) return;

this.dashboardLayouts.set(activeTabKey, newLayout);

if (oldLayout != null) {
stopListenForCreateDashboard(
oldLayout.eventHub,
this.handleCreateDashboard
);
this.createDashboardListenerRemovers.get(activeTabKey)?.();
this.closeDashboardListenerRemovers.get(activeTabKey)?.();
}

this.dashboardLayouts.set(activeTabKey, newLayout);

listenForCreateDashboard(newLayout.eventHub, this.handleCreateDashboard);
this.createDashboardListenerRemovers.set(
activeTabKey,
listenForCreateDashboard(newLayout.eventHub, this.handleCreateDashboard)
);
this.closeDashboardListenerRemovers.set(
activeTabKey,
listenForCloseDashboard(newLayout.eventHub, this.handleCloseDashboard)
);
}

handleCreateDashboard({
Expand All @@ -524,16 +536,38 @@ export class AppMainContainer extends Component<
const newId = nanoid();
const { setDashboardPluginData } = this.props;
setDashboardPluginData(newId, pluginId, data);
this.setState(({ tabs }) => ({
tabs: [
...tabs,
{
key: newId,
title,
},
],
activeTabKey: newId,
}));
this.setState(({ tabs }) => {
const existingTab = tabs.findIndex(
({ title: tabTitle }) => tabTitle === title
);
if (existingTab !== -1) {
// Replace the existing tab
return {
tabs: tabs.map((tab, index) =>
index === existingTab ? { key: newId, title } : tab
),
activeTabKey: newId,
};
}
return {
tabs: [
...tabs,
{
key: newId,
title,
},
],
activeTabKey: newId,
};
});
}

handleCloseDashboard(title: string): void {
const { tabs } = this.state;
const tab = tabs.find(t => t.title === title);
if (tab != null) {
this.handleTabClose(tab.key);
}
}

handleWidgetMenuClick(): void {
Expand Down
3 changes: 3 additions & 0 deletions packages/dashboard-core-plugins/src/panels/ConsolePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '@deephaven/console';
import {
type DashboardPanelProps,
emitCloseDashboard,
emitPanelOpen,
LayoutManagerContext,
LayoutUtils,
Expand Down Expand Up @@ -278,6 +279,8 @@ export class ConsolePanel extends PureComponent<
if (id != null) {
const { glEventHub } = this.props;
glEventHub.emit(PanelEvent.CLOSE, id);
// Just emit for all panels since there shouldn't be dashboard and panel name conflicts
emitCloseDashboard(glEventHub, title);
}
}
}
Expand Down
38 changes: 13 additions & 25 deletions packages/dashboard/src/DashboardEvents.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,23 @@
import type { EventHub } from '@deephaven/golden-layout';
import { makeEventFunctions } from '@deephaven/golden-layout';

export const CREATE_DASHBOARD = 'CREATE_DASHBOARD';

export const CLOSE_DASHBOARD = 'CLOSE_DASHBOARD';

export interface CreateDashboardPayload<T = unknown> {
pluginId: string;
title: string;
data: T;
}

export function stopListenForCreateDashboard<T = unknown>(
eventHub: EventHub,
handler: (p: CreateDashboardPayload<T>) => void
): void {
try {
eventHub.off(CREATE_DASHBOARD, handler);
} catch {
// golden-layout throws if the handler is not found. Instead catch it and no-op
}
}
export const {
listen: listenForCreateDashboard,
emit: emitCreateDashboard,
useListener: useCreateDashboardListener,
} = makeEventFunctions<[detail: CreateDashboardPayload]>(CREATE_DASHBOARD);

export function listenForCreateDashboard<T = unknown>(
eventHub: EventHub,
handler: (p: CreateDashboardPayload<T>) => void
): () => void {
eventHub.on(CREATE_DASHBOARD, handler);
return () => stopListenForCreateDashboard(eventHub, handler);
}

export function emitCreateDashboard<T = unknown>(
eventHub: EventHub,
payload: CreateDashboardPayload<T>
): void {
eventHub.emit(CREATE_DASHBOARD, payload);
}
export const {
listen: listenForCloseDashboard,
emit: emitCloseDashboard,
useListener: useCloseDashboardListener,
} = makeEventFunctions<[title: string]>(CLOSE_DASHBOARD);
33 changes: 9 additions & 24 deletions packages/embed-widget/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ import Log from '@deephaven/log';
import { useDashboardPlugins } from '@deephaven/plugin';
import {
getAllDashboardsData,
listenForCreateDashboard,
type CreateDashboardPayload,
setDashboardPluginData,
stopListenForCreateDashboard,
emitPanelOpen,
useCreateDashboardListener,
} from '@deephaven/dashboard';
import {
getVariableDescriptor,
Expand Down Expand Up @@ -147,31 +146,17 @@ function App(): JSX.Element {
const [goldenLayout, setGoldenLayout] = useState<GoldenLayout | null>(null);
const [dashboardId, setDashboardId] = useState('default-embed-widget'); // Can't be DEFAULT_DASHBOARD_ID because its dashboard layout is not stored in dashboardData

const handleGoldenLayoutChange = useCallback(
(newLayout: GoldenLayout) => {
function handleCreateDashboard({
pluginId,
data,
}: CreateDashboardPayload) {
const id = nanoid();
dispatch(setDashboardPluginData(id, pluginId, data));
setDashboardId(id);
}

setGoldenLayout(oldLayout => {
if (oldLayout != null) {
stopListenForCreateDashboard(
oldLayout.eventHub,
handleCreateDashboard
);
}
listenForCreateDashboard(newLayout.eventHub, handleCreateDashboard);
return newLayout;
});
const handleCreateDashboard = useCallback(
({ pluginId, data }: CreateDashboardPayload) => {
const id = nanoid();
dispatch(setDashboardPluginData(id, pluginId, data));
setDashboardId(id);
},
[dispatch]
);

useCreateDashboardListener(goldenLayout?.eventHub, handleCreateDashboard);

const [hasEmittedWidget, setHasEmittedWidget] = useState(false);

const handleDashboardInitialized = useCallback(() => {
Expand Down Expand Up @@ -243,7 +228,7 @@ function App(): JSX.Element {
]}
activeDashboard={dashboardId}
onLayoutInitialized={handleDashboardInitialized}
onGoldenLayoutChange={handleGoldenLayoutChange}
onGoldenLayoutChange={setGoldenLayout}
plugins={dashboardPlugins}
/>
</ErrorBoundary>
Expand Down
21 changes: 15 additions & 6 deletions packages/golden-layout/src/utils/EventUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect } from 'react';
import { useEffect, useLayoutEffect, useRef } from 'react';
import EventEmitter from './EventEmitter';

type AsArray<P> = P extends unknown[] ? P : [P];
Expand All @@ -16,7 +16,7 @@ export type EventEmitFunction<TParameters = []> = (
) => void;

export type EventListenerHook<TParameters = []> = (
eventEmitter: EventEmitter,
eventEmitter: EventEmitter | null | undefined,
handler: EventHandlerFunction<TParameters>
) => void;

Expand Down Expand Up @@ -57,10 +57,19 @@ export function makeUseListenerFunction<TParameters = []>(
event: string
): EventListenerHook<TParameters> {
return (eventEmitter, handler) => {
useEffect(
() => listenForEvent(eventEmitter, event, handler),
[eventEmitter, handler]
);
const eventEmitterRef = useRef<typeof eventEmitter>(null);
const handlerRef = useRef<typeof handler>(() => false);

if (
eventEmitterRef.current != eventEmitter ||
handlerRef.current != handler
) {
eventEmitterRef.current?.off(event, handlerRef.current);
eventEmitter?.on(event, handler);
}

eventEmitterRef.current = eventEmitter;
handlerRef.current = handler;
};
}

Expand Down
Loading