From 3f0c20f633e9ac064e45df838e03d4412ddafda8 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 05:04:59 +0200 Subject: [PATCH 1/4] fix(studio): compute keyframe percentages in the tween's own time frame A sub-composition tween's resolvedStart is composition-local, while the timeline element resolved for it is the sub-comp HOST, whose start is main-timeline absolute. toClipPercentage subtracted the two frames from each other, so a host mounted at 1.5s cached its 0s tween at -12% and its last tween's end keyframe at 88% instead of 100%. A clip-relative percentage can never be negative. resolveClipTimingBasis now returns the clip start in the frame the tween's own times are measured in: the composition mount (expandedParentStart for an expanded child, the parent composition clip's start otherwise, 0 for a root-composition element) is subtracted, and a sub-comp inner element that falls back to its host's window starts at 0 in that window. It moves to gsapShared so the post-commit cache writer can share it instead of resolving its own basis, which also gives that writer the sub-comp host fallback it was missing. --- .../src/hooks/gsapKeyframeCacheHelpers.ts | 22 ++-- packages/studio/src/hooks/gsapShared.test.ts | 119 ++++++++++++++++++ packages/studio/src/hooks/gsapShared.ts | 53 ++++++++ .../studio/src/hooks/keyframeCacheAstLoad.ts | 37 +----- .../studio/src/hooks/useGsapTweenCache.ts | 15 +-- 5 files changed, 190 insertions(+), 56 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index 929960a16..80fb02e01 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -4,7 +4,7 @@ */ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; -import { idFromSelector, toClipKeyframes } from "./gsapShared"; +import { idFromSelector, resolveClipTimingBasis, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( @@ -13,7 +13,7 @@ export function updateKeyframeCacheFromParsed( selectionId: string | undefined, mutation: Record, ): void { - const { setKeyframeCache, elements } = usePlayerStore.getState(); + const { setKeyframeCache, elements, domClipChildren } = usePlayerStore.getState(); const idsWithKeyframes = new Set(); const merged = new Map(); const sourceAnimations = new Map(); @@ -31,16 +31,16 @@ export function updateKeyframeCacheFromParsed( sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]); // Convert tween-relative percentages to clip-relative so diamonds - // render at the correct position within the timeline clip. - const timelineEl = elements.find( - (el) => el.domId === id || (el.key ?? el.id) === `${targetPath}#${id}`, - ); - const clipKeyframes = toClipKeyframes( - kfSource, - anim, - timelineEl?.start ?? 0, - timelineEl?.duration ?? 1, + // render at the correct position within the timeline clip. The basis comes + // from the shared resolver, so this writer agrees with the AST load on both + // the sub-comp host fallback and the tween's own time frame. + const { elStart, elDuration } = resolveClipTimingBasis( + id, + targetPath, + elements, + domClipChildren, ); + const clipKeyframes = toClipKeyframes(kfSource, anim, elStart, elDuration); const existing = merged.get(id); if (existing) { diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index 604b93b69..98717e1a8 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -6,6 +6,7 @@ import { idSelector, isInstantHold, parsePercentageKeyframes, + resolveClipTimingBasis, resolveEditableTweenDuration, toClipKeyframes, toClipPercentage, @@ -169,6 +170,124 @@ describe("toClipKeyframes", () => { }); }); +describe("resolveClipTimingBasis", () => { + // Measured on v-product-promo: `captions-comp` mounts at 1.5s for 12.5s, and the + // six tweens inside it resolve to 0..12.5 — composition-local, not main-timeline + // absolute. Subtracting the host's 1.5 mount from a 0s tween cached pct -12. + const host = { id: "captions-comp", domId: "captions-comp", start: 1.5, duration: 12.5 }; + const children = [{ id: "line", hostId: "captions-comp" }]; + + it("gives a sub-composition inner element the host window in the tween's own frame", () => { + expect(resolveClipTimingBasis("line", "captions.html", [host], children)).toEqual({ + elStart: 0, + elDuration: 12.5, + }); + }); + + it("leaves a root-composition element on the main timeline", () => { + const box = { id: "box", domId: "box", start: 3, duration: 2 }; + expect(resolveClipTimingBasis("box", "index.html", [box], [])).toEqual({ + elStart: 3, + elDuration: 2, + }); + }); + + it("rebases an expanded sub-comp child by its host mount", () => { + // Expanded children carry host-ABSOLUTE display starts; the tweens they own are + // still composition-local, so the basis is the child's local start. + const pill = { id: "pill", domId: "pill", start: 8, duration: 4, expandedParentStart: 6 }; + expect(resolveClipTimingBasis("pill", "scene.html", [pill], [])).toEqual({ + elStart: 2, + elDuration: 4, + }); + }); + + it("rebases by the parent composition clip when the child is not expanded", () => { + const parent = { id: "scene-comp", domId: "scene-comp", start: 5, duration: 10 }; + const pill = { + id: "pill", + domId: "pill", + start: 7, + duration: 3, + parentCompositionId: "scene-comp", + }; + expect(resolveClipTimingBasis("pill", "scene.html", [parent, pill], [])).toEqual({ + elStart: 2, + elDuration: 3, + }); + }); + + it("falls back to a unit window when neither the element nor a host resolves", () => { + expect(resolveClipTimingBasis("ghost", "index.html", [], [])).toEqual({ + elStart: 0, + elDuration: 1, + }); + }); +}); + +describe("sub-composition keyframe percentages", () => { + const host = { id: "captions-comp", domId: "captions-comp", start: 1.5, duration: 12.5 }; + const children = [{ id: "line", hostId: "captions-comp" }]; + const basis = () => resolveClipTimingBasis("line", "captions.html", [host], children); + const inner = (resolvedStart: number, duration?: number) => + ({ + id: `t-${resolvedStart}`, + method: "to", + targetSelector: "#line", + vars: {}, + resolvedStart, + duration, + }) as unknown as GsapAnimation; + const percentages = (animation: GsapAnimation) => { + const { elStart, elDuration } = basis(); + return toClipKeyframes( + [{ percentage: 0 }, { percentage: 100 }], + animation, + elStart, + elDuration, + ).map((row) => row.percentage); + }; + + it("puts a tween on the host's first frame at 0%, never below zero", () => { + // A clip-relative percentage can never be negative; this one cached -12. + expect(percentages(inner(0))).toEqual([0, 100]); + }); + + it("puts the last tween's end keyframe at 100%", () => { + expect(percentages(inner(12.1, 0.4))).toEqual([96.8, 100]); + }); + + it("keeps every measured tween of the fixture inside 0..100", () => { + for (const start of [0, 3.2, 3.5, 7.7, 8, 12.1]) { + for (const percentage of percentages(inner(start, 0.4))) { + expect(percentage).toBeGreaterThanOrEqual(0); + expect(percentage).toBeLessThanOrEqual(100); + } + } + }); + + it("keeps a root-composition tween at the head of its own clip", () => { + const box = { id: "box", domId: "box", start: 3, duration: 2 }; + const { elStart, elDuration } = resolveClipTimingBasis("box", "index.html", [box], []); + const rows = toClipKeyframes([{ percentage: 0 }], inner(3, 2), elStart, elDuration); + expect(rows[0]!.percentage).toBe(0); + }); + + it("passes tween percentages through for a zero-length clip", () => { + expect(toClipKeyframes([{ percentage: 40 }], inner(0, 0.4), 0, 0)[0]!.percentage).toBe(40); + }); + + it("round-trips a clip percentage through the basis it was written with", () => { + // The drag commit converts a dropped clip-% back to a time with this basis + // (useTimelineEditCallbacks) and compares it against the tween's own + // resolvedStart, so the basis has to be in the tween's frame on both sides. + const { elStart, elDuration } = basis(); + const absTime = elStart + (40 / 100) * elDuration; + expect(absTime).toBe(5); + expect(toClipPercentage(absTime, elStart, elDuration, 0)).toBe(40); + }); +}); + describe("idFromSelector", () => { it("round-trips every shape idSelector emits", () => { for (const id of ["hero-word", "el_1", "01-hook-hero-word", "my.class", "1box", '1"x']) { diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index 00c79ceda..d80db4ad3 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -259,6 +259,59 @@ export function toAbsoluteTime(tweenPos: number, tweenDur: number, percentage: n return tweenPos + (percentage / 100) * tweenDur; } +/** + * Timing basis for an element's keyframes, expressed in the TWEEN's own time + * frame. Sub-composition internals (e.g. pills inside a scene) aren't timeline + * clips themselves — they're derived at expand time — so they're absent from + * `elements`. Without a basis, elDuration defaulted to 1 and clip-relative + * keyframe percentages blew past 100% (rendering off the clip). Fall back to the + * sub-comp HOST's bounds, resolved via domClipChildren (the host's + * data-composition-src is stripped in the rendered DOM, so we can't query it). + * + * `elStart` is the clip's start in the frame the tween's own times are measured + * in. A sub-composition tween's resolvedStart is composition-local while a + * timeline element's start is main-timeline absolute, so passing the raw element + * start subtracted two different frames from each other: a host mounted at 1.5s + * cached its 0s tween at -12%, and a clip-relative percentage can never be + * negative. The composition's mount is `expandedParentStart` for an expanded + * child, the parent composition clip's start otherwise, and 0 for a + * root-composition element, whose start already IS the tween frame. + */ +export function resolveClipTimingBasis( + elementId: string, + sourceFile: string, + elements: ReadonlyArray<{ + domId?: string; + key?: string; + id: string; + start: number; + duration: number; + expandedParentStart?: number; + parentCompositionId?: string | null; + }>, + domClipChildren: ReadonlyArray<{ id: string; hostId: string }>, +): { elStart: number; elDuration: number } { + const direct = elements.find( + (el) => el.domId === elementId || (el.key ?? el.id) === `${sourceFile}#${elementId}`, + ); + if (direct) { + const parentId = direct.parentCompositionId; + const parent = parentId + ? elements.find((el) => el.domId === parentId || el.id === parentId) + : undefined; + const mount = direct.expandedParentStart ?? parent?.start ?? 0; + return { elStart: direct.start - mount, elDuration: direct.duration }; + } + const hostId = domClipChildren.find((c) => c.id === elementId)?.hostId; + const host = hostId + ? elements.find((el) => el.domId === hostId || (el.key ?? el.id) === `index.html#${hostId}`) + : undefined; + // The inner element is not a clip of its own: the host's window IS the frame + // its tweens are timed in, so the start in that frame is 0, not the host's + // main-timeline mount. + return { elStart: 0, elDuration: host?.duration ?? 1 }; +} + /** * An absolute time as a percentage of a timeline clip, at the one precision every * keyframe-cache writer must share. 0.001% keeps a beat-snapped keyframe centered diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index bca5f4b23..f940d6160 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -1,6 +1,6 @@ /** - * Reading a composition file's GSAP tweens into the keyframe cache: fetch, - * selector -> element id resolution, and the clip-relative timing basis. + * Reading a composition file's GSAP tweens into the keyframe cache: fetch and + * selector -> element id resolution. * Split from useGsapTweenCache to keep that file under the 600-line limit. */ import type { GsapAnimation, GsapKeyframesData, ParsedGsap } from "@hyperframes/core/gsap-parser"; @@ -11,7 +11,7 @@ import { elementCacheKeys, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { idFromSelector, toClipKeyframes } from "./gsapShared"; +import { idFromSelector, resolveClipTimingBasis, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, isStaticPositionHold, @@ -103,37 +103,6 @@ export async function fetchParsedAnimations( } } -/** - * Clip-relative timing basis for an element. Sub-composition internals (e.g. pills - * inside a scene) aren't timeline clips themselves — they're derived at expand time - * — so they're absent from `elements`. Without a basis, elDuration defaulted to 1 - * and clip-relative keyframe percentages blew past 100% (rendering off the clip). - * Fall back to the sub-comp HOST's bounds, resolved via domClipChildren (the host's - * data-composition-src is stripped in the rendered DOM, so we can't query it). - */ -export function resolveClipTimingBasis( - elementId: string, - sourceFile: string, - elements: ReadonlyArray<{ - domId?: string; - key?: string; - id: string; - start: number; - duration: number; - }>, - domClipChildren: ReadonlyArray<{ id: string; hostId: string }>, -): { elStart: number; elDuration: number } { - const direct = elements.find( - (el) => el.domId === elementId || (el.key ?? el.id) === `${sourceFile}#${elementId}`, - ); - if (direct) return { elStart: direct.start, elDuration: direct.duration }; - const hostId = domClipChildren.find((c) => c.id === elementId)?.hostId; - const host = hostId - ? elements.find((el) => el.domId === hostId || (el.key ?? el.id) === `index.html#${hostId}`) - : undefined; - return { elStart: host?.start ?? 0, elDuration: host?.duration ?? 1 }; -} - /** * Read one composition file's tweens into the keyframe cache. Split out of the * hook so the effect can run it per file without re-nesting the whole body. diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 0f89be655..23a8ea728 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -7,24 +7,17 @@ import { pruneKeyframeCacheToFiles, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { toAbsoluteTime, toClipPercentage } from "./gsapShared"; +import { resolveClipTimingBasis, toAbsoluteTime, toClipPercentage } from "./gsapShared"; import { deduplicateKeyframes, isStaticPositionHold, synthesizeFlatTweenKeyframes, } from "./gsapTweenSynth"; -import { - fetchParsedAnimations, - populateKeyframeCacheFromAst, - resolveClipTimingBasis, -} from "./keyframeCacheAstLoad"; +import { fetchParsedAnimations, populateKeyframeCacheFromAst } from "./keyframeCacheAstLoad"; // Re-exported so callers keep importing the GSAP cache surface from one module. -export { - fetchParsedAnimations, - resolveClipTimingBasis, - resolveSelectorElementIds, -} from "./keyframeCacheAstLoad"; +export { resolveClipTimingBasis } from "./gsapShared"; +export { fetchParsedAnimations, resolveSelectorElementIds } from "./keyframeCacheAstLoad"; /** The selected element's identity for matching tweens to it. */ export interface GsapElementTarget { From 12546985f588df8cf984d241dd27e3417edb3320 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 16:46:17 +0200 Subject: [PATCH 2/4] fix(studio): expand sub-comp children against their resolved parent host buildExpandedElements synthesized DOM-only sub-composition children against the top-level element rather than the parentHost it resolves immediately after. Under two-level nesting every child row therefore inherited the top-level window instead of its own host's, so the rows drew at the wrong offset and duration. --- .../hooks/useExpandedTimelineElements.test.ts | 39 +++++++++++++++++++ .../hooks/useExpandedTimelineElements.ts | 19 ++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts index 12adcd2cd..98b37ad81 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts @@ -243,6 +243,45 @@ describe("buildExpandedElements", () => { // The host row is replaced by its children. expect(out.some((e) => e.domId === "scene-host")).toBe(false); }); + + // Regression: DOM-only children were synthesized against the TOP-LEVEL element + // instead of the sub-comp host they actually live in, so every child row read + // the whole top-level window rather than its host's. + it("spans DOM-only children over their nested host's window, not the top-level one", () => { + const elements = [ + el({ id: "scene-host", start: 0, duration: 20, compositionSrc: "scene.html" }), + ]; + const manifest = [ + clip({ id: "scene-host", start: 0, duration: 20, compositionSrc: "scene.html" }), + clip({ id: "sub-host", start: 5, duration: 6, compositionSrc: "sub.html" }), + ]; + const parentMap = new Map([ + ["sub-host", "scene-host"], + ["pill-1", "sub-host"], + ]); + const domClipChildren = [ + { + id: "pill-1", + parentId: "sub-host", + hostId: "sub-host", + label: "pill-1", + stackingContextId: "css:0.0", + }, + ]; + + const out = buildExpandedElements( + elements, + manifest, + parentMap, + "scene-host", + "sub-host", + domClipChildren, + ); + const pill = out.find((e) => e.domId === "pill-1")!; + expect(pill.start).toBe(5); + expect(pill.duration).toBe(6); + expect(pill.sourceFile).toBe("sub.html"); + }); }); describe("resolveTimelineExpansionRawId", () => { diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts index 7ce6c5d66..17309385b 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts @@ -209,7 +209,13 @@ function buildChildElements( function domSiblingClips( domClipChildren: DomClipChild[], siblingParentId: string, - host: TimelineElement, + host: { + id: string | null; + start: number; + duration: number; + track: number; + compositionSrc?: string | null; + }, ): ClipManifestClip[] { return domClipChildren .filter((c) => c.parentId === siblingParentId) @@ -243,20 +249,23 @@ export function buildExpandedElements( const topLevelElement = elements.find((el) => el.id === topLevelId || el.domId === topLevelId); if (!topLevelElement) return filterToTopLevel(elements, parentMap); + // The sub-comp host the children actually live in: top-level host for 1-level + // nesting, a nested host for deeper nesting. Its start/file anchor edits. + const parentHost = manifest.find((c) => c.id === siblingParentId); + // Prefer real manifest children; fall back to DOM-only sub-comp children // (groups/pills) that have no data-start and thus never enter the manifest. + // Those are synthesized against the host they actually live in, not the + // top-level element, or every child row reads the whole top-level window. const siblings = (() => { const fromManifest = manifest.filter( (c) => c.id != null && parentMap.get(c.id) === siblingParentId, ); if (fromManifest.length > 0) return fromManifest; - return domSiblingClips(domClipChildren, siblingParentId, topLevelElement); + return domSiblingClips(domClipChildren, siblingParentId, parentHost ?? topLevelElement); })(); if (siblings.length === 0) return filterToTopLevel(elements, parentMap); - // The sub-comp host the children actually live in: top-level host for 1-level - // nesting, a nested host for deeper nesting. Its start/file anchor edits. - const parentHost = manifest.find((c) => c.id === siblingParentId); const editBasis = { start: parentHost?.start ?? topLevelElement.start, sourceFile: parentHost?.compositionSrc ?? topLevelElement.compositionSrc ?? undefined, From d48440a9bacb83aef8007b3667e78e02a2481d8f Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 05:15:07 +0200 Subject: [PATCH 3/4] fix(studio): keep the host row when drilling into a sub-composition The sub-composition drill-in replaced the host row with its children. Since expansion is also driven by the playhead alone (paused auto-expand), an ordinary seek into a sub-composition made the host row disappear, taking its keyframe lane with it: diamonds render per row from keyframeCache.get( elementKey), so no row means no diamonds. Reproduced live with no drag at all, seek 0 gave 3 diamonds, seek 7.68 gave 0, seek 0.2 gave 3. Make the expansion additive instead. The host row stays and its children are appended directly below it. The synthetic fractional lanes already used for children sit strictly between the host's lane and the next integer, so the host keeps its own row without colliding with anything. The time-keyed auto-expand itself is unchanged. --- .../hooks/useExpandedTimelineElements.test.ts | 69 ++++++++++++++++++- .../hooks/useExpandedTimelineElements.ts | 9 ++- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts index 98b37ad81..204c1509a 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts @@ -240,8 +240,73 @@ describe("buildExpandedElements", () => { expect(pills[0]!.duration).toBe(6); expect(pills[0]!.sourceFile).toBe("scene.html"); expect(pills.map((pill) => pill.stackingContextId)).toEqual(["css:0.0", "css:0.1", "css:0.1"]); - // The host row is replaced by its children. - expect(out.some((e) => e.domId === "scene-host")).toBe(false); + // The host row survives the expansion; its children are added under it. + expect(out.some((e) => e.id === "scene-host")).toBe(true); + }); + + // fallow-ignore-next-line code-duplication + it("keeps the host row and appends its children directly below it", () => { + const elements = [ + el({ + id: "s3", + domId: "s3", + key: "index.html#s3", + start: 16, + duration: 7, + compositionSrc: "stats.html", + }), + el({ id: "outro", start: 23, duration: 3, track: 1 }), + ]; + const manifest = [ + clip({ id: "s3", start: 16, duration: 7, compositionSrc: "stats.html" }), + clip({ id: "stat-1", start: 16.5, duration: 5 }), + clip({ id: "stat-2", start: 16.9, duration: 5 }), + ]; + const parentMap = new Map([ + ["stat-1", "s3"], + ["stat-2", "s3"], + ]); + + const out = buildExpandedElements(elements, manifest, parentMap, "s3", "s3"); + + const hostIndex = out.findIndex((e) => e.id === "s3"); + expect(hostIndex).toBeGreaterThanOrEqual(0); + // Host row untouched (same key → same keyframe lane), children nested under it. + expect(out[hostIndex]!.key).toBe("index.html#s3"); + expect(out[hostIndex]!.track).toBe(0); + expect(out[hostIndex + 1]!.domId).toBe("stat-1"); + expect(out[hostIndex + 2]!.domId).toBe("stat-2"); + // Exactly one more row than the old substitution behaviour (host + 2 children + outro). + expect(out).toHaveLength(4); + }); + + it("keeps the host row present at every playhead position (keyframe lane repro)", () => { + // Live repro with no drag at all: seek 0 gave 3 diamonds, seek 7.68 gave 0, + // seek 0.2 gave 3. Diamonds render per row from keyframeCache.get(elementKey), + // so the whole lane went with the host row whenever the paused drill-in + // substituted it for its children. + const elements = [ + el({ id: "scene", domId: "scene", key: "index.html#scene", start: 0, duration: 12 }), + ]; + const manifest = [ + clip({ id: "scene", start: 0, duration: 12, compositionSrc: "scene.html" }), + clip({ id: "headline", start: 0, duration: 12 }), + ]; + const parentMap = new Map([["headline", "scene"]]); + + for (const currentTime of [0, 7.68, 0.2]) { + const rawId = resolveTimelineExpansionRawId({ + selectedElementId: null, + isPlaying: false, + currentTime, + manifest, + parentMap, + }); + const rows = rawId + ? buildExpandedElements(elements, manifest, parentMap, rawId, rawId) + : elements; + expect(rows.map((row) => row.key)).toContain("index.html#scene"); + } }); // Regression: DOM-only children were synthesized against the TOP-LEVEL element diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts index 17309385b..57eae95b6 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts @@ -284,9 +284,16 @@ export function buildExpandedElements( ); if (expanded.length === 0) return filterToTopLevel(elements, parentMap); + // ADDITIVE drill-in: the host row stays and its children are appended under + // it. Expansion is also triggered by the playhead alone (paused auto-expand), + // so substituting the host row made it vanish on an ordinary seek — and with + // it the host's keyframe lane, since diamonds render per row from + // `keyframeCache.get(elementKey)`. The synthetic fractional lanes above sit + // strictly between the host's lane and the next integer, so the children have + // their own rows without the host having to give up its own. return elements .filter((el) => (el.key ?? el.id) === parentKey || !parentMap.has(el.domId ?? el.id)) - .flatMap((el) => ((el.key ?? el.id) === parentKey ? expanded : [el])); + .flatMap((el) => ((el.key ?? el.id) === parentKey ? [el, ...expanded] : [el])); } export function useExpandedTimelineElements(): TimelineElement[] { From acad7b268ea1e9e4b35e15265ac6c8a178e43865 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 19:02:05 +0200 Subject: [PATCH 4/4] fix(studio): keep every host row on the drill path, not just the top Drilling two levels deep spared only the top-level row, so the middle host lost its row and its keyframe lane with it. Spare every host between the drilled one and the top, and anchor the children under the deepest host that actually has a row. Also stops resolveClipTimingBasis handing back a main-timeline start when a clip names a parent composition that is absent from the element list. The mount is unknowable there, so the child's own window is the only safe frame. --- packages/studio/src/hooks/gsapShared.test.ts | 25 +++++++++++ packages/studio/src/hooks/gsapShared.ts | 12 ++++- .../hooks/useExpandedTimelineElements.test.ts | 45 +++++++++++++++++++ .../hooks/useExpandedTimelineElements.ts | 29 ++++++++++-- 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index 98717e1a8..9df0b2f65 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -217,6 +217,31 @@ describe("resolveClipTimingBasis", () => { }); }); + it("treats a child whose parent composition is missing as starting at 0", () => { + // The mount is unknowable, so the only safe frame is the child's own. The + // old `?? 0` handed back `start` unchanged, which is a main-timeline value + // masquerading as a composition-local one and caches negative percentages. + const pill = { + id: "pill", + domId: "pill", + start: 7, + duration: 3, + parentCompositionId: "not-in-elements", + }; + expect(resolveClipTimingBasis("pill", "scene.html", [pill], [])).toEqual({ + elStart: 0, + elDuration: 3, + }); + }); + + it("keeps a main-timeline clip's own start when it names no parent", () => { + const box = { id: "box", domId: "box", start: 4, duration: 2 }; + expect(resolveClipTimingBasis("box", "index.html", [box], [])).toEqual({ + elStart: 4, + elDuration: 2, + }); + }); + it("falls back to a unit window when neither the element nor a host resolves", () => { expect(resolveClipTimingBasis("ghost", "index.html", [], [])).toEqual({ elStart: 0, diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index d80db4ad3..a92b2ae71 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -299,8 +299,16 @@ export function resolveClipTimingBasis( const parent = parentId ? elements.find((el) => el.domId === parentId || el.id === parentId) : undefined; - const mount = direct.expandedParentStart ?? parent?.start ?? 0; - return { elStart: direct.start - mount, elDuration: direct.duration }; + const mount = direct.expandedParentStart ?? parent?.start; + if (mount !== undefined) return { elStart: direct.start - mount, elDuration: direct.duration }; + // No parent composition named, so this IS a main-timeline clip and its own + // start is already the basis. + if (!parentId) return { elStart: direct.start, elDuration: direct.duration }; + // It named a parent we cannot find, so the mount is unknowable. Its tweens + // are still composition-local, so treat its own window as the frame rather + // than subtracting nothing and handing back a main-timeline start, which is + // exactly the mixed-frame subtraction this function exists to prevent. + return { elStart: 0, elDuration: direct.duration }; } const hostId = domClipChildren.find((c) => c.id === elementId)?.hostId; const host = hostId diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts index 204c1509a..f7100997b 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts @@ -143,6 +143,51 @@ describe("buildExpandedElements", () => { expect(child.sourceFile).toBe("c.html"); // C's file, not b.html or a.html }); + it("keeps the middle host's row when drilling two levels deep", () => { + // A embeds B; C lives in B. Drilling into B must leave BOTH host rows + // standing: sparing only the top-level one drops B's row, and its keyframe + // lane goes with it because diamonds render per row. + const elements = [ + el({ id: "A", domId: "A", start: 10, duration: 8, compositionSrc: "a.html" }), + el({ id: "B", domId: "B", start: 12, duration: 4, track: 1, compositionSrc: "b.html" }), + ]; + const manifest = [ + clip({ id: "A", start: 10, duration: 8, compositionSrc: "a.html" }), + clip({ id: "B", start: 12, duration: 4, compositionSrc: "b.html" }), + clip({ id: "C", start: 13, duration: 2 }), + ]; + const parentMap = new Map([ + ["B", "A"], + ["C", "B"], + ]); + + const out = buildExpandedElements(elements, manifest, parentMap, "A", "B"); + const rows = out.map((e) => e.domId ?? e.id); + expect(rows).toContain("B"); + // The child sits under its own host, not under the top-level row. + expect(rows.indexOf("C")).toBeGreaterThan(rows.indexOf("B")); + }); + + it("still drills a host that exists only in the manifest, without a row for it", () => { + // Same shape, but B has no store element, so there is no row to spare. The + // children stay anchored to the top-level row rather than vanishing. + const elements = [ + el({ id: "A", domId: "A", start: 10, duration: 8, compositionSrc: "a.html" }), + ]; + const manifest = [ + clip({ id: "A", start: 10, duration: 8, compositionSrc: "a.html" }), + clip({ id: "B", start: 12, duration: 4, compositionSrc: "b.html" }), + clip({ id: "C", start: 13, duration: 2 }), + ]; + const parentMap = new Map([ + ["B", "A"], + ["C", "B"], + ]); + + const out = buildExpandedElements(elements, manifest, parentMap, "A", "B"); + expect(out.map((e) => e.domId ?? e.id)).toEqual(["A", "C"]); + }); + // Regression: an expanded child must share one identity (`key`) with the flat // store element for the same DOM id. Before the fix the child key fell back to // the colon form (`index.html:eyebrow:N`) while the store/selection used the diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts index 57eae95b6..8a1c95e58 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts @@ -284,16 +284,39 @@ export function buildExpandedElements( ); if (expanded.length === 0) return filterToTopLevel(elements, parentMap); + // Every host between the drilled one and the top level owns a row, so the + // drill has to spare all of them, not just the top. A middle host is still a + // host: dropping its row drops its keyframe lane with it. + const drillPath = new Set(); + for (let cursor: string | undefined = siblingParentId; cursor; ) { + if (drillPath.has(cursor)) break; + drillPath.add(cursor); + if (cursor === topLevelId) break; + cursor = parentMap.get(cursor); + } + // Children hang under the DEEPEST host on that path, so anchor them there + // when it has a row of its own and fall back to the top-level row when it + // does not (a host that lives only in the manifest never had one). + const anchorsChildren = (el: TimelineElement): boolean => + drillPath.has(siblingParentId) && elements.some((e) => (e.domId ?? e.id) === siblingParentId) + ? (el.domId ?? el.id) === siblingParentId + : (el.key ?? el.id) === parentKey; + // ADDITIVE drill-in: the host row stays and its children are appended under // it. Expansion is also triggered by the playhead alone (paused auto-expand), - // so substituting the host row made it vanish on an ordinary seek — and with + // so substituting the host row made it vanish on an ordinary seek, and with // it the host's keyframe lane, since diamonds render per row from // `keyframeCache.get(elementKey)`. The synthetic fractional lanes above sit // strictly between the host's lane and the next integer, so the children have // their own rows without the host having to give up its own. return elements - .filter((el) => (el.key ?? el.id) === parentKey || !parentMap.has(el.domId ?? el.id)) - .flatMap((el) => ((el.key ?? el.id) === parentKey ? [el, ...expanded] : [el])); + .filter( + (el) => + (el.key ?? el.id) === parentKey || + drillPath.has(el.domId ?? el.id) || + !parentMap.has(el.domId ?? el.id), + ) + .flatMap((el) => (anchorsChildren(el) ? [el, ...expanded] : [el])); } export function useExpandedTimelineElements(): TimelineElement[] {