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:
Xiaohan-Tian
2026-05-01 20:34:21 -07:00
parent cbd2fc60a0
commit d5795c5281
6 changed files with 234 additions and 36 deletions
+97
View File
@@ -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');
});
});
+39 -31
View File
@@ -9,6 +9,8 @@ import { KGAudioRegion } from '../../core/region/KGAudioRegion';
import { useProjectStore } from '../../stores/projectStore';
import { KGMainContentState } from '../../core/state/KGMainContentState';
const DRAG_START_THRESHOLD_PX = 4;
interface RegionItemProps {
id: string;
name: string;
@@ -73,7 +75,7 @@ const RegionItem: React.FC<RegionItemProps> = ({
// Use refs to track states for immediate access
const isResizingRef = useRef<boolean>(false);
const isDraggingRef = useRef<boolean>(false);
const hasMovedRef = useRef<boolean>(false);
const isPendingDragRef = useRef<boolean>(false);
// Canvas ref for note visualization
const canvasRef = useRef<HTMLCanvasElement | null>(null);
@@ -391,7 +393,7 @@ const RegionItem: React.FC<RegionItemProps> = ({
const activeTool = KGMainContentState.instance().getActiveTool();
if (activeTool === 'pencil') {
// Still allow click events to pass through for region selection
if (!hasMovedRef.current && onClick) {
if (onClick) {
if (DEBUG_MODE.REGION_ITEM) {
console.log(`REGION CLICKED (pencil mode): regionId=${id}`);
}
@@ -404,7 +406,7 @@ const RegionItem: React.FC<RegionItemProps> = ({
e.preventDefault();
// Reset movement tracking
hasMovedRef.current = false;
isPendingDragRef.current = false;
// Store initial mouse position
initialMousePosRef.current = { x: e.clientX, y: e.clientY };
@@ -423,21 +425,12 @@ const RegionItem: React.FC<RegionItemProps> = ({
onResizeStart(id, resizeEdge, e.clientX);
}
} else {
// Start dragging
// Wait for actual pointer movement before promoting this gesture to a drag.
if (DEBUG_MODE.REGION_ITEM) {
console.log(`DRAG START: regionId=${id}`);
console.log(`PENDING REGION INTERACTION: regionId=${id}`);
}
setIsDragging(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);
}
isPendingDragRef.current = true;
}
// 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
const handleGlobalMouseMove = (e: MouseEvent) => {
// Set the hasMovedRef to true as soon as there's movement
hasMovedRef.current = true;
if (isResizingRef.current) {
// Handle resize
if (DEBUG_MODE.REGION_ITEM) {
@@ -463,16 +453,35 @@ const RegionItem: React.FC<RegionItemProps> = ({
if (onResize) {
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
if (DEBUG_MODE.REGION_ITEM) {
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
if (onDrag) {
onDrag(id, deltaX, deltaY);
@@ -511,16 +520,15 @@ const RegionItem: React.FC<RegionItemProps> = ({
if (onDragEnd) {
onDragEnd(id);
}
// If there was no movement, treat it as a click
if (!hasMovedRef.current && onClick) {
if (DEBUG_MODE.REGION_ITEM) {
console.log(`REGION CLICKED: regionId=${id}`);
}
onClick(id);
} else if (isPendingDragRef.current && onClick) {
if (DEBUG_MODE.REGION_ITEM) {
console.log(`REGION CLICKED: regionId=${id}`);
}
onClick(id);
}
isPendingDragRef.current = false;
// Remove global event listeners
document.removeEventListener('mousemove', handleGlobalMouseMove);
document.removeEventListener('mouseup', handleGlobalMouseUp);
@@ -652,4 +660,4 @@ const RegionItem: React.FC<RegionItemProps> = ({
);
};
export default RegionItem;
export default RegionItem;
+8 -1
View File
@@ -492,6 +492,13 @@ const TrackGridItem: React.FC<TrackGridItemProps> = ({
currentDragTop.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
if (onRegionDragEnd) {
onRegionDragEnd(regionId, finalBarNumber, finalTrackIndex);
@@ -589,4 +596,4 @@ const TrackGridItem: React.FC<TrackGridItemProps> = ({
);
};
export default TrackGridItem;
export default TrackGridItem;
+8 -1
View File
@@ -386,6 +386,13 @@ const TrackGridPanel: React.FC<TrackGridPanelProps> = ({
// Find the region
const region = regions.find(r => r.id === regionId);
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
const targetTrack = tracks[finalTrackIndex];
@@ -680,4 +687,4 @@ const TrackGridPanel: React.FC<TrackGridPanelProps> = ({
);
};
export default TrackGridPanel;
export default TrackGridPanel;