diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx b/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx index 038e03d21..f5eeed3d3 100644 --- a/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx +++ b/packages/studio/src/hooks/useAnimatedPropertyCommit.test.tsx @@ -39,15 +39,16 @@ type Commit = ( ) => Promise; /** Renders the hook and hands its commit function to the caller via a ref callback. */ -function renderCommitHook( - mutations: Array>, +function renderHookWith( + animations: GsapAnimation[], + onMutation: (mutation: Record, label: string) => void, onReady: (commit: Commit) => void, ) { function Harness() { const { commitAnimatedProperties } = useAnimatedPropertyCommit({ - selectedGsapAnimations: [keyframedAnim], - gsapCommitMutation: async (_sel, mutation) => { - mutations.push(mutation); + selectedGsapAnimations: animations, + gsapCommitMutation: async (_sel, mutation, options) => { + onMutation(mutation, options.label); }, addGsapAnimation: vi.fn(), convertToKeyframes: vi.fn(), @@ -66,6 +67,13 @@ function renderCommitHook( return root; } +function renderCommitHook( + mutations: Array>, + onReady: (commit: Commit) => void, +) { + return renderHookWith([keyframedAnim], (mutation) => mutations.push(mutation), onReady); +} + // Regression (#1808): a "3D transform" / design-panel property edit on an // element that already has a keyframed tween is the ACTUAL path a manual // canvas nudge exercises (not the raw drag intercept) — with auto-keyframe @@ -103,3 +111,56 @@ describe("useAnimatedPropertyCommit — autoKeyframeEnabled toggle (#1808)", () act(() => root.unmount()); }); }); + +// Regression: commitStaticSet picked the FIRST `set` for the selector with no +// group check — a panel W edit on a static element merged `width` into the +// POSITION set (`tl.set("#el",{x,y,width})`), a mixed-group set the split +// machinery exists to prevent, labeled "Set 3D transform" in undo history. +describe("commitStaticSet group routing", () => { + const positionSet = { + id: "#box-set-0-position", + targetSelector: "#box", + propertyGroup: "position", + method: "set", + properties: { x: 10, y: 20 }, + } as unknown as GsapAnimation; + + function renderStaticHook( + committed: Array<{ mutation: Record; label: string }>, + onReady: (commit: Commit) => void, + ) { + return renderHookWith( + [positionSet], + (mutation, label) => committed.push({ mutation, label }), + onReady, + ); + } + + it("width edit creates a size set instead of contaminating the position set", async () => { + const committed: Array<{ mutation: Record; label: string }> = []; + let commit!: Commit; + renderStaticHook(committed, (c) => (commit = c)); + await act(async () => { + await commit(selection, { width: 500 }); + }); + const updates = committed.filter((c) => c.mutation.type === "update-properties"); + expect(updates).toHaveLength(0); + const adds = committed.filter((c) => c.mutation.type === "add"); + expect(adds).toHaveLength(1); + expect(adds[0]!.mutation.properties).toEqual({ width: 500 }); + expect(adds[0]!.label).toBe("Resize layer"); + }); + + it("x edit updates the position set with a Move label", async () => { + const committed: Array<{ mutation: Record; label: string }> = []; + let commit!: Commit; + renderStaticHook(committed, (c) => (commit = c)); + await act(async () => { + await commit(selection, { x: 400 }); + }); + const update = committed.find((c) => c.mutation.type === "update-properties"); + expect(update).toBeDefined(); + expect(update!.mutation.animationId).toBe("#box-set-0-position"); + expect(update!.label).toBe("Move layer"); + }); +}); diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts index a6ea2e308..9e5914f10 100644 --- a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts +++ b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts @@ -103,6 +103,22 @@ async function maybeAutoKeyframeSet( type Commit = NonNullable; +/** Undo-history label for a static-set commit, from the group it writes. */ +const STATIC_SET_LABELS: Partial, string>> = { + position: "Move layer", + scale: "Resize layer", + size: "Resize layer", + rotation: "Rotate layer", + visual: "Set opacity", + other: "Set 3D transform", +}; + +function staticSetLabel(propEntries: [string, number | string][]): string { + const groups = new Set(propEntries.map(([k]) => classifyPropertyGroup(k))); + const only = groups.size === 1 ? [...groups][0] : undefined; + return (only && STATIC_SET_LABELS[only]) || "Set properties"; +} + /** Merge ALL props into the static `set` in ONE commit (value-only, instant), then * auto-keyframe. One mutation — a per-property loop would shift the set's * group-derived id mid-way (e.g. reset adding `scale` to a rotation set), 404-ing @@ -133,7 +149,11 @@ async function commitSetProps( await commit( selection, { type: "update-properties", animationId: setAnim.id, properties }, - { label: "Set 3D transform", softReload: true, ...(instantPatch ? { instantPatch } : {}) }, + { + label: staticSetLabel(propEntries), + softReload: true, + ...(instantPatch ? { instantPatch } : {}), + }, ); await maybeAutoKeyframeSet(selection, setAnim, animations, commit); } @@ -152,22 +172,49 @@ async function commitStaticSet( commit: Commit, ): Promise { if (!selector) return; - // Update an existing `set` in ONE batched commit — NEVER a flat `to`/`from`. A - // set's id is GROUP-derived, so a per-prop loop shifts it the instant a new-group - // prop lands (e.g. `scale` onto a rotation set), 404-ing the next prop; commitSetProps - // sends them together. A static element with no set gets a dedicated `set` carrying - // ALL props in ONE `add`. - const existingSet = animations.find((a) => a.method === "set" && a.targetSelector === selector); - if (existingSet) { - await commitSetProps(selection, existingSet, propEntries, selector, animations, commit); - return; + // One commit per PROPERTY GROUP, each into a set that owns that group — never a + // flat `to`/`from`, and never a foreign-group set (a width edit used to merge + // into the element's position set, producing a mixed set the split machinery + // exists to prevent). Within a group everything batches into ONE commit: a + // set's id is group-derived, so a per-prop loop would shift the id mid-way and + // 404 the next update. + const byGroup = new Map(); + for (const entry of propEntries) { + const group = classifyPropertyGroup(entry[0]); + const batch = byGroup.get(group) ?? []; + batch.push(entry); + byGroup.set(group, batch); } - // Base `gsap.set` (off-timeline) — a static hold with no 0% keyframe marker, so - // adjusting a 3D transform on a non-keyframed element doesn't drop a keyframe on - // the timeline (matches the manual-drag UX). The global-set instant patch applies - // it straight to the element so the first edit shows with no soft-reload flash. + const sets = animations.filter((a) => a.method === "set" && a.targetSelector === selector); + for (const [group, batch] of byGroup) { + const existingSet = + // A set already dedicated to this group wins; else a mixed set that + // already carries a property of this group (merging same-group values + // there beats spawning a second writer for the same channel). + sets.find((a) => a.propertyGroup === group) ?? + sets.find((a) => Object.keys(a.properties).some((k) => classifyPropertyGroup(k) === group)); + if (existingSet) { + await commitSetProps(selection, existingSet, batch, selector, animations, commit); + } else { + await addGlobalStaticSet(selection, batch, selector, commit); + } + } +} + +/** + * Base `gsap.set` (off-timeline) — a static hold with no 0% keyframe marker, so + * adjusting a 3D transform on a non-keyframed element doesn't drop a keyframe on + * the timeline (matches the manual-drag UX). The global-set instant patch applies + * it straight to the element so the first edit shows with no soft-reload flash. + */ +async function addGlobalStaticSet( + selection: DomEditSelection, + batch: [string, number | string][], + selector: string, + commit: Commit, +): Promise { const numericProps: SetPatchProps = {}; - for (const [k, v] of propEntries) { + for (const [k, v] of batch) { if (typeof v === "number") numericProps[k as keyof SetPatchProps] = v; } await commit( @@ -177,11 +224,11 @@ async function commitStaticSet( targetSelector: selector, method: "set", position: 0, - properties: Object.fromEntries(propEntries), + properties: Object.fromEntries(batch), global: true, }, { - label: "Set 3D transform", + label: staticSetLabel(batch), softReload: true, ...(Object.keys(numericProps).length > 0 ? {