From 80e880d1a9c1d897ffd44c2bce984af92cf7b648 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 4 Aug 2026 10:40:58 -0700 Subject: [PATCH 1/2] feat(studio): automate a parameter without reloading the preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write path and the volume half of the panel surface. **A commit that persists without reloading.** For attributes the runtime applies to the live graph itself — an FX chain, its automation — a reload would only interrupt playback to reach the state the preview already has. `skipRefresh` and `refreshAfter` were already independent options; this exposes the combination that skips the reload but still re-reads the selection. Both halves are needed, and they were fighting each other. Without the reload, audio no longer chops on an edit. Without the resync, the panel keeps reading the selection snapshot it was built with, so a second edit computes from a pre-edit value and appears to do nothing — deleting one effect made every later delete a no-op. `handleDomAttributeLiveCommit` is untouched and still used for knob dragging, where a per-move re-render is exactly what you do not want. **Volume.** An automated track's slider is disabled, since a level set there would be overwritten by the envelope on the next tick, and the toggle beside it adds or deletes the lane. Adding seeds it with a single point at the level the slider already shows, so automating a track never changes how loud it is. **One shared reader** for both panel sections, which is what surfaced that resolving against an absent chain would have deleted every FX lane the moment someone automated a volume: the volume section does not parse the chain, so "no chain" now means "do not resolve" rather than "drop what cannot be resolved". The toggle itself lives with the FX controls it is shared with, and says `Automated` / `Automate` through the studio's own Tooltip rather than a native browser hover. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../src/components/StudioRightPanel.tsx | 2 + .../editor/propertyPanelAutomation.ts | 80 +++++++++++ .../editor/propertyPanelFlatMediaSection.tsx | 46 +++++-- .../editor/propertyPanelFlatProps.ts | 1 + .../editor/propertyPanelFxControls.tsx | 130 +++++++++++++++--- .../components/editor/propertyPanelTypes.ts | 5 + .../editor/useVolumeAutomation.test.tsx | 107 ++++++++++++++ .../components/editor/useVolumeAutomation.ts | 44 ++++++ .../studio/src/contexts/DomEditContext.tsx | 4 + .../src/hooks/useDomEditAttributeCommits.ts | 22 +++ .../studio/src/hooks/useDomEditCommits.ts | 2 + .../studio/src/hooks/useDomEditSession.ts | 2 + .../studio/src/hooks/useDomEditTextCommits.ts | 2 + 13 files changed, 422 insertions(+), 25 deletions(-) create mode 100644 packages/studio/src/components/editor/propertyPanelAutomation.ts create mode 100644 packages/studio/src/components/editor/useVolumeAutomation.test.tsx create mode 100644 packages/studio/src/components/editor/useVolumeAutomation.ts diff --git a/packages/studio/src/components/StudioRightPanel.tsx b/packages/studio/src/components/StudioRightPanel.tsx index 68088311f3..8ee3b9c88d 100644 --- a/packages/studio/src/components/StudioRightPanel.tsx +++ b/packages/studio/src/components/StudioRightPanel.tsx @@ -124,6 +124,7 @@ export function StudioRightPanel({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomPathOffsetCommit, @@ -361,6 +362,7 @@ export function StudioRightPanel({ onSetAttribute={handleDomAttributeCommit} onSetAttributes={handleDomAttributesCommit} onSetAttributeLive={handleDomAttributeLiveCommit} + onSetAttributeQuiet={handleDomAttributeQuietCommit} onApplyColorGradingScope={handleApplyColorGradingScope} onSetHtmlAttribute={handleDomHtmlAttributeCommit} onRemoveBackground={handleRemoveBackground} diff --git a/packages/studio/src/components/editor/propertyPanelAutomation.ts b/packages/studio/src/components/editor/propertyPanelAutomation.ts new file mode 100644 index 0000000000..d940eedfab --- /dev/null +++ b/packages/studio/src/components/editor/propertyPanelAutomation.ts @@ -0,0 +1,80 @@ +/** + * Reading and editing an element's automation from the property panel. + * + * Shared by the audio FX group (per-effect parameters) and the media section + * (track volume), so both agree on what "automated" means and both write the + * attribute the same way. + */ + +import { + HF_AUDIO_AUTOMATION_ATTR, + parseAutomation, + resolveAutomation, + resolveAutomationRange, + serializeAutomation, + type HfAutomation, +} from "@hyperframes/core/audio-automation"; +import type { HfAudioFxChain } from "@hyperframes/core/audio-fx"; + +const EMPTY: HfAutomation = { version: 1, lanes: [] }; + +/** + * The element's automation as the panel should treat it. + * + * Pass the chain to have it bound: a lane whose effect has been deleted is then + * dropped rather than reported as automating something. Pass `undefined` when + * the caller genuinely does not know the chain — the volume section does not + * parse it — and every lane is preserved instead. + * + * That distinction matters because callers write this value straight back to the + * attribute. Resolving against a chain that was merely unavailable would delete + * every FX lane the moment someone automated the volume. + * + * An unreadable attribute reads as no automation rather than breaking the panel; + * it is left untouched until the author changes something. + */ +export function readPanelAutomation( + raw: string | undefined, + chain: HfAudioFxChain | undefined, +): HfAutomation { + if (!raw) return EMPTY; + try { + const parsed = parseAutomation(raw); + return chain ? resolveAutomation(parsed, chain) : parsed; + } catch { + return EMPTY; + } +} + +/** Targets the element currently automates. */ +export function automatedTargetsOf(automation: HfAutomation): Set { + return new Set(automation.lanes.map((lane) => lane.target)); +} + +/** + * Add a lane for `target`, seeded with a single point at `current`. + * + * One point is a constant, so switching a parameter to an envelope does not + * change the sound — it only moves where the value comes from. The author then + * shapes it in the timeline. + */ +export function withSeededLane( + automation: HfAutomation, + target: string, + current: number, +): HfAutomation { + if (automation.lanes.some((lane) => lane.target === target)) return automation; + return { version: 1, lanes: [...automation.lanes, { target, points: [{ t: 0, v: current }] }] }; +} + +/** Drop one lane, handing its value back to the panel control. */ +export function withoutLane(automation: HfAutomation, target: string): HfAutomation { + return { version: 1, lanes: automation.lanes.filter((lane) => lane.target !== target) }; +} + +/** The attribute value for an automation set; empty when nothing is automated. */ +export function automationAttrValue(automation: HfAutomation): string { + return automation.lanes.length > 0 ? serializeAutomation(automation) : ""; +} + +export { HF_AUDIO_AUTOMATION_ATTR, resolveAutomationRange }; diff --git a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx index 1850a48905..997483e136 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx @@ -12,6 +12,7 @@ import { } from "./propertyPanelHelpers"; import { FlatSelectRow, FlatSlider } from "./propertyPanelFlatPrimitives"; import { FlatToggle } from "./propertyPanelFlatToggle"; +import { AutomationToggle } from "./propertyPanelFxControls"; // fallow-ignore-next-line complexity export function FlatMediaSection({ @@ -22,6 +23,9 @@ export function FlatMediaSection({ onSetAttribute, onSetHtmlAttribute, onRemoveBackground, + volumeAutomated, + onAutomateVolume, + onRemoveVolumeAutomation, }: { projectDir: string | null; element: DomEditSelection; @@ -29,6 +33,10 @@ export function FlatMediaSection({ onSetStyle: (prop: string, value: string) => void | Promise; onSetAttribute: (attr: string, value: string) => void | Promise; onSetHtmlAttribute: (attr: string, value: string | null) => void | Promise; + /** A volume lane in the timeline drives the level; the slider cannot. */ + volumeAutomated?: boolean; + onAutomateVolume?: () => void; + onRemoveVolumeAutomation?: () => void; onRemoveBackground?: ( inputPath: string, options: { @@ -197,15 +205,35 @@ export function FlatMediaSection({ )} {(isVideo || isAudio) && ( <> - void onSetAttribute("volume", formatNumericValue(next / 100))} - /> + {/* The slider is disabled while a lane owns the level: a value set + here would be overwritten by the envelope on the next tick. The + toggle beside it carries the tooltip. */} +
+
+ void onSetAttribute("volume", formatNumericValue(next / 100))} + /> +
+ onAutomateVolume() : undefined} + onRemoveAutomation={ + onRemoveVolumeAutomation ? () => onRemoveVolumeAutomation() : undefined + } + /> +
+ + + ); +} + +export function FxParamRow({ + param, + value, + onChange, + onCommit, + disabled, + automated, + onAutomate, + onRemoveAutomation, +}: ParamRowProps) { // While dragging, the slider is driven locally. Waiting for the value to come // back through the element attribute makes the control feel laggy and fights // the pointer. @@ -105,9 +163,19 @@ export function FxParamRow({ param, value, onChange, onCommit, disabled }: Param const numeric = typeof shown === "number" ? shown : Number(shown); const current = Number.isFinite(numeric) ? numeric : param.default; + const locked = Boolean(disabled) || Boolean(automated); + return ( - ); } @@ -157,10 +232,24 @@ interface FxParamsProps { onChange(params: HfAudioFxParamValues): void; onCommit?(params: HfAudioFxParamValues): void; disabled?: boolean; + /** Parameter keys this effect currently has a lane for. */ + automatedKeys?: ReadonlySet; + /** Absent when the effect cannot be automated at all, or nothing can write. */ + onAutomate?(key: string): void; + onRemoveAutomation?(key: string): void; } /** Every knob the effect declares, in registry order. */ -export function FxParams({ def, params, onChange, onCommit, disabled }: FxParamsProps) { +export function FxParams({ + def, + params, + onChange, + onCommit, + disabled, + automatedKeys, + onAutomate, + onRemoveAutomation, +}: FxParamsProps) { const set = useCallback( (key: string, value: number | string) => onChange({ ...params, [key]: value }), [params, onChange], @@ -171,16 +260,25 @@ export function FxParams({ def, params, onChange, onCommit, disabled }: FxParams ); return (
- {def.params.map((p) => ( - - ))} + {def.params.map((p) => { + // Only a parameter the registry marks automatable has an AudioParam + // behind it for an envelope to write to. + const canAutomate = p.kind === "number" && p.automatable === true; + const automated = automatedKeys?.has(p.key) ?? false; + return ( + + ); + })}
); } diff --git a/packages/studio/src/components/editor/propertyPanelTypes.ts b/packages/studio/src/components/editor/propertyPanelTypes.ts index 935ceb3f67..2756d0d00a 100644 --- a/packages/studio/src/components/editor/propertyPanelTypes.ts +++ b/packages/studio/src/components/editor/propertyPanelTypes.ts @@ -58,6 +58,11 @@ export interface PropertyPanelProps { value: string | null, onSettled?: (ok: boolean) => void, ) => void | Promise; + /** Persists without reloading the preview, but re-reads the selection after — + * for attributes the runtime applies to the live graph itself, where a reload + * would only interrupt playback, and where the panel still has to see the + * value it just wrote to compute the next edit from. */ + onSetAttributeQuiet?: (attr: string, value: string | null) => void | Promise; onApplyColorGradingScope?: ( scope: "source-file" | "project", value: string | null, diff --git a/packages/studio/src/components/editor/useVolumeAutomation.test.tsx b/packages/studio/src/components/editor/useVolumeAutomation.test.tsx new file mode 100644 index 0000000000..580bdbc25f --- /dev/null +++ b/packages/studio/src/components/editor/useVolumeAutomation.test.tsx @@ -0,0 +1,107 @@ +// @vitest-environment happy-dom +import { act } from "react"; +import { describe, expect, it, vi } from "vitest"; +import { createRoot } from "react-dom/client"; +import { useVolumeAutomation, type VolumeAutomationBinding } from "./useVolumeAutomation"; +import type { DomEditSelection } from "./domEditingTypes"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +function bind(dataAttributes: Record) { + const onSetAttribute = vi.fn(); + const captured: { current: VolumeAutomationBinding | null } = { current: null }; + function Probe() { + captured.current = useVolumeAutomation( + { dataAttributes } as unknown as DomEditSelection, + onSetAttribute, + ); + return null; + } + const host = document.createElement("div"); + document.body.append(host); + act(() => { + createRoot(host).render(); + }); + if (!captured.current) throw new Error("hook never ran"); + return { binding: captured.current, onSetAttribute }; +} + +const volumeLane = (v: number) => + JSON.stringify({ version: 1, lanes: [{ target: "volume", points: [{ t: 0, v }] }] }); + +describe("useVolumeAutomation", () => { + it("reports an unautomated track", () => { + expect(bind({ volume: "0.55" }).binding.volumeAutomated).toBe(false); + }); + + it("reports a track with a volume lane", () => { + expect(bind({ volume: "0.55", automation: volumeLane(0.2) }).binding.volumeAutomated).toBe( + true, + ); + }); + + it("does not count an FX lane as automating the volume", () => { + const automation = JSON.stringify({ + version: 1, + lanes: [{ target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }], + }); + expect(bind({ volume: "0.55", automation }).binding.volumeAutomated).toBe(false); + }); + + it("seeds a new lane at the level the slider already shows", () => { + // Automating a track must not change how loud it is. + const { binding, onSetAttribute } = bind({ volume: "0.55" }); + act(() => binding.onAutomateVolume()); + expect(onSetAttribute).toHaveBeenCalledWith( + "data-automation", + JSON.stringify({ version: 1, lanes: [{ target: "volume", points: [{ t: 0, v: 0.55 }] }] }), + ); + }); + + it("treats a missing data-volume as unity", () => { + const { binding, onSetAttribute } = bind({}); + act(() => binding.onAutomateVolume()); + expect(JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes[0].points[0].v).toBe(1); + }); + + it("keeps FX lanes when adding the volume one", () => { + const automation = JSON.stringify({ + version: 1, + lanes: [{ target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }], + }); + const { binding, onSetAttribute } = bind({ volume: "0.4", automation }); + act(() => binding.onAutomateVolume()); + expect( + JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes.map( + (l: { target: string }) => l.target, + ), + ).toEqual(["fx.n1.frequency", "volume"]); + }); + + it("deletes only the volume lane", () => { + const automation = JSON.stringify({ + version: 1, + lanes: [ + { target: "volume", points: [{ t: 0, v: 0.2 }] }, + { target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }, + ], + }); + const { binding, onSetAttribute } = bind({ volume: "0.4", automation }); + act(() => binding.onRemoveVolumeAutomation()); + expect( + JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes.map( + (l: { target: string }) => l.target, + ), + ).toEqual(["fx.n1.frequency"]); + }); + + it("clears the attribute when the volume lane was the only one", () => { + const { binding, onSetAttribute } = bind({ volume: "0.4", automation: volumeLane(0.2) }); + act(() => binding.onRemoveVolumeAutomation()); + expect(onSetAttribute).toHaveBeenCalledWith("data-automation", ""); + }); + + it("reads an unreadable attribute as no automation", () => { + expect(bind({ volume: "0.55", automation: "{not json" }).binding.volumeAutomated).toBe(false); + }); +}); diff --git a/packages/studio/src/components/editor/useVolumeAutomation.ts b/packages/studio/src/components/editor/useVolumeAutomation.ts new file mode 100644 index 0000000000..477d98a0b9 --- /dev/null +++ b/packages/studio/src/components/editor/useVolumeAutomation.ts @@ -0,0 +1,44 @@ +/** + * The volume lane's state and edits for the media section. + * + * Volume lives in a different panel section from the FX chain, but is automated + * the same way, so it reads and writes through the same helper the FX group uses + * rather than a second interpretation of the attribute. + */ + +import { VOLUME_TARGET } from "@hyperframes/core/audio-automation"; +import type { DomEditSelection } from "./domEditingTypes"; +import { + automationAttrValue, + HF_AUDIO_AUTOMATION_ATTR, + readPanelAutomation, + withoutLane, + withSeededLane, +} from "./propertyPanelAutomation"; + +export interface VolumeAutomationBinding { + volumeAutomated: boolean; + onAutomateVolume: () => void; + onRemoveVolumeAutomation: () => void; +} + +export function useVolumeAutomation( + element: DomEditSelection, + onSetAttribute: (attr: string, value: string) => void | Promise, +): VolumeAutomationBinding { + // The chain is not needed to resolve a volume lane — volume is always a valid + // target — so this deliberately does not parse it. + const automation = readPanelAutomation(element.dataAttributes?.["automation"], undefined); + const write = (next: Parameters[0]): void => { + void onSetAttribute(HF_AUDIO_AUTOMATION_ATTR, automationAttrValue(next)); + }; + const current = Number(element.dataAttributes?.["volume"] ?? "1"); + return { + volumeAutomated: automation.lanes.some((lane) => lane.target === VOLUME_TARGET), + // Seeded at the level the slider already shows, so automating the track does + // not change how loud it is. + onAutomateVolume: () => + write(withSeededLane(automation, VOLUME_TARGET, Number.isFinite(current) ? current : 1)), + onRemoveVolumeAutomation: () => write(withoutLane(automation, VOLUME_TARGET)), + }; +} diff --git a/packages/studio/src/contexts/DomEditContext.tsx b/packages/studio/src/contexts/DomEditContext.tsx index f89e27e915..b12134fd84 100644 --- a/packages/studio/src/contexts/DomEditContext.tsx +++ b/packages/studio/src/contexts/DomEditContext.tsx @@ -15,6 +15,7 @@ export interface DomEditActionsValue extends Pick< | "handleDomStyleCommit" | "handleDomAttributeCommit" | "handleDomAttributeLiveCommit" + | "handleDomAttributeQuietCommit" | "handleDomHtmlAttributeCommit" | "handleDomAttributesCommit" | "handleDomPathOffsetCommit" @@ -139,6 +140,7 @@ export function DomEditProvider({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomPathOffsetCommit, @@ -227,6 +229,7 @@ export function DomEditProvider({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomPathOffsetCommit, @@ -296,6 +299,7 @@ export function DomEditProvider({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomPathOffsetCommit, diff --git a/packages/studio/src/hooks/useDomEditAttributeCommits.ts b/packages/studio/src/hooks/useDomEditAttributeCommits.ts index 0c3e7a39c7..d4e76c2cb8 100644 --- a/packages/studio/src/hooks/useDomEditAttributeCommits.ts +++ b/packages/studio/src/hooks/useDomEditAttributeCommits.ts @@ -284,6 +284,27 @@ export function useDomEditAttributeCommits({ [commitDataAttribute], ); + /** + * Persist without reloading the preview, but re-read the selection afterwards. + * + * For attributes the runtime applies to the live graph itself — an audio FX + * chain, its automation — a reload would only interrupt playback to reach the + * state the preview already has. The resync is still needed: without it the + * panel keeps reading the selection snapshot it was built with, so a second + * edit computes from a pre-edit value and appears to do nothing. + */ + const handleDomAttributeQuietCommit = useCallback( + async (attr: string, value: string | null) => { + await commitDataAttribute(attr, value, { + label: `Edit ${attr.replace(/^(data-)?/, "").replace(/-/g, " ")}`, + coalescePrefix: "attr-quiet", + skipRefresh: true, + refreshAfter: true, + }); + }, + [commitDataAttribute], + ); + const handleDomHtmlAttributeCommit = useCallback( async (attr: string, value: string | null) => { if (!domEditSelection) return; @@ -343,6 +364,7 @@ export function useDomEditAttributeCommits({ return { handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, }; diff --git a/packages/studio/src/hooks/useDomEditCommits.ts b/packages/studio/src/hooks/useDomEditCommits.ts index 7a02be735d..12192452f4 100644 --- a/packages/studio/src/hooks/useDomEditCommits.ts +++ b/packages/studio/src/hooks/useDomEditCommits.ts @@ -374,6 +374,7 @@ export function useDomEditCommits({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, @@ -437,6 +438,7 @@ export function useDomEditCommits({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, diff --git a/packages/studio/src/hooks/useDomEditSession.ts b/packages/studio/src/hooks/useDomEditSession.ts index efec011f95..02ecbe4f78 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -222,6 +222,7 @@ export function useDomEditSession({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, @@ -484,6 +485,7 @@ export function useDomEditSession({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomPathOffsetCommit: handleGsapAwarePathOffsetCommit, diff --git a/packages/studio/src/hooks/useDomEditTextCommits.ts b/packages/studio/src/hooks/useDomEditTextCommits.ts index 7111356279..dbfe9551db 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.ts +++ b/packages/studio/src/hooks/useDomEditTextCommits.ts @@ -148,6 +148,7 @@ export function useDomEditTextCommits({ const { handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, } = useDomEditAttributeCommits({ @@ -474,6 +475,7 @@ export function useDomEditTextCommits({ handleDomStyleCommit, handleDomAttributeCommit, handleDomAttributeLiveCommit, + handleDomAttributeQuietCommit, handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, From a2df6c6f7e36edb63b16eebd923875acb2acc83e Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 4 Aug 2026 15:06:55 -0700 Subject: [PATCH 2/2] fix(studio): stop the last two automation writes reloading the preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The quiet commit added here was only used by the FX group. Two writers still went through the refreshing one, so they reloaded the preview and restarted every playing track — the exact chop the live write during a drag exists to avoid: - releasing a dragged breakpoint, so the audio hitched at the end of every point you moved; - clicking the volume toggle, while the same click on an effect parameter was already silent. Both are quiet now: still persisted, still resyncing the selection so a following edit computes from the value just written. Also fixes the seeded volume. `Number(dataAttributes.volume ?? "1")` is 0 for an attribute that is present but empty, so automating such a track started its lane at silence while the engine read the same empty value as unity. Co-Authored-By: Claude Opus 5 (1M context) --- .fallowrc.jsonc | 7 ++++ .../editor/useVolumeAutomation.test.tsx | 41 +++++++++++++------ .../components/editor/useVolumeAutomation.ts | 13 ++++-- .../player/components/useAutomationLanes.ts | 6 ++- 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/.fallowrc.jsonc b/.fallowrc.jsonc index 1121f1b360..1822f7a7f6 100644 --- a/.fallowrc.jsonc +++ b/.fallowrc.jsonc @@ -176,6 +176,13 @@ "withLane", ], }, + // propertyPanelAutomation is the shared reader for both panel sections; the + // FX group that consumes these two lands one PR upstack, so a per-PR audit + // against the merge base sees them as unused. + { + "file": "packages/studio/src/components/editor/propertyPanelAutomation.ts", + "exports": ["automatedTargetsOf", "resolveAutomationRange"], + }, // drawElementService is the bottom of the fast-capture Graphite stack // (#1917): its consumers (frameCapture in #1919) land two PRs upstack, so // a per-PR audit diffing against the merge base sees these exports as diff --git a/packages/studio/src/components/editor/useVolumeAutomation.test.tsx b/packages/studio/src/components/editor/useVolumeAutomation.test.tsx index 580bdbc25f..c6b4e4713b 100644 --- a/packages/studio/src/components/editor/useVolumeAutomation.test.tsx +++ b/packages/studio/src/components/editor/useVolumeAutomation.test.tsx @@ -8,12 +8,12 @@ import type { DomEditSelection } from "./domEditingTypes"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; function bind(dataAttributes: Record) { - const onSetAttribute = vi.fn(); + const onSetAttributeQuiet = vi.fn(); const captured: { current: VolumeAutomationBinding | null } = { current: null }; function Probe() { captured.current = useVolumeAutomation( { dataAttributes } as unknown as DomEditSelection, - onSetAttribute, + onSetAttributeQuiet, ); return null; } @@ -23,7 +23,7 @@ function bind(dataAttributes: Record) { createRoot(host).render(); }); if (!captured.current) throw new Error("hook never ran"); - return { binding: captured.current, onSetAttribute }; + return { binding: captured.current, onSetAttributeQuiet }; } const volumeLane = (v: number) => @@ -50,18 +50,18 @@ describe("useVolumeAutomation", () => { it("seeds a new lane at the level the slider already shows", () => { // Automating a track must not change how loud it is. - const { binding, onSetAttribute } = bind({ volume: "0.55" }); + const { binding, onSetAttributeQuiet } = bind({ volume: "0.55" }); act(() => binding.onAutomateVolume()); - expect(onSetAttribute).toHaveBeenCalledWith( + expect(onSetAttributeQuiet).toHaveBeenCalledWith( "data-automation", JSON.stringify({ version: 1, lanes: [{ target: "volume", points: [{ t: 0, v: 0.55 }] }] }), ); }); it("treats a missing data-volume as unity", () => { - const { binding, onSetAttribute } = bind({}); + const { binding, onSetAttributeQuiet } = bind({}); act(() => binding.onAutomateVolume()); - expect(JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes[0].points[0].v).toBe(1); + expect(JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes[0].points[0].v).toBe(1); }); it("keeps FX lanes when adding the volume one", () => { @@ -69,10 +69,10 @@ describe("useVolumeAutomation", () => { version: 1, lanes: [{ target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }], }); - const { binding, onSetAttribute } = bind({ volume: "0.4", automation }); + const { binding, onSetAttributeQuiet } = bind({ volume: "0.4", automation }); act(() => binding.onAutomateVolume()); expect( - JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes.map( + JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes.map( (l: { target: string }) => l.target, ), ).toEqual(["fx.n1.frequency", "volume"]); @@ -86,22 +86,37 @@ describe("useVolumeAutomation", () => { { target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }, ], }); - const { binding, onSetAttribute } = bind({ volume: "0.4", automation }); + const { binding, onSetAttributeQuiet } = bind({ volume: "0.4", automation }); act(() => binding.onRemoveVolumeAutomation()); expect( - JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes.map( + JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes.map( (l: { target: string }) => l.target, ), ).toEqual(["fx.n1.frequency"]); }); it("clears the attribute when the volume lane was the only one", () => { - const { binding, onSetAttribute } = bind({ volume: "0.4", automation: volumeLane(0.2) }); + const { binding, onSetAttributeQuiet } = bind({ volume: "0.4", automation: volumeLane(0.2) }); act(() => binding.onRemoveVolumeAutomation()); - expect(onSetAttribute).toHaveBeenCalledWith("data-automation", ""); + // Null, not "": the quiet path removes an attribute it is given null for. + expect(onSetAttributeQuiet).toHaveBeenCalledWith("data-automation", null); }); it("reads an unreadable attribute as no automation", () => { expect(bind({ volume: "0.55", automation: "{not json" }).binding.volumeAutomated).toBe(false); }); + + it("seeds at unity when data-volume is present but empty", () => { + // Number("") is 0, so `?? "1"` alone seeded the lane at silence while the + // engine read the same empty attribute as unity. + const { binding, onSetAttributeQuiet } = bind({ volume: "" }); + act(() => binding.onAutomateVolume()); + expect(JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes[0].points[0].v).toBe(1); + }); + + it("seeds at unity when data-volume is not a number", () => { + const { binding, onSetAttributeQuiet } = bind({ volume: "loud" }); + act(() => binding.onAutomateVolume()); + expect(JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes[0].points[0].v).toBe(1); + }); }); diff --git a/packages/studio/src/components/editor/useVolumeAutomation.ts b/packages/studio/src/components/editor/useVolumeAutomation.ts index 477d98a0b9..b0cfbb7aa1 100644 --- a/packages/studio/src/components/editor/useVolumeAutomation.ts +++ b/packages/studio/src/components/editor/useVolumeAutomation.ts @@ -24,15 +24,22 @@ export interface VolumeAutomationBinding { export function useVolumeAutomation( element: DomEditSelection, - onSetAttribute: (attr: string, value: string) => void | Promise, + onSetAttributeQuiet: (attr: string, value: string | null) => void | Promise, ): VolumeAutomationBinding { // The chain is not needed to resolve a volume lane — volume is always a valid // target — so this deliberately does not parse it. const automation = readPanelAutomation(element.dataAttributes?.["automation"], undefined); const write = (next: Parameters[0]): void => { - void onSetAttribute(HF_AUDIO_AUTOMATION_ATTR, automationAttrValue(next)); + // Quiet: clicking the toggle used to reload the preview and restart every + // playing track, while the same click on an effect parameter did not. + void onSetAttributeQuiet(HF_AUDIO_AUTOMATION_ATTR, automationAttrValue(next) || null); }; - const current = Number(element.dataAttributes?.["volume"] ?? "1"); + // `??` alone would let an empty `data-volume` through as Number("") === 0, so + // automating the track would seed its lane at silence. The engine reads the same + // empty value as unity. + const raw = element.dataAttributes?.["volume"]; + const parsed = raw ? Number(raw) : 1; + const current = Number.isFinite(parsed) ? parsed : 1; return { volumeAutomated: automation.lanes.some((lane) => lane.target === VOLUME_TARGET), // Seeded at the level the slider already shows, so automating the track does diff --git a/packages/studio/src/player/components/useAutomationLanes.ts b/packages/studio/src/player/components/useAutomationLanes.ts index 0e23ce8336..e85f1f3b77 100644 --- a/packages/studio/src/player/components/useAutomationLanes.ts +++ b/packages/studio/src/player/components/useAutomationLanes.ts @@ -58,7 +58,11 @@ export function useAutomationLanes(): UseAutomationLanesResult { const write = (next: HfAutomation, persist: boolean): void => { if (!domEdit || !isSelected) return; const value = next.lanes.length > 0 ? serializeAutomation(next) : ""; - if (persist) void domEdit.handleDomAttributeCommit(HF_AUDIO_AUTOMATION_ATTR, value); + // Quiet, not the refreshing commit: releasing a dragged point used to + // reload the preview, which restarts every playing track — the same chop + // the live write during the drag exists to avoid. Quiet still persists + // and still resyncs the selection, so the next edit sees this one. + if (persist) void domEdit.handleDomAttributeQuietCommit(HF_AUDIO_AUTOMATION_ATTR, value); // Dragging a point writes live: no preview refresh, so the composition // does not reload and restart playback on every pixel. else void domEdit.handleDomAttributeLiveCommit(HF_AUDIO_AUTOMATION_ATTR, value || null);