Skip to content

Commit 00df2e3

Browse files
committed
fix(studio): lane every tween and attribute tweens to their real target
Two halves of one inversion in the expanded timeline lanes: the tweens that should show were filtered out, and a tween that should not be there was the only survivor. Lane classification read the parser's whole-tween verdict, which is undefined for anything spanning more than one property group. `{x, opacity}` is the canonical HyperFrames entrance tween, so five of the seven tweens in the swiss-grid graphics example had no caret, no reserved row and no diamonds. Classify per property instead, through one helper both the rendered lanes and the reserved row heights count through so they cannot drift again. Attribution matched an unanchored leading id, so `#stat3 .block` was filed under `#stat3`. The child's diamonds landed on its ancestor and collided with the ancestor's own tween at the shared percentage, which the same-percentage merge then resolved by dropping the ease. Route attribution through resolveSelectorElementIds, which anchors a whole-selector id and otherwise resolves through the live preview DOM, and anchor its no-DOM fallback so a descendant selector resolves to nothing rather than to its ancestor. The merge rule is unchanged. Also brings the last property-lane call site onto the shared clip timing basis: an expanded sub-composition child's start is host-absolute while its tweens are local to its own file.
1 parent d48440a commit 00df2e3

13 files changed

Lines changed: 437 additions & 93 deletions

‎packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,69 @@ describe("updateKeyframeCacheFromParsed", () => {
276276
expect(usePlayerStore.getState().gsapAnimations.get("scene.html#box")).toEqual([animation]);
277277
});
278278

