fix(studio): let a hidden sub-composition child be shown again (#3559)

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.
This commit is contained in:
Felipe Caldas
2026-08-31 17:23:47 +00:00
committed by GitHub
parent f84b4c23dc
commit f18964de0c
7 changed files with 280 additions and 32 deletions
@@ -0,0 +1,96 @@
// @vitest-environment happy-dom
import { describe, expect, it } from "vitest";
import {
collectSubCompositionDomChildren,
collectSubCompositionHostState,
} from "./timelineSyncHydration";
import type { ClipManifestClip } from "../lib/playbackTypes";
const clip = (over: Partial<ClipManifestClip>): ClipManifestClip => ({
id: "x",
label: "x",
start: 0,
duration: 1,
track: 0,
kind: "element",
tagName: "div",
compositionId: null,
parentCompositionId: null,
compositionSrc: null,
assetUrl: null,
...over,
});
/**
* The shape a storyboard scene actually publishes: the clip that carries
* `data-hidden` is a `<video>` one level BELOW an id'd region wrapper.
*/
function mountScene(): Document {
document.body.innerHTML = `
<div id="scene-2-slot" data-composition-id="scene-2" data-composition-src="scene-2.html">
<div data-hf-inner-root>
<div id="scene-2-video-region" class="hf-region">
<video id="scene-2-video" class="clip" data-start="0" data-hidden></video>
</div>
<div id="scene-2-title" class="clip" data-start="0" data-hidden></div>
<div id="scene-2-caption" class="clip" data-start="0" data-timeline-locked
data-timeline-role="caption" data-fx-chain="blur" data-automation="opacity"></div>
</div>
</div>`;
return document;
}
// A composition clip is keyed by its ELEMENT id, not its `data-composition-id`
// (the runtime's clip tree publishes `scene-2-slot`), so the collector resolves
// the host with getElementById(clip.id) exactly as the sibling walk does.
const sceneClips = [clip({ id: "scene-2-slot", kind: "composition", compositionId: "scene-2" })];
describe("collectSubCompositionHostState", () => {
it("reaches a clip nested below an id'd wrapper, which the sibling walk cannot", () => {
const doc = mountScene();
// The walk that defines rows stops at the first id'd descendant, so the
// video inside the region wrapper is never recorded there. That is why the
// eye on a hidden scene video had no state to read.
const siblings = collectSubCompositionDomChildren(doc, sceneClips, new Map());
expect(siblings.map((child) => child.id)).toEqual([
"scene-2-video-region",
"scene-2-title",
"scene-2-caption",
]);
const state = collectSubCompositionHostState(doc, sceneClips);
expect(state.get("scene-2-video")?.hidden).toBe(true);
});
it("records every data-* attribute an expanded child row needs", () => {
const state = collectSubCompositionHostState(mountScene(), sceneClips);
expect(state.get("scene-2-title")).toEqual({ hidden: true });
expect(state.get("scene-2-caption")).toEqual({
timelineLocked: true,
timelineRole: "caption",
fxChain: "blur",
automation: "opacity",
});
});
it("omits elements carrying no state, so a visible child reads as visible", () => {
const state = collectSubCompositionHostState(mountScene(), sceneClips);
expect(state.has("scene-2-video-region")).toBe(false);
expect(state.get("scene-2-video")?.hidden).toBe(true);
});
it("returns empty without a document, rather than throwing", () => {
expect(collectSubCompositionHostState(null, sceneClips).size).toBe(0);
});
it("ignores clips that are not compositions", () => {
const doc = mountScene();
const state = collectSubCompositionHostState(doc, [clip({ id: "scene-2-slot" })]);
expect(state.size).toBe(0);
});
});
@@ -10,7 +10,7 @@
*/
import { usePlayerStore } from "../store/playerStore";
import type { TimelineElement, DomClipChild } from "../store/playerStore";
import type { TimelineElement, DomClipChild, SubCompositionHostState } from "../store/playerStore";
import { resolveCssStackingContextId } from "@hyperframes/core/runtime/stacking-context";
import type { ClipTree } from "@hyperframes/core/runtime/clipTree";
import { HF_AUDIO_GROUP_ATTR } from "@hyperframes/core/audio-groups";
@@ -137,6 +137,52 @@ export function collectSubCompositionDomChildren(
return out;
}
/** The host-element `data-*` state one element carries, or null when it has none. */
function readSubCompositionHostState(el: Element): SubCompositionHostState | null {
const state: SubCompositionHostState = {};
if (el.hasAttribute("data-hidden")) state.hidden = true;
if (el.hasAttribute("data-timeline-locked")) state.timelineLocked = true;
const timelineRole = el.getAttribute("data-timeline-role");
if (timelineRole) state.timelineRole = timelineRole;
const fxChain = el.getAttribute("data-fx-chain");
if (fxChain) state.fxChain = fxChain;
const automation = el.getAttribute("data-automation");
if (automation) state.automation = automation;
return Object.keys(state).length > 0 ? state : null;
}
/**
* Host-element state for every id'd element inside every sub-composition.
*
* This walk exists because nothing else in the pipeline can see these
* attributes. A clip whose parent composition is itself in the manifest is
* filtered out before the flat store is built, so an expanded child has no twin
* to inherit from, and the manifest carries timing rather than attributes.
*
* Deliberately separate from {@link collectSubCompositionDomChildren}: that walk
* defines which rows exist and writes `parentMap`, and it stops at the first
* id'd descendant. Scene footage commonly sits one level below an id'd region
* wrapper, so it is never reached there. This one descends the whole subtree and
* touches neither rows nor parentage.
*/
export function collectSubCompositionHostState(
iframeDoc: Document | null,
clips: readonly ClipManifestClip[],
): Map<string, SubCompositionHostState> {
const out = new Map<string, SubCompositionHostState>();
if (!iframeDoc) return out;
for (const clip of clips) {
if (clip.kind !== "composition" || !clip.id) continue;
const hostEl = iframeDoc.getElementById(clip.id);
if (!hostEl) continue;
for (const el of Array.from(hostEl.querySelectorAll("[id]"))) {
const state = readSubCompositionHostState(el);
if (state) out.set(el.id, state);
}
}
return out;
}
/** An iframe's document, or null when reading it throws (cross-origin, or the
* frame is mid-navigation). */
export function safeContentDocument(iframe: HTMLIFrameElement | null): Document | null {
@@ -270,6 +270,49 @@ describe("buildExpandedElements", () => {
expect(child.timelineLocked).toBe(true);
});
// The test above only covers a host with no compositionSrc, where the child's
// key falls back to the `index.html` scope and a flat store twin can exist. A
// REAL sub-composition scopes the child key to the sub-comp file, and clips
// whose parent composition is itself in the manifest are filtered out of the
// flat store before it is built (`processTimelineMessage`). So the twin never
// exists there and the inheritance above is dead code for the one case it was
// written for: the eye reported every hidden child visible, clicking it
// rewrote data-hidden, and nothing could be shown again.
it("reads hidden and locked off the live element when the child has no flat twin", () => {
// Only the host is in the flat store — exactly what the manifest filter leaves.
const elements = [
el({
id: "scene-2",
domId: "scene-2",
start: 3.25,
duration: 3.5,
compositionSrc: "scene-2.html",
}),
];
const manifest = [
clip({ id: "scene-2", start: 3.25, duration: 3.5, compositionSrc: "scene-2.html" }),
clip({ id: "scene-2-video", start: 3.25, duration: 3.5, parentCompositionId: "scene-2" }),
];
const parentMap = new Map([["scene-2-video", "scene-2"]]);
const hostState = new Map([["scene-2-video", { hidden: true, timelineLocked: true }]]);
const out = buildExpandedElements(
elements,
manifest,
parentMap,
"scene-2",
"scene-2",
[],
hostState,
);
const child = out.find((e) => e.domId === "scene-2-video")!;
// The child key is scoped to the sub-comp file, so no store element can match it.
expect(child.key).toBe("scene-2.html#scene-2-video");
expect(elements.some((element) => element.key === child.key)).toBe(false);
expect(child.hidden).toBe(true);
expect(child.timelineLocked).toBe(true);
});
// Sub-comp internals (group + pills) have no data-start, so they're not in the
// manifest. They arrive as DOM children and must still expand under their host.
it("expands DOM-only sub-comp children (no manifest clip) under the host", () => {
@@ -1,5 +1,10 @@
import { useMemo } from "react";
import { usePlayerStore, type TimelineElement, type DomClipChild } from "../store/playerStore";
import {
usePlayerStore,
type TimelineElement,
type DomClipChild,
type SubCompositionHostState,
} from "../store/playerStore";
import type { ClipManifestClip } from "../lib/playbackTypes";
import { createTimelineElementFromManifestClip } from "../lib/timelineDOM";
import { buildTimelineElementKey, splitTimelineElementKey } from "../lib/timelineElementHelpers";
@@ -131,18 +136,6 @@ interface DisplayBounds {
track: number;
}
/**
* State that lives on the live host element, not in the clip manifest:
* `data-hidden`, `data-timeline-locked`, `data-timeline-role`. A child row is
* built from a manifest clip with no hostEl to read, so
* createTimelineElementFromManifestClip cannot see any of it. The flat store
* element for the same child WAS built with one, so it is inherited from there.
*
* Without this the eye on an expanded child always reported the row visible, so
* clicking it wrote data-hidden again instead of removing it, and a hidden child
* could never be shown again (not even after a reload, since the attribute is in
* the source).
*/
/**
* Audio-group membership for an expanded child, from whichever source has it.
*
@@ -169,20 +162,34 @@ function childGroupState(
};
}
function hostElementState(flat: TimelineElement | undefined): Partial<TimelineElement> {
if (!flat) return {};
return {
hidden: flat.hidden,
timelineLocked: flat.timelineLocked,
timelineRole: flat.timelineRole,
// Same reason as the three above: these are read off the host element, which
// an expanded child is built without. Missing them, an audio child inside a
// sub-composition reserved no automation height and drew no lanes, while the
// property panel — reading the live DOM selection rather than this row —
// still showed the chain and its toggles.
fxChain: flat.fxChain,
automation: flat.automation,
};
/**
* State that lives on the live host element, not in the clip manifest:
* `data-hidden`, `data-timeline-locked`, `data-timeline-role`, `data-fx-chain`,
* `data-automation`. A child row is built from a manifest clip with no hostEl to
* read, so createTimelineElementFromManifestClip cannot see any of it.
*
* `live` is the reading taken off the element itself and is authoritative. For a
* child of a REAL sub-composition it is also the only source: such a clip is
* filtered out of the manifest before the flat store is built, so no twin exists
* to inherit from. The flat twin still covers the phantom-wrapper case, where
* the child does keep a store entry of its own.
*
* Without a reading of the element, the eye on an expanded child always reported
* the row visible, so clicking it wrote data-hidden again instead of removing
* it, and a hidden child could never be shown again (not even after a reload,
* since the attribute is in the source). Missing fxChain and automation, an
* audio child inside a sub-composition reserved no automation height and drew no
* lanes, while the property panel, which reads the live DOM selection rather
* than this row, still showed the chain and its toggles.
*/
function hostElementState(
flat: TimelineElement | undefined,
live: SubCompositionHostState | undefined,
): Partial<TimelineElement> {
if (!flat) return { ...live };
const { hidden, timelineLocked, timelineRole, fxChain, automation } = flat;
// `live` last: it is the reading off the element, so it wins wherever it has one.
return { hidden, timelineLocked, timelineRole, fxChain, automation, ...live };
}
// `display` bounds come from the top-level scene clip (where the expanded row is
@@ -196,6 +203,7 @@ function buildChildElements(
expandedHostKey: string,
elements: readonly TimelineElement[],
domChildrenById: ReadonlyMap<string, DomClipChild>,
hostStateById: ReadonlyMap<string, SubCompositionHostState>,
): TimelineElement[] {
const result: TimelineElement[] = [];
for (const child of siblings) {
@@ -223,7 +231,10 @@ function buildChildElements(
});
result.push({
...base,
...hostElementState(elements.find((element) => element.key === key)),
...hostElementState(
elements.find((element) => element.key === key),
domId ? hostStateById.get(domId) : undefined,
),
...childGroupState(
elements.find((element) => element.key === key),
domId ? domChildrenById.get(domId) : undefined,
@@ -309,6 +320,7 @@ export function buildExpandedElements(
topLevelId: string,
siblingParentId: string,
domClipChildren: DomClipChild[] = [],
subCompositionHostState: ReadonlyMap<string, SubCompositionHostState> = new Map(),
): TimelineElement[] {
const topLevelElement = elements.find((el) => el.id === topLevelId || el.domId === topLevelId);
if (!topLevelElement) return filterToTopLevel(elements, parentMap);
@@ -348,6 +360,7 @@ export function buildExpandedElements(
parentKey,
elements,
domChildrenById,
subCompositionHostState,
);
if (expanded.length === 0) return filterToTopLevel(elements, parentMap);
@@ -391,6 +404,7 @@ export function useExpandedTimelineElements(): TimelineElement[] {
const clipManifest = usePlayerStore((s) => s.clipManifest);
const clipParentMap = usePlayerStore((s) => s.clipParentMap);
const domClipChildren = usePlayerStore((s) => s.domClipChildren);
const subCompositionHostState = usePlayerStore((s) => s.subCompositionHostState);
const selectedElementId = usePlayerStore((s) => s.selectedElementId);
const currentTime = usePlayerStore((s) => s.currentTime);
@@ -432,6 +446,15 @@ export function useExpandedTimelineElements(): TimelineElement[] {
topLevel,
immediateParent,
domClipChildren,
subCompositionHostState,
);
}, [elements, clipManifest, clipParentMap, domClipChildren, rawId, selectedRawId]);
}, [
elements,
clipManifest,
clipParentMap,
domClipChildren,
subCompositionHostState,
rawId,
selectedRawId,
]);
}
@@ -19,6 +19,7 @@ import {
buildTimelineElementsFromClips,
clipTreeParentMap,
collectSubCompositionDomChildren,
collectSubCompositionHostState,
hydrateTimelineFromPreview,
isPreviewReadinessMessage,
safeContentDocument,
@@ -131,6 +132,9 @@ export function useTimelineSyncCallbacks({
const domClipChildren = collectSubCompositionDomChildren(iframeDoc, data.clips, parentMap);
usePlayerStore.getState().setClipParentMap(parentMap);
usePlayerStore.getState().setDomClipChildren(domClipChildren);
usePlayerStore
.getState()
.setSubCompositionHostState(collectSubCompositionHostState(iframeDoc, data.clips));
} catch {
// cross-origin or __clipTree not available — maps stay empty
}
@@ -23,9 +23,13 @@ import { createThumbnailSlice, type ThumbnailSlice } from "./thumbnailSlice";
export type { KeyframeCacheEntry } from "./keyframeSlice";
export { liveTime } from "./liveTime";
import type { TimelineElement, TimelineElementPatch } from "./timelineElement";
import type {
TimelineElement,
TimelineElementPatch,
SubCompositionHostState,
} from "./timelineElement";
export type { TimelineElement };
export type { TimelineElement, SubCompositionHostState };
export type ZoomMode = "fit" | "manual";
type TimelineTool = "select" | "razor";
@@ -212,6 +216,14 @@ interface PlayerState
*/
domClipChildren: DomClipChild[];
setDomClipChildren: (children: DomClipChild[]) => void;
/**
* Host-element state for every id'd element inside a sub-composition, keyed by
* dom id. Collected from the live preview because it is the only place that
* sees it: these elements are filtered out of `elements` before the flat store
* is built, and the clip manifest carries timing, not attributes.
*/
subCompositionHostState: Map<string, SubCompositionHostState>;
setSubCompositionHostState: (state: Map<string, SubCompositionHostState>) => void;
}
/** A sub-comp DOM-only timeline child (no data-start) and its nesting context. */
@@ -284,6 +296,7 @@ export function createTimelineResetState() {
clipManifest: null,
clipParentMap: new Map<string, string>(),
domClipChildren: [],
subCompositionHostState: new Map<string, SubCompositionHostState>(),
};
}
@@ -430,6 +443,8 @@ export const usePlayerStore = create<PlayerState>((set, get) => ({
setClipParentMap: (map) => set({ clipParentMap: map }),
domClipChildren: [],
setDomClipChildren: (children) => set({ domClipChildren: children }),
subCompositionHostState: new Map(),
setSubCompositionHostState: (state) => set({ subCompositionHostState: state }),
setIsPlaying: (playing) => {
if (get().isPlaying === playing) return;
@@ -118,3 +118,24 @@ export type TimelineElementPatch = Partial<
| "audioGroupAutomation"
>
>;
/**
* The `data-*` state an expanded sub-composition child needs but cannot reach.
*
* A child row is synthesized from a manifest clip with no element to read, and
* for a real sub-composition it has no flat store twin either: such clips are
* dropped before the flat store is built. Read off the live preview instead
* (`collectSubCompositionHostState`) and carried on the store by dom id.
*
* Without it the eye reported every hidden child visible, so clicking it wrote
* `data-hidden` a second time instead of removing it, and the element could
* never be shown again, not even after a reload, since the attribute is in the
* source.
*/
export interface SubCompositionHostState {
hidden?: boolean;
timelineLocked?: boolean;
timelineRole?: string;
fxChain?: string;
automation?: string;
}