Skip to content

fix(studio): resizing a cropped element keeps the same part of it in view #4743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
miguel-heygen merged 4 commits into from
Sep 30, 2026
Merged

fix(studio): resizing a cropped element keeps the same part of it in view #4743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3d39a7a
fix(studio): resizing a cropped element keeps the same part of it in …
miguel-heygen Sep 30, 2026
48ee967
fix(studio): the crop follows each axis GSAP does not tween, while dr…
miguel-heygen Sep 30, 2026
4175ab4
fix(studio): the live-tween check reads every timeline, not only the …
miguel-heygen Sep 30, 2026
f75c3f1
Merge branch 'main' into fix/studio-resize-scales-crop
miguel-heygen Sep 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 34 additions & 66 deletions packages/studio/src/components/editor/DomEditCropHandles.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vitest";
import type { DomEditSelection } from "./domEditing";
import type { OverlayRect } from "./domEditOverlayGeometry";
import { DomEditCropHandles } from "./DomEditCropHandles";
import { isElementCropLifted } from "./domEditOverlayCrop";

(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;

Expand Down Expand Up @@ -55,77 +56,39 @@ function render(
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", () => {
// The lift never touches the element's own clip-path: select+deselect must leave what the author
// wrote verbatim, and a direct A→B switch must drop A's lift, not B's.
describe("DomEditCropHandles clip lift", () => {
it("lifts on select without rewriting the inline clip, and drops the lift on unmount", () => {
const a = makeEl("a", "inset(16px round 12px)");
const { root } = render(a);
expect(a.style.getPropertyValue("clip-path")).toBe("none");
expect(isElementCropLifted(a)).toBe(true);
expect(a.style.getPropertyValue("clip-path")).toBe("inset(16px round 12px)");
act(() => root.unmount());
expect(isElementCropLifted(a)).toBe(false);
expect(a.style.getPropertyValue("clip-path")).toBe("inset(16px round 12px)");
});

it("restores A's own clip when switching directly to B", () => {
it("drops A's lift 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(isElementCropLifted(a)).toBe(false);
expect(isElementCropLifted(b)).toBe(true);
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%)");
expect(isElementCropLifted(a)).toBe(false);
act(() => root.unmount());
expect(a.style.getPropertyValue("clip-path")).toBe("circle(50% at 50% 50%)");
});

it("re-lifts synchronously after the commit path re-applies the cropped value", async () => {
const a = makeEl("a", "inset(10px)");
let resolveCommit: (() => void) | undefined;
const pendingCommit = new Promise<void>((resolve) => {
resolveCommit = resolve;
});
const onStyleCommit = vi.fn((property: string, value: string) => {
a.style.setProperty(property, value);
return pendingCommit;
});
const { root } = render(a, onStyleCommit);
const handle = document.querySelector<HTMLButtonElement>('[aria-label="Crop right"]');
expect(handle).toBeTruthy();

act(() =>
handle!.dispatchEvent(
new PointerEvent("pointerdown", { bubbles: true, pointerId: 1, clientX: 100 }),
),
);
act(() =>
handle!.dispatchEvent(
new PointerEvent("pointermove", { bubbles: true, pointerId: 1, clientX: 80 }),
),
);
act(() =>
handle!.dispatchEvent(
new PointerEvent("pointerup", { bubbles: true, pointerId: 1, clientX: 80 }),
),
);

expect(onStyleCommit).toHaveBeenCalledWith("clip-path", "inset(10px 30px 10px 10px)");
expect(a.style.getPropertyValue("clip-path")).toBe("none");
resolveCommit?.();
await act(async () => pendingCommit);
act(() => root.unmount());
expect(a.style.getPropertyValue("clip-path")).toBe("inset(10px 30px 10px 10px)");
});

it.each([
{ name: "a crop edge", clip: "inset(10px)", handle: "Crop right", dx: -20, dy: 0 },
{
Expand Down Expand Up @@ -189,33 +152,38 @@ describe("DomEditCropHandles clip lift/restore", () => {
expect(a.style.getPropertyValue("clip-path")).toBe("inset(0px 20px 0px 0px)");
});

it("re-lifts when the crop commit rejects", async () => {
it("commits the dragged crop and stays lifted when the save fails", async () => {
const a = makeEl("a", "inset(10px)");
const onStyleCommit = vi.fn((property: string, value: string) => {
a.style.setProperty(property, value);
return Promise.reject(new Error("persist failed"));
});
const { root } = render(a, onStyleCommit);
const handle = document.querySelector<HTMLButtonElement>('[aria-label="Crop right"]');

act(() =>
handle!.dispatchEvent(
new PointerEvent("pointerdown", { bubbles: true, pointerId: 2, clientX: 100 }),
),
);
act(() =>
handle!.dispatchEvent(
new PointerEvent("pointermove", { bubbles: true, pointerId: 2, clientX: 80 }),
),
);
await act(async () => {
handle!.dispatchEvent(
new PointerEvent("pointerup", { bubbles: true, pointerId: 2, clientX: 80 }),
const handle = document.querySelector<HTMLButtonElement>('[aria-label="Crop right"]')!;
const press = (type: string, clientX: number) =>
act(() =>
handle.dispatchEvent(new PointerEvent(type, { bubbles: true, pointerId: 1, clientX })),
);
press("pointerdown", 100);
press("pointermove", 80);
await act(async () => {
press("pointerup", 80);
await Promise.resolve();
});

expect(a.style.getPropertyValue("clip-path")).toBe("none");
expect(onStyleCommit).toHaveBeenCalledWith("clip-path", "inset(10px 30px 10px 10px)");
expect(isElementCropLifted(a)).toBe(true);
act(() => root.unmount());
});

it("draws the crop the element has now, after an undo rewrites it", () => {
const a = makeEl("a", "inset(0px 20px 0px 0px)");
const { root, rerender } = render(a);
const outline = () => document.querySelector<HTMLElement>(".border-dashed")!.style.width;
expect(outline()).toBe("180px");
a.style.setProperty("clip-path", "inset(0px 50px 0px 0px)");
rerender(a);
expect(outline()).toBe("150px");
act(() => root.unmount());
});
});
Expand Down
92 changes: 30 additions & 62 deletions packages/studio/src/components/editor/DomEditCropHandles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ import { type OverlayRect, RESIZE_HANDLE_HIT_PX } from "./domEditOverlayGeometry
import {
type CropEdge,
cropRectFromInsets,
dropElementCropLift,
hasCropInsets,
liftElementCrop,
readElementCropFrame,
readElementCropInsets,
resolveCropInsetFromEdgeDrag,
resolveCropInsetFromMoveDrag,
rotateDeltaIntoFrame,
} from "./domEditOverlayCrop";
import { buildInsetClipPathSides, type ClipPathInsetSides } from "./clipPathHelpers";
import { readCropFollowingResize } from "./cropResize";

interface CropGestureState {
edge: CropEdge | "move";
Expand All @@ -19,6 +23,7 @@ interface CropGestureState {
startY: number;
startInsets: ClipPathInsetSides;
insets: ClipPathInsetSides;
radius: number;
/** Element frame captured at gesture start: pointer deltas rotate into it. */
angleDeg: number;
scaleX: number;
Expand Down Expand Up @@ -83,6 +88,7 @@ function repositionHandleSize(rect: Rect): number {
}

const EDGES: CropEdge[] = ["top", "right", "bottom", "left"];
const NO_CROP = { top: 0, right: 0, bottom: 0, left: 0, radius: 0 };

/**
* Always-on crop, integrated with the selection (no crop "mode"): while a
Expand All @@ -92,7 +98,7 @@ const EDGES: CropEdge[] = ["top", "right", "bottom", "left"];
* grid guides framing); release commits `clip-path: inset(...)` through the
* normal style-commit path (one undo step per drag). When cropped, a center
* handle pans the crop window. Corners stay free for the selection's own resize
* handle. Leaving the selection restores the committed crop. The clip-path model
* handle. Leaving the selection drops the lift. The element's clip-path
* is the source of truth — nothing here mutates layout.
*/
export function DomEditCropHandles({
Expand All @@ -109,54 +115,29 @@ export function DomEditCropHandles({
// clip with an inset (or deletes it).
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, radius };
const { top, right, bottom, left } = parsed ?? NO_CROP;
return { element, croppable: parsed !== null, insets: { top, right, bottom, left } };
};
const [state, setState] = useState(() => cropStateFor(selection.element));

// 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
// effect runs. Read inside the guard so a drag's per-frame setState doesn't
// re-run getComputedStyle every frame.
// undo/redo that re-keys the node).
if (state.element !== selection.element) {
setState(cropStateFor(selection.element));
}

const hasCrop =
state.insets.top > 0 ||
state.insets.right > 0 ||
state.insets.bottom > 0 ||
state.insets.left > 0;
// The element's clip-path is the crop; state only holds a crop drag's draft.
const committed = readCropFollowingResize(selection.element) ?? NO_CROP;
const insets = dragging ? state.insets : committed;
const hasCrop = hasCropInsets(insets);

// 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. 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("");
// 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);
// cropped-away area can be dimmed. Keyed on the element so a direct A→B switch drops A's lift.
useEffect(() => {
const el = selection.element;
if (readElementCropInsets(el) === null) return;
preLiftInlineClipRef.current = el.style.getPropertyValue("clip-path");
committedClipRef.current = null;
el.style.setProperty("clip-path", "none");
liftedRef.current = true;
return () => {
liftedRef.current = false;
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");
};
liftElementCrop(el);
return () => dropElementCropLift(el);
}, [selection.element]);

// The crop applies in the element's LOCAL frame (clip-path precedes the
Expand All @@ -169,7 +150,7 @@ export function DomEditCropHandles({
// Crop rect in FRAME-LOCAL coordinates (origin = frame top-left).
const cropRect = cropRectFromInsets(
{ left: 0, top: 0, width: frame.width, height: frame.height },
state.insets,
insets,
frame.scaleX,
frame.scaleY,
);
Expand All @@ -180,19 +161,23 @@ export function DomEditCropHandles({
event.preventDefault();
event.stopPropagation();
event.currentTarget.setPointerCapture(event.pointerId);
// Read at press: a resize may have rescaled the crop since the last render.
const pressed = readCropFollowingResize(selection.element) ?? NO_CROP;
gestureRef.current = {
edge,
pointerId: event.pointerId,
startX: event.clientX,
startY: event.clientY,
startInsets: state.insets,
insets: state.insets,
startInsets: pressed,
insets: pressed,
radius: pressed.radius,
angleDeg: frame.angleDeg,
scaleX: frame.scaleX,
scaleY: frame.scaleY,
};
// Clip is already lifted by the selection effect; just flag the drag so the
// rule-of-thirds grid shows.
setState((prev) => ({ ...prev, insets: pressed }));
setDragging(true);
};

Expand Down Expand Up @@ -234,29 +219,12 @@ export function DomEditCropHandles({
const finishCropGesture = (event: ReactPointerEvent<HTMLElement>) => {
const gesture = endCropGesture(event);
if (!gesture) return;
// Commit to the file. The commit path re-applies the value to the live
// element synchronously, so re-lift in the same turn to keep showing the full
// content + dim while selected. Re-lift again on rejection so a failed commit
// still restores crop-mode presentation without an unhandled rejection.
const el = selection.element;
const reLift = () => {
if (liftedRef.current) el.style.setProperty("clip-path", "none");
};
const { insets } = gesture;
const committedValue = buildInsetClipPathSides(insets, state.radius);
if (committedValue === buildInsetClipPathSides(gesture.startInsets, state.radius)) return;
const cropped = insets.top > 0 || insets.right > 0 || insets.bottom > 0 || insets.left > 0;
const commit = onStyleCommit?.("clip-path", committedValue);
// handleDomStyleCommit applies the persisted value to the live element
// synchronously before its first await. Restore the crop-mode lift in this
// same turn so the browser never paints that intermediate cropped state.
reLift();
void Promise.resolve(commit).then(() => {
// Only a landed commit makes the rebuilt inset the restore value; a
// 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);
// The commit writes the element's clip-path (and puts it back if the save fails);
// the lift keeps it hidden while selected. A drag that ends where it started saves nothing.
const value = buildInsetClipPathSides(gesture.insets, gesture.radius);
if (value === buildInsetClipPathSides(gesture.startInsets, gesture.radius)) return;
const commit = onStyleCommit?.("clip-path", value);
void Promise.resolve(commit).catch(() => undefined);
};

const cancelCropGesture = (event: ReactPointerEvent<HTMLElement>) => {
Expand Down
Loading
Loading