279+
// `#stat3 .block` animates the BLOCK inside #stat3, not #stat3. An unanchored
280+
// `^#([\w-]+)/` prefix match filed it under "stat3", which both stole the
281+
// child's diamonds and collided with #stat3's own tween at the shared
282+
// percentage (dropping #stat3's ease as ambiguous).
283+
it("attributes a descendant selector to the child, not to its ancestor", () => {
284+
const parent: GsapAnimation = {
285+
id: "stat3-fromTo",
286+
targetSelector: "#stat3",
287+
method: "fromTo",
288+
position: 8.88,
289+
resolvedStart: 8.88,
290+
duration: 0.25,
291+
fromProperties: { y: 20 },
292+
properties: { y: 0 },
293+
ease: "power2.out",
294+
propertyGroup: "position",
295+
};
296+
const child: GsapAnimation = {
297+
id: "block-from",
298+
targetSelector: "#stat3 .block",
299+
method: "from",
300+
position: 9.13,
301+
resolvedStart: 9.13,
302+
duration: 0.3,
303+
properties: { opacity: 0 },
304+
ease: "power2.in",
305+
propertyGroup: "visual",
306+
};
307+
usePlayerStore.setState({
308+
elements: [
309+
{ id: "stat3-clip", domId: "stat3", tag: "div", start: 8.88, duration: 1, track: 0 },
310+
],
311+
});
312+
const doc = {
313+
querySelectorAll: (selector: string) =>
314+
(selector === "#stat3 .block"
315+
? [{ id: "stat3-block" }]
316+
: []) as unknown as NodeListOf<Element>,
317+
} as unknown as Document;
318+
319+
updateKeyframeCacheFromParsed([parent, child], "scene.html", "stat3", {}, doc);
320+
321+
expect(usePlayerStore.getState().gsapAnimations.get("scene.html#stat3")).toEqual([parent]);
322+
expect(usePlayerStore.getState().gsapAnimations.get("scene.html#stat3-block")).toEqual([child]);
323+
// With the collision gone, #stat3's own curve survives instead of being
324+
// deleted as an ambiguous same-percentage merge.
325+
const parentKeyframes = cache().get("scene.html#stat3")?.keyframes ?? [];
326+
expect(parentKeyframes.at(-1)?.ease).toBe("power2.out");
327+
expect(parentKeyframes.some((keyframe) => "easeAmbiguous" in keyframe)).toBe(false);
328+
});
329+
330+
it("attributes a descendant selector to nothing when there is no document", () => {
331+
const child: GsapAnimation = {
332+
...animWithKeyframes("block-from"),
333+
targetSelector: "#stat3 .block",
334+
};
335+
336+
updateKeyframeCacheFromParsed([child], "scene.html", "stat3", {});
337+
338+
expect(cache().has("scene.html#stat3")).toBe(false);
339+
expect(usePlayerStore.getState().gsapAnimations.has("scene.html#stat3")).toBe(false);
340+
});
341+
279342
it("does not cache a flat tween without animatable numeric properties", () => {
280343
const animation: GsapAnimation = {
281344
id: "flat-box",

‎packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts‎

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,66 +4,73 @@
44
*/
55
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
66
import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore";
7-
import { idFromSelector, resolveClipTimingBasis, toClipKeyframes } from "./gsapShared";
7+
import { resolveClipTimingBasis, resolveSelectorElementIds, toClipKeyframes } from "./gsapShared";
88
import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth";
99

1010
export function updateKeyframeCacheFromParsed(
1111
animations: GsapAnimation[],
1212
targetPath: string,
1313
selectionId: string | undefined,
1414
mutation: Record<string, unknown>,
15+
doc?: Document | null,
1516
): void {
1617
const { setKeyframeCache, elements, domClipChildren } = usePlayerStore.getState();
1718
const idsWithKeyframes = new Set<string>();
1819
const merged = new Map<string, KeyframeCacheEntry>();
1920
const sourceAnimations = new Map<string, GsapAnimation[]>();
2021
for (const anim of animations) {
21-
const id = idFromSelector(anim.targetSelector);
2222
const kfSource =
2323
anim.keyframes?.keyframes ?? synthesizeFlatTweenKeyframes(anim)?.keyframes ?? [];
24-
if (!id || kfSource.length === 0) continue;
25-
idsWithKeyframes.add(id);
26-
// Every tween that fed keyframeCache also lands in gsapAnimations, group or
27-
// not: a mixed-group tween (`{ x, opacity }` classifies to undefined) used to
28-
// cache diamonds with no source animation behind them, so the collapsed row
29-
// drew keyframes the expanded lanes couldn't render. Lane consumers do the
30-
// group filtering themselves (animationContributesLane).
31-
sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]);
24+
if (kfSource.length === 0) continue;
25+
// Attribute the tween to every element it actually animates. A leading-id
26+
// match filed `#stat3 .block` under `#stat3`: the child's diamonds landed on
27+
// its ancestor AND collided with the ancestor's own tween at the shared
28+
// percentage, which the same-% merge then resolved by dropping the ease.
29+
for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) {
30+
idsWithKeyframes.add(id);
31+
// Every tween that fed keyframeCache also lands in gsapAnimations, group or
32+
// not: a mixed-group tween (`{ x, opacity }` classifies to undefined) used to
33+
// cache diamonds with no source animation behind them, so the collapsed row
34+
// drew keyframes the expanded lanes couldn't render. Lane consumers do the
35+
// group filtering themselves (animationContributesLane).
36+
sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]);
3237

33-
// Convert tween-relative percentages to clip-relative so diamonds
34-
// render at the correct position within the timeline clip. The basis comes
35-
// from the shared resolver, so this writer agrees with the AST load on both
36-
// the sub-comp host fallback and the tween's own time frame.
37-
const { elStart, elDuration } = resolveClipTimingBasis(
38-
id,
39-
targetPath,
40-
elements,
41-
domClipChildren,
42-
);
43-
const clipKeyframes = toClipKeyframes(kfSource, anim, elStart, elDuration);
38+
// Convert tween-relative percentages to clip-relative so diamonds
39+
// render at the correct position within the timeline clip. The basis comes
40+
// from the shared resolver, so this writer agrees with the AST load on both
41+
// the sub-comp host fallback and the tween's own time frame.
42+
const { elStart, elDuration } = resolveClipTimingBasis(
43+
id,
44+
targetPath,
45+
elements,
46+
domClipChildren,
47+
);
48+
const clipKeyframes = toClipKeyframes(kfSource, anim, elStart, elDuration);
4449

45-
const existing = merged.get(id);
46-
if (existing) {
47-
// deduplicateKeyframes owns the same-% merge (including the easeAmbiguous
48-
// flag downstream lanes read); a second copy of that rule here is how the
49-
// two writers drift.
50-
existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]);
51-
} else {
52-
merged.set(id, {
53-
...anim.keyframes,
54-
format: anim.keyframes?.format ?? "percentage",
55-
keyframes: clipKeyframes,
56-
});
50+
const existing = merged.get(id);
51+
if (existing) {
52+
// deduplicateKeyframes owns the same-% merge (including the easeAmbiguous
53+
// flag downstream lanes read); a second copy of that rule here is how the
54+
// two writers drift.
55+
existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]);
56+
} else {
57+
merged.set(id, {
58+
...anim.keyframes,
59+
format: anim.keyframes?.format ?? "percentage",
60+
keyframes: clipKeyframes,
61+
});
62+
}
5763
}
5864
}
5965
for (const [id, entry] of merged) {
6066
for (const key of elementCacheKeys(targetPath, id)) setKeyframeCache(key, entry);
6167
writeGsapAnimationsForElement(targetPath, id, sourceAnimations.get(id));
6268
}
63-
const targetId =
64-
idFromSelector((mutation as { targetSelector?: string }).targetSelector) ?? selectionId;
65-
if (targetId && !idsWithKeyframes.has(targetId)) {
66-
clearKeyframeCacheForElement(targetPath, targetId);
69+
const mutationSelector = (mutation as { targetSelector?: string }).targetSelector;
70+
const mutated = mutationSelector ? resolveSelectorElementIds(mutationSelector, doc) : [];
71+
const targetIds = mutated.length > 0 ? mutated : selectionId ? [selectionId] : [];
72+
for (const targetId of targetIds) {
73+
if (!idsWithKeyframes.has(targetId)) clearKeyframeCacheForElement(targetPath, targetId);
6774
}
6875
}
6976

