mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-11-14 21:29:34 +00:00
fix(studio): let an FX parameter be typed, and stop a bare blur re-persisting it
Two failures with one root: the row never tracked whether a gesture actually
edited anything.
The number field could not be typed into. It was bound to the committed value,
while every keystroke was clamped into range and written live — and a live
write does not refresh the prop, so React put the old number straight back.
Typing 5000 into a 20..20000 knob wrote 20 on the first keystroke and got no
further; the knob could only be dragged. Worse, `Number("") === 0` passes
Number.isFinite, so select-all + Delete before retyping instantly live-wrote the
parameter MINIMUM — 20 Hz on a cutoff, -40 dB on a gain. The field now holds
what is being typed as text, and an empty field is a state on the way to a
number rather than a number to clamp and persist.
`commit()` hangs off pointerup, keyup and blur, all reachable with no edit —
clicking a slider thumb without moving it, or tabbing through the field. It
unconditionally wrote `latest`, which was seeded at mount and only ever written
by an edit. So: open an EQ row, run the carve or press undo, then click the
slider and release without moving it, and the row silently re-persisted its
mount-time number over the change. It also fired a full persisting write —
source patch, selection resync, preview reload, audio restart — for a gesture
that moved nothing.
Each fix is falsified by deletion. A fourth guard (resyncing `latest` on every
inbound value) was dropped rather than kept: the edit flag subsumes it, and no
test could be made to fail without it.
This commit is contained in:
@@ -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(
|
||||
<FxParamRow param={FREQUENCY} value={value} onChange={onChange} onCommit={onCommit} />,
|
||||
);
|
||||
});
|
||||
const number = () => host.querySelector<HTMLInputElement>(".hf-fx-number")!;
|
||||
const slider = () => host.querySelector<HTMLInputElement>(".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(
|
||||
<FxParamRow param={FREQUENCY} value={next} onChange={onChange} onCommit={onCommit} />,
|
||||
);
|
||||
});
|
||||
};
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -132,6 +132,26 @@ export function FxParamRow({
|
||||
* write applied on the way down.
|
||||
*/
|
||||
const [pending, setPending] = useState<number | null>(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<string | null>(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();
|
||||
}}
|
||||
|
||||
Reference in New Issue
Block a user