diff --git a/templates/design/actions/get-design.test.ts b/templates/design/actions/get-design.test.ts index a942a2a219..2087d21390 100644 --- a/templates/design/actions/get-design.test.ts +++ b/templates/design/actions/get-design.test.ts @@ -4,10 +4,13 @@ const mocks = vi.hoisted(() => { const selectChain = { from: vi.fn(), where: vi.fn(), + orderBy: vi.fn(), }; selectChain.from.mockReturnValue(selectChain); + selectChain.where.mockReturnValue(selectChain); return { + asc: vi.fn((column) => ({ asc: column })), eq: vi.fn((left, right) => ({ left, right })), getDb: vi.fn(() => ({ select: vi.fn(() => selectChain), @@ -23,6 +26,7 @@ vi.mock("@agent-native/core/sharing", () => ({ })); vi.mock("drizzle-orm", () => ({ + asc: mocks.asc, eq: mocks.eq, sql: vi.fn((strings, ...values) => ({ strings, values })), })); @@ -30,7 +34,11 @@ vi.mock("drizzle-orm", () => ({ vi.mock("../server/db/index.js", () => ({ getDb: mocks.getDb, schema: { - designFiles: { designId: "designFiles.designId" }, + designFiles: { + id: "designFiles.id", + designId: "designFiles.designId", + createdAt: "designFiles.createdAt", + }, }, })); @@ -40,7 +48,8 @@ import action from "./get-design.js"; describe("get-design", () => { beforeEach(() => { mocks.resolveAccess.mockReset(); - mocks.selectChain.where.mockReset(); + mocks.selectChain.orderBy.mockReset(); + mocks.asc.mockClear(); mocks.resolveAccess.mockResolvedValue({ role: "viewer", resource: { @@ -65,7 +74,7 @@ describe("get-design", () => { updatedAt: "2026-06-29T00:00:00.000Z", }, }); - mocks.selectChain.where.mockResolvedValue([ + mocks.selectChain.orderBy.mockResolvedValue([ { id: "file_123", filename: "index.html", @@ -107,6 +116,17 @@ describe("get-design", () => { expect(result.data).not.toContain("example-private-bridge-token"); }); + it("returns files in a stable order so a design lays itself out the same way twice", async () => { + // Heap order is not stable across writes, and this array feeds the overview + // screen stack plus each screen's index within its layout group. + await action.run({ id: "design_123" }); + + expect(mocks.selectChain.orderBy).toHaveBeenCalledWith( + { asc: "designFiles.createdAt" }, + { asc: "designFiles.id" }, + ); + }); + it("returns only the read-only preview token to an editor", async () => { mocks.resolveAccess.mockResolvedValueOnce({ role: "editor", diff --git a/templates/design/actions/get-design.ts b/templates/design/actions/get-design.ts index e140d3fea4..127111c0bd 100644 --- a/templates/design/actions/get-design.ts +++ b/templates/design/actions/get-design.ts @@ -1,6 +1,6 @@ import { defineAction } from "@agent-native/core"; import { resolveAccess } from "@agent-native/core/sharing"; -import { eq } from "drizzle-orm"; +import { asc, eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; @@ -26,11 +26,19 @@ export default defineAction({ const row = access.resource; const db = getDb(); - // Fetch associated files + // Fetch associated files in a stable order. This array feeds the overview + // canvas's screen stack and each screen's index within its layout group, so + // unordered rows (Postgres returns heap order, which an UPDATE can change) + // meant the same design could lay itself out differently on two loads. + // Note this is deterministic, not creation-ordered: files written in one + // batch share a `createdAt` to the millisecond and fall back to the id + // tiebreak. Nothing may depend on the index matching the order a generator + // wrote in — see the order-independence case in variant-lineup.test.ts. const files = await db .select() .from(schema.designFiles) - .where(eq(schema.designFiles.designId, id)); + .where(eq(schema.designFiles.designId, id)) + .orderBy(asc(schema.designFiles.createdAt), asc(schema.designFiles.id)); return { id: row.id, diff --git a/templates/design/actions/present-design-variants.spec.ts b/templates/design/actions/present-design-variants.spec.ts index 5ce7e0ea30..f7e4dac8ef 100644 --- a/templates/design/actions/present-design-variants.spec.ts +++ b/templates/design/actions/present-design-variants.spec.ts @@ -496,6 +496,56 @@ describe("present-design-variants", () => { ); }); + it("spaces desktop directions by their whole painted row, not just the primary frame", async () => { + await action.run({ + designId: "design_123", + prompt: "Landing page for a Super Mario game", + variants: [ + { id: "retro", label: "Classic 8-Bit Retro" }, + { id: "modern", label: "Modern 3D" }, + { id: "comic", label: "Comic Poster" }, + ], + }); + + const data = mocks.designData; + // Desktop-base directions (1440) each paint a 390 mobile preview to their + // right, so a cell is 1440 + 24 + 390 = 1854 wide before the 96 gap. + // Spacing by the primary width alone dropped the next direction 1110px + // inside the previous one's breakpoint row. + expect(Object.values(data.canvasFrames).map((frame) => frame.x)).toEqual([ + 0, 1950, 3900, + ]); + expect( + Object.values(data.canvasFrames).map((frame) => frame.width), + ).toEqual([1440, 1440, 1440]); + }); + + it("reserves every breakpoint the design already has", async () => { + mocks.designData.breakpointSet = { + id: "existing", + breakpoints: [ + { id: "m", label: "Mobile", widthPx: 390 }, + { id: "t", label: "Tablet", widthPx: 768 }, + { id: "d", label: "Desktop", widthPx: 1440 }, + ], + }; + + await action.run({ + designId: "design_123", + prompt: "Landing page for a Super Mario game", + variants: [ + { id: "retro", label: "Classic 8-Bit Retro" }, + { id: "modern", label: "Modern 3D" }, + ], + }); + + // 1440 base drops the redundant 1440 preview and reserves 768 + 390: + // 1440 + (24 + 768) + (24 + 390) = 2646, then the 96 gap. + expect( + Object.values(mocks.designData.canvasFrames).map((f) => f.x), + ).toEqual([0, 2742]); + }); + it("renders compact fallback variants from non-todo mobile direction data", async () => { await action.run({ designId: "design_123", diff --git a/templates/design/actions/present-design-variants.ts b/templates/design/actions/present-design-variants.ts index fe2644f995..de137950f4 100644 --- a/templates/design/actions/present-design-variants.ts +++ b/templates/design/actions/present-design-variants.ts @@ -20,6 +20,10 @@ import { } from "../shared/canvas-frames.js"; import { isUniqueConstraintViolation } from "../shared/db-conflict.js"; import { widthToPrefix } from "../shared/responsive-classes.js"; +import { + getResponsiveGroupWidth, + visibleBreakpointWidths, +} from "../shared/responsive-frame-layout.js"; import { annotateScreenHtmlForPersist } from "../shared/screen-annotation.js"; const VARIANT_GAP = 96; @@ -734,7 +738,19 @@ ${compact ? ".sidebar { padding: 16px; } .nav { grid-template-columns: repeat(2, `; } -function placeVariantScreens(screens: VariantScreen[]) { +/** + * Lays the generated directions out in rows of up to three. + * + * Each cell reserves the screen's WHOLE painted footprint, not just its + * primary frame: this action also installs a design-wide breakpoint set, and + * the overview paints one preview per breakpoint to the right of every primary + * frame. Spacing by `width + VARIANT_GAP` alone drops the next direction on top + * of the previous one's breakpoint row. + */ +function placeVariantScreens( + screens: VariantScreen[], + breakpointWidths: readonly number[], +) { const placements: CanvasFramePlacement[] = []; const columns = Math.min(MAX_COLUMNS, Math.max(1, screens.length)); let rowY = 0; @@ -754,7 +770,17 @@ function placeVariantScreens(screens: VariantScreen[]) { height: screen.height, z: rowStart + offset, }); - x += screen.width + VARIANT_GAP; + x += + getResponsiveGroupWidth({ + primaryWidth: screen.width, + // Frames are placed at their natural device width, so the breakpoint + // previews beside them are drawn unscaled. + scale: 1, + visibleWidths: visibleBreakpointWidths( + breakpointWidths, + screen.width, + ), + }) + VARIANT_GAP; rowHeight = Math.max(rowHeight, screen.height); } @@ -764,6 +790,22 @@ function placeVariantScreens(screens: VariantScreen[]) { return placements; } +/** Widths the overview will paint beside each primary frame once this call + * settles: the design's own set when it has one, otherwise the set this action + * is about to install. */ +function effectiveBreakpointWidths(currentBreakpointSet: unknown): number[] { + const breakpoints = hasBreakpointSet(currentBreakpointSet) + ? ((currentBreakpointSet as { breakpoints: Array<{ widthPx?: unknown }> }) + .breakpoints ?? []) + : DEFAULT_RESPONSIVE_BREAKPOINTS; + return breakpoints + .map((breakpoint) => breakpoint.widthPx) + .filter( + (widthPx): widthPx is number => + typeof widthPx === "number" && Number.isFinite(widthPx) && widthPx > 0, + ); +} + export default defineAction({ description: "Present generated design directions as normal screens on the Design " + @@ -917,13 +959,19 @@ export default defineAction({ }); } - const placements = placeVariantScreens(screens); await mutateDesignData({ designId, mutate: (current, { updatedAt }) => { + // Placement depends on the breakpoint set in effect after this call, so + // it is resolved here (inside the compare-and-set body) rather than + // against a design snapshot that a concurrent write may have moved on + // from. const mergedFrames = mergeCanvasFramePlacements({ existing: current.canvasFrames, - placements, + placements: placeVariantScreens( + screens, + effectiveBreakpointWidths(current.breakpointSet), + ), resolveFileId: (placement) => placement.fileId, }); const previousMetadata = isRecord(current.screenMetadata) diff --git a/templates/design/app/components/design/MultiScreenCanvas.culling.test.ts b/templates/design/app/components/design/MultiScreenCanvas.culling.test.ts index 7d295543bd..a60a4b0734 100644 --- a/templates/design/app/components/design/MultiScreenCanvas.culling.test.ts +++ b/templates/design/app/components/design/MultiScreenCanvas.culling.test.ts @@ -11,7 +11,8 @@ import { isFrameWithinOverscannedViewport, OVERVIEW_CULLING_ENABLED, OVERVIEW_CULLING_OVERSCAN_FACTOR, - OVERVIEW_LIVE_IFRAME_BUDGET, + OVERVIEW_LIVE_IFRAME_CEILING, + OVERVIEW_LIVE_SCREEN_BUDGET, type ScreenCullCandidate, type OverscannedViewportBounds, } from "./multi-screen/culling"; @@ -115,6 +116,7 @@ describe("MultiScreenCanvas viewport culling", () => { previous?: ReturnType; epoch?: number; budget?: number; + screenBudget?: number; } = {}, ) { return computeBoundedScreenCullState({ @@ -129,6 +131,7 @@ describe("MultiScreenCanvas viewport culling", () => { options.previous?.lastVisibleEpochByScreenId ?? new Map(), accessEpoch: options.epoch ?? 1, + liveScreenBudget: options.screenBudget, liveIframeBudget: options.budget, }); } @@ -139,19 +142,61 @@ describe("MultiScreenCanvas viewport culling", () => { ); const result = compute(candidates); - expect(OVERVIEW_LIVE_IFRAME_BUDGET).toBeGreaterThan(0); - expect(result.liveScreenIds.size).toBe(OVERVIEW_LIVE_IFRAME_BUDGET); - expect(result.mountedIframeCount).toBe(OVERVIEW_LIVE_IFRAME_BUDGET); + expect(OVERVIEW_LIVE_SCREEN_BUDGET).toBeGreaterThan(0); + expect(result.liveScreenIds.size).toBe(OVERVIEW_LIVE_SCREEN_BUDGET); + expect(result.mountedIframeCount).toBe(OVERVIEW_LIVE_SCREEN_BUDGET); expect( [...result.tierByScreenId.values()].filter( (tier) => tier === "visible", ), - ).toHaveLength(OVERVIEW_LIVE_IFRAME_BUDGET); + ).toHaveLength(OVERVIEW_LIVE_SCREEN_BUDGET); expect( [...result.tierByScreenId.values()].filter( (tier) => tier === "placeholder", ), - ).toHaveLength(120 - OVERVIEW_LIVE_IFRAME_BUDGET); + ).toHaveLength(120 - OVERVIEW_LIVE_SCREEN_BUDGET); + }); + + it("keeps a breakpoint-bearing board fully mounted instead of evicting on camera moves", () => { + // The reported flicker: 14 screens x (primary + two breakpoint previews) + // = 42 contexts. Charged against one flat 32-iframe budget this admitted + // only ten screens, so every committed pan/zoom re-ranked the survivors + // and destroyed/recreated the losers' documents. Budgeting screens keeps + // the whole board live, and the iframe ceiling still bounds memory. + const candidates = Array.from({ length: 14 }, (_, index) => + candidate(`responsive-${index}`, index * 2_700, 3), + ); + const first = compute(candidates); + expect(first.liveScreenIds.size).toBe(14); + expect(first.mountedIframeCount).toBe(42); + expect(first.mountedIframeCount).toBeLessThanOrEqual( + OVERVIEW_LIVE_IFRAME_CEILING, + ); + expect([...first.tierByScreenId.values()]).not.toContain("evicted"); + + // Same board one camera commit later: nothing may change hands. + const second = compute(candidates, { previous: first, epoch: 2 }); + expect(second.liveScreenIds).toEqual(first.liveScreenIds); + expect([...second.tierByScreenId.values()]).not.toContain("evicted"); + }); + + it("still bounds a huge breakpoint-bearing board by the iframe ceiling", () => { + const candidates = Array.from({ length: 120 }, (_, index) => + candidate( + `responsive-${String(index).padStart(3, "0")}`, + index * 500, + 4, + ), + ); + const result = compute(candidates); + + // 32 screens x 4 contexts would be 128, past the ceiling, so the ceiling + // binds first and no screen is ever partially mounted. + expect(result.mountedIframeCount).toBeLessThanOrEqual( + OVERVIEW_LIVE_IFRAME_CEILING, + ); + expect(result.mountedIframeCount % 4).toBe(0); + expect(result.liveScreenIds.size).toBe(OVERVIEW_LIVE_IFRAME_CEILING / 4); }); it("evicts least-recently-visible screens and restores them on revisit", () => { diff --git a/templates/design/app/components/design/MultiScreenCanvas.tsx b/templates/design/app/components/design/MultiScreenCanvas.tsx index 79a2b148dd..dbec185945 100644 --- a/templates/design/app/components/design/MultiScreenCanvas.tsx +++ b/templates/design/app/components/design/MultiScreenCanvas.tsx @@ -300,6 +300,7 @@ import { } from "./multi-screen/draft-primitives"; import { angleBetween, + BREAKPOINT_FRAME_GAP, cloneFrameGeometryById, deviceViewportFloorForWidth, findTopFrameEntryAtPoint, @@ -9820,8 +9821,6 @@ function areScreenPropsEqual(prev: ScreenProps, next: ScreenProps) { // ── Breakpoint preview row (§6.4) ──────────────────────────────────────────── -/** Gap between adjacent breakpoint frames in canvas pixels. */ -const BREAKPOINT_FRAME_GAP = 24; const BREAKPOINT_LABEL_WITH_WIDTH_MIN_FRAME_WIDTH = 128; /** diff --git a/templates/design/app/components/design/multi-screen/culling.ts b/templates/design/app/components/design/multi-screen/culling.ts index ece7739bf2..6ab0d1861d 100644 --- a/templates/design/app/components/design/multi-screen/culling.ts +++ b/templates/design/app/components/design/multi-screen/culling.ts @@ -39,13 +39,30 @@ export const OVERVIEW_CULLING_ENABLED = true; * recomputes. */ export const OVERVIEW_CULLING_OVERSCAN_FACTOR = 1.5; -/** Maximum number of evictable overview browsing contexts kept mounted at - * once. One screen costs its primary preview plus every breakpoint preview. +/** Maximum number of evictable overview SCREENS kept mounted at once. + * + * The pool is budgeted in screens rather than iframes because a screen is the + * unit the user perceives: a board where every screen carries breakpoint + * previews costs several browsing contexts per screen, and charging those + * against one flat iframe budget shrank the pool to a third of its intended + * size. Past that point every committed pan/zoom re-ranked the in-viewport set + * by distance and admitted a different subset, so screens were destroyed and + * re-created — a document reload, i.e. a visible flash — on ordinary camera + * movement. Generating a variant set installs a design-wide breakpoint set, so + * this regressed the moment variants existed. + * * Interaction-protected screens are the sole safety exception: if the user * explicitly selects more than this budget, preserving their live editor * state wins until that interaction ends, after which the pool contracts on * the next culling pass. */ -export const OVERVIEW_LIVE_IFRAME_BUDGET = 32; +export const OVERVIEW_LIVE_SCREEN_BUDGET = 32; + +/** Hard ceiling on total mounted browsing contexts, independent of the screen + * budget above. This is the memory backstop the original budget existed for — + * a long tour of a 100+ screen board still cannot accumulate contexts without + * bound — set high enough that a normal breakpoint-bearing board is limited by + * OVERVIEW_LIVE_SCREEN_BUDGET instead of by this. */ +export const OVERVIEW_LIVE_IFRAME_CEILING = 96; export type ScreenCullTier = /** Full content (iframe/DesignCanvas) is mounted and rendered normally. */ @@ -127,7 +144,8 @@ export function computeBoundedScreenCullState({ everVisibleScreenIds, lastVisibleEpochByScreenId, accessEpoch, - liveIframeBudget = OVERVIEW_LIVE_IFRAME_BUDGET, + liveScreenBudget = OVERVIEW_LIVE_SCREEN_BUDGET, + liveIframeBudget = OVERVIEW_LIVE_IFRAME_CEILING, }: { candidates: readonly ScreenCullCandidate[]; viewport: OverscannedViewportBounds | null; @@ -136,6 +154,7 @@ export function computeBoundedScreenCullState({ everVisibleScreenIds: ReadonlySet; lastVisibleEpochByScreenId: ReadonlyMap; accessEpoch: number; + liveScreenBudget?: number; liveIframeBudget?: number; }): BoundedScreenCullState { if (!OVERVIEW_CULLING_ENABLED) { @@ -187,19 +206,25 @@ export function computeBoundedScreenCullState({ if (protectedScreenIds.has(candidate.id)) add(candidate); } // Protected screens can temporarily exceed the normal pool. In that case - // no evictable iframe is admitted until the interaction set shrinks again. - const effectiveBudget = Math.max( + // no evictable screen is admitted until the interaction set shrinks again. + const effectiveScreenBudget = Math.max( + Math.max(0, Math.floor(liveScreenBudget)), + nextLive.size, + ); + const effectiveIframeBudget = Math.max( Math.max(0, Math.floor(liveIframeBudget)), mountedIframeCount, ); const tryAddWithinBudget = (candidate: ScreenCullCandidate) => { if (nextLive.has(candidate.id)) return; + if (nextLive.size + 1 > effectiveScreenBudget) return; if ( - mountedIframeCount + normalizedIframeCount(candidate) <= - effectiveBudget + mountedIframeCount + normalizedIframeCount(candidate) > + effectiveIframeBudget ) { - add(candidate); + return; } + add(candidate); }; const visibleCandidates = candidates diff --git a/templates/design/app/components/design/multi-screen/frame-geometry.ts b/templates/design/app/components/design/multi-screen/frame-geometry.ts index 7b51f6d934..6a90fd8422 100644 --- a/templates/design/app/components/design/multi-screen/frame-geometry.ts +++ b/templates/design/app/components/design/multi-screen/frame-geometry.ts @@ -1,3 +1,9 @@ +import { + BREAKPOINT_FRAME_GAP, + getResponsiveGroupWidth, + visibleBreakpointWidths, +} from "@shared/responsive-frame-layout"; + import { DEVICE_FRAME_VIEWPORTS, type DeviceFrameType } from "../types"; import { SURFACE_PADDING } from "./overview-layout"; import type { FrameGeometry, FrameGeometryById, Point } from "./types"; @@ -5,7 +11,8 @@ import type { FrameGeometry, FrameGeometryById, Point } from "./types"; const SCREEN_WIDTH = 320; const SCREEN_GAP = 56; const FRAME_LABEL_HEIGHT = 28; -const BREAKPOINT_FRAME_GAP = 24; + +export { BREAKPOINT_FRAME_GAP, visibleBreakpointWidths }; export interface BoundsRect { left: number; @@ -26,26 +33,6 @@ type ResponsiveLayoutScreen = { layoutGroupId?: string; }; -/** Drops any breakpoint whose width equals the primary frame's own width — a - * redundant duplicate of the base — also cleaning up designs authored before - * the default set excluded the primary width. */ -export function visibleBreakpointWidths( - breakpointWidths: readonly number[] | undefined, - primaryWidthPx: number | undefined, -): number[] { - const deduped = Array.from( - new Set( - (breakpointWidths ?? []).filter( - (width) => Number.isFinite(width) && width > 0, - ), - ), - ); - if (primaryWidthPx === undefined || !Number.isFinite(primaryWidthPx)) { - return deduped; - } - return deduped.filter((width) => Math.abs(width - primaryWidthPx) > 1); -} - /** Minimum height for a frame of the given width — one device viewport tall * before it grows to content. Keep in sync with deviceViewportHeight in * content-size-report.ts. */ @@ -85,12 +72,11 @@ export function getResponsiveScreenGroupSize( : (width * sourceHeight) / sourceWidth; }; return { - width: - baseWidth + - breakpoints.reduce( - (total, width) => total + BREAKPOINT_FRAME_GAP + width * scale, - 0, - ), + width: getResponsiveGroupWidth({ + primaryWidth: baseWidth, + scale, + visibleWidths: breakpoints, + }), height: Math.max( baseHeight, ...breakpoints.map((width) => breakpointNaturalHeight(width) * scale), diff --git a/templates/design/app/components/design/multi-screen/variant-lineup.test.ts b/templates/design/app/components/design/multi-screen/variant-lineup.test.ts new file mode 100644 index 0000000000..bfc8458273 --- /dev/null +++ b/templates/design/app/components/design/multi-screen/variant-lineup.test.ts @@ -0,0 +1,196 @@ +import { + getResponsiveGroupWidth, + visibleBreakpointWidths, +} from "@shared/responsive-frame-layout"; +import { describe, expect, it } from "vitest"; + +import { + getResponsiveScreenGroupSize, + resolveFrameGeometrySync, +} from "./frame-geometry"; +import type { FrameGeometry } from "./types"; + +/** + * A generated variant set must never land one direction on top of the previous + * one's breakpoint row. present-design-variants installs a design-wide + * breakpoint set, so every primary frame grows a preview row to its right; the + * lineup it writes has to reserve that width. + * + * Reported as "the variants are overlapping each other" plus flicker while + * zooming, because the overlapping row also pushed the board past the live + * browsing-context pool. + */ +const VARIANT_GAP = 96; +const PRIMARY_WIDTH = 1440; +const PRIMARY_HEIGHT = 900; + +/** Mirrors present-design-variants.placeVariantScreens. */ +function placeVariantScreens( + screens: ReadonlyArray<{ id: string; width: number; height: number }>, + breakpointWidths: readonly number[], +) { + const placements: Record = {}; + const columns = Math.min(3, Math.max(1, screens.length)); + let rowY = 0; + for (let rowStart = 0; rowStart < screens.length; rowStart += columns) { + const row = screens.slice(rowStart, rowStart + columns); + let x = 0; + let rowHeight = 0; + for (const [offset, screen] of row.entries()) { + placements[screen.id] = { + x, + y: rowY, + width: screen.width, + height: screen.height, + z: rowStart + offset, + }; + x += + getResponsiveGroupWidth({ + primaryWidth: screen.width, + scale: 1, + visibleWidths: visibleBreakpointWidths( + breakpointWidths, + screen.width, + ), + }) + VARIANT_GAP; + rowHeight = Math.max(rowHeight, screen.height); + } + rowY += rowHeight + VARIANT_GAP; + } + return placements; +} + +function variantScreens(breakpointWidths: number[]) { + return ["classic-8bit", "modern-3d", "comic-poster"].map((id) => ({ + id, + metadata: { width: PRIMARY_WIDTH, height: PRIMARY_HEIGHT }, + breakpointWidths, + layoutGroupId: "set-1", + })); +} + +type VariantScreen = ReturnType[number]; + +function overlappingPairs( + screens: readonly VariantScreen[], + geometryById: Record, +) { + const boxes = screens.map((screen) => { + const geometry = geometryById[screen.id]!; + const size = getResponsiveScreenGroupSize(screen, geometry); + return { + id: screen.id, + left: geometry.x, + right: geometry.x + size.width, + top: geometry.y, + bottom: geometry.y + size.height, + }; + }); + const pairs: string[] = []; + for (let a = 0; a < boxes.length; a += 1) { + for (let b = a + 1; b < boxes.length; b += 1) { + const first = boxes[a]!; + const second = boxes[b]!; + if ( + first.left < second.right - 1 && + first.right > second.left + 1 && + first.top < second.bottom - 1 && + first.bottom > second.top + 1 + ) { + pairs.push(`${first.id}/${second.id}`); + } + } + } + return pairs; +} + +function place(screens: readonly VariantScreen[], breakpointWidths: number[]) { + return placeVariantScreens( + screens.map((screen) => ({ + id: screen.id, + width: screen.metadata.width, + height: screen.metadata.height, + })), + breakpointWidths, + ); +} + +describe("generated variant lineup", () => { + it("reserves the breakpoint row so the persisted lineup never overlaps", () => { + const screens = variantScreens([390]); + const persisted = place(screens, [390]); + + // 1440 + 24 + 390 = 1854 painted, so the next cell starts at 1854 + 96. + expect(persisted["classic-8bit"]!.x).toBe(0); + expect(persisted["modern-3d"]!.x).toBe(1950); + expect(persisted["comic-poster"]!.x).toBe(3900); + expect(overlappingPairs(screens, persisted)).toEqual([]); + }); + + it("reserves every breakpoint when the design already has a full set", () => { + // The reported design: 1440 base with 768 and 390 previews. The primary's + // own width is not a preview, so only 768 + 390 are reserved. + const screens = variantScreens([1440, 768, 390]); + const persisted = place(screens, [1440, 768, 390]); + + expect(persisted["modern-3d"]!.x).toBe(2646 + VARIANT_GAP); + expect(overlappingPairs(screens, persisted)).toEqual([]); + }); + + it("is left alone by the client lineup repair, in any file order", () => { + const screens = variantScreens([390]); + const persisted = place(screens, [390]); + // get-design now orders files deterministically, but the lineup must not + // depend on that: a correct lineup has to survive any ordering. + for (const order of [ + [0, 1, 2], + [2, 0, 1], + [1, 2, 0], + ]) { + const ordered = order.map((index) => screens[index]!); + const { next } = resolveFrameGeometrySync({ + screens: ordered, + currentGeometryById: {}, + persistedGeometryById: persisted, + }); + expect(overlappingPairs(ordered, next)).toEqual([]); + for (const screen of ordered) { + expect(next[screen.id]!.x).toBe(persisted[screen.id]!.x); + } + } + }); + + it("survives a hide/show of the breakpoint frames", () => { + const screens = variantScreens([390]); + const persisted = place(screens, [390]); + // DesignEditor passes breakpointWidths: undefined while the frames are + // hidden, which used to be the only signal the repair keyed on. + const hidden = screens.map((screen) => ({ + ...screen, + breakpointWidths: undefined as unknown as number[], + })); + const { next } = resolveFrameGeometrySync({ + screens: hidden, + currentGeometryById: {}, + persistedGeometryById: persisted, + }); + expect(overlappingPairs(screens, next)).toEqual([]); + }); + + it("keeps a designer's own arrangement after they move one direction", () => { + const screens = variantScreens([390]); + const persisted = place(screens, [390]); + const nudged = { + ...persisted, + "comic-poster": { ...persisted["comic-poster"]!, x: 12_000, y: 4_000 }, + }; + const { next } = resolveFrameGeometrySync({ + screens, + currentGeometryById: {}, + persistedGeometryById: nudged, + }); + expect(next["comic-poster"]!.x).toBe(12_000); + expect(next["comic-poster"]!.y).toBe(4_000); + expect(overlappingPairs(screens, next)).toEqual([]); + }); +}); diff --git a/templates/design/changelog/2026-07-28-generated-design-variants-no-longer-overlap-each-other-on-th.md b/templates/design/changelog/2026-07-28-generated-design-variants-no-longer-overlap-each-other-on-th.md new file mode 100644 index 0000000000..31c026ad3d --- /dev/null +++ b/templates/design/changelog/2026-07-28-generated-design-variants-no-longer-overlap-each-other-on-th.md @@ -0,0 +1,6 @@ +--- +type: fixed +date: 2026-07-28 +--- + +Generated design variants no longer overlap each other on the overview board, and zooming or panning a board with breakpoint frames no longer flashes screen content diff --git a/templates/design/shared/responsive-frame-layout.ts b/templates/design/shared/responsive-frame-layout.ts new file mode 100644 index 0000000000..8763ab5dc9 --- /dev/null +++ b/templates/design/shared/responsive-frame-layout.ts @@ -0,0 +1,55 @@ +/** + * Geometry of one screen's responsive frame group: the primary frame plus the + * breakpoint previews painted to its right. + * + * This lives in `shared/` because both sides of the contract need it. Actions + * that *place* screens on the overview board (present-design-variants) must + * reserve the same width the canvas will actually *paint*, or the next screen + * lands underneath the previous screen's breakpoint row. Keeping the width in + * one place is what makes writer and renderer agree. + */ + +/** Gap between the primary frame and each breakpoint preview beside it. */ +export const BREAKPOINT_FRAME_GAP = 24; + +/** Drops any breakpoint whose width equals the primary frame's own width — a + * redundant duplicate of the base — also cleaning up designs authored before + * the default set excluded the primary width. */ +export function visibleBreakpointWidths( + breakpointWidths: readonly number[] | undefined, + primaryWidthPx: number | undefined, +): number[] { + const deduped = Array.from( + new Set( + (breakpointWidths ?? []).filter( + (width) => Number.isFinite(width) && width > 0, + ), + ), + ); + if (primaryWidthPx === undefined || !Number.isFinite(primaryWidthPx)) { + return deduped; + } + return deduped.filter((width) => Math.abs(width - primaryWidthPx) > 1); +} + +/** + * Total painted width of a screen's frame group: the primary box plus every + * breakpoint preview beside it, each drawn at the primary's own uniform + * `scale`. `visibleWidths` must already be filtered through + * `visibleBreakpointWidths` — callers differ in which width they dedupe + * against, but the arithmetic must not. + */ +export function getResponsiveGroupWidth({ + primaryWidth, + scale, + visibleWidths, +}: { + primaryWidth: number; + scale: number; + visibleWidths: readonly number[]; +}): number { + return visibleWidths.reduce( + (total, width) => total + BREAKPOINT_FRAME_GAP + width * scale, + Math.max(1, primaryWidth), + ); +}