From 25d7af8a5cdd4bdfb8c878678be788de99cb7806 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 18 Aug 2026 10:52:48 -0700 Subject: [PATCH] fix(studio,core): groups open by default, headers fit, and two contracts stop being promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/core/src/runtime/init.test.ts | 2 +- packages/core/src/runtime/init.ts | 34 +++++---- packages/core/src/runtime/media.ts | 2 +- packages/core/src/runtime/window.d.ts | 14 ++-- .../src/player/components/Timeline.test.ts | 1 - .../player/components/TimelineGroupRow.tsx | 17 +++-- .../player/components/TimelineLanes.test.tsx | 2 +- .../src/player/components/TimelineLanes.tsx | 5 +- .../timelineKeyboardNavigation.test.ts | 4 +- .../components/timelineKeyboardNavigation.ts | 8 +-- .../components/useTimelineLaneRowIndexes.ts | 2 +- .../components/useTimelineLogicalFocus.ts | 4 +- .../useTimelineLogicalRows.test.tsx | 4 +- .../components/useTimelineLogicalRows.ts | 6 +- .../components/useTimelineTrackDerivations.ts | 10 +-- .../components/useTimelineTrackLayout.test.ts | 21 ++++-- .../hooks/useExpandedTimelineElements.ts | 10 +++ .../studio/src/player/lib/timelineDOM.test.ts | 38 ++++++++++ packages/studio/src/player/lib/timelineDOM.ts | 69 ++++++++++++++++--- .../player/lib/timelineIframeHelpers.test.ts | 7 +- .../src/player/lib/timelineIframeHelpers.ts | 26 ++++--- .../studio/src/player/store/keyframeSlice.ts | 18 +++-- .../studio/src/player/store/playerStore.ts | 2 +- 23 files changed, 224 insertions(+), 82 deletions(-) diff --git a/packages/core/src/runtime/init.test.ts b/packages/core/src/runtime/init.test.ts index 3746d6b1a..8a9effa13 100644 --- a/packages/core/src/runtime/init.test.ts +++ b/packages/core/src/runtime/init.test.ts @@ -1352,7 +1352,7 @@ describe("initSandboxRuntimeModular", () => { // Behind the `audio-track-mute` canary — off until the host pushes it, so a // composition that already carries data-hidden on an audio element keeps // playing in preview for anyone not enrolled. - window.__hf?.setAudioMuteHidden?.(true); + window.__hf?.setCanaries?.({ "audio-track-mute": true }); const decodeSpy = vi .spyOn(WebAudioTransport.prototype, "decodeAudioElement") diff --git a/packages/core/src/runtime/init.ts b/packages/core/src/runtime/init.ts index 0f03fd83d..0d184a46a 100644 --- a/packages/core/src/runtime/init.ts +++ b/packages/core/src/runtime/init.ts @@ -192,18 +192,22 @@ export function initSandboxRuntimeModular(): void { soloedIds = new Set(ids); webAudio.setSolo(soloedIds); }; - // A2's preview/export parity fix — silencing `data-hidden` audio the way the - // render already does — behind the `audio-track-mute` canary, which is what - // that canary was declared for. Core cannot read the registry (canaries are - // resolved from the studio's install id), so the host pushes the resolved - // state on the same channel as solo. Default OFF = the shipped behaviour: a - // composition carrying `data-hidden` on an audio element keeps playing in - // preview until its author is enrolled. Non-studio hosts (CLI preview, the - // bare player) never push, so they stay on the old behaviour too. - let silenceHiddenAudio = false; - window.__hf.setAudioMuteHidden = (enabled) => { - if (silenceHiddenAudio === enabled) return; - silenceHiddenAudio = enabled; + // Canary states the HOST resolved, keyed by registry name. Core cannot + // resolve one itself — bucketing needs an install id it has no access to — + // so every runtime-visible flag arrives through this one channel rather than + // growing an `__hf` setter of its own. + // + // Every flag defaults OFF, which is the shipped behaviour: a host that never + // pushes (CLI preview, the bare player) behaves exactly as before. + const canaries: Record = {}; + // A2's preview/export parity fix: silence `data-hidden` audio the way the + // render already does. Off until enrolled, so a composition carrying + // `data-hidden` on an audio element keeps playing in preview meanwhile. + const silenceHiddenAudioEnabled = (): boolean => canaries["audio-track-mute"] === true; + window.__hf.setCanaries = (states) => { + const wasSilencing = silenceHiddenAudioEnabled(); + for (const [name, enabled] of Object.entries(states)) canaries[name] = enabled === true; + if (silenceHiddenAudioEnabled() === wasSilencing) return; // The active-clip set is built with this predicate baked in, so a flip // mid-session has to rebuild it. `stopAll()` first: bumping the generation // only rejects future STALE schedules, it does not stop sources already @@ -2103,7 +2107,7 @@ export function initSandboxRuntimeModular(): void { isWebAudioOwned: (el) => webAudio.ownsElement(el), isWebAudioRouted: (el) => webAudio.routesElement(el), isAudibleUnderSolo: (el) => isAudibleUnderSolo(soloedIds, el.id, audioGroupOf(el)), - silenceHiddenAudio, + silenceHiddenAudio: silenceHiddenAudioEnabled(), onAutoplayBlocked: () => { if (state.mediaAutoplayBlockedPosted) return; state.mediaAutoplayBlockedPosted = true; @@ -3006,7 +3010,7 @@ export function initSandboxRuntimeModular(): void { let foundActive = false; for (const rawEl of audioEls) { if (!(rawEl instanceof HTMLMediaElement) || !rawEl.isConnected) continue; - if (silenceHiddenAudio && rawEl.closest("[data-hidden]")) continue; + if (silenceHiddenAudioEnabled() && rawEl.closest("[data-hidden]")) continue; const start = Number.parseFloat(rawEl.dataset.start ?? ""); const durAttr = parseStrictFiniteTimingNumber(rawEl.dataset.duration); const end = durAttr != null && durAttr > 0 ? start + durAttr : Infinity; @@ -3114,7 +3118,7 @@ export function initSandboxRuntimeModular(): void { const audioEls = document.querySelectorAll("audio[data-start]"); for (const rawEl of audioEls) { if (!(rawEl instanceof HTMLMediaElement) || !rawEl.isConnected) continue; - if (silenceHiddenAudio && rawEl.closest("[data-hidden]")) continue; + if (silenceHiddenAudioEnabled() && rawEl.closest("[data-hidden]")) continue; const compStart = Number.parseFloat(rawEl.dataset.start ?? ""); if (!Number.isFinite(compStart)) continue; const mediaStart = readElementPlaybackStart(rawEl); diff --git a/packages/core/src/runtime/media.ts b/packages/core/src/runtime/media.ts index 4e44a1dbc..a3bafc64e 100644 --- a/packages/core/src/runtime/media.ts +++ b/packages/core/src/runtime/media.ts @@ -223,7 +223,7 @@ export function syncRuntimeMedia(params: { * isn't wired up at all, which reads as "always audible". */ isAudibleUnderSolo?: (el: HTMLMediaElement) => boolean; /** Silence media under a `data-hidden` ancestor, matching the render. Opt-in: - * the host pushes it via `__hf.setAudioMuteHidden` when the `audio-track-mute` + * the host pushes it via `__hf.setCanaries` when the `audio-track-mute` * canary is on. Absent/false = the shipped behaviour (hidden audio still * plays in preview). */ silenceHiddenAudio?: boolean; diff --git a/packages/core/src/runtime/window.d.ts b/packages/core/src/runtime/window.d.ts index 706c3fa0c..135f50600 100644 --- a/packages/core/src/runtime/window.d.ts +++ b/packages/core/src/runtime/window.d.ts @@ -44,11 +44,17 @@ declare global { */ setAudioSolo?: (ids: readonly string[]) => void; /** - * Studio's `audio-track-mute` canary state: silence audio under a - * `data-hidden` ancestor in preview, the way the render already does. - * Off until pushed — core cannot resolve a canary itself. + * Canary states resolved by the HOST and pushed in, because core cannot + * resolve one itself: bucketing needs an install id, which lives in the + * studio's localStorage or the CLI's seed. + * + * One channel for every flag rather than a setter each — a per-flag + * setter meant a new `__hf` method, a new pusher and a new type entry + * for every runtime-visible canary. Unknown names are ignored, and any + * flag absent from the record keeps its default (off), so a host that + * knows nothing about a given canary cannot silently enable it. */ - setAudioMuteHidden?: (enabled: boolean) => void; + setCanaries?: (states: Readonly>) => void; }; __playerReady?: boolean; __renderReady?: boolean; diff --git a/packages/studio/src/player/components/Timeline.test.ts b/packages/studio/src/player/components/Timeline.test.ts index 0ea738788..46df3319e 100644 --- a/packages/studio/src/player/components/Timeline.test.ts +++ b/packages/studio/src/player/components/Timeline.test.ts @@ -335,7 +335,6 @@ describe("Timeline provider boundary", () => { audioGroup: "voiceover", }, ], - expandedGroupIds: new Set(["voiceover"]), }); const root = createRoot(host); act(() => root.render(React.createElement(Timeline))); diff --git a/packages/studio/src/player/components/TimelineGroupRow.tsx b/packages/studio/src/player/components/TimelineGroupRow.tsx index 411e844e7..8c462d43f 100644 --- a/packages/studio/src/player/components/TimelineGroupRow.tsx +++ b/packages/studio/src/player/components/TimelineGroupRow.tsx @@ -25,10 +25,9 @@ interface TimelineGroupRowProps { top: number; height: number; virtualized: boolean; - contentOrigin: number; theme: TimelineTheme; rovingTargetId?: string | null; - expandedGroupIds: ReadonlySet; + collapsedGroupIds: ReadonlySet; expandedLaneOwnerIds: ReadonlySet; toggleGroupExpanded: (id: string) => void; toggleLaneOwnerExpanded: (id: string) => void; @@ -43,10 +42,9 @@ export function TimelineGroupRow({ top, height, virtualized, - contentOrigin, theme, rovingTargetId = null, - expandedGroupIds, + collapsedGroupIds, expandedLaneOwnerIds, toggleGroupExpanded, toggleLaneOwnerExpanded, @@ -106,7 +104,7 @@ export function TimelineGroupRow({ toggleGroupExpanded(group.id)} laneCount={groupAutomationLanes(memberElements).length} isLaneOpen={isLaneOpen} @@ -127,7 +125,14 @@ export function TimelineGroupRow({ onFxChainChange={(next) => writeGroupFxChain(next, false)} onFxChainPreview={(next) => writeGroupFxChain(next, true)} onOpenFxRack={openGroupFxRack} - columnWidth={contentOrigin >= LABEL_COL_W ? LABEL_COL_W : contentOrigin} + // Always the full label column, never squeezed down to `contentOrigin`. + // A track row can afford a narrow gutter because its CLIPS carry the + // name on the bar; a group row has no clips at all, so the gutter is + // the only place its name exists — and at the default fit the gutter is + // ~80px, which rendered the label at zero width and clipped the solo, + // FX and lane buttons off the side. Overhanging into the lane area is + // safe precisely because this row is empty (see `propertyRows={[]}`). + columnWidth={LABEL_COL_W} theme={theme} /> {isLaneOpen && ( diff --git a/packages/studio/src/player/components/TimelineLanes.test.tsx b/packages/studio/src/player/components/TimelineLanes.test.tsx index 86b61ae6e..70a07b531 100644 --- a/packages/studio/src/player/components/TimelineLanes.test.tsx +++ b/packages/studio/src/player/components/TimelineLanes.test.tsx @@ -118,7 +118,7 @@ function renderLanes(options: RenderLanesOptions = {}): { selectedElementId: null, selectedElementIds: next.selectedElementIds ?? new Set(), expandedClipIds: new Set(next.expandedClipIds ?? []), - expandedGroupIds: new Set(), + collapsedGroupIds: new Set(), expandedLaneOwnerIds: new Set(), groups: [], trackGroupOf: new Map(), diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 0760f0a9c..60041fb42 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -100,7 +100,7 @@ export function TimelineLanes({ // from resolving into a second timeline that renders the same logical rows. const lanesIdPrefix = `timeline-lanes${useId().replaceAll(":", "")}`; const expandedClipIds = usePlayerStore((s) => s.expandedClipIds); - const { expandedGroupIds, expandedLaneOwnerIds, toggleGroupExpanded, toggleLaneOwnerExpanded } = + const { collapsedGroupIds, expandedLaneOwnerIds, toggleGroupExpanded, toggleLaneOwnerExpanded } = useTimelineGroupDisclosure(); const automationLanes = useAutomationLanes(); useAutomationSelectionKeyboard({ lanes: automationLanes }); @@ -166,10 +166,9 @@ export function TimelineLanes({ top={rowGeometry.getRowTop(row)} height={rowGeometry.getRowHeight(row)} virtualized={rowsVirtualized} - contentOrigin={contentOrigin} theme={theme} rovingTargetId={keyboard.rovingTargetId} - expandedGroupIds={expandedGroupIds} + collapsedGroupIds={collapsedGroupIds} expandedLaneOwnerIds={expandedLaneOwnerIds} toggleGroupExpanded={toggleGroupExpanded} toggleLaneOwnerExpanded={toggleLaneOwnerExpanded} diff --git a/packages/studio/src/player/components/timelineKeyboardNavigation.test.ts b/packages/studio/src/player/components/timelineKeyboardNavigation.test.ts index 988c5a47d..853bb553b 100644 --- a/packages/studio/src/player/components/timelineKeyboardNavigation.test.ts +++ b/packages/studio/src/player/components/timelineKeyboardNavigation.test.ts @@ -58,7 +58,7 @@ function model(overrides: Partial[0] selectedElementId: "active", selectedElementIds: new Set(), expandedClipIds: new Set(["active"]), - expandedGroupIds: new Set(), + collapsedGroupIds: new Set(), expandedLaneOwnerIds: new Set(), groups: [], trackGroupOf: new Map(), @@ -248,7 +248,7 @@ describe("resolveTimelineNavigationTarget", () => { selectedElementId: null, selectedElementIds: new Set(), expandedClipIds: new Set(), - expandedGroupIds: new Set(), + collapsedGroupIds: new Set(), expandedLaneOwnerIds: new Set(), groups: [], trackGroupOf: new Map(), diff --git a/packages/studio/src/player/components/timelineKeyboardNavigation.ts b/packages/studio/src/player/components/timelineKeyboardNavigation.ts index a5b44d34c..351bfb564 100644 --- a/packages/studio/src/player/components/timelineKeyboardNavigation.ts +++ b/packages/studio/src/player/components/timelineKeyboardNavigation.ts @@ -77,8 +77,8 @@ export interface BuildTimelineLogicalRowsInput { selectedElementId: string | null; selectedElementIds: ReadonlySet; expandedClipIds: ReadonlySet; - /** Groups whose member rows the caret has shown (structural, not lanes). */ - expandedGroupIds: ReadonlySet; + /** Groups the caret has COLLAPSED — absent means expanded, the default. */ + collapsedGroupIds: ReadonlySet; /** Rows (clip id or group id) whose automation-lane rows the `∿` button opened. */ expandedLaneOwnerIds: ReadonlySet; groups: readonly TimelineTrackGroupInfo[]; @@ -251,7 +251,7 @@ export function buildTimelineLogicalRows({ selectedElementId, selectedElementIds, expandedClipIds, - expandedGroupIds, + collapsedGroupIds, expandedLaneOwnerIds, groups, trackGroupOf, @@ -300,7 +300,7 @@ export function buildTimelineLogicalRows({ // count. function emitGroup(group: TimelineTrackGroupInfo): void { const groupRowId = timelineGroupRowId(group.id); - const groupExpanded = expandedGroupIds.has(group.id); + const groupExpanded = !collapsedGroupIds.has(group.id); rows.push({ id: groupRowId, kind: "row", diff --git a/packages/studio/src/player/components/useTimelineLaneRowIndexes.ts b/packages/studio/src/player/components/useTimelineLaneRowIndexes.ts index 3a5fa8375..7ef978c5c 100644 --- a/packages/studio/src/player/components/useTimelineLaneRowIndexes.ts +++ b/packages/studio/src/player/components/useTimelineLaneRowIndexes.ts @@ -6,7 +6,7 @@ import type { TimelineTrackGroupInfo } from "./useTimelineTrackDerivations"; /** The four pieces of group-disclosure state a group row's header reads and writes. */ export function useTimelineGroupDisclosure() { return { - expandedGroupIds: usePlayerStore((s) => s.expandedGroupIds), + collapsedGroupIds: usePlayerStore((s) => s.collapsedGroupIds), expandedLaneOwnerIds: usePlayerStore((s) => s.expandedLaneOwnerIds), toggleGroupExpanded: usePlayerStore((s) => s.toggleGroupExpanded), toggleLaneOwnerExpanded: usePlayerStore((s) => s.toggleLaneOwnerExpanded), diff --git a/packages/studio/src/player/components/useTimelineLogicalFocus.ts b/packages/studio/src/player/components/useTimelineLogicalFocus.ts index ecc8bc8ce..8f6fe0e09 100644 --- a/packages/studio/src/player/components/useTimelineLogicalFocus.ts +++ b/packages/studio/src/player/components/useTimelineLogicalFocus.ts @@ -35,7 +35,7 @@ interface TimelineLogicalFocusInput { export function useTimelineLogicalFocus(input: TimelineLogicalFocusInput) { const expandedClipIds = usePlayerStore((state) => state.expandedClipIds); - const expandedGroupIds = usePlayerStore((state) => state.expandedGroupIds); + const collapsedGroupIds = usePlayerStore((state) => state.collapsedGroupIds); const expandedLaneOwnerIds = usePlayerStore((state) => state.expandedLaneOwnerIds); const projectId = usePlayerStore((state) => state.timelineProjectId); const logicalRows = useTimelineLogicalRows({ @@ -45,7 +45,7 @@ export function useTimelineLogicalFocus(input: TimelineLogicalFocusInput) { selectedElementId: input.selectedElementId, selectedElementIds: input.selectedElementIds, expandedClipIds, - expandedGroupIds, + collapsedGroupIds, expandedLaneOwnerIds, groups: input.groups, trackGroupOf: input.trackGroupOf, diff --git a/packages/studio/src/player/components/useTimelineLogicalRows.test.tsx b/packages/studio/src/player/components/useTimelineLogicalRows.test.tsx index 25e171e5e..1ba44d32e 100644 --- a/packages/studio/src/player/components/useTimelineLogicalRows.test.tsx +++ b/packages/studio/src/player/components/useTimelineLogicalRows.test.tsx @@ -22,7 +22,7 @@ const displayTrackOrder = tracks.map(([track]) => track); const laneCounts = new Map(); const selectedElementIds = new Set(); const expandedClipIds = new Set(); -const expandedGroupIds = new Set(); +const collapsedGroupIds = new Set(); const expandedLaneOwnerIds = new Set(); const groups: never[] = []; const trackGroupOf = new Map(); @@ -37,7 +37,7 @@ function Harness({ snapshots }: { snapshots: Array, + collapsedGroupIds: ReadonlySet, ): { tracks: [number, TimelineElement[]][]; groups: TimelineTrackGroupInfo[]; @@ -154,7 +154,7 @@ function groupTimelineTracks( emitted.add(groupId); const info = buildGroupInfo(groupId, trackNum, membership, rawByTrack); groups.push(info); - emitGroupRows(info, rawByTrack, trackGroupOf, tracks, expandedGroupIds.has(groupId)); + emitGroupRows(info, rawByTrack, trackGroupOf, tracks, !collapsedGroupIds.has(groupId)); } return { tracks, groups, trackGroupOf }; } @@ -183,7 +183,7 @@ export function useTimelineTrackDerivations(expandedElements: TimelineElement[]) return Array.from(map.entries()).sort(([a], [b]) => a - b); }, [expandedElements]); - const expandedGroupIds = usePlayerStore((s) => s.expandedGroupIds); + const collapsedGroupIds = usePlayerStore((s) => s.collapsedGroupIds); const { tracks, groups, trackGroupOf } = useMemo(() => { if (!isCanaryEnabled("audio-groups")) { return { @@ -192,8 +192,8 @@ export function useTimelineTrackDerivations(expandedElements: TimelineElement[]) trackGroupOf: new Map(), }; } - return groupTimelineTracks(rawTracks, expandedGroupIds); - }, [rawTracks, expandedGroupIds]); + return groupTimelineTracks(rawTracks, collapsedGroupIds); + }, [rawTracks, collapsedGroupIds]); const trackStyles = useMemo(() => { const map = new Map(); diff --git a/packages/studio/src/player/components/useTimelineTrackLayout.test.ts b/packages/studio/src/player/components/useTimelineTrackLayout.test.ts index fe175c787..3a072e3d2 100644 --- a/packages/studio/src/player/components/useTimelineTrackLayout.test.ts +++ b/packages/studio/src/player/components/useTimelineTrackLayout.test.ts @@ -55,11 +55,13 @@ describe("collapsed audio groups", () => { audioGroup: "voiceover", }); - function renderGrouped(): { + /** `collapsed` seeds the collapsed set — expanded is the default state. */ + function renderGrouped(collapsed = false): { layout: ReturnType; unmount: () => void; } { enabledCanaries.add("audio-groups"); + if (collapsed) usePlayerStore.setState({ collapsedGroupIds: new Set(["voiceover"]) }); const elements = [member("voice-1", 0), member("voice-2", 1)]; let layout: ReturnType | undefined; function Probe() { @@ -77,7 +79,7 @@ describe("collapsed audio groups", () => { // still reserve height, turning that null into visible dead space — the row // list and the logical rows have to agree. it("emits only the anchor row while the group is collapsed", () => { - const { layout, unmount } = renderGrouped(); + const { layout, unmount } = renderGrouped(true); expect(layout.groups).toHaveLength(1); expect(layout.groups[0]!.memberTracks).toEqual([0, 1]); // The anchor (0 - 0.5) and nothing else. @@ -94,14 +96,25 @@ describe("collapsed audio groups", () => { // display list — which a collapsed group does not appear in. Collapsed is the // default, so that was every group until someone opened it. it("carries its member elements even while collapsed", () => { - const { layout, unmount } = renderGrouped(); + const { layout, unmount } = renderGrouped(true); expect(layout.trackOrder).toEqual([-0.5]); // collapsed: no member rows expect(layout.groups[0]!.memberElements.map((el) => el.id)).toEqual(["voice-1", "voice-2"]); unmount(); }); + // The reason the set is stored inverted. As an expanded-set, "absent" could + // not tell never-touched from deliberately-collapsed, so a freshly created + // group started collapsed — grouping three tracks made all three vanish + // behind a header the user had not yet learned to open. + it("is expanded by default, with nothing seeded", () => { + const { layout, unmount } = renderGrouped(); + expect(usePlayerStore.getState().collapsedGroupIds.size).toBe(0); + expect(layout.trackOrder).toEqual([-0.5, 0, 1]); + unmount(); + }); + it("emits the member rows once the group is expanded", () => { - usePlayerStore.setState({ expandedGroupIds: new Set(["voiceover"]) }); + // Expanded is the default now — nothing to seed. const { layout, unmount } = renderGrouped(); expect(layout.trackOrder).toEqual([-0.5, 0, 1]); for (const track of layout.groups[0]!.memberTracks) { diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts index 64b903db3..b5561fcb4 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts @@ -159,6 +159,16 @@ function hostElementState(flat: TimelineElement | undefined): Partial { 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(` +
+ + +
+ `); + + 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(` +
+ + +
+ `); + 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", () => { diff --git a/packages/studio/src/player/lib/timelineDOM.ts b/packages/studio/src/player/lib/timelineDOM.ts index 2c49c49b3..bb4018b28 100644 --- a/packages/studio/src/player/lib/timelineDOM.ts +++ b/packages/studio/src/player/lib/timelineDOM.ts @@ -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>(); +/** + * 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 } +>(); + +/** Bumped by every observed mutation to group state in a document. */ +const groupRevisions = new WeakMap(); +const groupObservers = new WeakSet(); + +/** + * 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 }; } diff --git a/packages/studio/src/player/lib/timelineIframeHelpers.test.ts b/packages/studio/src/player/lib/timelineIframeHelpers.test.ts index 4a14e13d9..3a9b1abd1 100644 --- a/packages/studio/src/player/lib/timelineIframeHelpers.test.ts +++ b/packages/studio/src/player/lib/timelineIframeHelpers.test.ts @@ -97,8 +97,8 @@ describe("applyPreviewAudioFlags", () => { setAudioSolo: (ids: readonly string[]) => { calls.solo = [...ids]; }, - setAudioMuteHidden: (enabled: boolean) => { - calls.muteHidden = [enabled]; + setCanaries: (states: Record) => { + 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", () => { diff --git a/packages/studio/src/player/lib/timelineIframeHelpers.ts b/packages/studio/src/player/lib/timelineIframeHelpers.ts index 9331adedf..4b56990ba 100644 --- a/packages/studio/src/player/lib/timelineIframeHelpers.ts +++ b/packages/studio/src/player/lib/timelineIframeHelpers.ts @@ -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) => void } }) | null; - win?.__hf?.setAudioMuteHidden?.(enabled); + if (!win?.__hf?.setCanaries) return; + const states: Record = {}; + 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]); } diff --git a/packages/studio/src/player/store/keyframeSlice.ts b/packages/studio/src/player/store/keyframeSlice.ts index 0e8a5b786..d18bd31b4 100644 --- a/packages/studio/src/player/store/keyframeSlice.ts +++ b/packages/studio/src/player/store/keyframeSlice.ts @@ -63,8 +63,16 @@ export interface KeyframeSlice { /** Union-expand clips (keyframed clips are expanded by default on load). */ expandClips: (ids: readonly string[]) => void; - /** Groups whose member rows the caret has shown (structural, not lanes). */ - expandedGroupIds: Set; + /** + * Groups whose member rows the caret has HIDDEN (structural, not lanes). + * + * Inverted deliberately. As an expanded-set, "not in the set" could not tell + * never-touched from deliberately-collapsed, so every group defaulted to + * collapsed — and since nothing seeds the set on create, grouping three + * tracks made all three vanish behind a header the user had not yet learned + * to open. Groups are expanded until someone closes one. + */ + collapsedGroupIds: Set; toggleGroupExpanded: (id: string) => void; /** Rows (clip id or group id) whose automation-lane rows the `∿` button opened. */ @@ -127,13 +135,13 @@ export function createKeyframeSlice( return { expandedClipIds: next }; }), - expandedGroupIds: new Set(), + collapsedGroupIds: new Set(), toggleGroupExpanded: (id) => set((state) => { - const next = new Set(state.expandedGroupIds); + const next = new Set(state.collapsedGroupIds); if (next.has(id)) next.delete(id); else next.add(id); - return { expandedGroupIds: next }; + return { collapsedGroupIds: next }; }), expandedLaneOwnerIds: new Set(), diff --git a/packages/studio/src/player/store/playerStore.ts b/packages/studio/src/player/store/playerStore.ts index 389a0ca5a..2e2bad769 100644 --- a/packages/studio/src/player/store/playerStore.ts +++ b/packages/studio/src/player/store/playerStore.ts @@ -266,7 +266,7 @@ export function createTimelineResetState() { expandedClipIds: new Set(), // Per-composition: ids from comp A match nothing in B, silencing all of it. soloed: new Set(), - expandedGroupIds: new Set(), + collapsedGroupIds: new Set(), expandedLaneOwnerIds: new Set(), focusedEaseSegment: null, selectedElementIds: new Set(),