diff --git a/packages/studio/src/hooks/timelineAudioGroupVolume.test.ts b/packages/studio/src/hooks/timelineAudioGroupVolume.test.ts index ede1506bd..fd190adc7 100644 --- a/packages/studio/src/hooks/timelineAudioGroupVolume.test.ts +++ b/packages/studio/src/hooks/timelineAudioGroupVolume.test.ts @@ -138,6 +138,35 @@ describe("group attribute writes reach the store", () => { * index.html", because the runtime inlines a sub-comp as its own root element * that carries only the composition ID — the FILE is on the host above it. */ +describe("the mirror reaches sub-composition members", () => { + /** + * A group declared inside a sub-composition has no FLAT member to mirror onto + * — `childGroupState` keeps those members out of `elements` — so mirroring + * only `elements` made this whole function a no-op for it: the header kept the + * pre-write chain and `laneCount` stayed 0, so the lane disclosure never + * appeared for automation that now existed. + */ + it("mirrors a group write onto DomClipChild members as well as flat ones", async () => { + const react = await import("react"); + const { renderToStaticMarkup } = await import("react-dom/server"); + const harness = makeSetter(); + renderToStaticMarkup(react.createElement(harness.Probe)); + const setter = harness.get(); + usePlayerStore.getState().setElements([member("flat-1", 0)]); + usePlayerStore.getState().setDomClipChildren([ + { id: "sub-1", parentId: "host", hostId: "host", label: "Sub 1", audioGroup: "voiceover" }, + { id: "other", parentId: "host", hostId: "host", label: "Other", audioGroup: "sfx" }, + ]); + + setter?.setLive("voiceover", "data-label", "Voices"); + + const children = usePlayerStore.getState().domClipChildren; + expect(children.find((c) => c.id === "sub-1")?.audioGroupLabel).toBe("Voices"); + // A member of another group is untouched. + expect(children.find((c) => c.id === "other")?.audioGroupLabel).toBeUndefined(); + }); +}); + describe("resolveGroupSourceFile", () => { function livePreviewShape(): Document { const doc = document.implementation.createHTMLDocument("preview"); diff --git a/packages/studio/src/hooks/timelineAudioGroupVolume.ts b/packages/studio/src/hooks/timelineAudioGroupVolume.ts index 21355e5ca..4138369d3 100644 --- a/packages/studio/src/hooks/timelineAudioGroupVolume.ts +++ b/packages/studio/src/hooks/timelineAudioGroupVolume.ts @@ -79,9 +79,28 @@ function syncStoredGroupAttribute(groupId: string, attr: string, value: string | // a 500-clip composition was 1500 object spreads and 3 store notifications per // drag frame — at ~60/s, with every `elements`-keyed memo downstream // recomputing each time. - usePlayerStore.setState((state) => ({ - elements: state.elements.map((el) => (el.audioGroup === groupId ? { ...el, ...patch } : el)), - })); + // BOTH stores, because a group declared inside a sub-composition has no flat + // member to mirror onto: `childGroupState` keeps those members out of + // `elements` entirely, so their `audioGroup*` fields come from the + // `DomClipChild` record instead. Mirroring only `elements` made this whole + // function a no-op for such a group — the header kept the pre-write chain, its + // FX button showed the old count, and `laneCount` stayed 0 so the lane + // disclosure never appeared for automation that now existed. Verbatim the + // symptom this docblock claims to have fixed, fixed only for flat members. + // + // `setDomClipChildren` has one other writer, inside `processTimelineMessage`, + // which a live attribute patch does not trigger. + usePlayerStore.setState((state) => { + const next: Partial = { + elements: state.elements.map((el) => (el.audioGroup === groupId ? { ...el, ...patch } : el)), + }; + if (state.domClipChildren.some((child) => child.audioGroup === groupId)) { + next.domClipChildren = state.domClipChildren.map((child) => + child.audioGroup === groupId ? { ...child, ...patch } : child, + ); + } + return next; + }); } /** @@ -172,8 +191,6 @@ async function setAudioGroupAttribute({ domEditSaveTimestampRef, pendingTimelineEditPathRef, patchLive: (v) => patchLiveGroupAttribute(previewIframe, groupId, attr, v), - readLive: () => - previewIframe?.contentDocument?.getElementById(groupId)?.getAttribute(attr) ?? null, }); } diff --git a/packages/studio/src/hooks/timelineEditingHelpers.test.ts b/packages/studio/src/hooks/timelineEditingHelpers.test.ts index c923188c4..ca0ea7fea 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.test.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.test.ts @@ -455,7 +455,6 @@ describe("persistElementAttribute", () => { domEditSaveTimestampRef: { current: 0 }, pendingTimelineEditPathRef: { current: new Set() }, patchLive, - readLive: () => null, }), ).rejects.toThrow("Unable to patch element in index.html"); @@ -464,3 +463,49 @@ describe("persistElementAttribute", () => { fetchSpy.mockRestore(); }); }); + +describe("persistElementAttribute — unwind value", () => { + /** + * The unwind has to restore the value on DISK, not the one in the preview. + * + * Every live-write caller patches the DOM before committing (a fader drag is + * `setLive` per frame; hovering a preset auditions the whole chain), so by + * commit time the live DOM already holds the in-progress value. Reading it as + * `previousValue` made the unwind a no-op, and the group writer's catch — + * which deliberately re-mirrors the store off the live DOM — then mirrored the + * never-saved value: the panel agreed with the preview, and a reload dropped it. + */ + it("restores the file's value, not the audition already in the live DOM", async () => { + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue( + new Response( + JSON.stringify({ content: `` }), + ), + ); + const patched: Array = []; + const writeProjectFile = vi.fn(() => Promise.reject(new Error("save failed"))); + + await expect( + persistElementAttribute({ + projectId: "p", + targetPath: "index.html", + patchTarget: { id: "bgm" }, + attr: "data-volume", + value: "0.9", + label: "Set volume", + writeProjectFile, + recordEdit: vi.fn(), + domEditSaveTimestampRef: { current: 0 }, + pendingTimelineEditPathRef: { current: new Set() }, + // The live DOM is ALREADY at the new value when the commit runs — that + // is what `setLive` does on every drag frame. + patchLive: (v) => patched.push(v), + }), + ).rejects.toThrow("save failed"); + + // First the optimistic write, then the unwind — back to what the FILE said. + expect(patched).toEqual(["0.9", "0.25"]); + fetchSpy.mockRestore(); + }); +}); diff --git a/packages/studio/src/hooks/timelineEditingHelpers.ts b/packages/studio/src/hooks/timelineEditingHelpers.ts index 11f335155..7a9477875 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.ts @@ -410,8 +410,6 @@ export interface PersistElementAttributeInput { pendingTimelineEditPathRef: { current: Set }; /** Write the attribute directly on the live preview DOM node. */ patchLive: (value: string | null) => void; - /** Read the attribute's current value off the live preview DOM node. */ - readLive: () => string | null; } /** @@ -420,7 +418,7 @@ export interface PersistElementAttributeInput { * `setAudioGroupAttribute` (a group id addressed by its own DOM id) and * `useSetElementAttribute` (an arbitrary timeline clip) — same shape, only * how the live node is found and where the patch target resolves to differs, - * which is exactly what `patchLive`/`readLive`/`patchTarget` parameterize. + * which is exactly what `patchLive`/`patchTarget` parameterize. */ export async function persistElementAttribute({ projectId, @@ -434,10 +432,7 @@ export async function persistElementAttribute({ domEditSaveTimestampRef, pendingTimelineEditPathRef, patchLive, - readLive, }: PersistElementAttributeInput): Promise { - const previousValue = readLive(); - // Resolve the target BEFORE patching the live DOM. The optimistic patch used // to run first, and only the save was wrapped in the unwind — so an // unresolvable target threw with the live preview (and, through the callers' @@ -447,6 +442,17 @@ export async function persistElementAttribute({ if (readTagSnippetByTarget(before, patchTarget) === undefined) { throw new Error(`Unable to patch element in ${targetPath}`); } + // The unwind value comes from the FILE, not from `readLive()`. + // + // Every live-write caller patches the DOM before committing — a fader drag is + // `setLive` per frame, hovering a preset auditions the whole chain — so by the + // time this runs the live DOM already holds the in-progress value. Reading it + // here made `previousValue === value`, so the unwind below was a no-op, and + // `setQuiet`'s catch (which deliberately re-mirrors the store from the live + // DOM) then mirrored that same never-saved value. The group audibly had the + // preset, the panel agreed, and a reload dropped it — the failure class the + // target check above was added to close, still open on the live-write path. + const previousValue = readAttributeByTarget(before, patchTarget, attr) ?? null; patchLive(value); const operation: PatchOperation = { type: "attribute", property: attr, value }; diff --git a/packages/studio/src/hooks/timelineElementFxAttribute.ts b/packages/studio/src/hooks/timelineElementFxAttribute.ts index 3f135b1b2..329293153 100644 --- a/packages/studio/src/hooks/timelineElementFxAttribute.ts +++ b/packages/studio/src/hooks/timelineElementFxAttribute.ts @@ -75,9 +75,6 @@ async function setElementAttribute({ domEditSaveTimestampRef, pendingTimelineEditPathRef, patchLive: (v) => patchLiveElementAttribute(previewIframe, element, attr, v, activeCompPath), - readLive: () => - findTimelineElementInIframe(previewIframe, element, activeCompPath)?.getAttribute(attr) ?? - null, }); }