Skip to content

Commit 1ddf01a

Browse files
committed
fix(studio): let a hidden sub-composition child be shown again
The eye on an expanded sub-composition child always rendered as "Hide", whatever the source said. One click hid the element and every click after that rewrote the same attribute, so the row could never be shown again, not even after a reload, since data-hidden is in the file. buildChildElements synthesizes a child row from a manifest clip with no element to read, and compensated by inheriting hidden/timelineLocked/ timelineRole/fxChain/automation from the child's flat store twin. That twin does not exist for a real sub-composition: processTimelineMessage drops any clip whose parent composition is itself in the manifest before building the flat store, so the lookup always missed and the inheritance was dead code for the one case it was written for. It worked only for a phantom-wrapper parent, where the child does keep a store entry. Read the state off the live element instead. collectSubCompositionHostState walks each sub-composition host in the preview document and records the data-* state of every id'd descendant, keyed by dom id. The existing sibling walk cannot serve this: it defines which rows exist and writes parentMap, and it stops at the first id'd descendant, so scene footage sitting one level below an id'd region wrapper is never reached. The new walk descends the whole subtree and touches neither rows nor parentage. Reproduced on a 9-scene storyboard project where every scene is a sub-composition. Before: a scene video and title carrying data-hidden both announced "Hide track N", and clicking left the file byte-identical. After: both announce "Show track N", and hide/show round-trips the attribute. A top-level clip with the same attribute always announced "Show", which is what made the gap specific to expanded child rows. The existing regression test passed throughout because its fixture hands the child a flat twin with hidden: true and gives the host no compositionSrc, so the child key falls back to the index.html scope and a twin can exist. The added test models a real sub-composition instead.
1 parent f84b4c2 commit 1ddf01a

7 files changed

Lines changed: 280 additions & 32 deletions
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// @vitest-environment happy-dom
2+
3+
import { describe, expect, it } from "vitest";
4+
import {
5+
collectSubCompositionDomChildren,
6+
collectSubCompositionHostState,
7+
} from "./timelineSyncHydration";
8+
import type { ClipManifestClip } from "../lib/playbackTypes";
9+
10+
const clip = (over: Partial<ClipManifestClip>): ClipManifestClip => ({
11+
id: "x",
12+
label: "x",
13+
start: 0,
14+
duration: 1,
15+
track: 0,
16+
kind: "element",
17+
tagName: "div",
18+
compositionId: null,
19+
parentCompositionId: null,
20+
compositionSrc: null,
21+
assetUrl: null,
22+
...over,
23+
});
24+
25+
/**
26+
* The shape a storyboard scene actually publishes: the clip that carries
27+
* `data-hidden` is a `<video>` one level BELOW an id'd region wrapper.
28+
*/
29+
function mountScene(): Document {
30+
document.body.innerHTML = `
31+
<div id="scene-2-slot" data-composition-id="scene-2" data-composition-src="scene-2.html">
32+
<div data-hf-inner-root>
33+
<div id="scene-2-video-region" class="hf-region">
34+
<video id="scene-2-video" class="clip" data-start="0" data-hidden></video>
35+
</div>
36+
<div id="scene-2-title" class="clip" data-start="0" data-hidden></div>
37+
<div id="scene-2-caption" class="clip" data-start="0" data-timeline-locked
38+
data-timeline-role="caption" data-fx-chain="blur" data-automation="opacity"></div>
39+
</div>
40+
</div>`;
41+
return document;
42+
}
43+
44+
// A composition clip is keyed by its ELEMENT id, not its `data-composition-id`
45+
// (the runtime's clip tree publishes `scene-2-slot`), so the collector resolves
46+
// the host with getElementById(clip.id) exactly as the sibling walk does.
47+
const sceneClips = [clip({ id: "scene-2-slot", kind: "composition", compositionId: "scene-2" })];
48+
49+
describe("collectSubCompositionHostState", () => {
50+
it("reaches a clip nested below an id'd wrapper, which the sibling walk cannot", () => {
51+
const doc = mountScene();
52+
53+
// The walk that defines rows stops at the first id'd descendant, so the
54+
// video inside the region wrapper is never recorded there. That is why the
55+
// eye on a hidden scene video had no state to read.
56+
const siblings = collectSubCompositionDomChildren(doc, sceneClips, new Map());
57+
expect(siblings.map((child) => child.id)).toEqual([
58+
"scene-2-video-region",
59+
"scene-2-title",
60+
"scene-2-caption",
61+
]);
62+
63+
const state = collectSubCompositionHostState(doc, sceneClips);
64+
expect(state.get("scene-2-video")?.hidden).toBe(true);
65+
});
66+
67+
it("records every data-* attribute an expanded child row needs", () => {
68+
const state = collectSubCompositionHostState(mountScene(), sceneClips);
69+
70+
expect(state.get("scene-2-title")).toEqual({ hidden: true });
71+
expect(state.get("scene-2-caption")).toEqual({
72+
timelineLocked: true,
73+
timelineRole: "caption",
74+
fxChain: "blur",
75+
automation: "opacity",
76+
});
77+
});
78+
79+
it("omits elements carrying no state, so a visible child reads as visible", () => {
80+
const state = collectSubCompositionHostState(mountScene(), sceneClips);
81+
82+
expect(state.has("scene-2-video-region")).toBe(false);
83+
expect(state.get("scene-2-video")?.hidden).toBe(true);
84+
});
85+
86+
it("returns empty without a document, rather than throwing", () => {
87+
expect(collectSubCompositionHostState(null, sceneClips).size).toBe(0);
88+
});
89+
90+
it("ignores clips that are not compositions", () => {
91+
const doc = mountScene();
92+
const state = collectSubCompositionHostState(doc, [clip({ id: "scene-2-slot" })]);
93+
94+
expect(state.size).toBe(0);
95+
});
96+
});

