diff --git a/packages/studio/src/hooks/gsapDragCommit.test.ts b/packages/studio/src/hooks/gsapDragCommit.test.ts index 74822de145..221fbebffa 100644 --- a/packages/studio/src/hooks/gsapDragCommit.test.ts +++ b/packages/studio/src/hooks/gsapDragCommit.test.ts @@ -1,6 +1,6 @@ // @vitest-environment happy-dom -import { describe, expect, it, beforeEach } from "vitest"; +import { describe, expect, it, beforeEach, vi } from "vitest"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import type { DomEditSelection } from "../components/editor/domEditingTypes"; import { commitGsapPositionFromDrag } from "./gsapDragPositionCommit"; @@ -9,6 +9,7 @@ import { commitStaticGsapRotation, commitStaticGsapSize, findExistingPositionWrite, + materializeIfDynamic, parkPlayheadOnKeyframe, type GsapDragCommitCallbacks, } from "./gsapDragCommit"; @@ -27,6 +28,43 @@ const selection = (): DomEditSelection => }, }) as unknown as DomEditSelection; +function selectorlessSelection(): DomEditSelection { + return { + selector: ".shared", + element: document.createElement("div"), + } as unknown as DomEditSelection; +} + +describe("lower GSAP commit helpers fail closed", () => { + it("rejects instead of silently dropping a static position write without a stable selector", async () => { + const commitMutation = vi.fn(); + await expect( + commitStaticGsapPosition( + selectorlessSelection(), + { x: 10, y: 20 }, + { x: 0, y: 0 }, + ".shared", + null, + { commitMutation }, + ), + ).rejects.toMatchObject({ name: "GsapEditBlockedError", reason: "no-selector" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + + it("rejects runtime-dynamic materialization instead of rewriting source during a gesture", async () => { + const commitMutation = vi.fn(); + await expect( + materializeIfDynamic( + { ...flatTween(), hasUnresolvedKeyframes: true }, + null, + commitMutation, + selection(), + ), + ).rejects.toMatchObject({ name: "GsapEditBlockedError", reason: "source-uneditable" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); +}); + const flatTween = (): GsapAnimation => ({ id: "#puck-a-to", diff --git a/packages/studio/src/hooks/gsapDragCommit.ts b/packages/studio/src/hooks/gsapDragCommit.ts index 62fd190e75..e6f4197987 100644 --- a/packages/studio/src/hooks/gsapDragCommit.ts +++ b/packages/studio/src/hooks/gsapDragCommit.ts @@ -9,14 +9,14 @@ import { STUDIO_ORIGINAL_HEIGHT_ATTR, } from "../components/editor/manualEditsTypes"; import { usePlayerStore } from "../player/store/playerStore"; -import { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeKeyframes"; import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; import { roundTo3 } from "../utils/rounding"; -import { computeElementPercentage, idSelector, writeTargetSelector } from "./gsapShared"; +import { computeElementPercentage, writeTargetSelector } from "./gsapShared"; import { computeDraggedGsapPosition } from "./draggedGsapPosition"; import type { RuntimeTweenChange } from "./gsapRuntimePatch"; import { isGestureTransactionCommit, runGestureTransaction } from "./gestureTransaction"; import { setPatchFromUpdateProperty } from "./gsapDragStaticSetHelpers"; +import { GsapEditBlockedError } from "./gsapEditOutcome"; export { findExistingPositionWrite, findRotationSetAnimation, @@ -87,7 +87,7 @@ async function replaceKeyframedPositionHold( commitMutation: GsapDragCommitCallbacks["commitMutation"], ): Promise { const target = newTweenTarget(selection); - if (!target) return; + if (!target) throw new GsapEditBlockedError("no-selector"); const persist = async (commit: GsapDragCommitCallbacks["commitMutation"]) => { await commit( selection, @@ -130,40 +130,12 @@ export async function materializeIfDynamic( selection: DomEditSelection, ): Promise { if (!anim.hasUnresolvedKeyframes && !anim.hasUnresolvedSelector) return; - - if (anim.hasUnresolvedSelector) { - const allScanned = scanAllRuntimeKeyframes(iframe); - if (allScanned.size === 0) return; - const allElements = Array.from(allScanned.entries()).map(([id, data]) => ({ - selector: idSelector(id), - keyframes: data.keyframes, - easeEach: data.easeEach, - })); - await commitMutation( - selection, - { - type: "materialize-keyframes", - animationId: anim.id, - keyframes: allScanned.get(selection.id ?? "")?.keyframes ?? [], - allElements, - }, - { label: "Unroll dynamic animations", skipReload: true }, - ); - return `${anim.targetSelector}-to-0`; - } - - const runtime = readRuntimeKeyframes(iframe, anim.targetSelector); - if (!runtime || runtime.keyframes.length === 0) return; - await commitMutation( - selection, - { - type: "materialize-keyframes", - animationId: anim.id, - keyframes: runtime.keyframes, - easeEach: runtime.easeEach, - }, - { label: "Materialize dynamic keyframes", skipReload: true }, - ); + // Geometry commits must never rewrite runtime/computed source implicitly. + // The explicit Unroll action owns that source-destructive transition. + void iframe; + void commitMutation; + void selection; + throw new GsapEditBlockedError("source-uneditable"); } // ── Drag → GSAP position math ────────────────────────────────────────────── @@ -219,7 +191,7 @@ export async function commitStaticGsapPosition( // The patch reuses the WRITTEN target so the runtime moves exactly the element // the source write names. const target = newTweenTarget(selection); - if (!target) return; + if (!target) throw new GsapEditBlockedError("no-selector"); await callbacks.commitMutation( selection, { @@ -277,7 +249,7 @@ export async function commitStaticGsapRotation( } // New static hold → off-timeline `gsap.set` (no 0% keyframe marker) + instant patch. const target = newTweenTarget(selection); - if (!target) return; + if (!target) throw new GsapEditBlockedError("no-selector"); await callbacks.commitMutation( selection, { @@ -331,7 +303,7 @@ export async function commitStaticGsapSize( return; } const target = newTweenTarget(selection); - if (!target) return; + if (!target) throw new GsapEditBlockedError("no-selector"); await callbacks.commitMutation( selection, { diff --git a/packages/studio/src/hooks/gsapEditOutcome.ts b/packages/studio/src/hooks/gsapEditOutcome.ts new file mode 100644 index 0000000000..183a6659ee --- /dev/null +++ b/packages/studio/src/hooks/gsapEditOutcome.ts @@ -0,0 +1,72 @@ +import { editabilityForProvenance, type GsapAnimation } from "@hyperframes/core/gsap-parser"; + +export type GsapEditBlockReason = "no-selector" | "unroll-required" | "source-uneditable"; + +export type GsapEditOutcome = + | { status: "persisted" } + | { status: "blocked"; reason: GsapEditBlockReason }; + +const COPY: Record = { + "no-selector": "This layer needs a stable selector before Studio can save the edit.", + "unroll-required": + "This motion comes from a helper or loop. Choose Unroll to edit it explicitly.", + "source-uneditable": "This animation is computed at runtime. Edit the animation in the Code tab.", +}; + +export class GsapEditBlockedError extends Error { + constructor(readonly reason: GsapEditBlockReason) { + super(COPY[reason]); + this.name = "GsapEditBlockedError"; + } +} + +export function assertGsapEditPersisted(outcome: GsapEditOutcome): void { + if (outcome.status === "blocked") throw new GsapEditBlockedError(outcome.reason); +} + +function assertGsapAnimationDirectlyEditable(animation: GsapAnimation): void { + const editability = editabilityForProvenance(animation.provenance); + if (editability === "unroll") throw new GsapEditBlockedError("unroll-required"); + if ( + editability === "source" || + animation.hasUnresolvedKeyframes || + animation.hasUnresolvedSelector + ) { + throw new GsapEditBlockedError("source-uneditable"); + } +} + +export function isGsapEditBlockedError(error: unknown): error is GsapEditBlockedError { + return error instanceof GsapEditBlockedError; +} + +export function animationWritesAnyProperty( + animation: GsapAnimation, + properties: ReadonlySet, +): boolean { + return ( + Object.keys(animation.properties ?? {}).some((property) => properties.has(property)) || + Object.keys(animation.fromProperties ?? {}).some((property) => properties.has(property)) || + !!animation.keyframes?.keyframes.some((keyframe) => + Object.keys(keyframe.properties).some((property) => properties.has(property)), + ) + ); +} + +/** Fail-closed ownership check shared by drag, resize, rotate, and inspector edits. */ +export function directEditOutcomeForProperties( + animations: GsapAnimation[], + properties: ReadonlySet, +): GsapEditOutcome { + try { + for (const animation of animations) { + if (animationWritesAnyProperty(animation, properties)) { + assertGsapAnimationDirectlyEditable(animation); + } + } + return { status: "persisted" }; + } catch (error) { + if (isGsapEditBlockedError(error)) return { status: "blocked", reason: error.reason }; + throw error; + } +} diff --git a/packages/studio/src/hooks/gsapResizeIntercept.test.ts b/packages/studio/src/hooks/gsapResizeIntercept.test.ts index 4ed179dd66..cc1e241129 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.test.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.test.ts @@ -70,6 +70,8 @@ function keyframedScaleFixture(): GsapAnimation { } as unknown as GsapAnimation; } +// Resize/rotation hold tests intentionally pin the same no-conversion contract. +// fallow-ignore-next-line code-duplication it("updates a duration-zero size hold in place instead of converting it to keyframes", async () => { const el = document.createElement("div"); el.id = "box"; @@ -96,7 +98,7 @@ it("updates a duration-zero size hold in place instead of converting it to keyfr commitMutation, ); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); expect(commitMutation).toHaveBeenCalledTimes(1); expect(commitMutation.mock.calls[0]![1]).toEqual({ type: "update-properties", @@ -115,6 +117,92 @@ it("updates a duration-zero size hold in place instead of converting it to keyfr ); }); +// fallow-ignore-next-line code-duplication +it("requires explicit unroll for helper-authored resize before mutating", async () => { + const el = document.createElement("div"); + el.id = "box"; + document.body.append(el); + const selection = { id: "box", selector: "#box", element: el } as DomEditSelection; + const helperSize = { + id: "#box-to-size", + targetSelector: "#box", + propertyGroup: "size", + method: "to", + properties: { width: 150, height: 150 }, + duration: 1, + provenance: { kind: "helper", fn: "grow", callSite: 1 }, + } as unknown as GsapAnimation; + const commitMutation = vi.fn(); + + await expect( + tryGsapResizeIntercept( + selection, + { width: 344, height: 344 }, + [helperSize], + null, + commitMutation, + ), + ).resolves.toEqual({ status: "blocked", reason: "unroll-required" }); + expect(commitMutation).not.toHaveBeenCalled(); +}); + +// fallow-ignore-next-line code-duplication +it("reuses the ownership parse instead of fetching a resolved size group twice", async () => { + const el = document.createElement("div"); + el.id = "box"; + document.body.append(el); + const selection = { id: "box", selector: "#box", element: el } as DomEditSelection; + const sizeHold = { + id: "#box-set-size", + targetSelector: "#box", + propertyGroup: "size", + method: "set", + properties: { width: 150, height: 150 }, + } as unknown as GsapAnimation; + const fetchAnimations = vi.fn().mockResolvedValue([sizeHold]); + const commitMutation = vi.fn(); + + await expect( + tryGsapResizeIntercept( + selection, + { width: 344, height: 344 }, + [], + null, + commitMutation, + fetchAnimations, + ), + ).resolves.toEqual({ status: "persisted" }); + expect(fetchAnimations).toHaveBeenCalledTimes(1); + expect(commitMutation).toHaveBeenCalledWith( + selection, + expect.objectContaining({ type: "update-properties", animationId: sizeHold.id }), + expect.anything(), + ); +}); + +it("blocks when runtime size motion exists but the authored tween cannot be resolved", async () => { + const el = document.createElement("div"); + el.id = "box"; + document.body.append(el); + const selection = { id: "box", selector: "#box", element: el } as DomEditSelection; + const liveSizeTween = { + targets: () => [el], + vars: { width: 300, duration: 1 }, + duration: () => 1, + startTime: () => 0, + }; + const iframe = { + contentWindow: { __timelines: { main: { getChildren: () => [liveSizeTween] } } }, + contentDocument: document, + } as unknown as HTMLIFrameElement; + const commitMutation = vi.fn(); + + await expect( + tryGsapResizeIntercept(selection, { width: 344, height: 344 }, [], iframe, commitMutation), + ).resolves.toEqual({ status: "blocked", reason: "source-uneditable" }); + expect(commitMutation).not.toHaveBeenCalled(); +}); + it("computes a finite zero percentage for a zero-duration tween", () => { const animation = { id: "#box-to-0-size", @@ -158,7 +246,7 @@ async function runResize( commitMutation as never, async () => [keyframedScaleFixture()], ); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); return committed; } diff --git a/packages/studio/src/hooks/gsapResizeIntercept.ts b/packages/studio/src/hooks/gsapResizeIntercept.ts index e9eecef829..04eac1fc4d 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.ts @@ -24,11 +24,16 @@ import type { GsapDragCommitCallbacks } from "./gsapDragCommit"; import { pickClosestToPlayhead, readGsapPositionFromIframe } from "./gsapPositionDetection"; import { commitWholePropertyOffset } from "./gsapWholePropertyOffsetCommit"; import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; -import { isInstantHold, selectorFromSelection } from "./gsapShared"; +import { isInstantHold, selectorFromSelection, writeTargetSelector } from "./gsapShared"; import { roundTo3 } from "../utils/rounding"; import { resolveGroupTween, POSITION_CHANNELS } from "./gsapRuntimeBridge"; import { hasNonHoldTweenForElement } from "./gsapRuntimeKeyframes"; import { logResize } from "../utils/resizeDebug"; +import { + animationWritesAnyProperty, + directEditOutcomeForProperties, + type GsapEditOutcome, +} from "./gsapEditOutcome"; const IDENTITY_ONE_PROPS = new Set(["opacity", "autoAlpha", "scale", "scaleX", "scaleY"]); @@ -54,21 +59,44 @@ export async function tryGsapResizeIntercept( iframe: HTMLIFrameElement | null, commitMutation: GsapDragCommitCallbacks["commitMutation"], fetchFallbackAnimations?: () => Promise, -): Promise { +): Promise { + const fetchedAnimations = fetchFallbackAnimations ? await fetchFallbackAnimations() : []; + const allKnownAnimations = [...animations, ...fetchedAnimations]; // If the element already has a scale-group tween, resize should modify scale // (the user is resizing something whose visual size is driven by scale). // Otherwise, use the size group (width/height). - const hasScaleGroup = animations.some((a) => a.propertyGroup === "scale"); + const hasScaleGroup = allKnownAnimations.some((a) => a.propertyGroup === "scale"); const resizeGroup: PropertyGroupName = hasScaleGroup ? "scale" : "size"; + const resizeProperties = + resizeGroup === "scale" ? new Set(["scale", "scaleX", "scaleY"]) : new Set(["width", "height"]); + const editability = directEditOutcomeForProperties(allKnownAnimations, resizeProperties); + if (editability.status === "blocked") return editability; + const workingAnimations = animations.length > 0 ? animations : fetchedAnimations; + // The initial ownership fetch already supplied the complete parse. Only retain + // the fetch callback when a legacy mixed tween may be split and must then be + // re-read; otherwise resolveGroupTween would perform the same network read twice. + const postSplitFetch = workingAnimations.some((animation) => !animation.propertyGroup) + ? fetchFallbackAnimations + : undefined; const resolved = await resolveGroupTween( resizeGroup, - animations, + workingAnimations, selection, commitMutation, - fetchFallbackAnimations, + postSplitFetch, ); - let anim = resolved?.anim ?? null; + let anim = + resolved?.anim && animationWritesAnyProperty(resolved.anim, resizeProperties) + ? resolved.anim + : null; + const liveSelector = selectorFromSelection(selection); + const hasLiveResizeTween = liveSelector + ? hasNonHoldTweenForElement(iframe, liveSelector, undefined, [...resizeProperties]) + : false; + if (!anim && hasLiveResizeTween) { + return { status: "blocked", reason: "source-uneditable" }; + } logResize("intercept-enter", { hasScaleGroup, resizeGroup, @@ -77,16 +105,16 @@ export async function tryGsapResizeIntercept( size, }); if (!anim || isInstantHold(anim)) { - const sel = selectorFromSelection(selection); - if (!sel) return false; - const sizeSet = anim ?? findSizeSetAnimation(animations, sel, selection.element); + const sel = selectorFromSelection(selection) ?? writeTargetSelector(selection); + if (!sel) return { status: "blocked", reason: "no-selector" }; + const sizeSet = anim ?? 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 — // instead of a global set that resizes every frame. if (resizeGroup === "size") { const animatedTween = pickClosestToPlayhead( - animations.filter((a) => !isInstantHold(a) && resolveTweenDuration(a) > 0), + workingAnimations.filter((a) => !isInstantHold(a) && resolveTweenDuration(a) > 0), ); if (animatedTween) { logResize("intercept-route", { route: "keyframed-size", tweenId: animatedTween.id }); @@ -98,7 +126,7 @@ export async function tryGsapResizeIntercept( animatedTween, { commitMutation, fetchAnimations: fetchFallbackAnimations }, ); - if (handled) return true; + if (handled) return { status: "persisted" }; } } @@ -107,11 +135,11 @@ export async function tryGsapResizeIntercept( commitMutation, fetchAnimations: fetchFallbackAnimations, }); - return true; + return { status: "persisted" }; } const tweenDuration = resolveTweenDuration(anim); - if (tweenDuration <= 0) return false; + if (tweenDuration <= 0) return { status: "blocked", reason: "source-uneditable" }; const { activeKeyframePct, setActiveKeyframePct } = usePlayerStore.getState(); const pct = activeKeyframePct ?? computeCurrentPercentage(selection, anim); @@ -273,7 +301,7 @@ export async function tryGsapResizeIntercept( "Resize animation", ); await finalizeScaleResizeCommit(); - return true; + return { status: "persisted" }; } const ct = usePlayerStore.getState().currentTime; @@ -385,7 +413,7 @@ export async function tryGsapResizeIntercept( }, ); await finalizeScaleResizeCommit(); - return true; + return { status: "persisted" }; } const SIZE_PROPS = new Set(["width", "height"]); @@ -407,7 +435,7 @@ export async function tryGsapResizeIntercept( { label: `Resize (keyframe ${pct}%)`, softReload: true }, ); await finalizeScaleResizeCommit(); - return true; + return { status: "persisted" }; } // ── Rotation intercept ──────────────────────────────────────────────────── diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.test.ts b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts index de5c5ef78d..bc79366dfc 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.test.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts @@ -59,6 +59,67 @@ const stalePositionAnim = { afterEach(() => vi.restoreAllMocks()); describe("tryGsapDragIntercept — stale-parse guard (no resurrection after delete-all)", () => { + async function runHelperOwnedDrag(fetchFallbackAnimations?: () => Promise) { + const helperTween = { + ...stalePositionAnim, + provenance: { kind: "helper", fn: "slam", callSite: 1 }, + } as GsapAnimation; + const commitMutation = vi.fn(); + const result = await tryGsapDragIntercept( + selection, + { x: 10, y: 10 }, + [helperTween], + fakeIframe("puck-b", []), + commitMutation, + fetchFallbackAnimations, + ); + return { result, commitMutation }; + } + + it("blocks a live runtime position tween when no editable source mapping exists", async () => { + const liveTween = { + targets: () => [{ id: "puck-b" }], + vars: { y: 18, duration: 1 }, + duration: () => 1, + startTime: () => 0, + }; + const commitMutation = vi.fn(); + + const result = await tryGsapDragIntercept( + selection, + { x: 25, y: -10 }, + [], + fakeIframe("puck-b", [liveTween]), + commitMutation, + vi.fn().mockResolvedValue([]), + ); + + expect(result).toEqual({ status: "blocked", reason: "source-uneditable" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + + it("blocks a selector that cannot safely address one writable element", async () => { + const selectorless = { + ...selection, + id: undefined, + selector: undefined, + } as unknown as DomEditSelection; + const result = await tryGsapDragIntercept(selectorless, { x: 1, y: 1 }, [], null, vi.fn()); + expect(result).toEqual({ status: "blocked", reason: "no-selector" }); + }); + + it("requires explicit unroll for helper-authored source and performs no mutation", async () => { + const { result, commitMutation } = await runHelperOwnedDrag(); + expect(result).toEqual({ status: "blocked", reason: "unroll-required" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + + it("does not let an empty fallback response bypass cached helper provenance", async () => { + const { result, commitMutation } = await runHelperOwnedDrag(vi.fn().mockResolvedValue([])); + expect(result).toEqual({ status: "blocked", reason: "unroll-required" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + it("commits a static set (not the stale tween) when the runtime has no live position motion", async () => { const commitMutation = vi.fn(); // Runtime empty (tween deleted) — readRuntimeKeyframes returns null, so the @@ -73,7 +134,7 @@ describe("tryGsapDragIntercept — stale-parse guard (no resurrection after dele commitMutation, ); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); // No existing `set` for the selector → one `add` mutation with `method:"set"`. expect(commitMutation).toHaveBeenCalledTimes(1); const [, mutation] = commitMutation.mock.calls[0]; @@ -112,7 +173,7 @@ describe("tryGsapDragIntercept — stale-parse guard (no resurrection after dele commitMutation, ); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); const updates = commitMutation.mock.calls.filter(([, m]) => m.type === "update-properties"); expect(updates).toHaveLength(1); expect(updates[0][1]).toEqual({ @@ -151,7 +212,7 @@ describe("tryGsapDragIntercept — stale-parse guard (no resurrection after dele commitMutation, ); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); // One atomic in-place update, NOT an `add`/`add-keyframe`. const types = commitMutation.mock.calls.map(([, m]) => m.type); expect(types).toEqual(["update-properties"]); @@ -179,6 +240,124 @@ describe("tryGsapDragIntercept — stale-parse guard (no resurrection after dele }); describe("tryGsapRotationIntercept — instant holds", () => { + it("requires explicit unroll for helper-authored rotation before mutating", async () => { + const helperRotation = { + id: "#puck-b-to-rotation", + targetSelector: "#puck-b", + propertyGroup: "rotation", + method: "to", + properties: { rotation: 45 }, + position: 0, + duration: 1, + provenance: { kind: "helper", fn: "spin", callSite: 1 }, + } as unknown as GsapAnimation; + const commitMutation = vi.fn(); + + await expect( + tryGsapRotationIntercept(selection, 75, [helperRotation], null, commitMutation), + ).resolves.toEqual({ status: "blocked", reason: "unroll-required" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + + // Mirrors resize by design: both geometry routes must reuse one ownership parse. + // fallow-ignore-next-line code-duplication + it("reuses the ownership parse instead of fetching a resolved rotation group twice", async () => { + const rotationHold = { + id: "#puck-b-set-rotation", + targetSelector: "#puck-b", + propertyGroup: "rotation", + method: "set", + properties: { rotation: 45 }, + } as unknown as GsapAnimation; + const fetchAnimations = vi.fn().mockResolvedValue([rotationHold]); + const commitMutation = vi.fn(); + + await expect( + tryGsapRotationIntercept(selection, 75, [], null, commitMutation, fetchAnimations), + ).resolves.toEqual({ status: "persisted" }); + expect(fetchAnimations).toHaveBeenCalledTimes(1); + expect(commitMutation).toHaveBeenCalledWith( + selection, + expect.objectContaining({ type: "update-property", animationId: rotationHold.id }), + expect.anything(), + ); + }); + + it("rejects a selectorless rotation instead of reporting a handled no-op", async () => { + const selectorless = { + ...selection, + id: undefined, + selector: undefined, + } as unknown as DomEditSelection; + const commitMutation = vi.fn(); + + await expect( + tryGsapRotationIntercept(selectorless, 75, [], null, commitMutation), + ).resolves.toEqual({ status: "blocked", reason: "no-selector" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + + it("blocks helper-authored 3D rotation channels before mutating the group", async () => { + const helperRotationX = { + id: "#puck-b-to-rotation", + targetSelector: "#puck-b", + propertyGroup: "rotation", + method: "to", + properties: { rotationX: 45 }, + duration: 1, + provenance: { kind: "helper", fn: "tilt", callSite: 1 }, + } as unknown as GsapAnimation; + const commitMutation = vi.fn(); + + await expect( + tryGsapRotationIntercept(selection, 75, [helperRotationX], null, commitMutation), + ).resolves.toEqual({ status: "blocked", reason: "unroll-required" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + + it("does not let an unrelated helper-authored skew tween block 2D rotation", async () => { + const helperSkew = { + id: "#puck-b-to-rotation", + targetSelector: "#puck-b", + propertyGroup: "rotation", + method: "to", + properties: { skewX: 12 }, + duration: 1, + provenance: { kind: "helper", fn: "skew", callSite: 1 }, + } as unknown as GsapAnimation; + const commitMutation = vi.fn(); + + await expect( + tryGsapRotationIntercept(selection, 75, [helperSkew], null, commitMutation), + ).resolves.toEqual({ status: "persisted" }); + expect(commitMutation).toHaveBeenCalledWith( + selection, + expect.objectContaining({ type: "add", properties: { rotation: 75 } }), + expect.anything(), + ); + }); + + it("blocks when runtime rotation exists but the authored tween cannot be resolved", async () => { + const liveRotation = { + targets: () => [{ id: "puck-b" }], + vars: { rotation: 45, duration: 1 }, + duration: () => 1, + startTime: () => 0, + }; + const commitMutation = vi.fn(); + + await expect( + tryGsapRotationIntercept( + selection, + 75, + [], + fakeIframe("puck-b", [liveRotation]), + commitMutation, + ), + ).resolves.toEqual({ status: "blocked", reason: "source-uneditable" }); + expect(commitMutation).not.toHaveBeenCalled(); + }); + it("updates a duration-zero fromTo hold instead of converting it to keyframes", async () => { const rotationHold = { id: "#puck-b-fromTo-0-rotation", @@ -201,7 +380,7 @@ describe("tryGsapRotationIntercept — instant holds", () => { commitMutation, ); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); expect(commitMutation).toHaveBeenCalledTimes(1); expect(commitMutation.mock.calls[0]![1]).toEqual({ type: "update-property", @@ -243,40 +422,29 @@ describe("tryGsapDragIntercept — autoKeyframeEnabled toggle (#1808)", () => { }, } as unknown as GsapAnimation; - it("shifts the whole tween instead of adding a keyframe when the toggle is off", async () => { - usePlayerStore.setState({ autoKeyframeEnabled: false, currentTime: 2 }); // playhead at 100% + async function runAutoKeyframeDrag(enabled: boolean) { + usePlayerStore.setState({ autoKeyframeEnabled: enabled, currentTime: 2 }); const commitMutation = vi.fn(); - const iframe = fakeIframe("puck-b", []); - const handled = await tryGsapDragIntercept( selection, { x: -50, y: 0 }, [keyframedPositionAnim], - iframe, + fakeIframe("puck-b", []), commitMutation, ); + return { handled, types: commitMutation.mock.calls.map(([, mutation]) => mutation.type) }; + } - expect(handled).toBe(true); - const types = commitMutation.mock.calls.map(([, m]) => m.type); + it("shifts the whole tween instead of adding a keyframe when the toggle is off", async () => { + const { handled, types } = await runAutoKeyframeDrag(false); + expect(handled).toEqual({ status: "persisted" }); expect(types).toContain("replace-with-keyframes"); expect(types).not.toContain("add-keyframe"); }); it("still adds/updates a keyframe at the playhead when the toggle is on (default)", async () => { - usePlayerStore.setState({ autoKeyframeEnabled: true, currentTime: 2 }); - const commitMutation = vi.fn(); - const iframe = fakeIframe("puck-b", []); - - const handled = await tryGsapDragIntercept( - selection, - { x: -50, y: 0 }, - [keyframedPositionAnim], - iframe, - commitMutation, - ); - - expect(handled).toBe(true); - const types = commitMutation.mock.calls.map(([, m]) => m.type); + const { handled, types } = await runAutoKeyframeDrag(true); + expect(handled).toEqual({ status: "persisted" }); expect(types).not.toContain("replace-with-keyframes"); }); }); @@ -336,7 +504,7 @@ describe("tryGsapDragIntercept — motion paths", () => { it("creates a temporal keyframe at the exact playhead instead of redistributing path waypoints", async () => { const { commitMutation, handled } = await dragMotionPath(null); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); expect(commitMutation).toHaveBeenCalledWith( selection, { @@ -363,7 +531,7 @@ describe("tryGsapDragIntercept — motion paths", () => { it("keeps an explicitly selected path waypoint as a spatial edit", async () => { const { commitMutation, handled } = await dragMotionPath(50); - expect(handled).toBe(true); + expect(handled).toEqual({ status: "persisted" }); expect(commitMutation).toHaveBeenCalledWith( selection, { diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.ts b/packages/studio/src/hooks/gsapRuntimeBridge.ts index 5f8514630a..a35fb6d3f1 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.ts @@ -26,18 +26,23 @@ import { import { commitWholePropertyOffset } from "./gsapWholePropertyOffsetCommit"; import { resolveTweenDuration } from "../utils/globalTimeCompiler"; import type { GsapDragCommitCallbacks } from "./gsapDragCommit"; -import { isInstantHold, selectorFromSelection } from "./gsapShared"; +import { isInstantHold, selectorFromSelection, writeTargetSelector } from "./gsapShared"; import { findGsapPositionAnimation, pickClosestToPlayhead, readGsapPositionFromIframe, } from "./gsapPositionDetection"; import { hasNonHoldTweenForElement } from "./gsapRuntimeKeyframes"; +import { + animationWritesAnyProperty, + directEditOutcomeForProperties, + type GsapEditOutcome, +} from "./gsapEditOutcome"; // Position channels — used to scope the "has a live position tween?" check so a // sibling rotation/scale animation never forces a static position hold into the // keyframe branch (which corrupts it into a frozen duration-0 keyframed tween). -export const POSITION_CHANNELS = [ +export const POSITION_CHANNELS: string[] = [ "x", "y", "xPercent", @@ -50,6 +55,10 @@ export const POSITION_CHANNELS = [ "translateX", "translateY", ]; +const POSITION_CHANNEL_SET = new Set(POSITION_CHANNELS); + +const ROTATION_CHANNELS: string[] = ["rotation", "rotationX", "rotationY", "rotationZ"]; +const ROTATION_CHANNEL_SET = new Set(ROTATION_CHANNELS); // ── Property-group tween resolution ─────────────────────────────────────── @@ -121,11 +130,40 @@ export type { GsapDragCommitCallbacks }; /** * Attempt to handle a drag commit via the GSAP script mutation path. * - * Returns a Promise that resolves to true if the drag was handled via GSAP - * (caller should skip the CSS path), or false if no GSAP position animation - * exists. + * Returns an explicit persisted/blocked outcome. Callers must reject blocked + * outcomes so the gesture layer restores its runtime and overlay drafts. */ // fallow-ignore-next-line complexity +async function preflightGsapDragIntercept( + selection: DomEditSelection, + animations: GsapAnimation[], + iframe: HTMLIFrameElement | null, + fetchFallbackAnimations?: () => Promise, +): Promise { + const selector = selectorFromSelection(selection); + if (!selector) return { status: "blocked", reason: "no-selector" }; + + const fetchedAnimations = fetchFallbackAnimations ? await fetchFallbackAnimations() : []; + // The fallback API currently represents both a definitive empty parse and an + // exhausted fetch failure as `[]`. Keep the selected cache in the preflight + // set as well: ignoring it would let a transient fetch failure bypass helper / + // runtime-source ownership and reach a destructive split or property write. + const allKnownAnimations = [...animations, ...fetchedAnimations]; + const editability = directEditOutcomeForProperties(allKnownAnimations, POSITION_CHANNEL_SET); + if (editability.status === "blocked") return editability; + const sourceAnimations = fetchedAnimations.length > 0 ? fetchedAnimations : animations; + const posAnim = findGsapPositionAnimation(sourceAnimations, selector); + const hasLivePosition = hasNonHoldTweenForElement(iframe, selector, undefined, POSITION_CHANNELS); + + if (hasLivePosition && !posAnim) { + return { status: "blocked", reason: "source-uneditable" }; + } + if (!posAnim && !writeTargetSelector(selection)) { + return { status: "blocked", reason: "no-selector" }; + } + return { status: "persisted" }; +} + export async function tryGsapDragIntercept( selection: DomEditSelection, offset: { x: number; y: number }, @@ -133,12 +171,20 @@ export async function tryGsapDragIntercept( iframe: HTMLIFrameElement | null, commitMutation: GsapDragCommitCallbacks["commitMutation"], fetchFallbackAnimations?: () => Promise, - options?: { altKey?: boolean }, -): Promise { - const selector = selectorFromSelection(selection); - if (!selector) { - return false; + options?: { altKey?: boolean; preflightOnly?: boolean; preflightPassed?: boolean }, +): Promise { + if (!options?.preflightPassed) { + const preflight = await preflightGsapDragIntercept( + selection, + animations, + iframe, + fetchFallbackAnimations, + ); + if (preflight.status === "blocked" || options?.preflightOnly) return preflight; } + const selector = selectorFromSelection(selection); + // The preflight above proves this; retain a defensive result for DOM churn. + if (!selector) return { status: "blocked", reason: "no-selector" }; // Self-heal: enforce a single position write BEFORE committing. A corrupted // file can carry 2+ conflicting position writes for one selector (e.g. a @@ -218,11 +264,11 @@ export async function tryGsapDragIntercept( commitMutation, fetchAnimations: fetchFallbackAnimations, }); - return true; + return { status: "persisted" }; } if (!posAnim) { - return false; + return { status: "blocked", reason: "source-uneditable" }; } // Verify the anim ID is still valid in the current file. The React-state @@ -251,7 +297,7 @@ export async function tryGsapDragIntercept( } else { await commitGsapPositionFromDrag(selection, posAnim, offset, gsapPos, iframe, selector, cbs); } - return true; + return { status: "persisted" }; } // ── Runtime property readers (re-exported for external callers) ─────────── @@ -267,28 +313,47 @@ export async function tryGsapRotationIntercept( iframe: HTMLIFrameElement | null, commitMutation: GsapDragCommitCallbacks["commitMutation"], fetchFallbackAnimations?: () => Promise, -): Promise { - const selector = selectorFromSelection(selection); - if (!selector) return false; +): Promise { + const selector = selectorFromSelection(selection) ?? writeTargetSelector(selection); + if (!selector) return { status: "blocked", reason: "no-selector" }; + + const fetchedAnimations = fetchFallbackAnimations ? await fetchFallbackAnimations() : []; + const workingAnimations = animations.length > 0 ? animations : fetchedAnimations; + const editability = directEditOutcomeForProperties( + [...animations, ...fetchedAnimations], + ROTATION_CHANNEL_SET, + ); + if (editability.status === "blocked") return editability; + const postSplitFetch = workingAnimations.some((animation) => !animation.propertyGroup) + ? fetchFallbackAnimations + : undefined; // Resolve the rotation-group tween, splitting legacy mixed tweens if needed. const resolved = await resolveGroupTween( "rotation", - animations, + workingAnimations, selection, commitMutation, - fetchFallbackAnimations, + postSplitFetch, ); - const resolvedAnimations = resolved?.animations ?? animations; + const resolvedAnimations = resolved?.animations ?? workingAnimations; // Fallback: legacy heuristic for hand-written scripts - let anim = resolved?.anim ?? null; + let anim = + resolved?.anim && animationWritesAnyProperty(resolved.anim, ROTATION_CHANNEL_SET) + ? resolved.anim + : null; if (!anim) { - anim = animations.find((a) => "rotation" in a.properties || a.keyframes) ?? null; - if (!anim && fetchFallbackAnimations) { - const fresh = await fetchFallbackAnimations(); - anim = fresh.find((a) => "rotation" in a.properties || a.keyframes) ?? null; - } + anim = + workingAnimations.find((a) => animationWritesAnyProperty(a, ROTATION_CHANNEL_SET)) ?? null; + } + + const liveSelector = selectorFromSelection(selection); + const hasLiveRotationTween = liveSelector + ? hasNonHoldTweenForElement(iframe, liveSelector, undefined, ROTATION_CHANNELS) + : false; + if (!anim && hasLiveRotationTween) { + return { status: "blocked", reason: "source-uneditable" }; } // `angle` is the ABSOLUTE target rotation resolved by the gesture (gsap base + @@ -307,7 +372,7 @@ export async function tryGsapRotationIntercept( commitMutation, fetchAnimations: fetchFallbackAnimations, }); - return true; + return { status: "persisted" }; } const pct = computeCurrentPercentage(selection, anim); @@ -325,7 +390,7 @@ export async function tryGsapRotationIntercept( { commitMutation, fetchAnimations: fetchFallbackAnimations }, "Rotate animation", ); - return true; + return { status: "persisted" }; } // fallow-ignore-next-line code-duplication @@ -363,7 +428,7 @@ export async function tryGsapRotationIntercept( }, { label: `Rotate (keyframe ${pct}%)`, softReload: true }, ); - return true; + return { status: "persisted" }; } export { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeKeyframes"; diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx b/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx index f969b661dd..defec9f81f 100644 --- a/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx +++ b/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx @@ -41,14 +41,14 @@ type Commit = ( /** Renders the hook and hands its commit function to the caller via a ref callback. */ function renderHookWith( animations: GsapAnimation[], - onMutation: (mutation: Record, label: string) => void, + onMutation: (mutation: Record, label: string) => unknown | Promise, onReady: (commit: Commit) => void, ) { function Harness() { const { commitAnimatedProperties } = useAnimatedPropertyCommit({ selectedGsapAnimations: animations, gsapCommitMutation: async (_sel, mutation, options) => { - onMutation(mutation, options.label); + await onMutation(mutation, options.label); }, addGsapAnimation: vi.fn(), convertToKeyframes: vi.fn(), @@ -61,6 +61,48 @@ function renderHookWith( return mountReactHarness(); } +describe("useAnimatedPropertyCommit — ownership and rejection propagation", () => { + it("rejects a helper-authored property before sending a mutation", async () => { + const helperRotation = { + id: "#box-to-rotation", + targetSelector: "#box", + propertyGroup: "rotation", + method: "to", + properties: { rotationX: 10 }, + provenance: { kind: "helper", fn: "spin", callSite: 1 }, + } as unknown as GsapAnimation; + const mutations: Array> = []; + let commit!: Commit; + const root = renderHookWith( + [helperRotation], + (mutation) => mutations.push(mutation), + (ready) => (commit = ready), + ); + + await expect(commit(selection, { rotationX: 12 })).rejects.toMatchObject({ + name: "GsapEditBlockedError", + reason: "unroll-required", + }); + expect(mutations).toHaveLength(0); + act(() => root.unmount()); + }); + + it("rethrows a persistence failure to the telemetry wrapper", async () => { + const failure = new Error("save failed"); + let commit!: Commit; + const root = renderHookWith( + [keyframedAnim], + async () => { + throw failure; + }, + (ready) => (commit = ready), + ); + + await expect(commit(selection, { x: 50 })).rejects.toBe(failure); + act(() => root.unmount()); + }); +}); + function renderCommitHook( mutations: Array>, onReady: (commit: Commit) => void, @@ -74,35 +116,28 @@ function renderCommitHook( // off, it must shift the whole tween instead of adding/updating a keyframe // at the playhead. describe("useAnimatedPropertyCommit — autoKeyframeEnabled toggle (#1808)", () => { - it("shifts the whole tween instead of updating a keyframe when the toggle is off", async () => { - usePlayerStore.setState({ autoKeyframeEnabled: false, currentTime: 0 }); + async function runCommitWithAutoKeyframe(enabled: boolean) { + usePlayerStore.setState({ autoKeyframeEnabled: enabled, currentTime: 0 }); const mutations: Array> = []; let commit: Commit | undefined; const root = renderCommitHook(mutations, (fn) => (commit = fn)); + await act(async () => commit!(selection, { x: 50 })); + act(() => root.unmount()); + return mutations; + } - await act(async () => { - await commit!(selection, { x: 50 }); - }); - + it("shifts the whole tween instead of updating a keyframe when the toggle is off", async () => { + const mutations = await runCommitWithAutoKeyframe(false); expect(mutations).toHaveLength(1); expect(mutations[0]!.type).toBe("replace-with-keyframes"); - act(() => root.unmount()); }); it("still updates a keyframe at the playhead when the toggle is on (default)", async () => { - const mutations: Array> = []; - let commit: Commit | undefined; - const root = renderCommitHook(mutations, (fn) => (commit = fn)); - - await act(async () => { - await commit!(selection, { x: 50 }); - }); - + const mutations = await runCommitWithAutoKeyframe(true); expect(mutations.some((m) => m.type === "update-keyframe" || m.type === "add-keyframe")).toBe( true, ); expect(mutations.some((m) => m.type === "replace-with-keyframes")).toBe(false); - act(() => root.unmount()); }); }); diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts index 640f593281..5bc19ec4af 100644 --- a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts +++ b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts @@ -24,6 +24,7 @@ import { import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; import { roundTo3 } from "../utils/rounding"; import { commitWholePropertyOffset } from "./gsapWholePropertyOffsetCommit"; +import { assertGsapEditPersisted, directEditOutcomeForProperties } from "./gsapEditOutcome"; interface CommitAnimatedPropertyDeps { selectedGsapAnimations: GsapAnimation[]; @@ -68,6 +69,8 @@ function pickBestAnimation( if (candidates.length === 0) return undefined; if (candidates.length === 1) return candidates[0]; const currentTime = usePlayerStore.getState().currentTime; + // Intentional multi-signal ranking: group match, selector specificity, and playhead overlap. + // fallow-ignore-next-line complexity const scored = candidates.map((a) => { let score = 0; if (a.keyframes) score += 10; @@ -393,11 +396,19 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) { const { selectedGsapAnimations, gsapCommitMutation, previewIframeRef, bumpGsapCache } = deps; const commitAnimatedProperties = useCallback( + // This is the single routing boundary for set, keyframe, whole-tween, and first-group writes. + // fallow-ignore-next-line complexity async (selection: DomEditSelection, props: Record): Promise => { if (!gsapCommitMutation) return; const propEntries = Object.entries(props); if (propEntries.length === 0) return; const primaryProp = propEntries[0]![0]; + assertGsapEditPersisted( + directEditOutcomeForProperties( + selectedGsapAnimations, + new Set(propEntries.map(([property]) => property)), + ), + ); const iframe = previewIframeRef.current; const selector = selectorFromSelection(selection); @@ -529,8 +540,9 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) { return; } bumpGsapCache(); - } catch { + } catch (error) { bumpGsapCache(); + throw error; } }, [selectedGsapAnimations, gsapCommitMutation, previewIframeRef, bumpGsapCache], diff --git a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts index 4e6f6ef9bc..8dd9ba9757 100644 --- a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts +++ b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts @@ -30,6 +30,11 @@ export type ElementAnimationsOutcome = | { kind: "fetch-error" } | { kind: "cold" }; +export interface GsapAnimationFetchOptions { + /** Refuse the edit when the parse endpoint is unavailable instead of treating it as no motion. */ + failOnFetchError?: boolean; +} + /** * Classify a parse result for one element. Differentiates a hard fetch failure * (`parsed === null`) from a warm-but-empty cold parse (`animations.length === 0`) @@ -44,32 +49,48 @@ export function selectElementAnimationsOrRetry( return { kind: "resolved", animations: getAnimationsForElement(parsed.animations, target) }; } +// Retry policy deliberately distinguishes cold parses from hard fetch errors. +// fallow-ignore-next-line complexity +async function fetchElementAnimationsWithRetry( + projectId: string, + gsapSourceFile: string, + target: { id: string | null; selector: string | null }, + failOnFetchError: boolean, +): Promise { + let coldAttempts = 0; + let errorAttempts = 0; + for (;;) { + const parsed = await fetchParsedAnimations(projectId, gsapSourceFile); + const outcome = selectElementAnimationsOrRetry(parsed, target); + if (outcome.kind === "resolved") return outcome.animations; + if (outcome.kind === "fetch-error") { + if (errorAttempts >= FETCH_ERROR_RETRIES) { + if (failOnFetchError) throw new Error("GSAP animation ownership could not be verified"); + return []; + } + errorAttempts++; + await delay(FETCH_ERROR_DELAY_MS); + continue; + } + if (coldAttempts >= COLD_PARSE_RETRIES) return []; + coldAttempts++; + await delay(COLD_PARSE_DELAY_MS); + } +} + export function useGsapAnimationFetchFallback(projectId: string | null, gsapSourceFile: string) { return useCallback( - (selection: DomEditSelection) => async (): Promise => { - if (!projectId) return []; - const target = { id: selection.id ?? null, selector: selection.selector ?? null }; - // A drag can fire before the async parse is warm; a cold parse must retry - // rather than fall through to the no-animation path (which duplicates the - // tween). A hard fetch error is a different failure — retry only briefly. - let coldAttempts = 0; - let errorAttempts = 0; - for (;;) { - const parsed = await fetchParsedAnimations(projectId, gsapSourceFile); - const outcome = selectElementAnimationsOrRetry(parsed, target); - if (outcome.kind === "resolved") return outcome.animations; - if (outcome.kind === "fetch-error") { - if (errorAttempts >= FETCH_ERROR_RETRIES) return []; - errorAttempts++; - await delay(FETCH_ERROR_DELAY_MS); - continue; - } - // cold - if (coldAttempts >= COLD_PARSE_RETRIES) return []; - coldAttempts++; - await delay(COLD_PARSE_DELAY_MS); - } - }, + (selection: DomEditSelection, options?: GsapAnimationFetchOptions) => + async (): Promise => { + if (!projectId) return []; + const target = { id: selection.id ?? null, selector: selection.selector ?? null }; + return fetchElementAnimationsWithRetry( + projectId, + gsapSourceFile, + target, + options?.failOnFetchError === true, + ); + }, [projectId, gsapSourceFile], ); } diff --git a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx index 69a675f110..bbff8cf210 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx +++ b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx @@ -14,6 +14,8 @@ const mocks = vi.hoisted(() => ({ drag: vi.fn(), readPosition: vi.fn(), setPosition: vi.fn(), + commitAnimatedProperty: vi.fn(), + commitAnimatedProperties: vi.fn(), })); vi.mock("./gsapResizeIntercept", () => ({ tryGsapResizeIntercept: mocks.resize })); @@ -28,8 +30,8 @@ vi.mock("./gsapPositionDetection", () => ({ vi.mock("../utils/elementGsap", () => ({ setElementGsapPosition: mocks.setPosition })); vi.mock("./useAnimatedPropertyCommit", () => ({ useAnimatedPropertyCommit: () => ({ - commitAnimatedProperty: vi.fn(), - commitAnimatedProperties: vi.fn(), + commitAnimatedProperty: mocks.commitAnimatedProperty, + commitAnimatedProperties: mocks.commitAnimatedProperties, }), })); vi.mock("./useSafeGsapCommitMutation", () => ({ @@ -78,22 +80,51 @@ function mountResizeHandler(animations: GsapAnimation[]) { return { selection, fallback, commitMutation, resize: resize!, root }; } +type AwareEditingParams = Parameters[0]; + +function mountGroupHandler({ + gsapCommitMutation, + makeFetchFallback, + trackGsapInteractionFailure = vi.fn(), +}: Pick & + Partial>) { + let groupCommit!: (updates: DomEditGroupPathOffsetCommit[]) => Promise; + function Harness() { + groupCommit = useGsapAwareEditing({ + domEditSelection: null, + selectedGsapAnimations: [], + gsapCommitMutation, + previewIframeRef: { current: null }, + showToast: vi.fn(), + bumpGsapCache: vi.fn(), + makeFetchFallback, + trackGsapInteractionFailure, + handleDomBoxSizeCommit: vi.fn(), + addGsapAnimation: vi.fn(), + convertToKeyframes: vi.fn(), + setArcPath: vi.fn(), + updateArcSegment: vi.fn(), + }).handleGsapAwareGroupPathOffsetCommit; + return null; + } + const root = mountReactHarness(); + return { groupCommit: (updates: DomEditGroupPathOffsetCommit[]) => groupCommit(updates), root }; +} + describe("useGsapAwareEditing anchored resize", () => { - it("forwards the anchor offset to the DOM fallback when GSAP does not handle resize", async () => { - mocks.resize.mockResolvedValue(false); + it("rejects a blocked resize instead of falling through to a competing DOM write", async () => { + mocks.resize.mockResolvedValue({ status: "blocked", reason: "source-uneditable" }); const h = mountResizeHandler([]); - await act(() => h.resize(h.selection, { width: 300, height: 200 }, { x: -50, y: -25 })); - expect(h.fallback).toHaveBeenCalledWith( - h.selection, - { width: 300, height: 200 }, - { x: -50, y: -25 }, - ); + await expect( + act(() => h.resize(h.selection, { width: 300, height: 200 }, { x: -50, y: -25 })), + ).rejects.toMatchObject({ reason: "source-uneditable" }); + expect(h.fallback).not.toHaveBeenCalled(); act(() => h.root.unmount()); }); it("persists the anchor exactly once through GSAP position when size route handles resize", async () => { - mocks.resize.mockResolvedValue(true); - mocks.drag.mockResolvedValue(true); + mocks.resize.mockResolvedValue({ status: "persisted" }); + mocks.drag.mockResolvedValue({ status: "persisted" }); const h = mountResizeHandler([]); await act(() => h.resize(h.selection, { width: 300, height: 200 }, { x: -50, y: -25 })); expect(h.fallback).not.toHaveBeenCalled(); @@ -103,12 +134,12 @@ describe("useGsapAwareEditing anchored resize", () => { }); it("settles the live GSAP position before resize persistence reaches its first await", async () => { - let resolveResize!: (handled: boolean) => void; - const pendingResize = new Promise((resolve) => { + let resolveResize!: (outcome: { status: "persisted" }) => void; + const pendingResize = new Promise<{ status: "persisted" }>((resolve) => { resolveResize = resolve; }); mocks.resize.mockReturnValue(pendingResize); - mocks.drag.mockResolvedValue(true); + mocks.drag.mockResolvedValue({ status: "persisted" }); mocks.readPosition.mockReturnValue({ x: 120.4, y: 80.2 }); const h = mountResizeHandler([]); h.selection.element.setAttribute("data-hf-drag-gsap-base-x", "120.4"); @@ -126,7 +157,7 @@ describe("useGsapAwareEditing anchored resize", () => { mocks.resize.mock.invocationCallOrder[0]!, ); - resolveResize(true); + resolveResize({ status: "persisted" }); await act(() => commit); act(() => h.root.unmount()); }); @@ -134,7 +165,7 @@ describe("useGsapAwareEditing anchored resize", () => { it("passes a transaction-scoped commit wrapper into the resize path", async () => { mocks.resize.mockImplementation(async (selection, _size, _animations, _iframe, commit) => { await commit(selection, { type: "resize" }, { label: "Resize", softReload: true }); - return true; + return { status: "persisted" }; }); const h = mountResizeHandler([]); @@ -164,9 +195,12 @@ describe("useGsapAwareEditing anchored resize", () => { m: unknown, o: { coalesceKey?: string; label?: string; softReload?: boolean }, ) => Promise, + _fetch: unknown, + options?: { preflightOnly?: boolean }, ) => { + if (options?.preflightOnly) return { status: "persisted" }; await commit(selection, { type: "move" }, { label: "Move", softReload: true }); - return true; + return { status: "persisted" }; }, ); const commitMutation = vi.fn( @@ -175,26 +209,10 @@ describe("useGsapAwareEditing anchored resize", () => { return Promise.resolve(); }, ); - let groupCommit!: (updates: DomEditGroupPathOffsetCommit[]) => Promise; - function Harness() { - groupCommit = useGsapAwareEditing({ - domEditSelection: null, - selectedGsapAnimations: [], - gsapCommitMutation: commitMutation, - previewIframeRef: { current: null }, - showToast: vi.fn(), - bumpGsapCache: vi.fn(), - makeFetchFallback: () => vi.fn().mockResolvedValue([]), - trackGsapInteractionFailure: vi.fn(), - handleDomBoxSizeCommit: vi.fn(), - addGsapAnimation: vi.fn(), - convertToKeyframes: vi.fn(), - setArcPath: vi.fn(), - updateArcSegment: vi.fn(), - }).handleGsapAwareGroupPathOffsetCommit; - return null; - } - const root = mountReactHarness(); + const { groupCommit, root } = mountGroupHandler({ + gsapCommitMutation: commitMutation, + makeFetchFallback: () => vi.fn().mockResolvedValue([]), + }); const updates = [ { selection: { element: document.createElement("div"), id: "a", selector: "#a" }, @@ -215,6 +233,74 @@ describe("useGsapAwareEditing anchored resize", () => { act(() => root.unmount()); }); + it("preflights every group member before the first mutation", async () => { + const commitMutation = vi.fn().mockResolvedValue(undefined); + const makeFetchFallback = vi.fn(() => vi.fn().mockResolvedValue([])); + mocks.drag.mockImplementation( + async (_selection, _next, _animations, _iframe, _commit, _fetch, options) => { + if (options?.preflightOnly) { + return _selection.id === "blocked" + ? { status: "blocked", reason: "source-uneditable" } + : { status: "persisted" }; + } + await _commit(_selection, { type: "move" }, { label: "Move" }); + return { status: "persisted" }; + }, + ); + const { groupCommit, root } = mountGroupHandler({ + gsapCommitMutation: commitMutation, + makeFetchFallback, + }); + const updates = [ + { + selection: { element: document.createElement("div"), id: "ok", selector: "#ok" }, + next: { x: 10, y: 10 }, + }, + { + selection: { + element: document.createElement("div"), + id: "blocked", + selector: "#blocked", + }, + next: { x: 10, y: 10 }, + }, + ] as unknown as DomEditGroupPathOffsetCommit[]; + + await expect(groupCommit(updates)).rejects.toMatchObject({ + name: "GsapEditBlockedError", + reason: "source-uneditable", + }); + expect(commitMutation).not.toHaveBeenCalled(); + expect(mocks.drag).toHaveBeenCalledTimes(2); + expect(makeFetchFallback).toHaveBeenNthCalledWith(1, updates[0]!.selection, { + failOnFetchError: true, + }); + expect(makeFetchFallback).toHaveBeenNthCalledWith(2, updates[1]!.selection, { + failOnFetchError: true, + }); + act(() => root.unmount()); + }); + + it("fails a group preflight closed when ownership cannot be fetched", async () => { + const fetchError = new Error("parse endpoint unavailable"); + const commitMutation = vi.fn().mockResolvedValue(undefined); + const { groupCommit, root } = mountGroupHandler({ + gsapCommitMutation: commitMutation, + makeFetchFallback: () => vi.fn().mockRejectedValue(fetchError), + }); + const updates = [ + { + selection: { element: document.createElement("div"), id: "a", selector: "#a" }, + next: { x: 10, y: 10 }, + }, + ] as unknown as DomEditGroupPathOffsetCommit[]; + + await expect(groupCommit(updates)).rejects.toBe(fetchError); + expect(mocks.drag).not.toHaveBeenCalled(); + expect(commitMutation).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + it("restores once when resize persistence fails", async () => { const error = new Error("resize failed"); const restore = vi.fn(); @@ -228,7 +314,7 @@ describe("useGsapAwareEditing anchored resize", () => { }); it("does not apply the anchor twice when scale route already settles the drop point", async () => { - mocks.resize.mockResolvedValue(true); + 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 })); diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index b2071aa693..188ff58f80 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -30,6 +30,8 @@ import { logResize, logResizeSettle } from "../utils/resizeDebug"; import type { DomEditGroupPathOffsetCommit } from "../components/editor/DomEditOverlay"; import { runGestureTransaction } from "./gestureTransaction"; import { hasNonHoldTweenForElement } from "./gsapRuntimeKeyframes"; +import { assertGsapEditPersisted } from "./gsapEditOutcome"; +import type { GsapAnimationFetchOptions } from "./useGsapAnimationFetchFallback"; // Distinct coalesceKey per group drag so consecutive group drags don't fold // into one another's undo entry (module-local counter, not Date.now()). @@ -42,7 +44,10 @@ export interface UseGsapAwareEditingParams { previewIframeRef: React.RefObject; showToast: (message: string, tone?: "error" | "info") => void; bumpGsapCache: () => void; - makeFetchFallback: (selection: DomEditSelection) => () => Promise; + makeFetchFallback: ( + selection: DomEditSelection, + options?: GsapAnimationFetchOptions, + ) => () => Promise; trackGsapInteractionFailure: ( error: unknown, selection: DomEditSelection, @@ -112,7 +117,7 @@ export function useGsapAwareEditing({ ) => { if (gsapCommitMutation) { try { - await tryGsapDragIntercept( + const outcome = await tryGsapDragIntercept( selection, next, selectedGsapAnimations, @@ -121,6 +126,7 @@ export function useGsapAwareEditing({ makeFetchFallback(selection), modifiers, ); + assertGsapEditPersisted(outcome); } catch (error) { trackGsapInteractionFailure(error, selection, "drag", "Move animated layer"); throw error; @@ -155,16 +161,42 @@ export function useGsapAwareEditing({ coalesceKey, coalesceMs: Number.POSITIVE_INFINITY, }); + const preflightAnimations = new Map(); + // Editability is user-atomic: prove every member can be written before + // the first source mutation. Network failures after this point retain the + // existing multi-request semantics, but a blocked member can never leave + // earlier siblings partially moved. + for (const { selection } of updates) { + try { + const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); + preflightAnimations.set(selection, animations); + const outcome = await tryGsapDragIntercept( + selection, + { x: 0, y: 0 }, + animations, + previewIframeRef.current, + coalescedCommit, + undefined, + { preflightOnly: true }, + ); + assertGsapEditPersisted(outcome); + } catch (error) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + throw error; + } + } for (const { selection, next } of updates) { try { - await tryGsapDragIntercept( + const outcome = await tryGsapDragIntercept( selection, next, - [], + preflightAnimations.get(selection) ?? [], previewIframeRef.current, coalescedCommit, makeFetchFallback(selection), + { preflightPassed: true }, ); + assertGsapEditPersisted(outcome); } catch (error) { trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); throw error; @@ -217,7 +249,7 @@ export function useGsapAwareEditing({ if (gsapCommitMutation) { const commitMutation = commit(gsapCommitMutation); try { - const handled = await tryGsapResizeIntercept( + const outcome = await tryGsapResizeIntercept( selection, next, selectedGsapAnimations, @@ -225,26 +257,26 @@ export function useGsapAwareEditing({ commitMutation, makeFetchFallback(selection), ); - if (handled) { - logResize("intercept-handled", { - scaleRoute, - willForwardOffset: !!(offset && !scaleRoute), - }); - // Scale-route resize persists its residual position internally. - // Width/height persists the already-settled anchor through drag. - if (offset && !scaleRoute) { - await tryGsapDragIntercept( - selection, - offset, - selectedGsapAnimations, - previewIframeRef.current, - commitMutation, - makeFetchFallback(selection), - ); - } - logResizeSettle(selection.element, scaleRoute ? "gsap-scale" : "gsap-size"); - return; + assertGsapEditPersisted(outcome); + logResize("intercept-handled", { + scaleRoute, + willForwardOffset: !!(offset && !scaleRoute), + }); + // Scale-route resize persists its residual position internally. + // Width/height persists the already-settled anchor through drag. + if (offset && !scaleRoute) { + const dragOutcome = await tryGsapDragIntercept( + selection, + offset, + selectedGsapAnimations, + previewIframeRef.current, + commitMutation, + makeFetchFallback(selection), + ); + assertGsapEditPersisted(dragOutcome); } + logResizeSettle(selection.element, scaleRoute ? "gsap-scale" : "gsap-size"); + return; } catch (error) { trackGsapInteractionFailure(error, selection, "resize", "Resize animated layer"); throw error; @@ -278,8 +310,9 @@ export function useGsapAwareEditing({ try { // Single source of truth for rotation too: tryGsapRotationIntercept handles // tweened elements (keyframes) and static ones (a tl.set), so there's no - // CSS-var fallback. It returns false only for a selectorless element (no-op). - await tryGsapRotationIntercept( + // CSS-var fallback. Selectorless/computed source rejects so the gesture + // transaction can restore its draft instead of reporting a false success. + const outcome = await tryGsapRotationIntercept( selection, next.angle, selectedGsapAnimations, @@ -287,6 +320,7 @@ export function useGsapAwareEditing({ gsapCommitMutation, makeFetchFallback(selection), ); + assertGsapEditPersisted(outcome); } catch (error) { trackGsapInteractionFailure(error, selection, "rotation", "Rotate animated layer"); throw error; @@ -304,7 +338,10 @@ export function useGsapAwareEditing({ // ── Animated property commit ── - const { commitAnimatedProperty, commitAnimatedProperties } = useAnimatedPropertyCommit({ + const { + commitAnimatedProperty: commitAnimatedPropertyRaw, + commitAnimatedProperties: commitAnimatedPropertiesRaw, + } = useAnimatedPropertyCommit({ selectedGsapAnimations, gsapCommitMutation, addGsapAnimation: (sel, method, time) => addGsapAnimation(sel, method, time), @@ -313,6 +350,30 @@ export function useGsapAwareEditing({ bumpGsapCache, }); + const commitAnimatedProperties = useCallback( + async (selection: DomEditSelection, properties: Record) => { + try { + await commitAnimatedPropertiesRaw(selection, properties); + } catch (error) { + trackGsapInteractionFailure(error, selection, "property", "Edit animated property"); + throw error; + } + }, + [commitAnimatedPropertiesRaw, trackGsapInteractionFailure], + ); + + const commitAnimatedProperty = useCallback( + async (selection: DomEditSelection, property: string, value: number | string) => { + try { + await commitAnimatedPropertyRaw(selection, property, value); + } catch (error) { + trackGsapInteractionFailure(error, selection, "property", "Edit animated property"); + throw error; + } + }, + [commitAnimatedPropertyRaw, trackGsapInteractionFailure], + ); + // ── Arc path wrappers ── const handleSetArcPath = useCallback( diff --git a/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.test.tsx b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.test.tsx new file mode 100644 index 0000000000..789840ff2f --- /dev/null +++ b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.test.tsx @@ -0,0 +1,42 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { describe, expect, it, vi } from "vitest"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { mountReactHarness } from "./domSelectionTestHarness"; +import { GsapEditBlockedError } from "./gsapEditOutcome"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const trackStudioSaveFailure = vi.hoisted(() => vi.fn()); +vi.mock("../utils/studioSaveDiagnostics", () => ({ trackStudioSaveFailure })); + +import { useGsapInteractionFailureTelemetry } from "./useGsapInteractionFailureTelemetry"; + +describe("useGsapInteractionFailureTelemetry", () => { + it("surfaces the blocked reason instead of a generic save failure", () => { + const showToast = vi.fn(); + const selection = { + id: "clip", + selector: "#clip", + element: document.createElement("div"), + } as unknown as DomEditSelection; + let report!: ReturnType; + function Harness() { + report = useGsapInteractionFailureTelemetry("index.html", showToast); + return null; + } + const root = mountReactHarness(); + + act(() => report(new GsapEditBlockedError("unroll-required"), selection, "drag", "Move")); + + expect(showToast).toHaveBeenCalledWith( + "This motion comes from a helper or loop. Choose Unroll to edit it explicitly.", + "error", + ); + expect(trackStudioSaveFailure).toHaveBeenCalledWith( + expect.objectContaining({ source: "gsap_commit", mutationType: "drag", targetId: "clip" }), + ); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts index e451c3a915..80bef7f7c3 100644 --- a/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts +++ b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts @@ -1,6 +1,7 @@ import { useCallback } from "react"; import type { DomEditSelection } from "../components/editor/domEditing"; import { trackStudioSaveFailure } from "../utils/studioSaveDiagnostics"; +import { isGsapEditBlockedError } from "./gsapEditOutcome"; export function useGsapInteractionFailureTelemetry( activeCompPath: string | null, @@ -18,7 +19,10 @@ export function useGsapInteractionFailureTelemetry( targetSelector: selection.selector, targetSourceFile: selection.sourceFile, }); - showToast("Failed to save animated edit.", "error"); + showToast( + isGsapEditBlockedError(error) ? error.message : "Failed to save animated edit.", + "error", + ); }, [activeCompPath, showToast], );