Skip to content

Commit 4ea018a

Browse files
authored
fix(studio): align audio rows, automation lanes and headers (#3451)
* fix(core): harden audio FX and group identity * fix(core): address audio group review feedback * fix(core): align preview transport with grouped audio * test(core): pin audio group gain ceiling * fix(core): preserve solo bridge through stack * fix(engine): harden grouped audio rendering * docs(engine): explain grouped mix fallback invariant * test(engine): allow grouped mixes to finish on Windows * feat(lint): validate audio group membership and timing * test(lint): pin audio group membership guards * fix(studio): unify audio IDs and group state * fix(studio): make audio-group edits transactional * fix(studio): keep preview state synchronized * fix(studio): align audio rows, automation lanes and headers * fix(studio): stabilize timeline audio derivations * refactor(studio): simplify group metadata memoization * style(studio): keep timeline layout within size gate * fix(studio): keep timeline preset apply off auditions
1 parent 89069d2 commit 4ea018a

45 files changed

Lines changed: 2778 additions & 890 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/studio/src/components/editor/TimelineFxPopover.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ export function TimelineFxPopover({
9191
onOpenRack,
9292
}: TimelineFxPopoverProps) {
9393
const rootRef = useRef<HTMLDivElement | null>(null);
94-
const { audition, clearAudition } = useFxAudition(chain, onChainPreview, onAuditionTransport);
94+
const { audition, clearAudition, storedChain } = useFxAudition(
95+
chain,
96+
onChainPreview,
97+
onAuditionTransport,
98+
);
9599

96100
// Outside click dismisses like any other popover; the button itself is
97101
// excluded by pointerdown timing (the button's own click hasn't happened yet).
@@ -104,7 +108,7 @@ export function TimelineFxPopover({
104108
}, [onClose]);
105109

106110
const applyPreset = (id: string) => {
107-
const next = applyPresetToChain(chain, id, trackKind);
111+
const next = applyPresetToChain(storedChain(), id, trackKind);
108112
if (!next) return;
109113
clearAudition();
110114
onChainChange(next);
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { describe, expect, it } from "vitest";
2+
import { auditionStart } from "./useAuditionTransport";
3+
4+
const SPANS = [
5+
{ start: 2, duration: 7 },
6+
{ start: 18, duration: 7 },
7+
];
8+
9+
describe("auditionStart", () => {
10+
// Nothing to aim at — a caller with no spans keeps the old behaviour: play
11+
// from wherever the author left the playhead.
12+
it("stays put when there are no spans", () => {
13+
expect(auditionStart(undefined, 0)).toBeNull();
14+
expect(auditionStart([], 0)).toBeNull();
15+
});
16+
17+
// Already inside the clip: moving the playhead here would be the UI taking a
18+
// decision it was not asked for, and it would cost the author their place for
19+
// no gain.
20+
it("stays put when the playhead is already inside a span", () => {
21+
expect(auditionStart(SPANS, 2)).toBeNull();
22+
expect(auditionStart(SPANS, 8.9)).toBeNull();
23+
});
24+
25+
// The bug this exists for: hovering a preset at 0:00 on a group whose members
26+
// start at 0:02 played silence under the effect.
27+
it("jumps to the next span when the playhead is before or between them", () => {
28+
expect(auditionStart(SPANS, 0)).toBe(2);
29+
expect(auditionStart(SPANS, 9)).toBe(18);
30+
});
31+
32+
// Past everything, wrap to the first rather than play out the tail in silence.
33+
it("wraps to the first span when the playhead is past them all", () => {
34+
expect(auditionStart(SPANS, 40)).toBe(2);
35+
});
36+
});
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* Start playback for an audition, and put the playhead back on the way out.
3+
*
4+
* An audition writes the hovered preset to the running graph, which is silent
5+
* while the transport is paused — so a paused author hovering a preset heard
6+
* nothing at all and the affordance only worked mid-playback. Extracted from
7+
* `useFxLevelling`, where the property panel's rack owned it privately, because
8+
* the timeline's FX popover must render the preset shelf "exactly as FxSection
9+
* renders it — same props" (runbook C1 §2) and was passing no transport at all.
10+
*/
11+
12+
import { useRef } from "react";
13+
// The store's own module, not the `player` barrel: the barrel pulls the whole
14+
// timeline in, and the timeline's FX button imports this hook — a cycle.
15+
import { usePlayerStore } from "../../player/store/playerStore";
16+
17+
/** A clip the audition is meant to be heard through. */
18+
export interface AuditionSpan {
19+
start: number;
20+
duration: number;
21+
}
22+
23+
/**
24+
* Where to start playing so the audition is actually audible, or null to stay.
25+
*
26+
* Playing "from the playhead" only works when the thing being auditioned is
27+
* sounding there. A group whose members start at 0:02, hovered with the
28+
* playhead at 0:00, plays the rest of the mix unchanged — the transport runs,
29+
* the chain is in the graph, and the author hears nothing of the preset. So:
30+
* inside a span, stay; otherwise jump to the next one, wrapping to the first
31+
* when the playhead is past them all.
32+
*
33+
* Shared rather than the popover's own, which is where it went wrong the first
34+
* time: the property panel's rack has the identical hole, and giving the two
35+
* surfaces different audition behaviour is exactly what runbook C1 §2 forbids
36+
* when it says the shelf renders "exactly as FxSection renders it".
37+
*/
38+
export function auditionStart(
39+
spans: readonly AuditionSpan[] | undefined,
40+
at: number,
41+
): number | null {
42+
if (!spans || spans.length === 0) return null;
43+
if (spans.some((span) => at >= span.start && at < span.start + span.duration)) return null;
44+
const starts = spans.map((span) => span.start).sort((a, b) => a - b);
45+
return starts.find((start) => start > at) ?? starts[0] ?? null;
46+
}
47+
48+
export function useAuditionTransport(): (on: boolean, spans?: readonly AuditionSpan[]) => void {
49+
/**
50+
* Where the playhead was when an audition started the transport, so leaving
51+
* can put it back. Null means this audition did not start playback — the
52+
* transport was already running and must be left alone.
53+
*/
54+
const auditionReturn = useRef<number | null>(null);
55+
56+
/**
57+
* Already playing, this does nothing in either direction. The author started
58+
* that, and stopping their transport because they passed over a preset would
59+
* be the UI taking a decision that was not offered to it.
60+
*/
61+
return (on: boolean, spans?: readonly AuditionSpan[]): void => {
62+
const store = usePlayerStore.getState();
63+
if (on) {
64+
if (store.isPlaying || auditionReturn.current !== null) return;
65+
// Recorded BEFORE the seek, so leaving returns the author to where they
66+
// actually were rather than to the clip this jumped to.
67+
auditionReturn.current = store.currentTime;
68+
const from = auditionStart(spans, store.currentTime);
69+
if (from !== null) store.requestSeek(from);
70+
store.requestPlayback(true);
71+
return;
72+
}
73+
const returnTo = auditionReturn.current;
74+
if (returnTo === null) return;
75+
auditionReturn.current = null;
76+
store.requestPlayback(false, returnTo);
77+
};
78+
}

packages/studio/src/components/editor/useFxAudition.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,17 @@ export function useFxAudition(
5252
[chain, onChainPreview, onAuditionTransport],
5353
);
5454

55+
/**
56+
* The chain as the DOCUMENT has it, ignoring whatever is being auditioned.
57+
*
58+
* An audition writes through the preview channel, and the `chain` prop is
59+
* read back from that same live attribute — so mid-hover it is the hovered
60+
* preset, not the stored chain. Applying on top of it stacked the auditioned
61+
* preset into the saved chain: hover a reverb, click a different preset, and
62+
* both were persisted, which is heard as the effect running twice.
63+
*/
64+
const storedChain = useCallback(() => auditionBase.current ?? chain, [chain]);
65+
5566
/**
5667
* Drop whatever is being auditioned WITHOUT reverting the preview, for a
5768
* caller that is about to mutate the real chain anyway — reverting first
@@ -91,5 +102,5 @@ export function useFxAudition(
91102
[],
92103
);
93104

94-
return { audition, clearAudition };
105+
return { audition, clearAudition, storedChain };
95106
}

packages/studio/src/player/components/AutomationSelectionMenu.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ export const AutomationSelectionMenu = memo(function AutomationSelectionMenu({
4141
return createPortal(
4242
<div
4343
ref={menuRef}
44+
// z-[200] for the same reason the timeline's FX popover uses it: this is
45+
// portaled to `document.body`, but the ruler's sticky header sits at z-70
46+
// in the SAME root stacking context, so a z-50 menu opened near the top of
47+
// the timeline is painted through by the ruler and the playhead.
4448
className="hf-automation-menu fixed z-[200] min-w-[140px] rounded border border-panel-border-input bg-panel-bg-2 py-1 shadow-lg"
4549
style={{ left: adjustedX, top: adjustedY }}
4650
>

packages/studio/src/player/components/LayerDisclosureRow.tsx

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,56 @@
11
import { TRACK_H } from "./timelineLayout";
22
import { TrackClipCount } from "./TrackClipCount";
33

4-
// Layer row (Figma order: disclosure ∿, diamond, name) — the disclosure lives
4+
// Layer row (diamond, name, then the ∿ disclosure on the right edge) — the
5+
// disclosure lives
56
// here, not on the clip bar, and re-expands a collapsed layer. `∿` (not a
67
// caret) because a group's own row keeps the caret for its structural
78
// disclosure (member rows) — this button only ever means "show this row's
89
// lanes", so it needs its own distinct glyph.
10+
/**
11+
* The `∿` that shows or hides a row's lanes.
12+
*
13+
* Shared, because two layouts need the identical control: the keyframe layer
14+
* row below, and the plain track header — an audio track with automation keeps
15+
* its own look (music glyph, indent) and gains this, rather than being
16+
* re-rendered as a keyframe layer to get at the button.
17+
*/
18+
export function LaneToggleButton({
19+
name,
20+
isExpanded,
21+
lanesId,
22+
onToggle,
23+
}: {
24+
name: string;
25+
isExpanded: boolean;
26+
lanesId: string;
27+
onToggle: () => void;
28+
}) {
29+
return (
30+
<button
31+
type="button"
32+
// ponytail: No focus id here; keyboard routing belongs to the enclosing logical row.
33+
tabIndex={-1}
34+
aria-expanded={isExpanded}
35+
aria-controls={lanesId}
36+
aria-label={`${isExpanded ? "Hide" : "Show"} ${name} lanes`}
37+
title={`${isExpanded ? "Hide" : "Show"} lanes`}
38+
// h-6 w-6 = the 24x24 WCAG 2.2 minimum target. The glyph stays 11px;
39+
// only the hit box grows.
40+
className={`ml-auto flex h-6 w-6 shrink-0 items-center justify-center rounded border-0 bg-transparent p-0 text-[11px] leading-none focus-visible:outline focus-visible:outline-1 focus-visible:outline-[#3CE6AC] ${
41+
isExpanded ? "text-[#3CE6AC]" : "text-white/55 hover:text-white"
42+
}`}
43+
onPointerDown={(event) => event.stopPropagation()}
44+
onClick={(event) => {
45+
event.stopPropagation();
46+
onToggle();
47+
}}
48+
>
49+
<span aria-hidden="true"></span>
50+
</button>
51+
);
52+
}
53+
954
export function LayerDisclosureRow({
1055
name,
1156
clipCount,
@@ -45,37 +90,26 @@ export function LayerDisclosureRow({
4590
background: gutterBackground,
4691
}}
4792
>
48-
<button
49-
type="button"
50-
// ponytail: No focus id here; keyboard routing belongs to the enclosing logical row.
51-
tabIndex={-1}
52-
aria-expanded={isExpanded}
53-
aria-controls={lanesId}
54-
aria-label={`${isExpanded ? "Hide" : "Show"} ${name} lanes`}
55-
title={`${isExpanded ? "Hide" : "Show"} lanes`}
56-
// h-6 w-6 = the 24x24 WCAG 2.2 minimum target. The glyph stays 11px;
57-
// only the hit box grows.
58-
className={`flex h-6 w-6 shrink-0 items-center justify-center rounded border-0 bg-transparent p-0 text-[11px] leading-none focus-visible:outline focus-visible:outline-1 focus-visible:outline-[#3CE6AC] ${
59-
isExpanded ? "text-[#3CE6AC]" : "text-white/55 hover:text-white"
60-
}`}
61-
onPointerDown={(event) => event.stopPropagation()}
62-
onClick={(event) => {
63-
event.stopPropagation();
64-
onToggleClipExpanded();
65-
}}
66-
>
67-
<span aria-hidden="true"></span>
68-
</button>
6993
{/* Decorative: the disclosure button above already names the row's keyframe
7094
state, and aria-label on a plain span is not exposed reliably anyway. */}
7195
<span aria-hidden="true" className="shrink-0 text-[13px] leading-none text-white/40">
7296
7397
</span>
74-
<span className="min-w-0 flex-1 truncate font-medium" title={name}>
75-
{name}
76-
</span>
98+
{/* Wraps rather than truncating: a truncated name needs a hover to be
99+
read, which a scanned column cannot rely on. */}
100+
<span className="min-w-0 flex-1 break-words font-medium leading-tight">{name}</span>
77101
<TrackClipCount clipCount={clipCount} />
78102
{children}
103+
{/* Anchored right, on every header that has one: the lane toggle is the
104+
row's last word about itself, and a left-hand ∿ put it where the eye
105+
looks for identity instead. `ml-auto` rather than a spacer so it holds
106+
the edge whatever else the row grows. */}
107+
<LaneToggleButton
108+
name={name}
109+
isExpanded={isExpanded}
110+
lanesId={lanesId}
111+
onToggle={onToggleClipExpanded}
112+
/>
79113
</div>
80114
);
81115
}

packages/studio/src/player/components/PlayerControls.tsx

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { liveTime, usePlayerStore } from "../store/playerStore";
66
import { trackStudioEvent } from "../../utils/studioTelemetry";
77
import { Tooltip } from "../../components/ui";
88
import { useMountEffect } from "../../hooks/useMountEffect";
9-
import { useSoloBannerText } from "../../hooks/useAudioSoloBridge";
109
import { ShortcutsPanel } from "./ShortcutsPanel";
1110
import { SpeedMenu } from "./SpeedMenu";
1211
import { VolumeControl } from "./VolumeControl";
@@ -156,47 +155,12 @@ const FullscreenButton = memo(function FullscreenButton({
156155

157156
/* ── Main component ──────────────────────────────────────────────── */
158157

159-
/**
160-
* "Hearing only this" notice. Solo is a PREVIEW-only gate — it never touches an
161-
* attribute and never reaches the export — so the state has to say so out loud,
162-
* or a soloed session reads as a broken mix.
163-
*/
164-
const SoloBanner = memo(function SoloBanner({
165-
previewIframeRef,
166-
}: {
167-
previewIframeRef: { current: HTMLIFrameElement | null };
168-
}) {
169-
const bannerText = useSoloBannerText(previewIframeRef);
170-
const clearSolo = usePlayerStore.getState().clearSolo;
171-
if (bannerText === null) return null;
172-
return (
173-
<div
174-
role="status"
175-
className="flex h-7 items-center justify-center gap-2 border-b border-neutral-800 bg-neutral-900/90 px-3 text-[11px] text-neutral-300"
176-
>
177-
<span>
178-
Hearing only <span className="font-medium text-neutral-100">{bannerText}</span> — your
179-
export is not affected
180-
</span>
181-
<button
182-
type="button"
183-
onClick={() => clearSolo()}
184-
className="rounded px-1.5 py-0.5 font-medium text-studio-accent transition-colors hover:text-white"
185-
>
186-
Clear
187-
</button>
188-
</div>
189-
);
190-
});
191-
192158
interface PlayerControlsProps {
193159
onTogglePlay: () => void;
194160
onSeek: (time: number) => void;
195161
disabled?: boolean;
196162
isFullscreen?: boolean;
197163
onToggleFullscreen?: () => void;
198-
/** Needed to read the soloed clips' labels out of the preview document. */
199-
previewIframeRef?: { current: HTMLIFrameElement | null };
200164
}
201165

202166
export const PlayerControls = memo(function PlayerControls({
@@ -205,7 +169,6 @@ export const PlayerControls = memo(function PlayerControls({
205169
disabled = false,
206170
isFullscreen = false,
207171
onToggleFullscreen,
208-
previewIframeRef,
209172
}: PlayerControlsProps) {
210173
const isPlaying = usePlayerStore((s) => s.isPlaying);
211174
const duration = usePlayerStore((s) => s.duration);
@@ -258,7 +221,6 @@ export const PlayerControls = memo(function PlayerControls({
258221

259222
return (
260223
<div>
261-
{previewIframeRef && <SoloBanner previewIframeRef={previewIframeRef} />}
262224
<div
263225
className="grid h-10 grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)] items-center px-3"
264226
aria-disabled={disabled || undefined}

0 commit comments

Comments
 (0)