Skip to content

Commit 3545027

Browse files
vanceingallsclaude
andcommitted
fix(studio): let a track disclose its automation without a tween
The lane was mounted inside the property-lanes wrapper, which renders only for a track's GSAP keyframe clip — so an audio clip with no tween resolved to nothing: no disclosure caret, no reserved height, no lanes. Verified on a composition with one `<audio>`, an envelope, and no tweens anywhere: 0 carets, 0 lanes. The attribute still wrote and the render still baked it, so the feature failed silently for exactly the tracks it exists for. Same composition now: 1 caret, and expanding it draws the Volume lane. Automation counts as something to disclose. `resolveTrackKeyframeClip` takes a counter alongside the keyframe lane counts and qualifies a clip on either; the header asks the same counter about the clip it already holds. A function rather than another map threaded through the props: every caller then reads one cached parse, so the height a row reserves and the lanes drawn in it cannot drift apart. That drift is also fixed for the lane's own offset, which passed the raw tween count where every other consumer uses distinct property groups. Two tweens on one property drew one keyframe lane but pushed the automation lane down by two, spilling into the next track; one tween on two properties did the inverse and drew it over a diamond lane, stealing its pointer events. It now reads the same `laneCounts` map the reserved height and the drawn lanes use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8f61a00 commit 3545027

4 files changed

