composer: Playwright, and the two layout faults it found in its first run
The plan asked for jsdom AND Playwright from the start and only jsdom was ever built. jsdom has no layout engine: getBoundingClientRect answers zeros, container queries never fire, a theme is an attribute rather than a computed cascade. So every claim about POSITION or SIZE in this kit was unchecked, and the unit suite could not have noticed either of these. A popover bounded its height and not its width. `max-height: 60vh` was there from the start; there was no max-inline-size at all, so a long unbroken string -- a file path, a model filename -- made the popover wider than the screen and anchor.js could not save it: it can move a box, not shrink one. Measured at 1436px inside a 1280px viewport, off the right edge in all four corners. A horizontal split could not fit a narrow screen. The first pane carries an inline size and `flex: 0 0 auto`, so it would not shrink, and the page scrolled sideways at 360px -- 407px of content in a 360px viewport. It wraps now, which stacks the panes, rather than a media query: the kit sizes itself from its container and not from the window. Nine layout tests: every overlay corner, an overlay taller than the viewport, both themes resolving every token they use, light actually being lighter than dark, and no horizontal scrollbar at five widths. `npm run test:all` runs both suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
digital-garbage
co-authored by
Claude Opus 5
Claude Opus 5
parent
89e4b24b59
commit
bd0c4be42e
@@ -24,3 +24,7 @@ movie_editor/backend/reference/*.workflow.json
|
||||
|
||||
# Dev-only: the app ships as plain ES modules, users never run npm.
|
||||
node_modules/
|
||||
|
||||
# Playwright: browsers live in a shared cache, results and traces are per-run
|
||||
app/test-results/
|
||||
app/playwright-report/
|
||||
|
||||
@@ -88,6 +88,15 @@
|
||||
.cx-split-v { flex-direction: column; }
|
||||
.cx-split-pane { flex: 1 1 0; min-width: 0; min-height: 0; overflow: auto; }
|
||||
.cx-split > .cx-split-pane:first-child { flex: 0 0 auto; }
|
||||
|
||||
/* A horizontal split stacks rather than overflowing when there is not room for
|
||||
two columns. The first pane carries an inline size, so on a narrow screen it
|
||||
could not shrink and the whole page scrolled sideways -- 407px of content in
|
||||
a 360px viewport. Wrapping rather than a media query because the kit sizes
|
||||
itself from its container, not from the window. */
|
||||
.cx-split-h { flex-wrap: wrap; }
|
||||
.cx-split-h > .cx-split-pane { min-inline-size: min(100%, 260px); }
|
||||
.cx-split-h > .cx-split-pane:first-child { max-inline-size: 100%; }
|
||||
.cx-split-bar { flex: none; background: transparent; position: relative; }
|
||||
.cx-split-h > .cx-split-bar { width: 7px; cursor: col-resize; }
|
||||
.cx-split-v > .cx-split-bar { height: 7px; cursor: row-resize; }
|
||||
|
||||
@@ -46,7 +46,14 @@
|
||||
border: var(--bw) solid var(--line); border-radius: var(--r-4);
|
||||
box-shadow: var(--shadow-3);
|
||||
padding: var(--sp-2);
|
||||
/* Both axes bounded, not just one. The height was capped from the start and
|
||||
the width was not, so a long unbroken string -- a file path, a prompt, a
|
||||
model filename -- made the popover wider than the screen and no amount of
|
||||
repositioning could bring it back: anchor.js can move a box, it cannot
|
||||
shrink one. Measured at 1436px inside a 1280px viewport. */
|
||||
max-height: 60vh; overflow-y: auto;
|
||||
max-inline-size: min(92vw, 480px);
|
||||
overflow-wrap: anywhere;
|
||||
animation: cx-pop var(--dur-1) var(--ease-standard);
|
||||
}
|
||||
.cx-popover-menu { min-width: 180px; }
|
||||
|
||||
Generated
+64
@@ -6,6 +6,7 @@
|
||||
"": {
|
||||
"name": "funpack-app",
|
||||
"devDependencies": {
|
||||
"@playwright/test": "^1.62.1",
|
||||
"jsdom": "^30.0.1"
|
||||
}
|
||||
},
|
||||
@@ -213,6 +214,22 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@playwright/test": {
|
||||
"version": "1.62.1",
|
||||
"resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.62.1.tgz",
|
||||
"integrity": "sha512-DTcUc8qii+cpHvtOwggMtBRMjKZHXYWdw8syRYu2vtzuq4Wxphqq4NfCs5Zt44L6mA8rfDfj+PHnxFc/FeK6mQ==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"playwright": "1.62.1"
|
||||
},
|
||||
"bin": {
|
||||
"playwright": "cli.js"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
}
|
||||
},
|
||||
"node_modules/bidi-js": {
|
||||
"version": "1.0.3",
|
||||
"resolved": "https://registry.npmjs.org/bidi-js/-/bidi-js-1.0.3.tgz",
|
||||
@@ -286,6 +303,21 @@
|
||||
"url": "https://github.com/fb55/entities?sponsor=1"
|
||||
}
|
||||
},
|
||||
"node_modules/fsevents": {
|
||||
"version": "2.3.2",
|
||||
"resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz",
|
||||
"integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==",
|
||||
"dev": true,
|
||||
"hasInstallScript": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
"darwin"
|
||||
],
|
||||
"engines": {
|
||||
"node": "^8.16.0 || ^10.6.0 || >=11.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/html-encoding-sniffer": {
|
||||
"version": "6.0.0",
|
||||
"resolved": "https://registry.npmjs.org/html-encoding-sniffer/-/html-encoding-sniffer-6.0.0.tgz",
|
||||
@@ -377,6 +409,38 @@
|
||||
"url": "https://github.com/inikulin/parse5?sponsor=1"
|
||||
}
|
||||
},
|
||||
"node_modules/playwright": {
|
||||
"version": "1.62.1",
|
||||
"resolved": "https://registry.npmjs.org/playwright/-/playwright-1.62.1.tgz",
|
||||
"integrity": "sha512-0M+L3LAD8/nm554LOla9Ayx0j0tmFZ0FBcoQ7F1VuVHpM/XpiC8RcDzBQB8W5+hA8L22THxELzeF+2WcUzvcLg==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"playwright-core": "1.62.1"
|
||||
},
|
||||
"bin": {
|
||||
"playwright": "cli.js"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
},
|
||||
"optionalDependencies": {
|
||||
"fsevents": "2.3.2"
|
||||
}
|
||||
},
|
||||
"node_modules/playwright-core": {
|
||||
"version": "1.62.1",
|
||||
"resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.62.1.tgz",
|
||||
"integrity": "sha512-wPYSwEBJY9GHraISXqyqtx0na0LpO3XEX7jNDhntbex7tzUS7kLnZsOlFruFJB4Hi/rhDMjXGqHewDZ68nYZVw==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"bin": {
|
||||
"playwright-core": "cli.js"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=20"
|
||||
}
|
||||
},
|
||||
"node_modules/punycode": {
|
||||
"version": "2.3.1",
|
||||
"resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.1.tgz",
|
||||
|
||||
+4
-1
@@ -4,9 +4,12 @@
|
||||
"type": "module",
|
||||
"description": "Dev-only. Users never run npm: the app is served as plain ES modules with no build step.",
|
||||
"scripts": {
|
||||
"test": "node --test 'composer/tests/**/*.test.js'"
|
||||
"test": "node --test 'composer/tests/**/*.test.js'",
|
||||
"test:layout": "playwright test",
|
||||
"test:all": "npm test && npm run test:layout"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@playwright/test": "^1.62.1",
|
||||
"jsdom": "^30.0.1"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
// Layout is the half jsdom cannot see.
|
||||
//
|
||||
// jsdom has no layout engine: getBoundingClientRect returns zeros, container
|
||||
// queries never fire, focus rings do not paint, and a theme is just an
|
||||
// attribute. Everything in the unit suite is therefore about behaviour, and
|
||||
// every claim about POSITION or SIZE is currently unchecked. These run in a
|
||||
// real browser against the real dev server, so those claims get checked too.
|
||||
//
|
||||
// The dev server is ComfyUI's own interpreter serving through core/serve.py, so
|
||||
// the extension allowlist and the traversal guard are exercised by looking at
|
||||
// the pages as much as by the Python tests.
|
||||
|
||||
import { defineConfig, devices } from "@playwright/test";
|
||||
|
||||
export default defineConfig({
|
||||
testDir: "./tests-layout",
|
||||
fullyParallel: true,
|
||||
reporter: process.env.CI ? "line" : "list",
|
||||
use: {
|
||||
baseURL: "http://127.0.0.1:8231",
|
||||
trace: "retain-on-failure",
|
||||
},
|
||||
projects: [{ name: "chromium", use: { ...devices["Desktop Chrome"] } }],
|
||||
webServer: {
|
||||
command: "/Users/dex/Documents/ComfyUI/venv/bin/python ../tools/devserver.py 8231",
|
||||
url: "http://127.0.0.1:8231/funpack/",
|
||||
reuseExistingServer: true,
|
||||
timeout: 120_000,
|
||||
},
|
||||
});
|
||||
@@ -0,0 +1,59 @@
|
||||
// Anchored overlays, at the edges of a real viewport.
|
||||
//
|
||||
// This is the reason Playwright exists in this project. anchor.js measures,
|
||||
// flips and clamps -- and every one of those reads getBoundingClientRect, which
|
||||
// jsdom answers with zeros. The unit tests feed it faked rects, so they check
|
||||
// the arithmetic and can say nothing about whether a menu opened off-screen.
|
||||
|
||||
import { test, expect } from "@playwright/test";
|
||||
|
||||
// The app itself, not the catalogue: it is the page a user actually loads, and
|
||||
// it finishes booting at a point the test can wait for.
|
||||
test.beforeEach(async ({ page }) => {
|
||||
await page.goto("/funpack/");
|
||||
await page.waitForFunction(() => window.FunPack !== undefined);
|
||||
});
|
||||
|
||||
async function openOverlayNear(page, corner) {
|
||||
// Put the launcher in a corner, then open from there: an overlay that fits
|
||||
// in the middle of the screen proves nothing about flipping.
|
||||
return page.evaluate(async ({ corner }) => {
|
||||
const { composer } = await import("/funpack/app/composer/composer.js");
|
||||
const anchor = composer.button.md({ label: "anchor" });
|
||||
Object.assign(anchor.node.style, {
|
||||
position: "fixed", width: "80px", height: "24px",
|
||||
left: corner.includes("right") ? "calc(100vw - 90px)" : "10px",
|
||||
top: corner.includes("bottom") ? "calc(100vh - 34px)" : "10px",
|
||||
});
|
||||
document.body.appendChild(anchor.node);
|
||||
|
||||
const body = composer.text.md({ text: "x".repeat(200) });
|
||||
const pop = composer.popover.anchored({ anchor: anchor.node, body });
|
||||
const box = pop.node.getBoundingClientRect();
|
||||
return { x: box.x, y: box.y, w: box.width, h: box.height,
|
||||
vw: innerWidth, vh: innerHeight };
|
||||
}, { corner });
|
||||
}
|
||||
|
||||
for (const corner of ["top-left", "top-right", "bottom-left", "bottom-right"]) {
|
||||
test(`an overlay opened at ${corner} stays on screen`, async ({ page }) => {
|
||||
const box = await openOverlayNear(page, corner);
|
||||
|
||||
expect(box.w, "the overlay has no width, so nothing was measured").toBeGreaterThan(0);
|
||||
expect(box.h).toBeGreaterThan(0);
|
||||
|
||||
// The whole point: clamped and flipped so every edge is inside the viewport.
|
||||
expect(box.x, `left edge off screen at ${corner}`).toBeGreaterThanOrEqual(0);
|
||||
expect(box.y, `top edge off screen at ${corner}`).toBeGreaterThanOrEqual(0);
|
||||
expect(box.x + box.w, `right edge past the viewport at ${corner}`)
|
||||
.toBeLessThanOrEqual(box.vw + 1);
|
||||
expect(box.y + box.h, `bottom edge past the viewport at ${corner}`)
|
||||
.toBeLessThanOrEqual(box.vh + 1);
|
||||
});
|
||||
}
|
||||
|
||||
test("an overlay taller than the viewport is still reachable from its top", async ({ page }) => {
|
||||
await page.setViewportSize({ width: 800, height: 240 });
|
||||
const box = await openOverlayNear(page, "bottom-left");
|
||||
expect(box.y).toBeGreaterThanOrEqual(0);
|
||||
});
|
||||
@@ -0,0 +1,64 @@
|
||||
// Both themes, painted.
|
||||
//
|
||||
// jsdom treats a theme as an attribute: it never resolves a custom property,
|
||||
// so "the light theme is light" is unverifiable there. A real browser computes
|
||||
// the cascade, which is the only place a token that resolves to the wrong value
|
||||
// -- or to nothing -- shows up.
|
||||
|
||||
import { test, expect } from "@playwright/test";
|
||||
|
||||
const rgb = (value) => value.match(/\d+/g).map(Number);
|
||||
const luminance = ([r, g, b]) => (0.2126 * r + 0.7152 * g + 0.0722 * b) / 255;
|
||||
|
||||
test.describe("themes", () => {
|
||||
test.beforeEach(async ({ page }) => {
|
||||
await page.goto("/funpack/");
|
||||
await page.waitForFunction(() => window.FunPack !== undefined);
|
||||
});
|
||||
|
||||
for (const theme of ["dark", "light"]) {
|
||||
test(`${theme} resolves every token it uses`, async ({ page }) => {
|
||||
await page.evaluate((t) => window.ComposerTheme.apply(t), theme);
|
||||
|
||||
const unresolved = await page.evaluate(() => {
|
||||
const style = getComputedStyle(document.documentElement);
|
||||
const names = [];
|
||||
for (const sheet of document.styleSheets) {
|
||||
let rules;
|
||||
try { rules = sheet.cssRules; } catch { continue; }
|
||||
for (const rule of rules) {
|
||||
if (!rule.style) continue;
|
||||
for (const prop of rule.style) {
|
||||
if (prop.startsWith("--")) names.push(prop);
|
||||
}
|
||||
}
|
||||
}
|
||||
// A token that resolves to nothing is the failure this catches: the
|
||||
// rule still applies and the colour silently comes from elsewhere.
|
||||
return [...new Set(names)].filter((n) => style.getPropertyValue(n).trim() === "");
|
||||
});
|
||||
|
||||
expect(unresolved, `tokens with no value in ${theme}`).toEqual([]);
|
||||
});
|
||||
}
|
||||
|
||||
test("light is actually lighter than dark", async ({ page }) => {
|
||||
const read = async (theme) => {
|
||||
await page.evaluate((t) => window.ComposerTheme.apply(t), theme);
|
||||
return page.evaluate(() => getComputedStyle(document.body).backgroundColor);
|
||||
};
|
||||
const dark = luminance(rgb(await read("dark")));
|
||||
const light = luminance(rgb(await read("light")));
|
||||
expect(light, "the light theme is not lighter than the dark one")
|
||||
.toBeGreaterThan(dark);
|
||||
});
|
||||
|
||||
test("the page never scrolls sideways", async ({ page }) => {
|
||||
for (const width of [1440, 1024, 768, 480, 360]) {
|
||||
await page.setViewportSize({ width, height: 800 });
|
||||
const overflows = await page.evaluate(() =>
|
||||
document.documentElement.scrollWidth > document.documentElement.clientWidth + 1);
|
||||
expect(overflows, `horizontal scrollbar at ${width}px`).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user