From 0998f4fca20ba4aa976e50e07f12e95f9f2cb62c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 05:21:47 -0400 Subject: [PATCH] chore: rescue snapshot of dnits-zorder before devbox cleanup --- .../src/components/editor/useDomEditZOrder.ts | 14 +- packages/studio/src/domEditExports.test.tsx | 4 +- .../studio/src/hooks/domEditCommitRunner.ts | 3 +- .../src/hooks/elementLifecycleOpsTestUtils.ts | 1 - .../src/hooks/useDomEditCommits.test.tsx | 184 +++++++++--------- .../studio/src/hooks/useDomEditCommits.ts | 1 - ...seElementLifecycleOps.multiDelete.test.tsx | 10 - .../src/hooks/useElementLifecycleOps.test.tsx | 22 --- .../src/hooks/useElementLifecycleOps.ts | 9 +- .../src/hooks/useTimelineEditing.test.tsx | 1 - 10 files changed, 107 insertions(+), 142 deletions(-) diff --git a/packages/studio/src/components/editor/useDomEditZOrder.ts b/packages/studio/src/components/editor/useDomEditZOrder.ts index b8b2f15ff6..e504327741 100644 --- a/packages/studio/src/components/editor/useDomEditZOrder.ts +++ b/packages/studio/src/components/editor/useDomEditZOrder.ts @@ -108,11 +108,11 @@ export function resolveZIndexEntries( } export interface DomEditZOrder { - /** False in a read-only preview, for an element no longer in the preview, or at that end of its stacking set. */ + /** False in a read-only preview, for an element no longer in the live preview, or at that end of its stacking set. */ enabled: (sel: DomEditSelection, action: ZOrderAction) => boolean; /** Resolve and commit one step; false when nothing is sent to be saved. */ apply: (sel: DomEditSelection, action: ZOrderAction) => boolean; - /** Commit patches already resolved (the canvas menu resolves its own); false when none could be saved. */ + /** Commit patches already resolved (the canvas menu resolves its own); false when none could be sent to be saved. */ commit: ( sel: DomEditSelection, patches: ReadonlyArray, @@ -121,6 +121,10 @@ export interface DomEditZOrder { ) => boolean; } +/** A selection from before a preview reload holds an element of a document no window shows, with stale z values. */ +const isLive = (element: HTMLElement) => + element.isConnected && element.ownerDocument.defaultView !== null; + // The canvas menu's z-order (write, undo, lane mirror) for any caller in DomEditProvider. export function useDomEditZOrder(): DomEditZOrder { const { handleDomZIndexReorderCommit } = useDomEditActionsContext(); @@ -129,8 +133,7 @@ export function useDomEditZOrder(): DomEditZOrder { const activeCompPath = useStudioShellContextOptional()?.activeCompPath ?? null; const enabled = useCallback( - (sel, action) => - !readOnly && sel.element.isConnected && isZOrderActionEnabled(sel.element, action), + (sel, action) => !readOnly && isLive(sel.element) && isZOrderActionEnabled(sel.element, action), [readOnly], ); @@ -171,8 +174,7 @@ export function useDomEditZOrder(): DomEditZOrder { const apply = useCallback( (sel, action) => { - // A selection from before a preview reload holds a detached element and stale z values. - if (!sel.element.isConnected) return false; + if (!isLive(sel.element)) return false; const step = resolveZOrderStep(sel.element, action); return step !== null && commit(sel, step.patches, action, step.crossed); }, diff --git a/packages/studio/src/domEditExports.test.tsx b/packages/studio/src/domEditExports.test.tsx index cf9aad1e02..f67a900730 100644 --- a/packages/studio/src/domEditExports.test.tsx +++ b/packages/studio/src/domEditExports.test.tsx @@ -150,7 +150,9 @@ describe("DOM editing package exports", () => { expect(zOrder?.apply(sel, "bring-to-front")).toBe(false); expect(zOrder?.commit(sel, [{ element: back, zIndex: 1 }], "bring-to-front", null)).toBe(false); await mount(false); - parent.remove(); + // A reload leaves the old nodes connected to their old document, one no window shows. + document.implementation.createHTMLDocument().body.append(parent); + expect(back.isConnected).toBe(true); expect(zOrder?.enabled(sel, "bring-to-front"), "a selection from before a reload").toBe(false); expect(zOrder?.apply(sel, "bring-to-front")).toBe(false); expect(commitZ).not.toHaveBeenCalled(); diff --git a/packages/studio/src/hooks/domEditCommitRunner.ts b/packages/studio/src/hooks/domEditCommitRunner.ts index 79886814f2..623e25d079 100644 --- a/packages/studio/src/hooks/domEditCommitRunner.ts +++ b/packages/studio/src/hooks/domEditCommitRunner.ts @@ -91,8 +91,7 @@ export type DomEditCommitDeclineReason = | "styles-not-editable" | "not-text-editable" | "preview-stale" - | "persist-failed" - | "read-only"; + | "persist-failed"; export type DomEditCommitOutcome = | { ok: true; persistence?: DomEditPersistOutcome } diff --git a/packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts b/packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts index 5d56b30322..76a1905659 100644 --- a/packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts +++ b/packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts @@ -19,7 +19,6 @@ export function makeLifecycleOpsParams( projectIdRef: { current: null }, reloadPreview: vi.fn(), clearDomSelection: vi.fn(), - readOnlyPreview: false, ...overrides, }; } diff --git a/packages/studio/src/hooks/useDomEditCommits.test.tsx b/packages/studio/src/hooks/useDomEditCommits.test.tsx index 60b5b554b0..3e96cfd662 100644 --- a/packages/studio/src/hooks/useDomEditCommits.test.tsx +++ b/packages/studio/src/hooks/useDomEditCommits.test.tsx @@ -42,6 +42,7 @@ interface RenderDomEditCommitsOptions { onTrySdkPersist?: () => Promise; queueDomEditSave?: (save: () => Promise) => Promise; projectIdRef?: MutableRefObject; + readOnlyPreview?: boolean; } type FetchHandler = ( @@ -230,7 +231,7 @@ function renderDomEditCommits( refreshDomEditSelectionFromPreview: vi.fn(), buildDomSelectionFromTarget: vi.fn(async () => null), onTrySdkPersist: options.onTrySdkPersist, - readOnlyPreview: false, + readOnlyPreview: options.readOnlyPreview ?? false, }); return null; } @@ -264,97 +265,100 @@ describe("useDomEditCommits z-index reorder persistence", () => { document.body.replaceChildren(); }); - it("persists an N-element reorder with one batch POST, one undo entry, and NO iframe reload", async () => { - const original = - '
'; - const after = - '
'; - const fetchMock = vi.fn( - async ( - input: Parameters[0], - _init?: Parameters[1], - ): Promise => { - const url = requestUrl(input); - if (url.endsWith("/file-mutations/patch-element-batches")) { - return jsonResponse({ - durable: true, - files: [ - { - sourceFile: "index.html", - changed: true, - matched: [true, true, true], - before: original, - after, - }, - ], - }); - } - throw new Error(`Unexpected fetch: ${url}`); - }, - ); - vi.stubGlobal("fetch", fetchMock); - const { iframe, element } = createPreviewElement( - '
', - ); - element.id = "a"; - const elements = [ - element, - iframe.contentDocument!.getElementById("b")!, - iframe.contentDocument!.getElementById("c")!, - ]; - const rendered = renderDomEditCommits(createSelection(element), iframe); + it.each([false, true])( + "persists an N-element reorder with one batch POST, one undo entry, and NO iframe reload (read-only preview %s: the timeline's z sync stays editable)", + async (readOnlyPreview) => { + const original = + '
'; + const after = + '
'; + const fetchMock = vi.fn( + async ( + input: Parameters[0], + _init?: Parameters[1], + ): Promise => { + const url = requestUrl(input); + if (url.endsWith("/file-mutations/patch-element-batches")) { + return jsonResponse({ + durable: true, + files: [ + { + sourceFile: "index.html", + changed: true, + matched: [true, true, true], + before: original, + after, + }, + ], + }); + } + throw new Error(`Unexpected fetch: ${url}`); + }, + ); + vi.stubGlobal("fetch", fetchMock); + const { iframe, element } = createPreviewElement( + '
', + ); + element.id = "a"; + const elements = [ + element, + iframe.contentDocument!.getElementById("b")!, + iframe.contentDocument!.getElementById("c")!, + ]; + const rendered = renderDomEditCommits(createSelection(element), iframe, { readOnlyPreview }); - try { - await act(async () => { - await rendered.hook.handleDomZIndexReorderCommit( - elements.map((item, index) => ({ - element: item, - zIndex: 3 - index, - id: item.id, - sourceFile: "index.html", - })), - "z-reorder:test", + try { + await act(async () => { + await rendered.hook.handleDomZIndexReorderCommit( + elements.map((item, index) => ({ + element: item, + zIndex: 3 - index, + id: item.id, + sourceFile: "index.html", + })), + "z-reorder:test", + ); + }); + + const batchPosts = fetchMock.mock.calls.filter(([input]) => + requestUrl(input).endsWith("/file-mutations/patch-element-batches"), ); - }); - - const batchPosts = fetchMock.mock.calls.filter(([input]) => - requestUrl(input).endsWith("/file-mutations/patch-element-batches"), - ); - const singlePosts = fetchMock.mock.calls.filter(([input]) => - requestUrl(input).includes("/file-mutations/patch-elements-batch/"), - ); - expect(batchPosts).toHaveLength(1); - expect(singlePosts).toHaveLength(0); - expect(JSON.parse(String(batchPosts[0]?.[1]?.body))).toEqual({ - batches: [ - { - sourceFile: "index.html", - patches: expect.arrayContaining([ - expect.objectContaining({ target: expect.objectContaining({ id: "a" }) }), - expect.objectContaining({ target: expect.objectContaining({ id: "b" }) }), - expect.objectContaining({ target: expect.objectContaining({ id: "c" }) }), - ]), - }, - ], - }); - expect(rendered.recordEdit).toHaveBeenCalledTimes(1); - expect(rendered.recordEdit).toHaveBeenCalledWith({ - label: "Reorder layers", - coalesceKey: "z-reorder:test", - // Unbounded per-gesture fold window (keys are unique per gesture): - // the z entry and its mirror/lane counterpart fold across the server - // round-trip that separates them. - coalesceMs: Number.POSITIVE_INFINITY, - files: { "index.html": { before: original, after } }, - }); - // FIX: a z-only reorder must NOT remount the preview iframe ("the blink"). - // The live DOM + store already hold the final state and the server matched - // every style-only patch, so the reload is provably redundant. - expect(rendered.reloadPreview).not.toHaveBeenCalled(); - } finally { - rendered.cleanup(); - } - }); + const singlePosts = fetchMock.mock.calls.filter(([input]) => + requestUrl(input).includes("/file-mutations/patch-elements-batch/"), + ); + expect(batchPosts).toHaveLength(1); + expect(singlePosts).toHaveLength(0); + expect(JSON.parse(String(batchPosts[0]?.[1]?.body))).toEqual({ + batches: [ + { + sourceFile: "index.html", + patches: expect.arrayContaining([ + expect.objectContaining({ target: expect.objectContaining({ id: "a" }) }), + expect.objectContaining({ target: expect.objectContaining({ id: "b" }) }), + expect.objectContaining({ target: expect.objectContaining({ id: "c" }) }), + ]), + }, + ], + }); + expect(rendered.recordEdit).toHaveBeenCalledTimes(1); + expect(rendered.recordEdit).toHaveBeenCalledWith({ + label: "Reorder layers", + coalesceKey: "z-reorder:test", + // Unbounded per-gesture fold window (keys are unique per gesture): + // the z entry and its mirror/lane counterpart fold across the server + // round-trip that separates them. + coalesceMs: Number.POSITIVE_INFINITY, + files: { "index.html": { before: original, after } }, + }); + // FIX: a z-only reorder must NOT remount the preview iframe ("the blink"). + // The live DOM + store already hold the final state and the server matched + // every style-only patch, so the reload is provably redundant. + expect(rendered.reloadPreview).not.toHaveBeenCalled(); + } finally { + rendered.cleanup(); + } + }, + ); it("falls back to reloading when the server response omits matched[]", async () => { // Without a matched[] confirmation the persist can't be proven in sync with diff --git a/packages/studio/src/hooks/useDomEditCommits.ts b/packages/studio/src/hooks/useDomEditCommits.ts index 285aefef2d..eacf065279 100644 --- a/packages/studio/src/hooks/useDomEditCommits.ts +++ b/packages/studio/src/hooks/useDomEditCommits.ts @@ -295,7 +295,6 @@ export function useDomEditCommits({ onReorderShadow, forceReloadSdkSession, commitDomEditPatchBatches, - readOnlyPreview, }); return { diff --git a/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx b/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx index 8ee0cd7611..93d1a90403 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx +++ b/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx @@ -105,16 +105,6 @@ describe("useElementLifecycleOps — deleting a canvas multi-selection", () => { expect(requests.some((url) => url.includes("remove-elements"))).toBe(false); }); - it("deletes nothing in a read-only preview and says why", async () => { - const ops = mountDeleteOps({ projectIdRef: { current: "p1" }, readOnlyPreview: true }); - let outcome: unknown; - await act(async () => { - outcome = await ops.handleDomEditElementsDelete([selectionFor("a")]); - }); - expect(outcome).toEqual({ ok: false, reason: "read-only" }); - expect(requests).toEqual([]); - }); - it("reports missing project and selection without starting a request", async () => { const projectIdRef = { current: null as string | null }; const ops = mountDeleteOps({ projectIdRef }); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.test.tsx b/packages/studio/src/hooks/useElementLifecycleOps.test.tsx index b48ffd50b4..d740894154 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.test.tsx +++ b/packages/studio/src/hooks/useElementLifecycleOps.test.tsx @@ -121,28 +121,6 @@ describe("useElementLifecycleOps — z-index reorder payload", () => { act(() => root.unmount()); }); - it("a read-only preview reorders nothing and reports it unsaved, so the lane mirror is skipped", async () => { - const el = document.body.appendChild(document.createElement("div")); - el.id = "card"; - const batches = vi.fn(async () => ({ durable: true, allMatched: true, changed: true })); - let commit: ReorderCommit | undefined; - function Harness() { - commit = useElementLifecycleOps( - makeLifecycleOpsParams({ commitDomEditPatchBatches: batches, readOnlyPreview: true }), - ).handleDomZIndexReorderCommit; - return null; - } - const root = mountReactHarness(); - const result = await commit!([ - { element: el, zIndex: 3, id: "card", sourceFile: "index.html" }, - ]); - expect(result).toEqual({ durable: false, allMatched: false, changed: false }); - expect(batches).not.toHaveBeenCalled(); - expect(el.style.zIndex).toBe(""); - act(() => root.unmount()); - el.remove(); - }); - it("requests skipReload on every z-reorder persist (live DOM already final)", async () => { // The commit applies the z-index (and any injected position) to the live // iframe DOM and the store synchronously, so the persisted style-only patch diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index 231020dc35..25c934da88 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -37,7 +37,6 @@ interface UseElementLifecycleOpsParams extends DomEditCommitBaseParams { /** Resync the SDK session after a server-fallback delete. */ forceReloadSdkSession?: () => void; commitDomEditPatchBatches: CommitDomEditPatchBatches; - readOnlyPreview: boolean; /** Stage 7 Step 3b: called after a successful server-side element delete (shadow). */ onElementDeleted?: (selection: DomEditSelection) => void; } @@ -84,14 +83,12 @@ export function useElementLifecycleOps({ onReorderShadow, forceReloadSdkSession, commitDomEditPatchBatches, - readOnlyPreview, onElementDeleted, }: UseElementLifecycleOpsParams) { // fallow-ignore-next-line complexity const handleDomEditElementsDelete = useCallback( // fallow-ignore-next-line complexity async (selections: DomEditSelection[]): Promise => { - if (readOnlyPreview) return domEditCommitDeclined("read-only"); const pid = projectIdRef.current; if (!pid) return domEditCommitDeclined("no-project"); const [selection] = selections; @@ -224,7 +221,6 @@ export function useElementLifecycleOps({ onElementDeleted, forceReloadSdkSession, projectIdRef, - readOnlyPreview, reloadPreview, showToast, writeProjectFile, @@ -250,9 +246,6 @@ export function useElementLifecycleOps({ actionKind?: string, ) => { if (entries.length === 0) return Promise.resolve(); - // Refused before any write, so the lane mirror after it is skipped too. - if (readOnlyPreview) - return Promise.resolve({ durable: false, allMatched: false, changed: false }); // One async owner must bracket reveal tokens, optimistic DOM/store state, // atomic persistence, rollback, and the final persistence-count release. // Splitting those phases would make transaction ownership less explicit. @@ -391,7 +384,7 @@ export function useElementLifecycleOps({ } })(); }, - [commitDomEditPatchBatches, onReorderShadow, readOnlyPreview], + [commitDomEditPatchBatches, onReorderShadow], ); return { diff --git a/packages/studio/src/hooks/useTimelineEditing.test.tsx b/packages/studio/src/hooks/useTimelineEditing.test.tsx index 672e0ec095..fc7b07f7c4 100644 --- a/packages/studio/src/hooks/useTimelineEditing.test.tsx +++ b/packages/studio/src/hooks/useTimelineEditing.test.tsx @@ -248,7 +248,6 @@ function renderTimelineEditingHookWithLifecycle(input: { projectIdRef: { current: "p1" }, reloadPreview: vi.fn(), clearDomSelection: vi.fn(), - readOnlyPreview: false, commitDomEditPatchBatches: input.commitDomEditPatchBatches, }); const commitRef = useRef(lifecycle.handleDomZIndexReorderCommit);