TT-7761 TT-7762 TT-7763 TT-7765 cut paste buttons disabled properly

[sarahentzel]

No description provided.

[Copilot]

🟡 Changes recommended

Paste remains available during replacement-audio loading and can discard the incoming audio.

1 open finding

• Block paste until the player is ready · New

What changed in this PR

Updates waveform editing controls so cut, copy, and paste respect selection and busy states.

Changes:

• Tracks region selection and active paste operations.
• Disables editing during playback, saving, or duplicate paste attempts.
• Adds hotkey and button-state tests.

File	Description
web/​src/​components/​WSAudioPlayer.tsx	Updates waveform edit guards and control states.
web/​src/​components/​WSAudioPlayer.hotkeys.test.tsx	Tests selection, loading, playback, saving, and repeated paste behavior.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[gtryus]

I reviewed the four commits that touch just WSAudioPlayer.tsx and its hotkey test.

• The hasRegion → regionCount rename is cosmetic.

Smaller points, in the spirit of the article

1. regionSelected duplicates existing state. regionSelectedRef.current always equals currentSegmentRef.current !== undefined, except right after the blob-reset effect, which clears one but not the other. The article's "avoid redundant state" idea applies here. You could clear currentSegmentRef in the reset and have the handlers read currentSegmentRef.current directly. You would then need only the regionSelected state, which the render uses, and not the extra ref. As written, the two values can disagree after a blob change.

2. The new line in the blob effect is acceptable. setRegionSelected(false) is the article's "adjusting state when a prop changes" pattern. Here it sits inside an effect that already syncs with an external system (wsLoad / wsClear), so it adds no extra render pass. If wavesurfer already sends onCurrentSegment(undefined) when it loads or clears, the line is redundant, but I didn't check whether it does.

3. The enable rules are written twice and no longer match. This is the biggest practical issue. It isn't an effect problem, but it is the article's "compute it once during render" point:

   	Hotkey handler checks	Button disabled checks
   Cut	readyRef, regionSelectedRef	regionSelected (no ready check)
   Copy	regionSelectedRef (no ready check)	regionSelected
   Paste	readyRef, wsHasClipboard(), playingRef, mediaSaveInProgress, pasteBusyRef	playing, mediaSaveInProgress, pasteBusy, with canPaste hiding the button; no ready check

   For example, one commit blocks pasting by hotkey while the player isn't ready, but the Paste button stays enabled during that time. Clicking it does nothing, because handlePaste returns early. A fix would be to compute canCut, canCopy and canPaste once per render, use them for disabled, and have the handlers read the same values, either through a ref updated during render or by passing ready into the handler's dependencies.

4. The state-plus-ref pairs are justified. regionSelected/regionSelectedRef and pasteBusy/pasteBusyRef each have a combined setter. The state drives rendering and the ref lets hotkey callbacks stay stable. That matches existing code like setReady, so it's consistent, even though the article would prefer reading state directly.

Bottom line

The branch adds no effects and puts the new state changes in handlers, as the article advises. The one change worth making before merging is item 3: compute each button's enabled state once so the buttons and hotkeys agree. Item 1 is an optional cleanup.

[gtryus]

@sarahentzel were these the kinds of changes we talked about changing?

[sarahentzel]

only if the state variables are unnecessary and could be computed on the fly