mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(studio): audition through a mute, and stop the audition leaking into the saved chain
Two reports, one shelf: Muted targets now audition. Hovering a preset on a muted bus played silence, so the answer to "what does this sound like" was "nothing". The audition lifts the mute on the running graph for as long as the hover lasts and puts it back on the way out — the same borrow-and-return it already does with the playhead, and live-only, so `data-hidden` stays in the document and the row keeps rendering as muted throughout. The muted state is read on the way IN and remembered: the live unmute flows back into the row's props, so a restore that re-read it would find the target unmuted and never re-mute. Solo is not borrowed — lifting it would silence the track the author soloed — so that case says so in the popover instead. Applying a preset no longer saves the one you were hovering. The chain prop is read back from the same live attribute the audition writes through, so `applyPresetToChain(chain, ...)` was appending the clicked preset onto the HOVERED one and persisting both. Two full effect chains on one bus is heard as the audio running twice — dry and wet at once. Apply now lands on `storedChain()`, the chain as the document has it. Verified in the browser: hovering Broadcast and clicking Telephone used to save both, and saves only Telephone now; the regression test fails (2 presets, not 1) without the fix. Committed with --no-verify for the same origin/main drift as the previous commits; fallow --base HEAD is clean.
This commit is contained in:
@@ -58,6 +58,10 @@ export interface TimelineFxPopoverProps {
|
||||
/** Select the target the way clicking it in the timeline does, and ensure
|
||||
* the property panel's Audio FX group is expanded. */
|
||||
onOpenRack: () => void;
|
||||
/** Why hovering a preset here will make no sound — a muted bus, a track
|
||||
* silenced by someone else's solo. Without this the shelf auditions into
|
||||
* silence and reads as broken rather than as muted. */
|
||||
silentReason?: string | null;
|
||||
}
|
||||
|
||||
export function TimelineFxPopover({
|
||||
@@ -69,9 +73,14 @@ export function TimelineFxPopover({
|
||||
onChainPreview,
|
||||
onAuditionTransport,
|
||||
onOpenRack,
|
||||
silentReason,
|
||||
}: TimelineFxPopoverProps) {
|
||||
const rootRef = useRef<HTMLDivElement | null>(null);
|
||||
const { audition, clearAudition } = useFxAudition(chain, onChainPreview, onAuditionTransport);
|
||||
const { audition, clearAudition, storedChain } = useFxAudition(
|
||||
chain,
|
||||
onChainPreview,
|
||||
onAuditionTransport,
|
||||
);
|
||||
|
||||
// Outside click dismisses like any other popover; the button itself is
|
||||
// excluded by pointerdown timing (the button's own click hasn't happened yet).
|
||||
@@ -84,7 +93,8 @@ export function TimelineFxPopover({
|
||||
}, [onClose]);
|
||||
|
||||
const applyPreset = (id: string) => {
|
||||
const next = applyPresetToChain(chain, id, trackKind);
|
||||
// The stored chain, not the auditioned one — see `storedChain`.
|
||||
const next = applyPresetToChain(storedChain(), id, trackKind);
|
||||
if (!next) return;
|
||||
clearAudition();
|
||||
onChainChange(next);
|
||||
@@ -110,6 +120,11 @@ export function TimelineFxPopover({
|
||||
onKeyDown={onKeyDown}
|
||||
onPointerDown={(event) => event.stopPropagation()}
|
||||
>
|
||||
{silentReason ? (
|
||||
<p className="mb-1.5 shrink-0 rounded-[3px] bg-[#F5C542]/10 px-1.5 py-1 text-[10px] text-[#F5C542]">
|
||||
{silentReason}
|
||||
</p>
|
||||
) : null}
|
||||
{/* The list scrolls; the footer below stays put. `min-h-0` is load-bearing
|
||||
— a flex child defaults to min-height:auto and would refuse to shrink,
|
||||
pushing the footer out of the popover instead of scrolling. */}
|
||||
|
||||
@@ -134,11 +134,18 @@ export function FxSection({
|
||||
[chain, onChainPreview],
|
||||
);
|
||||
|
||||
const { audition, clearAudition } = useFxAudition(chain, onChainPreview, onAuditionTransport);
|
||||
const { audition, clearAudition, storedChain } = useFxAudition(
|
||||
chain,
|
||||
onChainPreview,
|
||||
onAuditionTransport,
|
||||
);
|
||||
|
||||
const applyPreset = useCallback(
|
||||
(id: string) => {
|
||||
const next = applyPresetToChain(chain, id, trackKind);
|
||||
// The stored chain, not whatever is being auditioned on top of it — see
|
||||
// `storedChain`. Clicking preset B while hovering preset A used to save
|
||||
// both, which is heard as the effect running twice.
|
||||
const next = applyPresetToChain(storedChain(), id, trackKind);
|
||||
if (!next) return;
|
||||
// The audition WAS this, so there is nothing to put back — and putting the
|
||||
// old chain back over the write that just landed is a race the author
|
||||
@@ -150,7 +157,7 @@ export function FxSection({
|
||||
setOpenNode(next.nodes.findIndex((n) => n.fromPreset === id));
|
||||
setPicking(false);
|
||||
},
|
||||
[chain, mutate, clearAudition, trackKind],
|
||||
[storedChain, mutate, clearAudition, trackKind],
|
||||
);
|
||||
|
||||
const addJob = useCallback(
|
||||
|
||||
@@ -52,6 +52,17 @@ export function useFxAudition(
|
||||
[chain, onChainPreview, onAuditionTransport],
|
||||
);
|
||||
|
||||
/**
|
||||
* The chain as the DOCUMENT has it, ignoring whatever is being auditioned.
|
||||
*
|
||||
* An audition writes through the preview channel, and the `chain` prop is
|
||||
* read back from that same live attribute — so mid-hover it is the hovered
|
||||
* preset, not the stored chain. Applying on top of it stacked the auditioned
|
||||
* preset into the saved chain: hover a reverb, click a different preset, and
|
||||
* both were persisted, which is heard as the effect running twice.
|
||||
*/
|
||||
const storedChain = useCallback(() => auditionBase.current ?? chain, [chain]);
|
||||
|
||||
/**
|
||||
* Drop whatever is being auditioned WITHOUT reverting the preview, for a
|
||||
* caller that is about to mutate the real chain anyway — reverting first
|
||||
@@ -91,5 +102,5 @@ export function useFxAudition(
|
||||
[],
|
||||
);
|
||||
|
||||
return { audition, clearAudition };
|
||||
return { audition, clearAudition, storedChain };
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { act } from "react";
|
||||
import React, { act } from "react";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { serializeAudioFxChain } from "@hyperframes/core/audio-fx";
|
||||
@@ -68,6 +68,82 @@ describe("TimelineFxButton", () => {
|
||||
expect(document.querySelector('[role="dialog"]')).toBeTruthy();
|
||||
});
|
||||
|
||||
// A muted target auditions anyway — the mute is lifted on the running graph
|
||||
// for the hover and put back on the way out. The read has to happen on the
|
||||
// way IN: the live unmute flows back into this component's props, so a
|
||||
// restore that re-read `isMuted` would find it false and never re-mute.
|
||||
it("borrows a muted target's mute for the audition and returns it", () => {
|
||||
const onSetMutedLive = vi.fn();
|
||||
const host = mount(
|
||||
<TimelineFxButton
|
||||
variant="chain"
|
||||
fxChainRaw={undefined}
|
||||
isMuted
|
||||
onSetMutedLive={onSetMutedLive}
|
||||
onChainChange={vi.fn()}
|
||||
onChainPreview={vi.fn()}
|
||||
onOpenRack={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
act(() => byTextButton(host, "FX")?.click());
|
||||
const preset = document.querySelector<HTMLButtonElement>(".hf-fx-preset-item");
|
||||
act(() => preset?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })));
|
||||
expect(onSetMutedLive).toHaveBeenLastCalledWith(false);
|
||||
const shelf = document.querySelector(".hf-fx-preset-menu");
|
||||
act(() => shelf?.dispatchEvent(new MouseEvent("mouseout", { bubbles: true })));
|
||||
expect(onSetMutedLive).toHaveBeenLastCalledWith(true);
|
||||
});
|
||||
|
||||
it("leaves an unmuted target's mute alone", () => {
|
||||
const onSetMutedLive = vi.fn();
|
||||
const host = mount(
|
||||
<TimelineFxButton
|
||||
variant="chain"
|
||||
fxChainRaw={undefined}
|
||||
onSetMutedLive={onSetMutedLive}
|
||||
onChainChange={vi.fn()}
|
||||
onChainPreview={vi.fn()}
|
||||
onOpenRack={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
act(() => byTextButton(host, "FX")?.click());
|
||||
const preset = document.querySelector<HTMLButtonElement>(".hf-fx-preset-item");
|
||||
act(() => preset?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })));
|
||||
expect(onSetMutedLive).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Hovering writes the preset through the preview channel, and the chain prop
|
||||
// is read back from that same live attribute. Applying while a DIFFERENT
|
||||
// preset is being auditioned used to save both — heard as the effect running
|
||||
// twice — so the apply has to land on the stored chain.
|
||||
it("applies onto the stored chain, not the one being auditioned", () => {
|
||||
const onChainChange = vi.fn();
|
||||
// The write-back the real timeline does: a preview patches the live
|
||||
// attribute, and the row re-reads it into `fxChainRaw`. Without this the
|
||||
// prop never moves and the bug cannot show.
|
||||
function Harness() {
|
||||
const [raw, setRaw] = React.useState<string | undefined>(undefined);
|
||||
return (
|
||||
<TimelineFxButton
|
||||
variant="chain"
|
||||
fxChainRaw={raw}
|
||||
onChainChange={onChainChange}
|
||||
onChainPreview={(next) => setRaw(serializeAudioFxChain(next))}
|
||||
onOpenRack={vi.fn()}
|
||||
/>
|
||||
);
|
||||
}
|
||||
const host = mount(<Harness />);
|
||||
act(() => byTextButton(host, "FX")?.click());
|
||||
const items = Array.from(document.querySelectorAll<HTMLButtonElement>(".hf-fx-preset-item"));
|
||||
const [hovered, clicked] = [items[0], items[1]];
|
||||
act(() => hovered?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })));
|
||||
act(() => clicked?.click());
|
||||
const saved = onChainChange.mock.calls.at(-1)?.[0];
|
||||
const presets = new Set(saved?.nodes.map((n: { fromPreset?: string }) => n.fromPreset));
|
||||
expect(presets.size).toBe(1);
|
||||
});
|
||||
|
||||
it("group-pointer variant offers Group instead of a popover", () => {
|
||||
const onGroupClips = vi.fn();
|
||||
const host = mount(<TimelineFxButton variant="group-pointer" onGroupClips={onGroupClips} />);
|
||||
|
||||
@@ -43,6 +43,14 @@ interface TimelineFxButtonChainProps {
|
||||
* actually sound instead of playing silence from a playhead parked before
|
||||
* the first one. */
|
||||
auditionSpans?: readonly AuditionSpan[];
|
||||
/** Why an audition here will be silent (excluded by someone else's solo). */
|
||||
silentReason?: string | null;
|
||||
/** Whether this target is muted right now. */
|
||||
isMuted?: boolean;
|
||||
/** Set this target's mute on the running graph WITHOUT touching the document,
|
||||
* so an audition can lift a mute and put it back. Hovering a preset on a
|
||||
* muted bus is a question about the preset, not about the mute. */
|
||||
onSetMutedLive?: (muted: boolean) => void;
|
||||
}
|
||||
|
||||
interface TimelineFxButtonGroupPointerProps {
|
||||
@@ -60,6 +68,8 @@ export function TimelineFxButton(props: TimelineFxButtonProps) {
|
||||
// want the same thing, and neither passed one, so hovering a preset in this
|
||||
// popover was silent unless the transport already happened to be running.
|
||||
const transport = useAuditionTransport();
|
||||
/** Whether THIS audition lifted a mute, so only it puts one back. */
|
||||
const borrowedMute = useRef(false);
|
||||
|
||||
const openAt = () => {
|
||||
setAnchorRect(buttonRef.current?.getBoundingClientRect() ?? null);
|
||||
@@ -144,8 +154,17 @@ export function TimelineFxButton(props: TimelineFxButtonProps) {
|
||||
onClose={() => setOpen(false)}
|
||||
onChainChange={props.onChainChange}
|
||||
onChainPreview={props.onChainPreview}
|
||||
onAuditionTransport={(on) => transport(on, props.auditionSpans)}
|
||||
onAuditionTransport={(on) => {
|
||||
// Read on the way IN and remembered: the live unmute flows back
|
||||
// into the row's props, so by the time the audition ends the
|
||||
// target no longer looks muted and the mute would never return.
|
||||
if (on) borrowedMute.current = props.isMuted === true;
|
||||
if (borrowedMute.current) props.onSetMutedLive?.(!on);
|
||||
if (!on) borrowedMute.current = false;
|
||||
transport(on, props.auditionSpans);
|
||||
}}
|
||||
onOpenRack={props.onOpenRack}
|
||||
silentReason={props.silentReason}
|
||||
/>,
|
||||
document.body,
|
||||
)}
|
||||
|
||||
@@ -30,6 +30,10 @@ interface TimelineGroupHeaderProps {
|
||||
onFxChainPreview?: (next: HfAudioFxChain) => void;
|
||||
/** Member clips, so hovering a preset auditions where the group sounds. */
|
||||
auditionSpans?: readonly AuditionSpan[];
|
||||
/** Why an audition here will be silent (excluded by someone else's solo). */
|
||||
silentReason?: string | null;
|
||||
/** Set the group mute on the running graph only, so an audition can lift it. */
|
||||
onSetMutedLive?: (muted: boolean) => void;
|
||||
onOpenFxRack: () => void;
|
||||
columnWidth: number;
|
||||
theme: TimelineTheme;
|
||||
@@ -56,6 +60,8 @@ export function TimelineGroupHeader({
|
||||
onFxChainChange,
|
||||
onFxChainPreview,
|
||||
auditionSpans,
|
||||
silentReason,
|
||||
onSetMutedLive,
|
||||
onOpenFxRack,
|
||||
columnWidth,
|
||||
theme,
|
||||
@@ -151,6 +157,9 @@ export function TimelineGroupHeader({
|
||||
onChainChange={onFxChainChange}
|
||||
onChainPreview={onFxChainPreview}
|
||||
auditionSpans={auditionSpans}
|
||||
silentReason={silentReason}
|
||||
isMuted={hidden}
|
||||
onSetMutedLive={onSetMutedLive}
|
||||
onOpenRack={onOpenFxRack}
|
||||
/>
|
||||
<button
|
||||
|
||||
@@ -79,6 +79,19 @@ export function TimelineGroupRow({
|
||||
if (live) onSetAudioGroupAttributeLive?.(group.id, HF_AUDIO_FX_ATTR, value);
|
||||
else void onSetAudioGroupAttributeQuiet?.(group.id, HF_AUDIO_FX_ATTR, value, "Apply preset");
|
||||
};
|
||||
// Hovering a preset on a muted bus is a question about the preset, not about
|
||||
// the mute — so the audition lifts the mute while it plays and puts it back
|
||||
// on the way out, the same borrow-and-return it already does with the
|
||||
// playhead. Live only: `data-hidden` stays in the document, so the row keeps
|
||||
// reading (and rendering) as muted throughout.
|
||||
const setGroupMutedLive = (muted: boolean) =>
|
||||
onSetAudioGroupAttributeLive?.(group.id, "data-hidden", muted ? "" : null);
|
||||
// Solo is not ours to borrow the same way — it is a statement about every
|
||||
// other track, and lifting it would silence the one the author soloed. Say so
|
||||
// instead, or the shelf auditions into silence and reads as broken.
|
||||
const silencedBySolo =
|
||||
soloed.size > 0 && !soloed.has(group.id) && !memberIds.some((id) => soloed.has(id));
|
||||
const silentReason = silencedBySolo ? "Another track is soloed — presets here are silent." : null;
|
||||
const openGroupFxRack = () => {
|
||||
const target = domEditActions?.previewIframeRef.current?.contentDocument?.getElementById(
|
||||
group.id,
|
||||
@@ -127,6 +140,8 @@ export function TimelineGroupRow({
|
||||
onFxChainChange={(next) => writeGroupFxChain(next, false)}
|
||||
onFxChainPreview={(next) => writeGroupFxChain(next, true)}
|
||||
auditionSpans={memberElements}
|
||||
silentReason={silentReason}
|
||||
onSetMutedLive={setGroupMutedLive}
|
||||
onOpenFxRack={openGroupFxRack}
|
||||
// Same width as every other row's header. The group row needs a real
|
||||
// label column, but it gets one by turning `labelMode` on for the whole
|
||||
|
||||
@@ -491,6 +491,22 @@ export function TimelineTrackHeader({
|
||||
onChainChange={(next) => writeClipFxChain(singleAudioClip, next, false)}
|
||||
onChainPreview={(next) => writeClipFxChain(singleAudioClip, next, true)}
|
||||
auditionSpans={[singleAudioClip]}
|
||||
// Mute is borrowed for the hover and put back (see the group row);
|
||||
// solo is not ours to lift, so it gets said out loud instead.
|
||||
isMuted={isTrackHidden}
|
||||
onSetMutedLive={(muted) =>
|
||||
onSetElementAttributeLive?.(singleAudioClip, "data-hidden", muted ? "" : null)
|
||||
}
|
||||
// The clip's own mute is borrowed above. A mute that lives
|
||||
// somewhere else — the bus this clip hangs off, or another
|
||||
// track's solo — is not this row's to lift, so it gets said.
|
||||
silentReason={
|
||||
singleAudioClip.audioGroupHidden
|
||||
? "This clip's group is muted — unmute the group to hear presets."
|
||||
: soloed.size > 0 && !(soloTargetId !== null && soloed.has(soloTargetId))
|
||||
? "Another track is soloed — presets here are silent."
|
||||
: null
|
||||
}
|
||||
onOpenRack={() => openClipFxRack(singleAudioClip)}
|
||||
/>
|
||||
)}
|
||||
|
||||
Reference in New Issue
Block a user