Skip to content

Commit

Permalink
Fix link style load timing issue in window popups (#994)
Browse files Browse the repository at this point in the history
* Update playwright

* Run e2e on preview build

* Add trackConsole

* Add a test

* Fix by waiting for link styles to load before rendering.

* Fix tests

* rush change

* Fix linter error

* Copy adoptedStyleSheets async.
  • Loading branch information
GerardasB committed Sep 4, 2024
1 parent 71f2d73 commit e4a5c8c
Show file tree
Hide file tree
Showing 25 changed files with 170 additions and 66 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Copyright (c) Bentley Systems, Incorporated. All rights reserved.
# Licensed under the MIT License. See LICENSE.md in the project root for license terms.
#----------------------------------------------------------------------------------------------
FROM mcr.microsoft.com/playwright:v1.41.2-jammy
FROM mcr.microsoft.com/playwright:v1.46.1-jammy

# Install rush
RUN npm install -g @microsoft/rush
Expand Down
10 changes: 10 additions & 0 deletions common/changes/@itwin/appui-react/fix-986_2024-09-03-19-38.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/appui-react",
"comment": "Fix link style load timing issue in window popups.",
"type": "none"
}
],
"packageName": "@itwin/appui-react"
}
26 changes: 13 additions & 13 deletions common/config/rush/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion e2e-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"playwright": "playwright"
},
"devDependencies": {
"@playwright/test": "^1.36.2",
"@playwright/test": "^1.46.1",
"appui-standalone-app": "workspace:*",
"tsx": "^4.7.1"
},
Expand Down
3 changes: 1 addition & 2 deletions e2e-tests/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ const config: PlaywrightTestConfig = {

/* Run your local dev server before starting the tests */
webServer: {
command:
"npm run --prefix ../test-apps/appui-test-app/standalone start:webserver",
command: "npm run --prefix ../test-apps/appui-test-app/standalone preview",
url: "http://localhost:3000/",
reuseExistingServer: !process.env.CI,
},
Expand Down
31 changes: 21 additions & 10 deletions e2e-tests/tests/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { BrowserContext, expect, Locator, Page } from "@playwright/test";
import {
BrowserContext,
ConsoleMessage,
expect,
Locator,
Page,
} from "@playwright/test";
import { PanelSide } from "../../ui/appui-react/src/appui-react/layout/widget-panels/PanelTypes";
import { WidgetState } from "../../ui/appui-react/src/appui-react/widgets/WidgetState";
import { StagePanelState } from "../../ui/appui-react/src/appui-react/stagepanels/StagePanelState";
Expand Down Expand Up @@ -248,14 +254,19 @@ export async function openComponentExamples(
await page.getByRole("menuitem", { name: "Component Examples" }).click();
}

export function trackWidgetLifecycle(page: Page, widgetId: string) {
const lifecycle = {
mountCount: 0,
unMountCount: 0,
};
page.on("console", (msg) => {
if (msg.text() === `Widget ${widgetId} mount`) lifecycle.mountCount++;
if (msg.text() === `Widget ${widgetId} unmount`) lifecycle.unMountCount++;
export function trackConsole<T = string>(
page: Page,
select?: (msg: ConsoleMessage) => Promise<T>
) {
const messages: T[] = [];
page.on("console", async (message) => {
if (select === undefined) {
messages.push(message.text() as T);
return;
}
const selected = await select(message);
if (selected === undefined) return;
messages.push(selected);
});
return lifecycle;
return messages as ReadonlyArray<T>;
}
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.
45 changes: 39 additions & 6 deletions e2e-tests/tests/popout-widget.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { test, expect, Locator } from "@playwright/test";
import { test, Locator, expect } from "@playwright/test";
import assert from "assert";
import {
WidgetState,
Expand All @@ -13,7 +13,7 @@ import {
popoutButtonLocator,
setWidgetState,
tabLocator,
trackWidgetLifecycle,
trackConsole,
widgetLocator,
} from "./Utils";

Expand Down Expand Up @@ -180,14 +180,15 @@ test.describe("popout widget", () => {
id,
});
await expect(widget).toBeVisible();
const widgetLifecycle = trackWidgetLifecycle(page, id);
const logs = trackConsole(page);
const popoutPage = await popoutWidget(widget);
await expect.poll(async () => popoutPage.isClosed()).toBe(false);
const text = popoutPage.getByText(id);
await expect(text).toBeVisible();

await popoutPage.close();

await expect.poll(async () => widgetLifecycle.mountCount).toBe(1);
await expect.poll(async () => widgetLifecycle.unMountCount).toBe(1);
await expect.poll(() => logs).toContain(`Widget ${id} mount`);
await expect.poll(() => logs).toContain(`Widget ${id} unmount`);
});
});

Expand Down Expand Up @@ -217,6 +218,38 @@ test("should copy shadow root styles", async ({ baseURL, page }) => {
await expect(borders).toHaveScreenshot();
});

test("should render after link styles are loaded", async ({
context,
page,
}) => {
context.route("**", (route) => route.continue());
await page.goto(`?frontstage=appui-test-app:TestPopout`);

const tab = tabLocator(page, "Widget 1");
const widget = widgetLocator({ tab });
await expect(widget).toBeVisible();

const logs = trackConsole<{
clientWidth: number;
clientHeight: number;
}>(page, async (msg) => {
if (msg.text().includes("LinkTest")) {
const val = await msg.args()[1].jsonValue();
return val;
}
});

await popoutWidget(widget);

await expect.poll(() => logs[0]).toBeTruthy();
expect(logs[0]).toEqual(
expect.objectContaining({
clientWidth: 10,
clientHeight: 15,
})
);
});

async function popoutWidget(widget: Locator) {
const context = widget.page().context();
const popoutButton = popoutButtonLocator(widget);
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
44 changes: 22 additions & 22 deletions e2e-tests/tests/widget-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { test, expect } from "@playwright/test";
import { expect, test } from "@playwright/test";
import assert from "assert";
import {
activeTabLocator,
Expand All @@ -14,7 +14,7 @@ import {
panelLocator,
setWidgetState,
tabLocator,
trackWidgetLifecycle,
trackConsole,
widgetLocator,
WidgetState,
} from "./Utils";
Expand Down Expand Up @@ -291,12 +291,13 @@ test.describe("widget state", () => {
});

test("should not mount unloaded widget", async ({ page }) => {
const widgetLifecycle = trackWidgetLifecycle(page, "WL-B");
const logs = trackConsole(page);

const tab = tabLocator(page, "WL-B");
await expect(tab).toBeHidden();

expect(widgetLifecycle.mountCount).toBe(0);
expect(widgetLifecycle.unMountCount).toBe(0);
expect(logs).not.toContain("Widget WL-B mount");
expect(logs).not.toContain("Widget WL-B unmount");
});
});

Expand All @@ -309,49 +310,48 @@ test.describe("widget lifecycle", () => {
});

test("should mount unloaded widget on open", async ({ page }) => {
const widgetLifecycle = trackWidgetLifecycle(page, "WL-B");
const logs = trackConsole(page);
await setWidgetState(page, "WL-B", WidgetState.Open);

expect(widgetLifecycle.mountCount).toBe(1);
expect(widgetLifecycle.unMountCount).toBe(0);
expect(logs).toContain("Widget WL-B mount");
expect(logs).not.toContain("Widget WL-B unmount");
});

test("should mount unloaded widget on close", async ({ page }) => {
const widgetLifecycle = trackWidgetLifecycle(page, "WL-B");
const logs = trackConsole(page);
await setWidgetState(page, "WL-B", WidgetState.Closed);

expect(widgetLifecycle.mountCount).toBe(1);
expect(widgetLifecycle.unMountCount).toBe(0);
expect(logs).toContain("Widget WL-B mount");
expect(logs).not.toContain("Widget WL-B unmount");
});

test("should mount unloaded widget on float", async ({ page }) => {
const widgetLifecycle = trackWidgetLifecycle(page, "WL-B");
const logs = trackConsole(page);
await setWidgetState(page, "WL-B", WidgetState.Floating);

expect(widgetLifecycle.mountCount).toBe(1);
expect(widgetLifecycle.unMountCount).toBe(0);
expect(logs).toContain("Widget WL-B mount");
expect(logs).not.toContain("Widget WL-B unmount");
});

test("should unmount and hide widget when unloading", async ({ page }) => {
const tab = tabLocator(page, "WL-A");
await expect(tab).toBeVisible({ timeout: 10000 });
const widgetLifecycle = trackWidgetLifecycle(page, "WL-A");
expect(widgetLifecycle.mountCount).toBe(0);
expect(widgetLifecycle.unMountCount).toBe(0);

const logs = trackConsole(page);
await setWidgetState(page, "WL-A", WidgetState.Unloaded);
await expect(tab).not.toBeVisible();
expect(widgetLifecycle.mountCount).toBe(0);
expect(widgetLifecycle.unMountCount).toBe(1);

expect(logs).not.toContain("Widget WL-A mount");
expect(logs).toContain("Widget WL-A unmount");
});

test("should mount and unmount unloaded widget", async ({ page }) => {
const widgetLifecycle = trackWidgetLifecycle(page, "WL-B");
const logs = trackConsole(page);
const tab = tabLocator(page, "WL-B");
await setWidgetState(page, "WL-B", WidgetState.Open);
await setWidgetState(page, "WL-B", WidgetState.Unloaded);
await expect(tab).toBeHidden();
expect(widgetLifecycle.mountCount).toBe(1);
expect(widgetLifecycle.unMountCount).toBe(1);
expect(logs).toContain("Widget WL-B mount");
expect(logs).toContain("Widget WL-B unmount");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ export function useWidgetDef(id: string) {

export function ToggleCameraItem() {
const activeViewport = useActiveViewport();
const [cameraOn, setCameraOn] = React.useState(activeViewport?.isCameraOn);
const [cameraOn, setCameraOn] = React.useState(
activeViewport ? activeViewport.isCameraOn : true
);

React.useEffect(() => {
return activeViewport?.onChangeView.addListener((vp) => {
Expand Down
1 change: 1 addition & 0 deletions test-apps/appui-test-app/standalone/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"build": "npm run build:backend && npm run build:frontend",
"build:backend": "tsc -p tsconfig.backend.json",
"build:frontend": "npm run copy:webfont && npm run pseudolocalize && vite build",
"preview": "vite preview --port 3000",
"clean": "rimraf lib build .rush/temp/package-deps*.json",
"lint": "eslint -f visualstudio \"./src/**/*.{ts,tsx}\" 1>&2",
"start": "run-p start:webserver start:electron",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,10 @@
height: 20px;
width: 100px;
}

#link-test {
width: 10px;
min-width: 10px;
height: 15px;
background: var(--iui-color-background-backdrop);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { createTestFrontstage } from "./createTestFrontstage";
import { ProgressRadial } from "@itwin/itwinui-react";
import { Logger } from "@itwin/core-bentley";
import { SampleAppIModelApp } from "../..";
import { loggerCategory } from "../../../common/TestAppConfiguration";

export const createTestPopoutFrontstage = () => {
{
Expand All @@ -30,6 +31,7 @@ export const createTestPopoutFrontstage = () => {
<div>Widget 1 content</div>
<div id="border-test" />
<FixedProgressRadial id="progress-radial" />
<LinkTest />
</>
),
},
Expand Down Expand Up @@ -91,3 +93,18 @@ function FixedProgressRadial(
}, []);
return <ProgressRadial key={key} ref={ref} {...props} />;
}

function LinkTest() {
const ref = React.useRef<HTMLDivElement>(null);
React.useEffect(() => {
const el = ref.current;
if (!el) return;
const clientWidth = el.clientWidth;
const clientHeight = el.clientHeight;
Logger.logInfo(loggerCategory, "LinkTest", {
clientWidth,
clientHeight,
});
}, []);
return <div ref={ref} id="link-test" style={{ background: "red" }} />;
}
Loading

0 comments on commit e4a5c8c

Please sign in to comment.