fix: random not able to exit piano roll hybrid mode issue when switching between different MIDI regions (introduced a region dragging start threshold)
This commit is contained in:
@@ -439,8 +439,9 @@ const MainContent: React.FC<MainContentProps> = ({
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If piano roll is visible, set this region as the active region
|
// Keep the active piano roll region in sync when that same region is updated.
|
||||||
if (showPianoRoll) {
|
// Do not switch the editor to an unrelated region from generic move/resize updates.
|
||||||
|
if (showPianoRoll && activeRegionId === regionId) {
|
||||||
setActiveRegionId(regionId);
|
setActiveRegionId(regionId);
|
||||||
|
|
||||||
if (DEBUG_MODE.MAIN_CONTENT) {
|
if (DEBUG_MODE.MAIN_CONTENT) {
|
||||||
|
|||||||
@@ -0,0 +1,97 @@
|
|||||||
|
import React from 'react';
|
||||||
|
import { describe, it, expect, beforeAll, beforeEach, vi } from 'vitest';
|
||||||
|
import { render, fireEvent } from '@testing-library/react';
|
||||||
|
import RegionItem from './RegionItem';
|
||||||
|
import { KGMidiRegion } from '../../core/region/KGMidiRegion';
|
||||||
|
import { KGMainContentState } from '../../core/state/KGMainContentState';
|
||||||
|
|
||||||
|
vi.mock('../../stores/projectStore', () => ({
|
||||||
|
useProjectStore: () => ({
|
||||||
|
selectedRegionIds: [],
|
||||||
|
timeSignature: { numerator: 4, denominator: 4 },
|
||||||
|
bpm: 120,
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('RegionItem', () => {
|
||||||
|
beforeAll(() => {
|
||||||
|
Object.defineProperty(HTMLCanvasElement.prototype, 'getContext', {
|
||||||
|
value: vi.fn(() => ({
|
||||||
|
clearRect: vi.fn(),
|
||||||
|
fillRect: vi.fn(),
|
||||||
|
})),
|
||||||
|
});
|
||||||
|
|
||||||
|
class ResizeObserverMock {
|
||||||
|
observe() {}
|
||||||
|
unobserve() {}
|
||||||
|
disconnect() {}
|
||||||
|
}
|
||||||
|
|
||||||
|
vi.stubGlobal('ResizeObserver', ResizeObserverMock);
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
KGMainContentState.instance().setActiveTool('pointer');
|
||||||
|
});
|
||||||
|
|
||||||
|
const renderRegion = (props: Partial<React.ComponentProps<typeof RegionItem>> = {}) => {
|
||||||
|
const midiRegion = new KGMidiRegion('midi-1', 'track-1', 0, 'Test Region', 0, 4);
|
||||||
|
|
||||||
|
return render(
|
||||||
|
<RegionItem
|
||||||
|
id="midi-1"
|
||||||
|
name="Test Region"
|
||||||
|
style={{ left: '0px', width: '120px', position: 'absolute' }}
|
||||||
|
onClick={vi.fn()}
|
||||||
|
onDragStart={vi.fn()}
|
||||||
|
onDrag={vi.fn()}
|
||||||
|
onDragEnd={vi.fn()}
|
||||||
|
midiRegion={midiRegion}
|
||||||
|
{...props}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
it('treats small pointer jitter as a click', () => {
|
||||||
|
const onClick = vi.fn();
|
||||||
|
const onDragStart = vi.fn();
|
||||||
|
const onDrag = vi.fn();
|
||||||
|
const onDragEnd = vi.fn();
|
||||||
|
|
||||||
|
const { container } = renderRegion({ onClick, onDragStart, onDrag, onDragEnd });
|
||||||
|
const region = container.querySelector('.track-region');
|
||||||
|
|
||||||
|
expect(region).toBeTruthy();
|
||||||
|
|
||||||
|
fireEvent.mouseDown(region!, { clientX: 100, clientY: 100 });
|
||||||
|
fireEvent.mouseMove(document, { clientX: 102, clientY: 102 });
|
||||||
|
fireEvent.mouseUp(document, { clientX: 102, clientY: 102 });
|
||||||
|
|
||||||
|
expect(onClick).toHaveBeenCalledWith('midi-1');
|
||||||
|
expect(onDragStart).not.toHaveBeenCalled();
|
||||||
|
expect(onDrag).not.toHaveBeenCalled();
|
||||||
|
expect(onDragEnd).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('starts a drag after crossing the movement threshold', () => {
|
||||||
|
const onClick = vi.fn();
|
||||||
|
const onDragStart = vi.fn();
|
||||||
|
const onDrag = vi.fn();
|
||||||
|
const onDragEnd = vi.fn();
|
||||||
|
|
||||||
|
const { container } = renderRegion({ onClick, onDragStart, onDrag, onDragEnd });
|
||||||
|
const region = container.querySelector('.track-region');
|
||||||
|
|
||||||
|
expect(region).toBeTruthy();
|
||||||
|
|
||||||
|
fireEvent.mouseDown(region!, { clientX: 100, clientY: 100 });
|
||||||
|
fireEvent.mouseMove(document, { clientX: 110, clientY: 100 });
|
||||||
|
fireEvent.mouseUp(document, { clientX: 110, clientY: 100 });
|
||||||
|
|
||||||
|
expect(onClick).not.toHaveBeenCalled();
|
||||||
|
expect(onDragStart).toHaveBeenCalledWith('midi-1', 100, 100);
|
||||||
|
expect(onDrag).toHaveBeenCalledWith('midi-1', 10, 0);
|
||||||
|
expect(onDragEnd).toHaveBeenCalledWith('midi-1');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -9,6 +9,8 @@ import { KGAudioRegion } from '../../core/region/KGAudioRegion';
|
|||||||
import { useProjectStore } from '../../stores/projectStore';
|
import { useProjectStore } from '../../stores/projectStore';
|
||||||
import { KGMainContentState } from '../../core/state/KGMainContentState';
|
import { KGMainContentState } from '../../core/state/KGMainContentState';
|
||||||
|
|
||||||
|
const DRAG_START_THRESHOLD_PX = 4;
|
||||||
|
|
||||||
interface RegionItemProps {
|
interface RegionItemProps {
|
||||||
id: string;
|
id: string;
|
||||||
name: string;
|
name: string;
|
||||||
@@ -73,7 +75,7 @@ const RegionItem: React.FC<RegionItemProps> = ({
|
|||||||
// Use refs to track states for immediate access
|
// Use refs to track states for immediate access
|
||||||
const isResizingRef = useRef<boolean>(false);
|
const isResizingRef = useRef<boolean>(false);
|
||||||
const isDraggingRef = useRef<boolean>(false);
|
const isDraggingRef = useRef<boolean>(false);
|
||||||
const hasMovedRef = useRef<boolean>(false);
|
const isPendingDragRef = useRef<boolean>(false);
|
||||||
|
|
||||||
// Canvas ref for note visualization
|
// Canvas ref for note visualization
|
||||||
const canvasRef = useRef<HTMLCanvasElement | null>(null);
|
const canvasRef = useRef<HTMLCanvasElement | null>(null);
|
||||||
@@ -391,7 +393,7 @@ const RegionItem: React.FC<RegionItemProps> = ({
|
|||||||
const activeTool = KGMainContentState.instance().getActiveTool();
|
const activeTool = KGMainContentState.instance().getActiveTool();
|
||||||
if (activeTool === 'pencil') {
|
if (activeTool === 'pencil') {
|
||||||
// Still allow click events to pass through for region selection
|
// Still allow click events to pass through for region selection
|
||||||
if (!hasMovedRef.current && onClick) {
|
if (onClick) {
|
||||||
if (DEBUG_MODE.REGION_ITEM) {
|
if (DEBUG_MODE.REGION_ITEM) {
|
||||||
console.log(`REGION CLICKED (pencil mode): regionId=${id}`);
|
console.log(`REGION CLICKED (pencil mode): regionId=${id}`);
|
||||||
}
|
}
|
||||||
@@ -404,7 +406,7 @@ const RegionItem: React.FC<RegionItemProps> = ({
|
|||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
|
|
||||||
// Reset movement tracking
|
// Reset movement tracking
|
||||||
hasMovedRef.current = false;
|
isPendingDragRef.current = false;
|
||||||
|
|
||||||
// Store initial mouse position
|
// Store initial mouse position
|
||||||
initialMousePosRef.current = { x: e.clientX, y: e.clientY };
|
initialMousePosRef.current = { x: e.clientX, y: e.clientY };
|
||||||
@@ -423,21 +425,12 @@ const RegionItem: React.FC<RegionItemProps> = ({
|
|||||||
onResizeStart(id, resizeEdge, e.clientX);
|
onResizeStart(id, resizeEdge, e.clientX);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Start dragging
|
// Wait for actual pointer movement before promoting this gesture to a drag.
|
||||||
if (DEBUG_MODE.REGION_ITEM) {
|
if (DEBUG_MODE.REGION_ITEM) {
|
||||||
console.log(`DRAG START: regionId=${id}`);
|
console.log(`PENDING REGION INTERACTION: regionId=${id}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
setIsDragging(true);
|
isPendingDragRef.current = true;
|
||||||
isDraggingRef.current = true;
|
|
||||||
|
|
||||||
// Change cursor to grabbing during drag
|
|
||||||
setCursor('grabbing');
|
|
||||||
|
|
||||||
// Call the onDragStart callback if provided
|
|
||||||
if (onDragStart) {
|
|
||||||
onDragStart(id, e.clientX, e.clientY);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add global event listeners for mouse move and up
|
// Add global event listeners for mouse move and up
|
||||||
@@ -447,9 +440,6 @@ const RegionItem: React.FC<RegionItemProps> = ({
|
|||||||
|
|
||||||
// Handle global mouse move for resize or drag
|
// Handle global mouse move for resize or drag
|
||||||
const handleGlobalMouseMove = (e: MouseEvent) => {
|
const handleGlobalMouseMove = (e: MouseEvent) => {
|
||||||
// Set the hasMovedRef to true as soon as there's movement
|
|
||||||
hasMovedRef.current = true;
|
|
||||||
|
|
||||||
if (isResizingRef.current) {
|
if (isResizingRef.current) {
|
||||||
// Handle resize
|
// Handle resize
|
||||||
if (DEBUG_MODE.REGION_ITEM) {
|
if (DEBUG_MODE.REGION_ITEM) {
|
||||||
@@ -463,16 +453,35 @@ const RegionItem: React.FC<RegionItemProps> = ({
|
|||||||
if (onResize) {
|
if (onResize) {
|
||||||
onResize(id, resizeEdge, deltaX);
|
onResize(id, resizeEdge, deltaX);
|
||||||
}
|
}
|
||||||
} else if (isDraggingRef.current) {
|
} else if (isDraggingRef.current || isPendingDragRef.current) {
|
||||||
|
const deltaX = e.clientX - initialMousePosRef.current.x;
|
||||||
|
const deltaY = e.clientY - initialMousePosRef.current.y;
|
||||||
|
const movedEnough = Math.hypot(deltaX, deltaY) >= DRAG_START_THRESHOLD_PX;
|
||||||
|
|
||||||
|
if (!isDraggingRef.current) {
|
||||||
|
if (!movedEnough) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (DEBUG_MODE.REGION_ITEM) {
|
||||||
|
console.log(`DRAG START: regionId=${id}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
isPendingDragRef.current = false;
|
||||||
|
setIsDragging(true);
|
||||||
|
isDraggingRef.current = true;
|
||||||
|
setCursor('grabbing');
|
||||||
|
|
||||||
|
if (onDragStart) {
|
||||||
|
onDragStart(id, initialMousePosRef.current.x, initialMousePosRef.current.y);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Handle drag
|
// Handle drag
|
||||||
if (DEBUG_MODE.REGION_ITEM) {
|
if (DEBUG_MODE.REGION_ITEM) {
|
||||||
console.log(`DRAG MOVE: regionId=${id}, trackIndex=${trackIndex}`);
|
console.log(`DRAG MOVE: regionId=${id}, trackIndex=${trackIndex}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Calculate delta from initial position
|
|
||||||
const deltaX = e.clientX - initialMousePosRef.current.x;
|
|
||||||
const deltaY = e.clientY - initialMousePosRef.current.y;
|
|
||||||
|
|
||||||
// Call the onDrag callback if provided
|
// Call the onDrag callback if provided
|
||||||
if (onDrag) {
|
if (onDrag) {
|
||||||
onDrag(id, deltaX, deltaY);
|
onDrag(id, deltaX, deltaY);
|
||||||
@@ -511,16 +520,15 @@ const RegionItem: React.FC<RegionItemProps> = ({
|
|||||||
if (onDragEnd) {
|
if (onDragEnd) {
|
||||||
onDragEnd(id);
|
onDragEnd(id);
|
||||||
}
|
}
|
||||||
|
} else if (isPendingDragRef.current && onClick) {
|
||||||
// If there was no movement, treat it as a click
|
if (DEBUG_MODE.REGION_ITEM) {
|
||||||
if (!hasMovedRef.current && onClick) {
|
console.log(`REGION CLICKED: regionId=${id}`);
|
||||||
if (DEBUG_MODE.REGION_ITEM) {
|
|
||||||
console.log(`REGION CLICKED: regionId=${id}`);
|
|
||||||
}
|
|
||||||
onClick(id);
|
|
||||||
}
|
}
|
||||||
|
onClick(id);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
isPendingDragRef.current = false;
|
||||||
|
|
||||||
// Remove global event listeners
|
// Remove global event listeners
|
||||||
document.removeEventListener('mousemove', handleGlobalMouseMove);
|
document.removeEventListener('mousemove', handleGlobalMouseMove);
|
||||||
document.removeEventListener('mouseup', handleGlobalMouseUp);
|
document.removeEventListener('mouseup', handleGlobalMouseUp);
|
||||||
|
|||||||
@@ -492,6 +492,13 @@ const TrackGridItem: React.FC<TrackGridItemProps> = ({
|
|||||||
currentDragTop.current = null;
|
currentDragTop.current = null;
|
||||||
currentDragRegion.current = null;
|
currentDragRegion.current = null;
|
||||||
|
|
||||||
|
if (finalBarNumber === region.barNumber && finalTrackIndex === region.trackIndex) {
|
||||||
|
if (DEBUG_MODE.TRACK_GRID_ITEM) {
|
||||||
|
console.log(`Skipping no-op drag update for region ${regionId}`);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Notify parent about drag end with final values
|
// Notify parent about drag end with final values
|
||||||
if (onRegionDragEnd) {
|
if (onRegionDragEnd) {
|
||||||
onRegionDragEnd(regionId, finalBarNumber, finalTrackIndex);
|
onRegionDragEnd(regionId, finalBarNumber, finalTrackIndex);
|
||||||
|
|||||||
@@ -387,6 +387,13 @@ const TrackGridPanel: React.FC<TrackGridPanelProps> = ({
|
|||||||
const region = regions.find(r => r.id === regionId);
|
const region = regions.find(r => r.id === regionId);
|
||||||
if (!region) return;
|
if (!region) return;
|
||||||
|
|
||||||
|
if (finalBarNumber === region.barNumber && finalTrackIndex === region.trackIndex) {
|
||||||
|
if (DEBUG_MODE.TRACK_GRID_PANEL) {
|
||||||
|
console.log(`Skipping no-op move for region ${regionId}`);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Get the target track
|
// Get the target track
|
||||||
const targetTrack = tracks[finalTrackIndex];
|
const targetTrack = tracks[finalTrackIndex];
|
||||||
if (!targetTrack) return;
|
if (!targetTrack) return;
|
||||||
|
|||||||
@@ -0,0 +1,78 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
|
import { act } from '@testing-library/react';
|
||||||
|
import { KGMidiTrack } from '../core/track/KGMidiTrack';
|
||||||
|
|
||||||
|
const mockProject = {
|
||||||
|
getTimeSignature: () => ({ numerator: 4, denominator: 4 }),
|
||||||
|
getMaxBars: () => 32,
|
||||||
|
getBarWidthMultiplier: () => 1,
|
||||||
|
getTracks: () => [new KGMidiTrack('Track 1', 0, 'acoustic_grand_piano')],
|
||||||
|
getBpm: () => 120,
|
||||||
|
getKeySignature: () => 'C major',
|
||||||
|
getName: () => 'Test Project',
|
||||||
|
getSelectedMode: () => 'major',
|
||||||
|
getIsLooping: () => false,
|
||||||
|
getLoopingRange: () => null,
|
||||||
|
};
|
||||||
|
|
||||||
|
const mockCore = {
|
||||||
|
getCurrentProject: () => mockProject,
|
||||||
|
setPlayheadUpdateCallback: vi.fn(),
|
||||||
|
setPlaybackStateChangeCallback: vi.fn(),
|
||||||
|
getSelectedItems: () => [],
|
||||||
|
onSelectionChanged: vi.fn(),
|
||||||
|
canUndo: () => false,
|
||||||
|
canRedo: () => false,
|
||||||
|
getUndoDescription: () => '',
|
||||||
|
getRedoDescription: () => '',
|
||||||
|
setOnCommandHistoryChanged: vi.fn(),
|
||||||
|
executeCommand: vi.fn(),
|
||||||
|
clearSelectedItems: vi.fn(),
|
||||||
|
getStatus: () => 'Ready',
|
||||||
|
getPlayheadPosition: () => 0,
|
||||||
|
getIsPlaying: () => false,
|
||||||
|
};
|
||||||
|
|
||||||
|
vi.mock('../core/KGCore', () => ({
|
||||||
|
KGCore: {
|
||||||
|
instance: () => mockCore,
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../core/config/ConfigManager', () => ({
|
||||||
|
ConfigManager: {
|
||||||
|
instance: () => ({
|
||||||
|
getIsInitialized: () => true,
|
||||||
|
get: () => false,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('projectStore piano roll state', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.resetModules();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('clears hybrid state when opening a MIDI region', async () => {
|
||||||
|
const { useProjectStore } = await import('./projectStore');
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
useProjectStore.getState().openHybridMode('midi-a', 'audio-a');
|
||||||
|
});
|
||||||
|
|
||||||
|
let state = useProjectStore.getState();
|
||||||
|
expect(state.pianoRollMode).toBe('hybrid');
|
||||||
|
expect(state.activeRegionId).toBe('midi-a');
|
||||||
|
expect(state.hybridAudioRegionId).toBe('audio-a');
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
useProjectStore.getState().openMidiPianoRoll('midi-b');
|
||||||
|
});
|
||||||
|
|
||||||
|
state = useProjectStore.getState();
|
||||||
|
expect(state.showPianoRoll).toBe(true);
|
||||||
|
expect(state.pianoRollMode).toBe('midi-edit');
|
||||||
|
expect(state.activeRegionId).toBe('midi-b');
|
||||||
|
expect(state.hybridAudioRegionId).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user