Skip to content

Commit bb5442f

Browse files
committed
fix(studio): repeat audio FX reveal requests
1 parent d9b7f33 commit bb5442f

3 files changed

Lines changed: 54 additions & 22 deletions

File tree

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,48 @@ afterEach(() => {
162162
});
163163

164164
describe("FxSection chain", () => {
165+
it("scrolls again when the same open parameter is revealed twice", () => {
166+
const descriptor = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "scrollIntoView");
167+
const scrollIntoView = vi.fn();
168+
Object.defineProperty(HTMLElement.prototype, "scrollIntoView", {
169+
configurable: true,
170+
value: scrollIntoView,
171+
});
172+
const chain: HfAudioFxChain = {
173+
version: 1,
174+
nodes: [
175+
{
176+
type: "lowpass",
177+
id: "filter-1",
178+
enabled: true,
179+
params: defaultAudioFxParams("lowpass"),
180+
},
181+
],
182+
};
183+
const renderFxSection = (revealNonce: number) => (
184+
<FxSection
185+
chain={chain}
186+
onChainChange={vi.fn()}
187+
onCarveChange={vi.fn()}
188+
carve={null}
189+
sourceOptions={[]}
190+
revealTarget="fx.filter-1.frequency"
191+
revealNonce={revealNonce}
192+
/>
193+
);
194+
195+
try {
196+
const { root } = renderInto(renderFxSection(1));
197+
expect(scrollIntoView).toHaveBeenCalledTimes(1);
198+
199+
act(() => root.render(renderFxSection(2)));
200+
expect(scrollIntoView).toHaveBeenCalledTimes(2);
201+
} finally {
202+
if (descriptor) Object.defineProperty(HTMLElement.prototype, "scrollIntoView", descriptor);
203+
else Reflect.deleteProperty(HTMLElement.prototype, "scrollIntoView");
204+
}
205+
});
206+
165207
it("says so when the track has no effects", () => {
166208
const { host } = mount();
167209
// "other", because the carve module is in the rack whenever a voice exists.

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

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -323,16 +323,12 @@ export function FxSection({
323323
}, [handBuilt, eqIds.length, showCarve]);
324324
const [openEq, setOpenEq] = useState<string | null>(null);
325325

326-
/**
327-
* The parameter a reveal request asked for, held until its row is on screen.
328-
*
329-
* Set during render rather than in an effect, the way the Motion panel
330-
* consumes its own focus request: the surface must open on the SAME commit
331-
* the request lands on, or the scroll below runs against a row that has not
332-
* mounted yet.
333-
*/
326+
/** The reveal request held until its row is mounted and scrolled. */
334327
const [consumedRevealNonce, setConsumedRevealNonce] = useState<number | null>(null);
335-
const pendingRevealRef = useRef<string | null>(null);
328+
const [pendingReveal, setPendingReveal] = useState<{
329+
nonce: number;
330+
target: string;
331+
} | null>(null);
336332
const rootRef = useRef<HTMLDivElement | null>(null);
337333
if (revealNonce != null && revealNonce !== consumedRevealNonce) {
338334
setConsumedRevealNonce(revealNonce);
@@ -351,27 +347,26 @@ export function FxSection({
351347
return next;
352348
});
353349
}
354-
pendingRevealRef.current = revealTarget ?? null;
355350
}
351+
setPendingReveal(where && revealTarget ? { nonce: revealNonce, target: revealTarget } : null);
356352
}
357353

358354
/**
359355
* Scroll the revealed parameter into view once its row has actually mounted.
360356
*
361-
* Keyed on the surfaces the block above opens, not on the request: the row
362-
* appears on the commit AFTER they change, so scrolling in the same pass would
363-
* miss it. Cleared once used, so a later re-render does not yank the panel
364-
* back to a parameter the author has since scrolled away from.
357+
* The request itself is a dependency so a second click on an already-open
358+
* surface still scrolls. It is cleared once used, so a later unrelated
359+
* re-render does not yank the panel back to an old parameter.
365360
*/
366361
useEffect(() => {
367-
const target = pendingRevealRef.current;
362+
const target = pendingReveal?.target;
368363
if (target && scrollRevealedRowIntoView(rootRef.current, target, chain)) {
369-
pendingRevealRef.current = null;
364+
setPendingReveal(null);
370365
}
371366
// `chain` is deliberately not a dependency: it changes on every knob edit,
372367
// and re-running then would scroll the panel while the author is dragging.
373368
// eslint-disable-next-line react-hooks/exhaustive-deps
374-
}, [openNode, openEq, carveOpen, collapsedRuns]);
369+
}, [openNode, openEq, carveOpen, collapsedRuns, pendingReveal]);
375370

376371
const addEq = useCallback(() => {
377372
clearAudition();

packages/studio/src/components/editor/useFxCarve.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,6 @@ import { carveLanes, measureCarve, mintCarveNodes } from "./useFxCarveNodes.js";
3737
import { spanOf } from "./propertyPanelAudioFxGroupUtils.js";
3838
import type { AudioTrackOption } from "./propertyPanelFxCarveModule.js";
3939

40-
/**
41-
* Rate the carve source is decoded at. Analysis is self-consistent because it
42-
* reads the decoded buffer's own rate, so this only has to be a sane audio rate.
43-
*/
44-
4540
/**
4641
* Which carve setting actually moved, by comparing the two snapshots.
4742
*

0 commit comments

Comments
 (0)