diff --git a/packages/studio/src/components/editor/DomEditCropHandles.test.tsx b/packages/studio/src/components/editor/DomEditCropHandles.test.tsx new file mode 100644 index 000000000..09c73decc --- /dev/null +++ b/packages/studio/src/components/editor/DomEditCropHandles.test.tsx @@ -0,0 +1,86 @@ +// @vitest-environment happy-dom +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, describe, expect, it } from "vitest"; +import type { DomEditSelection } from "./domEditing"; +import type { OverlayRect } from "./domEditOverlayGeometry"; +import { DomEditCropHandles } from "./DomEditCropHandles"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +const overlayRect: OverlayRect = { + left: 0, + top: 0, + width: 200, + height: 100, + editScaleX: 1, + editScaleY: 1, +}; + +function selectionFor(el: HTMLElement): DomEditSelection { + return { element: el, id: el.id, selector: `#${el.id}` } as unknown as DomEditSelection; +} + +function makeEl(id: string, clip: string): HTMLElement { + const el = document.createElement("div"); + el.id = id; + if (clip) el.style.setProperty("clip-path", clip); + document.body.append(el); + return el; +} + +function render(el: HTMLElement): { root: Root; rerender: (next: HTMLElement) => void } { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const draw = (target: HTMLElement) => + act(() => { + root.render( + undefined} + />, + ); + }); + draw(el); + return { root, rerender: draw }; +} + +// Regression: the deselect restore used a ref recomputed from RENDER state — on +// a direct A→B selection switch, state re-syncs to B before A's effect cleanup +// runs, so A used to get B's crop string (or lose its crop entirely). The +// restore value must be owned by A's own lift effect / crop gesture. +describe("DomEditCropHandles clip lift/restore", () => { + it("lifts on select and restores the inline clip verbatim on unmount", () => { + const a = makeEl("a", "inset(16px round 12px)"); + const { root } = render(a); + expect(a.style.getPropertyValue("clip-path")).toBe("none"); + act(() => root.unmount()); + expect(a.style.getPropertyValue("clip-path")).toBe("inset(16px round 12px)"); + }); + + it("restores A's own clip when switching directly to B", () => { + const a = makeEl("a", "inset(16px)"); + const b = makeEl("b", "inset(40px 8px 4px 2px)"); + const { root, rerender } = render(a); + rerender(b); + // A got ITS clip back, not B's (and not removed); B is now lifted. + expect(a.style.getPropertyValue("clip-path")).toBe("inset(16px)"); + expect(b.style.getPropertyValue("clip-path")).toBe("none"); + act(() => root.unmount()); + expect(b.style.getPropertyValue("clip-path")).toBe("inset(40px 8px 4px 2px)"); + }); + + it("never lifts an uneditable clip and leaves it untouched across select/deselect", () => { + const a = makeEl("a", "circle(50% at 50% 50%)"); + const { root } = render(a); + expect(a.style.getPropertyValue("clip-path")).toBe("circle(50% at 50% 50%)"); + act(() => root.unmount()); + expect(a.style.getPropertyValue("clip-path")).toBe("circle(50% at 50% 50%)"); + }); +}); diff --git a/packages/studio/src/components/editor/DomEditCropHandles.tsx b/packages/studio/src/components/editor/DomEditCropHandles.tsx index 746c774f9..7ccdaad3e 100644 --- a/packages/studio/src/components/editor/DomEditCropHandles.tsx +++ b/packages/studio/src/components/editor/DomEditCropHandles.tsx @@ -84,7 +84,7 @@ export function DomEditCropHandles({ const cropStateFor = (element: HTMLElement) => { const parsed = readElementCropInsets(element); const { radius, ...insets } = parsed ?? { top: 0, right: 0, bottom: 0, left: 0, radius: 0 }; - return { element, croppable: parsed !== null, insets: insets as ClipPathInsetSides, radius }; + return { element, croppable: parsed !== null, insets, radius }; }; const [state, setState] = useState(() => cropStateFor(selection.element)); @@ -102,32 +102,32 @@ export function DomEditCropHandles({ state.insets.bottom > 0 || state.insets.left > 0; - // Latest committed crop — re-applied to the element when the selection drops. - const committedRef = useRef(null); - committedRef.current = hasCrop ? buildInsetClipPathSides(state.insets, state.radius) : null; - // Lift the clip while the element is selected so the full content shows and the // cropped-away area can be dimmed; restore on deselect. Keyed on the element so // switching selections restores the previous one. Runs after render, so the // state re-sync above still reads the element's real committed clip. Restore // prefers the pre-lift inline value VERBATIM — the rebuilt inset only replaces // it after a crop gesture actually commits, so a mere select+deselect can - // never reformat (or drop) what the author wrote. + // never reformat (or drop) what the author wrote. Both refs are written only + // by THIS element's lift effect and crop gestures — never derived from render + // state, which by cleanup time already describes the NEXT selection (a direct + // A→B switch re-syncs state to B before A's cleanup runs). const liftedRef = useRef(false); const preLiftInlineClipRef = useRef(""); - const cropCommittedRef = useRef(false); + // null = no crop gesture committed this selection; "" = committed a crop + // removal; anything else = the exact committed clip-path value. + const committedClipRef = useRef(null); useEffect(() => { const el = selection.element; if (readElementCropInsets(el) === null) return; preLiftInlineClipRef.current = el.style.getPropertyValue("clip-path"); - cropCommittedRef.current = false; + committedClipRef.current = null; el.style.setProperty("clip-path", "none"); liftedRef.current = true; return () => { liftedRef.current = false; - const restore = cropCommittedRef.current - ? committedRef.current - : preLiftInlineClipRef.current || null; + const committed = committedClipRef.current; + const restore = committed !== null ? committed || null : preLiftInlineClipRef.current || null; if (restore) el.style.setProperty("clip-path", restore); else el.style.removeProperty("clip-path"); }; @@ -194,12 +194,17 @@ export function DomEditCropHandles({ const reLift = () => { if (liftedRef.current) el.style.setProperty("clip-path", "none"); }; - void Promise.resolve( - onStyleCommit?.("clip-path", buildInsetClipPathSides(state.insets, state.radius)), - ).then(() => { + const committedValue = buildInsetClipPathSides(state.insets, state.radius); + const cropped = + state.insets.top > 0 || + state.insets.right > 0 || + state.insets.bottom > 0 || + state.insets.left > 0; + void Promise.resolve(onStyleCommit?.("clip-path", committedValue)).then(() => { // Only a landed commit makes the rebuilt inset the restore value; a - // failed one keeps restoring the pre-lift clip. - cropCommittedRef.current = true; + // failed one keeps restoring the pre-lift clip. Store the value itself — + // by deselect time, render state describes the next selection. + committedClipRef.current = cropped ? committedValue : ""; reLift(); }, reLift); };