feat: persist piano roll zoom level into project; fixed an issue in the horizontal auto-scrolling logic in piano roll window
This commit is contained in:
@@ -33,6 +33,7 @@ vi.mock('../../stores/projectStore', () => ({
|
||||
|
||||
import {
|
||||
createPendingModeSwitchRequest,
|
||||
getRegionStartScrollLeft,
|
||||
getRegionPlayheadRelation,
|
||||
getScrollLeftForViewportRequest,
|
||||
} from './PianoRoll';
|
||||
@@ -57,6 +58,13 @@ describe('PianoRoll viewport switch helpers', () => {
|
||||
expect(getRegionPlayheadRelation(25, 16, 24)).toBe('after');
|
||||
});
|
||||
|
||||
it('uses the zoomed beat width when scrolling to a different region in piano-roll view', () => {
|
||||
document.documentElement.style.setProperty('--region-grid-beat-width', '80px');
|
||||
document.documentElement.style.setProperty('--region-grid-bar-width', 'calc(var(--region-grid-beat-width) * var(--time-signature-numerator))');
|
||||
|
||||
expect(getRegionStartScrollLeft(16)).toBe(1280);
|
||||
});
|
||||
|
||||
it('centers an in-region playhead when switching to region-scope sheet view', () => {
|
||||
const request = createPendingModeSwitchRequest({
|
||||
playheadBeat: 20,
|
||||
|
||||
@@ -96,7 +96,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
|
||||
useState<SpectrogramHeightResolution>(3);
|
||||
|
||||
// Piano roll zoom (1x–8x); updates --region-grid-beat-width CSS variable
|
||||
const [pianoRollZoom, setPianoRollZoom] = useState<number>(1);
|
||||
const [pianoRollZoom, setPianoRollZoom] = useState<number>(() => KGPianoRollState.instance().getPianoRollZoom());
|
||||
const [automationEnabled, setAutomationEnabled] = useState(false);
|
||||
const [automationType, setAutomationType] = useState<PianoRollAutomationType>('pitch-bend');
|
||||
const [sheetMusicViewEnabled, setSheetMusicViewEnabled] = useState(false);
|
||||
@@ -785,6 +785,8 @@ const PianoRoll: React.FC<PianoRollProps> = ({
|
||||
}
|
||||
}
|
||||
|
||||
KGPianoRollState.instance().setPianoRollZoom(nextZoom);
|
||||
KGCore.instance().getCurrentProject().setPianoRollZoom(nextZoom);
|
||||
setPianoRollZoom(nextZoom);
|
||||
}, [pianoRollZoom]);
|
||||
|
||||
@@ -1027,7 +1029,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
|
||||
};
|
||||
}, [pianoRollZoom]);
|
||||
|
||||
// Scroll horizontally to the active region's starting bar
|
||||
// Scroll horizontally to the active region's starting position
|
||||
useEffect(() => {
|
||||
if (!pianoRollNoteScrollRef.current || !activeRegion) {
|
||||
previousActiveRegionIdRef.current = activeRegion?.getId() ?? null;
|
||||
@@ -1043,27 +1045,17 @@ const PianoRoll: React.FC<PianoRollProps> = ({
|
||||
// Get the starting beat of the region
|
||||
const startBeat = activeRegion.getStartFromBeat();
|
||||
|
||||
// Get the time signature to calculate beats per bar
|
||||
const beatsPerBar = timeSignature.numerator;
|
||||
|
||||
// Calculate the bar number (0-indexed)
|
||||
const barNumber = Math.floor(startBeat / beatsPerBar);
|
||||
|
||||
if (DEBUG_MODE.PIANO_ROLL) {
|
||||
console.log(`Scrolling to region's starting bar: ${barNumber + 1} (startBeat: ${startBeat}, beatsPerBar: ${beatsPerBar})`);
|
||||
console.log(`Scrolling to region's starting position: startBeat=${startBeat}`);
|
||||
}
|
||||
|
||||
// Calculate the pixel position (each bar is --region-grid-bar-width wide, which is 160px by default)
|
||||
const barWidth = parseInt(getComputedStyle(document.documentElement).getPropertyValue('--region-grid-bar-width')) || 160;
|
||||
|
||||
// Calculate the scroll position to scroll to the starting bar
|
||||
const scrollPosition = barNumber * barWidth;
|
||||
const scrollPosition = getRegionStartScrollLeft(startBeat);
|
||||
|
||||
// Scroll to the calculated position
|
||||
pianoRollNoteScrollRef.current.scrollLeft = Math.max(0, scrollPosition);
|
||||
previousActiveRegionIdRef.current = activeRegion.getId();
|
||||
}
|
||||
}, [activeRegion, timeSignature]);
|
||||
}, [activeRegion]);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const request = pendingModeSwitchRequestRef.current;
|
||||
@@ -1589,3 +1581,11 @@ export function getScrollLeftForViewportRequest({
|
||||
container,
|
||||
});
|
||||
}
|
||||
|
||||
export function getRegionStartScrollLeft(startBeat: number): number {
|
||||
const beatWidth = parseInt(
|
||||
getComputedStyle(document.documentElement).getPropertyValue('--region-grid-beat-width')
|
||||
) || TOOLBAR_CONSTANTS.BASE_BAR_WIDTH;
|
||||
|
||||
return Math.max(0, startBeat * beatWidth);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
import React from 'react';
|
||||
import { describe, expect, it, beforeEach, vi } from 'vitest';
|
||||
import { render } from '@testing-library/react';
|
||||
import PianoRoll from './PianoRoll';
|
||||
import { createMockMidiRegion, createMockMidiTrack } from '../../test/utils/mock-data';
|
||||
|
||||
const pianoRollState = {
|
||||
zoom: 1,
|
||||
getCurrentSnap: vi.fn(() => 'NO SNAP'),
|
||||
getActiveTool: vi.fn(() => 'pointer'),
|
||||
getAutomationViewEnabled: vi.fn(() => false),
|
||||
getCurrentAutomationType: vi.fn(() => 'pitch-bend'),
|
||||
getPianoRollZoom: vi.fn(() => pianoRollState.zoom),
|
||||
setPianoRollZoom: vi.fn((zoom: number) => {
|
||||
pianoRollState.zoom = zoom;
|
||||
}),
|
||||
getSheetMusicViewEnabled: vi.fn(() => false),
|
||||
getSheetMusicTrackScopeEnabled: vi.fn(() => false),
|
||||
getSheetQuantization: vi.fn(() => '16,48'),
|
||||
setSheetMusicViewEnabled: vi.fn(),
|
||||
setActiveTool: vi.fn(),
|
||||
setAutomationViewEnabled: vi.fn(),
|
||||
setCurrentAutomationType: vi.fn(),
|
||||
setSheetQuantization: vi.fn(),
|
||||
setSheetMusicTrackScopeEnabled: vi.fn(),
|
||||
setCurrentSnap: vi.fn(),
|
||||
setCurrentSuitableChords: vi.fn(),
|
||||
setCurrentSuitableChordsPitchClasses: vi.fn(),
|
||||
};
|
||||
|
||||
const mockProject = {
|
||||
setPianoRollZoom: vi.fn(),
|
||||
};
|
||||
|
||||
const region = createMockMidiRegion({ id: 'region-1', trackId: '1', trackIndex: 0 });
|
||||
const track = createMockMidiTrack({ id: 1, regions: [region] });
|
||||
|
||||
const storeState = {
|
||||
maxBars: 8,
|
||||
tracks: [track],
|
||||
updateTrack: vi.fn(),
|
||||
timeSignature: { numerator: 4, denominator: 4 },
|
||||
showChatBox: false,
|
||||
showKGOnePanel: false,
|
||||
showEventListPanel: false,
|
||||
showInstrumentSelection: false,
|
||||
keySignature: 'C major',
|
||||
selectedMode: 'ionian',
|
||||
setSelectedMode: vi.fn(),
|
||||
playheadPosition: 0,
|
||||
isPlaying: false,
|
||||
autoScrollEnabled: false,
|
||||
bpm: 120,
|
||||
pianoRollScrollRequest: null,
|
||||
selectedNoteIds: [],
|
||||
automationRedrawVersion: 0,
|
||||
};
|
||||
|
||||
let latestToolbarProps: { zoom: number; onZoomChange: (value: number) => void } | null = null;
|
||||
|
||||
vi.mock('../../stores/projectStore', () => ({
|
||||
useProjectStore: Object.assign(
|
||||
(selector?: (state: typeof storeState) => unknown) => selector ? selector(storeState) : storeState,
|
||||
{
|
||||
getState: () => ({ setAutoScrollEnabled: vi.fn() }),
|
||||
setState: vi.fn(),
|
||||
}
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock('../../core/state/KGPianoRollState', () => ({
|
||||
KGPianoRollState: {
|
||||
instance: () => pianoRollState,
|
||||
SNAP_OPTIONS: ['NO SNAP'],
|
||||
QUANT_POS_OPTIONS: ['1/8'],
|
||||
QUANT_LEN_OPTIONS: ['1/8'],
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../../core/KGCore', () => ({
|
||||
KGCore: {
|
||||
FUNCTIONAL_CHORDS_DATA: { ionian: { name: 'Ionian' } },
|
||||
instance: () => ({
|
||||
getCurrentProject: () => mockProject,
|
||||
getSelectedItems: () => [],
|
||||
executeCommand: vi.fn(),
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../../core/config/ConfigManager', () => ({
|
||||
ConfigManager: {
|
||||
instance: () => ({
|
||||
getIsInitialized: () => true,
|
||||
get: vi.fn(),
|
||||
addChangeListener: () => () => undefined,
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../../util/scaleUtil', () => ({
|
||||
getSuitableChords: vi.fn(() => ({})),
|
||||
noteNameToPitchClass: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../../util/dialogUtil', () => ({
|
||||
showAlert: vi.fn(),
|
||||
showPrompt: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('./PianoRollHeader', () => ({ default: () => <div data-testid="header" /> }));
|
||||
vi.mock('./NoteAttributeBar', () => ({ default: () => <div data-testid="note-attribute-bar" /> }));
|
||||
vi.mock('./PianoRollContent', () => ({ default: () => <div data-testid="content" /> }));
|
||||
vi.mock('./PianoRollToolbar', () => ({
|
||||
default: (props: { zoom: number; onZoomChange: (value: number) => void }) => {
|
||||
latestToolbarProps = props;
|
||||
return <div data-testid="toolbar">{props.zoom}x</div>;
|
||||
},
|
||||
}));
|
||||
|
||||
describe('PianoRoll zoom persistence', () => {
|
||||
beforeEach(() => {
|
||||
latestToolbarProps = null;
|
||||
pianoRollState.zoom = 1;
|
||||
mockProject.setPianoRollZoom.mockReset();
|
||||
pianoRollState.getPianoRollZoom.mockClear();
|
||||
pianoRollState.setPianoRollZoom.mockClear();
|
||||
document.documentElement.style.setProperty('--region-piano-key-width', '60px');
|
||||
document.documentElement.style.setProperty('--region-grid-beat-width', '40px');
|
||||
});
|
||||
|
||||
it('restores the previous zoom after closing and reopening the piano roll', () => {
|
||||
const firstRender = render(
|
||||
<PianoRoll
|
||||
onClose={vi.fn()}
|
||||
regionId="region-1"
|
||||
initialPosition={{ x: 0, y: 0 }}
|
||||
initialSize={{ width: 800, height: 400 }}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(latestToolbarProps?.zoom).toBe(1);
|
||||
|
||||
latestToolbarProps?.onZoomChange(3);
|
||||
|
||||
expect(pianoRollState.setPianoRollZoom).toHaveBeenCalledWith(3);
|
||||
expect(mockProject.setPianoRollZoom).toHaveBeenCalledWith(3);
|
||||
|
||||
firstRender.unmount();
|
||||
|
||||
render(
|
||||
<PianoRoll
|
||||
onClose={vi.fn()}
|
||||
regionId="region-1"
|
||||
initialPosition={{ x: 0, y: 0 }}
|
||||
initialSize={{ width: 800, height: 400 }}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(latestToolbarProps?.zoom).toBe(3);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user