Skip to content

refactor(studio): lift the effect row out of the FX section file - #3182

Merged
vanceingalls merged 695 commits into
mainfrom
wa-20b1b-row-extract
Aug 13, 2026
Merged

refactor(studio): lift the effect row out of the FX section file#3182
vanceingalls merged 695 commits into
mainfrom
wa-20b1b-row-extract

Conversation

@vanceingalls

Copy link
Copy Markdown
Collaborator

The second extraction. What remains that is not the section's own job is one chain entry's UI — the row, its header, its params, and the automation-key helper that exists only to feed them.

Section is 401 lines now, under the cap. Pure move again.

🤖 Generated with Claude Code

@vanceingalls
vanceingalls force-pushed the wa-20b1a-carve-extract branch from 1e30acc to 881608d Compare August 11, 2026 08:34

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Byte-identical MOVE at ee9f52216c85; single-line delta (function FxNodeRow(export function FxNodeRow(). Zero hooks/refs/effects in the extracted subtree.

Call site at propertyPanelFxSection.tsx:295-317 is prop-for-prop byte-identical to the base call site — including the load-bearing key={node.id ?? \${node.type}-${i}`}` and its motivating comment.

Tests untouched (propertyPanelFxSection.test.tsx diff = ∅). Extracted behavior stays transitively pinned by the existing 895L section suite via the preserved DOM selectors (hf-fx-node, hf-fx-bypass, hf-fx-move, hf-fx-remove, etc.) and text strings.

OLD-assumption counterfactuals (pinned):

  • Key preserves row identity across reorder — keeps a half-typed value ... across a reorder (line 169).
  • Bypass toggles without deletion — bypasses without removing (line 152).
  • Preview/commit split during drag — previews while dragging and only persists on release (line 423).
  • Move-button edge disabling — cannot move the ends past themselves (line 408).
  • Preset label wins over def label — shows a preset node by the job it is doing (line 256).

Three invariants (live-automation-values map keying, onAutomate/onRemoveAutomation guard, null-render on unknown type) unpinned at the row seam but low-risk for a byte-identical move.

CI: same pre-existing .md preflight-red as #3181; nothing new to this PR.

— Review by tai (pr-review)

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: refactor(studio): lift the effect row out of the FX section file — #3182

Verdict: LGTM

Textbook file-extraction refactor. +260/-247 with zero behavioral delta.

  • FxNodeRowProps, FxMoveButton, FxNodeHeader, automatedKeysOf, FxNodeParams, FxNodeRow — all byte-identical to the removed code, only FxNodeRow gains the export keyword.
  • Import housekeeping correct: old file drops getAudioFxDef, HfAudioFxDef, fxAutomationTarget, FxParams (only used by moved code), retains defaultAudioFxParams which it still uses.
  • No hooks were added, removed, or reordered. Component identity and reconciliation behavior unchanged.

Same pattern as #3181, same verdict. Ship it.


Review by Miga

🤖 Generated with Claude Code

# Conflicts:
#	packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx
#	packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx
#	packages/studio/src/components/editor/propertyPanelFxSection.tsx
#	skills-manifest.json
#	skills/hyperframes-audio/SKILL.md
#	skills/hyperframes-audio/references/attributes.md
#	skills/hyperframes-audio/scripts/carve.mjs
Base automatically changed from wa-20b1a-carve-extract to main August 13, 2026 15:08
@vanceingalls
vanceingalls merged commit 2f97fc2 into main Aug 13, 2026
46 checks passed
@vanceingalls
vanceingalls deleted the wa-20b1b-row-extract branch August 13, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants