fix(studio): crop tool stands down for clips it cannot edit

The always-on crop lifted EVERY selected element's clip-path and restored
only what it could parse as a px inset — selecting an element with a
circle/polygon/percentage clip visually un-clipped it, and deselecting
deleted the authored clip outright.

readElementCropInsets is now tri-state (zeros = no clip, null = a clip
the tool can't represent): uneditable clips get no lift and no handles,
and the lift restores the pre-lift inline value verbatim unless a crop
gesture actually committed.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-11 04:07:57 -04:00
parent 3525c7ff52
commit cbfb6ba943
3 changed files with 98 additions and 36 deletions
@@ -77,36 +77,23 @@ export function DomEditCropHandles({
}: DomEditCropHandlesProps) { }: DomEditCropHandlesProps) {
const gestureRef = useRef<CropGestureState | null>(null); const gestureRef = useRef<CropGestureState | null>(null);
const [dragging, setDragging] = useState(false); const [dragging, setDragging] = useState(false);
const [state, setState] = useState(() => { // readElementCropInsets returns null for a clip this tool can't represent
const parsed = readElementCropInsets(selection.element); // (circle/polygon/non-px inset): the crop UI must fully stand down for that
return { // element — no lift, no handles — or select+deselect replaces the authored
element: selection.element, // clip with an inset (or deletes it).
insets: { const cropStateFor = (element: HTMLElement) => {
top: parsed.top, const parsed = readElementCropInsets(element);
right: parsed.right, const { radius, ...insets } = parsed ?? { top: 0, right: 0, bottom: 0, left: 0, radius: 0 };
bottom: parsed.bottom, return { element, croppable: parsed !== null, insets: insets as ClipPathInsetSides, radius };
left: parsed.left, };
} as ClipPathInsetSides, const [state, setState] = useState(() => cropStateFor(selection.element));
radius: parsed.radius,
};
});
// Re-sync when the selection targets a different element (reselect, or an // Re-sync when the selection targets a different element (reselect, or an
// undo/redo that re-keys the node): read its committed crop before the lift // undo/redo that re-keys the node): read its committed crop before the lift
// effect runs. Read inside the guard so a drag's per-frame setState doesn't // effect runs. Read inside the guard so a drag's per-frame setState doesn't
// re-run getComputedStyle every frame. // re-run getComputedStyle every frame.
if (state.element !== selection.element) { if (state.element !== selection.element) {
const liveInsets = readElementCropInsets(selection.element); setState(cropStateFor(selection.element));
setState({
element: selection.element,
insets: {
top: liveInsets.top,
right: liveInsets.right,
bottom: liveInsets.bottom,
left: liveInsets.left,
},
radius: liveInsets.radius,
});
} }
const hasCrop = const hasCrop =
@@ -120,17 +107,28 @@ export function DomEditCropHandles({
committedRef.current = hasCrop ? buildInsetClipPathSides(state.insets, state.radius) : 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 // Lift the clip while the element is selected so the full content shows and the
// cropped-away area can be dimmed; restore the committed crop on deselect. Keyed // cropped-away area can be dimmed; restore on deselect. Keyed on the element so
// on the element so switching selections restores the previous one. Runs after // switching selections restores the previous one. Runs after render, so the
// render, so the state re-sync above still reads the element's real committed clip. // 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.
const liftedRef = useRef(false); const liftedRef = useRef(false);
const preLiftInlineClipRef = useRef("");
const cropCommittedRef = useRef(false);
useEffect(() => { useEffect(() => {
const el = selection.element; const el = selection.element;
if (readElementCropInsets(el) === null) return;
preLiftInlineClipRef.current = el.style.getPropertyValue("clip-path");
cropCommittedRef.current = false;
el.style.setProperty("clip-path", "none"); el.style.setProperty("clip-path", "none");
liftedRef.current = true; liftedRef.current = true;
return () => { return () => {
liftedRef.current = false; liftedRef.current = false;
if (committedRef.current) el.style.setProperty("clip-path", committedRef.current); const restore = cropCommittedRef.current
? committedRef.current
: preLiftInlineClipRef.current || null;
if (restore) el.style.setProperty("clip-path", restore);
else el.style.removeProperty("clip-path"); else el.style.removeProperty("clip-path");
}; };
}, [selection.element]); }, [selection.element]);
@@ -198,7 +196,12 @@ export function DomEditCropHandles({
}; };
void Promise.resolve( void Promise.resolve(
onStyleCommit?.("clip-path", buildInsetClipPathSides(state.insets, state.radius)), onStyleCommit?.("clip-path", buildInsetClipPathSides(state.insets, state.radius)),
).then(reLift, reLift); ).then(() => {
// Only a landed commit makes the rebuilt inset the restore value; a
// failed one keeps restoring the pre-lift clip.
cropCommittedRef.current = true;
reLift();
}, reLift);
}; };
const cancelCropGesture = (event: ReactPointerEvent<HTMLElement>) => { const cancelCropGesture = (event: ReactPointerEvent<HTMLElement>) => {
@@ -212,6 +215,10 @@ export function DomEditCropHandles({
setState((prev) => ({ ...prev, insets: gesture.startInsets })); setState((prev) => ({ ...prev, insets: gesture.startInsets }));
}; };
// Uneditable clip (circle/polygon/non-px inset): the element renders exactly
// as authored and the crop tool shows nothing. All hooks above stay mounted.
if (!state.croppable) return null;
return ( return (
<> <>
{/* Dim the cropped-away area whenever the element is cropped and selected, {/* Dim the cropped-away area whenever the element is cropped and selected,
@@ -1,6 +1,8 @@
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { import {
cropRectFromInsets, cropRectFromInsets,
hugRectForElement,
readElementCropInsets,
resolveCropInsetFromEdgeDrag, resolveCropInsetFromEdgeDrag,
resolveCropInsetFromMoveDrag, resolveCropInsetFromMoveDrag,
} from "./domEditOverlayCrop"; } from "./domEditOverlayCrop";
@@ -104,3 +106,48 @@ describe("cropRectFromInsets", () => {
expect(r.height).toBe(0); expect(r.height).toBe(0);
}); });
}); });
describe("readElementCropInsets tri-state", () => {
// Regression: a clip-path the crop tool can't represent (circle/polygon/
// non-px inset) used to parse to ZEROS — indistinguishable from "no crop" —
// so selecting lifted the clip and deselecting removed/replaced it: the
// authored circle clip was silently destroyed by a mere select+deselect.
const fakeEl = (inlineClip: string) =>
({
style: { getPropertyValue: (p: string) => (p === "clip-path" ? inlineClip : "") },
ownerDocument: { defaultView: { getComputedStyle: () => ({ clipPath: "none" }) } },
}) as unknown as HTMLElement;
it("zeros for no clip", () => {
expect(readElementCropInsets(fakeEl(""))).toEqual({
top: 0,
right: 0,
bottom: 0,
left: 0,
radius: 0,
});
});
it("parses a px inset", () => {
expect(readElementCropInsets(fakeEl("inset(16px round 12px)"))).toEqual({
top: 16,
right: 16,
bottom: 16,
left: 16,
radius: 12,
});
});
it("null for a circle clip (uneditable, must not be lifted)", () => {
expect(readElementCropInsets(fakeEl("circle(50% at 50% 50%)"))).toBeNull();
});
it("null for a non-px inset (uneditable, must not be lifted)", () => {
expect(readElementCropInsets(fakeEl("inset(10%)"))).toBeNull();
});
it("hugRectForElement passes the rect through for uneditable clips", () => {
const rect = { left: 1, top: 2, width: 30, height: 40, editScaleX: 1, editScaleY: 1 };
expect(hugRectForElement(rect, fakeEl("circle(50%)"))).toEqual(rect);
});
});
@@ -28,15 +28,21 @@ export function cropRectFromInsets(
}; };
} }
/** Current inset crop of an element (inline first, computed fallback), or zeros. */ /**
export function readElementCropInsets(element: HTMLElement): ClipPathInsetSides & { * Current inset crop of an element (inline first, computed fallback).
radius: number; * Zeros = no clip (croppable, nothing cropped yet). `null` = the element
} { * carries a clip-path this tool cannot represent (circle/polygon/non-px
* inset) — croppers must not lift, edit, or restore it, or the clip gets
* silently replaced or destroyed on deselect.
*/
export function readElementCropInsets(
element: HTMLElement,
): (ClipPathInsetSides & { radius: number }) | null {
const inline = element.style.getPropertyValue("clip-path").trim(); const inline = element.style.getPropertyValue("clip-path").trim();
const value = const value =
inline || element.ownerDocument.defaultView?.getComputedStyle(element).clipPath.trim() || ""; inline || element.ownerDocument.defaultView?.getComputedStyle(element).clipPath.trim() || "";
const parsed = parseInsetClipPathSides(value === "none" ? "" : value); if (!value || value === "none") return { top: 0, right: 0, bottom: 0, left: 0, radius: 0 };
return parsed ?? { top: 0, right: 0, bottom: 0, left: 0, radius: 0 }; return parseInsetClipPathSides(value);
} }
export interface CropInsetDragInput { export interface CropInsetDragInput {
@@ -111,6 +117,8 @@ export function hugRectForElement(
element: HTMLElement, element: HTMLElement,
): CropScreenRect { ): CropScreenRect {
const insets = readElementCropInsets(element); const insets = readElementCropInsets(element);
if (insets.top <= 0 && insets.right <= 0 && insets.bottom <= 0 && insets.left <= 0) return rect; // Uneditable clip (null) can't be hugged — show the full element rect.
if (!insets || (insets.top <= 0 && insets.right <= 0 && insets.bottom <= 0 && insets.left <= 0))
return rect;
return cropRectFromInsets(rect, insets, rect.editScaleX, rect.editScaleY); return cropRectFromInsets(rect, insets, rect.editScaleX, rect.editScaleY);
} }