From 82d9c03d54c2f31afc1fc35864ecbfca2beb1775 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 10 Aug 2026 06:42:06 -0700 Subject: [PATCH 1/4] docs(plans): settle the three open rack-UX questions --- plans/audio-fx-ux/README.md | 46 +++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/plans/audio-fx-ux/README.md b/plans/audio-fx-ux/README.md index fbba9073f7..8f2ee65461 100644 --- a/plans/audio-fx-ux/README.md +++ b/plans/audio-fx-ux/README.md @@ -161,17 +161,31 @@ detent is what an equaliser looks like to everyone who has met one. Collapsed, it reads like every other module: "Bass +3, Middle −2, Treble +2", or "Flat" when nothing has been touched. -## What still needs deciding - -- Does the plain name **replace** the DSP name or sit beside it? Replacing is - friendlier but strands what the author learns. -- Should the **menus** be organised by complaint ("my voice sounds boomy") - rather than by effect family? The rack itself must stay in signal order, - because order is audible — but the menus have no such constraint, and the - preset section of the preview is written that way to show the difference. -- How much should **hover audition**? Hearing a preset before committing is the - single strongest affordance here. Cheap for static presets; a measuring script - has to analyse first and cannot preview instantly. +## Decided (2026-08-10) + +All three were open until now, and the first was blocking the wiring. + +**The plain name replaces the DSP name; the DSP name lives under Details.** +The rack reads plain top to bottom — `Remove Rumble`, not `highpass` — and the +header stays narrow, which matters because it already carries a summary, a +bypass, two arrows and a delete. Nothing is lost: opening a module shows the +DSP name beside its real parameters, so the author who wants the mapping finds +it exactly where the mechanism is, and the author who does not never meets it. + +**Presets sort by complaint; the effect list stays in signal order.** +The two menus stop competing to be the same thing. Presets are the casual +author's door and `PRESET_PROBLEM` already carries the line for all 18 of them, +so this costs no new writing. The effect list stays Filters / Dynamics / +Non-linear / Time — it is the expert's tool, and that grouping teaches the +signal path the rack itself is ordered by. + +**Everything auditions on hover, with a spinner for the ones that measure.** +Static presets apply to the playing audio and revert on leave, which the graph +rebuild path already supports. A carve or an Even Out Levels analyses first and +says so while it does — the same spinner the carve module already shows. This +is the expensive answer of the three: analysis is seconds, and a hover that +takes seconds is one the author has often already left, so whatever gets built +needs a cancel on leave and must not apply a result that arrives late. ## Status @@ -180,10 +194,12 @@ layer has now landed as `packages/core/src/audioFxCopy.ts` — `EFFECT_COPY`, `BANDS`, `PRESET_PROBLEM` and `SUMMARY`, with the completeness check as a test beside it rather than a build step. -Landing the data is not the same as wiring it. Nothing in the studio reads it -yet, and it should not until the three questions above are answered — whether -the plain name replaces the DSP name or sits beside it decides what the rack -renders, and building it twice to find out is the expensive way. +Landing the data is not the same as wiring it, and **nothing in the studio reads +it yet**. That was blocked on the naming question until 2026-08-10; it is now +just unbuilt. What it takes: `propertyPanelFxNodeRow.tsx` reads `EFFECT_COPY` +for the header and `SUMMARY` for the collapsed line, `propertyPanelFxSection`'s +preset menu groups by `PRESET_PROBLEM`, and the hover-audition path is new work +of its own. It has no entry for Tone or for the levelling module, because both carry their own copy in core (`audioEqSummary`, `levellingSummary`). That is the right home From 42c64819de6208058da35b44d094e8f4c7270820 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 10 Aug 2026 07:03:12 -0700 Subject: [PATCH 2/4] feat(studio): make the rack speak the author's language MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires `audioFxCopy` into the panel, to the three decisions recorded in `plans/audio-fx-ux/README.md` §Decided. **The plain name replaces the DSP name, which moves inside.** A module reads "Remove Rumble" with "Cutting everything below 80 Hz" under it, and opening it shows "Details — High-pass" above the knobs. Every knob is renamed too — `plainDef` writes the copy over the registry's labels and hints while leaving range, step, unit and automatability alone, because those are facts and only the words were ever the problem. A preset's own job name still wins where it has one: it is the most specific truth available. **Presets read as the complaint they answer.** "My voice sounds boomy", with `Boom Tame` underneath in small type — which is also how the name gets learned. The four shelves stay: eighteen sentences in a column is a wall, and Voice/Fix/Character/Space is already the author's grouping rather than the registry's. The effect list is untouched and stays in signal order, per the same decision. **Everything auditions on hover, and on focus — the keyboard reaches it too.** Hovering a preset or an offered effect plays it through the same preview channel a slider drag already uses, so it costs no new machinery and never touches the document. Three things this has to get right, each with a test that fails without it: - Leaving puts the stored chain back — captured when the audition starts, because the preview channel does not persist and the prop never changes. - Applying does NOT put it back. The audition *was* the preset, and reverting over the write that just landed is a race the author hears as it arriving and then leaving again. - The panel going away mid-audition reverts too. Deselecting a clip is not a decision, and a preview left playing is audible, absent from the document, and gone on the next reload. Tone does not audition: its bands arrive at 0 dB, so there is nothing to hear, and a hover that changes nothing teaches that hovering does nothing. **Even Out Levels auditions by measuring first**, which is the expensive answer of the three — it decodes the track and analyses it before there is anything to play. So it says "measuring…" while it works, the decode is cached per `src` (a hover that re-fetched a several-minute voiceover would be slower than the thing it previews), and a run counter drops any result that arrives after the pointer has left. Reverting puts both the chain and the automation back, because levelling is a node AND the lane that drives it — restoring only the chain leaves an envelope writing to a gain stage that is no longer there. Tests that addressed rows by their registry labels now look them up through `EFFECT_COPY`, and the preset buttons by preset id. Those tests are about which row carries the automate button, not about how it reads, and hard-coding the words would make every copy edit a test edit. Falsified: dropping the phase of each of the three audition invariants fails its own test, and removing the stale-run guard fails the levelling one. studio 3683 passing, 18 todo (was 3674); section file 510 lines, under the 600 cap. --- .../editor/propertyPanelAudioFxGroup.test.tsx | 118 ++++++++++- .../editor/propertyPanelAudioFxGroup.tsx | 102 ++++++++-- .../editor/propertyPanelFxNodeRow.tsx | 85 ++++++-- .../editor/propertyPanelFxPresetMenu.tsx | 47 ++++- .../editor/propertyPanelFxSection.test.tsx | 184 ++++++++++++++++-- .../editor/propertyPanelFxSection.tsx | 125 +++++++++++- 6 files changed, 595 insertions(+), 66 deletions(-) diff --git a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx index 16986e6ee8..fa6d78b440 100644 --- a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx @@ -4,8 +4,23 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createRoot } from "react-dom/client"; import { AudioFxGroup } from "./propertyPanelAudioFxGroup.js"; import type { DomEditSelection } from "./domEditingTypes"; +import { EFFECT_COPY } from "@hyperframes/core/audio-fx-copy"; import { liveTime, usePlayerStore } from "../../player"; +/** + * What a knob is CALLED in the panel, looked up rather than spelled out. + * + * The rack speaks the plain-language layer now, so a row is addressed by the + * parameter it belongs to and the copy decides the words. Hard-coding them here + * would make every copy edit a test edit, and these tests are about which row + * carries the automate button — not about how it reads. + */ +function plainLabel(effectId: string, key: string): string { + const label = EFFECT_COPY[effectId]?.params[key]?.label; + if (!label) throw new Error(`no copy for ${effectId}.${key}`); + return label; +} + (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; const CHAIN = JSON.stringify({ @@ -49,6 +64,11 @@ function audioSelection( return { dataAttributes, id: "bed", element: bed } as unknown as DomEditSelection; } +/** A button found by the text it contains, since several now read as sentences. */ +function byTextButton(host: HTMLElement, text: string): HTMLButtonElement | undefined { + return Array.from(host.querySelectorAll("button")).find((b) => b.textContent?.includes(text)); +} + function mount(dataAttributes: Record, alone = false, voices = 2) { // Every write is quiet: persisted without the preview reload that would // restart every playing track, but with a selection resync so the panel sees @@ -91,15 +111,17 @@ const writeTo = (calls: unknown[][], attr: string): unknown[] | undefined => describe("AudioFxGroup automation", () => { it("renders the chain's parameters", () => { const { host } = mount({ "fx-chain": CHAIN }); - expect(rowFor(host, "Cutoff")).toBeTruthy(); - expect(rowFor(host, "Q")).toBeTruthy(); + expect(rowFor(host, plainLabel("lowpass", "frequency"))).toBeTruthy(); + expect(rowFor(host, plainLabel("lowpass", "q"))).toBeTruthy(); }); it("seeds a new lane at the value the control already holds", () => { // Switching to an envelope must not change the sound — only where the value // comes from. The chain has frequency at 900, not the registry default. const { host, onSetAttributeQuiet } = mount({ "fx-chain": CHAIN }); - const button = rowFor(host, "Cutoff")!.querySelector(".hf-fx-automate") as HTMLButtonElement; + const button = rowFor(host, plainLabel("lowpass", "frequency"))!.querySelector( + ".hf-fx-automate", + ) as HTMLButtonElement; act(() => button.click()); const write = writeTo(onSetAttributeQuiet.mock.calls, "data-automation"); expect(write).toBeTruthy(); @@ -117,7 +139,13 @@ describe("AudioFxGroup automation", () => { lanes: [{ target: "volume", points: [{ t: 0, v: 0.5 }] }], }), }); - act(() => (rowFor(host, "Q")!.querySelector(".hf-fx-automate") as HTMLButtonElement).click()); + act(() => + ( + rowFor(host, plainLabel("lowpass", "q"))!.querySelector( + ".hf-fx-automate", + ) as HTMLButtonElement + ).click(), + ); expect( parseWrite(writeTo(onSetAttributeQuiet.mock.calls, "data-automation")!).lanes.map( (l: { target: string }) => l.target, @@ -133,11 +161,13 @@ describe("AudioFxGroup automation", () => { lanes: [{ target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }], }), }); - const cutoff = rowFor(host, "Cutoff")!; + const cutoff = rowFor(host, plainLabel("lowpass", "frequency"))!; expect(cutoff.querySelector('input[type="range"]')?.disabled).toBe(true); expect(cutoff.hasAttribute("data-automated")).toBe(true); expect( - rowFor(host, "Q")!.querySelector('input[type="range"]')?.disabled, + rowFor(host, plainLabel("lowpass", "q"))!.querySelector( + 'input[type="range"]', + )?.disabled, ).toBe(false); }); @@ -153,7 +183,11 @@ describe("AudioFxGroup automation", () => { }), }); act(() => - (rowFor(host, "Cutoff")!.querySelector(".hf-fx-automate") as HTMLButtonElement).click(), + ( + rowFor(host, plainLabel("lowpass", "frequency"))!.querySelector( + ".hf-fx-automate", + ) as HTMLButtonElement + ).click(), ); expect( parseWrite(writeTo(onSetAttributeQuiet.mock.calls, "data-automation")!).lanes.map( @@ -171,7 +205,11 @@ describe("AudioFxGroup automation", () => { }), }); act(() => - (rowFor(host, "Cutoff")!.querySelector(".hf-fx-automate") as HTMLButtonElement).click(), + ( + rowFor(host, plainLabel("lowpass", "frequency"))!.querySelector( + ".hf-fx-automate", + ) as HTMLButtonElement + ).click(), ); // Null rather than "": the live path removes an attribute it is given null for. expect(writeTo(onSetAttributeQuiet.mock.calls, "data-automation")![1]).toBeNull(); @@ -187,7 +225,9 @@ describe("AudioFxGroup automation", () => { }); // Nothing is automated, so every control stays live. expect( - rowFor(host, "Cutoff")!.querySelector('input[type="range"]')?.disabled, + rowFor(host, plainLabel("lowpass", "frequency"))!.querySelector( + 'input[type="range"]', + )?.disabled, ).toBe(false); }); }); @@ -328,6 +368,64 @@ describe("AudioFxGroup dynamic carve", () => { afterEach(() => vi.unstubAllGlobals()); + /** + * Hover-auditioning the leveller has to measure before there is anything to + * hear, and measuring a long voiceover takes seconds — by which time the + * pointer has usually moved on. Applying then would put levelling on a track + * nobody asked to level, through a channel that does not persist: audible, + * absent from the document, and gone on the next reload. + */ + it("drops a levelling measurement that lands after the pointer has gone", async () => { + const sampleRate = 48000; + const data = new Float32Array(sampleRate * 4); + for (let i = 0; i < data.length; i++) { + const t = i / sampleRate; + data[i] = t > 1 && t < 3 ? 0.7 * Math.sin(2 * Math.PI * 1000 * t) : 0; + } + let release: (() => void) | null = null; + const decoded = new Promise((r) => { + release = r; + }); + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ arrayBuffer: async () => new ArrayBuffer(8) })), + ); + vi.stubGlobal( + "OfflineAudioContext", + class { + async decodeAudioData() { + await decoded; + return { sampleRate, getChannelData: () => data }; + } + }, + ); + + const { host, onSetAttributeLive } = mount({ "fx-chain": CHAIN }); + document.getElementById("bed")?.setAttribute("src", "bed.wav"); + act(() => byTextButton(host, "Add effect")?.click()); + const level = byTextButton(host, "Even Out Levels"); + expect(level, "the levelling button was not offered").toBeTruthy(); + act(() => level?.focus()); + // Gone again before the decode finishes. + act(() => { + host + .querySelector(".hf-fx-add-menu") + ?.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + }); + await act(async () => { + release?.(); + await decoded; + // Two turns: the decode resolves, then the measurement it feeds. + await Promise.resolve(); + }); + + // The revert on the way out is allowed to write; a levelling stage is not. + const levelled = onSetAttributeLive.mock.calls.filter((c) => + String(c[1] ?? "").includes("fromLeveller"), + ); + expect(levelled).toEqual([]); + }); + it("automates the carve filters' gain from the voice, in the bed's own time", async () => { stubDecode(); // Voice starts 10s into the composition, bed at 0: the envelope is measured @@ -933,7 +1031,7 @@ describe("AudioFxGroup carve module readouts", () => { ], }), }); - const mixRow = rowFor(host, "Mix"); + const mixRow = rowFor(host, plainLabel("delay", "mix")); const number = mixRow?.querySelector(".hf-fx-number"); const slider = mixRow?.querySelector(".hf-fx-slider"); expect(number?.disabled).toBe(true); // the lane owns it diff --git a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx index b31020130c..8585c8c880 100644 --- a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx +++ b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx @@ -7,7 +7,7 @@ * budget, and self-contained enough to test on its own. */ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { defaultAudioFxParams, HF_AUDIO_FX_ATTR, @@ -492,23 +492,43 @@ export function AudioFxGroup({ * works, write once — but it listens to the track it is on rather than to a * voice above it, so it needs no source picker. */ - const runLeveller = async (): Promise => { + /** + * This track's audio, decoded once and kept. + * + * Levelling is measured from it, and hover-auditioning means measuring on every + * pass over the button — fetching and decoding a several-minute voiceover each + * time would make the audition slower than the thing it is previewing. Keyed by + * `src` so a track pointed at a different file re-decodes. + */ + const decoded = useRef<{ src: string; samples: Float32Array; sampleRate: number } | null>(null); + + const decodeTrack = async (): Promise<{ samples: Float32Array; sampleRate: number } | null> => { const el = element.element; const src = el?.getAttribute("src"); const doc = el?.ownerDocument; - if (!src || !doc) return; + if (!src || !doc) return null; + const cached = decoded.current; + if (cached?.src === src) return cached; + const Ctor = + window.OfflineAudioContext ?? + (window as unknown as { webkitOfflineAudioContext?: typeof OfflineAudioContext }) + .webkitOfflineAudioContext; + if (!Ctor) return null; + const res = await fetch(new URL(src, doc.baseURI).href); + const buffer = await new Ctor(1, 1, DECODE_SAMPLE_RATE).decodeAudioData( + await res.arrayBuffer(), + ); + const next = { src, samples: buffer.getChannelData(0), sampleRate: buffer.sampleRate }; + decoded.current = next; + return next; + }; + + const runLeveller = async (): Promise => { setAnalysing(true); try { - const Ctor = - window.OfflineAudioContext ?? - (window as unknown as { webkitOfflineAudioContext?: typeof OfflineAudioContext }) - .webkitOfflineAudioContext; - if (!Ctor) return; - const res = await fetch(new URL(src, doc.baseURI).href); - const buffer = await new Ctor(1, 1, DECODE_SAMPLE_RATE).decodeAudioData( - await res.arrayBuffer(), - ); - const result = levellingResult(chain, buffer.getChannelData(0), buffer.sampleRate); + const audio = await decodeTrack(); + if (!audio) return; + const result = levellingResult(chain, audio.samples, audio.sampleRate); if (!result) return; await onSetAttributeQuiet(HF_AUDIO_FX_ATTR, serializeAudioFxChain(result.chain)); // Merged by target, never written wholesale: the script describes its own @@ -529,6 +549,60 @@ export function AudioFxGroup({ } }; + const [auditioningLevel, setAuditioningLevel] = useState(false); + /** + * Bumped on every enter and leave, so a measurement can tell whether the + * pointer is still on the button when it finishes. + * + * Decoding a long voiceover takes seconds, and a hover that takes seconds is + * one the author has usually already left. Applying the result then would put + * levelling on a track nobody asked to level, through a channel that does not + * persist — so it would be audible, invisible in the document, and gone on the + * next reload. This counter is what makes a late result a no-op. + */ + const auditionRun = useRef(0); + + /** + * Measure this track and play the levelling without persisting it. + * + * `false` puts the stored chain and automation back. Both attributes, because + * levelling is a node AND the lane that drives it: reverting only the chain + * would leave an envelope writing to a gain stage that is no longer there. + */ + const auditionLevel = async (on: boolean): Promise => { + const run = ++auditionRun.current; + if (!on) { + setAuditioningLevel(false); + void onSetAttributeLive( + HF_AUDIO_FX_ATTR, + chain.nodes.length ? serializeAudioFxChain(chain) : null, + ); + void onSetAttributeLive(HF_AUDIO_AUTOMATION_ATTR, automationAttrValue(automation) || null); + return; + } + setAuditioningLevel(true); + try { + const audio = await decodeTrack(); + // Gone, or superseded by a later hover. Either way this result is stale. + if (!audio || run !== auditionRun.current) return; + const result = levellingResult(chain, audio.samples, audio.sampleRate); + if (!result || run !== auditionRun.current) return; + void onSetAttributeLive(HF_AUDIO_FX_ATTR, serializeAudioFxChain(result.chain)); + const lane = result.automation.lanes[0]; + if (lane) { + void onSetAttributeLive( + HF_AUDIO_AUTOMATION_ATTR, + automationAttrValue(withLane(automation, lane)) || null, + ); + } + } catch { + // Same as the real run: a track that cannot be decoded simply does not + // audition, rather than failing the panel. + } finally { + if (run === auditionRun.current) setAuditioningLevel(false); + } + }; + const removeLeveller = (): void => { const { chain: next, removedTarget } = removeLevelling(chain); void onSetAttributeQuiet(HF_AUDIO_FX_ATTR, serializeAudioFxChain(next)); @@ -722,6 +796,8 @@ export function AudioFxGroup({ onLevel={() => void runLeveller()} onRemoveLevel={removeLeveller} levelled={chain.nodes.some((n) => n.fromLeveller)} + onAuditionLevel={(on) => void auditionLevel(on)} + auditioningLevel={auditioningLevel} carvedAgainstBy={carvedAgainstBy} analysing={analysing} /> diff --git a/packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx b/packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx index 613a4ab350..edbb48b193 100644 --- a/packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx @@ -3,8 +3,14 @@ * * An entry in the chain, as opposed to a composite module — the carve and the * Tone EQ own several nodes each and have their own files. + * + * The row speaks the author's language, not the registry's. `EFFECT_COPY` + * supplies the name and every knob's name, `SUMMARY` the sentence under it, and + * the DSP name moves inside — it is a fact about the mechanism, so it belongs + * with the mechanism. See `plans/audio-fx-ux/README.md` §Decided. */ +import { useMemo } from "react"; import { defaultAudioFxParams, getAudioFxDef, @@ -12,9 +18,33 @@ import { type HfAudioFxNode, type HfAudioFxParamValues, } from "@hyperframes/core/audio-fx"; +import { EFFECT_COPY, SUMMARY } from "@hyperframes/core/audio-fx-copy"; import { fxAutomationTarget } from "@hyperframes/core/audio-automation"; import { FxParams } from "./propertyPanelFxControls.js"; +/** + * The registry's definition with the plain names written over it. + * + * Over rather than instead of: the registry stays the authority on range, step, + * unit and what is automatable, and only the words change. A parameter with no + * copy keeps its own label rather than disappearing — `audioFxCopy.test.ts` is + * what makes sure there is never one. + */ +function plainDef(def: HfAudioFxDef): HfAudioFxDef { + const copy = EFFECT_COPY[def.id]; + if (!copy) return def; + return { + ...def, + params: def.params.map((param) => { + const plain = copy.params[param.key]; + if (!plain) return param; + // The registry's hint explains the mechanism, which is still the better + // tooltip than none — but the plain one wins where it exists. + return { ...param, label: plain.label, ...(plain.hint ? { hint: plain.hint } : {}) }; + }), + }; +} + interface FxNodeRowProps { node: HfAudioFxNode; index: number; @@ -218,18 +248,26 @@ export function FxNodeRow({ onRemove, onPreview, }: FxNodeRowProps) { - const def = getAudioFxDef(node.type); - if (!def) return null; + const registryDef = getAudioFxDef(node.type); + const def = useMemo(() => (registryDef ? plainDef(registryDef) : null), [registryDef]); + if (!registryDef || !def) return null; const bypassed = node.enabled === false; + const params = node.params ?? defaultAudioFxParams(node.type); + // What this effect is doing to the sound, as a sentence. The rack is read top + // to bottom far more often than any one module is opened, so this is the line + // that decides whether an author can follow their own mix. + const summary = SUMMARY[node.type]?.(params); return (
onMove(index, delta)} onRemove={() => onRemove(index)} /> + {summary ? ( +

+ {summary} +

+ ) : null} {open ? ( - + <> + {/* The DSP name, once, where the mechanism is. An author who wants to + know what "Remove Rumble" really is finds out by opening it; one who + does not never has to meet the word. */} +

+ Details — {registryDef.label} +

+ + ) : null}
); diff --git a/packages/studio/src/components/editor/propertyPanelFxPresetMenu.tsx b/packages/studio/src/components/editor/propertyPanelFxPresetMenu.tsx index f841968a8b..e661024c79 100644 --- a/packages/studio/src/components/editor/propertyPanelFxPresetMenu.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxPresetMenu.tsx @@ -12,6 +12,7 @@ import { HF_AUDIO_FX_PRESET_FAMILIES, type HfAudioFxPresetFamily, } from "@hyperframes/core/audio-fx-presets"; +import { PRESET_PROBLEM } from "@hyperframes/core/audio-fx-copy"; /** * Shelf names in the author's language, which is deliberately not the effect @@ -28,21 +29,46 @@ const FAMILY_LABEL: Record = { export interface FxPresetMenuProps { onPick(id: string): void; + /** + * Play this preset on the running audio without persisting it, and revert on + * `null`. Absent when there is no preview channel to hear it through. + */ + onAudition?(id: string | null): void; } -export function FxPresetMenu({ onPick }: FxPresetMenuProps) { +/** + * Presets read as the complaint they answer, not as their own names. + * + * The name is a thing you have to already know — "Telephone", "Broadcast" — and + * the author arriving here does not know it; they know their voice sounds + * boomy. So the sentence leads and the name follows underneath, which is also + * how the name gets learned. The four shelves stay because eighteen sentences + * in a column is a wall, and they are already the author's grouping rather than + * the registry's. See `plans/audio-fx-ux/README.md` §Decided. + */ +export function FxPresetMenu({ onPick, onAudition }: FxPresetMenuProps) { return ( -
+
onAudition(null) : undefined} + // Focus leaving the shelf is the keyboard's version of the pointer leaving + // it. Moving between two buttons inside fires this and then the next + // button's focus, so it reverts and re-auditions rather than sticking. + onBlur={onAudition ? () => onAudition(null) : undefined} + > {HF_AUDIO_FX_PRESET_FAMILIES.map((family) => ( -
- +
+ {FAMILY_LABEL[family]} {audioFxPresetsByFamily(family).map((preset) => ( ))}
diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx index cdbb760306..56b52f6c18 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx @@ -8,6 +8,22 @@ import { type HfAudioFxChain, } from "@hyperframes/core/audio-fx"; import { DEFAULT_CARVE } from "@hyperframes/core/audio-carve"; +import { EFFECT_COPY, PRESET_PROBLEM } from "@hyperframes/core/audio-fx-copy"; +import { getAudioFxPreset } from "@hyperframes/core/audio-fx-presets"; + +/** + * What a knob is CALLED in the panel, looked up rather than spelled out. + * + * The rack speaks the plain-language layer now, so a row is addressed by the + * parameter it belongs to and the copy decides the words. Hard-coding them here + * would make every copy edit a test edit, and these tests are about which row + * carries the automate button — not about how it reads. + */ +function plainLabel(effectId: string, key: string): string { + const label = EFFECT_COPY[effectId]?.params[key]?.label; + if (!label) throw new Error(`no copy for ${effectId}.${key}`); + return label; +} import { createRoot } from "react-dom/client"; import { FxSection } from "./propertyPanelFxSection.js"; @@ -56,7 +72,7 @@ function mount(overrides: Partial[0]> = {}) { const onChainChange = vi.fn(); const onChainPreview = vi.fn(); const onCarveChange = vi.fn(); - const { host } = renderInto( + const { host, root } = renderInto( [0]> = {}) { onLevel={overrides.onLevel} onRemoveLevel={overrides.onRemoveLevel} levelled={overrides.levelled} + onAuditionLevel={overrides.onAuditionLevel} + auditioningLevel={overrides.auditioningLevel} />, ); - return { host, onChainChange, onChainPreview, onCarveChange }; + return { host, root, onChainChange, onChainPreview, onCarveChange }; } const click = (el: Element | null | undefined) => { @@ -86,6 +104,19 @@ const click = (el: Element | null | undefined) => { const byText = (host: HTMLElement, sel: string, text: string) => Array.from(host.querySelectorAll(sel)).find((e) => e.textContent?.trim() === text); +/** + * A preset button, found by the preset it applies rather than by its words. + * + * The shelf leads with the complaint and follows with the name, so a button's + * text is two sentences and neither of them alone is what an author would call + * it. Addressing it by id keeps the test about what applying it does. + */ +const presetButton = (host: HTMLElement, id: string): Element | undefined => + Array.from(host.querySelectorAll(".hf-fx-preset-item")).find( + (e) => + e.querySelector(".hf-fx-preset-name")?.textContent?.trim() === getAudioFxPreset(id)?.label, + ); + /** * React tracks an input's value on the DOM node, so assigning `.value` and * dispatching looks like a no-op change and the handler never fires. Going @@ -140,7 +171,7 @@ describe("FxSection chain", () => { const labels = Array.from(host.querySelectorAll(".hf-fx-label")).map((e) => e.textContent?.trim(), ); - for (const p of def.params) expect(labels).toContain(p.label); + for (const p of def.params) expect(labels).toContain(plainLabel("compressor", p.key)); }); it("uses a select for an enum parameter and a slider for a number", () => { @@ -218,7 +249,7 @@ describe("FxSection chain", () => { it("applies a preset as ordinary nodes, tagged with where they came from", () => { const { host, onChainChange } = mount({ chain: { version: 1, nodes: [] } }); click(byText(host, "button", "Presets")); - click(byText(host, "button", "Telephone")); + click(presetButton(host, "telephone")); const next = onChainChange.mock.calls[0]![0] as HfAudioFxChain; // The band, its honk and de-mud shaping, and the soft clip — a chain the @@ -237,6 +268,125 @@ describe("FxSection chain", () => { expect(new Set(next.nodes.map((n) => n.id)).size).toBe(next.nodes.length); }); + it("names an effect for the job it does, and keeps the DSP name for inside", () => { + // The rack is read by somebody who has never opened a mixer. "Remove Rumble" + // is what they came here for; "High-pass" is a fact about the mechanism, so + // it waits until they open the module and ask. + const { host } = mount({ chain: chainOf("highpass") }); + const node = fxCard(host); + const name = node.querySelector(".hf-fx-node-name")?.textContent?.trim(); + expect(name).toBe(EFFECT_COPY.highpass?.title); + expect(name).not.toBe(getAudioFxDef("highpass")?.label); + // And a sentence under it, so the rack reads top to bottom. + expect(node.querySelector(".hf-fx-node-summary")?.textContent).toContain("Cutting everything"); + // The first node is open by default, which is where the DSP name lives. + expect(node.querySelector(".hf-fx-node-mechanism")?.textContent).toContain( + getAudioFxDef("highpass")?.label, + ); + }); + + it("offers presets as the complaint they answer", () => { + const { host } = mount({ chain: { version: 1, nodes: [] } }); + click(byText(host, "button", "Presets")); + const item = presetButton(host, "telephone"); + expect(item?.querySelector(".hf-fx-preset-problem")?.textContent).toBe( + PRESET_PROBLEM.telephone, + ); + // The name is still there, under it — which is how it gets learned. + expect(item?.querySelector(".hf-fx-preset-name")?.textContent).toBe("Telephone"); + }); + + describe("hover-audition", () => { + /** Focus is the keyboard's hover, and both go through the same handler. */ + const enter = (el: Element | null | undefined) => { + if (!el) throw new Error("element not found"); + act(() => (el as HTMLElement).focus()); + }; + const leave = (host: HTMLElement, sel: string) => + act(() => { + host.querySelector(sel)?.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + }); + + it("plays a preset without committing to it", () => { + const { host, onChainPreview, onChainChange } = mount({ chain: { version: 1, nodes: [] } }); + click(byText(host, "button", "Presets")); + enter(presetButton(host, "telephone")); + + const heard = onChainPreview.mock.calls.at(-1)?.[0] as HfAudioFxChain; + expect(heard.nodes.length).toBeGreaterThan(0); + expect(heard.nodes.every((n) => n.fromPreset === "telephone")).toBe(true); + // Heard, not written. Hovering is not a decision. + expect(onChainChange).not.toHaveBeenCalled(); + }); + + it("puts the chain back on the way out", () => { + const { host, onChainPreview } = mount({ chain: chainOf("peaking") }); + click(byText(host, "button", "Presets")); + enter(presetButton(host, "telephone")); + leave(host, ".hf-fx-preset-menu"); + + const back = onChainPreview.mock.calls.at(-1)?.[0] as HfAudioFxChain; + expect(back.nodes.map((n) => n.type)).toEqual(["peaking"]); + }); + + it("does not put the old chain back over the preset it just applied", () => { + // The audition WAS the preset, so reverting after the write is a race the + // author hears as it arriving and then leaving again. + // + // Applying closes the shelf, so the pointer never leaves it — the revert + // that would fire is the panel's own teardown, which is what unmounting + // here exercises. It is also the real route: applying a preset and then + // clicking another clip does exactly this. + const { host, root, onChainPreview } = mount({ chain: { version: 1, nodes: [] } }); + click(byText(host, "button", "Presets")); + enter(presetButton(host, "telephone")); + const auditions = onChainPreview.mock.calls.length; + click(presetButton(host, "telephone")); + act(() => root.unmount()); + + expect(onChainPreview.mock.calls.length).toBe(auditions); + }); + + it("puts the chain back if the panel goes away mid-audition", () => { + // Deselecting the clip while hovering is not a decision either, and the + // preview channel does not persist — so without this the author hears a + // chain the document does not have until something else writes. + const { host, root, onChainPreview } = mount({ chain: chainOf("peaking") }); + click(byText(host, "button", "Presets")); + enter(presetButton(host, "telephone")); + act(() => root.unmount()); + + const back = onChainPreview.mock.calls.at(-1)?.[0] as HfAudioFxChain; + expect(back.nodes.map((n) => n.type)).toEqual(["peaking"]); + }); + + it("auditions an effect the add menu is offering", () => { + const { host, onChainPreview, onChainChange } = mount({ chain: chainOf("peaking") }); + click(byText(host, "button", "Add effect")); + enter(byText(host, "button", "Reverb")); + + const heard = onChainPreview.mock.calls.at(-1)?.[0] as HfAudioFxChain; + expect(heard.nodes.map((n) => n.type)).toEqual(["peaking", "reverb"]); + expect(onChainChange).not.toHaveBeenCalled(); + }); + + it("asks for a levelling measurement on hover, and calls it off on the way out", () => { + // The one module that cannot answer instantly: there is nothing to hear + // until the track has been decoded and measured. + const onAuditionLevel = vi.fn(); + const { host } = mount({ + chain: { version: 1, nodes: [] }, + onLevel: vi.fn(), + onAuditionLevel, + }); + click(byText(host, "button", "Add effect")); + enter(byText(host, "button", "Even Out Levels")); + expect(onAuditionLevel).toHaveBeenLastCalledWith(true); + leave(host, ".hf-fx-add-menu"); + expect(onAuditionLevel).toHaveBeenLastCalledWith(false); + }); + }); + it("adds a preset to what is already there rather than replacing it", () => { const existing: HfAudioFxChain = { version: 1, @@ -246,7 +396,7 @@ describe("FxSection chain", () => { }; const { host, onChainChange } = mount({ chain: existing }); click(byText(host, "button", "Presets")); - click(byText(host, "button", "Cut Rumble")); + click(presetButton(host, "rumble-cut")); const next = onChainChange.mock.calls[0]![0] as HfAudioFxChain; expect(next.nodes.map((n) => n.id)).toContain("mine"); @@ -762,8 +912,12 @@ describe("automation in the panel", () => { // Saturate: `output` is a make-up gain, but the curve's type and threshold // are rebuilt wholesale and cannot be scheduled. const { host } = automatable(idChain("saturate")); - expect(rowFor(host, "Output")?.querySelector(".hf-fx-automate")).toBeTruthy(); - expect(rowFor(host, "Threshold")?.querySelector(".hf-fx-automate")).toBeNull(); + expect( + rowFor(host, plainLabel("saturate", "output"))?.querySelector(".hf-fx-automate"), + ).toBeTruthy(); + expect( + rowFor(host, plainLabel("saturate", "threshold"))?.querySelector(".hf-fx-automate"), + ).toBeNull(); }); it("offers nothing for a worklet effect, which exposes no AudioParams", () => { @@ -774,7 +928,9 @@ describe("automation in the panel", () => { it("asks to automate a parameter by node id and key", () => { const onAutomateParam = vi.fn(); const { host } = automatable(idChain("lowpass", "n7"), { onAutomateParam }); - const button = rowFor(host, "Cutoff")!.querySelector(".hf-fx-automate") as HTMLButtonElement; + const button = rowFor(host, plainLabel("lowpass", "frequency"))!.querySelector( + ".hf-fx-automate", + ) as HTMLButtonElement; expect(button.hasAttribute("title")).toBe(false); act(() => button.click()); expect(onAutomateParam).toHaveBeenCalledWith("n7", "frequency"); @@ -784,12 +940,12 @@ describe("automation in the panel", () => { const { host } = automatable(idChain("lowpass"), { automatedTargets: new Set(["fx.n1.frequency"]), }); - const row = rowFor(host, "Cutoff")!; + const row = rowFor(host, plainLabel("lowpass", "frequency"))!; expect(row.querySelector('input[type="range"]')?.disabled).toBe(true); expect(row.querySelector('input[type="number"]')?.disabled).toBe(true); expect(row.hasAttribute("data-automated")).toBe(true); // A sibling parameter on the same effect stays editable. - const q = rowFor(host, "Q")!; + const q = rowFor(host, plainLabel("lowpass", "q"))!; expect(q.querySelector('input[type="range"]')?.disabled).toBe(false); }); @@ -799,7 +955,9 @@ describe("automation in the panel", () => { automatedTargets: new Set(["fx.n1.frequency"]), onRemoveParamAutomation, }); - const button = rowFor(host, "Cutoff")!.querySelector(".hf-fx-automate") as HTMLButtonElement; + const button = rowFor(host, plainLabel("lowpass", "frequency"))!.querySelector( + ".hf-fx-automate", + ) as HTMLButtonElement; expect(button.getAttribute("aria-pressed")).toBe("true"); expect(button.getAttribute("aria-label")).toMatch(/remove/i); // The wording lives in the Tooltip component, which only renders its bubble @@ -815,7 +973,9 @@ describe("automation in the panel", () => { const { host } = automatable(idChain("lowpass"), { automatedTargets: new Set(["fx.n1.frequency"]), }); - const button = rowFor(host, "Cutoff")!.querySelector(".hf-fx-automate") as HTMLButtonElement; + const button = rowFor(host, plainLabel("lowpass", "frequency"))!.querySelector( + ".hf-fx-automate", + ) as HTMLButtonElement; // Tooltip positions itself from the trigger's box and gives up on a 0x0 // one, which is every element in happy-dom. vi.spyOn(button, "getBoundingClientRect").mockReturnValue({ diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.tsx index 3414780d8e..37b3b53789 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.tsx @@ -5,7 +5,7 @@ * is not an entry in the chain. */ -import { useCallback, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { defaultAudioFxParams, HF_AUDIO_FX, @@ -64,6 +64,17 @@ export interface FxSectionProps { onRemoveLevel?(): void; /** Whether a levelling stage is already on the track. */ levelled?: boolean; + /** + * Hover-audition of the levelling script: measure this track and play the + * result without persisting it, and put it back on `false`. + * + * Separate from `onChainPreview` because it is the one audition that cannot be + * synthesised from the chain in hand — the numbers do not exist until the + * audio has been decoded and measured. + */ + onAuditionLevel?(on: boolean): void; + /** Whether that measurement is running, so the button can say so. */ + auditioningLevel?: boolean; /** Structural edits and gesture-end writes; this is the one that persists. */ onChainChange(chain: HfAudioFxChain): void; /** Continuous updates while a control is being dragged. */ @@ -104,6 +115,8 @@ export function FxSection({ onLevel, onRemoveLevel, levelled, + onAuditionLevel, + auditioningLevel, }: FxSectionProps) { // Falls back to the persisting write when no preview handler is supplied, which // keeps the control working rather than going dead. @@ -139,6 +152,48 @@ export function FxSection({ [chain, onChainPreview], ); + /** + * The chain as it is really stored, captured when an audition starts. + * + * Auditioning writes through the preview channel, which does not persist and + * does not come back as a new `chain` prop — so reverting has to remember what + * was there rather than read it back. Null means nothing is being auditioned, + * which is also what makes a stray leave a no-op instead of a write. + */ + const auditionBase = useRef(null); + + /** + * Play something without committing to it, and put it back on the way out. + * + * Hearing a preset before choosing it is the strongest affordance in this + * panel — see `plans/audio-fx-ux/README.md` §Decided. It costs nothing new: + * the preview channel a slider drag already uses rebuilds the running graph + * without touching the document. + */ + const audition = useCallback( + (make: ((base: HfAudioFxChain) => HfAudioFxChain) | null) => { + if (!onChainPreview) return; + if (make) { + auditionBase.current ??= chain; + onChainPreview(make(auditionBase.current)); + } else if (auditionBase.current) { + onChainPreview(auditionBase.current); + auditionBase.current = null; + } + }, + [chain, onChainPreview], + ); + + // Leaving by any route other than the pointer — the element deselected, the + // panel closed — would otherwise leave the audition playing over a chain the + // document does not have. + useEffect( + () => () => { + if (auditionBase.current) onChainPreview?.(auditionBase.current); + }, + [onChainPreview], + ); + const applyPreset = useCallback( (id: string) => { const preset = getAudioFxPreset(id); @@ -147,6 +202,10 @@ export function FxSection({ // real thing to want, and replacing silently would throw work away — so // the destructive option is a separate gesture, not the default one. const next = applyAudioFxPreset(chain, preset); + // The audition WAS this, so there is nothing to put back — and putting the + // old chain back over the write that just landed is a race the author + // hears as the preset arriving and then leaving again. + auditionBase.current = null; mutate(next.nodes); // Land on the first node the preset wrote, so the author can hear what // arrived and immediately see what it is made of. @@ -156,16 +215,26 @@ export function FxSection({ [chain, mutate], ); + /** One effect at its defaults, appended — what both adding and auditioning do. */ + const withEffect = useCallback( + (base: HfAudioFxChain, type: string): HfAudioFxChain => ({ + ...base, + nodes: [ + ...base.nodes, + { type, id: mintAudioFxNodeId(base), enabled: true, params: defaultAudioFxParams(type) }, + ], + }), + [], + ); + const addEffect = useCallback( (type: string) => { - mutate([ - ...chain.nodes, - { type, id: mintAudioFxNodeId(chain), enabled: true, params: defaultAudioFxParams(type) }, - ]); + auditionBase.current = null; + mutate(withEffect(chain, type).nodes); setOpenNode(chain.nodes.length); setAdding(false); }, - [chain, mutate], + [chain, mutate, withEffect], ); const updateNode = useCallback( @@ -208,6 +277,7 @@ export function FxSection({ const [openEq, setOpenEq] = useState(null); const addEq = useCallback(() => { + auditionBase.current = null; const { chain: next, eqId } = addAudioEq(chain); mutate(next.nodes); setOpenEq(eqId); @@ -321,7 +391,21 @@ export function FxSection({
{adding ? ( -
+
{ + audition(null); + onAuditionLevel?.(false); + }} + // The keyboard's version of leaving. Tabbing between two entries fires + // this and then the next one's focus, so it reverts and re-auditions. + onBlur={() => { + audition(null); + onAuditionLevel?.(false); + }} + >
Tone @@ -337,8 +421,16 @@ export function FxSection({ else onLevel(); setAdding(false); }} + // The one module here that cannot answer instantly: it has to + // decode the track and measure it before there is anything to + // hear. So it says it is working rather than doing nothing + // visible, and whoever handles this must drop a result that + // arrives after the pointer has gone. + onMouseEnter={levelled ? undefined : () => onAuditionLevel?.(true)} + onFocus={levelled ? undefined : () => onAuditionLevel?.(true)} > {levelled ? "Remove levelling" : "Even Out Levels"} + {auditioningLevel ? measuring… : null} ) : null} @@ -374,7 +471,19 @@ export function FxSection({
) : null} - {picking ? : null} + {picking ? ( + { + const preset = id ? getAudioFxPreset(id) : null; + audition(preset ? (base) => applyAudioFxPreset(base, preset) : null); + } + : undefined + } + /> + ) : null} {adding || picking ? null : (
From 5b5de60388b8b5e2fb0842e56fb6a4729d6975e8 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 10 Aug 2026 07:09:08 -0700 Subject: [PATCH 3/4] fix(studio): stop the audition reverting itself, and cancel it sideways MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in the hover-audition that shipped in the commit before this, both invisible to its tests because a static mount never re-renders and never moves the pointer between two entries. **It reverted itself about thirty times a second.** The teardown that puts the chain back was keyed on `onChainPreview`, which the group passes as an inline arrow — and the group re-renders on every playhead tick to move the automation readouts. So React tore the effect down and re-ran it on every tick, and each teardown saw an audition in progress and undid it. The preset was heard for one frame and then silently reverted with the pointer still on the button, during playback, which is the only time there is anything to audition at all. The handler moves into a ref and the effect gets empty deps, so it runs on teardown and at no other time. **Moving to the effect beside it left a measurement in flight.** The levelling audition was only called off by leaving the whole shelf, so sliding from Even Out Levels to Reverb kept the decode running — and when it finished it wrote a levelled version of the chain as it had been, on top of the reverb being auditioned, through the channel the document never sees. Exactly the failure the run counter was added to prevent, one gesture to its left. Every entry in the shelf now calls its neighbours' auditions off: the effects cancel levelling, levelling cancels the chain audition, and Tone cancels both despite having none of its own. The keyboard path was already right — `focusout` bubbles, so a move within the menu fires the shelf's own handler. Only the mouse leaked. Falsified: restoring the dep array fails the re-render test, and dropping the effect buttons' cancel fails the sideways-move one. studio 3685 passing, 18 todo. --- .../editor/propertyPanelAudioFxGroup.test.tsx | 69 +++++++++++++++---- .../editor/propertyPanelFxSection.test.tsx | 28 ++++++++ .../editor/propertyPanelFxSection.tsx | 49 +++++++++++-- 3 files changed, 124 insertions(+), 22 deletions(-) diff --git a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx index fa6d78b440..509a0b642c 100644 --- a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx @@ -366,23 +366,21 @@ describe("AudioFxGroup dynamic carve", () => { ); } - afterEach(() => vi.unstubAllGlobals()); - /** - * Hover-auditioning the leveller has to measure before there is anything to - * hear, and measuring a long voiceover takes seconds — by which time the - * pointer has usually moved on. Applying then would put levelling on a track - * nobody asked to level, through a channel that does not persist: audible, - * absent from the document, and gone on the next reload. + * The same voice, but the decode does not finish until it is let go. + * + * Hover-auditioning the leveller is the one path where the result can arrive + * after the author has moved on, so the tests that cover that need to hold the + * decode open across a second gesture. */ - it("drops a levelling measurement that lands after the pointer has gone", async () => { + function stubGatedDecode(): { release: () => void; decoded: Promise } { const sampleRate = 48000; const data = new Float32Array(sampleRate * 4); for (let i = 0; i < data.length; i++) { const t = i / sampleRate; data[i] = t > 1 && t < 3 ? 0.7 * Math.sin(2 * Math.PI * 1000 * t) : 0; } - let release: (() => void) | null = null; + let release = (): void => {}; const decoded = new Promise((r) => { release = r; }); @@ -399,7 +397,29 @@ describe("AudioFxGroup dynamic carve", () => { } }, ); + return { release: () => release(), decoded }; + } + /** Let the held decode finish, and the measurement it feeds after it. */ + async function settleDecode(release: () => void, decoded: Promise): Promise { + await act(async () => { + release(); + await decoded; + await Promise.resolve(); + }); + } + + afterEach(() => vi.unstubAllGlobals()); + + /** + * Hover-auditioning the leveller has to measure before there is anything to + * hear, and measuring a long voiceover takes seconds — by which time the + * pointer has usually moved on. Applying then would put levelling on a track + * nobody asked to level, through a channel that does not persist: audible, + * absent from the document, and gone on the next reload. + */ + it("drops a levelling measurement that lands after the pointer has gone", async () => { + const { release, decoded } = stubGatedDecode(); const { host, onSetAttributeLive } = mount({ "fx-chain": CHAIN }); document.getElementById("bed")?.setAttribute("src", "bed.wav"); act(() => byTextButton(host, "Add effect")?.click()); @@ -412,12 +432,7 @@ describe("AudioFxGroup dynamic carve", () => { .querySelector(".hf-fx-add-menu") ?.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); }); - await act(async () => { - release?.(); - await decoded; - // Two turns: the decode resolves, then the measurement it feeds. - await Promise.resolve(); - }); + await settleDecode(release, decoded); // The revert on the way out is allowed to write; a levelling stage is not. const levelled = onSetAttributeLive.mock.calls.filter((c) => @@ -426,6 +441,30 @@ describe("AudioFxGroup dynamic carve", () => { expect(levelled).toEqual([]); }); + /** + * Sliding from the leveller to the effect beside it is not leaving the menu, + * so the shelf's own leave never fires — and the measurement already in flight + * used to land on top of whatever was being auditioned next, writing a + * levelled version of the chain as it was through a channel the document never + * sees. Every entry in the shelf calls its neighbours' auditions off. + */ + it("calls the levelling measurement off when the pointer moves to the effect beside it", async () => { + const { release, decoded } = stubGatedDecode(); + const { host, onSetAttributeLive } = mount({ "fx-chain": CHAIN }); + document.getElementById("bed")?.setAttribute("src", "bed.wav"); + act(() => byTextButton(host, "Add effect")?.click()); + act(() => byTextButton(host, "Even Out Levels")?.focus()); + // Straight to a neighbour, without ever leaving the shelf. + act(() => + byTextButton(host, "Reverb")?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })), + ); + await settleDecode(release, decoded); + + expect( + onSetAttributeLive.mock.calls.filter((c) => String(c[1] ?? "").includes("fromLeveller")), + ).toEqual([]); + }); + it("automates the carve filters' gain from the voice, in the bed's own time", async () => { stubDecode(); // Voice starts 10s into the composition, bed at 0: the envelope is measured diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx index 56b52f6c18..c012466cd8 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.test.tsx @@ -360,6 +360,34 @@ describe("FxSection chain", () => { expect(back.nodes.map((n) => n.type)).toEqual(["peaking"]); }); + it("survives the panel re-rendering under it, which playback does constantly", () => { + // The group re-renders on every playhead tick to move the automation + // readouts, handing down a fresh preview callback each time. A teardown + // keyed on that callback ran on every tick, so an audition reverted itself + // about thirty times a second — during playback, which is the only time + // there is anything to audition. + const { host, root, onChainPreview } = mount({ chain: chainOf("peaking") }); + click(byText(host, "button", "Presets")); + enter(presetButton(host, "telephone")); + const auditions = onChainPreview.mock.calls.length; + + // Same behaviour, new identity — exactly what a tick hands down. + act(() => + root.render( + onChainPreview(next)} + carve={null} + onCarveChange={vi.fn()} + sourceOptions={[{ id: "vo", label: "Voiceover" }]} + />, + ), + ); + + expect(onChainPreview.mock.calls.length).toBe(auditions); + }); + it("auditions an effect the add menu is offering", () => { const { host, onChainPreview, onChainChange } = mount({ chain: chainOf("peaking") }); click(byText(host, "button", "Add effect")); diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.tsx index 37b3b53789..d632847618 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.tsx @@ -184,14 +184,28 @@ export function FxSection({ [chain, onChainPreview], ); + /** + * The preview handler as of the last render, held rather than closed over. + * + * The teardown below must run on teardown and at no other time, so its deps + * have to be empty — and `onChainPreview` is an inline arrow in the group, + * which re-renders on every playhead tick to move the automation readouts. A + * dep on it made React tear down and re-run the effect on every one of those + * ticks, so an audition reverted itself about 30 times a second while the + * pointer was still on the button: the preset was heard for a frame during + * playback, which is the exact case the whole affordance exists for. + */ + const previewRef = useRef(onChainPreview); + previewRef.current = onChainPreview; + // Leaving by any route other than the pointer — the element deselected, the // panel closed — would otherwise leave the audition playing over a chain the // document does not have. useEffect( () => () => { - if (auditionBase.current) onChainPreview?.(auditionBase.current); + if (auditionBase.current) previewRef.current?.(auditionBase.current); }, - [onChainPreview], + [], ); const applyPreset = useCallback( @@ -426,7 +440,14 @@ export function FxSection({ // hear. So it says it is working rather than doing nothing // visible, and whoever handles this must drop a result that // arrives after the pointer has gone. - onMouseEnter={levelled ? undefined : () => onAuditionLevel?.(true)} + onMouseEnter={ + levelled + ? undefined + : () => { + audition(null); + onAuditionLevel?.(true); + } + } onFocus={levelled ? undefined : () => onAuditionLevel?.(true)} > {levelled ? "Remove levelling" : "Even Out Levels"} @@ -440,9 +461,14 @@ export function FxSection({ // must not include it. className="hf-fx-add-composite rounded-[3px] bg-panel-surface px-1.5 py-0.5 text-[10px] text-panel-text-1 hover:text-panel-text-0" title="Bass, middle and treble on one set of faders." - // No audition: a Tone module arrives with every band at 0 dB, so - // there is nothing to hear until a fader moves. A hover that - // changes nothing teaches that hovering does nothing. + // No audition of its own: a Tone module arrives with every band at + // 0 dB, so there is nothing to hear until a fader moves, and a + // hover that changes nothing teaches that hovering does nothing. + // It still has to call the neighbours' auditions off. + onMouseEnter={() => { + audition(null); + onAuditionLevel?.(false); + }} onClick={addEq} > Tone (EQ) @@ -460,7 +486,16 @@ export function FxSection({ className="hf-fx-add-item rounded-[3px] bg-panel-surface px-1.5 py-0.5 text-[10px] text-panel-text-1 hover:text-panel-text-0" title={d.description} onClick={() => addEffect(d.id)} - onMouseEnter={() => audition((base) => withEffect(base, d.id))} + // Cancels the levelling audition as well as starting its own. + // The shelf's leave handler only fires on the way OUT of the + // menu, so sliding from Even Out Levels straight to here left a + // measurement in flight — and it landed on top of this one, a + // levelled version of the chain as it was, written through a + // channel the document never sees. + onMouseEnter={() => { + onAuditionLevel?.(false); + audition((base) => withEffect(base, d.id)); + }} onFocus={() => audition((base) => withEffect(base, d.id))} > {d.label} From 1549312922042da3e4bd83c7624e48e71f532172 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 10 Aug 2026 07:10:50 -0700 Subject: [PATCH 4/4] docs(plans): record the rack wiring and what it cost --- plans/audio-fx-ux/README.md | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/plans/audio-fx-ux/README.md b/plans/audio-fx-ux/README.md index 8f2ee65461..c231dd07fa 100644 --- a/plans/audio-fx-ux/README.md +++ b/plans/audio-fx-ux/README.md @@ -194,12 +194,24 @@ layer has now landed as `packages/core/src/audioFxCopy.ts` — `EFFECT_COPY`, `BANDS`, `PRESET_PROBLEM` and `SUMMARY`, with the completeness check as a test beside it rather than a build step. -Landing the data is not the same as wiring it, and **nothing in the studio reads -it yet**. That was blocked on the naming question until 2026-08-10; it is now -just unbuilt. What it takes: `propertyPanelFxNodeRow.tsx` reads `EFFECT_COPY` -for the header and `SUMMARY` for the collapsed line, `propertyPanelFxSection`'s -preset menu groups by `PRESET_PROBLEM`, and the hover-audition path is new work -of its own. +**It is wired.** `propertyPanelFxNodeRow.tsx` takes the name from `EFFECT_COPY`, +the sentence under it from `SUMMARY`, and every knob's name from the same place +via `plainDef` — which writes the words over the registry's def and leaves range, +step, unit and automatability alone. The DSP name sits above the knobs as +`Details — High-pass`. The preset shelf leads with `PRESET_PROBLEM` and follows +with the preset's own name. Hover and focus both audition, through the same +preview channel a slider drag uses; the leveller measures first, says +"measuring…" while it does, caches the decode per `src`, and drops a result that +arrives after the pointer has gone. + +Three things that took a second pass, all worth knowing before touching this +again. An audition has to survive the panel re-rendering under it — the group +re-renders every playhead tick, so anything keyed on its inline callbacks tears +down thirty times a second. Applying must NOT revert, since the audition *was* +the thing applied. And moving between two entries in a shelf is not leaving it, +so each entry has to call its neighbours' auditions off itself. + +What is still not wired: `PROFILES`, below. It has no entry for Tone or for the levelling module, because both carry their own copy in core (`audioEqSummary`, `levellingSummary`). That is the right home