mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-10 22:20:14 +00:00
fix(studio,core): groups open by default, headers fit, and two contracts stop being promises
The four items left after the browser pass, plus the two architectural findings from the review that were held for a decision. Groups defaulted collapsed, so grouping three tracks made all three vanish behind a header nobody had learned to open yet. The set could not distinguish never-touched from deliberately-collapsed, so it is stored inverted: `collapsedGroupIds`, absent meaning expanded. Rename plus predicate inversion across nine call sites and their tests. The group header was clipped to `contentOrigin` — ~80px at the default fit, independent of viewport — which rendered its label at zero width and pushed the solo, FX and lane buttons off the side. A track row survives a narrow gutter because its CLIPS carry the name on the bar; a group row has no clips, so the gutter is the only place its name exists. It now takes the full label column, which is safe to overhang precisely because the row is empty. Measured 80 -> 232, label 0 -> 45px. Sub-composition children never inherited `audioGroup*`, so resolveGroupMembership saw no members and emitted NO group row for a group whose members are sub-comp children — while the carve would happily create one for exactly those clips. Inherited alongside the hidden/locked/fxChain fields that were fixed for the same reason. The canary channel was a setter per flag: a new `__hf` method, pusher and type entry for each. Replaced with one `__hf.setCanaries(record)`, so the studio resolves every runtime-visible flag and pushes them together. Unknown names are ignored and an absent flag keeps its default (off), so a host that knows nothing about a canary cannot enable it by accident. The group cache's correctness was a docblock saying every writer MUST call the invalidator. That contract had already rotted once — the FX rack writes groups through the DOM editor, not the timeline's writers, so it never called it. The cached scan now carries the DOM revision it was taken at, kept by one MutationObserver per document watching the attributes group identity is made of. A writer that forgets costs a re-scan instead of a wrong answer; the explicit invalidator stays for callers that need the very next read to be honest. Verified in the browser: group expanded on load with no seeding, header 232px with the label and all four controls visible, `setCanaries` present on the runtime and the per-flag setter gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ff4965037c
commit
25d7af8a5c
@@ -158,6 +158,44 @@ describe("group info cache", () => {
|
||||
invalidateGroupInfoCache(doc);
|
||||
expect(parseMember(doc).audioGroupHidden).toBe(false);
|
||||
});
|
||||
|
||||
// The explicit invalidator is a convenience, not the contract. A cache whose
|
||||
// only defence is "every writer must remember to call this" rots the first
|
||||
// time a writer does not know it exists — which is precisely what happened
|
||||
// with the FX rack, whose group writes go through the DOM editor rather than
|
||||
// the timeline's own writers. The scan carries the DOM revision it was taken
|
||||
// at, so a forgotten call costs a re-scan rather than a wrong answer.
|
||||
it("expires itself on a group edit nobody announced", async () => {
|
||||
const doc = makeDoc(`
|
||||
<div data-composition-id="root">
|
||||
<audio id="voice-1" data-start="0" data-duration="5" data-audio-group="voiceover"></audio>
|
||||
<hf-audio-group id="voiceover" data-label="Voices"></hf-audio-group>
|
||||
</div>
|
||||
`);
|
||||
|
||||
expect(parseMember(doc).audioGroupHidden).toBe(false);
|
||||
|
||||
// No invalidateGroupInfoCache call anywhere in this test.
|
||||
doc.getElementById("voiceover")?.setAttribute("data-hidden", "");
|
||||
await new Promise((resolve) => setTimeout(resolve, 0)); // observer microtask
|
||||
|
||||
expect(parseMember(doc).audioGroupHidden).toBe(true);
|
||||
});
|
||||
|
||||
it("notices a member joining the group, not just an attribute edit", async () => {
|
||||
const doc = makeDoc(`
|
||||
<div data-composition-id="root">
|
||||
<audio id="voice-1" data-start="0" data-duration="5" data-audio-group="voiceover"></audio>
|
||||
<hf-audio-group id="voiceover" data-label="Voices"></hf-audio-group>
|
||||
</div>
|
||||
`);
|
||||
expect(parseMember(doc).audioGroupLabel).toBe("Voices");
|
||||
|
||||
doc.getElementById("voiceover")?.setAttribute("data-label", "Narration");
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(parseMember(doc).audioGroupLabel).toBe("Narration");
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseTimelineFromDOM — canonical playback rate", () => {
|
||||
|
||||
@@ -12,7 +12,8 @@ import type { TimelineElement } from "../store/playerStore";
|
||||
import type { ClipManifestClip } from "./playbackTypes";
|
||||
import { resolveCssStackingContextId } from "@hyperframes/core/runtime/stacking-context";
|
||||
import { readClipTiming } from "@hyperframes/core/composition-contract";
|
||||
import { resolveAudioGroups } from "@hyperframes/core/audio-groups";
|
||||
import { HF_AUDIO_GROUP_ATTR, resolveAudioGroups } from "@hyperframes/core/audio-groups";
|
||||
import { HF_AUDIO_FX_ATTR } from "@hyperframes/core/audio-fx";
|
||||
import {
|
||||
resolveMediaElement,
|
||||
applyMediaMetadataFromElement,
|
||||
@@ -77,17 +78,62 @@ interface GroupInfo {
|
||||
fxChain?: string;
|
||||
}
|
||||
|
||||
const groupInfoCache = new WeakMap<Document, Map<string, GroupInfo>>();
|
||||
/**
|
||||
* The cached scan, plus the DOM revision it was taken at.
|
||||
*
|
||||
* Keeping the revision beside the entry is what makes staleness *detectable*
|
||||
* rather than a documented obligation. The cache is keyed on the document, and
|
||||
* group edits are applied as live patches precisely so the iframe never
|
||||
* reloads, so the key alone never changes: an explicit "remember to invalidate"
|
||||
* contract silently rots the first time a new writer forgets — which is exactly
|
||||
* what happened with the FX rack, whose group writes go through the DOM editor
|
||||
* rather than the timeline's own writers.
|
||||
*/
|
||||
const groupInfoCache = new WeakMap<
|
||||
Document,
|
||||
{ revision: number; entries: Map<string, GroupInfo> }
|
||||
>();
|
||||
|
||||
/** Bumped by every observed mutation to group state in a document. */
|
||||
const groupRevisions = new WeakMap<Document, number>();
|
||||
const groupObservers = new WeakSet<Document>();
|
||||
|
||||
/**
|
||||
* Watch a document for any change to group state, so the cache expires itself.
|
||||
*
|
||||
* One observer per document, attached the first time a group is read from it.
|
||||
* It watches the attributes a group's identity is made of, anywhere in the
|
||||
* tree, plus added/removed nodes — which covers a group element appearing, a
|
||||
* member joining or leaving, and any group attribute being edited, by any
|
||||
* writer, without that writer having to know this cache exists.
|
||||
*/
|
||||
function observeGroupState(doc: Document): void {
|
||||
if (groupObservers.has(doc) || typeof MutationObserver === "undefined" || !doc.body) return;
|
||||
groupObservers.add(doc);
|
||||
const observer = new MutationObserver(() => {
|
||||
groupRevisions.set(doc, (groupRevisions.get(doc) ?? 0) + 1);
|
||||
});
|
||||
observer.observe(doc.body, {
|
||||
subtree: true,
|
||||
childList: true,
|
||||
attributes: true,
|
||||
attributeFilter: [
|
||||
HF_AUDIO_GROUP_ATTR,
|
||||
HF_AUDIO_FX_ATTR,
|
||||
"data-label",
|
||||
"data-volume",
|
||||
"data-hidden",
|
||||
"id",
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop the cached group scan for a document.
|
||||
*
|
||||
* MUST be called by every live write to group state. The cache is keyed on the
|
||||
* document, and group edits are applied as live patches precisely so the iframe
|
||||
* never reloads — so the key never changes and the entry would otherwise live
|
||||
* forever. Left stale, a muted group could never be unmuted (the header keeps
|
||||
* reading `hidden:false` and re-writes `data-hidden`), the bus slider snapped
|
||||
* back, and a second FX preset built on a stale chain, discarding the first.
|
||||
* Belt to the observer's braces: a caller that has just written and wants the
|
||||
* very next read to be honest cannot wait for the observer's microtask. Callers
|
||||
* that forget are no longer punished — the revision check catches them.
|
||||
*/
|
||||
export function invalidateGroupInfoCache(doc: Document | null | undefined): void {
|
||||
if (doc) groupInfoCache.delete(doc);
|
||||
@@ -95,7 +141,10 @@ export function invalidateGroupInfoCache(doc: Document | null | undefined): void
|
||||
|
||||
function groupInfoFor(doc: Document | null | undefined, groupId: string): GroupInfo {
|
||||
if (!doc) return { label: groupId, volume: 1, hidden: false };
|
||||
let info = groupInfoCache.get(doc);
|
||||
observeGroupState(doc);
|
||||
const revision = groupRevisions.get(doc) ?? 0;
|
||||
const cached = groupInfoCache.get(doc);
|
||||
let info = cached && cached.revision === revision ? cached.entries : undefined;
|
||||
if (!info) {
|
||||
info = new Map(
|
||||
resolveAudioGroups(doc).map((group) => [
|
||||
@@ -108,7 +157,7 @@ function groupInfoFor(doc: Document | null | undefined, groupId: string): GroupI
|
||||
},
|
||||
]),
|
||||
);
|
||||
groupInfoCache.set(doc, info);
|
||||
groupInfoCache.set(doc, { revision, entries: info });
|
||||
}
|
||||
return info.get(groupId) ?? { label: groupId, volume: 1, hidden: false };
|
||||
}
|
||||
|
||||
@@ -97,8 +97,8 @@ describe("applyPreviewAudioFlags", () => {
|
||||
setAudioSolo: (ids: readonly string[]) => {
|
||||
calls.solo = [...ids];
|
||||
},
|
||||
setAudioMuteHidden: (enabled: boolean) => {
|
||||
calls.muteHidden = [enabled];
|
||||
setCanaries: (states: Record<string, boolean>) => {
|
||||
calls.canaries = [states];
|
||||
},
|
||||
},
|
||||
};
|
||||
@@ -119,7 +119,8 @@ describe("applyPreviewAudioFlags", () => {
|
||||
applyPreviewAudioFlags(iframe, false, 1, new Set(["voice-1"]));
|
||||
|
||||
expect(calls.solo).toEqual(["voice-1"]);
|
||||
expect(calls.muteHidden).toEqual([false]);
|
||||
// Every runtime-visible flag in one push, each resolved by the host.
|
||||
expect(calls.canaries?.[0]).toMatchObject({ "audio-track-mute": expect.any(Boolean) });
|
||||
});
|
||||
|
||||
it("pushes an empty solo set rather than skipping the call", () => {
|
||||
|
||||
@@ -143,17 +143,27 @@ export function setPreviewMediaVolume(iframe: HTMLIFrameElement | null, volume:
|
||||
} catch {}
|
||||
}
|
||||
|
||||
/** Push the `audio-track-mute` canary state into the preview runtime, which
|
||||
* defaults it off (see `window.__hf.setAudioMuteHidden`). Direct call, not a
|
||||
* control message: it is a runtime flag, not a transport command, and the
|
||||
* player host has no equivalent property to set. */
|
||||
function setPreviewMuteHidden(iframe: HTMLIFrameElement | null, enabled: boolean): void {
|
||||
/**
|
||||
* Every canary the preview runtime can act on, resolved here and pushed as one
|
||||
* record (see `window.__hf.setCanaries`). Core has no install id, so it cannot
|
||||
* bucket for itself; a flag missing from this list simply stays off in the
|
||||
* runtime, which is the shipped behaviour.
|
||||
*
|
||||
* Adding a runtime-visible canary means adding its name here and reading it in
|
||||
* core — no new `__hf` method, pusher or type entry per flag.
|
||||
*/
|
||||
const RUNTIME_CANARIES = ["audio-track-mute", "audio-groups", "audio-fx-rack"] as const;
|
||||
|
||||
function setPreviewCanaries(iframe: HTMLIFrameElement | null): void {
|
||||
if (!iframe) return;
|
||||
try {
|
||||
const win = iframe.contentWindow as
|
||||
| (Window & { __hf?: { setAudioMuteHidden?: (enabled: boolean) => void } })
|
||||
| (Window & { __hf?: { setCanaries?: (states: Record<string, boolean>) => void } })
|
||||
| null;
|
||||
win?.__hf?.setAudioMuteHidden?.(enabled);
|
||||
if (!win?.__hf?.setCanaries) return;
|
||||
const states: Record<string, boolean> = {};
|
||||
for (const name of RUNTIME_CANARIES) states[name] = isCanaryEnabled(name);
|
||||
win.__hf.setCanaries(states);
|
||||
} catch {}
|
||||
}
|
||||
|
||||
@@ -188,7 +198,7 @@ export function applyPreviewAudioFlags(
|
||||
// Volume too: the transport comes back at unity after a reload, so a preview
|
||||
// the author had turned down came back loud.
|
||||
setPreviewMediaVolume(iframe, volume);
|
||||
setPreviewMuteHidden(iframe, isCanaryEnabled("audio-track-mute"));
|
||||
setPreviewCanaries(iframe);
|
||||
setPreviewSolo(iframe, [...soloed]);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user