diff --git a/packages/studio/src/hooks/gsapEditOutcome.ts b/packages/studio/src/hooks/gsapEditOutcome.ts index 183a6659e..9ec2dddd1 100644 --- a/packages/studio/src/hooks/gsapEditOutcome.ts +++ b/packages/studio/src/hooks/gsapEditOutcome.ts @@ -3,7 +3,25 @@ import { editabilityForProvenance, type GsapAnimation } from "@hyperframes/core/ export type GsapEditBlockReason = "no-selector" | "unroll-required" | "source-uneditable"; export type GsapEditOutcome = - | { status: "persisted" } + | { + status: "persisted"; + /** + * Whether this edit already accounted for where the gesture left the + * element, so the caller must not persist the drag offset on top. + * + * The scale route needs it: a committed scale renders around the element + * centre rather than the dragged corner, so it measures the difference + * and writes the position itself. Every other route moves nothing the + * caller has not already been told about, and the caller owns the offset. + * + * It has to be reported rather than inferred. The caller used to guess + * from "does this element have a scale-group tween", which is true for an + * element whose scale is an instant hold — but that resize commits + * width/height, not scale, so the guess withheld an offset nobody wrote + * and the element snapped back to its authored position on every drag. + */ + ownsDragOffset?: boolean; + } | { status: "blocked"; reason: GsapEditBlockReason }; const COPY: Record = { diff --git a/packages/studio/src/hooks/gsapResizeGeometrySweep.test.ts b/packages/studio/src/hooks/gsapResizeGeometrySweep.test.ts new file mode 100644 index 000000000..29136939a --- /dev/null +++ b/packages/studio/src/hooks/gsapResizeGeometrySweep.test.ts @@ -0,0 +1,289 @@ +// @vitest-environment happy-dom +/** + * What the resize COMMITS, checked as geometry rather than as structure. + * + * The structural sweep beside this one proves a resize never addresses a + * missing animation and never leaves a tween spanning two property groups. + * Neither says the box ends up the size the user dragged it to, which is the + * thing they are actually looking at. + * + * The invariant is split by who owns the drop point, because the two halves are + * genuinely different jobs: + * + * - The committed size or scale must reproduce the RENDERED box the user + * dropped, whatever rotation is on the element. This is the resize's job in + * every route. + * - When the resize reports `ownsDragOffset`, the box must also land on the + * drop POINT, because it has taken responsibility for the position. When it + * does not, position is the drag's job and is not asserted here. + * + * Rotation is the reason this exists. The committed scale is worked out from + * the element's CSS box, and a rotated element's rendered box is not its CSS + * box — so the two are only equal if the drafted size is in CSS-box terms all + * the way through. A sweep across rotations is what tells us it is. + */ +import { afterEach, expect, it, vi } from "vitest"; +import { classifyTweenPropertyGroup } from "@hyperframes/core/gsap-parser"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { usePlayerStore } from "../player/store/playerStore"; +import { tryGsapResizeIntercept } from "./gsapResizeIntercept"; + +afterEach(() => { + vi.restoreAllMocks(); + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); + document.body.innerHTML = ""; +}); + +const LAYOUT = { left: 120, top: 520 }; + +interface Pose { + box: { w: number; h: number }; + pos: { x: number; y: number }; + scale: { x: number; y: number }; +} + +/** The AABB a browser reports for `translate() rotate() scale()` about the centre. */ +function renderRect(pose: Pose, rotationDeg: number) { + const rad = (rotationDeg * Math.PI) / 180; + const [cos, sin] = [Math.abs(Math.cos(rad)), Math.abs(Math.sin(rad))]; + const [sw, sh] = [pose.box.w * pose.scale.x, pose.box.h * pose.scale.y]; + const w = sw * cos + sh * sin; + const h = sw * sin + sh * cos; + const cx = LAYOUT.left + pose.box.w / 2 + pose.pos.x; + const cy = LAYOUT.top + pose.box.h / 2 + pose.pos.y; + return { x: cx - w / 2, y: cy - h / 2, w, h }; +} + +type Props = Record; + +function tween(id: string, properties: Props, duration: number): GsapAnimation { + return { + id, + targetSelector: "#el", + propertyGroup: classifyTweenPropertyGroup(properties), + method: "to", + properties, + position: 0, + resolvedStart: 0, + duration, + ...(duration === 0 ? { extras: { immediateRender: "__raw:true" } } : {}), + } as unknown as GsapAnimation; +} + +interface Case { + name: string; + /** The element's untransformed CSS box. */ + box: { w: number; h: number }; + /** Where it sat, and at what scale, before the gesture. */ + base: { x: number; y: number }; + liveScale: { x: number; y: number }; + rotation: number; + /** The box the user dragged to, and where the draft put it. */ + drop: { w: number; h: number; x: number; y: number }; + animations: () => GsapAnimation[]; + /** Whether this route takes responsibility for where the box lands. */ + settles: boolean; +} + +const ROTATIONS = [0, -8, 45, -47, 90, 180]; + +/** + * The routes a resize can take, and whether each SETTLES the drop point. + * + * Only a committed scale moves the box: it renders about the element centre + * rather than the dragged corner, so the route measures the difference and + * writes the position. Every other route commits width and height and moves + * nothing, which leaves the anchor to the drag. An element whose scale is an + * instant hold has a scale tween and still commits size, so it belongs with + * the size routes here however it looks from the animation list. + */ +const ROUTES = { + "scale tween": { animations: () => [tween("#el-scale", { scale: 1 }, 2)], settles: true }, + "scale longhands": { + animations: () => [tween("#el-scale", { scaleX: 1, scaleY: 1 }, 2)], + settles: true, + }, + "scale instant hold": { animations: () => [tween("#el-scale", { scale: 1 }, 0)], settles: false }, + "size tween": { + animations: () => [tween("#el-size", { width: 630, height: 408 }, 2)], + settles: false, + }, + "size instant hold": { + animations: () => [tween("#el-size", { width: 630, height: 408 }, 0)], + settles: false, + }, +} as const; + +function buildCases(): Case[] { + const cases: Case[] = []; + for (const [routeName, route] of Object.entries(ROUTES)) { + for (const rotation of ROTATIONS) { + for (const [dropName, drop] of Object.entries({ + shrink: { w: 326, h: 213, x: 60, y: 40 }, + grow: { w: 980, h: 640, x: -120, y: -90 }, + "near zero": { w: 12, h: 8, x: 200, y: 160 }, + "aspect flip": { w: 900, h: 90, x: 10, y: 10 }, + })) { + cases.push({ + name: `${routeName} / rotation ${rotation} / ${dropName}`, + box: { w: 630, h: 408 }, + base: { x: 40, y: 25 }, + liveScale: { x: 1, y: 1 }, + rotation, + drop, + animations: route.animations, + settles: route.settles, + }); + } + } + } + return cases; +} + +const CASES = buildCases(); + +/** The scale and size the run committed, read at the playhead. */ +function committed(calls: unknown[][]) { + let scale: { x: number; y: number } | null = null; + let size: { w: number; h: number } | null = null; + const take = (source: Props | undefined) => { + if (!source) return; + const sx = source.scaleX ?? source.scale; + const sy = source.scaleY ?? source.scale; + if (sx != null && sy != null) scale = { x: sx, y: sy }; + if (source.width != null && source.height != null) { + size = { w: source.width, h: source.height }; + } + }; + for (const call of calls) { + const mutation = call[1] as { + properties?: Props; + percentage?: number; + keyframes?: Array<{ percentage: number; properties: Props }>; + }; + if (mutation.keyframes) { + for (const frame of mutation.keyframes) if (frame.percentage === 0) take(frame.properties); + continue; + } + if (mutation.percentage != null && mutation.percentage !== 0) continue; + take(mutation.properties); + } + return { scale, size }; +} + +/** The element as the gesture leaves it: drafted box, base pose, live pose. */ +function mountCase(testCase: Case, live: Pose) { + const el = document.createElement("div"); + el.id = "el"; + el.setAttribute("data-hf-studio-original-box-width", String(testCase.box.w)); + el.setAttribute("data-hf-studio-original-box-height", String(testCase.box.h)); + el.setAttribute("data-hf-drag-gsap-base-x", String(testCase.base.x)); + el.setAttribute("data-hf-drag-gsap-base-y", String(testCase.base.y)); + el.setAttribute("data-hf-studio-box-size", "true"); + el.style.width = `${testCase.drop.w}px`; + el.style.height = `${testCase.drop.h}px`; + document.body.append(el); + + el.getBoundingClientRect = () => { + const w = Number.parseFloat(el.style.width) || live.box.w; + const h = Number.parseFloat(el.style.height) || live.box.h; + const rect = renderRect({ ...live, box: { w, h } }, testCase.rotation); + return { ...rect, width: rect.w, height: rect.h } as unknown as DOMRect; + }; + const gsap = { + set: (_target: Element, vars: Props) => { + if (vars.x != null) live.pos.x = vars.x; + if (vars.y != null) live.pos.y = vars.y; + if (vars.scaleX != null) live.scale.x = vars.scaleX; + if (vars.scaleY != null) live.scale.y = vars.scaleY; + }, + getProperty: (_target: Element, prop: string) => + ({ + scaleX: live.scale.x, + scaleY: live.scale.y, + x: live.pos.x, + y: live.pos.y, + rotation: testCase.rotation, + })[prop] ?? 0, + }; + Object.assign(window, { gsap }); + const iframe = { + contentWindow: { gsap, __timelines: { main: { getChildren: () => [] } } }, + contentDocument: document, + } as unknown as HTMLIFrameElement; + return { el, iframe }; +} + +/** One run: mount the case, drive the intercept, hand the result to the judge. */ +async function runCase(testCase: Case): Promise { + document.body.innerHTML = ""; + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); + const live: Pose = { + box: { ...testCase.box }, + pos: { x: testCase.drop.x, y: testCase.drop.y }, + scale: { ...testCase.liveScale }, + }; + const { el, iframe } = mountCase(testCase, live); + const dropped = el.getBoundingClientRect(); + const animations = testCase.animations(); + const commitMutation = vi.fn(); + + const outcome = await tryGsapResizeIntercept( + { id: "el", selector: "#el", element: el } as DomEditSelection, + { width: testCase.drop.w, height: testCase.drop.h }, + animations, + iframe, + commitMutation as never, + async () => animations, + ); + + const { scale, size } = committed(commitMutation.mock.calls); + const settled = renderRect( + { box: size ?? testCase.box, pos: { ...live.pos }, scale: scale ?? testCase.liveScale }, + testCase.rotation, + ); + const owns = outcome.status === "persisted" && outcome.ownsDragOffset === true; + return judge(testCase, dropped, settled, owns); +} + +/** + * What the run got wrong, if anything. 1px: position rounds to whole pixels and + * scale keeps three decimals. + */ +function judge( + testCase: Case, + dropped: DOMRect, + settled: { x: number; y: number; w: number; h: number }, + owns: boolean, +): string[] { + const off = (a: number, b: number) => Math.abs(a - b) > 1; + if (off(settled.w, dropped.width) || off(settled.h, dropped.height)) { + return [ + `${testCase.name} — box ${settled.w.toFixed(1)}x${settled.h.toFixed(1)}` + + `, dropped ${dropped.width.toFixed(1)}x${dropped.height.toFixed(1)}`, + ]; + } + // Claiming the drop point is only honest for the routes that settle it. The + // size routes commit width and height and move nothing, so the drag still owns + // the anchor — and a run that claims otherwise makes the caller withhold an + // offset nobody writes. The position check below cannot see that on its own: + // the fixture's live pose starts at the drop, which is where the gesture + // leaves it, so a size route trivially "lands" there. + if (owns !== testCase.settles) { + return [`${testCase.name} — ownsDragOffset ${owns}, expected ${testCase.settles}`]; + } + if (owns && (off(settled.x, dropped.x) || off(settled.y, dropped.y))) { + return [ + `${testCase.name} — landed ${settled.x.toFixed(1)},${settled.y.toFixed(1)}` + + `, dropped ${dropped.x.toFixed(1)},${dropped.y.toFixed(1)}`, + ]; + } + return []; +} + +it(`sweeps ${CASES.length} rotations and drops for the box the user dropped`, async () => { + const failures: string[] = []; + for (const testCase of CASES) failures.push(...(await runCase(testCase))); + expect(failures).toEqual([]); +}); diff --git a/packages/studio/src/hooks/gsapResizeIntercept.test.ts b/packages/studio/src/hooks/gsapResizeIntercept.test.ts index ba4a03db0..4045b28c8 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.test.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.test.ts @@ -98,7 +98,7 @@ it("updates a duration-zero size hold in place instead of converting it to keyfr commitMutation, ); - expect(handled).toEqual({ status: "persisted" }); + expect(handled).toMatchObject({ status: "persisted" }); expect(commitMutation).toHaveBeenCalledTimes(1); expect(commitMutation.mock.calls[0]![1]).toEqual({ type: "update-properties", @@ -246,7 +246,7 @@ async function runResize( commitMutation as never, async () => [keyframedScaleFixture()], ); - expect(handled).toEqual({ status: "persisted" }); + expect(handled).toMatchObject({ status: "persisted" }); return committed; } diff --git a/packages/studio/src/hooks/gsapResizeIntercept.ts b/packages/studio/src/hooks/gsapResizeIntercept.ts index 82a6fbfb8..71b1adfba 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.ts @@ -149,7 +149,20 @@ export async function tryGsapResizeIntercept( if (!anim || isInstantHold(anim)) { const sel = selectorFromSelection(selection) ?? writeTargetSelector(selection); if (!sel) return { status: "blocked", reason: "no-selector" }; - const sizeSet = anim ?? findSizeSetAnimation(workingAnimations, sel, selection.element); + // A scale hold is not a size hold. + // + // `anim` is the tween resolved for THIS resize's group, and for a + // scale-driven element that is the one carrying `scale`. Handing it to the + // size commit wrote `width` and `height` into it, leaving one tween that + // spans two property groups — which the parser then classifies as neither, + // so it loses its group suffix and its id along with it. Every later edit + // of that element looked for a scale tween and a size tween, found no + // group at all, and the element became uneditable: "animation not found". + // Size goes to a size hold of its own, and the scale hold is left alone. + const sizeSet = + resizeGroup === "size" + ? (anim ?? findSizeSetAnimation(workingAnimations, sel, selection.element)) + : findSizeSetAnimation(workingAnimations, sel, selection.element); // If the element is animated (has a real tween, not just a static size // hold), keyframe the size at the playhead so other keyframes keep theirs — @@ -239,7 +252,14 @@ export async function tryGsapResizeIntercept( // 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; + // + // "Agree" is measured in PIXELS, not in scale. A fixed 0.01 of scale is + // invisible on a 40px box and two pixels of height on a 408px one, so a + // free drag whose axes happened to land within it silently gave back a box + // shorter than the one dropped. The question is only ever whether using one + // value for both axes would move an edge, so ask that. + const uniformDrift = Math.abs(newScaleX - newScaleY) * cssH; + nonUniformScale = uniformDrift > 0.5; useScaleLonghands = nonUniformScale || tweenUsesScaleLonghands(anim); resizeProps = useScaleLonghands ? { scaleX: newScaleX, scaleY: newScaleY } @@ -290,10 +310,13 @@ export async function tryGsapResizeIntercept( // ponytail: for a 3D-rotated element the rects are AABBs, so the anchor is // approximate rather than corner-exact. // fallow-ignore-next-line complexity - const finalizeScaleResizeCommit = async () => { - if (!scaleDraftEl) return; + const finalizeScaleResizeCommit = async (): Promise => { + // Only the scale route captures the element, so a null draft means this + // resize took the size route and never moved anything: the drop point is + // the drag's to settle, not ours. + if (!scaleDraftEl) return false; clearStudioBoxSize(scaleDraftEl); - if (!scaleDraftDropPoint || !selector) return; + if (!scaleDraftDropPoint || !selector) return false; // Put the committed scale on the live element before measuring. // // This step reads where the commit lands the box and shifts the position @@ -331,10 +354,12 @@ export async function tryGsapResizeIntercept( setElementGsapPosition(scaleDraftEl, base.x, base.y); const post = scaleDraftEl.getBoundingClientRect(); const residual = { x: scaleDraftDropPoint.x - post.x, y: scaleDraftDropPoint.y - post.y }; - if (!Number.isFinite(residual.x) || !Number.isFinite(residual.y)) return; + if (!Number.isFinite(residual.x) || !Number.isFinite(residual.y)) return false; if (Math.abs(residual.x) < 0.5 && Math.abs(residual.y) < 0.5) { logResize("scale-finalize", { skipped: "already-on-drop-point", residual, base }); - return; + // Settled, with nothing to write. Still ours: forwarding the drag offset + // on top would move the box off the point it is already sitting on. + return true; } // The ONE corrected position — rounded once so the live runtime and the // persisted file agree exactly (commitStaticGsapPosition composes the same @@ -385,13 +410,14 @@ export async function tryGsapResizeIntercept( commitMutation, fetchAnimations: fetchFallbackAnimations, }); - return; + return true; } const existingSet = findExistingPositionWrite(currentAnimations, selector, selection.element); await commitStaticGsapPosition(selection, delta, base, selector, existingSet, { commitMutation, fetchAnimations: fetchFallbackAnimations, }); + return true; }; // With auto-keyframe off (#1808), `anim` is already a real (non-"set") @@ -408,8 +434,7 @@ export async function tryGsapResizeIntercept( { commitMutation, fetchAnimations: fetchFallbackAnimations }, "Resize animation", ); - await finalizeScaleResizeCommit(); - return { status: "persisted" }; + return { status: "persisted", ownsDragOffset: await finalizeScaleResizeCommit() }; } const ct = usePlayerStore.getState().currentTime; @@ -520,8 +545,7 @@ export async function tryGsapResizeIntercept( softReload: true, }, ); - await finalizeScaleResizeCommit(); - return { status: "persisted" }; + return { status: "persisted", ownsDragOffset: await finalizeScaleResizeCommit() }; } const SIZE_PROPS = new Set(["width", "height"]); @@ -542,8 +566,7 @@ export async function tryGsapResizeIntercept( }, { label: `Resize (keyframe ${pct}%)`, softReload: true }, ); - await finalizeScaleResizeCommit(); - return { status: "persisted" }; + return { status: "persisted", ownsDragOffset: await finalizeScaleResizeCommit() }; } // ── Rotation intercept ──────────────────────────────────────────────────── diff --git a/packages/studio/src/hooks/gsapResizeMixedTween.test.ts b/packages/studio/src/hooks/gsapResizeMixedTween.test.ts new file mode 100644 index 000000000..b5665e253 --- /dev/null +++ b/packages/studio/src/hooks/gsapResizeMixedTween.test.ts @@ -0,0 +1,168 @@ +// @vitest-environment happy-dom +/** + * Resizing `#card` in the shipped playground fails with "animation not found". + * + * The element carries five tweens, all at position 0 and all duration 0, and + * one of them mixes `scale` with `width`/`height`. That spans two property + * groups, so the parser gives it no group at all and the bare id `#card-to-0` — + * which means the resize finds neither a scale tween nor a size tween and has + * to split the mixed one apart before it can commit. + * + * This drives the real intercept against that exact set, with a server stand-in + * that answers the way the real one does: an id it cannot find is a 404. Any id + * the intercept sends that is not in the list it was last handed reproduces the + * failure. + */ +import { afterEach, expect, it, vi } from "vitest"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { usePlayerStore } from "../player/store/playerStore"; +import { tryGsapResizeIntercept } from "./gsapResizeIntercept"; + +afterEach(() => { + vi.restoreAllMocks(); + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); + document.body.innerHTML = ""; +}); + +function hold( + id: string, + group: string | undefined, + properties: Record, +): GsapAnimation { + return { + id, + targetSelector: "#card", + propertyGroup: group, + method: "to", + properties, + position: 0, + resolvedStart: 0, + duration: 0, + extras: { immediateRender: "__raw:true" }, + } as unknown as GsapAnimation; +} + +/** `#card` as the playground actually holds it. */ +function cardAnimations(): GsapAnimation[] { + return [ + hold("#card-to-0-other", "other", { rotationY: -540, rotationX: 720, _auto: 0 }), + hold("#card-to-0-rotation", "rotation", { rotation: 720 }), + hold("#card-to-0", undefined, { scale: 1.2, width: 326, height: 213 }), + hold("#card-to-0-position", "position", { x: -1180, y: 128 }), + hold("#card-to-0-other-2", "other", { z: 50 }), + ]; +} + +it("never sends an animation id the server does not have", async () => { + const el = document.createElement("div"); + el.id = "card"; + el.setAttribute("data-hf-studio-original-box-width", "326"); + el.setAttribute("data-hf-studio-original-box-height", "213"); + document.body.append(el); + + // The server's view of the file, and its 404. + let current = cardAnimations(); + const rejected: string[] = []; + const sent: string[] = []; + const commitMutation = vi.fn(async (_selection: unknown, mutation: Record) => { + const animationId = mutation.animationId as string | undefined; + if (animationId) { + sent.push(`${String(mutation.type)}:${animationId}`); + if (!current.some((a) => a.id === animationId)) { + rejected.push(animationId); + throw new Error("animation not found"); + } + } + // Splitting the mixed tween is what the real server does with it. + if (mutation.type === "split-into-property-groups") { + current = [ + hold("#card-to-0-other", "other", { rotationY: -540, rotationX: 720, _auto: 0 }), + hold("#card-to-0-rotation", "rotation", { rotation: 720 }), + hold("#card-to-0-position", "position", { x: -1180, y: 128 }), + hold("#card-to-0-other-2", "other", { z: 50 }), + hold("#card-to-0-scale", "scale", { scale: 1.2 }), + hold("#card-to-0-size", "size", { width: 326, height: 213 }), + ]; + } + }); + + const selection = { id: "card", selector: "#card", element: el } as DomEditSelection; + await tryGsapResizeIntercept( + selection, + { width: 500, height: 320 }, + cardAnimations(), + null, + commitMutation as never, + async () => current, + ); + + expect(rejected).toEqual([]); + expect(sent).not.toHaveLength(0); +}); + +/** + * How `#card` got into that state: a resize wrote `width`/`height` into the + * tween that carried `scale`. + * + * One tween spanning two property groups is classified as neither, so it loses + * its group suffix and its id with it — and every later edit of the element + * looks for a scale tween and a size tween, finds no group at all, and fails. + */ +it("does not write size into the tween that carries scale", async () => { + const el = document.createElement("div"); + el.id = "card"; + el.setAttribute("data-hf-studio-original-box-width", "630"); + el.setAttribute("data-hf-studio-original-box-height", "408"); + document.body.append(el); + + // The element before the damage: one instant scale hold, nothing else. + const scaleHold = hold("#card-to-0-scale", "scale", { scale: 1.2 }); + const commitMutation = vi.fn(); + + await tryGsapResizeIntercept( + { id: "card", selector: "#card", element: el } as DomEditSelection, + { width: 326, height: 213 }, + [scaleHold], + null, + commitMutation as never, + async () => [scaleHold], + ); + + const intoScaleHold = commitMutation.mock.calls + .map((call) => call[1] as { animationId?: string; properties?: Record }) + .filter((mutation) => mutation.animationId === "#card-to-0-scale") + .flatMap((mutation) => Object.keys(mutation.properties ?? {})); + + expect(intoScaleHold).not.toContain("width"); + expect(intoScaleHold).not.toContain("height"); +}); + +/** + * Whether the caller must persist the drag offset is the resize's answer to + * give, not something to infer from the element's tweens. + * + * An element whose scale is an instant hold HAS a scale-group tween and still + * commits width/height. Guessing from the tweens withheld an offset nobody had + * written, and the element snapped back to its authored position on every drag. + */ +it("leaves the drag offset to the caller when it commits size, not scale", async () => { + const el = document.createElement("div"); + el.id = "card"; + el.setAttribute("data-hf-studio-original-box-width", "630"); + el.setAttribute("data-hf-studio-original-box-height", "408"); + document.body.append(el); + + const scaleHold = hold("#card-to-0-scale", "scale", { scale: 1.2 }); + const outcome = await tryGsapResizeIntercept( + { id: "card", selector: "#card", element: el } as DomEditSelection, + { width: 326, height: 213 }, + [scaleHold], + null, + vi.fn() as never, + async () => [scaleHold], + ); + + expect(outcome.status).toBe("persisted"); + expect(outcome.status === "persisted" && outcome.ownsDragOffset).not.toBe(true); +}); diff --git a/packages/studio/src/hooks/gsapResizeSweep.test.ts b/packages/studio/src/hooks/gsapResizeSweep.test.ts new file mode 100644 index 000000000..43e2b9768 --- /dev/null +++ b/packages/studio/src/hooks/gsapResizeSweep.test.ts @@ -0,0 +1,274 @@ +// @vitest-environment happy-dom +/** + * Every shape of animated element a composition can hand the resize, swept. + * + * Both faults this branch fixes were found one composition at a time, which is + * a bad way to find the third. So this drives the real intercept across the + * cross-product of what an element's tweens can look like and holds every run + * to the two rules that were broken: + * + * 1. Never address an animation the source does not have. Sending a stale id + * is what "animation not found" is, and it leaves the element unsavable. + * 2. Never leave a tween spanning two property groups. The parser classifies + * such a tween as neither, so it loses its group suffix and its id along + * with it, and every later edit has nothing to address. + * + * The server stand-in answers the way the real one does — an id it cannot find + * is a rejection — and applies what it is told, so a run that corrupts the + * animation list is caught by the next mutation in the same run rather than by + * a person noticing weeks later. + */ +import { afterEach, expect, it, vi } from "vitest"; +import { classifyTweenPropertyGroup } from "@hyperframes/core/gsap-parser"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { usePlayerStore } from "../player/store/playerStore"; +import { tryGsapResizeIntercept } from "./gsapResizeIntercept"; + +afterEach(() => { + vi.restoreAllMocks(); + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); + document.body.innerHTML = ""; +}); + +type Props = Record; + +function tween(id: string, properties: Props, duration: number): GsapAnimation { + return { + id, + targetSelector: "#el", + propertyGroup: classifyTweenPropertyGroup(properties), + method: "to", + properties, + position: 0, + resolvedStart: 0, + duration, + ...(duration === 0 ? { extras: { immediateRender: "__raw:true" } } : {}), + } as unknown as GsapAnimation; +} + +/** The dimensions an element's animations actually vary across. */ +const SCALE = { + none: null, + "instant hold": () => tween("#el-scale", { scale: 1.2 }, 0), + tween: () => tween("#el-scale", { scale: 1.2 }, 2), + longhands: () => tween("#el-scale", { scaleX: 1.2, scaleY: 1.1 }, 2), +} as const; +const SIZE = { + none: null, + "instant hold": () => tween("#el-size", { width: 300, height: 200 }, 0), + tween: () => tween("#el-size", { width: 300, height: 200 }, 2), +} as const; +const POSITION = { + none: null, + "static hold": () => tween("#el-position", { x: 40, y: 60 }, 0), + tween: () => tween("#el-position", { x: 40, y: 60 }, 2), +} as const; +const EXTRA = { + none: null, + // What a 3D card carries, and the shape that produced two same-group ids. + "3d and rotation": () => [ + tween("#el-other", { rotationY: -540, rotationX: 720, _auto: 0 }, 0), + tween("#el-rotation", { rotation: 720 }, 0), + tween("#el-other-2", { z: 50 }, 0), + ], + // A tween that already spans two groups, which the resize has to split. + "a mixed tween": () => [tween("#el-mixed", { scale: 1.2, width: 300, height: 200 }, 0)], +} as const; + +interface Recorded { + rejected: string[]; + corrupted: string[]; +} + +/** + * The source, as the server sees it: a list of animations, a rejection for an + * id that is not in it, and the effect of each mutation applied. + */ +function fakeSource(initial: GsapAnimation[]) { + let current = initial; + const recorded: Recorded = { rejected: [], corrupted: [] }; + + const mergeInto = (id: string, properties: Props) => { + current = current.map((animation) => { + if (animation.id !== id) return animation; + const before = classifyTweenPropertyGroup(animation.properties ?? {}); + const merged = { ...(animation.properties ?? {}), ...properties }; + const after = classifyTweenPropertyGroup(merged); + // Mixing is only a fault when the resize CAUSED it. A tween that already + // spanned two groups is an input, and splitting it is the point. + if (before !== undefined && after === undefined) recorded.corrupted.push(id); + return { ...animation, properties: merged, propertyGroup: after } as GsapAnimation; + }); + }; + + const split = (id: string) => { + const target = current.find((animation) => animation.id === id); + if (!target) return; + const byGroup = new Map(); + for (const [key, value] of Object.entries(target.properties ?? {})) { + const group = classifyTweenPropertyGroup({ [key]: value as number }) ?? "other"; + byGroup.set(group, { ...(byGroup.get(group) ?? {}), [key]: value as number }); + } + current = [ + ...current.filter((animation) => animation.id !== id), + ...[...byGroup].map(([group, properties]) => + tween(`#el-split-${group}`, properties, target.duration ?? 0), + ), + ]; + }; + + const apply = (mutation: Record, id: string | undefined) => { + const properties = (mutation.properties ?? {}) as Props; + if (mutation.type === "split-into-property-groups") return id && split(id); + if (!id) { + if (mutation.type === "add") + current = [...current, tween(`#el-added-${current.length}`, properties, 0)]; + return; + } + mergeInto(id, properties); + const framed = (mutation.keyframes as Array<{ properties: Props }> | undefined) ?? []; + for (const frame of framed) mergeInto(id, frame.properties); + }; + + const commitMutation = vi.fn(async (_selection: unknown, mutation: Record) => { + const id = mutation.animationId as string | undefined; + if (id && !current.some((animation) => animation.id === id)) { + recorded.rejected.push(`${String(mutation.type)}:${id}`); + throw new Error("animation not found"); + } + apply(mutation, id); + }); + + return { commitMutation, recorded, animations: () => current }; +} + +type Dimension = Array<[string, () => GsapAnimation[]]>; + +function dimension( + entries: Record GsapAnimation | GsapAnimation[])>, +): Dimension { + return Object.entries(entries).map(([name, make]) => [ + name, + () => { + if (!make) return []; + const made = make(); + return Array.isArray(made) ? made : [made]; + }, + ]); +} + +function buildCases() { + const dimensions = [dimension(SCALE), dimension(SIZE), dimension(POSITION), dimension(EXTRA)]; + let combos: Array GsapAnimation[]]>> = [[]]; + for (const next of dimensions) { + combos = combos.flatMap((combo) => next.map((entry) => [...combo, entry])); + } + const labels = ["scale", "size", "position", "extra"]; + return combos.map((combo) => ({ + name: combo.map(([name], index) => `${labels[index]} ${name}`).join(" / "), + animations: combo.flatMap(([, make]) => make()), + })); +} + +const CASES = buildCases(); + +it(`sweeps ${CASES.length} animated shapes without a stale id or a mixed tween`, async () => { + const failures: string[] = []; + + for (const testCase of CASES) { + document.body.innerHTML = ""; + const el = document.createElement("div"); + el.id = "el"; + el.setAttribute("data-hf-studio-original-box-width", "630"); + el.setAttribute("data-hf-studio-original-box-height", "408"); + document.body.append(el); + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); + + const source = fakeSource(testCase.animations); + try { + await tryGsapResizeIntercept( + { id: "el", selector: "#el", element: el } as DomEditSelection, + { width: 326, height: 213 }, + testCase.animations, + null, + source.commitMutation as never, + async () => source.animations(), + ); + } catch (error) { + // A rejection is recorded below; anything else is worth reporting as-is. + if (!(error instanceof Error) || error.message !== "animation not found") { + failures.push(`${testCase.name} — threw ${String(error)}`); + } + } + + if (source.recorded.rejected.length > 0) { + failures.push(`${testCase.name} — stale id: ${source.recorded.rejected.join(", ")}`); + } + if (source.recorded.corrupted.length > 0) { + failures.push(`${testCase.name} — mixed tween: ${source.recorded.corrupted.join(", ")}`); + } + } + + expect(failures).toEqual([]); +}); + +/** + * Which tween a resize edits when the element has several in the same group. + * + * A composition animates the same property more than once — a scale-in early + * and a scale-out late — and the one the user means is the one under the + * playhead. Editing the wrong one changes a moment they are not looking at and + * leaves the moment they ARE looking at unchanged, which reads as "the resize + * did nothing". + */ +function scaleAt(id: string, position: number): GsapAnimation { + return { + ...tween(id, { scale: 1 }, 2), + position, + resolvedStart: position, + keyframes: { + keyframes: [ + { percentage: 0, properties: { scale: 1 } }, + { percentage: 100, properties: { scale: 1.4 } }, + ], + }, + } as unknown as GsapAnimation; +} + +const PLAYHEADS: Array<[number, string]> = [ + [0.5, "#el-early"], + [1.9, "#el-early"], + [3, "#el-early"], + [3.1, "#el-late"], + [4.5, "#el-late"], + [9, "#el-late"], +]; + +it.each(PLAYHEADS)("at t=%s edits the tween under the playhead (%s)", async (time, expected) => { + document.body.innerHTML = ""; + const el = document.createElement("div"); + el.id = "el"; + el.setAttribute("data-hf-studio-original-box-width", "630"); + el.setAttribute("data-hf-studio-original-box-height", "408"); + document.body.append(el); + usePlayerStore.setState({ currentTime: time, activeKeyframePct: null }); + + const animations = [scaleAt("#el-early", 0), scaleAt("#el-late", 4)]; + const commitMutation = vi.fn(); + await tryGsapResizeIntercept( + { id: "el", selector: "#el", element: el } as DomEditSelection, + { width: 326, height: 213 }, + animations, + null, + commitMutation as never, + async () => animations, + ); + + const touched = new Set( + commitMutation.mock.calls + .map((call) => (call[1] as { animationId?: string }).animationId) + .filter((id): id is string => id != null), + ); + expect([...touched]).toEqual([expected]); +}); diff --git a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx index bbff8cf21..89ab3f6fd 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx +++ b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx @@ -313,8 +313,8 @@ describe("useGsapAwareEditing anchored resize", () => { act(() => h.root.unmount()); }); - it("does not apply the anchor twice when scale route already settles the drop point", async () => { - mocks.resize.mockResolvedValue({ status: "persisted" }); + it("does not apply the anchor twice when the resize already settled the drop point", async () => { + mocks.resize.mockResolvedValue({ status: "persisted", ownsDragOffset: true }); const scale = { propertyGroup: "scale" } as GsapAnimation; const h = mountResizeHandler([scale]); await act(() => h.resize(h.selection, { width: 300, height: 200 }, { x: -50, y: -25 })); @@ -322,4 +322,21 @@ describe("useGsapAwareEditing anchored resize", () => { expect(h.fallback).not.toHaveBeenCalled(); act(() => h.root.unmount()); }); + + /** + * The same element, and the resize says it did NOT settle the drop point. + * + * This is the shape that broke: an element whose scale is an instant hold has + * a scale-group tween and still commits width/height. Reading the tweens said + * "scale route, it settles its own position", so the offset was withheld, + * nobody wrote it, and the element snapped back on every drag. + */ + it("applies the anchor when the resize leaves the drop point to the caller", async () => { + mocks.resize.mockResolvedValue({ status: "persisted" }); + const scale = { propertyGroup: "scale" } as GsapAnimation; + const h = mountResizeHandler([scale]); + await act(() => h.resize(h.selection, { width: 300, height: 200 }, { x: -50, y: -25 })); + expect(mocks.drag).toHaveBeenCalledTimes(1); + act(() => h.root.unmount()); + }); }); diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 188ff58f8..0e84c1582 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -258,13 +258,21 @@ export function useGsapAwareEditing({ makeFetchFallback(selection), ); assertGsapEditPersisted(outcome); + // What the resize actually did, not what its animations suggest + // it would do. An element whose scale is an instant hold has a + // scale-group tween and still commits width/height, so guessing + // from the tweens withheld an offset nobody had written and the + // element snapped back to its authored position on every drag. + const ownsDragOffset = + outcome.status === "persisted" && outcome.ownsDragOffset === true; logResize("intercept-handled", { scaleRoute, - willForwardOffset: !!(offset && !scaleRoute), + ownsDragOffset, + willForwardOffset: !!(offset && !ownsDragOffset), }); - // Scale-route resize persists its residual position internally. - // Width/height persists the already-settled anchor through drag. - if (offset && !scaleRoute) { + // A resize that moved the element itself has already written + // where it landed. Everything else leaves the anchor to the drag. + if (offset && !ownsDragOffset) { const dragOutcome = await tryGsapDragIntercept( selection, offset, @@ -275,7 +283,7 @@ export function useGsapAwareEditing({ ); assertGsapEditPersisted(dragOutcome); } - logResizeSettle(selection.element, scaleRoute ? "gsap-scale" : "gsap-size"); + logResizeSettle(selection.element, ownsDragOffset ? "gsap-scale" : "gsap-size"); return; } catch (error) { trackGsapInteractionFailure(error, selection, "resize", "Resize animated layer");