Skip to content

Commit 0caeeb4

Browse files
committed
feat(studio): give the clips on a track one automation lane row per property
Several clips can share a track row. The row was named after whichever of them was selected, carried its clip count as a badge, and listed only that clip's envelopes — so a lane belonging to one slice read as governing the whole row, and clicking a sibling silently swapped which envelopes existed. A lane is a property over time, not a clip's private strip. Clips on one row now share a lane row when it is the same property of the same effect, keyed by `laneGroupKey` (the label, which is what identifies a parameter to a reader — node ids are minted per chain and collide across clips). Each clip draws its own envelope over its own span, and a clip that does not automate the property leaves that stretch empty rather than drawing a flat line that would claim an envelope exists. Gestures stay per clip: every clip keeps its own binding, its own useAutomationLaneGestures and its own selection box. A shared row is a shared lane track, not a shared envelope, so two clips' curves can never drag as one thing. That is why the slot loops over components rather than over lanes. Row height and the label column follow the grouped count, and the header names the track once the row holds several clips. Disclosure had the same selection-dependence: it was stored against the active clip, so expanding one slice and clicking another collapsed the row. The caret now opens and closes every clip on the row together, and any expanded clip holds it open. The remove button acts only on the clip the header is showing, since that is the only one a write can reach; a row that clip is absent from offers none. Effects stay on the CLIP. There is no track entity to own a chain — data-track-index is parsed in one place, only to choose a row, and both runtimes build audio per element.
1 parent d4d8062 commit 0caeeb4

11 files changed

Lines changed: 637 additions & 80 deletions

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import { CaretRight } from "@phosphor-icons/react";
2-
import type { TimelineElement } from "../store/playerStore";
32
import { TRACK_H } from "./timelineLayout";
43
import { TrackClipCount } from "./TrackClipCount";
54

