fix: when piano roll window is not visible, select a row on list event table will cause current MIDI region lose selection
This commit is contained in:
@@ -1,8 +1,18 @@
|
|||||||
import React from 'react';
|
import React from 'react';
|
||||||
import { describe, expect, it, vi } from 'vitest';
|
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
import { fireEvent, render, screen } from '@testing-library/react';
|
import { fireEvent, render, screen } from '@testing-library/react';
|
||||||
import ListEventPanel from './ListEventPanel';
|
import ListEventPanel from './ListEventPanel';
|
||||||
import { createMockMidiNote, createMockMidiPitchBend, createMockMidiRegion, createMockMidiTrack } from '../test/utils/mock-data';
|
import { KGMidiControllerEvent } from '../core/midi/KGMidiControllerEvent';
|
||||||
|
import { KGMidiNote } from '../core/midi/KGMidiNote';
|
||||||
|
import { KGMidiPitchBend } from '../core/midi/KGMidiPitchBend';
|
||||||
|
import { KGRegion } from '../core/region/KGRegion';
|
||||||
|
import {
|
||||||
|
createMockMidiControllerEvent,
|
||||||
|
createMockMidiNote,
|
||||||
|
createMockMidiPitchBend,
|
||||||
|
createMockMidiRegion,
|
||||||
|
createMockMidiTrack,
|
||||||
|
} from '../test/utils/mock-data';
|
||||||
|
|
||||||
const region = createMockMidiRegion({
|
const region = createMockMidiRegion({
|
||||||
id: 'region-1',
|
id: 'region-1',
|
||||||
@@ -11,11 +21,27 @@ const region = createMockMidiRegion({
|
|||||||
startFromBeat: 4,
|
startFromBeat: 4,
|
||||||
notes: [createMockMidiNote({ id: 'note-1', pitch: 60, startBeat: 1, endBeat: 2, velocity: 96 })],
|
notes: [createMockMidiNote({ id: 'note-1', pitch: 60, startBeat: 1, endBeat: 2, velocity: 96 })],
|
||||||
pitchBends: [createMockMidiPitchBend({ id: 'bend-1', beat: 0.5, value: 12288 })],
|
pitchBends: [createMockMidiPitchBend({ id: 'bend-1', beat: 0.5, value: 12288 })],
|
||||||
|
controllerEventsByType: Array.from({ length: 128 }, (_, index) => (
|
||||||
|
index === 11 ? [createMockMidiControllerEvent({ id: 'cc11-1', beat: 0.75, value: 100 })] : []
|
||||||
|
)),
|
||||||
});
|
});
|
||||||
const track = createMockMidiTrack({ id: 1, regions: [region] });
|
const track = createMockMidiTrack({ id: 1, regions: [region] });
|
||||||
|
|
||||||
vi.mock('../stores/projectStore', () => ({
|
type MockStoreState = {
|
||||||
useProjectStore: () => ({
|
tracks: typeof track[];
|
||||||
|
activeRegionId: string | null;
|
||||||
|
selectedRegionIds: string[];
|
||||||
|
timeSignature: { numerator: number; denominator: number };
|
||||||
|
selectedNoteIds: string[];
|
||||||
|
selectedPitchBendIds: string[];
|
||||||
|
selectedControllerEventIds: string[];
|
||||||
|
playheadPosition: number;
|
||||||
|
updateTrack: ReturnType<typeof vi.fn>;
|
||||||
|
refreshProjectState: ReturnType<typeof vi.fn>;
|
||||||
|
bumpAutomationRedrawVersion: ReturnType<typeof vi.fn>;
|
||||||
|
};
|
||||||
|
|
||||||
|
const storeState: MockStoreState = {
|
||||||
tracks: [track],
|
tracks: [track],
|
||||||
activeRegionId: 'region-1',
|
activeRegionId: 'region-1',
|
||||||
selectedRegionIds: ['region-1'],
|
selectedRegionIds: ['region-1'],
|
||||||
@@ -27,10 +53,64 @@ vi.mock('../stores/projectStore', () => ({
|
|||||||
updateTrack: vi.fn().mockResolvedValue(undefined),
|
updateTrack: vi.fn().mockResolvedValue(undefined),
|
||||||
refreshProjectState: vi.fn(),
|
refreshProjectState: vi.fn(),
|
||||||
bumpAutomationRedrawVersion: vi.fn(),
|
bumpAutomationRedrawVersion: vi.fn(),
|
||||||
}),
|
};
|
||||||
|
|
||||||
|
let selectedItems: Array<KGRegion | KGMidiNote | KGMidiPitchBend | KGMidiControllerEvent> = [];
|
||||||
|
|
||||||
|
const syncStoreSelectionFromCore = () => {
|
||||||
|
storeState.selectedRegionIds = selectedItems
|
||||||
|
.filter(item => item instanceof KGRegion)
|
||||||
|
.map(item => item.getId());
|
||||||
|
storeState.selectedNoteIds = selectedItems
|
||||||
|
.filter(item => item instanceof KGMidiNote)
|
||||||
|
.map(item => item.getId());
|
||||||
|
storeState.selectedPitchBendIds = selectedItems
|
||||||
|
.filter(item => item instanceof KGMidiPitchBend)
|
||||||
|
.map(item => item.getId());
|
||||||
|
storeState.selectedControllerEventIds = selectedItems
|
||||||
|
.filter(item => item instanceof KGMidiControllerEvent)
|
||||||
|
.map(item => item.getId());
|
||||||
|
};
|
||||||
|
|
||||||
|
vi.mock('../stores/projectStore', () => ({
|
||||||
|
useProjectStore: () => storeState,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../core/KGCore', () => ({
|
||||||
|
KGCore: {
|
||||||
|
instance: vi.fn(() => ({
|
||||||
|
getSelectedItems: () => selectedItems,
|
||||||
|
addSelectedItems: (items: typeof selectedItems) => {
|
||||||
|
const nextIds = new Set(items.map(item => item.getId()));
|
||||||
|
selectedItems = [...selectedItems.filter(item => !nextIds.has(item.getId())), ...items];
|
||||||
|
syncStoreSelectionFromCore();
|
||||||
|
},
|
||||||
|
removeSelectedItems: (items: typeof selectedItems) => {
|
||||||
|
const removedIds = new Set(items.map(item => item.getId()));
|
||||||
|
selectedItems = selectedItems.filter(item => !removedIds.has(item.getId()));
|
||||||
|
syncStoreSelectionFromCore();
|
||||||
|
},
|
||||||
|
})),
|
||||||
|
},
|
||||||
}));
|
}));
|
||||||
|
|
||||||
describe('ListEventPanel', () => {
|
describe('ListEventPanel', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
selectedItems = [region];
|
||||||
|
region.select();
|
||||||
|
region.getNotes().forEach(note => note.deselect());
|
||||||
|
region.getPitchBends().forEach(pitchBend => pitchBend.deselect());
|
||||||
|
region.getControllerEventsByType().forEach(events => events.forEach(controllerEvent => controllerEvent.deselect()));
|
||||||
|
storeState.activeRegionId = 'region-1';
|
||||||
|
storeState.selectedRegionIds = ['region-1'];
|
||||||
|
storeState.selectedNoteIds = [];
|
||||||
|
storeState.selectedPitchBendIds = [];
|
||||||
|
storeState.selectedControllerEventIds = [];
|
||||||
|
storeState.updateTrack.mockClear();
|
||||||
|
storeState.refreshProjectState.mockClear();
|
||||||
|
storeState.bumpAutomationRedrawVersion.mockClear();
|
||||||
|
});
|
||||||
|
|
||||||
it('renders note and pitch bend rows and toggles them independently', () => {
|
it('renders note and pitch bend rows and toggles them independently', () => {
|
||||||
render(<ListEventPanel isVisible={true} />);
|
render(<ListEventPanel isVisible={true} />);
|
||||||
|
|
||||||
@@ -50,4 +130,31 @@ describe('ListEventPanel', () => {
|
|||||||
fireEvent.click(screen.getByRole('button', { name: 'Pitch Bends' }));
|
fireEvent.click(screen.getByRole('button', { name: 'Pitch Bends' }));
|
||||||
expect(screen.getByText('Pitch Bend')).toBeInTheDocument();
|
expect(screen.getByText('Pitch Bend')).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('keeps the region selected while selecting rows', () => {
|
||||||
|
const { rerender } = render(<ListEventPanel isVisible={true} />);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByText('C4').closest('tr')!);
|
||||||
|
rerender(<ListEventPanel isVisible={true} />);
|
||||||
|
|
||||||
|
expect(storeState.selectedRegionIds).toEqual(['region-1']);
|
||||||
|
expect(storeState.selectedNoteIds).toEqual(['note-1']);
|
||||||
|
expect(screen.queryByText('Please select a MIDI region, or open one in the Piano Roll, to view its event list.')).not.toBeInTheDocument();
|
||||||
|
expect(screen.getByText('C4').closest('tr')).toHaveClass('selected');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('supports additive row selection without dropping the owning region', () => {
|
||||||
|
const { rerender } = render(<ListEventPanel isVisible={true} />);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByText('C4').closest('tr')!);
|
||||||
|
rerender(<ListEventPanel isVisible={true} />);
|
||||||
|
fireEvent.click(screen.getByText('Pitch Bend').closest('tr')!, { shiftKey: true });
|
||||||
|
rerender(<ListEventPanel isVisible={true} />);
|
||||||
|
|
||||||
|
expect(storeState.selectedRegionIds).toEqual(['region-1']);
|
||||||
|
expect(storeState.selectedNoteIds).toEqual(['note-1']);
|
||||||
|
expect(storeState.selectedPitchBendIds).toEqual(['bend-1']);
|
||||||
|
expect(screen.getByText('C4').closest('tr')).toHaveClass('selected');
|
||||||
|
expect(screen.getByText('Raw 12288 | 0.500 | 1.00 st').closest('tr')).toHaveClass('selected');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -297,6 +297,11 @@ const ListEventPanel: React.FC<ListEventPanelProps> = ({ isVisible }) => {
|
|||||||
.filter(row => nextSelectedIds.has(row.id))
|
.filter(row => nextSelectedIds.has(row.id))
|
||||||
.map(row => row.type === 'note' ? row.note : row.type === 'pitch-bend' ? row.pitchBend : row.controllerEvent);
|
.map(row => row.type === 'note' ? row.note : row.type === 'pitch-bend' ? row.pitchBend : row.controllerEvent);
|
||||||
const core = KGCore.instance();
|
const core = KGCore.instance();
|
||||||
|
const previouslySelectedRegionEvents = core.getSelectedItems().filter(item =>
|
||||||
|
(item instanceof KGMidiNote && activeMidiRegion.getNotes().some(note => note.getId() === item.getId())) ||
|
||||||
|
(item instanceof KGMidiPitchBend && activeMidiRegion.getPitchBends().some(pitchBend => pitchBend.getId() === item.getId())) ||
|
||||||
|
(item instanceof KGMidiControllerEvent && activeMidiRegion.getAllControllerEventsFlattened().some(({ event }) => event.getId() === item.getId()))
|
||||||
|
);
|
||||||
|
|
||||||
activeMidiRegion.getNotes().forEach(note => {
|
activeMidiRegion.getNotes().forEach(note => {
|
||||||
if (nextSelectedIds.has(note.getId())) note.select();
|
if (nextSelectedIds.has(note.getId())) note.select();
|
||||||
@@ -313,7 +318,9 @@ const ListEventPanel: React.FC<ListEventPanelProps> = ({ isVisible }) => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
core.clearSelectedItems();
|
if (previouslySelectedRegionEvents.length > 0) {
|
||||||
|
core.removeSelectedItems(previouslySelectedRegionEvents);
|
||||||
|
}
|
||||||
if (selectedEvents.length > 0) {
|
if (selectedEvents.length > 0) {
|
||||||
core.addSelectedItems(selectedEvents);
|
core.addSelectedItems(selectedEvents);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,128 @@
|
|||||||
|
import React from 'react';
|
||||||
|
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
import { fireEvent, render, screen } from '@testing-library/react';
|
||||||
|
import MainContent from './MainContent';
|
||||||
|
import { KGMidiRegion } from '../core/region/KGMidiRegion';
|
||||||
|
import { createMockMidiTrack } from '../test/utils/mock-data';
|
||||||
|
|
||||||
|
const midiRegion = new KGMidiRegion('region-1', '1', 0, 'Region 1', 0, 4);
|
||||||
|
const track = createMockMidiTrack({ id: 1, regions: [midiRegion] });
|
||||||
|
|
||||||
|
const storeState = {
|
||||||
|
tracks: [track],
|
||||||
|
maxBars: 8,
|
||||||
|
barWidthMultiplier: 1,
|
||||||
|
reorderTracks: vi.fn(),
|
||||||
|
updateTrack: vi.fn(),
|
||||||
|
updateTrackProperties: vi.fn(),
|
||||||
|
timeSignature: { numerator: 4, denominator: 4 },
|
||||||
|
setPlayheadPosition: vi.fn(),
|
||||||
|
playheadPosition: 0,
|
||||||
|
isPlaying: false,
|
||||||
|
autoScrollEnabled: false,
|
||||||
|
setAutoScrollEnabled: vi.fn(),
|
||||||
|
clearAllSelections: vi.fn(() => {
|
||||||
|
storeState.selectedRegionIds = [];
|
||||||
|
}),
|
||||||
|
setSelectedTrack: vi.fn(),
|
||||||
|
selectedRegionIds: [] as string[],
|
||||||
|
showPianoRoll: false,
|
||||||
|
activeRegionId: null as string | null,
|
||||||
|
setShowPianoRoll: vi.fn((show: boolean) => {
|
||||||
|
storeState.showPianoRoll = show;
|
||||||
|
}),
|
||||||
|
setActiveRegionId: vi.fn((regionId: string | null) => {
|
||||||
|
storeState.activeRegionId = regionId;
|
||||||
|
}),
|
||||||
|
pianoRollMode: 'midi-edit' as const,
|
||||||
|
openMidiPianoRoll: vi.fn(),
|
||||||
|
openSpectrogramViewer: vi.fn(),
|
||||||
|
openHybridMode: vi.fn(),
|
||||||
|
hybridAudioRegionId: null as string | null,
|
||||||
|
addTrack: vi.fn(),
|
||||||
|
addAudioTrack: vi.fn(),
|
||||||
|
projectName: 'Test Project',
|
||||||
|
savedProjectName: 'Test Project',
|
||||||
|
requestPianoRollScroll: vi.fn(),
|
||||||
|
mainContentScrollRequest: null,
|
||||||
|
activeTrackAutomationTrackId: null,
|
||||||
|
activeTrackAutomationType: null,
|
||||||
|
selectedTrackAutomationPointIds: [],
|
||||||
|
bumpTrackAutomationRedrawVersion: vi.fn(),
|
||||||
|
refreshProjectState: vi.fn(),
|
||||||
|
showInstrumentSelection: false,
|
||||||
|
isLooping: false,
|
||||||
|
loopingRange: [0, 0] as [number, number],
|
||||||
|
};
|
||||||
|
|
||||||
|
// eslint-disable-next-line no-unused-vars
|
||||||
|
type StoreSelector = (...args: [typeof storeState]) => unknown;
|
||||||
|
// eslint-disable-next-line no-unused-vars
|
||||||
|
type RegionClickHandler = (...args: [string, { shiftKey: boolean }]) => void;
|
||||||
|
|
||||||
|
vi.mock('../stores/projectStore', () => ({
|
||||||
|
useProjectStore: (selector?: StoreSelector) => (
|
||||||
|
selector ? selector(storeState) : storeState
|
||||||
|
),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../core/KGCore', () => ({
|
||||||
|
KGCore: {
|
||||||
|
instance: () => ({
|
||||||
|
addSelectedItems: (items: KGMidiRegion[]) => {
|
||||||
|
storeState.selectedRegionIds = items.map(item => item.getId());
|
||||||
|
},
|
||||||
|
clearSelectedItems: () => {
|
||||||
|
storeState.selectedRegionIds = [];
|
||||||
|
},
|
||||||
|
executeCommand: vi.fn(),
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../hooks/useRegionOperations', () => ({
|
||||||
|
useRegionOperations: () => ({
|
||||||
|
deleteSelectedRegions: vi.fn(() => false),
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('./track/TrackInfoPanel', () => ({
|
||||||
|
default: () => <div data-testid="track-info-panel" />,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('./track/TrackGridPanel', () => ({
|
||||||
|
default: ({ onRegionClick }: { onRegionClick?: RegionClickHandler }) => (
|
||||||
|
<button type="button" onClick={() => onRegionClick?.('region-1', { shiftKey: false })}>
|
||||||
|
select-region
|
||||||
|
</button>
|
||||||
|
),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('./piano-roll/PianoRoll', () => ({
|
||||||
|
default: () => <div data-testid="piano-roll" />,
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('MainContent', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
storeState.selectedRegionIds = [];
|
||||||
|
storeState.activeRegionId = null;
|
||||||
|
storeState.showPianoRoll = false;
|
||||||
|
storeState.clearAllSelections.mockClear();
|
||||||
|
storeState.setSelectedTrack.mockClear();
|
||||||
|
storeState.setShowPianoRoll.mockClear();
|
||||||
|
storeState.setActiveRegionId.mockClear();
|
||||||
|
storeState.openMidiPianoRoll.mockClear();
|
||||||
|
storeState.openSpectrogramViewer.mockClear();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('updates activeRegionId when selecting a region with piano roll closed', () => {
|
||||||
|
render(<MainContent />);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: 'select-region' }));
|
||||||
|
|
||||||
|
expect(storeState.activeRegionId).toBe('region-1');
|
||||||
|
expect(storeState.setActiveRegionId).toHaveBeenCalledWith('region-1');
|
||||||
|
expect(storeState.showPianoRoll).toBe(false);
|
||||||
|
expect(storeState.openMidiPianoRoll).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -587,6 +587,7 @@ const MainContent: React.FC<MainContentProps> = ({
|
|||||||
: null;
|
: null;
|
||||||
|
|
||||||
setSelectedRegionId(lastSelectedRegionId);
|
setSelectedRegionId(lastSelectedRegionId);
|
||||||
|
setActiveRegionId(lastSelectedRegionId);
|
||||||
|
|
||||||
if (DEBUG_MODE.MAIN_CONTENT) {
|
if (DEBUG_MODE.MAIN_CONTENT) {
|
||||||
console.log(`Selected regions: ${selectedRegions.map(selectedRegion => selectedRegion.getId()).join(', ')}`);
|
console.log(`Selected regions: ${selectedRegions.map(selectedRegion => selectedRegion.getId()).join(', ')}`);
|
||||||
@@ -598,7 +599,6 @@ const MainContent: React.FC<MainContentProps> = ({
|
|||||||
|
|
||||||
if (!lastSelectedRegionId) {
|
if (!lastSelectedRegionId) {
|
||||||
setShowPianoRoll(false);
|
setShowPianoRoll(false);
|
||||||
setActiveRegionId(null);
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user