fix(ui): DH-19717: Fix deephaven.ui persistence in dashboards#1386
Merged
mofojed merged 8 commits intoJul 15, 2026
Conversation
mofojed
commented
Jul 13, 2026
Member
- Followup to feat: DH-19717: Add per-user persistence for dashboards #1352 which added persistence for ui.dashboards.
- test: add dashboard persistence e2e tests (DH-19717)
- fix(ui): persist active tab in ui.tabs across refresh (DH-19717)
- fix(ui): persist active tab in a nested dashboard stack (DH-19717)
- fix(ui): don't drop debounced input changes on server re-render (DH-19717)
- fix(ui): return a stable set_state function from use_state (DH-19717)
- fix(ui): persist state for all panels in a nested dashboard (DH-19717)
- Tested against the example that failed testing in Enterprise
Adds e2e tests and fixtures covering dashboard/layout persistence across a page refresh: input values, moved panels, active tab in a stack, active tab in ui.tabs, and custom columns on ui.table.
Store the selected tab in the panel's persistent state via usePersistentState so it is restored on rehydration. When the server controls the selection (selectedKey provided) it remains the source of truth.
On rehydration, panels remount before their portals are registered, so openedMetadataRef starts undefined and the panel-open effect called setActiveContentItem for every panel (last one winning), clobbering the active tab restored from the persisted layout. Only bring a panel to the front when its metadata actually changed from a previously-seen value.
…9717) useDebouncedCallback cancels its pending call whenever the callback reference changes. Because the server hands each input a new onChange callable on every re-render, editing one field and then another in quick succession caused the first field's re-render to cancel the second field's still-pending debounced update - so that value never reached the server and was lost on refresh. Hold propOnChange in a ref so the debounced callback stays stable across re-renders.
use_state created a new setter closure on every render. Callables are serialized to the client by object identity, so a fresh setter each render was assigned a new callable id - the client received a new onChange (etc.) prop on every render. This churned props and, for debounced inputs, caused a re-render triggered by one field to cancel another field's pending (debounced) change, losing it before it reached the server. Cache the setter per hook index in RenderContext so the same instance is returned across renders and serializes to a stable callable id. This supersedes the useDebouncedOnChange workaround, which is reverted.
usePanelManager.handleDataChange merged each panel's update into the initial widgetData, which is never updated. Sequential updates from different panels therefore each carried only their own state (plus the stale initial), so the last panel to update overwrote the others - only its state was persisted. Accumulate the latest panel states in a ref so every panel's state is retained. Also update the open/close path to emit the accumulated states. Extends the e2e custom-columns test to add and verify custom columns on both tables in the stack, and adds a usePanelManager unit test covering sequential multi-panel updates.
|
ui docs preview (Available for 14 days) |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes deephaven.ui dashboard state restoration across refreshes.
Changes:
- Persists tabs, nested panel layouts, active panels, and per-panel state.
- Stabilizes
use_statesetters to prevent dropped debounced input updates. - Adds unit and end-to-end persistence coverage.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/ui_dashboard_persistence.spec.ts |
Adds dashboard persistence E2E tests. |
tests/app.d/ui_dashboard_persistence.py |
Provides E2E dashboard fixtures. |
tests/app.d/tests.app |
Registers the persistence fixtures. |
plugins/ui/test/deephaven/ui/hooks/test_state.py |
Tests stable state-setter identity. |
plugins/ui/src/js/src/layout/usePanelManager.ts |
Accumulates state across all panels. |
plugins/ui/src/js/src/layout/usePanelManager.test.ts |
Tests multi-panel state accumulation. |
plugins/ui/src/js/src/layout/ReactPanel.tsx |
Preserves the active nested-dashboard panel during rehydration. |
plugins/ui/src/js/src/elements/Tabs.tsx |
Persists uncontrolled tab selection. |
plugins/ui/src/deephaven/ui/hooks/use_state.py |
Uses cached state setters. |
plugins/ui/src/deephaven/ui/_internal/RenderContext.py |
Implements stable setter caching and cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dgodinez-dh
previously approved these changes
Jul 13, 2026
dgodinez-dh
left a comment
Contributor
There was a problem hiding this comment.
Changes look good. Looks like some TS issues Tabs selectionProps and one failing chromium e2e test that passed for firefox and webkit. But I figure those should be small fixes.
usePersistentState was typed with React's Key (which includes bigint), but the Spectrum Tabs selectedKey/defaultSelectedKey only accept string | number, breaking the type check. Derive the key type from the component props.
|
ui docs preview (Available for 14 days) |
…rsist-active-tabs
|
ui docs preview (Available for 14 days) |
dgodinez-dh
approved these changes
Jul 15, 2026
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.