Skip to content

fix(ui): DH-19717: Fix deephaven.ui persistence in dashboards#1386

Merged
mofojed merged 8 commits into
deephaven:mainfrom
mofojed:bender_DH-19717-ui-persist-active-tabs
Jul 15, 2026
Merged

fix(ui): DH-19717: Fix deephaven.ui persistence in dashboards#1386
mofojed merged 8 commits into
deephaven:mainfrom
mofojed:bender_DH-19717-ui-persist-active-tabs

Conversation

@mofojed

@mofojed mofojed commented Jul 13, 2026

Copy link
Copy Markdown
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

mofojed added 6 commits July 10, 2026 13:33
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.
@mofojed
mofojed requested review from Copilot and vbabich July 13, 2026 19:03
@mofojed mofojed self-assigned this Jul 13, 2026
@mofojed
mofojed requested review from dgodinez-dh and removed request for vbabich July 13, 2026 19:04
@github-actions

Copy link
Copy Markdown

ui docs preview (Available for 14 days)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_state setters 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
dgodinez-dh previously approved these changes Jul 13, 2026

@dgodinez-dh dgodinez-dh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@github-actions

Copy link
Copy Markdown

ui docs preview (Available for 14 days)

@github-actions

Copy link
Copy Markdown

ui docs preview (Available for 14 days)

@mofojed
mofojed requested a review from dgodinez-dh July 15, 2026 15:02
@mofojed
mofojed merged commit 29d8f47 into deephaven:main Jul 15, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants