From be3451aae43779d47cd593e12bdf8fb2f4da2889 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 20:34:57 +0200 Subject: [PATCH] fix(studio): resolve a panel edit through the lane groups it can see animIdForProp matched on the parser's whole-tween propertyGroup, which is undefined for a legacy mixed tween such as {x, opacity}. Such a tween never matched, so an edit to either property fell through to the selection's default animation, a different tween than the lane the user is editing. Resolve through animationLaneGroups, the same per-keyframe helper the rendered lanes and the reserved row heights already count groups with. --- .../src/components/editor/PropertyPanel.tsx | 10 ++-- .../components/TimelinePropertyLanes.test.tsx | 47 +++++++++++++++++++ .../components/TimelinePropertyLanes.tsx | 22 +++++++++ 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/packages/studio/src/components/editor/PropertyPanel.tsx b/packages/studio/src/components/editor/PropertyPanel.tsx index 0c9905e80..89e7f2508 100644 --- a/packages/studio/src/components/editor/PropertyPanel.tsx +++ b/packages/studio/src/components/editor/PropertyPanel.tsx @@ -16,7 +16,7 @@ import { } from "./propertyPanelHelpers"; import { MetricField, Section } from "./propertyPanelPrimitives"; import { createTransformCommitHandlers } from "./propertyPanelTransformCommit"; -import { classifyPropertyGroup } from "@hyperframes/core/gsap-parser"; +import { resolveAnimIdForProperty } from "../../player/components/TimelinePropertyLanes"; import { resolveEditingSections } from "@hyperframes/core/editing"; import { MediaSection } from "./propertyPanelMediaSection"; import { ColorGradingSection } from "./propertyPanelColorGradingSection"; @@ -241,12 +241,8 @@ export const PropertyPanel = memo(function PropertyPanel(props: PropertyPanelPro const navKeyframes = cacheEntry?.keyframes ?? gsapKeyframes; const seekFromKfPct = (pct: number) => onSeekToTime?.(elStart + (pct / 100) * elDuration); - const animIdForProp = (prop: string): string => { - const group = classifyPropertyGroup(prop); - const groupAnim = gsapAnimations?.find((a) => a.propertyGroup === group); - if (groupAnim) return groupAnim.id; - return gsapAnimId ?? ""; - }; + const animIdForProp = (prop: string): string => + resolveAnimIdForProperty(prop, gsapAnimations, gsapAnimId); const displayX = gsapRuntimeValues?.x ?? manualOffset.x; const displayY = gsapRuntimeValues?.y ?? manualOffset.y; diff --git a/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx b/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx index c36b65bf8..d5219093e 100644 --- a/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx +++ b/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx @@ -7,6 +7,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { TimelineClipDiamonds } from "./TimelineClipDiamonds"; import { getTimelinePropertyLanes, + resolveAnimIdForProperty, TimelinePropertyLanes, type TimelinePropertyLanesProps, } from "./TimelinePropertyLanes"; @@ -555,3 +556,49 @@ describe("TimelinePropertyLanes", () => { act(() => root.unmount()); }); }); + +describe("resolveAnimIdForProperty", () => { + /** A legacy mixed tween: the parser leaves propertyGroup undefined for it. */ + const mixed = { + id: "mixed-1", + targetSelector: "#box", + method: "to", + position: 0, + properties: {}, + keyframes: { + keyframes: [ + { percentage: 0, properties: { x: 0, opacity: 0 } }, + { percentage: 100, properties: { x: 40, opacity: 1 } }, + ], + }, + } as unknown as GsapAnimation; + + it("routes both groups of a mixed tween to that tween, not the fallback", () => { + expect(mixed.propertyGroup).toBeUndefined(); + + expect(resolveAnimIdForProperty("x", [mixed], "fallback")).toBe("mixed-1"); + expect(resolveAnimIdForProperty("opacity", [mixed], "fallback")).toBe("mixed-1"); + }); + + it("falls back only when no tween animates the property's group", () => { + expect(resolveAnimIdForProperty("rotation", [mixed], "fallback")).toBe("fallback"); + expect(resolveAnimIdForProperty("rotation", [mixed], undefined)).toBe(""); + }); + + it("prefers a single-group tween that owns the lane", () => { + const opacityOnly = { + ...mixed, + id: "opacity-1", + propertyGroup: "visual", + keyframes: { + keyframes: [ + { percentage: 0, properties: { opacity: 0 } }, + { percentage: 100, properties: { opacity: 1 } }, + ], + }, + } as unknown as GsapAnimation; + + expect(resolveAnimIdForProperty("opacity", [opacityOnly, mixed], "fallback")).toBe("opacity-1"); + expect(resolveAnimIdForProperty("x", [opacityOnly, mixed], "fallback")).toBe("mixed-1"); + }); +}); diff --git a/packages/studio/src/player/components/TimelinePropertyLanes.tsx b/packages/studio/src/player/components/TimelinePropertyLanes.tsx index 398360e44..8091b3df5 100644 --- a/packages/studio/src/player/components/TimelinePropertyLanes.tsx +++ b/packages/studio/src/player/components/TimelinePropertyLanes.tsx @@ -77,6 +77,28 @@ export function animationLaneGroups(animation: GsapAnimation): PropertyGroupName return Array.from(groups); } +/** + * Which tween a panel edit to `prop` belongs to. + * + * Matches on the groups the tween's KEYFRAMES animate, not on the parser's + * whole-tween `propertyGroup` verdict: that field is undefined for a legacy + * mixed tween such as `{ x, opacity }`, so matching it dropped every such + * tween and sent the edit to the selection's default animation instead, which + * is a different tween than the lane the user is looking at. + * {@link animationLaneGroups} is the single owner the rendered lanes count + * groups through, so resolving here through the same helper keeps the panel + * and the lanes on one answer. + */ +export function resolveAnimIdForProperty( + prop: string, + animations: readonly GsapAnimation[] | undefined, + fallbackAnimId: string | undefined, +): string { + const group = classifyPropertyGroup(prop); + const groupAnim = animations?.find((a) => animationLaneGroups(a).includes(group)); + return groupAnim?.id ?? fallbackAnimId ?? ""; +} + /** A tween contributes a property lane when it animates at least one property * on at least one editable keyframe (real or synthesized). */ export function animationContributesLane(animation: GsapAnimation): boolean {