WIP Close widget if all panels opened for the widget are closed

- Need to handle the close correctly on the server side still - probably add all exported objects to a liveness scope and release it? - Or am I just not handling it correctly in the UI still? Clicking the same widget twice gives an error now instead of "refreshing" that panel - Should write some unit tests
mofojed · Oct 17, 2023 · b7b7de8 · b7b7de8
1 parent 8a80a4d
commit b7b7de8
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 28 deletions.
diff --git a/plugins/ui/src/js/src/DashboardPlugin.tsx b/plugins/ui/src/js/src/DashboardPlugin.tsx
@@ -2,6 +2,7 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import shortid from 'shortid';
 import {
   DashboardPluginComponentProps,
+  PanelEvent,
   useListener,
 } from '@deephaven/dashboard';
 import Log from '@deephaven/log';
@@ -30,7 +31,7 @@ export function DashboardPlugin({
       dragEvent,
       fetch,
       metadata = {},
-      panelId = shortid.generate(),
+      panelId: widgetId = shortid.generate(),
       widget,
     }: {
       dragEvent?: React.DragEvent;
@@ -44,16 +45,25 @@ export function DashboardPlugin({
         // Only want to listen for Element panels trying to be opened
         return;
       }
-      log.info('Opening element with ID', panelId);
+      log.info('Opening widget with ID', widgetId);
       setWidgetMap(prevWidgetMap => {
         const newWidgetMap = new Map<string, WidgetWrapper>(prevWidgetMap);
-        newWidgetMap.set(panelId, { definition: widget, fetch });
+        newWidgetMap.set(widgetId, { definition: widget, fetch, id: widgetId });
         return newWidgetMap;
       });
     },
     []
   );
 
+  const handleWidgetClose = useCallback((widget: WidgetWrapper) => {
+    log.debug('Closing widget', widget);
+    setWidgetMap(prevWidgetMap => {
+      const newWidgetMap = new Map<string, WidgetWrapper>(prevWidgetMap);
+      newWidgetMap.delete(widget.id);
+      return newWidgetMap;
+    });
+  }, []);
+
   useEffect(() => {
     const cleanups = [registerComponent(PortalPanel.displayName, PortalPanel)];
 
@@ -63,14 +73,18 @@ export function DashboardPlugin({
   }, [registerComponent]);
 
   // TODO: We need to change up the event system for how objects are opened, since in this case it could be opening multiple panels
-  useListener(layout.eventHub, 'PanelEvent.OPEN', handlePanelOpen);
+  useListener(layout.eventHub, PanelEvent.OPEN, handlePanelOpen);
 
   const widgetHandlers = useMemo(
     () =>
-      [...widgetMap.entries()].map(([panelId, widget]) => (
-        <WidgetHandler key={panelId} widget={widget} />
+      [...widgetMap.entries()].map(([widgetId, widget]) => (
+        <WidgetHandler
+          key={widgetId}
+          widget={widget}
+          onClose={handleWidgetClose}
+        />
       )),
-    [widgetMap]
+    [handleWidgetClose, widgetMap]
   );
 
   return (

diff --git a/plugins/ui/src/js/src/DocumentHandler.tsx b/plugins/ui/src/js/src/DocumentHandler.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useCallback, useRef } from 'react';
 import { WidgetDefinition } from '@deephaven/dashboard';
 import Log from '@deephaven/log';
 import { ElementNode, getElementType } from './ElementUtils';
@@ -14,10 +14,32 @@ export interface DocumentHandlerProps {
 
   /** The root element to render */
   element: ElementNode;
+
+  /** Triggered when all panels opened from this document have closed */
+  onClose?: () => void;
 }
 
-function DocumentHandler({ definition, element }: DocumentHandlerProps) {
+function DocumentHandler({
+  definition,
+  element,
+  onClose,
+}: DocumentHandlerProps) {
   log.debug('Rendering document', element);
+  const panelOpenCountRef = useRef(0);
+
+  const handlePanelOpen = useCallback(() => {
+    panelOpenCountRef.current += 1;
+  }, []);
+
+  const handlePanelClose = useCallback(() => {
+    panelOpenCountRef.current -= 1;
+    if (panelOpenCountRef.current < 0) {
+      throw new Error('Panel open count when negative');
+    }
+    if (panelOpenCountRef.current === 0) {
+      onClose?.();
+    }
+  }, [onClose]);
 
   const { children } = element.props ?? {};
   const childrenArray = Array.isArray(children) ? children : [children];
@@ -39,7 +61,12 @@ function DocumentHandler({ definition, element }: DocumentHandlerProps) {
           title = child.props.title ?? title;
         }
         return (
-          <ReactPanel title={title} key={key}>
+          <ReactPanel
+            title={title}
+            key={key}
+            onOpen={handlePanelOpen}
+            onClose={handlePanelClose}
+          >
             <ElementView element={child} />
           </ReactPanel>
         );

diff --git a/plugins/ui/src/js/src/ReactPanel.tsx b/plugins/ui/src/js/src/ReactPanel.tsx
@@ -1,7 +1,7 @@
-import { useEffect, useMemo, useRef, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import ReactDOM from 'react-dom';
 import shortid from 'shortid';
-import { LayoutUtils, PanelEvent } from '@deephaven/dashboard';
+import { LayoutUtils, PanelEvent, useListener } from '@deephaven/dashboard';
 import Log from '@deephaven/log';
 import PortalPanel from './PortalPanel';
 import useLayout from './useLayout';
@@ -14,12 +14,18 @@ export interface ReactPanelProps {
 
   /** Title of the panel */
   title: string;
+
+  /** Triggered when this panel is opened */
+  onOpen?: () => void;
+
+  /** Triggered when this panel is closed */
+  onClose?: () => void;
 }
 
 /**
  * Adds and tracks a panel to the GoldenLayout. When the child element is updated, the contents of the panel will also be updated. When unmounted, the panel will be removed.
  */
-function ReactPanel({ children, title }: ReactPanelProps) {
+function ReactPanel({ children, onClose, onOpen, title }: ReactPanelProps) {
   const layout = useLayout();
   const panelId = useMemo(() => shortid(), []);
   const [element, setElement] = useState<HTMLElement>();
@@ -32,11 +38,24 @@ function ReactPanel({ children, title }: ReactPanelProps) {
       if (panelOpenRef.current) {
         LayoutUtils.closeComponent(layout.root, { id: panelId });
         panelOpenRef.current = false;
+        onClose?.();
+      }
+    },
+    [layout, onClose, panelId]
+  );
+
+  const handlePanelClosed = useCallback(
+    closedPanelId => {
+      if (closedPanelId === panelId) {
+        log.debug('Panel closed', panelId);
+        onClose?.();
       }
     },
-    [layout, panelId]
+    [onClose, panelId]
   );
 
+  useListener(layout.eventHub, PanelEvent.CLOSED, handlePanelClosed);
+
   useEffect(() => {
     if (panelOpenRef.current === false) {
       const config = {
@@ -58,8 +77,10 @@ function ReactPanel({ children, title }: ReactPanelProps) {
       LayoutUtils.openComponent({ root, config });
       log.debug('Opened panel', panelId, config);
       panelOpenRef.current = true;
+
+      onOpen?.();
     }
-  }, [children, layout, panelId, title]);
+  }, [children, layout, onOpen, panelId, title]);
 
   return element ? ReactDOM.createPortal(children, element) : null;
 }

diff --git a/plugins/ui/src/js/src/WidgetHandler.tsx b/plugins/ui/src/js/src/WidgetHandler.tsx
@@ -19,30 +19,38 @@ import {
 } from './ElementUtils';
 import { JsWidget, WidgetMessageEvent, WidgetWrapper } from './WidgetTypes';
 import DocumentHandler from './DocumentHandler';
-import shortid from 'shortid';
 
 const log = Log.module('@deephaven/js-plugin-ui/WidgetHandler');
 
 export interface WidgetHandlerProps {
   /** Widget for this to handle */
   widget: WidgetWrapper;
+
+  /** Triggered when all panels opened from this widget have closed */
+  onClose?: (widget: WidgetWrapper) => void;
 }
 
-function WidgetHandler(props: WidgetHandlerProps) {
-  const { widget: wrapper } = props;
+function WidgetHandler({ onClose, widget: wrapper }: WidgetHandlerProps) {
   const dh = useApi();
 
   const [widget, setWidget] = useState<JsWidget>();
   const [element, setElement] = useState<ElementNode>();
 
+  useEffect(
+    () => () => {
+      widget?.close();
+    },
+    [widget]
+  );
+
   // Bi-directional communication as defined in https://www.npmjs.com/package/json-rpc-2.0
   const jsonClient = useMemo(
     () =>
       widget != null
         ? new JSONRPCServerAndClient(
             new JSONRPCServer(),
             new JSONRPCClient(request => {
-              log.info('Sending request', request);
+              log.debug('Sending request', request);
               widget.sendMessage(JSON.stringify(request), []);
             })
           )
@@ -65,7 +73,7 @@ function WidgetHandler(props: WidgetHandlerProps) {
         // Need to re-hydrate any objects that are defined
         if (isCallableNode(value)) {
           const callableId = value[CALLABLE_KEY];
-          log.info('Registering callableId', callableId);
+          log.debug2('Registering callableId', callableId);
           return async (...args: unknown[]) => {
             log.debug('Callable called', callableId, ...args);
             return jsonClient?.request(callableId, args);
@@ -87,9 +95,9 @@ function WidgetHandler(props: WidgetHandlerProps) {
         return;
       }
 
-      log.info('Adding methods to jsonClient');
+      log.debug('Adding methods to jsonClient');
       jsonClient.addMethod('documentUpdated', async (params: [ElementNode]) => {
-        log.info('documentUpdated', params[0]);
+        log.debug2('documentUpdated', params[0]);
         setElement(params[0]);
       });
 
@@ -105,7 +113,7 @@ function WidgetHandler(props: WidgetHandlerProps) {
       return;
     }
     function receiveData(data: string, exportedObjects: ExportedObject[]) {
-      log.info('Data received', data, exportedObjects);
+      log.debug2('Data received', data, exportedObjects);
       const parsedData = parseData(data, exportedObjects);
       jsonClient?.receiveAndSend(parsedData);
     }
@@ -120,12 +128,12 @@ function WidgetHandler(props: WidgetHandlerProps) {
       }
     );
 
-    log.info('Receiving initial data');
+    log.debug('Receiving initial data');
     // We need to get the initial data and process it. It should be a documentUpdated command.
     receiveData(widget.getDataAsString(), widget.exportedObjects);
 
     return () => {
-      log.info('Cleaning up listener');
+      log.debug('Cleaning up listener');
       cleanup();
     };
   }, [dh, jsonClient, parseData, widget]);
@@ -136,9 +144,10 @@ function WidgetHandler(props: WidgetHandlerProps) {
       async function loadWidgetInternal() {
         const newWidget = await wrapper.fetch();
         if (isCancelled) {
+          newWidget.close();
           return;
         }
-        log.info('newWidget', wrapper.definition, newWidget);
+        log.debug('newWidget', wrapper.definition, newWidget);
         setWidget(newWidget);
       }
       loadWidgetInternal();
@@ -149,12 +158,21 @@ function WidgetHandler(props: WidgetHandlerProps) {
     [wrapper]
   );
 
+  const handleDocumentClose = useCallback(() => {
+    log.debug('Widget document closed', wrapper.definition);
+    onClose?.(wrapper);
+  }, [onClose, wrapper]);
+
   return useMemo(
     () =>
       element ? (
-        <DocumentHandler definition={wrapper.definition} element={element} />
+        <DocumentHandler
+          definition={wrapper.definition}
+          element={element}
+          onClose={handleDocumentClose}
+        />
       ) : null,
-    [element, wrapper]
+    [element, handleDocumentClose, wrapper]
   );
 }
 

diff --git a/plugins/ui/src/js/src/WidgetTypes.ts b/plugins/ui/src/js/src/WidgetTypes.ts
@@ -17,11 +17,13 @@ export interface JsWidget extends WidgetMessageDetails {
     listener: (event: WidgetMessageEvent) => void
   ) => () => void;
   sendMessage: (message: string, args: unknown[]) => void;
+  close: () => void;
 }
 
 export type WidgetFetch = () => Promise<JsWidget>;
 
 export type WidgetWrapper = {
   definition: WidgetDefinition;
   fetch: WidgetFetch;
+  id: string;
 };