Clean up based on review

- Extract getPreservedData method
- Add tests
- Add some more comments

plugins/ui/src/js/src/DashboardPlugin.tsx

@@ -31,6 +31,7 @@ import {
 import PortalPanel from './layout/PortalPanel';
 import PortalPanelManager from './layout/PortalPanelManager';
 import DashboardWidgetHandler from './widget/DashboardWidgetHandler';
+import { getPreservedData } from './widget/WidgetUtils';
 
 const NAME_ELEMENT = 'deephaven.ui.Element';
 const DASHBOARD_ELEMENT = 'deephaven.ui.Dashboard';
@@ -101,10 +102,7 @@ export function DashboardPlugin(
           fetch,
           id: widgetId,
           widget,
-          data: {
-            // We don't want to blow away the panel IDs that may already be open for this widget
-            panelIds: oldWidget?.data?.panelIds ?? [],
-          },
+          data: getPreservedData(oldWidget?.data),
         });
         return newWidgetMap;
       });

plugins/ui/src/js/src/layout/ReactPanel.test.tsx

@@ -197,7 +197,7 @@ it('does not call openComponent or setActiveContentItem if panel already exists
   expect(onOpen).toHaveBeenCalledTimes(1);
   expect(onClose).not.toHaveBeenCalled();
 
-  // Now check that it focuses it if it's called after the metadat changes
+  // Now check that it focuses it if it's called after the metadata changes
   rerender(
     <PortalPanelManagerContext.Provider value={portals}>
       {makeReactPanelManager({

plugins/ui/src/js/src/layout/ReactPanel.tsx

@@ -70,7 +70,15 @@ function ReactPanel({ children, title }: ReactPanelProps) {
   useListener(eventHub, PanelEvent.CLOSED, handlePanelClosed);
 
   useEffect(
-    /** Opens a panel in the layout if necessary. Triggered when the panel metadata changes or the panel has not been opened yet. */
+    /**
+     * Opens a panel in the layout if necessary. There are a few cases this is triggered:
+     * 1. Panel has not been opened yet: we need to open the panel in this case.
+     * 2. Panel metadata changes: we need to update the panel with the new metadata, show the panel in it's current stack,
+     *    and refresh the content with new state.
+     * 3. Widget is being re-hydrated: we need to check if the panel ID is already open, and then use that existing portal.
+     *    We don't need to focus in this case, as this is when a whole dashboard is being re-hydrated - not when the user is
+     *    opening this widget in particular.
+     */
     function openIfNecessary() {
       const itemConfig = { id: panelId };
       const existingStack = LayoutUtils.getStackForConfig(parent, itemConfig);

plugins/ui/src/js/src/widget/DocumentHandler.tsx

@@ -48,7 +48,15 @@ function DocumentHandler({
   log.debug('Rendering document', widget);
   const panelOpenCountRef = useRef(0);
   const panelIdIndex = useRef(0);
+
+  // Using `useState` here to initialize the data only once.
+  // We don't want to use `useMemo`, because we only want it to be initialized once with the `initialData` (uncontrolled)
+  // We don't want to use `useRef`, because we only want to run `structuredClone` once, and you can't pass an
+  // initialization function into `useRef` like you can with `useState`
   const [widgetData] = useState<WidgetData>(() => structuredClone(data));
+
+  // panelIds that are currently opened within this document. This list is tracked by the `onOpen`/`onClose` call on the `ReactPanelManager` from a child component.
+  // Note that the initial widget data provided will be the `panelIds` for this document to use; this array is what is actually opened currently.
   const [panelIds] = useState<string[]>([]);
 
   const handleOpen = useCallback(
@@ -68,8 +76,8 @@ function DocumentHandler({
 
   const handleClose = useCallback(
     (panelId: string) => {
-      const panelIndex = panelIds?.indexOf(panelId);
-      if (panelIndex === -1 || panelIndex == null) {
+      const panelIndex = panelIds.indexOf(panelId);
+      if (panelIndex === -1) {
         throw new Error('Panel close received for unknown panel');
       }
       panelOpenCountRef.current -= 1;

plugins/ui/src/js/src/widget/WidgetUtils.test.tsx

@@ -15,6 +15,7 @@ import {
   elementComponentMap,
   getComponentForElement,
   getComponentTypeForElement,
+  getPreservedData,
 } from './WidgetUtils';
 
 describe('getComponentTypeForElement', () => {
@@ -75,3 +76,23 @@ describe('getComponentForElement', () => {
     );
   });
 });
+
+describe('getPreservedData', () => {
+  it('should handle undefined widget data', () => {
+    const actual = getPreservedData(undefined);
+    expect(actual).toEqual({});
+  });
+  it('should handle empty widget data', () => {
+    const actual = getPreservedData({});
+    expect(actual).toEqual({});
+  });
+  it('should return only the panelIds from the widget data', () => {
+    const widgetData = {
+      panelIds: ['1', '2', '3'],
+      state: { foo: 'bar' },
+    };
+
+    const actual = getPreservedData(widgetData);
+    expect(actual).toEqual({ panelIds: widgetData.panelIds });
+  });
+});

plugins/ui/src/js/src/widget/WidgetUtils.tsx

@@ -4,6 +4,7 @@ import React, { ComponentType } from 'react';
 // Importing `Item` and `Section` compnents directly since they should not be
 // wrapped due to how Spectrum collection components consume them.
 import { Item, Section } from '@deephaven/components';
+import { ReadonlyWidgetData } from './WidgetTypes';
 import {
   ElementNode,
   ELEMENT_KEY,
@@ -82,3 +83,23 @@ export function getComponentForElement(element: ElementNode): React.ReactNode {
 
   return newElement.props?.children;
 }
+
+/** Data keys of a widget to preserve across re-opening. */
+const PRESERVED_DATA_KEYS: (keyof ReadonlyWidgetData)[] = ['panelIds'];
+const PRESERVED_DATA_KEYS_SET = new Set<string>(PRESERVED_DATA_KEYS);
+
+/**
+ * Returns an object with only the data preserved that should be preserved when re-opening (i.e. opening it again from console) a widget.
+ * For example, if you re-open a widget, you want to keep the `panelIds` data because that will re-open the widget to where it was before.
+ * However, we do _not_ want to preserve the `state` in this case - we want to widget to start from a fresh state.
+ * Similar to how when you re-open a table, it'll open in the same spot, but all UI applied filters/operations will be reset.
+ * @param oldData The old data to get the preserved data from
+ * @returns The data to preserve
+ */
+export function getPreservedData(
+  oldData: ReadonlyWidgetData = {}
+): ReadonlyWidgetData {
+  return Object.fromEntries(
+    Object.entries(oldData).filter(([key]) => PRESERVED_DATA_KEYS_SET.has(key))
+  );
+}