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:
Xiaohan-Tian
2026-05-08 17:09:53 -07:00
parent 37da0f6885
commit a1c8727d44
4 changed files with 259 additions and 17 deletions
+122 -15
View File
@@ -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,26 +21,96 @@ 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] });
type MockStoreState = {
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],
activeRegionId: 'region-1',
selectedRegionIds: ['region-1'],
timeSignature: { numerator: 4, denominator: 4 },
selectedNoteIds: [],
selectedPitchBendIds: [],
selectedControllerEventIds: [],
playheadPosition: 4,
updateTrack: vi.fn().mockResolvedValue(undefined),
refreshProjectState: 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', () => ({ vi.mock('../stores/projectStore', () => ({
useProjectStore: () => ({ useProjectStore: () => storeState,
tracks: [track], }));
activeRegionId: 'region-1',
selectedRegionIds: ['region-1'], vi.mock('../core/KGCore', () => ({
timeSignature: { numerator: 4, denominator: 4 }, KGCore: {
selectedNoteIds: [], instance: vi.fn(() => ({
selectedPitchBendIds: [], getSelectedItems: () => selectedItems,
selectedControllerEventIds: [], addSelectedItems: (items: typeof selectedItems) => {
playheadPosition: 4, const nextIds = new Set(items.map(item => item.getId()));
updateTrack: vi.fn().mockResolvedValue(undefined), selectedItems = [...selectedItems.filter(item => !nextIds.has(item.getId())), ...items];
refreshProjectState: vi.fn(), syncStoreSelectionFromCore();
bumpAutomationRedrawVersion: vi.fn(), },
}), 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');
});
}); });
+8 -1
View File
@@ -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);
} }
+128
View File
@@ -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();
});
});
+1 -1
View File
@@ -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;
} }