From 7a6e7558ce99f5cb41818a9f448a9c74ad4a08d7 Mon Sep 17 00:00:00 2001 From: Xiaohan-Tian <157918347+Xiaohan-Tian@users.noreply.github.com> Date: Sat, 2 May 2026 23:02:37 -0700 Subject: [PATCH] fix: pause button while recording should stop recoding --- src/components/Toolbar.tsx | 8 ++- src/hooks/useGlobalKeyboardHandler.ts | 10 ++-- src/stores/projectStore.ts | 10 +++- .../project-store-sync.integration.test.ts | 53 ++++++++++++++++++- src/test/mocks/audio-interface.ts | 3 +- 5 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/components/Toolbar.tsx b/src/components/Toolbar.tsx index ca5d52f..87be89c 100644 --- a/src/components/Toolbar.tsx +++ b/src/components/Toolbar.tsx @@ -40,7 +40,7 @@ const Toolbar: React.FC = () => { projectName, setProjectName, savedProjectName, setSavedProjectName, bpm, timeSignature, keySignature, setStatus, - isPlaying, startPlaying, stopPlaying, setPlayheadPosition, + isPlaying, startPlaying, stopTransport, setPlayheadPosition, currentTime, setBpm, setTimeSignature, setKeySignature, maxBars, setMaxBars, barWidthMultiplier, setBarWidthMultiplier, @@ -518,12 +518,10 @@ const Toolbar: React.FC = () => { console.log("Pause button clicked"); } try { + await stopTransport(); if (isRecording) { - await stopRecording(); setStatus("Recording stopped — notes committed"); - return; } - await stopPlaying(); } catch (error) { console.error("Failed to stop playback:", error); setStatus("Failed to stop playback"); @@ -1109,4 +1107,4 @@ const Toolbar: React.FC = () => { ); }; -export default Toolbar; \ No newline at end of file +export default Toolbar; diff --git a/src/hooks/useGlobalKeyboardHandler.ts b/src/hooks/useGlobalKeyboardHandler.ts index 916f397..101274d 100644 --- a/src/hooks/useGlobalKeyboardHandler.ts +++ b/src/hooks/useGlobalKeyboardHandler.ts @@ -14,7 +14,7 @@ import { KGMidiRegion } from '../core/region/KGMidiRegion'; * Handles keyboard shortcuts defined in the configuration */ export const useGlobalKeyboardHandler = () => { - const { undo, redo, setStatus, isPlaying, startPlaying, stopPlaying, toggleLoop, projectName, savedProjectName, setSavedProjectName, setProjectName, isRecording, startRecording, stopRecording, activeRegionId, selectedRegionIds, setActiveRegionId, setShowPianoRoll } = useProjectStore(); + const { undo, redo, setStatus, isPlaying, startPlaying, stopTransport, toggleLoop, projectName, savedProjectName, setSavedProjectName, setProjectName, isRecording, startRecording, stopRecording, activeRegionId, selectedRegionIds, setActiveRegionId, setShowPianoRoll } = useProjectStore(); useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { @@ -134,8 +134,8 @@ export const useGlobalKeyboardHandler = () => { startPlaying(); setStatus('Playback started'); } else { - stopPlaying(); - setStatus('Playback stopped'); + stopTransport(); + setStatus(isRecording ? 'Recording stopped — notes committed' : 'Playback stopped'); } } catch (error) { console.error('Play/pause failed:', error); @@ -216,5 +216,5 @@ export const useGlobalKeyboardHandler = () => { return () => { document.removeEventListener('keydown', handleKeyDown, { capture: true }); }; - }, [undo, redo, setStatus, isPlaying, startPlaying, stopPlaying, toggleLoop, projectName, isRecording, startRecording, stopRecording, activeRegionId, selectedRegionIds, setActiveRegionId, setShowPianoRoll]); // Include dependencies for store actions -}; \ No newline at end of file + }, [undo, redo, setStatus, isPlaying, startPlaying, stopTransport, toggleLoop, projectName, savedProjectName, setSavedProjectName, setProjectName, isRecording, startRecording, stopRecording, activeRegionId, selectedRegionIds, setActiveRegionId, setShowPianoRoll]); // Include dependencies for store actions +}; diff --git a/src/stores/projectStore.ts b/src/stores/projectStore.ts index f7159a0..342815d 100644 --- a/src/stores/projectStore.ts +++ b/src/stores/projectStore.ts @@ -137,6 +137,7 @@ interface ProjectState { setAutoScrollEnabled: (enabled: boolean) => void; startPlaying: () => Promise; stopPlaying: () => Promise; + stopTransport: () => Promise; toggleLoop: () => void; toggleMetronome: () => void; setBpm: (bpm: number) => void; @@ -821,6 +822,14 @@ export const useProjectStore = create((set, get) => { set({ isPlaying: false }); }, + stopTransport: async () => { + if (get().isRecording) { + await get().stopRecording(); + return; + } + await get().stopPlaying(); + }, + startRecording: async () => { const { activeRegionId, timeSignature, playheadPosition, setPlayheadPosition } = get(); @@ -1242,4 +1251,3 @@ export const useProjectStore = create((set, get) => { - diff --git a/src/test/integration/store/project-store-sync.integration.test.ts b/src/test/integration/store/project-store-sync.integration.test.ts index bec552c..dcdbf28 100644 --- a/src/test/integration/store/project-store-sync.integration.test.ts +++ b/src/test/integration/store/project-store-sync.integration.test.ts @@ -3,7 +3,7 @@ * Tests the critical data flow: Store Actions → Core Models → UI State Updates */ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; -import { act, renderHook } from '@testing-library/react'; +import { act } from '@testing-library/react'; // Import core classes import { KGCore } from '../../../core/KGCore'; @@ -315,6 +315,57 @@ describe('Project Store Synchronization Integration Tests', () => { expect(recordingCallbacksSpy).toHaveBeenCalled(); expect(startPlayingSpy).toHaveBeenCalledWith({ preserveLoopPreroll: true }); }); + + it('should commit recording when transport is stopped during recording', async () => { + const testTrack = new KGMidiTrack('Recording Track', 0, 'acoustic_grand_piano'); + const testRegion = new KGMidiRegion('record-region', 'track-0', 0, 'Recording Region', 16, 16); + testTrack.addRegion(testRegion); + testProject.setTracks([testTrack]); + + await act(async () => { + await useProjectStore.getState().loadProject(testProject); + }); + + const core = KGCore.instance(); + vi.spyOn(core, 'startPlaying').mockResolvedValue(undefined); + const executeCommandSpy = vi.spyOn(core, 'executeCommand'); + const setRecordingCallbacksSpy = vi.spyOn(KGMidiInput.instance(), 'setRecordingCallbacks'); + vi.spyOn(mockAudioInterface, 'getTransportPosition').mockReturnValue(5); + + const { setActiveRegionId, setPlayheadPosition, startRecording, stopTransport } = useProjectStore.getState(); + + act(() => { + setActiveRegionId(testRegion.getId()); + setPlayheadPosition(4); + }); + + await act(async () => { + await startRecording(); + }); + + const noteOn = setRecordingCallbacksSpy.mock.calls.at(-1)?.[0]; + const noteOff = setRecordingCallbacksSpy.mock.calls.at(-1)?.[1]; + expect(noteOn).toBeTypeOf('function'); + expect(noteOff).toBeTypeOf('function'); + + act(() => { + noteOn?.(60); + noteOff?.(60); + }); + + await act(async () => { + await stopTransport(); + }); + + const storeState = useProjectStore.getState(); + expect(storeState.isRecording).toBe(false); + expect(storeState.isPlaying).toBe(false); + expect(storeState.recordingNotes).toHaveLength(0); + expect(executeCommandSpy).toHaveBeenCalled(); + expect(mockAudioInterface.stopPlayback).toHaveBeenCalled(); + expect(setRecordingCallbacksSpy).toHaveBeenLastCalledWith(null, null); + expect(testRegion.getNotes()).toHaveLength(1); + }); }); describe('Selection State Synchronization', () => { diff --git a/src/test/mocks/audio-interface.ts b/src/test/mocks/audio-interface.ts index 29f822d..5201046 100644 --- a/src/test/mocks/audio-interface.ts +++ b/src/test/mocks/audio-interface.ts @@ -30,6 +30,7 @@ export const mockAudioInterface = { // Transport getCurrentBeat: vi.fn().mockReturnValue(0), + getTransportPosition: vi.fn().mockReturnValue(0), setBpm: vi.fn().mockReturnValue(undefined), // Singleton pattern @@ -37,4 +38,4 @@ export const mockAudioInterface = { }; // Mock the class constructor -export const mockKGAudioInterfaceClass = vi.fn(() => mockAudioInterface); \ No newline at end of file +export const mockKGAudioInterfaceClass = vi.fn(() => mockAudioInterface);