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 ??