fix(studio): crop restore value is owned by the lifted element's gesture

The deselect restore read 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 after a committed crop gesture A was restored with B's crop string (or
lost its crop when B had none). The committed value is now written at
gesture-commit time (tri-state: none committed / crop removal / the exact
committed string), so cleanup never touches render state. Adds component
tests for lift/restore ordering, including the direct A→B switch and the
uneditable-clip stand-down.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-11 14:58:17 -04:00
parent fe52966728
commit b7fddd548a
2 changed files with 107 additions and 16 deletions
@@ -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(
<DomEditCropHandles
selection={selectionFor(target)}
overlayRect={overlayRect}
onStyleCommit={() => 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%)");
});
});
@@ -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<string | null>(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<string | null>(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);
};