Skip to content

Commit 54af4a8

Browse files
committed
feat(studio): two faces — a module opens on one knob, the rest one click away
Rule 1 of `plans/audio-fx-ux/README.md`. A module opens on its name, a line saying what it is for, and the single control that carries it; every other parameter is behind a Details disclosure. Nothing is hidden — it is ordered. `EFFECT_COPY.primary` names that control and `primaryEnds` says what its two ends sound like, which is the question an author actually has: a number tells them where the knob is, not which way to move it. The DSP name moves onto the disclosure itself, so it is read at the moment the author asks what this really is and never before. Ten of the fifteen effects get this. The other five — compressor, gate, saturate, reverb, bitcrush — name their primary as "strength", meaning they want a single DERIVED control over several parameters, which is `PROFILES`, whose figures are proposed rather than measured and which has not shipped. They open on all their controls until it does. That is the honest state: inventing one knob for them now would be a knob that lies about what it sets. The lookup needs no special case for it either — "strength" names no parameter today, and the day it does name one, this starts using it without being told. Worth naming: `peaking`'s primary is "how much", and that only became coherent in the commit before this one. Boosting an unspecified frequency means nothing, so one knob was a lie while the module was generic; now that picking the job is what picks the range, it is the whole truth. The disclosure is local state, keyed by node like the row itself, so it stays with its effect across a reorder — and an effect arriving in the open slot arrives closed, like any other. Tests that address a non-primary knob now open Details first, which is also the gesture a user makes. Falsified: expanding Details unconditionally fails the two-faces test, and dropping the "is it a real parameter" guard fails three. studio 3688 passing, 18 todo.
1 parent d10375c commit 54af4a8

3 files changed

Lines changed: 164 additions & 24 deletions

File tree

packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,17 @@ function audioSelection(
6464
return { dataAttributes, id: "bed", element: bed } as unknown as DomEditSelection;
6565
}
6666

