Skip to content

Commit 04899cb

Browse files
mgottschocodex
mgottscho
and
codex
authored
[web] Fix hosted viewer layout and close action (#32)
# Problem Hosted session and comparison banners could be auto-placed into an implicit second root-grid column, squeezing the viewer workspace into half of the browser width. The viewer header also exposed separate open-bundle and comparison actions instead of a single way to close the current design and start fresh. ### Before The hosted workspace is only 344 px wide in this 1280 x 720 viewport and begins at x=936, leaving most of the viewer blank. ![Before: hosted viewer squeezed into the right side of the page](https://raw.githubusercontent.com/xlsynth/nettle/3554500f0fa08142717120958b822134184a61fb/docs/images/pr-32/hosted-layout-before.jpg) # Solution - Pin the header, banner, landing content, workspace, and footer to the root grid's sole column while preserving the hosted-banner overlay and workspace offset. - Render the current bundle or comparison name as plain text and replace the header open/compare controls with one `Close design` action across bundle, comparison, and module-pairing views. - Reset active opening, bundle, comparison, hosted-session, dialog, and error state when closing. Hosted capability URLs push the splash route so browser Back restores the design. - Update unit and Playwright coverage for close behavior, hosted history restoration, and full-width viewer geometry. - Document in `AGENTS.md` that substantial GUI changes require before-and-after screenshots in their pull request descriptions. ### After The same design and viewport now use the full 1280 px workspace with no horizontal overflow, and the header exposes the new `Close design` action. ![After: hosted viewer fills the page and shows Close design](https://raw.githubusercontent.com/xlsynth/nettle/3554500f0fa08142717120958b822134184a61fb/docs/images/pr-32/hosted-layout-after.jpg) Validation: - `npm --workspace web run check` - `npm --workspace web run test` (39 files, 331 tests) - Hosted production build - Static production build - Playwright app suite (12 passed, 5 optional fixture skips) - `node scripts/run-netlist-e2e.mjs` - Manual browser inspection of the updated hosted build, including full-width geometry, close-to-splash behavior, and browser-Back restoration --------- Co-authored-by: Codex <noreply@openai.com>
1 parent 244773b commit 04899cb

14 files changed

Lines changed: 271 additions & 246 deletions

‎AGENTS.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,8 @@
1515
version tag alone is not an acceptable pin.
1616
- Keep dependency pins and their integrity metadata under version control, and
1717
update them deliberately in the same change that updates the dependency.
18+
19+
## GUI pull requests
20+
21+
- Whenever a pull request makes a substantial GUI change, include before-and-after
22+
screenshots in the pull request description.

‎docs/images/pr-32/hosted-layout-after.jpg‎

122 KB
Loading

‎docs/images/pr-32/hosted-layout-before.jpg‎

52.7 KB
Loading

‎web/e2e/app.spec.ts‎

Lines changed: 72 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,10 @@ const openFixture = async (page: Page) => {
195195
await expect(page.locator(".mode-badge.local")).toContainText("LOCAL");
196196
await expect(page.getByText("Bundle ready")).toBeVisible();
197197
await expect(page.getByText(/slang .*ready/i)).toHaveCount(0);
198-
await expect(page.getByRole("button", { name: "Open bundle" })).toContainText(
199-
"nettle-browser-fixture.nettle",
200-
);
198+
await expect(page.locator(".project-name")).toHaveText("nettle-browser-fixture.nettle");
199+
await expect(page.getByRole("button", { name: "Close design" })).toBeVisible();
200+
await expect(page.getByRole("button", { name: "Open bundle" })).toHaveCount(0);
201+
await expect(page.getByRole("button", { name: "Compare Nettle bundles" })).toHaveCount(0);
201202
};
202203

203204
const referenceSessionToken = "a".repeat(64);
@@ -342,10 +343,60 @@ test("uploads, reloads, and shares a bundle comparison", async ({ page }) => {
342343
await expect(page.locator(".mode-badge.diff").getByText("DIFF", { exact: true })).toBeVisible();
343344
await expect(page.getByText("Shareable comparison")).toBeVisible();
344345
await expectShareableDownloadLinks(page);
346+
const shellBox = await page.getByRole("application").boundingBox();
347+
const bannerBox = await page
348+
.getByRole("complementary", { name: "Comparison privacy information" })
349+
.boundingBox();
350+
const workspaceBox = await page.locator(".workspace").boundingBox();
351+
expect(shellBox).not.toBeNull();
352+
expect(bannerBox).not.toBeNull();
353+
expect(workspaceBox).not.toBeNull();
354+
if (shellBox && bannerBox && workspaceBox) {
355+
expect(Math.abs(bannerBox.x - shellBox.x)).toBeLessThan(2);
356+
expect(Math.abs(workspaceBox.x - shellBox.x)).toBeLessThan(2);
357+
expect(Math.abs(bannerBox.width - shellBox.width)).toBeLessThan(2);
358+
expect(Math.abs(workspaceBox.width - shellBox.width)).toBeLessThan(2);
359+
expect(workspaceBox.y).toBeGreaterThanOrEqual(bannerBox.y + bannerBox.height - 2);
360+
}
345361
expect(page.url()).toBe(comparisonUrl);
346362
expect(runtimeErrors).toEqual([]);
347363
});
348364

365+
test("keeps a hosted session full-width and restores it with Back after closing", async ({
366+
page,
367+
}) => {
368+
const runtimeErrors = captureRuntimeErrors(page);
369+
await installHostedComparisonApi(page);
370+
await page.goto(`/s/${referenceSessionToken}`);
371+
372+
await expect(page.getByText("Shareable session", { exact: true })).toBeVisible();
373+
await expect(page.getByRole("button", { name: "Close design" })).toBeVisible();
374+
const sessionUrl = page.url();
375+
const shellBox = await page.getByRole("application").boundingBox();
376+
const bannerBox = await page
377+
.getByRole("complementary", { name: "Shareable session information" })
378+
.boundingBox();
379+
const workspaceBox = await page.locator(".workspace").boundingBox();
380+
expect(shellBox).not.toBeNull();
381+
expect(bannerBox).not.toBeNull();
382+
expect(workspaceBox).not.toBeNull();
383+
if (shellBox && bannerBox && workspaceBox) {
384+
expect(Math.abs(bannerBox.x - shellBox.x)).toBeLessThan(2);
385+
expect(Math.abs(workspaceBox.x - shellBox.x)).toBeLessThan(2);
386+
expect(Math.abs(bannerBox.width - shellBox.width)).toBeLessThan(2);
387+
expect(Math.abs(workspaceBox.width - shellBox.width)).toBeLessThan(2);
388+
expect(workspaceBox.y).toBeGreaterThanOrEqual(bannerBox.y + bannerBox.height - 2);
389+
}
390+
391+
await page.getByRole("button", { name: "Close design" }).click();
392+
await expect(page).toHaveURL("/");
393+
await expect(page.getByRole("heading", { name: "Open a design" })).toBeVisible();
394+
await page.goBack();
395+
await expect(page).toHaveURL(sessionUrl);
396+
await expect(page.getByText("Shareable session", { exact: true })).toBeVisible();
397+
expect(runtimeErrors).toEqual([]);
398+
});
399+
349400
test("uploads and compares a bundle with a queued source build", async ({ page }) => {
350401
const runtimeErrors = captureRuntimeErrors(page);
351402
const uploadKinds = await installHostedComparisonApi(page, { queueSourceOnce: true });
@@ -515,7 +566,8 @@ test("automatically opens a bundle supplied by the viewer host", async ({ page }
515566

516567
await page.goto("/");
517568
await expect(page.locator(".mode-badge.local")).toContainText("LOCAL");
518-
await expect(page.getByRole("button", { name: "Open bundle" })).toContainText("startup.nettle");
569+
await expect(page.locator(".project-name")).toHaveText("startup.nettle");
570+
await expect(page.getByRole("button", { name: "Close design" })).toBeVisible();
519571
await expect(page.locator(".schematic-node.kind-module")).toHaveCount(1);
520572
expect(runtimeErrors).toEqual([]);
521573
});
@@ -557,9 +609,10 @@ test("automatically opens a comparison supplied by the viewer host", async ({ pa
557609
await page.goto("/");
558610

559611
await expect(page.locator(".mode-badge.diff").getByText("DIFF", { exact: true })).toBeVisible();
560-
await expect(page.getByRole("button", { name: "Open bundle" })).toContainText(
612+
await expect(page.locator(".project-name")).toHaveText(
561613
"hosted-reference.nettle → hosted-candidate.nettle",
562614
);
615+
await expect(page.getByRole("button", { name: "Close design" })).toBeVisible();
563616
await expect(page.getByLabel("Schematic matching policy")).toHaveValue("aggressive");
564617
await expect(page.locator(".node-interaction.diff-heuristic")).not.toHaveCount(0);
565618
await expect(page.getByRole("region", { name: "Read-only source diff" })).toBeVisible();
@@ -698,19 +751,15 @@ test("compares two bundles with source and schematic diff controls", async ({ pa
698751
await page.getByRole("button", { name: "Next schematic change" }).click();
699752
await expect(page.locator(".node-shape.selected, .schematic-edge.active")).not.toHaveCount(0);
700753

701-
const nodeCount = await page.locator(".node-interaction").count();
702-
await page.getByRole("button", { name: "Compare Nettle bundles" }).click();
703-
const replacement = page.getByRole("dialog", { name: "Compare Nettle bundles" });
704-
await replacement.getByLabel("Choose candidate .nettle bundle file").setInputFiles({
705-
name: "corrupt-candidate.nettle",
706-
mimeType: "application/zip",
707-
buffer: Buffer.from("not a bundle"),
708-
});
709-
await replacement.getByRole("button", { name: "Compare bundles", exact: true }).click();
710-
await expect(replacement.getByRole("alert")).toBeVisible();
711-
await replacement.getByRole("button", { name: "Close compare bundles dialog" }).click();
712-
await expect(page.locator(".mode-badge.diff")).toBeVisible();
713-
await expect(page.locator(".node-interaction")).toHaveCount(nodeCount);
754+
await expect(page.locator(".project-name")).toHaveText(
755+
"nettle-comparison-reference.nettle → nettle-comparison-candidate.nettle",
756+
);
757+
await expect(page.getByRole("button", { name: "Close design" })).toBeVisible();
758+
await expect(page.getByRole("button", { name: "Open bundle" })).toHaveCount(0);
759+
await expect(page.getByRole("button", { name: "Compare Nettle bundles" })).toHaveCount(0);
760+
await page.getByRole("button", { name: "Close design" }).click();
761+
await expect(page.getByRole("heading", { name: "Open a design" })).toBeVisible();
762+
await expect(page.locator(".mode-badge.diff")).toHaveCount(0);
714763
expect(runtimeErrors).toEqual([]);
715764
});
716765

@@ -970,23 +1019,14 @@ test("resizes the source pane and keeps schematic controls in view", async ({ pa
9701019
expect(runtimeErrors).toEqual([]);
9711020
});
9721021

973-
test("rejects an invalid replacement without discarding the active bundle", async ({ page }) => {
1022+
test("closes an active bundle and reopens from the landing page", async ({ page }) => {
9741023
const runtimeErrors = captureRuntimeErrors(page);
9751024
await openFixture(page);
9761025

977-
await page.getByRole("button", { name: "Open bundle" }).click();
978-
await page
979-
.getByRole("dialog", { name: "Open Nettle bundle" })
980-
.getByLabel("Choose a .nettle bundle")
981-
.setInputFiles({
982-
name: "broken.nettle",
983-
mimeType: "application/zip",
984-
buffer: Buffer.from("not a zip archive"),
985-
});
986-
await expect(
987-
page.getByRole("dialog", { name: "Open Nettle bundle" }).getByRole("alert"),
988-
).toContainText("end-of-central-directory");
989-
await page.getByRole("button", { name: "Close open bundle dialog" }).click();
1026+
await page.getByRole("button", { name: "Close design" }).click();
1027+
await expect(page.getByRole("heading", { name: "Open a design" })).toBeVisible();
1028+
await expect(page.locator(".schematic-node.kind-module")).toHaveCount(0);
1029+
await page.getByLabel("Open a .nettle bundle locally").setInputFiles(fixture);
9901030
await expect(page.locator(".schematic-node.kind-module")).toHaveCount(1);
9911031
expect(runtimeErrors).toEqual([]);
9921032
});

‎web/src/App.test.tsx‎

Lines changed: 60 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ vi.mock("./components/ComparisonWorkspaceView", async () => {
4343
reference,
4444
candidate,
4545
initialPolicy,
46-
onCompareBundles,
46+
onCloseDesign,
4747
hostedReference,
4848
hostedCandidate,
4949
shareableComparison,
@@ -54,7 +54,7 @@ vi.mock("./components/ComparisonWorkspaceView", async () => {
5454
reference: { provider: { marker: number } };
5555
candidate: { provider: { marker: number } };
5656
initialPolicy: string;
57-
onCompareBundles: () => void;
57+
onCloseDesign: () => void;
5858
hostedReference?: unknown;
5959
hostedCandidate?: unknown;
6060
shareableComparison?: boolean;
@@ -70,8 +70,8 @@ vi.mock("./components/ComparisonWorkspaceView", async () => {
7070
);
7171
return (
7272
<>
73-
<button type="button" onClick={onCompareBundles}>
74-
Compare Nettle bundles
73+
<button type="button" onClick={onCloseDesign}>
74+
Close design
7575
</button>
7676
<output data-testid="comparison-workspace">{mountedIdentity}</output>
7777
{shareableComparison ? (
@@ -340,7 +340,7 @@ describe("App comparison installation", () => {
340340
);
341341
});
342342

343-
it("persists a matching change made through the comparison dialog", async () => {
343+
it("closes a hosted comparison to the landing page and lets Back restore it", async () => {
344344
const referenceToken = "a".repeat(64);
345345
const candidateToken = "b".repeat(64);
346346
window.history.replaceState(
@@ -353,47 +353,22 @@ describe("App comparison installation", () => {
353353
await waitFor(() =>
354354
expect(screen.getByTestId("comparison-workspace").textContent).toBe("1:2:aggressive"),
355355
);
356-
fireEvent.click(screen.getByRole("button", { name: "Compare Nettle bundles" }));
357-
fireEvent.change(screen.getByLabelText(/Matching policy/), {
358-
target: { value: "conservative" },
359-
});
360-
fireEvent.click(screen.getByRole("button", { name: "Compare bundles" }));
361-
362-
await waitFor(() =>
363-
expect(screen.getByTestId("comparison-workspace").textContent).toBe("3:4:conservative"),
364-
);
365-
expect(window.location.pathname).toBe(`/compare/${referenceToken}/${candidateToken}`);
366-
expect(window.location.search).toBe("?matching=conservative");
367-
});
356+
fireEvent.click(screen.getByRole("button", { name: "Close design" }));
368357

369-
it("keeps a hosted comparison shareable when its sides are swapped", async () => {
370-
const referenceToken = "a".repeat(64);
371-
const candidateToken = "b".repeat(64);
372-
window.history.replaceState(
373-
null,
374-
"",
375-
`/compare/${referenceToken}/${candidateToken}?matching=conservative&referenceModule=reference_top&candidateModule=candidate_top`,
376-
);
377-
render(<App />);
378-
379-
await waitFor(() =>
380-
expect(screen.getByTestId("comparison-workspace").textContent).toBe("1:2:conservative"),
381-
);
382-
fireEvent.click(screen.getByRole("button", { name: "Compare Nettle bundles" }));
383-
fireEvent.click(screen.getByRole("button", { name: "Swap reference and candidate bundles" }));
384-
fireEvent.click(screen.getByRole("button", { name: "Compare bundles" }));
358+
expect(window.location.pathname).toBe("/");
359+
expect(screen.getByRole("heading", { name: "Open a design" })).toBeTruthy();
360+
expect(screen.queryByRole("button", { name: "Close design" })).toBeNull();
385361

362+
const restored = new Promise<void>((resolve) => {
363+
window.addEventListener("popstate", () => resolve(), { once: true });
364+
});
365+
window.history.back();
366+
await restored;
386367
await waitFor(() =>
387-
expect(screen.getByTestId("comparison-workspace").textContent).toBe("3:4:conservative"),
388-
);
389-
expect(screen.getByTestId("shareable-comparison").textContent).toBe("shareable");
390-
expect(window.location.pathname).toBe(`/compare/${candidateToken}/${referenceToken}`);
391-
expect(new URLSearchParams(window.location.search).get("referenceModule")).toBe(
392-
"candidate_top",
393-
);
394-
expect(new URLSearchParams(window.location.search).get("candidateModule")).toBe(
395-
"reference_top",
368+
expect(screen.getByTestId("comparison-workspace").textContent).toBe("3:4:aggressive"),
396369
);
370+
expect(window.location.pathname).toBe(`/compare/${referenceToken}/${candidateToken}`);
371+
expect(window.location.search).toBe("?matching=aggressive");
397372
});
398373

399374
it("restarts an in-flight direct comparison after matching-only navigation", async () => {
@@ -579,34 +554,28 @@ describe("App comparison installation", () => {
579554
expect(screen.getByText("SHAREABLE")).toBeTruthy();
580555
});
581556

582-
it("reuses a ready hosted bundle as the reference while keeping the candidate local", async () => {
557+
it("closes a hosted session to the landing page and lets Back restore it", async () => {
583558
const token = "a".repeat(64);
584559
window.history.replaceState(null, "", `/s/${token}`);
585560
render(<App />);
586561

587562
await waitFor(() => expect(screen.getByText("SHAREABLE")).toBeTruthy());
588563
expect(harness.getHostedSessionStatus).toHaveBeenCalledOnce();
589564
expect(harness.loadHostedBundle).toHaveBeenCalledOnce();
590-
fireEvent.click(screen.getByRole("button", { name: "Compare Nettle bundles" }));
565+
fireEvent.click(screen.getByRole("button", { name: "Close design" }));
591566

592-
const dialog = screen.getByRole("dialog", {
593-
name: "Compare Nettle bundles",
594-
});
595-
expect(within(dialog).getByText("design.nettle")).toBeTruthy();
596-
expect(within(dialog).getByText(/Reference already has a shareable URL/)).toBeTruthy();
597-
expect(within(dialog).getByText(/Any local bundle stays in this browser/)).toBeTruthy();
598-
fireEvent.change(within(dialog).getByLabelText("Choose candidate .nettle bundle file"), {
599-
target: { files: [new File(["candidate"], "candidate.nettle")] },
600-
});
601-
fireEvent.click(within(dialog).getByRole("button", { name: "Compare bundles" }));
567+
expect(window.location.pathname).toBe("/");
568+
expect(screen.getByRole("heading", { name: "Open a design" })).toBeTruthy();
602569

603-
await waitFor(() =>
604-
expect(screen.getByTestId("comparison-workspace").textContent).toBe("2:3:conservative"),
605-
);
606-
expect(screen.getByTestId("hosted-comparison-origin").textContent).toBe("reference");
607-
expect(harness.getHostedSessionStatus).toHaveBeenCalledOnce();
608-
expect(harness.loadHostedBundle).toHaveBeenCalledOnce();
609-
expect(harness.createHostedSession).not.toHaveBeenCalled();
570+
const restored = new Promise<void>((resolve) => {
571+
window.addEventListener("popstate", () => resolve(), { once: true });
572+
});
573+
window.history.back();
574+
await restored;
575+
await waitFor(() => expect(screen.getByText("SHAREABLE")).toBeTruthy());
576+
expect(window.location.pathname).toBe(`/s/${token}`);
577+
expect(harness.getHostedSessionStatus).toHaveBeenCalledTimes(2);
578+
expect(harness.loadHostedBundle).toHaveBeenCalledTimes(2);
610579
});
611580

612581
it("opens a local bundle without contacting hosted APIs", async () => {
@@ -626,6 +595,22 @@ describe("App comparison installation", () => {
626595
expect(harness.open).toHaveBeenCalledOnce();
627596
});
628597

598+
it("closes a local static bundle back to the static landing page", async () => {
599+
render(<App mode="static" />);
600+
fireEvent.change(screen.getByLabelText("Open a .nettle bundle locally"), {
601+
target: {
602+
files: [new File(["bundle"], "local.nettle", { type: "application/zip" })],
603+
},
604+
});
605+
606+
await waitFor(() => expect(screen.getByText("LOCAL · NOT UPLOADED")).toBeTruthy());
607+
fireEvent.click(screen.getByRole("button", { name: "Close design" }));
608+
609+
expect(screen.getByRole("heading", { name: "Open a design" })).toBeTruthy();
610+
expect(screen.getByText("Static mode")).toBeTruthy();
611+
expect(screen.queryByRole("button", { name: "Close design" })).toBeNull();
612+
});
613+
629614
it("keeps the landing action loading while launching a local bundle", async () => {
630615
const provider = deferred<{
631616
fileName: string;
@@ -797,7 +782,7 @@ describe("App comparison installation", () => {
797782
expect(screen.getByRole("dialog", { name: "Compare Nettle bundles" })).toBeTruthy();
798783
});
799784

800-
it("remounts a replacement with the same filenames and snapshot IDs", async () => {
785+
it("closes a comparison before starting a fresh comparison with the same filenames", async () => {
801786
render(<App />);
802787
fireEvent.click(
803788
screen.getByRole("button", {
@@ -822,7 +807,19 @@ describe("App comparison installation", () => {
822807
expect(screen.getByTestId("comparison-workspace").textContent).toBe("1:2:conservative"),
823808
);
824809

825-
fireEvent.click(screen.getByRole("button", { name: "Compare Nettle bundles" }));
810+
fireEvent.click(screen.getByRole("button", { name: "Close design" }));
811+
expect(screen.getByRole("heading", { name: "Open a design" })).toBeTruthy();
812+
fireEvent.click(
813+
screen.getByRole("button", {
814+
name: "Open and compare two .nettle bundles",
815+
}),
816+
);
817+
fireEvent.change(screen.getByLabelText("Choose reference .nettle bundle file"), {
818+
target: { files: [reference] },
819+
});
820+
fireEvent.change(screen.getByLabelText("Choose candidate .nettle bundle file"), {
821+
target: { files: [candidate] },
822+
});
826823
fireEvent.change(screen.getByLabelText(/Matching policy/), {
827824
target: { value: "aggressive" },
828825
});

0 commit comments

Comments
 (0)