65
// Layer row (Figma order: disclosure ▸/▾, diamond, name) — the disclosure lives
76
// here, not on the clip bar, and re-expands a collapsed layer.
87
export function LayerDisclosureRow({
9-
keyframeClip,
8+
name,
109
clipCount,
1110
isExpanded,
1211
gutterBackground,
@@ -15,7 +14,10 @@ export function LayerDisclosureRow({
1514
onToggleClipExpanded,
1615
children,
1716
}: {
18-
keyframeClip: TimelineElement;
17+
/** What this row is called. The active clip's own name when it is alone on the
18+
* track; the track itself once it holds several, since naming a shared row
19+
* after one of its clips reads as if the rows under it were that clip's. */
20+
name: string;
1921
clipCount: number;
2022
isExpanded: boolean;
2123
gutterBackground: string;
@@ -31,7 +33,6 @@ export function LayerDisclosureRow({
3133
/** Trailing controls that act on the LAYER (the visibility eye), not on a lane. */
3234
children?: React.ReactNode;
3335
}) {
34-
const name = keyframeClip.label ?? keyframeClip.domId ?? keyframeClip.id;
3536
return (
3637
<div
3738
className="absolute left-0 top-0 flex items-center gap-1.5 overflow-hidden px-1.5 text-[11px]"

packages/studio/src/player/components/Timeline.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
getTimelineLaneTop,
3838
createTimelineRowGeometry,
3939
} from "./timelineLayout";
40+
import { AUTOMATION_LANE_H } from "./automationLaneHeight";
4041
import { formatTime } from "../lib/time";
4142
import { usePlayerStore } from "../store/playerStore";
4243
import { TimelineEditProvider } from "../../contexts/TimelineEditContext";
@@ -559,6 +560,42 @@ describe("Timeline provider boundary", () => {
559560
act(() => root.unmount());
560561
});
561562

563+
// The caret belongs to the row, not to whichever clip on it is selected: the
564+
// automation lanes below it are the track's, shared per property. Toggling one
565+
// clip left the row's state depending on the selection, and a collapse that
566+
// only dropped the active clip left the row stuck open.
567+
it("expands and collapses every clip on a shared track together", () => {
568+
const host = createSizedTimelineHost(720);
569+
const automation = JSON.stringify({
570+
version: 1,
571+
lanes: [{ target: "volume", points: [{ t: 0, v: 1 }] }],
572+
});
573+
usePlayerStore.setState({
574+
duration: 8,
575+
timelineReady: true,
576+
elements: [
577+
{ id: "narration-1", tag: "audio", start: 0, duration: 4, track: 0, automation },
578+
{ id: "narration-2", tag: "audio", start: 4, duration: 4, track: 0, automation },
579+
],
580+
});
581+
const root = createRoot(host);
582+
act(() => root.render(React.createElement(Timeline)));
583+
584+
const row = host.querySelector<HTMLElement>('[data-el-id="narration-1"]')?.parentElement
585+
?.parentElement;
586+
// A row of several clips is named for the track, so the caret is too.
587+
const caret = () => host.querySelector<HTMLButtonElement>('button[aria-label$=" keyframes"]');
588+
expect(caret()?.getAttribute("aria-label")).toBe("Expand Track 1 keyframes");
589+
590+
act(() => caret()?.click());
591+
// One shared volume row, and BOTH clips hold it open.
592+
expectTrackExpansion(row, ["narration-1", "narration-2"], TRACK_H + AUTOMATION_LANE_H);
593+
594+
act(() => caret()?.click());
595+
expectTrackExpansion(row, [], TRACK_H);
596+
act(() => root.unmount());
597+
});
598+
562599
it("marks every clip in selectedElementIds as selected", () => {
563600
const host = createSizedTimelineHost(720);
564601

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

Lines changed: 99 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ import { simplifyPoints } from "./automationSimplify";
4444
import { pointInSelection, pointsIn, replaceRange } from "./automationLaneSelection";
4545
import { getTimelineLaneTop } from "./timelineLayout";
4646
import { defaultTimelineTheme } from "./timelineTheme";
47+
import { groupAutomationLanes } from "./automationLaneData";
48+
import { isAudioTimelineElement } from "../../utils/timelineInspector";
49+
import { getTimelineElementIdentity } from "../lib/timelineElementHelpers";
4750
import type { TimelineElement } from "../store/playerStore";
4851
import type { UseAutomationLanesResult } from "./useAutomationLanes";
4952

@@ -483,35 +486,42 @@ export function TimelineAutomationLane({
483486
);
484487
}
485488

486-
export interface TimelineAutomationLaneSlotProps {
487-
element: TimelineElement;
488-
isSelected: boolean;
489-
lanes: UseAutomationLanesResult;
490-
pps: number;
491-
/** Keyframe lanes already stacked above, which automation sits under. */
492-
laneCount: number;
493-
accentColor: string;
494-
/** Composition-time playhead; the slot converts it to clip-local. */
495-
currentTime: number;
496-
/** Composition-time beat grid; the slot converts it to clip-local too. */
497-
beatTimes?: readonly number[];
489+
/** Which shared rows one clip draws into, and with which of its lanes. */
490+
interface ClipLaneRow {
491+
lane: HfAutomationLane;
492+
rowIndex: number;
498493
}
499494

500495
/**
501-
* Every automated parameter on this clip, one lane per row — the way a DAW
502-
* stacks them, so two envelopes can be read and edited without swapping a
503-
* control to see either.
496+
* One clip's envelopes, each in the shared row its property owns.
497+
*
498+
* Its own component because every clip on the row needs its own binding, its own
499+
* gestures and its own selection box — a shared row is a shared lane track, not a
500+
* shared envelope, and two clips' curves must never drag as one thing. Hooks
501+
* cannot run in a loop, so the loop is over components.
504502
*/
505-
export function TimelineAutomationLaneSlot({
503+
function ClipAutomationLanes({
506504
element,
505+
rows,
507506
isSelected,
508507
lanes,
509508
pps,
510-
laneCount,
509+
top,
511510
accentColor,
512511
currentTime,
513512
beatTimes,
514-
}: TimelineAutomationLaneSlotProps) {
513+
}: {
514+
element: TimelineElement;
515+
rows: readonly ClipLaneRow[];
516+
isSelected: boolean;
517+
lanes: UseAutomationLanesResult;
518+
pps: number;
519+
/** y of the first automation row on this track. */
520+
top: number;
521+
accentColor: string;
522+
currentTime: number;
523+
beatTimes?: readonly number[];
524+
}) {
515525
// Beats inside this clip, in the clip's own frame — the lane's times are
516526
// clip-local, and a beat outside the clip can never be snapped to anyway.
517527
const snapTimes = useMemo(
@@ -525,19 +535,19 @@ export function TimelineAutomationLaneSlot({
525535
// Stale-selection guard: the selected lane's target can vanish out from under
526536
// it (e.g. its effect got deleted from the chain, dropping the lane), leaving
527537
// a rectangle selecting nothing. Clear it rather than let it point at a
528-
// target that no longer draws.
538+
// target that no longer draws. Above the empty-rows return, because a clip
539+
// that draws nothing is exactly when a selection goes stale.
529540
useEffect(() => {
530541
const target = bound.selection?.target;
531542
if (target !== undefined && !bound.lanes.some((lane) => lane.target === target)) {
532543
bound.onRangeClear();
533544
}
534545
}, [bound]);
535-
if (bound.lanes.length === 0) return null;
546+
if (rows.length === 0) return null;
536547
const inClip = currentTime >= element.start && currentTime <= element.start + element.duration;
537-
const top = getTimelineLaneTop(laneCount);
538548
return (
539549
<>
540-
{bound.lanes.map((lane, index) => {
550+
{rows.map(({ lane, rowIndex }) => {
541551
const range = resolveAutomationRange(lane.target, bound.chain ?? undefined);
542552
// A lane whose target no longer resolves was already dropped upstream;
543553
// this is belt and braces so a row can never draw on the wrong axis.
@@ -548,7 +558,7 @@ export function TimelineAutomationLaneSlot({
548558
duration={element.duration}
549559
widthPx={Math.max(element.duration * pps, 4)}
550560
leftPx={element.start * pps}
551-
topPx={top + index * AUTOMATION_LANE_H}
561+
topPx={top + rowIndex * AUTOMATION_LANE_H}
552562
automation={bound.automation}
553563
target={lane.target}
554564
range={range}
@@ -577,3 +587,69 @@ export function TimelineAutomationLaneSlot({
577587
</>
578588
);
579589
}
590+
591+
export interface TimelineAutomationLaneSlotProps {
592+
/** Every clip on the track, in row order — not just the selected one. */
593+
elements: readonly TimelineElement[];
594+
isSelected: (element: TimelineElement) => boolean;
595+
lanes: UseAutomationLanesResult;
596+
pps: number;
597+
/** Keyframe lanes already stacked above, which automation sits under. */
598+
laneCount: number;
599+
accentColor: string;
600+
/** Composition-time playhead; the slot converts it to clip-local. */
601+
currentTime: number;
602+
/** Composition-time beat grid; the slot converts it to clip-local too. */
603+
beatTimes?: readonly number[];
604+
}
605+
606+
/**
607+
* Every automated parameter on this TRACK, one lane per row — the way a DAW
608+
* stacks them, so two envelopes can be read and edited without swapping a
609+
* control to see either.
610+
*
611+
* Rows belong to the track, not to a clip: clips sharing a row share a row per
612+
* property (see `groupAutomationLanes`), each drawing over its own span, and a
613+
* clip that does not automate that property leaves its stretch empty. Binding one
614+
* clip at a time is what made the visible envelopes change with the selection.
615+
*/
616+
export function TimelineAutomationLaneSlot({
617+
elements,
618+
isSelected,
619+
lanes,
620+
pps,
621+
laneCount,
622+
accentColor,
623+
currentTime,
624+
beatTimes,
625+
}: TimelineAutomationLaneSlotProps) {
626+
const clips = elements.filter(isAudioTimelineElement);
627+
const rowsByClip = new Map<string, ClipLaneRow[]>();
628+
groupAutomationLanes(clips).forEach((group, rowIndex) => {
629+
for (const entry of group.entries) {
630+
const key = getTimelineElementIdentity(entry.element);
631+
const rows = rowsByClip.get(key);
632+
if (rows) rows.push({ lane: entry.lane, rowIndex });
633+
else rowsByClip.set(key, [{ lane: entry.lane, rowIndex }]);
634+
}
635+
});
636+
const top = getTimelineLaneTop(laneCount);
637+
return (
638+
<>
639+
{clips.map((element) => (
640+
<ClipAutomationLanes
641+
key={getTimelineElementIdentity(element)}
642+
element={element}
643+
rows={rowsByClip.get(getTimelineElementIdentity(element)) ?? []}
644+
isSelected={isSelected(element)}
645+
lanes={lanes}
646+
pps={pps}
647+
top={top}
648+
accentColor={accentColor}
649+
currentTime={currentTime}
650+
beatTimes={beatTimes}
651+
/>
652+
))}
653+
</>
654+
);
655+
}

packages/studio/src/player/components/TimelineAutomationLaneSlot.test.tsx

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { act } from "react";
33
import { describe, expect, it, vi } from "vitest";
44
import { createRoot } from "react-dom/client";
55
import { TimelineAutomationLaneSlot } from "./TimelineAutomationLane";
6+
import { AUTOMATION_LANE_H } from "./automationLaneHeight";
7+
import { PAD_X } from "./automationLaneGeometry";
8+
import { getTimelineLaneTop } from "./timelineLayout";
9+
import { elementAutomation, elementAutomationLanes, elementFxChain } from "./automationLaneData";
610
import type { AutomationLaneBinding, UseAutomationLanesResult } from "./useAutomationLanes";
711
import type { TimelineElement } from "../store/timelineElement";
812

@@ -40,8 +44,8 @@ function mountSlot(binding: Partial<AutomationLaneBinding>) {
4044
act(() => {
4145
createRoot(host).render(
4246
<TimelineAutomationLaneSlot
43-
element={element}
44-
isSelected={false}
47+
elements={[element]}
48+
isSelected={() => false}
4549
lanes={lanes}
4650
pps={100}
4751
laneCount={0}
@@ -53,6 +57,106 @@ function mountSlot(binding: Partial<AutomationLaneBinding>) {
5357
return { onRangeClear };
5458
}
5559

60+
/** Two narration slices sharing a row, each with its own chain. */
61+
const chainOf = (nodes: unknown[]) => JSON.stringify({ version: 1, nodes });
62+
const lanesOf = (...targets: string[]) =>
63+
JSON.stringify({
64+
version: 1,
65+
lanes: targets.map((target) => ({ target, points: [{ t: 0, v: 1 }] })),
66+
});
67+
68+
const narration1: TimelineElement = {
69+
...element,
70+
id: "narration-1",
71+
key: "narration-1",
72+
start: 0,
73+
duration: 4,
74+
fxChain: chainOf([
75+
{ type: "lowpass", id: "n1", params: { frequency: 8000, q: 0.7, poles: "2" } },
76+
{ type: "peaking", id: "n2", params: { frequency: 1000, gain: -3, q: 1.4 } },
77+
]),
78+
automation: lanesOf("fx.n2.q"),
79+
};
80+
const narration2: TimelineElement = {
81+
...element,
82+
id: "narration-2",
83+
key: "narration-2",
84+
start: 4,
85+
duration: 4,
86+
fxChain: chainOf([{ type: "peaking", id: "n1", params: { frequency: 1000, gain: -6, q: 1.4 } }]),
87+
automation: lanesOf("fx.n1.q", "volume"),
88+
};
89+
90+
/** Reads what a clip really carries, the way the live binding does. */
91+
const readingBind = (element: TimelineElement, isSelected: boolean): AutomationLaneBinding => ({
92+
automation: elementAutomation(element),
93+
lanes: elementAutomationLanes(element),
94+
chain: elementFxChain(element),
95+
onPreview: vi.fn(),
96+
onCommit: vi.fn(),
97+
onSelect: vi.fn(),
98+
readOnly: !isSelected,
99+
commitTargetKey: null,
100+
selection: null,
101+
onRangeSelect: vi.fn(),
102+
onRangeClear: vi.fn(),
103+
});
104+
105+
/** Every drawn envelope as `row @ left`, which is the whole claim under test. */
106+
function mountRow(elements: readonly TimelineElement[], selectedKey?: string) {
107+
const host = document.createElement("div");
108+
document.body.append(host);
109+
act(() => {
110+
createRoot(host).render(
111+
<TimelineAutomationLaneSlot
112+
elements={elements}
113+
isSelected={(el) => el.key === selectedKey}
114+
lanes={{ bind: readingBind }}
115+
pps={100}
116+
laneCount={0}
117+
accentColor="#0af"
118+
currentTime={0}
119+
/>,
120+
);
121+
});
122+
return [...host.querySelectorAll<HTMLElement>(".hf-automation-lane")]
123+
.map((lane) => `${lane.style.top} @ ${lane.querySelector("svg")?.style.left}`)
124+
.sort();
125+
}
126+
127+
const ROW_0 = `${getTimelineLaneTop(0)}px`;
128+
const ROW_1 = `${getTimelineLaneTop(0) + AUTOMATION_LANE_H}px`;
129+
130+
describe("TimelineAutomationLaneSlot shared rows", () => {
131+
it("draws two clips' envelopes for one property in the same row", () => {
132+
// One lane track, two envelopes — same row, each over its own span. The
133+
// 1 kHz peaking Q is `fx.n2.q` on one clip and `fx.n1.q` on the other.
134+
expect(mountRow([narration1, narration2])).toEqual(
135+
[
136+
`${ROW_0} @ ${0 - PAD_X}px`,
137+
`${ROW_0} @ ${400 - PAD_X}px`,
138+
`${ROW_1} @ ${400 - PAD_X}px`,
139+
].sort(),
140+
);
141+
});
142+
143+
it("leaves a clip's stretch empty in a row it does not automate", () => {
144+
// Only narration-2 has a volume envelope, so row 1 carries one curve and
145+
// narration-1's half of it stays blank rather than drawing a flat line.
146+
expect(mountRow([narration1, narration2]).filter((row) => row.startsWith(ROW_1))).toEqual([
147+
`${ROW_1} @ ${400 - PAD_X}px`,
148+
]);
149+
});
150+
151+
it("keeps the same rows whichever clip is selected", () => {
152+
// The bug this replaces: the row listed only the selected clip's lanes, so
153+
// clicking a sibling swapped which envelopes existed.
154+
expect(mountRow([narration1, narration2], "narration-1")).toEqual(
155+
mountRow([narration1, narration2], "narration-2"),
156+
);
157+
});
158+
});
159+
56160
describe("TimelineAutomationLaneSlot stale-selection guard", () => {
57161
it("clears the selection when its lane's target no longer exists", () => {
58162
const { onRangeClear } = mountSlot({

0 commit comments

Comments
 (0)