From 20ef79862057b13c2e7ec7b777241888a3d3ca75 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 6 Aug 2026 16:18:13 -0700 Subject: [PATCH] fix(studio): stop a uniform resize writing a scale GSAP ignores A uniform drag committed the `scale` shorthand. If the tween's keyframes already stated `scaleX` and `scaleY`, the commit left both forms in the same keyframe, and GSAP animates each property name independently, so the longhands ran alongside the shorthand and won. The resize therefore computed the right number, wrote it to the file, and did nothing: the element snapped back to its old size the moment the handle was released. Reproduced from a real session, where a drop at 384px on a 630px element wrote {scaleX: 1, scaleY: 1, scale: 0.61} and rendered at the original size. The mixing hazard was already known in the other direction, where a non-uniform drag takes a rewrite path that normalizes every keyframe to the longhands. This makes the condition symmetric: whenever the tween already speaks longhands, a uniform drag speaks them too. --- .../src/hooks/gsapResizeIntercept.test.ts | 50 +++++++++++++++++++ .../studio/src/hooks/gsapResizeIntercept.ts | 32 +++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/packages/studio/src/hooks/gsapResizeIntercept.test.ts b/packages/studio/src/hooks/gsapResizeIntercept.test.ts index 7af5f3676..f30eb42b0 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.test.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.test.ts @@ -331,3 +331,53 @@ it("scales from the element's real box, not a hardcoded fallback", async () => { expect(scale).toBeCloseTo(2, 1); expect(scale).toBeLessThan(3); }); + +/** + * The bug: a uniform drag committed the `scale` shorthand into a tween whose + * keyframes already stated `scaleX`/`scaleY`. GSAP animates each property name + * independently, so the keyframe ran as `{ scaleX: 1, scaleY: 1, scale: 0.61 }` + * and the longhands won. The resize computed the right number, wrote it, and + * the element snapped straight back to its old size on release. + */ +it("does not mix the scale shorthand into a tween that speaks longhands", async () => { + const el = document.createElement("div"); + el.id = "clip"; + el.setAttribute("data-hf-studio-original-box-width", "630"); + el.setAttribute("data-hf-studio-original-box-height", "252"); + document.body.append(el); + const selection = { id: "clip", selector: "#clip", element: el } as DomEditSelection; + const longhandTween = { + ...scaleFromTween(), + keyframes: { + keyframes: [ + { percentage: 0, properties: { scaleX: 1, scaleY: 1 } }, + { percentage: 100, properties: { scaleX: 1.15, scaleY: 1.15 } }, + ], + }, + } as unknown as GsapAnimation; + const commitMutation = vi.fn(); + + // A uniform drop, so the old code took the shorthand branch and wrote + // `scale` into keyframes that already stated the longhands. + await tryGsapResizeIntercept( + selection, + { width: 384, height: 216 }, + [longhandTween], + fakeIframe(el, { scaleX: 1, scaleY: 1 }), + commitMutation, + ); + + const frames = commitMutation.mock.calls + .map((call) => call[1] as { keyframes?: Array<{ properties: Record }> }) + .flatMap((mutation) => mutation.keyframes ?? []); + expect(frames.length).toBeGreaterThan(0); + for (const frame of frames) { + const names = Object.keys(frame.properties); + const hasShorthand = names.includes("scale"); + const hasLonghand = names.includes("scaleX") || names.includes("scaleY"); + expect(hasShorthand && hasLonghand).toBe(false); + } + // And the resize still lands: 384/630 is about 0.61. + const resized = frames.find((frame) => frame.properties.scaleX != null); + expect(resized?.properties.scaleX).toBeCloseTo(0.61, 1); +}); diff --git a/packages/studio/src/hooks/gsapResizeIntercept.ts b/packages/studio/src/hooks/gsapResizeIntercept.ts index 95ab8fdb5..6574a0a71 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.ts @@ -73,6 +73,22 @@ function originalBoxSize( return Number.isFinite(inline) && inline > 0 ? inline : null; } +/** + * Whether this tween already states scale as `scaleX`/`scaleY`. + * + * Both forms are legal, and either alone is fine. A tween holding both is not: + * GSAP animates each property name independently, so the longhands run + * alongside the shorthand and win, which silently discards whatever the + * shorthand was set to. + */ +function tweenUsesScaleLonghands(anim: GsapAnimation | null): boolean { + const isLonghand = (name: string) => name === "scaleX" || name === "scaleY"; + const inKeyframes = (anim?.keyframes?.keyframes ?? []).some((frame) => + Object.keys(frame.properties ?? {}).some(isLonghand), + ); + return inKeyframes || Object.keys(anim?.properties ?? {}).some(isLonghand); +} + // ── Resize intercept ────────────────────────────────────────────────────── // fallow-ignore-next-line complexity @@ -180,6 +196,8 @@ export async function tryGsapResizeIntercept( let scaleDraftEl: HTMLElement | null = null; let scaleDraftDropPoint: { x: number; y: number } | null = null; let nonUniformScale = false; + /** Whether this commit writes scaleX/scaleY rather than the `scale` shorthand. */ + let useScaleLonghands = false; if (resizeGroup === "scale") { // Iframe-realm element — instanceof HTMLElement fails across realms; the // selector targets composition elements, and every use below is duck-typed. @@ -210,8 +228,18 @@ export async function tryGsapResizeIntercept( // can't represent it — committing width-derived scale used to snap the // height at drop. Commit scaleX/scaleY longhands instead; keep the uniform // shorthand when the two agree (aspect-true drags, shift-drags). + // + // Unless the tween already speaks longhands, in which case a uniform drag + // has to as well. GSAP animates each property name on its own, so a + // keyframe holding `{ scaleX: 1, scaleY: 1, scale: 0.61 }` runs all three + // and the longhands win: the resize commits correctly and then does + // nothing, and the element snaps back to its old size on release. The + // tween never mixes the two forms in either direction. nonUniformScale = Math.abs(newScaleX - newScaleY) > 0.01; - resizeProps = nonUniformScale ? { scaleX: newScaleX, scaleY: newScaleY } : { scale: newScaleX }; + useScaleLonghands = nonUniformScale || tweenUsesScaleLonghands(anim); + resizeProps = useScaleLonghands + ? { scaleX: newScaleX, scaleY: newScaleY } + : { scale: newScaleX }; logResize("intercept-route", { route: "scale-tween", cssW, @@ -370,7 +398,7 @@ export async function tryGsapResizeIntercept( // normalizes every keyframe to the longhands. For an in-range resize the // min/max window math below degenerates to the tween's own start/duration, // so timing is unchanged. - if ((outsideRange || nonUniformScale) && ts !== null) { + if ((outsideRange || useScaleLonghands) && ts !== null) { // For flat tweens, synthesize the keyframes from the tween's properties const kfs = anim.keyframes?.keyframes ??