diff --git a/packages/studio/src/components/editor/propertyPanelFxControls.test.tsx b/packages/studio/src/components/editor/propertyPanelFxControls.test.tsx new file mode 100644 index 0000000000..e498875822 --- /dev/null +++ b/packages/studio/src/components/editor/propertyPanelFxControls.test.tsx @@ -0,0 +1,115 @@ +// @vitest-environment happy-dom +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { FxParamRow } from "./propertyPanelFxControls"; +import type { HfAudioFxNumberParam } from "@hyperframes/core/audio-fx"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +/** A frequency knob: wide range, so a clamp to the minimum is unmistakable. */ +const FREQUENCY: HfAudioFxNumberParam = { + kind: "number", + key: "frequency", + label: "Frequency", + min: 20, + max: 20000, + step: 1, + default: 1000, + unit: "Hz", +}; + +/** React tracks its own value on the node, so a plain assignment is ignored. */ +function setInputValue(input: HTMLInputElement, text: string): void { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")?.set; + setter?.call(input, text); + input.dispatchEvent(new Event("input", { bubbles: true })); +} + +function mount(value: number) { + const onChange = vi.fn(); + const onCommit = vi.fn(); + const host = document.createElement("div"); + document.body.append(host); + let root!: Root; + act(() => { + root = createRoot(host); + root.render( + , + ); + }); + const number = () => host.querySelector(".hf-fx-number")!; + const slider = () => host.querySelector(".hf-fx-slider")!; + const type = (text: string) => { + act(() => { + number().focus(); + setInputValue(number(), text); + }); + }; + /** A new value arrives through the prop — a carve, or an undo. */ + const receive = (next: number) => { + act(() => { + root.render( + , + ); + }); + }; + return { host, number, slider, onChange, onCommit, type, receive }; +} + +describe("FxParamRow number field", () => { + it("shows what is being typed instead of snapping back to the stored value", () => { + // Every keystroke is clamped and written live, and the live write does not + // refresh the prop — so a field bound to the committed value put the old + // number straight back. Typing 5000 wrote 20 on the first keystroke and the + // knob could not be typed into at all, only dragged. + const { number, type } = mount(1000); + type("5"); + expect(number().value).toBe("5"); + type("5000"); + expect(number().value).toBe("5000"); + }); + + it("does not write the parameter minimum while the field is empty", () => { + // `Number("") === 0` passes Number.isFinite, so select-all + Delete before + // retyping used to clamp to the minimum and live-write it — 20 Hz here. + const { onChange, type } = mount(1000); + type(""); + expect(onChange).not.toHaveBeenCalled(); + type("800"); + expect(onChange).toHaveBeenLastCalledWith("frequency", 800); + }); +}); + +describe("FxParamRow commit", () => { + it("stays quiet when a gesture changed nothing", () => { + // commit() hangs off pointerup, keyup and blur, all reachable without an + // edit. Firing then costs a source patch, a selection resync, a preview + // reload and an audio restart for a gesture that moved nothing. + const { slider, number, onCommit } = mount(1000); + act(() => slider().dispatchEvent(new Event("pointerup", { bubbles: true }))); + act(() => number().dispatchEvent(new Event("blur", { bubbles: true }))); + expect(onCommit).not.toHaveBeenCalled(); + }); + + it("does not re-persist a stale value over a change that arrived meanwhile", () => { + // The scenario: open an EQ row, run the carve (or press undo), which + // rewrites the chain so this row's value becomes 400. Then click the slider + // thumb and release without moving it. `latest` was seeded at mount and only + // written by an edit, so it still held 1000 — and the release wrote it back, + // undoing the carve with no gesture that looks like an edit. + const { slider, onCommit, type, receive } = mount(1000); + // An edit happened earlier in this row's life, so `latest` holds 1000. + type("1000"); + act(() => slider().dispatchEvent(new Event("pointerup", { bubbles: true }))); + onCommit.mockClear(); + + receive(400); + act(() => slider().dispatchEvent(new Event("pointerup", { bubbles: true }))); + expect(onCommit).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/studio/src/components/editor/propertyPanelFxControls.tsx b/packages/studio/src/components/editor/propertyPanelFxControls.tsx index 4fd3254bdb..63d7bfd6da 100644 --- a/packages/studio/src/components/editor/propertyPanelFxControls.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxControls.tsx @@ -132,6 +132,26 @@ export function FxParamRow({ * write applied on the way down. */ const [pending, setPending] = useState(null); + /** + * What the number field is showing while it has focus. + * + * The field cannot be bound to the committed value: every keystroke is + * clamped into range and written live, and the live write does not refresh + * the prop — so React put the old number straight back and typing `5000` into + * a 20..20000 knob wrote 20 on the first keystroke and never got further. Held + * as TEXT, so a half-typed "-" or "" is a state the field can be in rather + * than a number to clamp and persist. + */ + const [typing, setTyping] = useState(null); + /** + * Did this gesture actually change anything? + * + * `commit()` hangs off pointerup, keyup and blur, all of which fire without + * an edit — clicking a slider thumb without moving it, or tabbing through the + * field. Committing then re-persisted whatever `latest` happened to hold and + * silently reverted any change that had arrived meanwhile. + */ + const edited = useRef(false); useEffect(() => { if (!dragging) setLocal(value); }, [value, dragging]); @@ -146,6 +166,7 @@ export function FxParamRow({ const p = param as HfAudioFxNumberParam; const next = Math.min(p.max, Math.max(p.min, raw)); latest.current = next; + edited.current = true; setLocal(next); onChange(param.key, next); }, @@ -154,6 +175,12 @@ export function FxParamRow({ const commit = useCallback(() => { setDragging(false); + // Nothing was edited, so there is nothing to persist. Without this a bare + // focus/blur — or a click on the slider thumb that never moved — fired a + // full persisting write: source patch, selection resync, preview reload and + // an audio restart, for a gesture that changed no value. + if (!edited.current) return; + edited.current = false; if (typeof latest.current === "number") setPending(latest.current); onCommit?.(param.key, latest.current); }, [onCommit, param.key]); @@ -228,13 +255,25 @@ export function FxParamRow({ min={param.min} max={param.max} step={param.step} - value={display(param, current)} + value={typing ?? display(param, current)} disabled={locked} + onFocus={() => setTyping(display(param, current))} onChange={(e) => { - const next = Number(e.target.value); + const text = e.target.value; + setTyping(text); + // An empty field, a lone "-", or a trailing "." are all states on the + // way to a number, not numbers. `Number("")` is 0, which passes + // Number.isFinite — so clearing the field to retype used to clamp to + // the parameter MINIMUM and write it live: 20 Hz on a cutoff, -40 dB + // on a gain. + if (text.trim() === "") return; + const next = Number(text); if (Number.isFinite(next)) handleNumber(next); }} - onBlur={commit} + onBlur={() => { + commit(); + setTyping(null); + }} onKeyDown={(e) => { if (e.key === "Enter") commit(); }}