From 4ea018a4a7ccdfee4e77538c336c37d366876779 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 23 Aug 2026 18:48:08 -0700 Subject: [PATCH] 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 --- .../components/editor/TimelineFxPopover.tsx | 8 +- .../editor/useAuditionTransport.test.ts | 36 + .../components/editor/useAuditionTransport.ts | 78 ++ .../src/components/editor/useFxAudition.ts | 13 +- .../components/AutomationSelectionMenu.tsx | 4 + .../player/components/LayerDisclosureRow.tsx | 84 ++- .../src/player/components/PlayerControls.tsx | 38 - .../src/player/components/Timeline.test.ts | 41 +- .../studio/src/player/components/Timeline.tsx | 7 +- .../TimelineAutomationLane.test.tsx | 67 ++ .../components/TimelineAutomationLane.tsx | 101 ++- .../components/TimelineAutomationLaneSlot.tsx | 25 +- .../components/TimelineFxButton.test.tsx | 188 ++++- .../player/components/TimelineFxButton.tsx | 187 ++++- .../components/TimelineGroupHeader.test.tsx | 5 - .../player/components/TimelineGroupHeader.tsx | 202 +++--- .../components/TimelineGroupLaneLabels.tsx | 88 +++ .../components/TimelineGroupRow.test.tsx | 146 ++++ .../player/components/TimelineGroupRow.tsx | 187 +++-- .../player/components/TimelineLanes.test.tsx | 6 +- .../src/player/components/TimelineLanes.tsx | 48 +- .../components/TimelineTrackHeader.test.tsx | 314 +++++++- .../player/components/TimelineTrackHeader.tsx | 682 ++++++++---------- .../TimelineTrackPlainHeader.test.tsx | 128 ++-- .../components/TimelineTrackPlainHeader.tsx | 118 +-- .../components/automationLaneData.test.ts | 94 +++ .../player/components/automationLaneData.ts | 23 + .../components/groupAutomationElement.test.ts | 51 ++ .../components/groupAutomationElement.ts | 37 + .../player/components/timelineCallbacks.ts | 8 +- .../timelineKeyboardNavigation.test.ts | 4 +- .../components/timelineKeyboardNavigation.ts | 33 +- .../src/player/components/timelineLayout.ts | 2 +- .../player/components/timelineViewModel.ts | 31 +- .../components/trackHeaderLabelRows.tsx | 328 +++++++++ .../components/useTimelineClipDisclosure.ts | 41 ++ .../components/useTimelineLaneRowIndexes.ts | 2 +- .../components/useTimelineLogicalFocus.ts | 4 +- .../useTimelineLogicalRows.test.tsx | 4 +- .../components/useTimelineLogicalRows.ts | 6 +- .../components/useTimelineTrackDerivations.ts | 55 +- .../components/useTimelineTrackLayout.test.ts | 119 +++ .../components/useTimelineTrackLayout.ts | 16 +- .../studio/src/player/store/keyframeSlice.ts | 18 +- .../studio/src/player/store/playerStore.ts | 3 +- 45 files changed, 2784 insertions(+), 896 deletions(-) create mode 100644 packages/studio/src/components/editor/useAuditionTransport.test.ts create mode 100644 packages/studio/src/components/editor/useAuditionTransport.ts create mode 100644 packages/studio/src/player/components/TimelineGroupLaneLabels.tsx create mode 100644 packages/studio/src/player/components/TimelineGroupRow.test.tsx create mode 100644 packages/studio/src/player/components/groupAutomationElement.test.ts create mode 100644 packages/studio/src/player/components/groupAutomationElement.ts create mode 100644 packages/studio/src/player/components/trackHeaderLabelRows.tsx create mode 100644 packages/studio/src/player/components/useTimelineClipDisclosure.ts diff --git a/packages/studio/src/components/editor/TimelineFxPopover.tsx b/packages/studio/src/components/editor/TimelineFxPopover.tsx index 70f827bf6f..a63eba137a 100644 --- a/packages/studio/src/components/editor/TimelineFxPopover.tsx +++ b/packages/studio/src/components/editor/TimelineFxPopover.tsx @@ -91,7 +91,11 @@ export function TimelineFxPopover({ onOpenRack, }: TimelineFxPopoverProps) { const rootRef = useRef(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). @@ -104,7 +108,7 @@ export function TimelineFxPopover({ }, [onClose]); const applyPreset = (id: string) => { - const next = applyPresetToChain(chain, id, trackKind); + const next = applyPresetToChain(storedChain(), id, trackKind); if (!next) return; clearAudition(); onChainChange(next); diff --git a/packages/studio/src/components/editor/useAuditionTransport.test.ts b/packages/studio/src/components/editor/useAuditionTransport.test.ts new file mode 100644 index 0000000000..694e6e1fd3 --- /dev/null +++ b/packages/studio/src/components/editor/useAuditionTransport.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { auditionStart } from "./useAuditionTransport"; + +const SPANS = [ + { start: 2, duration: 7 }, + { start: 18, duration: 7 }, +]; + +describe("auditionStart", () => { + // Nothing to aim at — a caller with no spans keeps the old behaviour: play + // from wherever the author left the playhead. + it("stays put when there are no spans", () => { + expect(auditionStart(undefined, 0)).toBeNull(); + expect(auditionStart([], 0)).toBeNull(); + }); + + // Already inside the clip: moving the playhead here would be the UI taking a + // decision it was not asked for, and it would cost the author their place for + // no gain. + it("stays put when the playhead is already inside a span", () => { + expect(auditionStart(SPANS, 2)).toBeNull(); + expect(auditionStart(SPANS, 8.9)).toBeNull(); + }); + + // The bug this exists for: hovering a preset at 0:00 on a group whose members + // start at 0:02 played silence under the effect. + it("jumps to the next span when the playhead is before or between them", () => { + expect(auditionStart(SPANS, 0)).toBe(2); + expect(auditionStart(SPANS, 9)).toBe(18); + }); + + // Past everything, wrap to the first rather than play out the tail in silence. + it("wraps to the first span when the playhead is past them all", () => { + expect(auditionStart(SPANS, 40)).toBe(2); + }); +}); diff --git a/packages/studio/src/components/editor/useAuditionTransport.ts b/packages/studio/src/components/editor/useAuditionTransport.ts new file mode 100644 index 0000000000..104b744471 --- /dev/null +++ b/packages/studio/src/components/editor/useAuditionTransport.ts @@ -0,0 +1,78 @@ +/** + * Start playback for an audition, and put the playhead back on the way out. + * + * An audition writes the hovered preset to the running graph, which is silent + * while the transport is paused — so a paused author hovering a preset heard + * nothing at all and the affordance only worked mid-playback. Extracted from + * `useFxLevelling`, where the property panel's rack owned it privately, because + * the timeline's FX popover must render the preset shelf "exactly as FxSection + * renders it — same props" (runbook C1 §2) and was passing no transport at all. + */ + +import { useRef } from "react"; +// The store's own module, not the `player` barrel: the barrel pulls the whole +// timeline in, and the timeline's FX button imports this hook — a cycle. +import { usePlayerStore } from "../../player/store/playerStore"; + +/** A clip the audition is meant to be heard through. */ +export interface AuditionSpan { + start: number; + duration: number; +} + +/** + * Where to start playing so the audition is actually audible, or null to stay. + * + * Playing "from the playhead" only works when the thing being auditioned is + * sounding there. A group whose members start at 0:02, hovered with the + * playhead at 0:00, plays the rest of the mix unchanged — the transport runs, + * the chain is in the graph, and the author hears nothing of the preset. So: + * inside a span, stay; otherwise jump to the next one, wrapping to the first + * when the playhead is past them all. + * + * Shared rather than the popover's own, which is where it went wrong the first + * time: the property panel's rack has the identical hole, and giving the two + * surfaces different audition behaviour is exactly what runbook C1 §2 forbids + * when it says the shelf renders "exactly as FxSection renders it". + */ +export function auditionStart( + spans: readonly AuditionSpan[] | undefined, + at: number, +): number | null { + if (!spans || spans.length === 0) return null; + if (spans.some((span) => at >= span.start && at < span.start + span.duration)) return null; + const starts = spans.map((span) => span.start).sort((a, b) => a - b); + return starts.find((start) => start > at) ?? starts[0] ?? null; +} + +export function useAuditionTransport(): (on: boolean, spans?: readonly AuditionSpan[]) => void { + /** + * Where the playhead was when an audition started the transport, so leaving + * can put it back. Null means this audition did not start playback — the + * transport was already running and must be left alone. + */ + const auditionReturn = useRef(null); + + /** + * Already playing, this does nothing in either direction. The author started + * that, and stopping their transport because they passed over a preset would + * be the UI taking a decision that was not offered to it. + */ + return (on: boolean, spans?: readonly AuditionSpan[]): void => { + const store = usePlayerStore.getState(); + if (on) { + if (store.isPlaying || auditionReturn.current !== null) return; + // Recorded BEFORE the seek, so leaving returns the author to where they + // actually were rather than to the clip this jumped to. + auditionReturn.current = store.currentTime; + const from = auditionStart(spans, store.currentTime); + if (from !== null) store.requestSeek(from); + store.requestPlayback(true); + return; + } + const returnTo = auditionReturn.current; + if (returnTo === null) return; + auditionReturn.current = null; + store.requestPlayback(false, returnTo); + }; +} diff --git a/packages/studio/src/components/editor/useFxAudition.ts b/packages/studio/src/components/editor/useFxAudition.ts index 6fff72d65d..3c7d9e6b61 100644 --- a/packages/studio/src/components/editor/useFxAudition.ts +++ b/packages/studio/src/components/editor/useFxAudition.ts @@ -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 }; } diff --git a/packages/studio/src/player/components/AutomationSelectionMenu.tsx b/packages/studio/src/player/components/AutomationSelectionMenu.tsx index f64890a03b..d5db655280 100644 --- a/packages/studio/src/player/components/AutomationSelectionMenu.tsx +++ b/packages/studio/src/player/components/AutomationSelectionMenu.tsx @@ -41,6 +41,10 @@ export const AutomationSelectionMenu = memo(function AutomationSelectionMenu({ return createPortal(
diff --git a/packages/studio/src/player/components/LayerDisclosureRow.tsx b/packages/studio/src/player/components/LayerDisclosureRow.tsx index 1dc27408dc..ef5b580a29 100644 --- a/packages/studio/src/player/components/LayerDisclosureRow.tsx +++ b/packages/studio/src/player/components/LayerDisclosureRow.tsx @@ -1,11 +1,56 @@ import { TRACK_H } from "./timelineLayout"; import { TrackClipCount } from "./TrackClipCount"; -// Layer row (Figma order: disclosure ∿, diamond, name) — the disclosure lives +// Layer row (diamond, name, then the ∿ disclosure on the right edge) — the +// disclosure lives // here, not on the clip bar, and re-expands a collapsed layer. `∿` (not a // 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, @@ -45,37 +90,26 @@ export function LayerDisclosureRow({ background: gutterBackground, }} > - {/* Decorative: the disclosure button above already names the row's keyframe state, and aria-label on a plain span is not exposed reliably anyway. */} - - {name} - + {/* Wraps rather than truncating: a truncated name needs a hover to be + read, which a scanned column cannot rely on. */} + {name} {children} + {/* Anchored right, on every header that has one: the lane toggle is the + 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/PlayerControls.tsx b/packages/studio/src/player/components/PlayerControls.tsx index 0f6484b66e..ed879fbbf3 100644 --- a/packages/studio/src/player/components/PlayerControls.tsx +++ b/packages/studio/src/player/components/PlayerControls.tsx @@ -6,7 +6,6 @@ import { liveTime, usePlayerStore } from "../store/playerStore"; import { trackStudioEvent } from "../../utils/studioTelemetry"; import { Tooltip } from "../../components/ui"; import { useMountEffect } from "../../hooks/useMountEffect"; -import { useSoloBannerText } from "../../hooks/useAudioSoloBridge"; import { ShortcutsPanel } from "./ShortcutsPanel"; import { SpeedMenu } from "./SpeedMenu"; import { VolumeControl } from "./VolumeControl"; @@ -156,47 +155,12 @@ const FullscreenButton = memo(function FullscreenButton({ /* ── Main component ──────────────────────────────────────────────── */ -/** - * "Hearing only this" notice. Solo is a PREVIEW-only gate — it never touches an - * attribute and never reaches the export — so the state has to say so out loud, - * or a soloed session reads as a broken mix. - */ -const SoloBanner = memo(function SoloBanner({ - previewIframeRef, -}: { - previewIframeRef: { current: HTMLIFrameElement | null }; -}) { - const bannerText = useSoloBannerText(previewIframeRef); - const clearSolo = usePlayerStore.getState().clearSolo; - if (bannerText === null) return null; - return ( -
- - Hearing only {bannerText} — your - export is not affected - - -
- ); -}); - interface PlayerControlsProps { onTogglePlay: () => void; onSeek: (time: number) => void; disabled?: boolean; isFullscreen?: boolean; onToggleFullscreen?: () => void; - /** Needed to read the soloed clips' labels out of the preview document. */ - previewIframeRef?: { current: HTMLIFrameElement | null }; } export const PlayerControls = memo(function PlayerControls({ @@ -205,7 +169,6 @@ export const PlayerControls = memo(function PlayerControls({ disabled = false, isFullscreen = false, onToggleFullscreen, - previewIframeRef, }: PlayerControlsProps) { const isPlaying = usePlayerStore((s) => s.isPlaying); const duration = usePlayerStore((s) => s.duration); @@ -258,7 +221,6 @@ export const PlayerControls = memo(function PlayerControls({ return (
- {previewIframeRef && }
{ act(() => root.unmount()); }); + // The same assertion, with a group present. It passed on an empty fixture + // while TimelineGroupRow called the THROWING context hook — one grouped clip + // and the whole timeline render died, not just the row. + it("renders without the provider even when a group row is on screen", () => { + const host = createSizedTimelineHost(640); + usePlayerStore.setState({ + duration: 4, + timelineReady: true, + elements: [ + { + id: "voice-1", + domId: "voice-1", + tag: "audio", + start: 0, + duration: 2, + track: 0, + audioGroup: "voiceover", + }, + { + id: "voice-2", + domId: "voice-2", + tag: "audio", + start: 2, + duration: 2, + track: 1, + audioGroup: "voiceover", + }, + ], + }); + const root = createRoot(host); + act(() => root.render(React.createElement(Timeline))); + + expect(host.querySelector('[role="treegrid"]')).not.toBeNull(); + act(() => root.unmount()); + }); + it("renders the complete track list while row virtualization is gated off", () => { const host = createSizedTimelineHost(640); usePlayerStore.setState({ @@ -389,7 +425,10 @@ describe("Timeline provider boundary", () => { button.click(); }); - const row = button.parentElement?.parentElement; + // Up from the rowheader rather than counting parents: the header now lays + // its name and its controls out on two lines, so the button sits one level + // deeper than it used to. + const row = button.closest('[role="rowheader"]')?.parentElement; // Row children: [TimelineTrackHeader (sticky column), time-mapped content]. const trackContent = row?.children.item(1); expect(onToggleTrackHidden).toHaveBeenCalledWith(0, false, 1); diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index a5f71d9056..6274d915e1 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -33,7 +33,7 @@ import { useTimelinePerformanceTelemetry } from "./useTimelinePerformanceTelemet import { getEffectiveTimelineDuration, getTimelinePreviewElement, - hasKeyframedTimelineClips, + timelineNeedsLabelColumn, } from "./timelineViewModel"; import { useTimelineSelectionLifecycle } from "./useTimelineSelectionLifecycle"; import { useTimelineShiftModifier } from "./useTimelineShiftModifier"; @@ -115,7 +115,10 @@ export const Timeline = memo(function Timeline({ const selectedElementIds = usePlayerStore((s) => s.selectedElementIds); const focusedEaseSegment = usePlayerStore((s) => s.focusedEaseSegment); const gsapAnimations = usePlayerStore((s) => s.gsapAnimations); - const labelMode = useMemo(() => hasKeyframedTimelineClips(gsapAnimations), [gsapAnimations]); + const labelMode = useMemo( + () => timelineNeedsLabelColumn(gsapAnimations, expandedElements), + [gsapAnimations, expandedElements], + ); // The label column provides pre-t=0 space; otherwise keep TRACKS_LEFT_PAD after the gutter. const contentOrigin = labelMode ? LABEL_COL_W + GUTTER : GUTTER + TRACKS_LEFT_PAD; const contentGutter = labelMode ? GUTTER : 0; diff --git a/packages/studio/src/player/components/TimelineAutomationLane.test.tsx b/packages/studio/src/player/components/TimelineAutomationLane.test.tsx index 94da3b6745..d1b9849c37 100644 --- a/packages/studio/src/player/components/TimelineAutomationLane.test.tsx +++ b/packages/studio/src/player/components/TimelineAutomationLane.test.tsx @@ -1747,3 +1747,70 @@ describe("TimelineAutomationLane stretch", () => { expect(svg.style.cursor).not.toBe("col-resize"); }); }); + +describe("TimelineAutomationLane — a read-only lane offers nothing to grab", () => { + /** Hover the lane the way a pointer entering it does. */ + function hover(container: HTMLElement): SVGSVGElement { + const svg = container.querySelector("svg")!; + stubBox(svg, { left: 0, top: 0, width: 400, height: 48 }); + // React implements onPointerEnter through the delegated `pointerover` + // event, not a native `pointerenter` — which does not bubble and so never + // reaches its listener. + fire(svg, "pointerover"); + return svg as SVGSVGElement; + } + + const handles = (container: HTMLElement) => + [...container.querySelectorAll("circle[data-automation-point]")] as SVGCircleElement[]; + + // A grab handle raised on hover is an offer, and a carve lane cannot honour + // it: the analysis rewrites these envelopes on every run, so a point moved + // here is discarded rather than saved. Dimming alone did not say that — the + // handles still came up under the cursor and the drag silently did nothing. + it("keeps its point handles hidden on hover", () => { + const { container } = render( + , + ); + hover(container); + const drawn = handles(container); + expect(drawn.length).toBeGreaterThan(0); + for (const c of drawn) { + expect(c.style.opacity).toBe("0"); + expect(c.style.cursor).toBe("default"); + } + }); + + it("still raises them on an editable lane, so the gate is readOnly and not hover", () => { + const { container } = render(); + hover(container); + const drawn = handles(container); + expect(drawn.length).toBeGreaterThan(0); + for (const c of drawn) { + expect(c.style.opacity).toBe("1"); + expect(c.style.cursor).toBe("grab"); + } + }); + + it("says why, in the lane, once hovered", () => { + const { container } = render( + , + ); + expect(container.querySelector("[data-automation-readonly-note]")).toBeNull(); + hover(container); + expect(container.querySelector("[data-automation-readonly-note]")?.textContent).toContain( + "Owned by the carve.", + ); + }); + + it("says nothing when no reason was given", () => { + const { container } = render( + , + ); + hover(container); + expect(container.querySelector("[data-automation-readonly-note]")).toBeNull(); + }); +}); diff --git a/packages/studio/src/player/components/TimelineAutomationLane.tsx b/packages/studio/src/player/components/TimelineAutomationLane.tsx index 061fea8e4c..eb8dfa2c7a 100644 --- a/packages/studio/src/player/components/TimelineAutomationLane.tsx +++ b/packages/studio/src/player/components/TimelineAutomationLane.tsx @@ -86,6 +86,75 @@ function pointCircleStyle( } /** Pointer shape: a read-only lane can only be selected, a live one edited. */ +/** + * Whether a point's grab handle is up. + * + * A raised handle is an offer to drag, so a read-only lane keeps them down: the + * carve rewrites its envelopes from the analysis on every run, and a point + * moved here is discarded rather than saved. Hidden rather than unmounted + * either way, so the hit area survives — a point dragged past the lane's edge + * fires pointerleave mid-gesture, and a handle that vanished then would drop + * the drag. A live drag and a selected range both keep them up regardless: the + * range is the subject of a pending Delete, and which points it caught cannot + * depend on where the mouse is. + */ +/** The svg's tooltip: what this lane's gestures actually are. */ +function laneTitle(readOnly: boolean | undefined): string { + return readOnly + ? "Drag a box to select points, which also selects this clip; then double-click to add a point" + : "Double-click to add a point, drag to shape, double-click a point to type a value, right-click or Shift+click to remove it. Drag the background to draw a box around points, then Delete to remove them or drag one to move them all. Alt-drag the line to curve it. Shift locks an axis mid-drag; Alt ignores the grid."; +} + +/** + * Why this lane will not take an edit, in the lane itself. + * + * Dimming and a default cursor say "not editable" but not WHY, and the why is + * the part that matters: the carve rewrites these envelopes from its own + * analysis on every run, so a point moved here is discarded rather than saved. + * Only while hovered, so a stack of read-only lanes is not a stack of notices, + * and never over a selection the author is about to act on. + * + * Separate from the gesture `hint` slot, which belongs to handlers that do not + * run on a read-only lane at all. + */ +function ReadOnlyNote({ + readOnly, + note, + hovered, + hasRange, + leftPx, + widthPx, +}: { + readOnly: boolean | undefined; + note: string | undefined; + hovered: boolean; + /** A selection is the subject of a pending action; do not cover it. */ + hasRange: boolean; + leftPx: number; + widthPx: number; +}) { + if (!readOnly || !note || !hovered || hasRange) return null; + return ( +
+ {note} +
+ ); +} + +function pointHandleOpacity(args: { + readOnly: boolean | undefined; + hovered: boolean; + dragging: boolean; + hasRange: boolean; +}): number { + if (args.dragging || args.hasRange) return 1; + return !args.readOnly && args.hovered ? 1 : 0; +} + function laneCursor(readOnly: boolean | undefined, dragging: boolean, stretching: boolean): string { // A stretch handle wins over everything it might also sit above: the handle is // a few px wide and always overlaps whatever is under the selection edge, so @@ -120,6 +189,9 @@ export interface TimelineAutomationLaneProps { snapTimes?: readonly number[]; /** Editing writes to the selected element, so an unselected clip is read-only. */ readOnly?: boolean; + /** Why `readOnly`, shown in the lane on hover. Without it the lane only looks + * disabled; the author still has to guess what would let them edit it. */ + readOnlyNote?: string; /** Called when a read-only lane is pressed: selects the clip so it goes live. */ onSelect?(): void; /** Active selection box on THIS lane, or null. */ @@ -142,6 +214,7 @@ export function TimelineAutomationLane({ onCommit, snapTimes, readOnly, + readOnlyNote, onSelect, rangeSelection, onRangeSelect, @@ -372,11 +445,7 @@ export function TimelineAutomationLane({ role="group" aria-label={`${range.label} automation`} > - - {readOnly - ? "Drag a box to select points, which also selects this clip; then double-click to add a point" - : "Double-click to add a point, drag to shape, double-click a point to type a value, right-click or Shift+click to remove it. Drag the background to draw a box around points, then Delete to remove them or drag one to move them all. Alt-drag the line to curve it. Shift locks an axis mid-drag; Alt ignores the grid."} - + {laneTitle(readOnly)} {/* No plate behind the envelope: the lane used to darken its clip's width, which drew a box inside the row and made a stack of lanes read as tiles rather than as rows of one timeline. The row background shows through, and @@ -433,15 +502,14 @@ export function TimelineAutomationLane({ fill={accentColor} stroke={stroke} strokeWidth={strokeWidth} - // Hidden rather than unmounted, so the hit area survives: a point - // dragged past the lane's edge fires pointerleave mid-gesture, and a - // handle that vanishes then would drop the drag. A drag in progress - // and a selected range both keep them up for the same reason — the - // range is the subject of a pending Delete, and which points it - // caught cannot depend on where the mouse is. style={{ cursor: readOnly ? "default" : "grab", - opacity: hovered || dragIndex !== null || rangeSelection ? 1 : 0, + opacity: pointHandleOpacity({ + readOnly, + hovered, + dragging: dragIndex !== null, + hasRange: Boolean(rangeSelection), + }), }} onContextMenu={(e) => { e.preventDefault(); @@ -484,6 +552,15 @@ export function TimelineAutomationLane({
) : null} + + {menuAt && rangeSelection ? ( {clips.map((element) => ( diff --git a/packages/studio/src/player/components/TimelineFxButton.test.tsx b/packages/studio/src/player/components/TimelineFxButton.test.tsx index 2d037ceb98..0ceac9cd44 100644 --- a/packages/studio/src/player/components/TimelineFxButton.test.tsx +++ b/packages/studio/src/player/components/TimelineFxButton.test.tsx @@ -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,45 +68,191 @@ describe("TimelineFxButton", () => { expect(document.querySelector('[role="dialog"]')).toBeTruthy(); }); - // The reported symptom was "the grouping button did nothing". The dialog WAS - // opening — it positioned at `anchorRect.bottom + 4` with no flip and no - // clamp, and this button lives in a track header at the bottom of the studio - // window, so it opened past the viewport edge. The test below it passed - // throughout: happy-dom reports an all-zero rect for an unlaid-out button, - // which lands the dialog at top:4 — on screen, and nothing like the app. + // 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( + , + ); + act(() => byTextButton(host, "FX")?.click()); + const preset = document.querySelector(".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( + , + ); + act(() => byTextButton(host, "FX")?.click()); + const preset = document.querySelector(".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(undefined); + return ( + setRaw(serializeAudioFxChain(next))} + onOpenRack={vi.fn()} + /> + ); + } + const host = mount(); + act(() => byTextButton(host, "FX")?.click()); + const items = Array.from(document.querySelectorAll(".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); + }); + + // The design doc calls the sentence this dialog carries "the highest-leverage + // copy in this plan": it is the concept of a submix bus delivered without the + // word, to an author who has never met one. The old pointer auto-named the + // group on one click and never mentioned the shared volume. + it("group-pointer variant names the group and explains what one is", () => { + const onGroupClips = vi.fn(); + const host = mount( + , + ); + act(() => byTextButton(host, "FX")?.click()); + const dialog = document.querySelector('[role="dialog"]'); + expect(dialog?.textContent).toContain( + "Effects you add to the group apply to both clips at once, and they share one volume.", + ); + const group = Array.from(document.body.querySelectorAll("button")).find( + (b) => b.textContent === "Group", + ); + act(() => group?.click()); + expect(onGroupClips).toHaveBeenCalledWith("Voiceover"); + }); + + // Three or more must not read "both". + it("counts the clips in the explanation", () => { + mount(); + act(() => byTextButton(document.body as HTMLElement, "FX")?.click()); + expect(document.querySelector('[role="dialog"]')?.textContent).toContain("all 3 clips at once"); + }); + + // §1.4 keeps groups audio-only in v1, and §5 is explicit that a deliberate + // limit must be stated: "silent ones just send authors hunting for something + // that was never built." + it("states the video limit instead of offering a name field", () => { + const onGroupClips = vi.fn(); + const host = mount( + , + ); + act(() => byTextButton(host, "FX")?.click()); + const dialog = document.querySelector('[role="dialog"]'); + expect(dialog?.textContent).toContain("Video audio can't be grouped yet"); + expect(document.querySelector('input[aria-label="Group name"]')).toBeNull(); + expect( + Array.from(document.body.querySelectorAll("button")).some((b) => b.textContent === "Group"), + ).toBe(false); + }); + + // #3421: the dialog positioned itself at `anchorRect.bottom + 4` with no flip + // and no clamp, and this button lives in a track header at the bottom of the + // studio window — so it opened past the viewport edge and was reported as + // "the grouping button did nothing". Numbers derive from GROUP_DIALOG_SIZE + // (256x160) against happy-dom's 1024x768 window and a 12px margin. it("flips the group dialog above the anchor when there is no room below", () => { - const host = mount(); + const host = mount( + , + ); const fx = byTextButton(host, "FX"); - // A track header near the bottom edge of the (1024x768) window. fx!.getBoundingClientRect = () => ({ left: 300, top: 760, right: 320, bottom: 776, width: 20, height: 16 }) as DOMRect; act(() => fx?.click()); const dialog = document.querySelector('[role="dialog"]') as HTMLElement; expect(dialog).toBeTruthy(); - // Above the anchor, and fully inside the viewport: 644 + 112 === 756. - expect(dialog.style.top).toBe("644px"); + const top = Number.parseFloat(dialog.style.top); + // Fully inside the viewport on both edges, which the unclamped form was not. + expect(top).toBeGreaterThanOrEqual(12); + expect(top + 160).toBeLessThanOrEqual(768 - 12); }); it("keeps the group dialog inside the right edge of the window", () => { - const host = mount(); + const host = mount( + , + ); const fx = byTextButton(host, "FX"); - // Hard against the right edge: 224px wide + a 12px margin has to fit. fx!.getBoundingClientRect = () => ({ left: 1010, top: 100, right: 1024, bottom: 116, width: 14, height: 16 }) as DOMRect; act(() => fx?.click()); const dialog = document.querySelector('[role="dialog"]') as HTMLElement; - expect(dialog.style.left).toBe("788px"); - expect(dialog.style.top).toBe("120px"); + const left = Number.parseFloat(dialog.style.left); + expect(left).toBeGreaterThanOrEqual(12); + expect(left + 256).toBeLessThanOrEqual(1024 - 12); }); - it("group-pointer variant offers Group instead of a popover", () => { + it("carries the typed name into the group it creates", () => { const onGroupClips = vi.fn(); - const host = mount(); + const host = mount( + , + ); act(() => byTextButton(host, "FX")?.click()); - const groupButton = document.body.querySelectorAll("button"); - const group = Array.from(groupButton).find((b) => b.textContent === "Group"); - expect(group).toBeDefined(); + const input = document.querySelector('input[aria-label="Group name"]'); + expect(input).not.toBeNull(); + // React tracks the input's value on the node, so assigning `.value` + // directly is swallowed — the native setter is what makes onChange fire. + const setValue = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + "value", + )?.set; + act(() => { + if (input && setValue) { + setValue.call(input, "SFX"); + input.dispatchEvent(new Event("input", { bubbles: true })); + } + }); + const group = Array.from(document.body.querySelectorAll("button")).find( + (b) => b.textContent === "Group", + ); act(() => group?.click()); - expect(onGroupClips).toHaveBeenCalledTimes(1); + expect(onGroupClips).toHaveBeenCalledWith("SFX"); }); }); diff --git a/packages/studio/src/player/components/TimelineFxButton.tsx b/packages/studio/src/player/components/TimelineFxButton.tsx index 65070df804..26e628c471 100644 --- a/packages/studio/src/player/components/TimelineFxButton.tsx +++ b/packages/studio/src/player/components/TimelineFxButton.tsx @@ -9,7 +9,7 @@ * it gets the grouping pointer instead of the popover). */ -import { useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { createPortal } from "react-dom"; import { enabledAudioFxNodes, @@ -19,14 +19,31 @@ import { import type { HfAudioNameKind } from "@hyperframes/core/audio-carve"; import { TimelineFxPopover } from "../../components/editor/TimelineFxPopover.js"; import { resolveFloatingPanelPosition } from "../../components/editor/floatingPanel.js"; +import { + useAuditionTransport, + type AuditionSpan, +} from "../../components/editor/useAuditionTransport.js"; -// Estimated, like FORMAT_PANEL_SIZE in RenderQueue: `w-56` is exact, and only +/** + * Naming a group is the moment the whole feature is explained. + * + * The design doc calls the sentence below "the highest-leverage copy in this + * plan and it should be written before the routing is" — it is the concept of a + * submix bus delivered without the word, to an author who has never met one. + * The old pointer skipped both the name and the sentence: it made an + * auto-named group on one click and said only "Group these clips to add effects + * to all of them", which leaves out the shared volume entirely. + */ +// Estimated, like FORMAT_PANEL_SIZE in RenderQueue: `w-64` is exact, and only // the flip decision uses the height — the clamp keeps the dialog on screen // either way. -const GROUP_DIALOG_SIZE = { width: 224, height: 112 }; +const GROUP_DIALOG_SIZE = { width: 256, height: 160 }; /** Where the grouping dialog goes: flipped above the anchor when there is no - * room below, and clamped so neither edge leaves the viewport. */ + * room below, and clamped so neither edge leaves the viewport. This button + * lives in a track header at the BOTTOM of the studio window, so an unclamped + * `anchorRect.bottom + 4` opened it past the viewport edge and read as "the + * grouping button did nothing". */ function groupDialogPosition(anchorRect: DOMRect): { left: number; top: number } { const { left, top } = resolveFloatingPanelPosition( anchorRect, @@ -37,6 +54,94 @@ function groupDialogPosition(anchorRect: DOMRect): { left: number; top: number } return { left, top }; } +function GroupNameDialog({ + anchorRect, + clipCount, + defaultLabel, + refusal, + onCancel, + onConfirm, +}: { + anchorRect: DOMRect; + clipCount: number; + defaultLabel?: string; + refusal?: string; + onCancel: () => void; + onConfirm: (label: string) => void; +}) { + const [label, setLabel] = useState(defaultLabel ?? "Voiceover"); + const inputRef = useRef(null); + // Focused on open so the name can be typed over without a second click — + // the field is the only thing here that wants input. + useEffect(() => inputRef.current?.select(), []); + if (refusal) { + return ( +
event.stopPropagation()} + onKeyDown={(event) => { + if (event.key !== "Escape") return; + event.stopPropagation(); + onCancel(); + }} + > +

{refusal}

+
+ ); + } + const confirm = () => onConfirm(label.trim() || defaultLabel || "Voiceover"); + return ( +
event.stopPropagation()} + onKeyDown={(event) => { + if (event.key === "Escape") { + event.stopPropagation(); + onCancel(); + } + if (event.key === "Enter") confirm(); + }} + > +

Name this group

+ setLabel(event.currentTarget.value)} + className="w-full rounded border border-white/20 bg-black/30 px-1.5 py-1 text-[11px] text-white outline-none focus:border-[#3CE6AC]" + /> + {/* The sentence. No jargon, and it names both things a bus does. */} +

+ Effects you add to the group apply to {clipCount === 2 ? "both" : `all ${clipCount}`} clips + at once, and they share one volume. +

+
+ + +
+
+ ); +} + function parseFxChainOrEmpty(raw: string | undefined): HfAudioFxChain { if (!raw) return { version: 1, nodes: [] }; try { @@ -53,12 +158,31 @@ interface TimelineFxButtonChainProps { fxChainRaw: string | undefined; onChainChange: (next: HfAudioFxChain) => void; onChainPreview?: (next: HfAudioFxChain) => void; - onAuditionTransport?: (on: boolean) => void; + /** The clips this chain is heard through, so an audition starts where they + * actually sound rather than from a playhead parked before the first. */ + auditionSpans?: readonly AuditionSpan[]; + /** 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 { variant: "group-pointer"; - onGroupClips: () => void; + /** Create the group under this name. */ + onGroupClips: (label: string) => void; + /** How many clips are about to be grouped, for the copy that explains it. */ + clipCount: number; + /** Seeded into the name field — "Voiceover" per the design's own mockup. */ + defaultLabel?: string; + /** Why this track cannot be grouped at all. Present, the dialog states the + * limit instead of offering a name field — groups are audio-only in v1 + * (§1.4), and the doc is explicit that a deliberate limit has to be said: + * "silent ones just send authors hunting for something that was never + * built." */ + refusal?: string; } type TimelineFxButtonProps = TimelineFxButtonChainProps | TimelineFxButtonGroupPointerProps; @@ -67,6 +191,12 @@ export function TimelineFxButton(props: TimelineFxButtonProps) { const [open, setOpen] = useState(false); const buttonRef = useRef(null); const [anchorRect, setAnchorRect] = useState(null); + // Owned here rather than threaded from each caller: both timeline call sites + // 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); @@ -94,30 +224,17 @@ export function TimelineFxButton(props: TimelineFxButtonProps) { {open && anchorRect && createPortal( -
event.stopPropagation()} - > -

Group these clips to add effects to all of them.

- -
, + setOpen(false)} + onConfirm={(label) => { + setOpen(false); + props.onGroupClips(label); + }} + />, document.body, )}
@@ -156,7 +273,15 @@ export function TimelineFxButton(props: TimelineFxButtonProps) { onClose={() => setOpen(false)} onChainChange={props.onChainChange} onChainPreview={props.onChainPreview} - onAuditionTransport={props.onAuditionTransport} + 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} />, document.body, diff --git a/packages/studio/src/player/components/TimelineGroupHeader.test.tsx b/packages/studio/src/player/components/TimelineGroupHeader.test.tsx index 41546eb374..936547d423 100644 --- a/packages/studio/src/player/components/TimelineGroupHeader.test.tsx +++ b/packages/studio/src/player/components/TimelineGroupHeader.test.tsx @@ -23,11 +23,6 @@ function renderHeader( laneCount={0} isLaneOpen={false} onToggleLanes={vi.fn()} - hidden={false} - onToggleHidden={vi.fn()} - isSoloed={false} - isHalfLitSolo={false} - onToggleSolo={vi.fn()} onFxChainChange={vi.fn()} onOpenFxRack={vi.fn()} columnWidth={220} diff --git a/packages/studio/src/player/components/TimelineGroupHeader.tsx b/packages/studio/src/player/components/TimelineGroupHeader.tsx index b28994b310..d477f732af 100644 --- a/packages/studio/src/player/components/TimelineGroupHeader.tsx +++ b/packages/studio/src/player/components/TimelineGroupHeader.tsx @@ -1,8 +1,8 @@ -import { SpeakerHigh, SpeakerSlash } from "@phosphor-icons/react"; import type { HfAudioFxChain } from "@hyperframes/core/audio-fx"; import { TRACK_H } from "./timelineLayout"; import type { TimelineTheme } from "./timelineTheme"; import { TimelineFxButton } from "./TimelineFxButton"; +import type { AuditionSpan } from "../../components/editor/useAuditionTransport.js"; interface TimelineGroupHeaderProps { label: string; @@ -14,20 +14,13 @@ interface TimelineGroupHeaderProps { laneCount: number; isLaneOpen: boolean; onToggleLanes: () => void; - /** The group element's own `data-hidden` — mutes every member at once. */ - hidden: boolean; - onToggleHidden: () => void; - /** This group id is itself in the soloed set (fully lit). */ - isSoloed: boolean; - /** Not soloed itself, but at least one member is (half-lit). */ - isHalfLitSolo: boolean; /** `add: true` (⌘/Ctrl-click) toggles membership; a plain click is exclusive. */ - onToggleSolo: (options?: { add?: boolean }) => void; /** C1: the group's serialized `data-fx-chain`, when set. */ fxChain?: string; onFxChainChange: (next: HfAudioFxChain) => void; onFxChainPreview?: (next: HfAudioFxChain) => void; - onFxAuditionTransport?: (on: boolean) => void; + /** Member clips, so hovering a preset auditions where the group sounds. */ + auditionSpans?: readonly AuditionSpan[]; onOpenFxRack: () => void; columnWidth: number; theme: TimelineTheme; @@ -35,8 +28,57 @@ interface TimelineGroupHeaderProps { /** * A group's own row header: caret (member disclosure) + `▤` + label + count + - * mute + solo + FX + `∿ n` (lane disclosure). + * FX + `∿ n` (lane disclosure). */ + +/** + * The group's name, which IS the way into its rack — a group is an element + * carrying `data-fx-chain`, so selecting it is what puts the chain in the + * property panel. Its own component because the header it sits in already + * carries six controls and was over the complexity gate with this inline. + */ +function GroupNameButton({ + label, + memberCount, + onOpenFxRack, +}: { + label: string; + memberCount: number; + onOpenFxRack: () => void; +}) { + return ( + + ); +} + export function TimelineGroupHeader({ label, memberCount, @@ -45,15 +87,10 @@ export function TimelineGroupHeader({ laneCount, isLaneOpen, onToggleLanes, - hidden, - onToggleHidden, - isSoloed, - isHalfLitSolo, - onToggleSolo, fxChain, onFxChainChange, onFxChainPreview, - onFxAuditionTransport, + auditionSpans, onOpenFxRack, columnWidth, theme, @@ -71,16 +108,19 @@ export function TimelineGroupHeader({ borderRight: `1px solid ${theme.gutterBorder}`, }} > + {/* One line, like a track header's: caret and name, then every control + anchored to the right edge. The name wraps and the controls are + `shrink-0`, so they hold the edge and the name gives way — no second + line needed to keep five controls off the label. */} - - - {label} - - - - - - )} - + ); } diff --git a/packages/studio/src/player/components/TimelineGroupLaneLabels.tsx b/packages/studio/src/player/components/TimelineGroupLaneLabels.tsx new file mode 100644 index 0000000000..6eb77512e0 --- /dev/null +++ b/packages/studio/src/player/components/TimelineGroupLaneLabels.tsx @@ -0,0 +1,88 @@ +/** + * The label column beside a group's own automation lanes. + * + * The designs draw a group lane as `▤ Volume 0.42` on an accent rail — and + * the rail is load-bearing, not decoration: "Scope is carried by colour, not by + * depth — a lane the group owns has an accent rail and names the group; a + * clip's lane is neutral." Two lanes both called Volume, doing entirely + * different things, otherwise sit eight pixels apart with nothing to tell them + * apart. + * + * The number is the value AT THE PLAYHEAD, not the stored seed: a readout that + * showed the seed would stand still while the curve is audibly working. + */ + +import { sampleAutomationLane } from "@hyperframes/core/audio-automation"; +import { groupAutomationLanes } from "./automationLaneData"; +import { AUTOMATION_LANE_H } from "./automationLaneHeight"; +import type { TimelineElement } from "../store/playerStore"; +import { useLivePlayheadTime } from "../../hooks/useLivePlayheadTime"; + +export function TimelineGroupLaneLabels({ + groupElement, + groupLabel, + top, + columnWidth, + gutterBackground, + accentColor, +}: { + /** The group wearing a clip's shape — see `groupAutomationElement`. */ + groupElement: TimelineElement; + groupLabel: string; + /** y of the first lane, matching the canvas slot's own offset. */ + top: number; + columnWidth: number; + gutterBackground: string; + accentColor: string; +}) { + // The LIVE playhead, not the row's `currentTime` prop — that one only moves + // on seek, so the readout sat frozen while the curve was audibly working, + // which is precisely the failure this number exists to prevent. + const currentTime = useLivePlayheadTime(); + // The SAME source the curves and the reserved height use + // (`TimelineGroupRow`), not raw `elementAutomationLanes`. Raw lanes are + // neither deduped by property nor filtered for resolvability, and the old + // `if (!parts) return null` consumed an index without drawing — so one + // unresolvable target slid every later label one row off the curve it names. + const laneGroups = groupAutomationLanes([groupElement]); + return ( + <> + {laneGroups.map((laneGroup, index) => { + const lane = laneGroup.entries[0]?.lane; + if (!lane) return null; + const parts = { name: laneGroup.name, param: laneGroup.param }; + // A group's clock is composition time (§1.3), so the playhead needs no + // clip-local rebase here — unlike a clip's lane. + const value = sampleAutomationLane(lane, currentTime); + return ( +
+ + + {parts.name} + {parts.param ? ( + {parts.param} + ) : null} + + + {value.toFixed(2)} + +
+ ); + })} + + ); +} diff --git a/packages/studio/src/player/components/TimelineGroupRow.test.tsx b/packages/studio/src/player/components/TimelineGroupRow.test.tsx new file mode 100644 index 0000000000..7ecc34f84c --- /dev/null +++ b/packages/studio/src/player/components/TimelineGroupRow.test.tsx @@ -0,0 +1,146 @@ +// @vitest-environment happy-dom +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TimelineGroupRow } from "./TimelineGroupRow"; +import { TimelineEditProvider } from "../../contexts/TimelineEditContext"; +import { defaultTimelineTheme } from "./timelineTheme"; +import type { TimelineTrackGroupInfo } from "./useTimelineTrackDerivations"; +import type { TimelineElement } from "../store/playerStore"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +vi.mock("../../telemetry/canary", () => ({ isCanaryEnabled: () => true })); + +afterEach(() => { + document.body.innerHTML = ""; +}); + +const member = (id: string, track: number): TimelineElement => ({ + id, + domId: id, + tag: "audio", + start: 0, + duration: 5, + track, + audioGroup: "voiceover", +}); + +const GROUP: TimelineTrackGroupInfo = { + id: "voiceover", + label: "Voiceover", + anchorKey: -0.5, + memberTracks: [0, 1], + memberElements: [member("vo-1", 0), member("vo-2", 1)], + volume: 1, + hidden: false, +}; + +function renderRow(overrides: Partial = {}) { + const onSetAudioGroupAttributeQuiet = vi.fn(); + const onSetElementAttributeQuiet = vi.fn(); + const host = document.createElement("div"); + document.body.append(host); + act(() => + createRoot(host).render( + + ({ lanes: [] }) } as never} + pps={10} + currentTime={0} + compositionDuration={60} + contentGutter={0} + trackContentWidth={800} + /> + , + ), + ); + return { host, onSetAudioGroupAttributeQuiet, onSetElementAttributeQuiet }; +} + +describe("TimelineGroupRow", () => { + // C1 names this as the step's own definition of done: "opening the popover on + // a GROUP and applying a preset results in exactly ONE `data-fx-chain` write, + // on the group element, and zero writes on members". A group IS a bus — a + // write that fanned out to the members would be batch-apply wearing a bus's + // clothes, which is the one thing §1 rules out. + it("applies a preset to the group element only, never to its members", () => { + const { host, onSetAudioGroupAttributeQuiet, onSetElementAttributeQuiet } = renderRow(); + const fx = Array.from(host.querySelectorAll("button")).find((b) => + b.getAttribute("aria-label")?.startsWith("Effects"), + ); + act(() => fx?.click()); + const preset = document.querySelector(".hf-fx-preset-item"); + act(() => preset?.click()); + + expect(onSetAudioGroupAttributeQuiet).toHaveBeenCalledTimes(1); + const [groupId, attr] = onSetAudioGroupAttributeQuiet.mock.calls[0] ?? []; + expect(groupId).toBe("voiceover"); + expect(attr).toBe("data-fx-chain"); + // The members are the point: not one write reaches them. + expect(onSetElementAttributeQuiet).not.toHaveBeenCalled(); + }); + + // A disclosure over nothing tells the author their group has no automation + // only AFTER they open an empty row. Track headers already gate their own + // toggle on having something to disclose; the group's did not. + it("hides the lane toggle until the group actually automates something", () => { + const laneToggle = (host: HTMLElement) => + Array.from(host.querySelectorAll("button")).find((b) => + /lanes$/.test(b.getAttribute("aria-label") ?? ""), + ); + + expect(laneToggle(renderRow().host)).toBeUndefined(); + + const automated = renderRow({ + fxChain: JSON.stringify({ + version: 1, + nodes: [{ type: "peaking", id: "p1", params: { frequency: 1000, gain: -3, q: 1 } }], + }), + automation: JSON.stringify({ + version: 1, + lanes: [{ target: "fx.p1.gain", points: [{ t: 0, v: 0 }] }], + }), + }); + expect(laneToggle(automated.host)).toBeDefined(); + }); + + // Same shape as a track header: caret and name on the left, every control in + // one right-anchored group. It was two lines — name, then controls — which is + // what let a stray child overflow the 48px box on the track side. + it("keeps the caret, the name and every control on one line", () => { + const { host } = renderRow({ + fxChain: JSON.stringify({ + version: 1, + nodes: [{ type: "peaking", id: "p1", params: { frequency: 1000, gain: -3, q: 1 } }], + }), + automation: JSON.stringify({ + version: 1, + lanes: [{ target: "fx.p1.gain", points: [{ t: 0, v: 0 }] }], + }), + }); + const header = host.querySelector('[role="rowheader"]'); + // Caret, name, control group — no second line. + expect(header?.children).toHaveLength(3); + const controls = header?.lastElementChild as HTMLElement | null; + expect(controls?.className).toContain("ml-auto"); + // Both controls live in that group, so they share the right edge. + expect(controls?.querySelectorAll("button")).toHaveLength(2); + // The member count rides with the name, not out with the controls. + expect(controls?.querySelector('[title="2 tracks"]')).toBeNull(); + act(() => undefined); + }); +}); diff --git a/packages/studio/src/player/components/TimelineGroupRow.tsx b/packages/studio/src/player/components/TimelineGroupRow.tsx index 5d88b4a6c1..b41f982d14 100644 --- a/packages/studio/src/player/components/TimelineGroupRow.tsx +++ b/packages/studio/src/player/components/TimelineGroupRow.tsx @@ -1,6 +1,3 @@ -import type { TimelineElement } from "../store/playerStore"; -import { usePlayerStore } from "../store/playerStore"; -import { isGroupHalfLitUnderSolo } from "../store/audioSoloSlice"; import { HF_AUDIO_FX_ATTR, serializeAudioFxChain, @@ -11,28 +8,43 @@ import type { TimelineTrackGroupInfo } from "./useTimelineTrackDerivations"; import type { TimelineLogicalRow } from "./timelineKeyboardNavigation"; import { TimelineTrackRow } from "./TimelineTrackRow"; import { TimelineGroupHeader } from "./TimelineGroupHeader"; -import { TimelineGroupBusStrip } from "./TimelineGroupBusStrip"; import { groupAutomationLanes } from "./automationLaneData"; -import { LABEL_COL_W } from "./timelineLayout"; -import { useTimelineEditContext } from "../../contexts/TimelineEditContext"; +import { groupAutomationElement } from "./groupAutomationElement"; +import { TimelineAutomationLaneSlot } from "./TimelineAutomationLaneSlot"; +import { TimelineGroupLaneLabels } from "./TimelineGroupLaneLabels"; +import { LABEL_COL_W, TRACK_H } from "./timelineLayout"; +import type { UseAutomationLanesResult } from "./useAutomationLanes"; +import { useDomEditSelectionContextOptional } from "../../contexts/DomEditContext"; +import { useTimelineEditContextOptional } from "../../contexts/TimelineEditContext"; import { useDomEditActionsContextOptional } from "../../contexts/DomEditContext"; +/** Accent rail on a group-owned lane — the same green the member rail uses, so + * "this belongs to the group" reads the same in both places (groups doc §5). */ +const GROUP_LANE_ACCENT = "#3CE6AC"; + interface TimelineGroupRowProps { index: number; rowKey: number; group: TimelineTrackGroupInfo; logicalRow: TimelineLogicalRow; - tracks: readonly (readonly [number, readonly TimelineElement[]])[]; top: number; height: number; virtualized: boolean; contentOrigin: number; theme: TimelineTheme; rovingTargetId?: string | null; - expandedGroupIds: ReadonlySet; + collapsedGroupIds: ReadonlySet; expandedLaneOwnerIds: ReadonlySet; toggleGroupExpanded: (id: string) => void; toggleLaneOwnerExpanded: (id: string) => void; + lanes: UseAutomationLanesResult; + pps: number; + currentTime: number; + /** A group's lanes are in composition time (§1.3), so this is their span. */ + compositionDuration: number; + beatTimes?: readonly number[]; + contentGutter: number; + trackContentWidth: number; } /** A group's own row: the accessible shell (shared with track rows) plus the group header. */ @@ -41,31 +53,45 @@ export function TimelineGroupRow({ rowKey, group, logicalRow, - tracks, top, height, virtualized, contentOrigin, theme, rovingTargetId = null, - expandedGroupIds, + collapsedGroupIds, expandedLaneOwnerIds, toggleGroupExpanded, toggleLaneOwnerExpanded, + lanes, + pps, + currentTime, + compositionDuration, + beatTimes, + contentGutter, + trackContentWidth, }: TimelineGroupRowProps) { - const memberElements = group.memberTracks.flatMap( - (track) => tracks.find(([t]) => t === track)?.[1] ?? [], - ); - const memberLabels = group.memberTracks.map((track, i) => { - const owner = tracks.find(([t]) => t === track)?.[1]?.find((el) => el.audioGroup); - return owner?.label ?? owner?.id ?? `track ${i + 1}`; - }); + // From the group, NOT from `tracks`: a collapsed group emits no member rows + // into the display list, and every one of these reads silently degraded to + // empty in that (default) state — the lane count + // read 0, and the bus strip fell back to "track 1", "track 2". + const memberElements = group.memberElements; + // The group wearing a clip's shape so the lane machinery can render it — see + // `groupAutomationElement` for why that beats a second, parallel lane path. + const groupElement = groupAutomationElement(group, compositionDuration); + // The binder writes through the dom-edit selection, so a group lane is + // editable exactly when the group is the selected element — which clicking + // its name in the header does. + const domSelection = useDomEditSelectionContextOptional()?.domEditSelection ?? null; + const isGroupSelected = domSelection?.id === group.id; const isLaneOpen = expandedLaneOwnerIds.has(group.id); - const { onSetAudioGroupAttributeLive, onSetAudioGroupAttributeQuiet } = useTimelineEditContext(); + // Optional, like every sibling row: Timeline renders outside the edit + // provider in read-only hosts (Timeline.test.ts asserts it), and the throwing + // hook took the whole timeline down with it the moment a group existed — + // not just this row. + const { onSetAudioGroupAttributeLive, onSetAudioGroupAttributeQuiet } = + useTimelineEditContextOptional(); const domEditActions = useDomEditActionsContextOptional(); - const soloed = usePlayerStore((s) => s.soloed); - const toggleSolo = usePlayerStore((s) => s.toggleSolo); - const memberIds = memberElements.map((el) => el.key ?? el.id); const writeGroupFxChain = (next: HfAudioFxChain, live: boolean) => { const value = next.nodes.length ? serializeAudioFxChain(next) : null; if (live) onSetAudioGroupAttributeLive?.(group.id, HF_AUDIO_FX_ATTR, value); @@ -95,51 +121,84 @@ export function TimelineGroupRow({ borderColor={theme.rowBorder} rovingTargetId={rovingTargetId} > - toggleGroupExpanded(group.id)} - laneCount={groupAutomationLanes(memberElements).length} - isLaneOpen={isLaneOpen} - onToggleLanes={() => toggleLaneOwnerExpanded(group.id)} - hidden={group.hidden} - onToggleHidden={() => - onSetAudioGroupAttributeQuiet?.( - group.id, - "data-hidden", - group.hidden ? null : "", - group.hidden ? "Unmute group" : `Mute group ${group.label}`, - ) - } - isSoloed={soloed.has(group.id)} - isHalfLitSolo={isGroupHalfLitUnderSolo(soloed, group.id, memberIds)} - onToggleSolo={(options) => toggleSolo(group.id, options)} - fxChain={group.fxChain} - onFxChainChange={(next) => writeGroupFxChain(next, false)} - onFxChainPreview={(next) => writeGroupFxChain(next, true)} - onOpenFxRack={openGroupFxRack} - columnWidth={contentOrigin >= LABEL_COL_W ? LABEL_COL_W : contentOrigin} - theme={theme} - /> - {isLaneOpen && ( - - onSetAudioGroupAttributeLive?.(group.id, "data-volume", String(value)) - } - onVolumeCommit={(value) => - onSetAudioGroupAttributeQuiet?.( - group.id, - "data-volume", - String(value), - "Set group volume", - ) - } + {/* Header and its lane labels in ONE sticky column — the shape + `TimelineTrackHeader` already uses: a fixed TRACK_H line box with the + lane rows absolutely positioned beneath it, the whole thing pinned. + The labels used to be SIBLINGS of the header, so `absolute left-0` + resolved against the ROW, and the row is what scrolls horizontally — + they slid away with the canvas. Sticky lives here rather than on the + header, which keeps its own box inside; a zero-width sticky wrapper + around the labels alone does not work either, because as a flex item + after the header it starts at x = columnWidth, i.e. inside the lanes. */} +
= LABEL_COL_W ? LABEL_COL_W : contentOrigin }} + > + toggleGroupExpanded(group.id)} + // The GROUP's own lanes, not its members'. `∿` is per-row (groups doc + // §5: "∿ is lit on vo-1 but not vo-2, the same control per row"), and + // counting the members' here made the group advertise curves it does + // not own and cannot show. + laneCount={groupAutomationLanes([groupElement]).length} + isLaneOpen={isLaneOpen} + onToggleLanes={() => toggleLaneOwnerExpanded(group.id)} + fxChain={group.fxChain} + onFxChainChange={(next) => writeGroupFxChain(next, false)} + onFxChainPreview={(next) => writeGroupFxChain(next, true)} + auditionSpans={memberElements} + 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 + // timeline (see Timeline.tsx) rather than by overhanging alone — an + // overhanging header paints opaquely across the rest of its row and + // stays pinned there through horizontal scroll. + columnWidth={contentOrigin >= LABEL_COL_W ? LABEL_COL_W : contentOrigin} theme={theme} /> + {/* The group's OWN curves, under the strip. Selected-gated exactly like a + clip's: the binder writes through the dom-edit selection, so a lane is + editable once the group is selected — which clicking its name does. */} + {/* The label column for those lanes, on the accent rail — inside the + sticky column above, so they pin with the header. */} + {isLaneOpen && ( + = LABEL_COL_W ? LABEL_COL_W : contentOrigin} + gutterBackground={theme.gutterBackground} + accentColor={GROUP_LANE_ACCENT} + /> + )} +
+ {isLaneOpen && ( + // The same offset content cell a track row wraps its lanes in — the + // slot positions absolutely, so mounted straight on the row it resolved + // against the row instead and drew the envelope across the label gutter + // from x=0. +
+ isGroupSelected} + lanes={lanes} + pps={pps} + // Below the strip, which sits directly under the header row. + laneCount={0} + topOffset={TRACK_H} + accentColor={GROUP_LANE_ACCENT} + currentTime={currentTime} + beatTimes={beatTimes} + /> +
)} ); diff --git a/packages/studio/src/player/components/TimelineLanes.test.tsx b/packages/studio/src/player/components/TimelineLanes.test.tsx index c290075e2c..b32c3088de 100644 --- a/packages/studio/src/player/components/TimelineLanes.test.tsx +++ b/packages/studio/src/player/components/TimelineLanes.test.tsx @@ -118,7 +118,7 @@ function renderLanes(options: RenderLanesOptions = {}): { selectedElementId: null, selectedElementIds: next.selectedElementIds ?? new Set(), expandedClipIds: new Set(next.expandedClipIds ?? []), - expandedGroupIds: new Set(), + collapsedGroupIds: new Set(), expandedLaneOwnerIds: new Set(), groups: [], trackGroupOf: new Map(), @@ -203,8 +203,8 @@ describe("TimelineLanes track numbering", () => { const second = view.host.querySelector('button[aria-label="Hide track 2"]'); act(() => second?.click()); - // Third argument is the display row the button announced, so the undo entry - // records the same row the user just read off it. + // Both, and they are different numbers: the real key acts, the display row + // is what the undo-history label must announce (see `onToggleTrackHidden`). expect(onToggleTrackHidden).toHaveBeenCalledWith(TRACK_B, true, 2); act(() => view.root.unmount()); }); diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 993ea46255..2243a79673 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -1,4 +1,4 @@ -import { Fragment, useId } from "react"; +import { Fragment, useId, useMemo } from "react"; import { BeatStrip, BeatBackgroundLines } from "./BeatStrip"; import { TimelineClip } from "./TimelineClip"; import { TimelineCompactDiamonds } from "./TimelineCompactDiamonds"; @@ -9,6 +9,7 @@ import { useAutomationSelectionKeyboard } from "../../hooks/useAutomationSelecti import { TimelineTrackHeader } from "./TimelineTrackHeader"; import { TimelineGroupRow } from "./TimelineGroupRow"; import { useTimelineLaneRowIndexes, useTimelineGroupDisclosure } from "./useTimelineLaneRowIndexes"; +import { useTimelineClipDisclosure } from "./useTimelineClipDisclosure"; import { isTrackRowExpanded, resolveTrackKeyframeClip, @@ -22,7 +23,6 @@ import { usePlayerStore } from "../store/playerStore"; import { isMultiDragPassenger, multiDragPassengerOffsetPx } from "./timelineMultiDragPreview"; import { useTimelineMultiDragActorWindows } from "./useTimelineMultiDragActorWindows"; import type { TimelineLanesProps } from "./timelineLaneProps"; -import { trackStudioKeyframeLaneExpand } from "../../telemetry/events"; import { isAudioTimelineElement, isMusicTrack } from "../../utils/timelineInspector"; import { createClipGestureHandlers } from "./timelineClipGestureHandlers"; import { renderClipChildren, resolveClipRenderContext } from "./timelineClipChildren"; @@ -100,29 +100,24 @@ export function TimelineLanes({ // from resolving into a second timeline that renders the same logical rows. const lanesIdPrefix = `timeline-lanes${useId().replaceAll(":", "")}`; const expandedClipIds = usePlayerStore((s) => s.expandedClipIds); - const { expandedGroupIds, expandedLaneOwnerIds, toggleGroupExpanded, toggleLaneOwnerExpanded } = + const { collapsedGroupIds, expandedLaneOwnerIds, toggleGroupExpanded, toggleLaneOwnerExpanded } = useTimelineGroupDisclosure(); const automationLanes = useAutomationLanes(); + // A group's automation clock is COMPOSITION time (groups doc §1.3), so its + // synthetic lane element spans the whole composition rather than a clip. + const compositionDuration = usePlayerStore((s) => s.duration); useAutomationSelectionKeyboard({ lanes: automationLanes }); - const expandClips = usePlayerStore((s) => s.expandClips); - const setClipExpanded = usePlayerStore((s) => s.setClipExpanded); - const toggleClipExpanded = usePlayerStore((s) => s.toggleClipExpanded); const { logicalRowsByTrack, groupByAnchor } = useTimelineLaneRowIndexes(logicalRows, groups); - // The caret belongs to the ROW, so it opens and closes every clip on it at - // once. Toggling only the active clip left the row's state depending on which - // sibling happened to be selected: expand one, click another, and the row - // collapsed under a caret that still pointed down. - const toggleRowExpandedTracked = (keys: readonly string[]) => { - const willExpand = !keys.some((key) => expandedClipIds.has(key)); - trackStudioKeyframeLaneExpand({ expanded: willExpand }); - if (willExpand) expandClips(keys); - else for (const key of keys) setClipExpanded(key, false); - }; - const toggleClipExpandedTracked = (key: string) => { - const willExpand = !expandedClipIds.has(key); - trackStudioKeyframeLaneExpand({ expanded: willExpand }); - toggleClipExpanded(key); - }; + // Which tracks are group MEMBERS, so their headers can render the level-2 + // nesting their `aria-level` already reports. + const groupMemberTracks = useMemo( + () => new Set(groups.flatMap((group) => group.memberTracks)), + [groups], + ); + const { + toggleRowExpanded: toggleRowExpandedTracked, + toggleClipExpanded: toggleClipExpandedTracked, + } = useTimelineClipDisclosure(); const actorWindows = useTimelineMultiDragActorWindows( multiDragPreview, rowsVirtualized, @@ -163,17 +158,23 @@ export function TimelineLanes({ rowKey={rowKey} group={group} logicalRow={groupLogicalRow} - tracks={tracks} top={rowGeometry.getRowTop(row)} height={rowGeometry.getRowHeight(row)} virtualized={rowsVirtualized} contentOrigin={contentOrigin} theme={theme} rovingTargetId={keyboard.rovingTargetId} - expandedGroupIds={expandedGroupIds} + collapsedGroupIds={collapsedGroupIds} expandedLaneOwnerIds={expandedLaneOwnerIds} toggleGroupExpanded={toggleGroupExpanded} toggleLaneOwnerExpanded={toggleLaneOwnerExpanded} + lanes={automationLanes} + pps={pps} + currentTime={currentTime} + compositionDuration={compositionDuration} + beatTimes={beatAnalysis?.beatTimes} + contentGutter={contentGutter} + trackContentWidth={trackContentWidth} /> ); } @@ -290,6 +291,7 @@ export function TimelineLanes({ currentTime={currentTime} isTrackHidden={isTrackHidden} isAudioTrack={isAudioTrack} + isGroupMember={groupMemberTracks.has(trackNum)} theme={theme} onToggleClipExpanded={() => { const keys = els.map(getTimelineElementIdentity); diff --git a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx index 52002b68f4..a455303991 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx @@ -7,9 +7,9 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { TimelinePropertyLanes } from "./TimelinePropertyLanes"; import { TimelineTrackHeader } from "./TimelineTrackHeader"; import { defaultTimelineTheme } from "./timelineTheme"; -import type { TimelineElement } from "../store/playerStore"; +import { type TimelineElement } from "../store/playerStore"; import type { TimelineEditCallbacks } from "./timelineCallbacks"; -import { getTimelineLaneTop, LABEL_COL_W } from "./timelineLayout"; +import { getTimelineLaneTop, LABEL_COL_W, TRACK_H } from "./timelineLayout"; import { AUTOMATION_LANE_H } from "./automationLaneHeight"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -72,6 +72,9 @@ interface RenderHeaderOptions { onTogglePropertyGroupKeyframe?: TimelineEditCallbacks["onTogglePropertyGroupKeyframe"]; onToggleTrackHidden?: TimelineEditCallbacks["onToggleTrackHidden"]; onRemoveAutomationLane?: (target: string) => void; + isAudioTrack?: boolean; + isGroupMember?: boolean; + isTrackHidden?: boolean; } function renderHeader(options: RenderHeaderOptions = {}): { @@ -82,7 +85,20 @@ function renderHeader(options: RenderHeaderOptions = {}): { const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); - const render = (next: RenderHeaderOptions) => { + const render = (raw: RenderHeaderOptions) => { + // Defaults resolved once, up front, rather than as a `??` per prop in the + // JSX — a dozen of those is a dozen branches through one arrow. + const next = { + keyframeClip: ELEMENT, + clipCount: 1, + animations: [POSITION, OPACITY], + currentTime: 0, + isAudioTrack: false, + isGroupMember: false, + isTrackHidden: false, + onToggleTrackHidden: vi.fn(), + ...raw, + }; act(() => { root.render( { + // §5: gain stages multiply. A group fading to 0.42 under a clip fading to + // 0.80 plays at 0.34, and an author who drew both hears something quieter + // than either with nothing on screen to say why. Not a warning; an + // explanation. + it("says so when the clip's group is fading the same parameter", () => { + const automation = JSON.stringify({ + version: 1, + lanes: [ + { + target: "volume", + points: [ + { t: 0, v: 1 }, + { t: 2, v: 0.4 }, + ], + }, + ], + }); + const clip: TimelineElement = { + ...ELEMENT, + tag: "audio", + automation, + audioGroup: "voiceover", + audioGroupLabel: "Voiceover", + audioGroupAutomation: automation, + }; + const view = renderHeader({ keyframeClip: clip, trackElements: [clip], animations: [] }); + expect(view.host.textContent).toContain("Voiceover is also fading this."); + act(() => view.root.unmount()); + }); + + // The same clip with an un-automated group must stay quiet — the note is + // only honest when the two curves actually multiply. + it("stays quiet when the group automates nothing", () => { + const automation = JSON.stringify({ + version: 1, + lanes: [ + { + target: "volume", + points: [ + { t: 0, v: 1 }, + { t: 2, v: 0.4 }, + ], + }, + ], + }); + const clip: TimelineElement = { + ...ELEMENT, + tag: "audio", + automation, + audioGroup: "voiceover", + audioGroupLabel: "Voiceover", + }; + const view = renderHeader({ keyframeClip: clip, trackElements: [clip], animations: [] }); + expect(view.host.textContent).not.toContain("is also fading this"); + act(() => view.root.unmount()); + }); + // An expanded sub-composition child sits on the MASTER timeline at a // host-absolute start, but its tweens are parsed from its own file and are // local to it. Feeding the raw start straight into the clip-% math put every @@ -180,6 +254,55 @@ describe("TimelineTrackHeader", () => { act(() => view.root.unmount()); }); + // The visibility control is the old hide eye. On an audio track it silences + // rather than hides, and the row already says so with a speaker elsewhere — + // so the eye's slot stays empty there. A non-audio track is untouched. + it("keeps the visibility control off audio track headers", () => { + const audio: TimelineElement = { ...ELEMENT, tag: "audio" }; + const view = renderHeader({ + keyframeClip: audio, + trackElements: [audio], + isAudioTrack: true, + animations: [], + }); + const labels = Array.from(view.host.querySelectorAll("button")).map((b) => + b.getAttribute("aria-label"), + ); + expect(labels.some((l) => l && /^(Hide|Show) track/.test(l))).toBe(false); + expect(labels).not.toContain("Mute"); + act(() => view.root.unmount()); + }); + + // The escape hatch. `data-hidden` on audio silences it in preview and drops it + // from the render; the panel's "Muted" is the unrelated HTML `muted` + // attribute, and nothing else writes it. Withholding the eye unconditionally + // meant a track hidden by "Hide all" (or by hand, or before that rule existed) + // was silent with no control anywhere to bring it back. + it("offers the eye on an audio track that is already hidden, so it can be restored", () => { + const audio: TimelineElement = { ...ELEMENT, tag: "audio" }; + const view = renderHeader({ + keyframeClip: audio, + trackElements: [audio], + isAudioTrack: true, + isTrackHidden: true, + animations: [], + }); + const labels = Array.from(view.host.querySelectorAll("button")).map((b) => + b.getAttribute("aria-label"), + ); + expect(labels.some((l) => l && /^Show track/.test(l))).toBe(true); + act(() => view.root.unmount()); + }); + + it("keeps it on a non-audio track", () => { + const view = renderHeader({ isAudioTrack: false }); + const labels = Array.from(view.host.querySelectorAll("button")).map((b) => + b.getAttribute("aria-label"), + ); + expect(labels.some((l) => l && /^Hide track/.test(l))).toBe(true); + act(() => view.root.unmount()); + }); + // The eye acts on the layer, so it has to be reachable without a pointer and // in every disclosure state — a hover-gated eye is unusable by keyboard. it("keeps the visibility eye mounted whether the layer is expanded or collapsed", () => { @@ -614,7 +737,9 @@ describe("TimelineTrackHeader", () => { it("names the header for the track, not for one of the clips on it", () => { const view = renderHeader({ ...ROW, keyframeClip: NARRATION_2 }); expect(view.host.textContent).not.toContain("narration-2"); - expect(view.host.querySelector('[title="Track 1"]')?.textContent).toBe("Track 1"); + // Asserted on the rendered text, not a `title`: the name wraps now rather + // than truncating, so it no longer carries a tooltip to be found by. + expect(view.host.textContent).toContain("Track 1"); // Alone on the track it is still named for itself. view.rerender({ ...ROW, @@ -622,7 +747,170 @@ describe("TimelineTrackHeader", () => { trackElements: [NARRATION_2], clipCount: 1, }); - expect(view.host.querySelector('[title="narration-2"]')?.textContent).toBe("narration-2"); + expect(view.host.textContent).toContain("narration-2"); + act(() => view.root.unmount()); + }); + }); + + describe("audio ids and canary gates", () => { + const VOICE: TimelineElement = { + id: "voice-1", + key: "index.html#voice-1", + domId: "voice-1", + tag: "audio", + start: 0, + duration: 5, + track: 0, + }; + const VOICE_2: TimelineElement = { + ...VOICE, + id: "voice-2", + key: "index.html#voice-2", + domId: "voice-2", + }; + + // The set is pushed straight into the runtime, which compares it against + // `el.id`. A store key here matches nothing, `isAudibleUnderSolo` returns + // false for every element, and soloing silences the whole preview. + // A member row is `aria-level="2"`, and without this it looked identical to + // every top-level row — the nesting existed for a screen reader and not for + // an eye. B2's design called for the accent rail; only the semantics shipped. + it("indents a group member's row and gives it the accent rail", () => { + const view = renderHeader({ + keyframeClip: VOICE, + animations: [], + expanded: false, + isAudioTrack: true, + }); + const header = () => view.host.querySelector('[role="rowheader"]'); + + expect(header()?.style.paddingLeft).toBe(""); + expect(header()?.style.borderLeft).toBe(""); + expect(header()?.style.background).not.toContain("linear-gradient"); + + view.rerender({ + keyframeClip: VOICE, + animations: [], + expanded: false, + isAudioTrack: true, + isGroupMember: true, + }); + expect(header()?.style.paddingLeft).toBe("14px"); + expect(header()?.style.borderLeft).toContain("2px"); + // And a lighter gutter, so the row reads as sitting INSIDE its group + // rather than beside it. Overlaid on the theme's own fill rather than a + // hard-coded colour, so it follows whatever the gutter is. + expect(header()?.style.background).toContain("linear-gradient"); + act(() => view.root.unmount()); + }); + + it("offers the FX button on every audio track", () => { + const view = renderHeader({ + keyframeClip: VOICE, + animations: [], + expanded: false, + isAudioTrack: true, + }); + expect(view.host.querySelector('button[aria-label="Effects"]')).not.toBeNull(); + act(() => view.root.unmount()); + }); + + // A visual track has no chain to open, so the button must not follow the + // header onto every row. + it("withholds the FX button from a non-audio track", () => { + const view = renderHeader({ + keyframeClip: ELEMENT, + animations: [], + expanded: false, + }); + expect(view.host.querySelector('button[aria-label="Effects"]')).toBeNull(); + act(() => view.root.unmount()); + }); + + // A chain belongs to ONE bus, so a track carrying several ungrouped clips + // gets the pointer instead of the FX button — group first, then mix. + it("shows the group pointer on a multi-clip ungrouped audio track", () => { + const opts = { + keyframeClip: VOICE, + trackElements: [VOICE, VOICE_2], + clipCount: 2, + animations: [], + expanded: false, + isAudioTrack: true, + }; + const pointer = (host: HTMLElement) => + host.querySelector('button[aria-label="Effects — group these clips first"]'); + const view = renderHeader(opts); + expect(pointer(view.host)).not.toBeNull(); + // One clip needs no grouping — the real FX button takes its place. + view.rerender({ ...opts, trackElements: [VOICE], clipCount: 1 }); + expect(pointer(view.host)).toBeNull(); + expect(view.host.querySelector('button[aria-label="Effects"]')).not.toBeNull(); + act(() => view.root.unmount()); + }); + + // The header is a 48px column of exactly TWO lines — what the row is, then + // what you can do to it. The group pointer used to render as a sibling of + // both, making a third: 17 + 24 + 24 + gaps in a 48px box, which + // `justify-center` then spilled evenly out of the top and bottom. The name + // rode 10px above its own row and the pointer collided with the row below. + // The header GROWS by AUTOMATION_LANE_H for every open lane, and the lanes + // are absolutely positioned from its top. `justify-center` on the header + // itself therefore centred the two lines in the FULL height, so opening a + // lane pushed the name and its controls down on top of the lane rows. + it("pins the two lines to the top TRACK_H, whatever the header grows to", () => { + const automated: TimelineElement = { + ...VOICE, + automation: JSON.stringify({ + version: 1, + lanes: [{ target: "volume", points: [{ t: 0, v: 1 }] }], + }), + }; + const view = renderHeader({ + keyframeClip: automated, + trackElements: [automated], + clipCount: 1, + animations: [], + expanded: true, + isAudioTrack: true, + }); + const header = view.host.querySelector('[role="rowheader"]'); + const lines = header?.children[0] as HTMLElement | undefined; + expect(lines?.style.height).toBe(`${TRACK_H}px`); + // The lane row is a sibling of the wrapper, not inside it — it stacks + // BELOW the two lines rather than sharing their box. + expect((header?.children.length ?? 0) > 1).toBe(true); + act(() => view.root.unmount()); + }); + + // The header is ONE line: name, clip count, then every control anchored to + // the right edge. It was two — a name line and a control line — which is + // what let a stray third child overflow the 48px box; now there is a single + // row and the controls share one right-aligned group. + it("keeps the name and every control on one line, controls to the right", () => { + const view = renderHeader({ + keyframeClip: VOICE, + trackElements: [VOICE, VOICE_2], + clipCount: 2, + animations: [], + expanded: false, + isAudioTrack: true, + }); + const header = view.host.querySelector('[role="rowheader"]'); + // One TRACK_H-tall wrapper holding one line. + const lines = header?.children[0]; + expect(lines?.children).toHaveLength(1); + // The controls live in a right-anchored group at the end of that line, + // after the name and the clip count — `ml-auto` is what holds the edge. + const line = lines?.children[0]; + const controls = line?.lastElementChild as HTMLElement | null; + expect(controls?.className).toContain("ml-auto"); + expect( + controls?.querySelector('button[aria-label="Effects — group these clips first"]'), + ).not.toBeNull(); + // And the clip count is beside the name, not out with the controls. + expect(controls?.querySelector('[aria-label="2 clips"]')).toBeNull(); + expect(line?.querySelector('[aria-label="2 clips"]')).not.toBeNull(); act(() => view.root.unmount()); }); }); diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index 1879242310..b382ce1259 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -11,22 +11,34 @@ import type { TimelineEditCallbacks } from "./timelineCallbacks"; import { useTimelineEditContextOptional } from "../../contexts/TimelineEditContext"; import { useDomEditActionsContextOptional } from "../../contexts/DomEditContext"; import { mintGroupId } from "../../components/editor/useFxCarveGrouping"; +import { runtimeAudioId } from "../lib/timelineElementHelpers"; import { TimelineFxButton } from "./TimelineFxButton"; import { getTimelinePropertyLanes } from "./TimelinePropertyLanes"; -import { groupAutomationLanes } from "./automationLaneData"; +import { elementFxChain, groupAutomationLanes, isCarveLane } from "./automationLaneData"; import { AUTOMATION_LANE_H } from "./automationLaneHeight"; import { clipTimingStart } from "../../hooks/gsapShared"; -import { LayerDisclosureRow } from "./LayerDisclosureRow"; -import { LABEL_COL_W, LANE_H, getTimelineLaneTop } from "./timelineLayout"; +import { LaneToggleButton, LayerDisclosureRow } from "./LayerDisclosureRow"; +import { LABEL_COL_W, TRACK_H, getTimelineLaneTop } from "./timelineLayout"; import type { TimelineTheme } from "./timelineTheme"; -import { - resolveLaneHeaderState, - type KeyframeNavigationState, - type TimelinePropertyLane, -} from "./trackHeaderLaneState"; -import { valueReadout } from "./trackHeaderLaneValues"; import { trackDisplaySuffix } from "./timelineTrackDisplay"; -import { timelineLogicalRowCellId, timelinePropertyRowId } from "./timelineNavigationIdentity"; +import { AutomationLaneHeaderRow, PropertyGroupHeaderRow } from "./trackHeaderLabelRows"; +import { useMemo } from "react"; + +/** Accent rail + inset marking a row as a group MEMBER, matching the level-2 + * nesting its `aria-level` already reports. */ +const GROUP_MEMBER_RAIL = "#3CE6AC59"; +const GROUP_MEMBER_INDENT = 14; +/** A hair lighter than `gutterBackground`, so a member row reads as sitting + * INSIDE its group rather than beside it. Overlaid rather than hard-coded so + * it tracks whatever the theme's gutter is. */ +const GROUP_MEMBER_TINT = "rgba(255,255,255,0.035)"; + +/** The gutter fill for a row, tinted when it belongs to a group. */ +function gutterFill(base: string, isGroupMember: boolean): string { + return isGroupMember + ? `linear-gradient(${GROUP_MEMBER_TINT}, ${GROUP_MEMBER_TINT}), ${base}` + : base; +} interface TimelineTrackHeaderProps { /** The track's real key: a FRACTIONAL z-order sort value. Routes callbacks; @@ -58,6 +70,8 @@ interface TimelineTrackHeaderProps { currentTime: number; isTrackHidden: boolean; isAudioTrack: boolean; + /** This track is a member of an audio group — indents the row under its header. */ + isGroupMember?: boolean; rovingTargetId?: string | null; theme: TimelineTheme; onToggleClipExpanded: () => void; @@ -69,267 +83,6 @@ interface TimelineTrackHeaderProps { onSeek?: (time: number) => void; } -// Figma layout: prev-keyframe ‹, the add/remove toggle (children), next ›. -function PropertyGroupNavigation({ - navigation, - label, - expandedElement, - onSeek, - children, -}: { - navigation: KeyframeNavigationState; - label: string; - expandedElement: TimelineElement; - onSeek?: (time: number) => void; - children: React.ReactNode; -}) { - // The 12x20px glyph is all the lane row has room for, so the WCAG 24x24 - // target is met with a centered transparent ::before overlay instead of a - // bigger box; focus-visible matches every other control in this header. - const CHEVRON_BUTTON_CLASS = - "relative h-5 w-3 border-0 bg-transparent p-0 text-white/55 hover:text-white disabled:text-white/15 " + - "focus-visible:outline focus-visible:outline-1 focus-visible:outline-[#3CE6AC] " + - "before:absolute before:left-1/2 before:top-1/2 before:h-6 before:w-6 " + - "before:-translate-x-1/2 before:-translate-y-1/2 before:content-['']"; - const seekTo = (keyframe: { percentage: number } | null) => { - if (keyframe) { - onSeek?.(expandedElement.start + (keyframe.percentage / 100) * expandedElement.duration); - } - }; - return ( - - - {children} - - - ); -} - -function PropertyGroupHeaderRow({ - lanesId, - lane, - laneIndex, - isLastLane, - expandedElement, - currentTime, - clipPercentage, - gutterBackground, - columnWidth, - onTogglePropertyGroupKeyframe, - onSeek, - rovingTargetId = null, -}: { - lanesId: string; - lane: TimelinePropertyLane; - laneIndex: number; - isLastLane: boolean; - expandedElement: TimelineElement; - currentTime: number; - clipPercentage: number; - gutterBackground: string; - columnWidth: number; - onTogglePropertyGroupKeyframe?: TimelineEditCallbacks["onTogglePropertyGroupKeyframe"]; - onSeek?: (time: number) => void; - rovingTargetId: string | null; -}) { - const elementId = expandedElement.key ?? expandedElement.id; - const { navigation, values, label, toggleTarget } = resolveLaneHeaderState( - lane, - currentTime, - clipPercentage, - ); - - return ( -
- {/* Tree connector: vertical spine (top-half on the last lane) + branch tick. */} -
- ); -} - -/** - * One envelope's row in the label column. - * - * Named here rather than inside the lane, on the same tree connector the - * keyframe rows use: an automation lane is a child of its clip exactly as a - * property group is, and drawing its name over the envelope put the label on top - * of the curve it describes and scrolled it away from its own row. - */ -function AutomationLaneHeaderRow({ - target, - label, - name, - param, - top, - isLastLane, - gutterBackground, - columnWidth, - onRemove, -}: { - /** The lane the ACTIVE clip draws in this row, or null when it draws none — - * the row belongs to the property, and a clip may be absent from it. */ - target: string | null; - /** The whole thing on one line: the row's identity, its tooltip, and the - * remove button's name. */ - label: string; - /** What the effect is — "Peaking EQ 1.6 kHz". */ - name: string; - /** Which knob the envelope drives. Empty when there is no second line to draw. */ - param: string; - top: number; - isLastLane: boolean; - gutterBackground: string; - columnWidth: number; - onRemove?: (target: string) => void; -}) { - return ( -
- {/* Tree connector, as the keyframe rows draw it: spine down the row, branch - tick at the name's own height. */} -
- ); -} - // fallow-ignore-next-line complexity export function TimelineTrackHeader({ trackNumber, @@ -345,6 +98,7 @@ export function TimelineTrackHeader({ currentTime, isTrackHidden, isAudioTrack, + isGroupMember = false, theme, onToggleClipExpanded, onToggleTrackHidden, @@ -369,26 +123,94 @@ export function TimelineTrackHeader({ // order. `target` is the ACTIVE clip's lane in that row, which is the only one // the remove button can write to; null when the row belongs to its siblings. const activeKey = keyframeClip ? (keyframeClip.key ?? keyframeClip.id) : null; - const automationRows = groupAutomationLanes(trackElements).map((group) => ({ - key: group.key, - label: group.key, - name: group.name, - param: group.param, - target: - group.entries.find((entry) => (entry.element.key ?? entry.element.id) === activeKey)?.lane - .target ?? null, - })); + const groupOwner = useMemo( + () => trackElements.find((element) => element.audioGroup)?.audioGroup, + [trackElements], + ); + const groupLabelForNote = useMemo( + () => trackElements.find((element) => element.audioGroupLabel)?.audioGroupLabel, + [trackElements], + ); + const groupAutomationRaw = useMemo( + () => trackElements.find((element) => element.audioGroupAutomation)?.audioGroupAutomation, + [trackElements], + ); + const groupFxChainRaw = useMemo( + () => trackElements.find((element) => element.audioGroupFxChain)?.audioGroupFxChain, + [trackElements], + ); + // Which parameters this track's GROUP also automates. Gain stages multiply, + // and §5 asks for the explanation rather than leaving the author to wonder + // why two curves they drew sound quieter than either. + const groupAutomatedTargets = useMemo( + () => + new Set( + groupAutomationLanes( + groupOwner + ? [ + { + id: groupOwner, + tag: "audio", + start: 0, + duration: 0, + track: 0, + ...(groupAutomationRaw ? { automation: groupAutomationRaw } : {}), + ...(groupFxChainRaw ? { fxChain: groupFxChainRaw } : {}), + }, + ] + : [], + ).map((lane) => lane.key), + ), + [groupAutomationRaw, groupFxChainRaw, groupOwner], + ); + const revealAudioFx = usePlayerStore((s) => s.setRevealedAudioFxTarget); + /** + * Which element's rack a lane's reveal opens, in the PANEL's id space. + * + * The bare dom id, not the timeline's `sourceFile#domId` composite: the + * property panel identifies its element by `element.id`, so a composite would + * never match — the same boundary `runtimeAudioId` exists for. + */ + const revealElementId = keyframeClip ? runtimeAudioId(keyframeClip) : null; + const automationRows = useMemo( + () => + groupAutomationLanes(trackElements).map((group) => { + const active = group.entries.find( + (entry) => (entry.element.key ?? entry.element.id) === activeKey, + ); + return { + key: group.key, + label: group.key, + name: group.name, + param: group.param, + target: active?.lane.target ?? null, + // Every entry in a row is the same parameter, so the first answers for the + // row when the active clip is absent from it. + isCarve: (() => { + const entry = active ?? group.entries[0]; + return entry ? isCarveLane(entry.lane.target, elementFxChain(entry.element)) : false; + })(), + }; + }), + [activeKey, trackElements], + ); // 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; - // Solo is per-clip/per-group, never per track (design doc §2.2) — this header - // acts on the track's first clip as a pragmatic stand-in for "this track", - // the same simplification the mute button doesn't need to make (it patches - // every clip on the track at once). - const soloTargetId = trackElements[0] ? (trackElements[0].key ?? trackElements[0].id) : null; - const soloed = usePlayerStore((s) => s.soloed); - const toggleSolo = usePlayerStore((s) => s.toggleSolo); + // 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 @@ -400,6 +222,11 @@ export function TimelineTrackHeader({ const singleAudioClip = isAudioTrack && clipCount === 1 && trackElements.length > 0 ? trackElements[0] : null; const isTrackGrouped = trackElements.some((el) => el.audioGroup); + // A video track carries sound the render mixes but preview never routes + // through Web Audio, which is why §1.4 keeps groups audio-only. It still + // needs to be TOLD that, so it earns the button and a refusal. + const isVideoWithAudioTrack = + !isAudioTrack && trackElements.some((el) => el.tag.toLowerCase() === "video"); const writeClipFxChain = (clip: TimelineElement, next: HfAudioFxChain, live: boolean) => { const value = next.nodes.length ? serializeAudioFxChain(next) : null; if (live) onSetElementAttributeLive?.(clip, HF_AUDIO_FX_ATTR, value); @@ -408,74 +235,138 @@ export function TimelineTrackHeader({ const openClipFxRack = (clip: TimelineElement) => { void domEditActions?.handleTimelineElementSelect(clip); }; - const groupUngroupedClips = () => { + // DOM ids, matching the carve picker's other caller — membership is read back + // by `resolveAudioGroups`, which only ever sees the document. A clip with no + // DOM id cannot be a member (resolveAudioGroups skips it), so a track holding + // one cannot be grouped WHOLE — and grouping the rest would quietly leave + // those clips outside the bus, past every fader, mute and effect, while the + // UI showed the track as grouped. The button is withheld instead of acting on + // a subset, which is also why the carve path's loud guard cannot catch this: + // the unresolvable ids were filtered out before the call. + const groupableClipIds = trackElements.map(runtimeAudioId); + const allGroupableClipIds = groupableClipIds.every((id): id is string => id !== null) + ? groupableClipIds + : null; + const canGroupWholeTrack = (allGroupableClipIds?.length ?? 0) >= 2; + const groupUngroupedClips = (label: string) => { const doc = domEditActions?.previewIframeRef.current?.contentDocument; if (!doc || !onGroupClips) return; - const clipIds = trackElements.map((el) => el.key ?? el.id); - void onGroupClips(clipIds, mintGroupId(doc)); + if (!allGroupableClipIds || allGroupableClipIds.length < 2) return; + void onGroupClips(allGroupableClipIds, mintGroupId(doc), label); }; return (
- {!keyframeClip || !disclosable ? ( + {!isKeyframeLayer ? ( <> - el.audioGroupHidden)} - isSoloed={soloTargetId !== null && soloed.has(soloTargetId)} - onToggleSolo={soloTargetId ? (options) => toggleSolo(soloTargetId, options) : undefined} - onToggleTrackHidden={onToggleTrackHidden} - /> - {singleAudioClip && ( - writeClipFxChain(singleAudioClip, next, false)} - onChainPreview={(next) => writeClipFxChain(singleAudioClip, next, true)} - onOpenRack={() => openClipFxRack(singleAudioClip)} + {/* The two lines own exactly TRACK_H, not the whole header. + `justify-center` on the header itself centred them in its FULL + height — which grows by AUTOMATION_LANE_H per open lane — so + opening one pushed the name and its controls down THROUGH the lane + rows below, which are absolutely positioned from the top. */} +
+ + {singleAudioClip && ( + writeClipFxChain(singleAudioClip, next, false)} + onChainPreview={(next) => writeClipFxChain(singleAudioClip, next, true)} + // Muted, an audition is silent — so the hover lifts the mute on + // the running graph and puts it back on the way out, the same + // borrow-and-return it already does with the playhead. + auditionSpans={[singleAudioClip]} + isMuted={isTrackHidden} + onSetMutedLive={(muted) => + onSetElementAttributeLive?.( + singleAudioClip, + "data-hidden", + muted ? "" : null, + ) + } + onOpenRack={() => openClipFxRack(singleAudioClip)} + /> + )} + {clipCount > 1 && + !isTrackGrouped && + (isAudioTrack ? canGroupWholeTrack : isVideoWithAudioTrack) && ( + + )} + {/* The lane disclosure, on the row's own layout rather than by + swapping it for a keyframe-layer row. */} + {disclosable && ( + + )} + + } /> - )} - {isAudioTrack && clipCount > 1 && !isTrackGrouped && ( - - )} +
) : ( <> 1 - ? `Track${trackDisplaySuffix(trackDisplayNumber)}` - : (keyframeClip.label ?? keyframeClip.domId ?? keyframeClip.id) - } + name={laneOwnerName} clipCount={clipCount} isExpanded={isExpanded} - gutterBackground={theme.gutterBackground} + gutterBackground={gutterFill(theme.gutterBackground, isGroupMember)} columnWidth={showTrackLabel ? LABEL_COL_W : contentOrigin} lanesId={lanesId} onToggleClipExpanded={onToggleClipExpanded} @@ -489,57 +380,86 @@ export function TimelineTrackHeader({ hidden={isTrackHidden} trackNumber={trackNumber} trackDisplayNumber={trackDisplayNumber} - visible - isAudioTrack={isAudioTrack} + // Audio: only while hidden — see the plain header. + visible={!isAudioTrack || isTrackHidden} 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) => { + // Bound outside the closure so it narrows: `row.target` is null on a + // row the active clip is absent from. + const revealTarget = row.target; + return ( + { + // Select FIRST: the rack is the property panel's view of + // the selected element, so a reveal aimed at an unselected + // clip lands on a panel that says "Nothing selected". The + // request survives the selection — it is stored, not an + // event — so the rack consumes it as it mounts. + openClipFxRack(keyframeClip); + revealAudioFx({ + elementKey: revealElementId, + automationTarget: revealTarget, + }); + } + : undefined + } + /> + ); + })}
); } diff --git a/packages/studio/src/player/components/TimelineTrackPlainHeader.test.tsx b/packages/studio/src/player/components/TimelineTrackPlainHeader.test.tsx index a9a5f8de62..b508c3e349 100644 --- a/packages/studio/src/player/components/TimelineTrackPlainHeader.test.tsx +++ b/packages/studio/src/player/components/TimelineTrackPlainHeader.test.tsx @@ -3,12 +3,15 @@ /** * The visibility control's accessible contract. * - * The audio wording ran behind the `audio-track-mute` canary at 0%, so it had - * never rendered in any suite: it returned "Muted" / "Mute", which named the - * CURRENT state rather than the action, and dropped the track suffix so every - * audio row shared one accessible name. Music plus VO is the ordinary case, so - * that is two identical buttons. Pinned here because the label and the icon are - * the whole identity of this control. + * The wording ran behind the `audio-track-mute` canary at 0%, so it had never + * rendered in any suite: it returned "Muted" / "Mute", which named the CURRENT + * state rather than the action, and dropped the track suffix so every audio row + * shared one accessible name. Pinned here because the label, the icon and the + * callback's arguments are the whole identity of this control. + * + * The mute presentation itself is gone (`remove mute and solo from tracks and + * groups`): the control is the visibility eye it always was, and an audio row + * renders it only when already hidden, which is the way back out. */ import React, { act } from "react"; @@ -24,7 +27,7 @@ afterEach(() => { function renderButton(props: { hidden: boolean; - isAudioTrack?: boolean; + visible?: boolean; trackDisplayNumber: number | null; }): { host: HTMLElement; unmount: () => void; onToggle: ReturnType } { const host = document.createElement("div"); @@ -37,8 +40,7 @@ function renderButton(props: { hidden: props.hidden, trackNumber: 7, trackDisplayNumber: props.trackDisplayNumber, - visible: true, - isAudioTrack: props.isAudioTrack, + visible: props.visible ?? true, onToggle, }), ), @@ -49,31 +51,7 @@ function renderButton(props: { const labelOf = (host: HTMLElement) => host.querySelector("button")?.getAttribute("aria-label"); describe("VisibilityButton", () => { - it("names the action, not the state, on an audible audio track", () => { - const view = renderButton({ hidden: false, isAudioTrack: true, trackDisplayNumber: 2 }); - expect(labelOf(view.host)).toBe("Mute track 2"); - view.unmount(); - }); - - // The half that was wrong: a muted row read "Muted", so nothing told a - // screen-reader user that activating it would unmute. - it("promises the un-mute when the audio track is already muted", () => { - const view = renderButton({ hidden: true, isAudioTrack: true, trackDisplayNumber: 2 }); - expect(labelOf(view.host)).toBe("Unmute track 2"); - view.unmount(); - }); - - it("keeps each audio row's name unique, so two tracks are distinguishable", () => { - const first = renderButton({ hidden: false, isAudioTrack: true, trackDisplayNumber: 1 }); - const second = renderButton({ hidden: false, isAudioTrack: true, trackDisplayNumber: 3 }); - expect(labelOf(first.host)).toBe("Mute track 1"); - expect(labelOf(second.host)).toBe("Mute track 3"); - expect(labelOf(first.host)).not.toBe(labelOf(second.host)); - first.unmount(); - second.unmount(); - }); - - it("still says Hide/Show on a visual track", () => { + it("names the action, not the state", () => { const shown = renderButton({ hidden: false, trackDisplayNumber: 2 }); expect(labelOf(shown.host)).toBe("Hide track 2"); shown.unmount(); @@ -82,15 +60,34 @@ describe("VisibilityButton", () => { hiddenRow.unmount(); }); + it("keeps each row's name unique, so two tracks are distinguishable", () => { + const first = renderButton({ hidden: false, trackDisplayNumber: 1 }); + const second = renderButton({ hidden: false, trackDisplayNumber: 3 }); + expect(labelOf(first.host)).toBe("Hide track 1"); + expect(labelOf(second.host)).toBe("Hide track 3"); + expect(labelOf(first.host)).not.toBe(labelOf(second.host)); + first.unmount(); + second.unmount(); + }); + // The callback acts on the REAL track key; the display row rides along so the // undo-history label announces the same row this button just did, instead of // re-deriving it from an ordering that has no group anchors in it. it("toggles the real track number and passes the row it announced", () => { - const view = renderButton({ hidden: false, isAudioTrack: true, trackDisplayNumber: 2 }); + const view = renderButton({ hidden: false, trackDisplayNumber: 2 }); view.host.querySelector("button")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); expect(view.onToggle).toHaveBeenCalledWith(7, true, 2); view.unmount(); }); + + // `visible={false}` renders a SPACER, not nothing: every row's control + // columns have to line up whether or not this row offers the control. + it("holds the column with a spacer when withheld", () => { + const view = renderButton({ hidden: false, visible: false, trackDisplayNumber: 2 }); + expect(view.host.querySelector("button")).toBeNull(); + expect(view.host.querySelector("span")).toBeTruthy(); + view.unmount(); + }); }); describe("PlainTrackHeader", () => { @@ -98,7 +95,6 @@ describe("PlainTrackHeader", () => { const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); - const onToggleSolo = vi.fn(); act(() => root.render( React.createElement(PlainTrackHeader, { @@ -110,52 +106,34 @@ describe("PlainTrackHeader", () => { isAudioTrack: true, onToggleTrackHidden: vi.fn(), showTrackLabel: true, - isGroupMuted: false, - isSoloed: false, - onToggleSolo, ...overrides, }), ), ); - return { host, unmount: () => act(() => root.unmount()), onToggleSolo }; + return { host, unmount: () => act(() => root.unmount()) }; } - const soloButton = (host: HTMLElement) => - host.querySelector('button[aria-label="Hear only this"]'); - const labelSpan = (host: HTMLElement) => host.querySelector("span.min-w-0"); - - it("offers solo on an audio track and reports its pressed state", () => { - const view = renderHeader({ isSoloed: true }); - expect(soloButton(view.host)?.getAttribute("aria-pressed")).toBe("true"); - view.unmount(); - }); - - it("withholds solo from a visual track", () => { - const view = renderHeader({ isAudioTrack: false }); - expect(soloButton(view.host)).toBeNull(); - view.unmount(); - }); - - // A group-muted member is silent without being hidden itself, so the strike - // is the only thing that says so — and the title has to explain why, since a - // user who never touched THIS row's mute is looking for the reason. - it("strikes the label through when the row's own mute is on", () => { - const view = renderHeader({ isTrackHidden: true }); - expect(labelSpan(view.host)?.className).toContain("line-through"); - view.unmount(); - }); - - it("strikes it through for a group mute too, and says which", () => { - const view = renderHeader({ isGroupMuted: true }); - expect(labelSpan(view.host)?.className).toContain("line-through"); - expect(labelSpan(view.host)?.getAttribute("title")).toBe("Voiceover (group muted)"); - view.unmount(); - }); - - it("leaves an audible row unstruck", () => { + // A row that says what it is with a speaker does not also need the hide + // affordance sitting in the eye's slot. + it("withholds the eye from an audible audio track", () => { const view = renderHeader(); - expect(labelSpan(view.host)?.className).not.toContain("line-through"); - expect(labelSpan(view.host)?.getAttribute("title")).toBe("Voiceover"); + expect(view.host.querySelector("button")).toBeNull(); + view.unmount(); + }); + + // Withholding it unconditionally withheld the only way back: `data-hidden` + // silences the clip in preview and drops it from the render, and nothing else + // writes it, so a track hidden by hand or by "Hide all" was silent with no + // control anywhere to restore it. + it("offers it back once the audio track is hidden", () => { + const view = renderHeader({ isTrackHidden: true }); + expect(labelOf(view.host)).toBe("Show track 1"); + view.unmount(); + }); + + it("keeps the eye on a visual track", () => { + const view = renderHeader({ isAudioTrack: false }); + expect(labelOf(view.host)).toBe("Hide track 1"); view.unmount(); }); }); diff --git a/packages/studio/src/player/components/TimelineTrackPlainHeader.tsx b/packages/studio/src/player/components/TimelineTrackPlainHeader.tsx index cbff6c338e..d8d9b3caf6 100644 --- a/packages/studio/src/player/components/TimelineTrackPlainHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackPlainHeader.tsx @@ -1,29 +1,18 @@ -import { Eye, EyeSlash, SpeakerHigh, SpeakerSlash } from "@phosphor-icons/react"; +import type React from "react"; +import { Eye, EyeSlash } from "@phosphor-icons/react"; import { Music } from "../../icons/SystemIcons"; -import { TimelineSoloButton } from "./TimelineSoloButton"; import type { TimelineEditCallbacks } from "./timelineCallbacks"; import { TrackClipCount } from "./TrackClipCount"; import { trackDisplaySuffix } from "./timelineTrackDisplay"; -/** - * Audio tracks say "Mute", not "Hide" — the eye IS mute for sound-only rows. - * - * Action-phrased and per-track, like the visual branch: `hidden` here is the - * CURRENT state, so the name has to promise the opposite. It read "Muted" / - * "Mute" until the rollout, which named the state instead of the action (a - * screen-reader user could not tell that activating an already-muted row would - * unmute it) and dropped `suffix`, so every audio row shared one accessible - * name — music plus VO being the ordinary case. The wording matches the undo - * entry `timelineTrackVisibility` writes for the same click, where `hidden` is - * the INCOMING state and the two therefore read inverted. - */ -function visibilityButtonLabel(showAsMute: boolean, hidden: boolean, suffix: string): string { - if (showAsMute) return hidden ? `Unmute track${suffix}` : `Mute track${suffix}`; +// Hide, plainly. The speaker variant was the mute presentation; with mute gone +// this is the visibility eye it always was, and audio rows do not render it. +function visibilityButtonLabel(hidden: boolean, suffix: string): string { return hidden ? `Show track${suffix}` : `Hide track${suffix}`; } -function visibilityButtonIcon(showAsMute: boolean, hidden: boolean) { - const Icon = showAsMute ? (hidden ? SpeakerSlash : SpeakerHigh) : hidden ? EyeSlash : Eye; +function visibilityButtonIcon(hidden: boolean) { + const Icon = hidden ? EyeSlash : Eye; return