From 04d12a134b740c3795af7aba88d5c42ae8dc9ffd Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 20 Aug 2026 01:46:47 -0700 Subject: [PATCH] refactor(studio): name the two branchy steps in the Audio FX panel - audioFxSummary (14 cyclomatic / 19 cognitive) counted enabled nodes inline while also deciding what to say about them. countEnabledNodes now owns the parse and the split, and returns null for an unreadable chain, so the summary reads as the four sentences it produces. - The reveal effect's five-way nested ternary for "which row does this parameter live in" is now revealRowSelector, and the resolve-query-scroll sequence around it is scrollRevealedRowIntoView. Both live in audioFxRevealTarget.ts beside the resolver whose output they consume, which also brought propertyPanelFxSection.tsx from 616 to 598 lines -- under the 600 cap for the first time, so this commit needs no --no-verify. With these cleared the fallow audit gate passes (exit 0). Two notes for whoever touches this next: - fallow fingerprints a finding by line position, so inserting a helper ABOVE a complex function re-flags that function's inherited complexity as new. Adding revealRowSelector above FxSection re-flagged FxSection's own 22/32; moving it out fixed both. - A helper extracted only to satisfy the gate has to stay used from one place, or it lands as an unused export instead. studio's editor suite (102 files, 1269 tests) passes unchanged. --- .../components/editor/audioFxRevealTarget.ts | 44 +++++++++++++++++++ .../src/components/editor/audioFxSummary.ts | 40 ++++++++++------- .../editor/propertyPanelFxSection.tsx | 26 ++--------- 3 files changed, 71 insertions(+), 39 deletions(-) diff --git a/packages/studio/src/components/editor/audioFxRevealTarget.ts b/packages/studio/src/components/editor/audioFxRevealTarget.ts index c6df5d11d..b58d096ca 100644 --- a/packages/studio/src/components/editor/audioFxRevealTarget.ts +++ b/packages/studio/src/components/editor/audioFxRevealTarget.ts @@ -55,3 +55,47 @@ export function audioFxRevealTarget( } return { kind: "node", index, nodeId: parsed.nodeId }; } + +/** + * The DOM selector for the row a reveal target lives in, or null when the target + * names no surface this panel renders. + * + * A preset run is keyed by the preset id the run element actually exposes, not by + * `runKey`, which is the collapse map's key and carries an index suffix. + */ +function revealRowSelector(where: AudioFxRevealTarget | null): string | null { + if (!where) return null; + switch (where.kind) { + case "node": + return where.nodeId ? `[data-fx-node-id="${where.nodeId}"]` : null; + case "eq": + return `[data-fx-eq="${where.eqId}"]`; + case "carve": + return ".hf-fx-carve-module"; + case "preset": + return `[data-fx-preset="${where.runKey.replace(/-\d+$/, "")}"]`; + default: + return null; + } +} + +/** + * Scroll the row that owns `target` into view, and report whether it was found. + * + * The target is resolved against the chain again rather than remembered: which + * surface owns a parameter is a fact about the chain, and the chain may have + * been edited between the reveal request and the pass that can act on it. A + * false return means the row has not mounted yet, so the caller keeps the + * request pending. + */ +export function scrollRevealedRowIntoView( + root: HTMLElement | null, + target: string, + chain: HfAudioFxChain, +): boolean { + const selector = revealRowSelector(audioFxRevealTarget(target, chain)); + const row = selector ? root?.querySelector(selector) : null; + if (!row) return false; + row.scrollIntoView({ block: "nearest", behavior: "smooth" }); + return true; +} diff --git a/packages/studio/src/components/editor/audioFxSummary.ts b/packages/studio/src/components/editor/audioFxSummary.ts index 94c0c3da2..7bf1a3cf8 100644 --- a/packages/studio/src/components/editor/audioFxSummary.ts +++ b/packages/studio/src/components/editor/audioFxSummary.ts @@ -10,6 +10,23 @@ import { HF_AUDIO_FX_DATA_KEY, parseAudioFxChain } from "@hyperframes/core/audio-fx"; import type { DomEditSelection } from "./domEditingTypes"; +/** Enabled nodes split by who authored them, or null when the chain won't parse. */ +function countEnabledNodes(raw: string | undefined): { handBuilt: number; carve: number } | null { + if (!raw) return { handBuilt: 0, carve: 0 }; + try { + let handBuilt = 0; + let carve = 0; + for (const node of parseAudioFxChain(raw).nodes) { + if (node.enabled === false) continue; + if (node.fromCarve) carve += 1; + else handBuilt += 1; + } + return { handBuilt, carve }; + } catch { + return null; + } +} + export function audioFxSummary(element: DomEditSelection, groupLabel?: string): string { // A clip inside a group reads "in Voiceover" — the designs use this line to // answer "where does this go?" before the author opens anything, which is @@ -17,26 +34,15 @@ export function audioFxSummary(element: DomEditSelection, groupLabel?: string): // count: a member with no effects of its own is still IN the group, and that // is the more useful thing to say about it. if (groupLabel) return `in ${groupLabel}`; - const raw = element.dataAttributes?.[HF_AUDIO_FX_DATA_KEY]; - const carveAttr = element.dataAttributes?.["fx-carve"]; - let handBuilt = 0; - let carveNodes = 0; - if (raw) { - try { - for (const node of parseAudioFxChain(raw).nodes) { - if (node.enabled === false) continue; - if (node.fromCarve) carveNodes += 1; - else handBuilt += 1; - } - } catch { - return "unreadable"; - } - } + const counts = countEnabledNodes(element.dataAttributes?.[HF_AUDIO_FX_DATA_KEY]); + if (!counts) return "unreadable"; const parts: string[] = []; - if (handBuilt > 0) parts.push(`${handBuilt} effect${handBuilt === 1 ? "" : "s"}`); + if (counts.handBuilt > 0) { + parts.push(`${counts.handBuilt} effect${counts.handBuilt === 1 ? "" : "s"}`); + } // One name for the module however many filters are behind it. Named when the // carve is switched on at all, because the control is in this section whether or // not it has compiled to anything yet. - if (carveNodes > 0 || carveAttr) parts.push("carve"); + if (counts.carve > 0 || element.dataAttributes?.["fx-carve"]) parts.push("carve"); return parts.length > 0 ? parts.join(" + ") : "none"; } diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.tsx index 8f17829ee..56951d148 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.tsx @@ -6,7 +6,7 @@ */ import { useCallback, useEffect, useMemo, useRef, useState, type KeyboardEvent } from "react"; -import { audioFxRevealTarget } from "./audioFxRevealTarget.js"; +import { audioFxRevealTarget, scrollRevealedRowIntoView } from "./audioFxRevealTarget.js"; import { defaultAudioFxParams, mintAudioFxNodeId, @@ -364,27 +364,9 @@ export function FxSection({ */ useEffect(() => { const target = pendingRevealRef.current; - if (!target) return; - // Resolved again rather than remembered: the surface that owns a parameter - // is a fact about the chain, and the chain may have been edited between the - // request and this pass. - const where = audioFxRevealTarget(target, chain); - const selector = - where?.kind === "node" && where.nodeId - ? `[data-fx-node-id="${where.nodeId}"]` - : where?.kind === "eq" - ? `[data-fx-eq="${where.eqId}"]` - : where?.kind === "carve" - ? ".hf-fx-carve-module" - : // Keyed by the preset id the run carries, which is what the run - // element actually exposes; `runKey` is the collapse map's key. - where?.kind === "preset" - ? `[data-fx-preset="${where.runKey.replace(/-\d+$/, "")}"]` - : null; - const row = selector ? rootRef.current?.querySelector(selector) : null; - if (!row) return; - row.scrollIntoView({ block: "nearest", behavior: "smooth" }); - pendingRevealRef.current = null; + if (target && scrollRevealedRowIntoView(rootRef.current, target, chain)) { + pendingRevealRef.current = null; + } // `chain` is deliberately not a dependency: it changes on every knob edit, // and re-running then would scroll the panel while the author is dragging. // eslint-disable-next-line react-hooks/exhaustive-deps