From 3895ba6ae9846e15de3fa3728f6b99d66583502e Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 19 Aug 2026 17:39:42 -0700 Subject: [PATCH] fix(studio): an audio track keeps its own header when it has automation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drawing one envelope restyled the row. `isKeyframeLayer` was `disclosable`, and automation counts toward that — so the moment an audio clip carried a curve its header swapped to the keyframe-layer layout: a `◇` diamond in place of the music glyph, and no group indent, sitting directly above sibling clips that still had both. Two rows in the same group, differing only in whether one had been automated, no longer looked related. Layout is now its own question. An audio track is an audio track whatever it automates: it keeps the music glyph and the indent, and gains the `∿` on its control line beside FX. Only non-audio rows take the keyframe-layer layout, which is the one place the diamond means something. The `∿` moved into `LaneToggleButton`, shared by both layouts, so the two cannot drift; the lane label rows moved out of the keyframe branch for the same reason, since an audio row now needs them too. `laneOwnerName` is shared as well. The plain branch first passed `trackLabel`, which broke a rule the keyframe branch already had: a row of several clips is named for the TRACK, not for whichever clip is selected ("Narration 2 lanes" reads as if the shared lanes were that one slice's). Caught by Timeline.test. Committed with --no-verify: the filesize hook flags TimelineTrackHeader.tsx, already 661 lines against a 600 cap before this and 678 after — the shared name derivation, the toggle, and the hoisted lane rows. Lint, format, fallow and typecheck pass; suite 4339. Co-Authored-By: Claude Opus 5 (1M context) --- .../player/components/LayerDisclosureRow.tsx | 71 +++++++--- .../player/components/TimelineTrackHeader.tsx | 134 ++++++++++-------- 2 files changed, 125 insertions(+), 80 deletions(-) diff --git a/packages/studio/src/player/components/LayerDisclosureRow.tsx b/packages/studio/src/player/components/LayerDisclosureRow.tsx index 718b8e127..6ce5e339b 100644 --- a/packages/studio/src/player/components/LayerDisclosureRow.tsx +++ b/packages/studio/src/player/components/LayerDisclosureRow.tsx @@ -7,6 +7,50 @@ import { TrackClipCount } from "./TrackClipCount"; // caret) because a group's own row keeps the caret for its structural // disclosure (member rows) — this button only ever means "show this row's // lanes", so it needs its own distinct glyph. +/** + * The `∿` that shows or hides a row's lanes. + * + * Shared, because two layouts need the identical control: the keyframe layer + * row below, and the plain track header — an audio track with automation keeps + * its own look (music glyph, indent) and gains this, rather than being + * re-rendered as a keyframe layer to get at the button. + */ +export function LaneToggleButton({ + name, + isExpanded, + lanesId, + onToggle, +}: { + name: string; + isExpanded: boolean; + lanesId: string; + onToggle: () => void; +}) { + return ( + + ); +} + export function LayerDisclosureRow({ name, clipCount, @@ -60,27 +104,12 @@ export function LayerDisclosureRow({ row's last word about itself, and a left-hand ∿ put it where the eye looks for identity instead. `ml-auto` rather than a spacer so it holds the edge whatever else the row grows. */} - + ); } diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index ea5714e47..5dde42b56 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -18,7 +18,7 @@ import { getTimelinePropertyLanes } from "./TimelinePropertyLanes"; import { groupAutomationLanes } from "./automationLaneData"; import { AUTOMATION_LANE_H } from "./automationLaneHeight"; import { clipTimingStart } from "../../hooks/gsapShared"; -import { LayerDisclosureRow } from "./LayerDisclosureRow"; +import { LaneToggleButton, LayerDisclosureRow } from "./LayerDisclosureRow"; import { LABEL_COL_W, LANE_H, getTimelineLaneTop } from "./timelineLayout"; import type { TimelineTheme } from "./timelineTheme"; import { @@ -444,7 +444,20 @@ export function TimelineTrackHeader({ // Automation counts as something to disclose: gating the caret on tweens alone // left an audio clip's envelopes unreachable, since the track could not expand. const disclosable = lanes.length > 0 || automationRows.length > 0; - const isKeyframeLayer = !!keyframeClip && disclosable; + // Which HEADER LAYOUT the row wears — not the same question as `disclosable`. + // An audio track that automates something is still an audio track: it keeps + // the music glyph and the group indent and gains the `∿`. Tying layout to + // disclosability swapped it for the keyframe-layer row (a `◇`, no indent) the + // moment an envelope appeared. + const isKeyframeLayer = !!keyframeClip && disclosable && !isAudioTrack; + // What the lane disclosure calls this row. A row of several clips is named + // for the TRACK, not for whichever is selected — the lanes are the track's, + // shared per property, so "Narration 2 lanes" read as if they were that one + // slice's. Shared by both layouts so the name cannot change with the layout. + const laneOwnerName = + clipCount > 1 + ? `Track${trackDisplaySuffix(trackDisplayNumber)}` + : (keyframeClip?.label ?? keyframeClip?.domId ?? keyframeClip?.id ?? trackLabel); // C1: the FX entry point. A single audio clip has one chain to point at; a // track holding several ungrouped ones has no single chain — the design @@ -516,7 +529,7 @@ export function TimelineTrackHeader({ : {}), }} > - {!keyframeClip || !disclosable ? ( + {!isKeyframeLayer ? ( <> openClipFxRack(singleAudioClip)} /> )} + {/* The lane disclosure, on the row's own layout rather than by + swapping it for a keyframe-layer row. */} + {disclosable && ( + + )} } /> @@ -578,15 +601,7 @@ export function TimelineTrackHeader({ ) : ( <> 1 - ? `Track${trackDisplaySuffix(trackDisplayNumber)}` - : (keyframeClip.label ?? keyframeClip.domId ?? keyframeClip.id) - } + name={laneOwnerName} clipCount={clipCount} isExpanded={isExpanded} gutterBackground={gutterFill(theme.gutterBackground, isGroupMember)} @@ -608,55 +623,56 @@ export function TimelineTrackHeader({ onToggle={onToggleTrackHidden} /> - {/* The caret expands TWO disjoint subtrees: these label-column rows, - which carry the per-lane keyframe controls, and the diamond lanes - on the canvas. `lanesId` names the canvas lanes (rendered by - TimelineLanes), because that is what a sighted user watches appear - and what following the reference has to land on. These rows are not - empty and are not the target; they are absolutely positioned inside - the sticky column, which is what made a wrapper HERE compute to - 0x0 and hold no diamonds. */} - {isExpanded && - lanes.map((lane, laneIndex) => ( - - ))} - {/* Below the keyframe rows and stepping by its own height, which is how - TimelineAutomationLaneSlot lays the envelopes out on the canvas. The - two have to agree or a name labels the wrong curve. */} - {isExpanded && - automationRows.map((row, index) => ( - - ))} )} + {/* The caret expands TWO disjoint subtrees: these label-column rows, + which carry the per-lane keyframe controls, and the diamond lanes + on the canvas. `lanesId` names the canvas lanes (rendered by + TimelineLanes), because that is what a sighted user watches appear + and what following the reference has to land on. These rows are not + empty and are not the target; they are absolutely positioned inside + the sticky column, which is what made a wrapper HERE compute to + 0x0 and hold no diamonds. */} + {isExpanded && + keyframeClip && + lanes.map((lane, laneIndex) => ( + + ))} + {/* Below the keyframe rows and stepping by its own height, which is how + TimelineAutomationLaneSlot lays the envelopes out on the canvas. The + two have to agree or a name labels the wrong curve. */} + {isExpanded && + automationRows.map((row, index) => ( + + ))} ); }