From 3e21e11a192d7a573a32463e719c7977e34599a1 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Fri, 7 Aug 2026 13:24:03 -0600 Subject: [PATCH 1/2] fix: DH-23205: Don't wrap children in a panel when when already inside a panel --- plugins/ui/src/js/src/layout/Column.tsx | 22 +++++++++++++--------- plugins/ui/src/js/src/layout/Row.tsx | 22 +++++++++++++--------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/plugins/ui/src/js/src/layout/Column.tsx b/plugins/ui/src/js/src/layout/Column.tsx index fa5b7a74e..eab2ef495 100644 --- a/plugins/ui/src/js/src/layout/Column.tsx +++ b/plugins/ui/src/js/src/layout/Column.tsx @@ -50,6 +50,18 @@ function LayoutColumn({ function Column({ children, width }: ColumnElementProps): JSX.Element { const panelId = usePanelId(); const initialLayoutConfig = useInitialLayoutConfig(); + + if (panelId != null) { + // We're inside a panel (e.g. ui.panel(ui.column(...))), so render as a Flex + // regardless of any persisted layout. Wrapping bare children in a panel here + // would nest a panel inside a panel and throw a NestedPanelError. + return ( + + {children} + + ); + } + if (initialLayoutConfig != null) { // If there's already an initial layout defined, user has likely already customized their layout. // Don't add a column here, or normalize the children which might add a stack unnecessarily. @@ -59,15 +71,7 @@ function Column({ children, width }: ColumnElementProps): JSX.Element { return <>{wrapBareChildrenInPanel(children)}; } - if (panelId == null) { - return {children}; - } - - return ( - - {children} - - ); + return {children}; } export default Column; diff --git a/plugins/ui/src/js/src/layout/Row.tsx b/plugins/ui/src/js/src/layout/Row.tsx index d318dac19..c6ab42329 100644 --- a/plugins/ui/src/js/src/layout/Row.tsx +++ b/plugins/ui/src/js/src/layout/Row.tsx @@ -46,6 +46,18 @@ function LayoutRow({ children, height }: RowElementProps): JSX.Element | null { function Row({ children, height }: RowElementProps): JSX.Element { const panelId = usePanelId(); const initialLayoutConfig = useInitialLayoutConfig(); + + if (panelId != null) { + // We're inside a panel (e.g. ui.panel(ui.row(...))), so render as a Flex + // regardless of any persisted layout. Wrapping bare children in a panel here + // would nest a panel inside a panel and throw a NestedPanelError. + return ( + + {children} + + ); + } + if (initialLayoutConfig != null) { // If there's already an initial layout defined, user has likely already customized their layout. // Don't add a row here, or normalize the children which might add a stack unnecessarily. @@ -55,15 +67,7 @@ function Row({ children, height }: RowElementProps): JSX.Element { return <>{wrapBareChildrenInPanel(children)}; } - if (panelId == null) { - return {children}; - } - - return ( - - {children} - - ); + return {children}; } export default Row; From e47d5f75f77e56bacd2b02912ac1cad8cc73b074 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Fri, 7 Aug 2026 13:24:24 -0600 Subject: [PATCH 2/2] Unit tests --- plugins/ui/src/js/src/layout/Column.test.tsx | 69 ++++++++++++++++++++ plugins/ui/src/js/src/layout/Row.test.tsx | 69 ++++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 plugins/ui/src/js/src/layout/Column.test.tsx create mode 100644 plugins/ui/src/js/src/layout/Row.test.tsx diff --git a/plugins/ui/src/js/src/layout/Column.test.tsx b/plugins/ui/src/js/src/layout/Column.test.tsx new file mode 100644 index 000000000..5ad13b7bc --- /dev/null +++ b/plugins/ui/src/js/src/layout/Column.test.tsx @@ -0,0 +1,69 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import type { DashboardLayoutConfig } from '@deephaven/dashboard'; +import Column from './Column'; +import { ReactPanelContext } from './ReactPanelContext'; +import { InitialLayoutConfigContext } from './InitialLayoutConfigContext'; +import { wrapBareChildrenInPanel } from './LayoutUtils'; + +// Mock Flex so we can detect the "inside a panel" branch without pulling in the +// full Spectrum provider stack. +jest.mock('@deephaven/components', () => ({ + ...jest.requireActual('@deephaven/components'), + Flex: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), +})); + +// Spy on wrapBareChildrenInPanel so we can assert the rehydration branch is not +// taken when we're inside a panel. +jest.mock('./LayoutUtils', () => ({ + ...jest.requireActual('./LayoutUtils'), + wrapBareChildrenInPanel: jest.fn((children: React.ReactNode) => children), +})); + +const mockLayoutConfig = [ + { type: 'row', content: [] }, +] as unknown as DashboardLayoutConfig; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +it('renders as a Flex when inside a panel, even during rehydration (DH-23205)', () => { + // Regression: a ui.column used as the content of a ui.panel must render as a + // Flex, not wrap its bare children in a nested panel, which would throw a + // NestedPanelError. This must hold even when an initial layout config exists. + render( + + + hello + + + ); + + expect(screen.getByTestId('flex')).toHaveTextContent('hello'); + expect(wrapBareChildrenInPanel).not.toHaveBeenCalled(); +}); + +it('renders as a Flex when inside a panel with no initial layout config', () => { + render( + + hello + + ); + + expect(screen.getByTestId('flex')).toHaveTextContent('hello'); + expect(wrapBareChildrenInPanel).not.toHaveBeenCalled(); +}); + +it('wraps bare children in a panel during rehydration when not inside a panel', () => { + render( + + hello + + ); + + expect(screen.queryByTestId('flex')).not.toBeInTheDocument(); + expect(wrapBareChildrenInPanel).toHaveBeenCalled(); +}); diff --git a/plugins/ui/src/js/src/layout/Row.test.tsx b/plugins/ui/src/js/src/layout/Row.test.tsx new file mode 100644 index 000000000..afa4d437d --- /dev/null +++ b/plugins/ui/src/js/src/layout/Row.test.tsx @@ -0,0 +1,69 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import type { DashboardLayoutConfig } from '@deephaven/dashboard'; +import Row from './Row'; +import { ReactPanelContext } from './ReactPanelContext'; +import { InitialLayoutConfigContext } from './InitialLayoutConfigContext'; +import { wrapBareChildrenInPanel } from './LayoutUtils'; + +// Mock Flex so we can detect the "inside a panel" branch without pulling in the +// full Spectrum provider stack. +jest.mock('@deephaven/components', () => ({ + ...jest.requireActual('@deephaven/components'), + Flex: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), +})); + +// Spy on wrapBareChildrenInPanel so we can assert the rehydration branch is not +// taken when we're inside a panel. +jest.mock('./LayoutUtils', () => ({ + ...jest.requireActual('./LayoutUtils'), + wrapBareChildrenInPanel: jest.fn((children: React.ReactNode) => children), +})); + +const mockLayoutConfig = [ + { type: 'row', content: [] }, +] as unknown as DashboardLayoutConfig; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +it('renders as a Flex when inside a panel, even during rehydration (DH-23205)', () => { + // Regression: a ui.row used as the content of a ui.panel must render as a + // Flex, not wrap its bare children in a nested panel, which would throw a + // NestedPanelError. This must hold even when an initial layout config exists. + render( + + + hello + + + ); + + expect(screen.getByTestId('flex')).toHaveTextContent('hello'); + expect(wrapBareChildrenInPanel).not.toHaveBeenCalled(); +}); + +it('renders as a Flex when inside a panel with no initial layout config', () => { + render( + + hello + + ); + + expect(screen.getByTestId('flex')).toHaveTextContent('hello'); + expect(wrapBareChildrenInPanel).not.toHaveBeenCalled(); +}); + +it('wraps bare children in a panel during rehydration when not inside a panel', () => { + render( + + hello + + ); + + expect(screen.queryByTestId('flex')).not.toBeInTheDocument(); + expect(wrapBareChildrenInPanel).toHaveBeenCalled(); +});