Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions plugins/ui/src/js/src/layout/Column.test.tsx
Original file line number Diff line number Diff line change
@@ -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 }) => (
<div data-testid="flex">{children}</div>
),
}));

// 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(
<ReactPanelContext.Provider value="test-panel-id">
<InitialLayoutConfigContext.Provider value={mockLayoutConfig}>
<Column width={100}>hello</Column>
</InitialLayoutConfigContext.Provider>
</ReactPanelContext.Provider>
);

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(
<ReactPanelContext.Provider value="test-panel-id">
<Column width={100}>hello</Column>
</ReactPanelContext.Provider>
);

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(
<InitialLayoutConfigContext.Provider value={mockLayoutConfig}>
<Column width={100}>hello</Column>
</InitialLayoutConfigContext.Provider>
);

expect(screen.queryByTestId('flex')).not.toBeInTheDocument();
expect(wrapBareChildrenInPanel).toHaveBeenCalled();
});
22 changes: 13 additions & 9 deletions plugins/ui/src/js/src/layout/Column.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Flex width={`${width}%`} direction="column">
{children}
</Flex>
);
}

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.
Expand All @@ -59,15 +71,7 @@ function Column({ children, width }: ColumnElementProps): JSX.Element {
return <>{wrapBareChildrenInPanel(children)}</>;
}

if (panelId == null) {
return <LayoutColumn width={width}>{children}</LayoutColumn>;
}

return (
<Flex width={`${width}%`} direction="column">
{children}
</Flex>
);
return <LayoutColumn width={width}>{children}</LayoutColumn>;
}

export default Column;
69 changes: 69 additions & 0 deletions plugins/ui/src/js/src/layout/Row.test.tsx
Original file line number Diff line number Diff line change
@@ -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 }) => (
<div data-testid="flex">{children}</div>
),
}));

// 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(
<ReactPanelContext.Provider value="test-panel-id">
<InitialLayoutConfigContext.Provider value={mockLayoutConfig}>
<Row height={100}>hello</Row>
</InitialLayoutConfigContext.Provider>
</ReactPanelContext.Provider>
);

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(
<ReactPanelContext.Provider value="test-panel-id">
<Row height={100}>hello</Row>
</ReactPanelContext.Provider>
);

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(
<InitialLayoutConfigContext.Provider value={mockLayoutConfig}>
<Row height={100}>hello</Row>
</InitialLayoutConfigContext.Provider>
);

expect(screen.queryByTestId('flex')).not.toBeInTheDocument();
expect(wrapBareChildrenInPanel).toHaveBeenCalled();
});
22 changes: 13 additions & 9 deletions plugins/ui/src/js/src/layout/Row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Flex height={`${height}%`} direction="row">
{children}
</Flex>
);
}

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.
Expand All @@ -55,15 +67,7 @@ function Row({ children, height }: RowElementProps): JSX.Element {
return <>{wrapBareChildrenInPanel(children)}</>;
}

if (panelId == null) {
return <LayoutRow height={height}>{children}</LayoutRow>;
}

return (
<Flex height={`${height}%`} direction="row">
{children}
</Flex>
);
return <LayoutRow height={height}>{children}</LayoutRow>;
}

export default Row;
Loading