‎packages/studio/src/hooks/gsapShared.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,71 @@ export function idFromSelector(selector: string | undefined | null): string | nu
110110
return (attribute[1] ?? "").replace(/\\(["\\])/g, "$1");
111111
}
112112

113+
/** Either shape {@link idSelector} emits, anchored to the WHOLE selector. */
114+
const WHOLE_SELECTOR_ID = /^(#[\w-]+|\[id="(?:\\.|[^"\\])*"\])$/;
115+
116+
/**
117+
* The id a selector addresses **as a whole**, or null. `"#stat3 .block"` animates
118+
* the `.block` INSIDE `#stat3`, not `#stat3`, so the unanchored leading-id match
119+
* of {@link idFromSelector} is wrong for attribution: it files the child's
120+
* keyframes under its ancestor. `idFromSelector` stays unanchored on purpose
121+
* (two non-attribution callers want the leading id); attribution goes through
122+
* here, or through the DOM (see resolveSelectorElementIds).
123+
*/
124+
function wholeSelectorElementId(selector: string): string | null {
125+
const trimmed = selector.trim();
126+
return WHOLE_SELECTOR_ID.test(trimmed) ? idFromSelector(trimmed) : null;
127+
}
128+
129+
/**
130+
* Resolve a tween's target selector to the ids of the element(s) it animates.
131+
* A whole-selector `#id` resolves directly; anything else (a class like `.dot`,
132+
* a group `.a, .b`, or a descendant selector) is matched against the live
133+
* preview DOM so class/selector tweens (e.g. `gsap.from(".dot", {stagger})`)
134+
* attribute to every element they animate — not just one parsed from the string.
135+
* With no DOM, only whole-selector ids resolve: a descendant selector has no
136+
* answer that isn't a guess at its ancestor.
137+
*/
138+
export function resolveSelectorElementIds(
139+
selector: string,
140+
doc: Document | null | undefined,
141+
): string[] {
142+
const bareId = wholeSelectorElementId(selector);
143+
if (bareId) return [bareId];
144+
const ids = new Set<string>();
145+
for (const part of selector.split(",")) {
146+
const sel = part.trim();
147+
if (!sel) continue;
148+
if (!doc) {
149+
const whole = wholeSelectorElementId(sel);
150+
if (whole) ids.add(whole);
151+
continue;
152+
}
153+
try {
154+
for (const el of Array.from(doc.querySelectorAll(sel))) {
155+
if (el.id) ids.add(el.id);
156+
}
157+
} catch {
158+
// An unsupported/invalid selector never reached the DOM, so the leading id
159+
// is the best available answer (`[id="01-hook"]:has(>*)` still names it).
160+
const lead = idFromSelector(sel);
161+
if (lead) ids.add(lead);
162+
}
163+
}
164+
return Array.from(ids);
165+
}
166+
167+
/**
168+
* The clip start in the frame the element's OWN tweens are measured in. An
169+
* expanded sub-composition child sits on the master timeline at a host-absolute
170+
* `start`, but its tweens are parsed from its own source file and are local to
171+
* it, so the two must be brought into one frame before any clip-% math — or
172+
* every keyframe rebases to a percentage far outside the clip.
173+
*/
174+
export function clipTimingStart(element: { start: number; expandedParentStart?: number }): number {
175+
return element.start - (element.expandedParentStart ?? 0);
176+
}
177+
113178
export function selectorFromSelection(selection: DomEditSelection): string | null {
114179
if (selection.id) return idSelector(selection.id);
115180
if (selection.selector) return selection.selector;

‎packages/studio/src/hooks/keyframeCacheAstLoad.ts‎

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,50 +11,15 @@ import {
1111
elementCacheKeys,
1212
writeGsapAnimationsForElement,
1313
} from "./gsapKeyframeCacheHelpers";
14-
import { idFromSelector, resolveClipTimingBasis, toClipKeyframes } from "./gsapShared";
14+
import { resolveClipTimingBasis, resolveSelectorElementIds, toClipKeyframes } from "./gsapShared";
1515
import {
1616
deduplicateKeyframes,
1717
isStaticPositionHold,
1818
synthesizeFlatTweenKeyframes,
1919
} from "./gsapTweenSynth";
2020

21-
/**
22-
* Resolve a tween's target selector to the ids of the element(s) it animates.
23-
* A bare `#id` resolves directly; anything else (a class like `.dot`, a group
24-
* `.a, .b`, or a descendant selector) is matched against the live preview DOM so
25-
* class/selector tweens (e.g. `gsap.from(".dot", {stagger})`) attribute to every
26-
* element they animate — not just one parsed from the string. Falls back to a
27-
* leading `#id` when there's no DOM (so the cache still populates pre-iframe).
28-
*/
29-
// fallow-ignore-next-line complexity
30-
export function resolveSelectorElementIds(
31-
selector: string,
32-
doc: Document | null | undefined,
33-
): string[] {
34-
// A whole-selector id match (either shape) addresses exactly one element.
35-
const bareId = /^(#[\w-]+|\[id="(?:\\.|[^"\\])*"\])$/.test(selector)
36-
? idFromSelector(selector)
37-
: null;
38-
if (bareId) return [bareId];
39-
if (!doc) {
40-
const lead = idFromSelector(selector);
41-
return lead ? [lead] : [];
42-
}
43-
const ids = new Set<string>();
44-
for (const part of selector.split(",")) {
45-
const sel = part.trim();
46-
if (!sel) continue;
47-
try {
48-
for (const el of Array.from(doc.querySelectorAll(sel))) {
49-
if (el.id) ids.add(el.id);
50-
}
51-
} catch {
52-
const lead = idFromSelector(sel);
53-
if (lead) ids.add(lead);
54-
}
55-
}
56-
return Array.from(ids);
57-
}
21+
export { resolveSelectorElementIds };
22+
5823
/**
5924
* The slice of the parse response callers actually read. The endpoint returns
6025
* the full `ParsedGsap` (preamble/postamble and all), but nothing downstream of

‎packages/studio/src/hooks/useGsapScriptCommits.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,9 @@ function syncCommittedGsapMutation({
186186
targetPath,
187187
selection.id ?? undefined,
188188
mutation,
189+
// The live preview document is what resolves a class / descendant tween to
190+
// the elements it really animates; without it only whole-id selectors do.
191+
iframe?.contentDocument,
189192
);
190193
}
191194
refreshMutationPreview(iframe, result, options, reloadPreview, onCacheInvalidate);

‎packages/studio/src/hooks/useGsapTweenCache.test.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,24 @@ describe("resolveSelectorElementIds", () => {
8686
expect(resolveSelectorElementIds(".a, .b", doc).sort()).toEqual(["x", "y"]);
8787
});
8888

89-
it("falls back to a leading #id when there is no DOM", () => {
90-
expect(resolveSelectorElementIds("#card .label", null)).toEqual(["card"]);
89+
// A DOM-less LEADING-id fallback attributed `#card .label` to `#card`, the
90+
// ancestor it merely scopes to. Without a DOM there is nothing to resolve the
91+
// descendant against, so the honest answer is no element at all.
92+
it("resolves nothing for a compound selector when there is no DOM", () => {
93+
expect(resolveSelectorElementIds("#card .label", null)).toEqual([]);
9194
expect(resolveSelectorElementIds(".dot", null)).toEqual([]);
9295
});
9396

97+
it("still resolves every whole-id part of a group selector without a DOM", () => {
98+
expect(resolveSelectorElementIds("#a, #b", null)).toEqual(["a", "b"]);
99+
});
100+
94101
// The `[id="…"]` form is what writers emit for a CSS-unsafe id (digit-leading,
95102
// dotted). The old local `#id`-only regex read no id at all for those, so they
96103
// silently dropped out of both DOM-less paths.
97104
it("falls back to a bracketed id when there is no DOM", () => {
98-
expect(resolveSelectorElementIds('[id="01-hook"] .label', null)).toEqual(["01-hook"]);
105+
expect(resolveSelectorElementIds('[id="01-hook"]', null)).toEqual(["01-hook"]);
106+
expect(resolveSelectorElementIds('[id="01-hook"] .label', null)).toEqual([]);
99107
});
100108

101109
it("falls back to a bracketed id when querySelectorAll rejects the selector", () => {

‎packages/studio/src/player/components/TimelineLanes.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { TimelineClipDiamonds } from "./TimelineClipDiamonds";
66
import { TimelinePropertyLanes } from "./TimelinePropertyLanes";
77
import { TimelineTrackHeader } from "./TimelineTrackHeader";
88
import { resolveTrackKeyframeClip } from "./useTimelineTrackLayout";
9+
import { clipTimingStart } from "../../hooks/gsapShared";
910
import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity";
1011
import type { MusicBeatAnalysis } from "@hyperframes/core/beats";
1112
import { getTimelineEditCapabilities, resolveBlockedTimelineEditIntent } from "./timelineEditing";
@@ -527,7 +528,10 @@ export function TimelineLanes({
527528
<TimelinePropertyLanes
528529
key={`${clipKey}-property-lanes`}
529530
animations={gsapAnimations.get(elementKey) ?? []}
530-
clipStart={previewElement.start}
531+
// clipTimingStart, not the raw start: an expanded sub-comp
532+
// child's start is host-absolute while its tweens are
533+
// local to its own file.
534+
clipStart={clipTimingStart(previewElement)}
531535
clipDuration={previewElement.duration}
532536
clipLeftPx={previewElement.start * pps}
533537
clipWidthPx={Math.max(previewElement.duration * pps, 4)}

0 commit comments

Comments
 (0)