diff --git a/packages/studio/src/components/editor/CanvasContextMenu.test.tsx b/packages/studio/src/components/editor/CanvasContextMenu.test.tsx index 903986321..10b9c23e6 100644 --- a/packages/studio/src/components/editor/CanvasContextMenu.test.tsx +++ b/packages/studio/src/components/editor/CanvasContextMenu.test.tsx @@ -3,7 +3,11 @@ import React, { act } from "react"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { installReactActEnvironment, makeSelection } from "../../hooks/domSelectionTestHarness"; +import { resolveZIndexEntries } from "../nle/PreviewOverlays"; +import { useElementLifecycleOps } from "../../hooks/useElementLifecycleOps"; +import type { DomEditPatchBatch } from "../../hooks/domEditCommitTypes"; import { CanvasContextMenu } from "./CanvasContextMenu"; +import type { ZOrderAction, ZOrderPatch } from "./canvasContextMenuZOrder"; import type { DomEditSelection } from "./domEditing"; installReactActEnvironment(); @@ -24,7 +28,7 @@ afterEach(() => { function renderMenu(props: { selection: DomEditSelection; - onApplyZIndex?: () => void; + onApplyZIndex?: (patches: ZOrderPatch[], action: ZOrderAction) => void; onDelete?: (selection: DomEditSelection) => void; }) { root = createRoot(host); @@ -113,3 +117,110 @@ describe("CanvasContextMenu — handler gating", () => { expect(document.body.querySelector(".border-t")).toBeNull(); }); }); + +// ── Menu z-action → commit path (wired the way PreviewOverlays wires the app) ── + +function pressMenuItem(label: string) { + const button = zOrderButtons().find((b) => b.textContent === label); + expect(button).toBeDefined(); + act(() => { + button!.dispatchEvent( + new PointerEvent("pointerdown", { bubbles: true, cancelable: true, button: 0 }), + ); + }); +} + +/** Target (static, earlier in DOM) below an equal-z sibling — z action must renumber. */ +function makeStaticFamily() { + const parent = document.createElement("div"); + const target = document.createElement("div"); + target.id = "target"; + // In happy-dom an unset computed position is "" (not "static"), which would + // skip the commit hook's static-position injection; declare it explicitly so + // the test exercises the browser default. + target.style.position = "static"; + const other = document.createElement("div"); + other.id = "other"; + parent.append(target, other); + document.body.append(parent); + return { parent, target, other }; +} + +interface CapturedBatchCall { + batches: DomEditPatchBatch[]; + options: { label: string; coalesceKey: string }; +} + +/** Mount the REAL commit hook (persist layer mocked at commitDomEditPatchBatches). */ +function renderCommitHook(captured: CapturedBatchCall[]) { + type Commit = ReturnType["handleDomZIndexReorderCommit"]; + let commit: Commit | undefined; + function Harness() { + ({ handleDomZIndexReorderCommit: commit } = useElementLifecycleOps({ + activeCompPath: "index.html", + showToast: vi.fn(), + writeProjectFile: vi.fn(async () => {}), + domEditSaveTimestampRef: { current: 0 }, + editHistory: { recordEdit: vi.fn(async () => {}) }, + projectIdRef: { current: null }, + reloadPreview: vi.fn(), + clearDomSelection: vi.fn(), + commitDomEditPatchBatches: async (batches, options) => { + captured.push({ batches, options }); + }, + })); + return null; + } + const hookHost = document.createElement("div"); + document.body.append(hookHost); + const hookRoot = createRoot(hookHost); + act(() => hookRoot.render()); + return { commit: commit!, cleanup: () => act(() => hookRoot.unmount()) }; +} + +describe("CanvasContextMenu — z-action commit path", () => { + it("never mutates live styles itself and persists the position patch for a static element", async () => { + const { target } = makeStaticFamily(); + const selection = makeSelection("Target", target); + const captured: CapturedBatchCall[] = []; + const { commit, cleanup } = renderCommitHook(captured); + + // Wire onApplyZIndex the way the app does (PreviewOverlays → the commit + // hook), asserting the menu has NOT touched the DOM when it fires — the + // hook must capture true pre-change styles for its rollback. + const stylesAtApply: Array<{ zIndex: string; position: string }> = []; + renderMenu({ + selection, + onApplyZIndex: (patches, action) => { + stylesAtApply.push({ zIndex: target.style.zIndex, position: target.style.position }); + const { entries } = resolveZIndexEntries(selection, patches); + void commit(entries, undefined, action); + }, + }); + + await act(async () => pressMenuItem("Bring forward")); + + // The menu left the element pristine; only the commit hook wrote styles. + expect(stylesAtApply).toEqual([{ zIndex: "", position: "static" }]); + expect(target.style.zIndex).toBe("1"); + expect(target.style.position).toBe("relative"); + + // The persisted payload carries BOTH the z-index and the injected position, + // so the reorder survives the post-commit reloadPreview(). + expect(captured).toHaveLength(1); + const targetPatch = captured[0]?.batches + .flatMap((batch) => batch.patches) + .find((patch) => patch.target.id === "target"); + expect(targetPatch?.operations).toEqual( + expect.arrayContaining([ + { type: "inline-style", property: "z-index", value: "1" }, + { type: "inline-style", property: "position", value: "relative" }, + ]), + ); + // F7: the action kind is part of the default undo coalesce key, so two + // different menu actions never merge into one undo step. + expect(captured[0]?.options.coalesceKey).toContain("bring-forward"); + + cleanup(); + }); +}); diff --git a/packages/studio/src/components/editor/CanvasContextMenu.tsx b/packages/studio/src/components/editor/CanvasContextMenu.tsx index a7a124128..2d1e87db2 100644 --- a/packages/studio/src/components/editor/CanvasContextMenu.tsx +++ b/packages/studio/src/components/editor/CanvasContextMenu.tsx @@ -7,10 +7,13 @@ * useContextMenuDismiss. * * ── Wiring (z-order persistence) ───────────────────────────────────────────── - * Z-index changes are applied optimistically to the live iframe element(s) via + * Z-index changes are resolved against the live iframe DOM via * `resolveZOrderChange`, which returns a MULTI-element patch list (tie-aware: * moving a target past an equal-z sibling can require renumbering the affected - * set). The patches are surfaced through the `onApplyZIndex` prop. + * set). The patches are surfaced through the `onApplyZIndex` prop; the menu + * itself never mutates element styles — handleDomZIndexReorderCommit applies + * the live z-index (and injects position when needed) in the same synchronous + * flow, and captures the TRUE prior styles for its failure rollback. * * The prop MUST be wired at the call site to route through the full persist * path. PreviewOverlays.tsx builds the per-patch PatchTargets (the selected @@ -27,6 +30,7 @@ import { useContextMenuDismiss } from "../../hooks/useContextMenuDismiss"; import { isZOrderActionEnabled, resolveZOrderChange, + type ZOrderAction, type ZOrderPatch, } from "./canvasContextMenuZOrder"; @@ -38,12 +42,15 @@ interface CanvasContextMenuProps { selection: DomEditSelection; onClose: () => void; /** - * Called with the resolved z-order patch list after an optimistic DOM update. - * Each patch is an { element, zIndex } pair (the target and, when a renumber - * is needed, affected siblings). Wire to handleDomZIndexReorderCommit (see - * module-level wiring comment). + * Called with the resolved z-order patch list and the menu action that + * produced it (the action feeds the undo coalesce key, so two DIFFERENT + * actions never merge into one undo step). Each patch is an + * { element, zIndex } pair (the target and, when a renumber is needed, + * affected siblings). The menu does NOT touch the live DOM — wire to + * handleDomZIndexReorderCommit, which applies the live styles itself + * (see module-level wiring comment). */ - onApplyZIndex?: (patches: ZOrderPatch[]) => void; + onApplyZIndex?: (patches: ZOrderPatch[], action: ZOrderAction) => void; /** * Delete the selected element. Wire to handleDomEditElementDelete from * useDomEditActionsContext — same path as the Delete/Backspace hotkey. @@ -93,20 +100,15 @@ export const CanvasContextMenu = memo(function CanvasContextMenu({ const el = selection.element; function handleZAction(action: ZAction) { - // No persist handler → do NOT touch the live iframe DOM. An optimistic - // write with nothing to persist just reverts on the next reload. if (!onApplyZIndex) return; const patches = resolveZOrderChange(el, action); if (patches === null) return; - // Optimistic update — visible immediately even before persist completes. - for (const patch of patches) { - patch.element.style.zIndex = String(patch.zIndex); - const view = patch.element.ownerDocument?.defaultView; - if (view && view.getComputedStyle(patch.element).position === "static") { - patch.element.style.position = "relative"; - } - } - onApplyZIndex(patches); + // Do NOT pre-apply styles here: handleDomZIndexReorderCommit writes the + // live z-index (and injects position:relative for static elements) in the + // same synchronous flow, so feedback is still instant — and it must read + // the PRE-change styles itself, both to capture true rollback values and + // to detect a static position that needs persisting. + onApplyZIndex(patches, action); onClose(); } diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index e410f2c9d..75cc29818 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -27,7 +27,7 @@ import { useDomEditCompositionRect } from "./useDomEditCompositionRect"; import { useMountEffect } from "../../hooks/useMountEffect"; import { startOffCanvasIndicatorRefresh } from "./offCanvasIndicatorRefresh"; import { CanvasContextMenu } from "./CanvasContextMenu"; -import type { ZOrderPatch } from "./canvasContextMenuZOrder"; +import type { ZOrderAction, ZOrderPatch } from "./canvasContextMenuZOrder"; import { getPreviewTargetFromPointer } from "../../utils/studioPreviewHelpers"; // Re-exports for external consumers — preserving existing import paths. @@ -91,12 +91,17 @@ interface DomEditOverlayProps { */ onDeleteSelection?: (selection: DomEditSelection) => void; /** - * Called with the resolved z-order patch list after an optimistic DOM update. - * The patch list is tie-aware and may include sibling elements (see - * canvasContextMenuZOrder). Wire to handleDomZIndexReorderCommit from + * Called with the resolved z-order patch list and the menu action that + * produced it (feeds the undo coalesce key). The patch list is tie-aware and + * may include sibling elements (see canvasContextMenuZOrder); the live DOM is + * NOT yet mutated. Wire to handleDomZIndexReorderCommit from * useDomEditActionsContext. See CanvasContextMenu.tsx module comment. */ - onApplyZIndex?: (selection: DomEditSelection, patches: ZOrderPatch[]) => void; + onApplyZIndex?: ( + selection: DomEditSelection, + patches: ZOrderPatch[], + action: ZOrderAction, + ) => void; } // fallow-ignore-next-line complexity @@ -562,8 +567,8 @@ export const DomEditOverlay = memo(function DomEditOverlay({ } onApplyZIndex={ onApplyZIndex - ? (patches) => { - onApplyZIndex(contextMenu.sel, patches); + ? (patches, action) => { + onApplyZIndex(contextMenu.sel, patches, action); } : undefined } diff --git a/packages/studio/src/components/editor/LayersPanel.tsx b/packages/studio/src/components/editor/LayersPanel.tsx index b84bb47f6..2c2ae25cd 100644 --- a/packages/studio/src/components/editor/LayersPanel.tsx +++ b/packages/studio/src/components/editor/LayersPanel.tsx @@ -15,6 +15,7 @@ import { import { Layers } from "../../icons/SystemIcons"; import { useLayerDrag, isLayerDraggable, type LayerReorderEvent } from "./useLayerDrag"; import { computeReorderZValues, getElementZIndex } from "../../player/lib/layerOrdering"; +import { deriveTimelineStoreKey } from "../../player/lib/timelineElementHelpers"; const TAG_ICONS: Record = { video: "Vi", @@ -280,9 +281,17 @@ export const LayersPanel = memo(function LayersPanel() { selector: layer.selector, selectorIndex: layer.selectorIndex, sourceFile: layer.sourceFile, + key: deriveTimelineStoreKey({ + domId: layer.id, + selector: layer.selector, + selectorIndex: layer.selectorIndex, + sourceFile: layer.sourceFile, + }), })); - handleDomZIndexReorderCommit(entries); + // "layer-drag" keeps consecutive drops of the same sibling set coalescing + // into one undo step, without merging with a context-menu z action. + handleDomZIndexReorderCommit(entries, undefined, "layer-drag"); }, [handleDomZIndexReorderCommit], ); diff --git a/packages/studio/src/components/editor/canvasContextMenuZOrder.test.ts b/packages/studio/src/components/editor/canvasContextMenuZOrder.test.ts index a0ae6b4f7..c188da517 100644 --- a/packages/studio/src/components/editor/canvasContextMenuZOrder.test.ts +++ b/packages/studio/src/components/editor/canvasContextMenuZOrder.test.ts @@ -375,6 +375,27 @@ describe("resolveZOrderChange – excludes non-painting siblings", () => { expect(order.indexOf("target")).toBeLessThan(order.indexOf("a")); }); + it("ignores