Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: UI loading duplicate panels in embed iframe (#1043) #1082

Merged
merged 4 commits into from
Jan 13, 2025
Merged
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
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ RUN npm run build
# Now build the Python bundles
RUN find ./plugins -maxdepth 1 -mindepth 1 -type d -exec python3 -m build --wheel {} \;

FROM ghcr.io/deephaven/server:edge
FROM ghcr.io/deephaven/server:0.37.0
COPY --link --from=build /work/ /opt/deephaven/config/plugins/
RUN pip install /opt/deephaven/config/plugins/plugins/*/dist/*.whl

Expand Down
2 changes: 1 addition & 1 deletion plugins/ui/src/js/src/widget/DocumentUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function getRootChildren(
if (nonLayoutCount === childrenArray.length) {
// Just wrap it in a panel
return (
<ReactPanel key="root" title={widget.name ?? widget.id ?? widget.type}>
<ReactPanel title={widget.name ?? widget.id ?? widget.type}>
{children}
</ReactPanel>
);
Expand Down
34 changes: 29 additions & 5 deletions tests/app.d/ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ def ui_basic_component():
)


@ui.component
def ui_multi_panel_component():
return [
ui.panel(ui.button("Hello"), title="foo"),
ui.panel(ui.text("World"), title="bar"),
]


@ui.component
def ui_boom_component():
raise Exception("BOOM!")
Expand All @@ -40,7 +48,7 @@ def ui_cell(label: str = "Cell"):


@ui.component
def ui_cells():
def ui_cells_component():
id_iter, _ = ui.use_state(lambda: count())
cells, set_cells = ui.use_state(lambda: [next(id_iter)])

Expand All @@ -55,21 +63,37 @@ def delete_cell(delete_id: int):
lambda i: ui.flex(
ui_cell(label=f"Cell {i}"),
ui.action_button(
ui.icon("vsTrash"),
ui.icon("trash"),
aria_label="Delete cell",
on_press=lambda: delete_cell(i),
on_press=lambda _: delete_cell(i),
),
align_items="end",
key=str(i),
),
cells,
),
ui.action_button(ui.icon("vsAdd"), "Add cell", on_press=add_cell),
ui.action_button(ui.icon("add"), "Add cell", on_press=add_cell),
overflow="auto",
)


ui_component = ui_basic_component()
ui_multi_panel = ui_multi_panel_component()
ui_boom = ui_boom_component()
ui_boom_counter = ui_boom_counter_component()
ui_cells = ui_cells()
ui_cells = ui_cells_component()

ui_dashboard = ui.dashboard(
ui.column(
ui.stack(
ui.panel(ui_basic_component(), title="Component"),
ui.panel(ui_boom_counter_component(), title="Boom Counter"),
active_item_index=0,
height=75,
),
ui.row(
ui_multi_panel_component(),
height=25,
),
)
)
26 changes: 26 additions & 0 deletions tests/ui_embed_widget.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { expect, test } from '@playwright/test';
import { gotoPage, SELECTORS, waitForLoad } from './utils';

test('UI single panel loads in embed widget', async ({ page }) => {
await gotoPage(page, '/iframe/widget/?name=ui_component');
await expect(page.locator(SELECTORS.REACT_PANEL_VISIBLE)).toBeVisible();
await waitForLoad(page);
await expect(page).toHaveScreenshot();

Check failure on line 8 in tests/ui_embed_widget.spec.ts

View workflow job for this annotation

GitHub Actions / e2e-test

[chromium] › ui_embed_widget.spec.ts:4:5 › UI single panel loads in embed widget

1) [chromium] › ui_embed_widget.spec.ts:4:5 › UI single panel loads in embed widget ────────────── Error: Screenshot comparison failed: 5808 pixels (ratio 0.01 of all image pixels) are different. Expected: /work/tests/ui_embed_widget.spec.ts-snapshots/UI-single-panel-loads-in-embed-widget-1-chromium-linux.png Received: /work/test-results/ui_embed_widget-UI-single-panel-loads-in-embed-widget-chromium/UI-single-panel-loads-in-embed-widget-1-actual.png Diff: /work/test-results/ui_embed_widget-UI-single-panel-loads-in-embed-widget-chromium/UI-single-panel-loads-in-embed-widget-1-diff.png Call log: - expect.toHaveScreenshot with timeout 15000ms - verifying given screenshot expectation - taking page screenshot - disabled all CSS animations - waiting for fonts to load... - fonts loaded - 5808 pixels (ratio 0.01 of all image pixels) are different. - waiting 100ms before taking screenshot - taking page screenshot - disabled all CSS animations - waiting for fonts to load... - fonts loaded - captured a stable screenshot - 5808 pixels (ratio 0.01 of all image pixels) are different. 6 | await expect(page.locator(SELECTORS.REACT_PANEL_VISIBLE)).toBeVisible(); 7 | await waitForLoad(page); > 8 | await expect(page).toHaveScreenshot(); | ^ 9 | }); 10 | 11 | test('UI multi panel loads in embed widget', async ({ page }) => { at /work/tests/ui_embed_widget.spec.ts:8:22
});

test('UI multi panel loads in embed widget', async ({ page }) => {
await gotoPage(page, '/iframe/widget/?name=ui_multi_panel');
await expect(page.locator(SELECTORS.REACT_PANEL)).toHaveCount(2);
// Wait for the titles because embed-widget has a slight delay in showing the headers
await expect(page.getByText('foo')).toBeVisible();
await expect(page.getByText('bar')).toBeVisible();
await waitForLoad(page);
await expect(page).toHaveScreenshot();
});

test('UI dashboard loads in embed widget', async ({ page }) => {
await gotoPage(page, '/iframe/widget/?name=ui_dashboard');
await expect(page.locator(SELECTORS.REACT_PANEL)).toHaveCount(4);
await waitForLoad(page);
await expect(page).toHaveScreenshot();
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 10 additions & 2 deletions tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,18 @@ export async function gotoPage(
await page.goto(url, options);
await expect(
page.getByRole('progressbar', { name: 'Loading...', exact: true })
).not.toBeVisible();
).toHaveCount(0);
});
}

/**
* Waits for all loading spinners to disappear
* @param page The page
*/
export async function waitForLoad(page: Page): Promise<void> {
await expect(page.locator('.loading-spinner')).toHaveCount(0);
}

/**
* Opens a panel by clicking on the Panels button and then the panel button
* @param page The page
Expand Down Expand Up @@ -71,7 +79,7 @@ export async function openPanel(
// check for panel to be loaded
await expect(page.locator(panelLocator)).toHaveCount(panelCount + 1);
if (awaitLoad) {
await expect(page.locator('.loading-spinner')).toHaveCount(0);
await waitForLoad(page);
}
});
}
Loading