feat: DH-19717: Add per-user persistence for dashboards#1352
Open
mofojed wants to merge 5 commits into
Open
Conversation
Member
mofojed
commented
May 23, 2026
mofojed
commented
- Nested dashboards now store the last layout config
- When reloading the dashboard, it just uses that layout config and Rows, Columns, and Stacks do not try and build their own layout
- Will need to test this extensively. Especially with dashboards that have changed the layout after the fact; I think in those cases we just do a "best" effort, and panels may open in weird locations. User should have the option to discard their override (at least in Enterprise).
- Matching Enterprise PR for testing with Enterprise
- Nested dashboards now store the last layout config - When reloading the dashboard, it just uses that layout config and Rows, Columns, and Stacks do not try and build their own layout - Will need to test this extensively. Especially with dashboards that have changed the layout after the fact; I think in those cases we just do a "best" effort, and panels may open in weird locations. User should have the option to discard their override (at least in Enterprise).
- It's not laying out correctly, plots aren't working
… loop - Throttle handleDataChange via useThrottledCallback (1000ms) and skip setDashboardData when the merged widgetData is deep-equal to the last committed value, breaking the re-render loop. - Accumulate partial widget data updates in a ref so intermediate changes are preserved between throttled flushes; flush on unmount. - Preserve widgetData in onLayoutConfigChange (was overwriting the entire dashboardData with just layoutConfig).
- Falls back to the panel ID in this case, which is what is needed for on Enterprise dashboard
mofojed
requested review from
a team,
bmingles and
Copilot
and removed request for
a team
|
ui docs preview (Available for 14 days) |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds per-user persistence for nested dashboards by saving/restoring the GoldenLayout layoutConfig plus associated widgetData, and updates nested layout behavior to avoid rebuilding layout structure when a persisted layout exists.
Changes:
- Persist nested dashboard
layoutConfigandwidgetDataviausePersistentState, including throttled widget data commits to prevent render loops. - Introduce
InitialLayoutConfigContextsoRow/Column/Stackcan skip creating GoldenLayout layout items when rehydrating. - Switch
UIComponentto useuseDhId()for stable IDs (instead of__dhIdfrom props).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/ui/src/js/src/widget/DocumentHandler.tsx | Expands debug logging to include initialData. |
| plugins/ui/src/js/src/UIComponent.tsx | Uses useDhId() to derive a stable component ID for persistence. |
| plugins/ui/src/js/src/layout/Stack.tsx | Wraps stack creation to skip layout-item creation when an initial layout is present. |
| plugins/ui/src/js/src/layout/Row.tsx | Skips row layout-item creation when an initial layout is present. |
| plugins/ui/src/js/src/layout/Column.tsx | Skips column layout-item creation when an initial layout is present. |
| plugins/ui/src/js/src/layout/NestedDashboardContent.tsx | Removes the separate nested dashboard content component (logic moved into NestedDashboard). |
| plugins/ui/src/js/src/layout/NestedDashboard.tsx | Adds persistence for layout + widget data, throttled widget data updates, and provides initial layout context. |
| plugins/ui/src/js/src/layout/InitialLayoutConfigContext.ts | New context/hook to expose whether a persisted initial layout config exists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onClose, | ||
| }: DocumentHandlerProps): JSX.Element { | ||
| log.debug('Rendering document', widget); | ||
| log.debug('Rendering document', widget, initialData); |
| @@ -0,0 +1,17 @@ | |||
| import { DashboardLayoutConfig } from '@deephaven/dashboard'; | |||
|
|
||
| /** | ||
| * Gets the initial layout config from the nearest context. | ||
| * @returns The initial layout config or null if not set |
| const { layoutConfig, widgetData } = dashboardData ?? {}; | ||
|
|
||
| // We want to know if the initial layoutConfig is set so we know if the dashboard has previously been loaded. | ||
| // User may have moved panels around, and we don't want the layout rows/columns to be blow away their changes |
Comment on lines
+47
to
+49
| // Don't add a row here | ||
| // eslint-disable-next-line react/jsx-no-useless-fragment | ||
| return <>{children}</>; |
Comment on lines
+52
to
+56
| if (initialLayoutConfig != null) { | ||
| // If there's already an initial layout defined, user has likely already customized their layout. | ||
| // Don't add a row here | ||
| // eslint-disable-next-line react/jsx-no-useless-fragment | ||
| return <>{children}</>; |
Comment on lines
+54
to
+57
| // Don't add a row here | ||
| const { children } = props; | ||
| // eslint-disable-next-line react/jsx-no-useless-fragment | ||
| return <>{children}</>; |
Comment on lines
+54
to
+113
| const [dashboardData, setDashboardData] = usePersistentState< | ||
| DashboardData | undefined | ||
| >(undefined, { type: 'NestedDashboardData', version: 1 }); | ||
| const [layoutInitialized, setLayoutInitialized] = useState(false); | ||
| const layoutSettings = useMemo( | ||
| () => ({ hasHeaders: showHeaders }), | ||
| [showHeaders] | ||
| ); | ||
| const { layoutConfig, widgetData } = dashboardData ?? {}; | ||
|
|
||
| // We want to know if the initial layoutConfig is set so we know if the dashboard has previously been loaded. | ||
| // User may have moved panels around, and we don't want the layout rows/columns to be blow away their changes | ||
| const [initialLayoutConfig] = useState(() => layoutConfig); | ||
| const [initialWidgetData] = useState(() => widgetData); | ||
|
|
||
| // Track the latest committed widgetData and any pending merged updates so | ||
| // we can throttle calls to setDashboardData and skip no-op updates that | ||
| // would otherwise re-render in a loop. | ||
| const lastWidgetDataRef = useRef<WidgetData | undefined>(initialWidgetData); | ||
| const pendingWidgetDataRef = useRef<WidgetData | undefined>(undefined); | ||
|
|
||
| const flushDataChange = useThrottledCallback( | ||
| () => { | ||
| const pending = pendingWidgetDataRef.current; | ||
| if (pending == null) { | ||
| return; | ||
| } | ||
| pendingWidgetDataRef.current = undefined; | ||
|
|
||
| const last = lastWidgetDataRef.current; | ||
| // Deep-equality check to avoid triggering a state update (and the | ||
| // re-render loop) when the merged widgetData hasn't actually changed. | ||
| if (last != null && JSON.stringify(last) === JSON.stringify(pending)) { | ||
| return; | ||
| } | ||
| lastWidgetDataRef.current = pending; | ||
| setDashboardData( | ||
| oldData => | ||
| ({ | ||
| ...oldData, | ||
| widgetData: pending, | ||
| }) as DashboardData | ||
| ); | ||
| }, | ||
| DATA_CHANGE_THROTTLE_MS, | ||
| { flushOnUnmount: true } | ||
| ); | ||
|
|
||
| useEffect(() => () => flushDataChange.flush(), [flushDataChange]); | ||
|
|
||
| const handleDataChange = useCallback( | ||
| (data: WidgetData) => { | ||
| // Accumulate partial updates between throttled flushes so we don't | ||
| // lose intermediate widget data changes. | ||
| const base = pendingWidgetDataRef.current ?? lastWidgetDataRef.current; | ||
| pendingWidgetDataRef.current = { ...base, ...data }; | ||
| flushDataChange(); | ||
| }, | ||
| [flushDataChange] | ||
| ); |
Comment on lines
+10
to
+13
| DashboardLayoutConfig, | ||
| Dashboard as DHCDashboard, | ||
| usePersistentState, | ||
| } from '@deephaven/dashboard'; |
|
ui docs preview (Available for 14 days) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.