67+
/**
68+
* Open a module's Details, where every control that is not the primary one now
69+
* lives — a module opens on one knob and the rest is one click away.
70+
*/
71+
function openDetails(host: HTMLElement, index = 0): void {
72+
const buttons = Array.from(host.querySelectorAll<HTMLButtonElement>(".hf-fx-node-details"));
73+
const button = buttons[index];
74+
if (!button) throw new Error("no Details disclosure to open");
75+
act(() => button.click());
76+
}
77+
6778
/** A button found by the text it contains, since several now read as sentences. */
6879
function byTextButton(host: HTMLElement, text: string): HTMLButtonElement | undefined {
6980
return Array.from(host.querySelectorAll("button")).find((b) => b.textContent?.includes(text));
@@ -111,7 +122,11 @@ const writeTo = (calls: unknown[][], attr: string): unknown[] | undefined =>
111122
describe("AudioFxGroup automation", () => {
112123
it("renders the chain's parameters", () => {
113124
const { host } = mount({ "fx-chain": CHAIN });
125+
// The one knob that carries the module is on the open face; the rest are one
126+
// click away, which is what Details is.
114127
expect(rowFor(host, plainLabel("lowpass", "frequency"))).toBeTruthy();
128+
expect(rowFor(host, plainLabel("lowpass", "q"))).toBeNull();
129+
openDetails(host);
115130
expect(rowFor(host, plainLabel("lowpass", "q"))).toBeTruthy();
116131
});
117132

@@ -139,6 +154,7 @@ describe("AudioFxGroup automation", () => {
139154
lanes: [{ target: "volume", points: [{ t: 0, v: 0.5 }] }],
140155
}),
141156
});
157+
openDetails(host);
142158
act(() =>
143159
(
144160
rowFor(host, plainLabel("lowpass", "q"))!.querySelector(
@@ -164,6 +180,7 @@ describe("AudioFxGroup automation", () => {
164180
const cutoff = rowFor(host, plainLabel("lowpass", "frequency"))!;
165181
expect(cutoff.querySelector<HTMLInputElement>('input[type="range"]')?.disabled).toBe(true);
166182
expect(cutoff.hasAttribute("data-automated")).toBe(true);
183+
openDetails(host);
167184
expect(
168185
rowFor(host, plainLabel("lowpass", "q"))!.querySelector<HTMLInputElement>(
169186
'input[type="range"]',

packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx

Lines changed: 97 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* with the mechanism. See `plans/audio-fx-ux/README.md` §Decided.
1111
*/
1212

13-
import { useMemo } from "react";
13+
import { useMemo, useState } from "react";
1414
import {
1515
defaultAudioFxParams,
1616
getAudioFxDef,
@@ -22,6 +22,29 @@ import { EFFECT_COPY, SUMMARY } from "@hyperframes/core/audio-fx-copy";
2222
import { fxAutomationTarget } from "@hyperframes/core/audio-automation";
2323
import { FxParams } from "./propertyPanelFxControls.js";
2424

25+
/**
26+
* The one control that carries the module, if it has one.
27+
*
28+
* "Two faces": a module opens on its name, a line about what it is for, and one
29+
* knob — the rest is one click away and never in the way. Nothing is hidden; it
30+
* is ordered.
31+
*
32+
* `primary` is either a real parameter key or the string "strength", which means
33+
* the module wants a single DERIVED control over several parameters — the
34+
* `PROFILES` idea, whose figures are proposed rather than measured and which has
35+
* not shipped. Until it does, those five effects (compressor, gate, saturate,
36+
* reverb, bitcrush) open on all of their controls, which is honest: the one knob
37+
* they want does not exist yet, and inventing one would be a knob that lies.
38+
*/
39+
function primaryParamOf(def: HfAudioFxDef): string | null {
40+
const primary = EFFECT_COPY[def.id]?.primary;
41+
// "strength" names no parameter, which is exactly what makes this work: the
42+
// day `PROFILES` ships and a derived `strength` knob really is in the
43+
// registry, this starts using it without being told.
44+
if (!primary) return null;
45+
return def.params.some((p) => p.key === primary) ? primary : null;
46+
}
47+
2548
/**
2649
* The registry's definition with the plain names written over it.
2750
*
@@ -250,7 +273,17 @@ export function FxNodeRow({
250273
}: FxNodeRowProps) {
251274
const registryDef = getAudioFxDef(node.type);
252275
const def = useMemo(() => (registryDef ? plainDef(registryDef) : null), [registryDef]);
253-
if (!registryDef || !def) return null;
276+
const primary = registryDef ? primaryParamOf(registryDef) : null;
277+
/** The same def cut down to the one knob, so the open face reuses every wire. */
278+
const onlyPrimary = useMemo(
279+
() => (def && primary ? { ...def, params: def.params.filter((p) => p.key === primary) } : def),
280+
[def, primary],
281+
);
282+
// Local, because nothing outside the row needs to know. Keyed by node id like
283+
// the row itself, so it stays with its effect across a reorder.
284+
const [details, setDetails] = useState(false);
285+
if (!registryDef || !def || !onlyPrimary) return null;
286+
const copy = EFFECT_COPY[node.type];
254287
const bypassed = node.enabled === false;
255288
const params = node.params ?? defaultAudioFxParams(node.type);
256289
// What this effect is doing to the sound, as a sentence. The rack is read top
@@ -285,24 +318,68 @@ export function FxNodeRow({
285318
) : null}
286319
{open ? (
287320
<>
288-
{/* The DSP name, once, where the mechanism is. An author who wants to
289-
know what "Remove Rumble" really is finds out by opening it; one who
290-
does not never has to meet the word. */}
291-
<p className="hf-fx-node-mechanism border-t border-panel-border-input px-1.5 pt-1 font-mono text-[9px] uppercase tracking-wide text-panel-text-4">
292-
Details — {registryDef.label}
293-
</p>
294-
<FxNodeParams
295-
node={node}
296-
def={def}
297-
index={index}
298-
disabled={Boolean(disabled) || bypassed}
299-
automatedTargets={automatedTargets}
300-
liveAutomationValues={liveAutomationValues}
301-
onUpdate={onUpdate}
302-
onPreview={onPreview}
303-
onAutomateParam={onAutomateParam}
304-
onRemoveParamAutomation={onRemoveParamAutomation}
305-
/>
321+
{/* What it is for, before what it is made of. */}
322+
{copy?.does ? (
323+
<p className="hf-fx-node-does border-t border-panel-border-input px-1.5 py-1 text-[10px] text-panel-text-4">
324+
{copy.does}
325+
</p>
326+
) : null}
327+
{primary && !details ? (
328+
<>
329+
<FxNodeParams
330+
node={node}
331+
def={onlyPrimary}
332+
index={index}
333+
disabled={Boolean(disabled) || bypassed}
334+
automatedTargets={automatedTargets}
335+
liveAutomationValues={liveAutomationValues}
336+
onUpdate={onUpdate}
337+
onPreview={onPreview}
338+
onAutomateParam={onAutomateParam}
339+
onRemoveParamAutomation={onRemoveParamAutomation}
340+
/>
341+
{/* What the two ends of that knob sound like. A number tells an
342+
author where the control is; this tells them which way to move
343+
it, which is the question they actually have. */}
344+
{copy?.primaryEnds ? (
345+
<p className="hf-fx-node-ends flex justify-between gap-2 px-1.5 pb-1 text-[9px] text-panel-text-4">
346+
<span className="truncate">{copy.primaryEnds.low}</span>
347+
<span className="truncate text-right">{copy.primaryEnds.high}</span>
348+
</p>
349+
) : null}
350+
</>
351+
) : null}
352+
{/* The DSP name lives on the disclosure, so it is read at the moment
353+
the author asks what this really is — and never before. */}
354+
{primary ? (
355+
<button
356+
type="button"
357+
className="hf-fx-node-details flex w-full items-center gap-1 border-t border-panel-border-input px-1.5 py-1 text-left font-mono text-[9px] uppercase tracking-wide text-panel-text-4 hover:text-panel-text-0"
358+
aria-expanded={details}
359+
onClick={() => setDetails((was) => !was)}
360+
>
361+
<span aria-hidden="true">{details ? "\u25BE" : "\u25B8"}</span>
362+
Details — {registryDef.label}
363+
</button>
364+
) : (
365+
<p className="hf-fx-node-mechanism border-t border-panel-border-input px-1.5 pt-1 font-mono text-[9px] uppercase tracking-wide text-panel-text-4">
366+
Details — {registryDef.label}
367+
</p>
368+
)}
369+
{details || !primary ? (
370+
<FxNodeParams
371+
node={node}
372+
def={def}
373+
index={index}
374+
disabled={Boolean(disabled) || bypassed}
375+
automatedTargets={automatedTargets}
376+
liveAutomationValues={liveAutomationValues}
377+
onUpdate={onUpdate}
378+
onPreview={onPreview}
379+
onAutomateParam={onAutomateParam}
380+
onRemoveParamAutomation={onRemoveParamAutomation}
381+
/>
382+
) : null}
306383
</>
307384
) : null}
308385
</div>

packages/studio/src/components/editor/propertyPanelFxSection.test.tsx

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@ const click = (el: Element | null | undefined) => {
102102
(el as HTMLElement).click();
103103
});
104104
};
105+
/**
106+
* Open a module's Details, where every control that is not the primary one now
107+
* lives — a module opens on one knob and the rest is one click away.
108+
*/
109+
function openDetails(host: HTMLElement, index = 0): void {
110+
const buttons = Array.from(host.querySelectorAll<HTMLButtonElement>(".hf-fx-node-details"));
111+
const button = buttons[index];
112+
if (!button) throw new Error("no Details disclosure to open");
113+
act(() => button.click());
114+
}
115+
105116
const byText = (host: HTMLElement, sel: string, text: string) =>
106117
Array.from(host.querySelectorAll(sel)).find((e) => e.textContent?.trim() === text);
107118

@@ -284,6 +295,9 @@ describe("FxSection chain", () => {
284295
});
285296

286297
render(chainOfNodes(a, b));
298+
// Frequency is behind Details for a peaking node — the module opens on how
299+
// much, now that picking the module is what picks the range.
300+
openDetails(host);
287301
// Only the first card is open, which is the one being edited.
288302
const openFrequency = (): HTMLInputElement =>
289303
host.querySelector<HTMLInputElement>(".hf-fx-node .hf-fx-number")!;
@@ -295,6 +309,9 @@ describe("FxSection chain", () => {
295309
// The author moves that effect down; the other one takes the open slot.
296310
render(chainOfNodes(b, a));
297311

312+
// Its own Details, not the one that was open: the disclosure is per module,
313+
// so the effect arriving in the slot arrives closed like any other.
314+
openDetails(host);
298315
expect(openFrequency().value).toBe("1600");
299316
});
300317

@@ -331,9 +348,31 @@ describe("FxSection chain", () => {
331348
expect(name).not.toBe(getAudioFxDef("highpass")?.label);
332349
// And a sentence under it, so the rack reads top to bottom.
333350
expect(node.querySelector(".hf-fx-node-summary")?.textContent).toContain("Cutting everything");
334-
// The first node is open by default, which is where the DSP name lives.
335-
expect(node.querySelector(".hf-fx-node-mechanism")?.textContent).toContain(
336-
getAudioFxDef("highpass")?.label,
351+
// Open, it says what it is for and offers ONE knob — the rest is behind a
352+
// disclosure, which is also the only place the DSP name appears.
353+
expect(node.querySelector(".hf-fx-node-does")?.textContent).toBe(EFFECT_COPY.highpass?.does);
354+
expect(node.querySelectorAll(".hf-fx-row")).toHaveLength(1);
355+
const details = node.querySelector(".hf-fx-node-details");
356+
expect(details?.textContent).toContain(getAudioFxDef("highpass")?.label);
357+
expect(details?.getAttribute("aria-expanded")).toBe("false");
358+
359+
openDetails(host);
360+
expect(node.querySelectorAll(".hf-fx-row").length).toBe(
361+
getAudioFxDef("highpass")?.params.length,
362+
);
363+
});
364+
365+
it("opens a module on all of its controls when its one knob does not exist yet", () => {
366+
// Five effects want a single DERIVED control over several parameters — the
367+
// `PROFILES` idea, whose figures are proposed rather than measured. Until it
368+
// ships they open on everything, which is honest: inventing one knob for
369+
// them now would be a knob that lies about what it sets.
370+
const { host } = mount({ chain: chainOf("compressor") });
371+
const node = fxCard(host);
372+
expect(EFFECT_COPY.compressor?.primary).toBe("strength");
373+
expect(node.querySelector(".hf-fx-node-details")).toBeNull();
374+
expect(node.querySelectorAll(".hf-fx-row").length).toBe(
375+
getAudioFxDef("compressor")?.params.length,
337376
);
338377
});
339378

@@ -654,6 +693,9 @@ describe("FxSection chain", () => {
654693
// Persisting on every input event refreshes the preview, which reloads the
655694
// composition and restarts audio — that is what made playback stutter.
656695
const { host, onChainChange, onChainPreview } = mount({ chain: chainOf("peaking") });
696+
// Details, because this is about the drag mechanics on a real control and
697+
// the frequency it asserts on is not the one knob the module opens with.
698+
openDetails(host);
657699
const slider = fxCard(host).querySelector<HTMLInputElement>(".hf-fx-slider")!;
658700
act(() => slider.dispatchEvent(new Event("pointerdown", { bubbles: true })));
659701
for (const v of ["5000", "10000", "15000"]) {
@@ -705,6 +747,7 @@ describe("FxSection chain", () => {
705747
sourceOptions: [{ id: "vo", label: "Voiceover" }],
706748
};
707749
const { host, root } = renderInto(<FxSection {...shared} chain={chainOf("peaking")} />);
750+
openDetails(host);
708751
const slider = fxCard(host).querySelector<HTMLInputElement>(".hf-fx-slider")!;
709752
const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")?.set;
710753
act(() => slider.dispatchEvent(new Event("pointerdown", { bubbles: true })));
@@ -747,6 +790,7 @@ describe("FxSection chain", () => {
747790

748791
it("clamps a typed value into the renderable range", () => {
749792
const { host, onChainChange } = mount({ chain: chainOf("peaking") });
793+
openDetails(host);
750794
const input = fxCard(host).querySelector<HTMLInputElement>(".hf-fx-number")!;
751795
typeInto(input, "999999");
752796
// React delegates onBlur through focusout, which is the event that bubbles.
@@ -1024,7 +1068,9 @@ describe("automation in the panel", () => {
10241068
expect(row.querySelector<HTMLInputElement>('input[type="range"]')?.disabled).toBe(true);
10251069
expect(row.querySelector<HTMLInputElement>('input[type="number"]')?.disabled).toBe(true);
10261070
expect(row.hasAttribute("data-automated")).toBe(true);
1027-
// A sibling parameter on the same effect stays editable.
1071+
// A sibling parameter on the same effect stays editable — one click in,
1072+
// which is where every control that is not the primary one lives.
1073+
openDetails(host);
10281074
const q = rowFor(host, plainLabel("lowpass", "q"))!;
10291075
expect(q.querySelector<HTMLInputElement>('input[type="range"]')?.disabled).toBe(false);
10301076
});

0 commit comments

Comments
 (0)