packages/studio/src/player/hooks/timelineSyncHydration.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*/
1111

1212
import { usePlayerStore } from "../store/playerStore";
13-
import type { TimelineElement, DomClipChild } from "../store/playerStore";
13+
import type { TimelineElement, DomClipChild, SubCompositionHostState } from "../store/playerStore";
1414
import { resolveCssStackingContextId } from "@hyperframes/core/runtime/stacking-context";
1515
import type { ClipTree } from "@hyperframes/core/runtime/clipTree";
1616
import { HF_AUDIO_GROUP_ATTR } from "@hyperframes/core/audio-groups";
@@ -137,6 +137,52 @@ export function collectSubCompositionDomChildren(
137137
return out;
138138
}
139139

140+
/** The host-element `data-*` state one element carries, or null when it has none. */
141+
function readSubCompositionHostState(el: Element): SubCompositionHostState | null {
142+
const state: SubCompositionHostState = {};
143+
if (el.hasAttribute("data-hidden")) state.hidden = true;
144+
if (el.hasAttribute("data-timeline-locked")) state.timelineLocked = true;
145+
const timelineRole = el.getAttribute("data-timeline-role");
146+
if (timelineRole) state.timelineRole = timelineRole;
147+
const fxChain = el.getAttribute("data-fx-chain");
148+
if (fxChain) state.fxChain = fxChain;
149+
const automation = el.getAttribute("data-automation");
150+
if (automation) state.automation = automation;
151+
return Object.keys(state).length > 0 ? state : null;
152+
}
153+
154+
/**
155+
* Host-element state for every id'd element inside every sub-composition.
156+
*
157+
* This walk exists because nothing else in the pipeline can see these
158+
* attributes. A clip whose parent composition is itself in the manifest is
159+
* filtered out before the flat store is built, so an expanded child has no twin
160+
* to inherit from, and the manifest carries timing rather than attributes.
161+
*
162+
* Deliberately separate from {@link collectSubCompositionDomChildren}: that walk
163+
* defines which rows exist and writes `parentMap`, and it stops at the first
164+
* id'd descendant. Scene footage commonly sits one level below an id'd region
165+
* wrapper, so it is never reached there. This one descends the whole subtree and
166+
* touches neither rows nor parentage.
167+
*/
168+
export function collectSubCompositionHostState(
169+
iframeDoc: Document | null,
170+
clips: readonly ClipManifestClip[],
171+
): Map<string, SubCompositionHostState> {
172+
const out = new Map<string, SubCompositionHostState>();
173+
if (!iframeDoc) return out;
174+
for (const clip of clips) {
175+
if (clip.kind !== "composition" || !clip.id) continue;
176+
const hostEl = iframeDoc.getElementById(clip.id);
177+
if (!hostEl) continue;
178+
for (const el of Array.from(hostEl.querySelectorAll("[id]"))) {
179+
const state = readSubCompositionHostState(el);
180+
if (state) out.set(el.id, state);
181+
}
182+
}
183+
return out;
184+
}
185+
140186
/** An iframe's document, or null when reading it throws (cross-origin, or the
141187
* frame is mid-navigation). */
142188
export function safeContentDocument(iframe: HTMLIFrameElement | null): Document | null {

packages/studio/src/player/hooks/useExpandedTimelineElements.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,49 @@ describe("buildExpandedElements", () => {
270270
expect(child.timelineLocked).toBe(true);
271271
});
272272

273+
// The test above only covers a host with no compositionSrc, where the child's
274+
// key falls back to the `index.html` scope and a flat store twin can exist. A
275+
// REAL sub-composition scopes the child key to the sub-comp file, and clips
276+
// whose parent composition is itself in the manifest are filtered out of the
277+
// flat store before it is built (`processTimelineMessage`). So the twin never
278+
// exists there and the inheritance above is dead code for the one case it was
279+
// written for: the eye reported every hidden child visible, clicking it
280+
// rewrote data-hidden, and nothing could be shown again.
281+
it("reads hidden and locked off the live element when the child has no flat twin", () => {
282+
// Only the host is in the flat store — exactly what the manifest filter leaves.
283+
const elements = [
284+
el({
285+
id: "scene-2",
286+
domId: "scene-2",
287+
start: 3.25,
288+
duration: 3.5,
289+
compositionSrc: "scene-2.html",
290+
}),
291+
];
292+
const manifest = [
293+
clip({ id: "scene-2", start: 3.25, duration: 3.5, compositionSrc: "scene-2.html" }),
294+
clip({ id: "scene-2-video", start: 3.25, duration: 3.5, parentCompositionId: "scene-2" }),
295+
];
296+
const parentMap = new Map([["scene-2-video", "scene-2"]]);
297+
const hostState = new Map([["scene-2-video", { hidden: true, timelineLocked: true }]]);
298+
299+
const out = buildExpandedElements(
300+
elements,
301+
manifest,
302+
parentMap,
303+
"scene-2",
304+
"scene-2",
305+
[],
306+
hostState,
307+
);
308+
const child = out.find((e) => e.domId === "scene-2-video")!;
309+
// The child key is scoped to the sub-comp file, so no store element can match it.
310+
expect(child.key).toBe("scene-2.html#scene-2-video");
311+
expect(elements.some((element) => element.key === child.key)).toBe(false);
312+
expect(child.hidden).toBe(true);
313+
expect(child.timelineLocked).toBe(true);
314+
});
315+
273316
// Sub-comp internals (group + pills) have no data-start, so they're not in the
274317
// manifest. They arrive as DOM children and must still expand under their host.
275318
it("expands DOM-only sub-comp children (no manifest clip) under the host", () => {

packages/studio/src/player/hooks/useExpandedTimelineElements.ts

Lines changed: 52 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { useMemo } from "react";
2-
import { usePlayerStore, type TimelineElement, type DomClipChild } from "../store/playerStore";
2+
import {
3+
usePlayerStore,
4+
type TimelineElement,
5+
type DomClipChild,
6+
type SubCompositionHostState,
7+
} from "../store/playerStore";
38
import type { ClipManifestClip } from "../lib/playbackTypes";
49
import { createTimelineElementFromManifestClip } from "../lib/timelineDOM";
510
import { buildTimelineElementKey, splitTimelineElementKey } from "../lib/timelineElementHelpers";
@@ -131,18 +136,6 @@ interface DisplayBounds {
131136
track: number;
132137
}
133138

134-
/**
135-
* State that lives on the live host element, not in the clip manifest:
136-
* `data-hidden`, `data-timeline-locked`, `data-timeline-role`. A child row is
137-
* built from a manifest clip with no hostEl to read, so
138-
* createTimelineElementFromManifestClip cannot see any of it. The flat store
139-
* element for the same child WAS built with one, so it is inherited from there.
140-
*
141-
* Without this the eye on an expanded child always reported the row visible, so
142-
* clicking it wrote data-hidden again instead of removing it, and a hidden child
143-
* could never be shown again (not even after a reload, since the attribute is in
144-
* the source).
145-
*/
146139
/**
147140
* Audio-group membership for an expanded child, from whichever source has it.
148141
*
@@ -169,20 +162,34 @@ function childGroupState(
169162
};
170163
}
171164

172-
function hostElementState(flat: TimelineElement | undefined): Partial<TimelineElement> {
173-
if (!flat) return {};
174-
return {
175-
hidden: flat.hidden,
176-
timelineLocked: flat.timelineLocked,
177-
timelineRole: flat.timelineRole,
178-
// Same reason as the three above: these are read off the host element, which
179-
// an expanded child is built without. Missing them, an audio child inside a
180-
// sub-composition reserved no automation height and drew no lanes, while the
181-
// property panel — reading the live DOM selection rather than this row —
182-
// still showed the chain and its toggles.
183-
fxChain: flat.fxChain,
184-
automation: flat.automation,
185-
};
165+
/**
166+
* State that lives on the live host element, not in the clip manifest:
167+
* `data-hidden`, `data-timeline-locked`, `data-timeline-role`, `data-fx-chain`,
168+
* `data-automation`. A child row is built from a manifest clip with no hostEl to
169+
* read, so createTimelineElementFromManifestClip cannot see any of it.
170+
*
171+
* `live` is the reading taken off the element itself and is authoritative. For a
172+
* child of a REAL sub-composition it is also the only source: such a clip is
173+
* filtered out of the manifest before the flat store is built, so no twin exists
174+
* to inherit from. The flat twin still covers the phantom-wrapper case, where
175+
* the child does keep a store entry of its own.
176+
*
177+
* Without a reading of the element, the eye on an expanded child always reported
178+
* the row visible, so clicking it wrote data-hidden again instead of removing
179+
* it, and a hidden child could never be shown again (not even after a reload,
180+
* since the attribute is in the source). Missing fxChain and automation, an
181+
* audio child inside a sub-composition reserved no automation height and drew no
182+
* lanes, while the property panel, which reads the live DOM selection rather
183+
* than this row, still showed the chain and its toggles.
184+
*/
185+
function hostElementState(
186+
flat: TimelineElement | undefined,
187+
live: SubCompositionHostState | undefined,
188+
): Partial<TimelineElement> {
189+
if (!flat) return { ...live };
190+
const { hidden, timelineLocked, timelineRole, fxChain, automation } = flat;
191+
// `live` last: it is the reading off the element, so it wins wherever it has one.
192+
return { hidden, timelineLocked, timelineRole, fxChain, automation, ...live };
186193
}
187194

188195
// `display` bounds come from the top-level scene clip (where the expanded row is
@@ -196,6 +203,7 @@ function buildChildElements(
196203
expandedHostKey: string,
197204
elements: readonly TimelineElement[],
198205
domChildrenById: ReadonlyMap<string, DomClipChild>,
206+
hostStateById: ReadonlyMap<string, SubCompositionHostState>,
199207
): TimelineElement[] {
200208
const result: TimelineElement[] = [];
201209
for (const child of siblings) {
@@ -223,7 +231,10 @@ function buildChildElements(
223231
});
224232
result.push({
225233
...base,
226-
...hostElementState(elements.find((element) => element.key === key)),
234+
...hostElementState(
235+
elements.find((element) => element.key === key),
236+
domId ? hostStateById.get(domId) : undefined,
237+
),
227238
...childGroupState(
228239
elements.find((element) => element.key === key),
229240
domId ? domChildrenById.get(domId) : undefined,
@@ -309,6 +320,7 @@ export function buildExpandedElements(
309320
topLevelId: string,
310321
siblingParentId: string,
311322
domClipChildren: DomClipChild[] = [],
323+
subCompositionHostState: ReadonlyMap<string, SubCompositionHostState> = new Map(),
312324
): TimelineElement[] {
313325
const topLevelElement = elements.find((el) => el.id === topLevelId || el.domId === topLevelId);
314326
if (!topLevelElement) return filterToTopLevel(elements, parentMap);
@@ -348,6 +360,7 @@ export function buildExpandedElements(
348360
parentKey,
349361
elements,
350362
domChildrenById,
363+
subCompositionHostState,
351364
);
352365
if (expanded.length === 0) return filterToTopLevel(elements, parentMap);
353366

@@ -391,6 +404,7 @@ export function useExpandedTimelineElements(): TimelineElement[] {
391404
const clipManifest = usePlayerStore((s) => s.clipManifest);
392405
const clipParentMap = usePlayerStore((s) => s.clipParentMap);
393406
const domClipChildren = usePlayerStore((s) => s.domClipChildren);
407+
const subCompositionHostState = usePlayerStore((s) => s.subCompositionHostState);
394408
const selectedElementId = usePlayerStore((s) => s.selectedElementId);
395409
const currentTime = usePlayerStore((s) => s.currentTime);
396410

@@ -432,6 +446,15 @@ export function useExpandedTimelineElements(): TimelineElement[] {
432446
topLevel,
433447
immediateParent,
434448
domClipChildren,
449+
subCompositionHostState,
435450
);
436-
}, [elements, clipManifest, clipParentMap, domClipChildren, rawId, selectedRawId]);
451+
}, [
452+
elements,
453+
clipManifest,
454+
clipParentMap,
455+
domClipChildren,
456+
subCompositionHostState,
457+
rawId,
458+
selectedRawId,
459+
]);
437460
}

packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
buildTimelineElementsFromClips,
2020
clipTreeParentMap,
2121
collectSubCompositionDomChildren,
22+
collectSubCompositionHostState,
2223
hydrateTimelineFromPreview,
2324
isPreviewReadinessMessage,
2425
safeContentDocument,
@@ -131,6 +132,9 @@ export function useTimelineSyncCallbacks({
131132
const domClipChildren = collectSubCompositionDomChildren(iframeDoc, data.clips, parentMap);
132133
usePlayerStore.getState().setClipParentMap(parentMap);
133134
usePlayerStore.getState().setDomClipChildren(domClipChildren);
135+
usePlayerStore
136+
.getState()
137+
.setSubCompositionHostState(collectSubCompositionHostState(iframeDoc, data.clips));
134138
} catch {
135139
// cross-origin or __clipTree not available — maps stay empty
136140
}

0 commit comments

Comments
 (0)