Lines changed: 82 additions & 12 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ export function TimelineLanes({
552552
isSelected={isSelected}
553553
lanes={automationLanes}
554554
pps={pps}
555-
laneCount={(gsapAnimations.get(elementKey) ?? []).length}
555+
laneCount={laneCounts.get(elementKey) ?? 0}
556556
accentColor={clipStyle.accent}
557557
currentTime={currentTime}
558558
/>

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Music } from "../../icons/SystemIcons";
44
import type { TimelineElement } from "../store/playerStore";
55
import type { TimelineEditCallbacks } from "./timelineCallbacks";
66
import { getTimelinePropertyLanes } from "./TimelinePropertyLanes";
7+
import { automationLaneCountOf } from "./useTimelineTrackLayout";
78
import { clipTimingStart } from "../../hooks/gsapShared";
89
import { LayerDisclosureRow } from "./LayerDisclosureRow";
910
import { TrackClipCount } from "./TrackClipCount";
@@ -310,7 +311,11 @@ export function TimelineTrackHeader({
310311
// Label mode = keyframe view; the label column stays LABEL_COL_W (Timeline.tsx
311312
// owns the gutter past it, so a 0% diamond isn't clipped by this panel).
312313
const showTrackLabel = contentOrigin >= LABEL_COL_W;
313-
const isKeyframeLayer = !!keyframeClip && lanes.length > 0;
314+
// Automation counts as something to disclose: gating the caret on tweens alone
315+
// left an audio clip's envelopes unreachable, since the track could not expand.
316+
const disclosable =
317+
lanes.length > 0 || (keyframeClip ? automationLaneCountOf(keyframeClip) : 0) > 0;
318+
const isKeyframeLayer = !!keyframeClip && disclosable;
314319

315320
return (
316321
<div
@@ -327,7 +332,7 @@ export function TimelineTrackHeader({
327332
borderRight: `1px solid ${theme.gutterBorder}`,
328333
}}
329334
>
330-
{!keyframeClip || lanes.length === 0 ? (
335+
{!keyframeClip || !disclosable ? (
331336
<PlainTrackHeader
332337
trackNumber={trackNumber}
333338
trackDisplayNumber={trackDisplayNumber}

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { afterEach, describe, expect, it } from "vitest";
77
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
88
import { LANE_H, TRACK_H } from "./timelineLayout";
99
import { getTimelinePropertyLanes } from "./TimelinePropertyLanes";
10-
import { useTimelineTrackLayout } from "./useTimelineTrackLayout";
10+
import { resolveTrackKeyframeClip, useTimelineTrackLayout } from "./useTimelineTrackLayout";
1111

1212
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
1313

@@ -92,3 +92,53 @@ describe("useTimelineTrackLayout", () => {
9292
unmount();
9393
});
9494
});
95+
const audioClip = (id: string, over: Partial<TimelineElement> = {}): TimelineElement => ({
96+
id,
97+
key: id,
98+
tag: "audio",
99+
start: 0,
100+
duration: 10,
101+
track: 10,
102+
...over,
103+
});
104+
105+
describe("resolveTrackKeyframeClip", () => {
106+
const none = new Map<string, number>();
107+
108+
it("picks an audio clip that has only automation, no tweens", () => {
109+
// Gating on tweens alone left an audio clip's envelopes unreachable: no
110+
// clip resolved, so the track got no caret, no height and no lanes.
111+
const bgm = audioClip("bgm");
112+
const picked = resolveTrackKeyframeClip([bgm], none, null, new Set(), () => 1);
113+
expect(picked).toBe(bgm);
114+
});
115+
116+
it("still resolves nothing when a clip has neither", () => {
117+
expect(resolveTrackKeyframeClip([audioClip("bgm")], none, null, new Set(), () => 0)).toBeNull();
118+
});
119+
120+
it("prefers the selected clip over the one with more to show", () => {
121+
const a = audioClip("a");
122+
const b = audioClip("b");
123+
const picked = resolveTrackKeyframeClip([a, b], new Map([["b", 4]]), "a", new Set(), (e) =>
124+
e.id === "a" ? 1 : 0,
125+
);
126+
expect(picked).toBe(a);
127+
});
128+
129+
it("counts tweens and automation together when breaking a tie", () => {
130+
const a = audioClip("a");
131+
const b = audioClip("b");
132+
const picked = resolveTrackKeyframeClip(
133+
[a, b],
134+
new Map([
135+
["a", 1],
136+
["b", 1],
137+
]),
138+
null,
139+
new Set(),
140+
(e) => (e.id === "b" ? 3 : 0),
141+
);
142+
expect(picked).toBe(b);
143+
});
144+
});

packages/studio/src/player/components/useTimelineTrackLayout.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,17 @@ import {
1616

1717
export { getTrackStyle } from "./timelineIcons";
1818

19+
/**
20+
* Automation lanes on one clip, or 0 for anything that is not audio.
21+
*
22+
* An audio clip can be worth expanding without carrying a single tween, so this
23+
* counts toward whether a track has anything to disclose. A function rather than
24+
* a map so every caller reads the same cached parse and none can drift.
25+
*/
26+
export function automationLaneCountOf(element: TimelineElement): number {
27+
return isAudioTimelineElement(element) ? elementAutomationLanes(element).length : 0;
28+
}
29+
1930
/**
2031
* The single keyframed element whose property lanes a track shows when expanded.
2132
* A track can hold several elements (same z-index is common), but keyframes are
@@ -29,10 +40,15 @@ export function resolveTrackKeyframeClip(
2940
laneCounts: ReadonlyMap<string, number>,
3041
selectedElementId: string | null,
3142
selectedElementIds: ReadonlySet<string>,
43+
automationLaneCount: (element: TimelineElement) => number = automationLaneCountOf,
3244
): TimelineElement | null {
33-
const keyframed = elements.filter(
34-
(element) => (laneCounts.get(element.key ?? element.id) ?? 0) >= 1,
35-
);
45+
// Automation counts toward "has something to disclose". Without it an audio
46+
// clip carrying envelopes but no tweens resolved to null, so its track got no
47+
// caret, no reserved height and no lanes — the automation was unreachable for
48+
// exactly the tracks the feature is for.
49+
const disclosable = (element: TimelineElement): number =>
50+
(laneCounts.get(element.key ?? element.id) ?? 0) + automationLaneCount(element);
51+
const keyframed = elements.filter((element) => disclosable(element) >= 1);
3652
if (keyframed.length === 0) return null;
3753
const selected = keyframed.find((element) => {
3854
const key = element.key ?? element.id;
@@ -41,8 +57,9 @@ export function resolveTrackKeyframeClip(
4157
if (selected) return selected;
4258
// Most lanes wins, first one on a tie (same as the old stable sort), but as a
4359
// reduce over the already non-empty list so there's no index to assert on.
44-
const lanesOf = (element: TimelineElement) => laneCounts.get(element.key ?? element.id) ?? 0;
45-
return keyframed.reduce((best, element) => (lanesOf(element) > lanesOf(best) ? element : best));
60+
return keyframed.reduce((best, element) =>
61+
disclosable(element) > disclosable(best) ? element : best,
62+
);
4663
}
4764

4865
/** Lanes per clip: the count of distinct property groups whose tween contributes
@@ -91,9 +108,7 @@ function useTimelineRowHeights(
91108
{
92109
clipId,
93110
laneCount: laneCounts.get(clipId) ?? 0,
94-
automationLaneCount: isAudioTimelineElement(active)
95-
? elementAutomationLanes(active).length
96-
: 0,
111+
automationLaneCount: automationLaneCountOf(active),
97112
},
98113
];
99114
});

0 commit comments

Comments
 (0)