diff --git a/packages/studio/src/hooks/gsapDragCommit.ts b/packages/studio/src/hooks/gsapDragCommit.ts index e6f4197987..d2d17c50b3 100644 --- a/packages/studio/src/hooks/gsapDragCommit.ts +++ b/packages/studio/src/hooks/gsapDragCommit.ts @@ -135,7 +135,7 @@ export async function materializeIfDynamic( void iframe; void commitMutation; void selection; - throw new GsapEditBlockedError("source-uneditable"); + throw new GsapEditBlockedError("source-uneditable", "geometry-unresolved-source"); } // ── Drag → GSAP position math ────────────────────────────────────────────── diff --git a/packages/studio/src/hooks/gsapEditOutcome.test.ts b/packages/studio/src/hooks/gsapEditOutcome.test.ts new file mode 100644 index 0000000000..12aa7ff158 --- /dev/null +++ b/packages/studio/src/hooks/gsapEditOutcome.test.ts @@ -0,0 +1,115 @@ +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import { describe, expect, it } from "vitest"; +import { buildStudioSaveFailureProperties } from "../utils/studioSaveDiagnostics"; +import { + GsapEditBlockedError, + assertGsapEditPersisted, + directEditOutcomeForProperties, +} from "./gsapEditOutcome"; + +function blockDetailFor(error: unknown): unknown { + return buildStudioSaveFailureProperties({ source: "gsap_commit", error }).block_detail; +} + +/** Minimal animation that writes `x`, so the editability check actually runs. */ +function animation(overrides: Partial = {}): GsapAnimation { + return { + id: "a1", + targetSelector: "#box", + method: "to", + position: 0, + properties: { x: 100 }, + ...overrides, + } as GsapAnimation; +} + +const POSITION = new Set(["x"]); + +describe("a blocked GSAP edit names which of the nine causes it hit", () => { + // The three causes inside assertGsapAnimationDirectlyEditable share one + // message, so only the detail distinguishes them. Asserted through the + // exported wrapper because that is the path a real edit takes — and the + // wrapper used to rebuild the outcome from `reason` alone, dropping it. + it("reports a runtime-dynamic provenance", () => { + const outcome = directEditOutcomeForProperties( + [animation({ provenance: { kind: "runtime-dynamic" } })], + POSITION, + ); + expect(outcome).toEqual({ + status: "blocked", + reason: "source-uneditable", + detail: "provenance-runtime-dynamic", + }); + }); + + it("reports unresolved keyframes separately from provenance", () => { + const outcome = directEditOutcomeForProperties( + [animation({ hasUnresolvedKeyframes: true })], + POSITION, + ); + expect(outcome).toEqual({ + status: "blocked", + reason: "source-uneditable", + detail: "unresolved-keyframes", + }); + }); + + it("reports an unresolved selector separately from provenance", () => { + const outcome = directEditOutcomeForProperties( + [animation({ hasUnresolvedSelector: true })], + POSITION, + ); + expect(outcome).toEqual({ + status: "blocked", + reason: "source-uneditable", + detail: "unresolved-selector", + }); + }); + + it("still routes a helper or loop to unroll, which has its own message", () => { + const outcome = directEditOutcomeForProperties( + [animation({ provenance: { kind: "loop" } })], + POSITION, + ); + expect(outcome).toEqual({ status: "blocked", reason: "unroll-required", detail: undefined }); + }); + + it("leaves a literal tween editable", () => { + expect(directEditOutcomeForProperties([animation()], POSITION)).toEqual({ + status: "persisted", + }); + }); + + it("carries the detail through a returned outcome, not just a thrown one", () => { + // gsapRuntimeBridge returns blocked outcomes rather than throwing. + expect(() => + assertGsapEditPersisted({ + status: "blocked", + reason: "source-uneditable", + detail: "no-position-tween", + }), + ).toThrow( + expect.objectContaining({ name: "GsapEditBlockedError", detail: "no-position-tween" }), + ); + }); + + it("surfaces the detail on the telemetry payload", () => { + expect( + blockDetailFor( + new GsapEditBlockedError("source-uneditable", "live-rotation-no-source-tween"), + ), + ).toBe("live-rotation-no-source-tween"); + }); + + it("omits the detail for a block that has no specific cause", () => { + expect(blockDetailFor(new GsapEditBlockedError("no-selector"))).toBeUndefined(); + }); + + it("ignores a detail-shaped field on an error that is not a block", () => { + // Guards the name check: any error can carry a `detail`, and reporting a + // save failure's detail as a block cause would be a silent mislabel. + const impostor = Object.assign(new Error("network down"), { detail: "unresolved-selector" }); + expect(blockDetailFor(impostor)).toBeUndefined(); + expect(blockDetailFor("not an error")).toBeUndefined(); + }); +}); diff --git a/packages/studio/src/hooks/gsapEditOutcome.ts b/packages/studio/src/hooks/gsapEditOutcome.ts index 9ec2dddd1e..71f4cf0ab7 100644 --- a/packages/studio/src/hooks/gsapEditOutcome.ts +++ b/packages/studio/src/hooks/gsapEditOutcome.ts @@ -2,6 +2,27 @@ import { editabilityForProvenance, type GsapAnimation } from "@hyperframes/core/ export type GsapEditBlockReason = "no-selector" | "unroll-required" | "source-uneditable"; +/** + * Which of the nine situations produced a block. The user-facing `reason` stays + * coarse — three messages — but "source-uneditable" alone covers nine distinct + * causes, and `edit_blocked` telemetry could not tell them apart. That matters + * because the copy ("This animation is computed at runtime") is only literally + * true for `provenance-runtime-dynamic`; the others are parser or source-match + * limits, where the animation may well be plain authored source. + * + * Telemetry only. Nothing branches on it. + */ +export type GsapEditBlockDetail = + | "provenance-runtime-dynamic" + | "unresolved-keyframes" + | "unresolved-selector" + | "geometry-unresolved-source" + | "live-position-no-source-tween" + | "no-position-tween" + | "live-rotation-no-source-tween" + | "live-resize-no-source-tween" + | "zero-duration-tween"; + export type GsapEditOutcome = | { status: "persisted"; @@ -22,7 +43,7 @@ export type GsapEditOutcome = */ ownsDragOffset?: boolean; } - | { status: "blocked"; reason: GsapEditBlockReason }; + | { status: "blocked"; reason: GsapEditBlockReason; detail?: GsapEditBlockDetail }; const COPY: Record = { "no-selector": "This layer needs a stable selector before Studio can save the edit.", @@ -32,25 +53,32 @@ const COPY: Record = { }; export class GsapEditBlockedError extends Error { - constructor(readonly reason: GsapEditBlockReason) { + constructor( + readonly reason: GsapEditBlockReason, + readonly detail?: GsapEditBlockDetail, + ) { super(COPY[reason]); this.name = "GsapEditBlockedError"; } } export function assertGsapEditPersisted(outcome: GsapEditOutcome): void { - if (outcome.status === "blocked") throw new GsapEditBlockedError(outcome.reason); + if (outcome.status === "blocked") throw new GsapEditBlockedError(outcome.reason, outcome.detail); } 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"); + // Same message for all three, but they are different problems: only the first + // is genuinely a runtime-computed value. + if (editability === "source") { + throw new GsapEditBlockedError("source-uneditable", "provenance-runtime-dynamic"); + } + if (animation.hasUnresolvedKeyframes) { + throw new GsapEditBlockedError("source-uneditable", "unresolved-keyframes"); + } + if (animation.hasUnresolvedSelector) { + throw new GsapEditBlockedError("source-uneditable", "unresolved-selector"); } } @@ -84,7 +112,8 @@ export function directEditOutcomeForProperties( } return { status: "persisted" }; } catch (error) { - if (isGsapEditBlockedError(error)) return { status: "blocked", reason: error.reason }; + if (isGsapEditBlockedError(error)) + return { status: "blocked", reason: error.reason, detail: error.detail }; throw error; } } diff --git a/packages/studio/src/hooks/gsapResizeIntercept.test.ts b/packages/studio/src/hooks/gsapResizeIntercept.test.ts index 4045b28c8a..2037c7f19f 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.test.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.test.ts @@ -180,11 +180,15 @@ it("reuses the ownership parse instead of fetching a resolved size group twice", ); }); -it("blocks when runtime size motion exists but the authored tween cannot be resolved", async () => { +function makeBoxSelection(): { el: HTMLElement; selection: DomEditSelection } { const el = document.createElement("div"); el.id = "box"; document.body.append(el); - const selection = { id: "box", selector: "#box", element: el } as DomEditSelection; + return { el, selection: { id: "box", selector: "#box", element: el } as DomEditSelection }; +} + +it("blocks when runtime size motion exists but the authored tween cannot be resolved", async () => { + const { el, selection } = makeBoxSelection(); const liveSizeTween = { targets: () => [el], vars: { width: 300, duration: 1 }, @@ -199,7 +203,42 @@ it("blocks when runtime size motion exists but the authored tween cannot be reso await expect( tryGsapResizeIntercept(selection, { width: 344, height: 344 }, [], iframe, commitMutation), - ).resolves.toEqual({ status: "blocked", reason: "source-uneditable" }); + ).resolves.toEqual({ + status: "blocked", + reason: "source-uneditable", + detail: "live-resize-no-source-tween", + }); + expect(commitMutation).not.toHaveBeenCalled(); +}); + +it("blocks a tween with no positive duration instead of committing into it", async () => { + const { selection } = makeBoxSelection(); + // A zero-duration `from` is not an instant hold (those are `set`/`to`), so it + // reaches the duration check: a resize has no timeline position to land on. + const zeroDurationTween = { + id: "#box-size", + targetSelector: "#box", + propertyGroup: "size", + method: "from", + position: 0, + duration: 0, + properties: { width: 150, height: 150 }, + } as unknown as GsapAnimation; + const commitMutation = vi.fn(); + + await expect( + tryGsapResizeIntercept( + selection, + { width: 344, height: 344 }, + [zeroDurationTween], + null, + commitMutation, + ), + ).resolves.toEqual({ + status: "blocked", + reason: "source-uneditable", + detail: "zero-duration-tween", + }); expect(commitMutation).not.toHaveBeenCalled(); }); diff --git a/packages/studio/src/hooks/gsapResizeIntercept.ts b/packages/studio/src/hooks/gsapResizeIntercept.ts index 71b1adfba4..9353213dba 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.ts @@ -137,7 +137,12 @@ export async function tryGsapResizeIntercept( ? hasNonHoldTweenForElement(iframe, liveSelector, undefined, [...resizeProperties]) : false; if (!anim && hasLiveResizeTween) { - return { status: "blocked", reason: "source-uneditable" }; + // Third twin of the position/rotation cases: a live tween with no source match. + return { + status: "blocked", + reason: "source-uneditable", + detail: "live-resize-no-source-tween", + }; } logResize("intercept-enter", { hasScaleGroup, @@ -194,7 +199,12 @@ export async function tryGsapResizeIntercept( } const tweenDuration = resolveTweenDuration(anim); - if (tweenDuration <= 0) return { status: "blocked", reason: "source-uneditable" }; + if (tweenDuration <= 0) { + // The tween exists in source but has no positive duration, so there is no + // timeline position at which a resize could land — a different cause from + // the live-tween-without-a-source-match cases above. + return { status: "blocked", reason: "source-uneditable", detail: "zero-duration-tween" }; + } const { activeKeyframePct, setActiveKeyframePct } = usePlayerStore.getState(); const pct = activeKeyframePct ?? computeCurrentPercentage(selection, anim); diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.test.ts b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts index bc79366dfc..62d3cbfa91 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.test.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts @@ -94,7 +94,11 @@ describe("tryGsapDragIntercept — stale-parse guard (no resurrection after dele vi.fn().mockResolvedValue([]), ); - expect(result).toEqual({ status: "blocked", reason: "source-uneditable" }); + expect(result).toEqual({ + status: "blocked", + reason: "source-uneditable", + detail: "live-position-no-source-tween", + }); expect(commitMutation).not.toHaveBeenCalled(); }); @@ -354,7 +358,11 @@ describe("tryGsapRotationIntercept — instant holds", () => { fakeIframe("puck-b", [liveRotation]), commitMutation, ), - ).resolves.toEqual({ status: "blocked", reason: "source-uneditable" }); + ).resolves.toEqual({ + status: "blocked", + reason: "source-uneditable", + detail: "live-rotation-no-source-tween", + }); expect(commitMutation).not.toHaveBeenCalled(); }); diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.ts b/packages/studio/src/hooks/gsapRuntimeBridge.ts index a35fb6d3f1..e66ed6335a 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.ts @@ -156,7 +156,13 @@ async function preflightGsapDragIntercept( const hasLivePosition = hasNonHoldTweenForElement(iframe, selector, undefined, POSITION_CHANNELS); if (hasLivePosition && !posAnim) { - return { status: "blocked", reason: "source-uneditable" }; + // GSAP is visibly moving this element but the parser found no position + // tween for it — a source-match gap, not necessarily computed source. + return { + status: "blocked", + reason: "source-uneditable", + detail: "live-position-no-source-tween", + }; } if (!posAnim && !writeTargetSelector(selection)) { return { status: "blocked", reason: "no-selector" }; @@ -268,7 +274,7 @@ export async function tryGsapDragIntercept( } if (!posAnim) { - return { status: "blocked", reason: "source-uneditable" }; + return { status: "blocked", reason: "source-uneditable", detail: "no-position-tween" }; } // Verify the anim ID is still valid in the current file. The React-state @@ -353,7 +359,12 @@ export async function tryGsapRotationIntercept( ? hasNonHoldTweenForElement(iframe, liveSelector, undefined, ROTATION_CHANNELS) : false; if (!anim && hasLiveRotationTween) { - return { status: "blocked", reason: "source-uneditable" }; + // Rotation twin of the position case above: live tween, no source match. + return { + status: "blocked", + reason: "source-uneditable", + detail: "live-rotation-no-source-tween", + }; } // `angle` is the ABSOLUTE target rotation resolved by the gesture (gsap base + diff --git a/packages/studio/src/utils/studioSaveDiagnostics.ts b/packages/studio/src/utils/studioSaveDiagnostics.ts index 865723193c..f4a8c3ebf0 100644 --- a/packages/studio/src/utils/studioSaveDiagnostics.ts +++ b/packages/studio/src/utils/studioSaveDiagnostics.ts @@ -165,9 +165,21 @@ export function buildStudioSaveFailureProperties( target_id: input.targetId ?? undefined, target_selector: input.targetSelector ?? undefined, target_source_file: input.targetSourceFile ?? undefined, + block_detail: gsapBlockDetail(input.error), }; } +/** + * The specific cause behind a blocked GSAP edit. `error_message` only carries + * the coarse copy, and one of those strings covers six different situations, + * so the message alone cannot say which one a user hit. + */ +function gsapBlockDetail(error: unknown): string | undefined { + if (!(error instanceof Error) || error.name !== "GsapEditBlockedError") return undefined; + const detail = (error as { detail?: unknown }).detail; + return typeof detail === "string" ? detail : undefined; +} + export function trackStudioSaveFailure(input: StudioSaveFailureInput): void { trackStudioEvent("save_failure", buildStudioSaveFailureProperties(input)); }