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.
This commit is contained in:
Miguel Angel Simon Sierra
2026-08-06 16:18:13 -07:00
parent 477f77629b
commit 20ef798620
2 changed files with 80 additions and 2 deletions
@@ -331,3 +331,53 @@ it("scales from the element's real box, not a hardcoded fallback", async () => {
expect(scale).toBeCloseTo(2, 1); expect(scale).toBeCloseTo(2, 1);
expect(scale).toBeLessThan(3); 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<string, number> }> })
.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);
});
@@ -73,6 +73,22 @@ function originalBoxSize(
return Number.isFinite(inline) && inline > 0 ? inline : null; 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 ────────────────────────────────────────────────────── // ── Resize intercept ──────────────────────────────────────────────────────
// fallow-ignore-next-line complexity // fallow-ignore-next-line complexity
@@ -180,6 +196,8 @@ export async function tryGsapResizeIntercept(
let scaleDraftEl: HTMLElement | null = null; let scaleDraftEl: HTMLElement | null = null;
let scaleDraftDropPoint: { x: number; y: number } | null = null; let scaleDraftDropPoint: { x: number; y: number } | null = null;
let nonUniformScale = false; let nonUniformScale = false;
/** Whether this commit writes scaleX/scaleY rather than the `scale` shorthand. */
let useScaleLonghands = false;
if (resizeGroup === "scale") { if (resizeGroup === "scale") {
// Iframe-realm element — instanceof HTMLElement fails across realms; the // Iframe-realm element — instanceof HTMLElement fails across realms; the
// selector targets composition elements, and every use below is duck-typed. // 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 // can't represent it — committing width-derived scale used to snap the
// height at drop. Commit scaleX/scaleY longhands instead; keep the uniform // height at drop. Commit scaleX/scaleY longhands instead; keep the uniform
// shorthand when the two agree (aspect-true drags, shift-drags). // 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; 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", { logResize("intercept-route", {
route: "scale-tween", route: "scale-tween",
cssW, cssW,
@@ -370,7 +398,7 @@ export async function tryGsapResizeIntercept(
// normalizes every keyframe to the longhands. For an in-range resize the // 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, // min/max window math below degenerates to the tween's own start/duration,
// so timing is unchanged. // 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 // For flat tweens, synthesize the keyframes from the tween's properties
const kfs = const kfs =
anim.keyframes?.keyframes ?? anim.keyframes?.keyframes ??