fix: when piano roll window is not visible, select a row on list event table will cause current MIDI region lose selection

2026-05-08 17:09:53 -07:00
parent 37da0f6885
commit a1c8727d44
4 changed files with 259 additions and 17 deletions
@@ -1,8 +1,18 @@
 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 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({
  id: 'region-1',
@@ -11,26 +21,96 @@ const region = createMockMidiRegion({
  startFromBeat: 4,
  notes: [createMockMidiNote({ id: 'note-1', pitch: 60, startBeat: 1, endBeat: 2, velocity: 96 })],
  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] });

+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', () => ({
-  useProjectStore: () => ({
-    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(),
-  }),
+  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', () => {
+  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', () => {
    render(<ListEventPanel isVisible={true} />);

@@ -50,4 +130,31 @@ describe('ListEventPanel', () => {
    fireEvent.click(screen.getByRole('button', { name: 'Pitch Bends' }));
    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))
      .map(row => row.type === 'note' ? row.note : row.type === 'pitch-bend' ? row.pitchBend : row.controllerEvent);
    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 => {
      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) {
      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;

    setSelectedRegionId(lastSelectedRegionId);
+    setActiveRegionId(lastSelectedRegionId);

    if (DEBUG_MODE.MAIN_CONTENT) {
      console.log(`Selected regions: ${selectedRegions.map(selectedRegion => selectedRegion.getId()).join(', ')}`);
@@ -598,7 +599,6 @@ const MainContent: React.FC<MainContentProps> = ({

    if (!lastSelectedRegionId) {
      setShowPianoRoll(false);
-      setActiveRegionId(null);
      return;
    }