From 1f330818d83aee2099d736caa45c5b8db8ad987c Mon Sep 17 00:00:00 2001 From: Xiaohan-Tian <157918347+Xiaohan-Tian@users.noreply.github.com> Date: Mon, 25 May 2026 13:50:28 -0700 Subject: [PATCH] feat: enhance multi-region selection feature to support both shift and cmd/ctrl --- src/components/MainContent.css | 4 - src/components/MainContent.test.tsx | 256 ++++++++++++++- src/components/MainContent.tsx | 305 ++++++++++++++++-- .../global-track/GlobalChordLane.test.tsx | 30 ++ .../global-track/GlobalChordLane.tsx | 17 +- .../global-track/GlobalKeySignatureLane.tsx | 15 +- .../global-track/GlobalMarkerLane.tsx | 9 +- .../global-track/GlobalTempoLane.test.tsx | 26 ++ .../global-track/GlobalTempoLane.tsx | 17 +- src/components/interfaces.ts | 2 + src/components/track/Region.css | 8 - src/components/track/RegionItem.test.tsx | 17 +- src/components/track/RegionItem.tsx | 11 +- src/components/track/TrackGridItem.tsx | 28 +- src/components/track/TrackGridPanel.test.tsx | 78 ++++- src/components/track/TrackGridPanel.tsx | 33 +- 16 files changed, 763 insertions(+), 93 deletions(-) diff --git a/src/components/MainContent.css b/src/components/MainContent.css index 3afbeb0..b9d4937 100644 --- a/src/components/MainContent.css +++ b/src/components/MainContent.css @@ -340,10 +340,6 @@ transition: box-shadow 0.1s ease, border-color 0.1s ease; } -.global-marker-region:hover { - box-shadow: 0 0 0 1px rgba(255, 255, 255, 0.45); -} - .global-marker-region.selected { border-color: #ffffff; box-shadow: none; diff --git a/src/components/MainContent.test.tsx b/src/components/MainContent.test.tsx index f280c18..7ada95d 100644 --- a/src/components/MainContent.test.tsx +++ b/src/components/MainContent.test.tsx @@ -1,10 +1,11 @@ import React from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import MainContent from './MainContent'; import { KGMidiRegion } from '../core/region/KGMidiRegion'; import { KGAudioRegion } from '../core/region/KGAudioRegion'; import { KGChordRegion } from '../core/region/KGChordRegion'; +import { KGTempoRegion } from '../core/region/KGTempoRegion'; import { KGAudioTrack } from '../core/track/KGAudioTrack'; import { createDefaultGlobalTracks, GlobalTrackType } from '../core/global-track'; import { createMockMidiTrack } from '../test/utils/mock-data'; @@ -71,7 +72,7 @@ const storeState = { // 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; +type RegionClickHandler = (...args: [string, { shiftKey: boolean; metaKey: boolean; ctrlKey: boolean }]) => void; const finishTrackCreateDialogClose = () => { const overlay = document.querySelector('.dialog-overlay'); @@ -116,15 +117,27 @@ vi.mock('./track/TrackInfoPanel', () => ({ vi.mock('./track/TrackGridPanel', () => ({ default: ({ onRegionClick }: { onRegionClick?: RegionClickHandler }) => ( <> - - - + + + + ), })); @@ -139,6 +152,9 @@ vi.mock('./piano-roll/PianoRoll', () => ({ describe('MainContent', () => { beforeEach(() => { + midiTrack.setRegions([midiRegion, anotherMidiRegion]); + audioTrack.setRegions([audioRegion]); + storeState.tracks = [midiTrack, audioTrack]; storeState.globalTracks = createDefaultGlobalTracks(); storeState.selectedRegionIds = []; storeState.activeRegionId = null; @@ -207,6 +223,77 @@ describe('MainContent', () => { expect(storeState.openSpectrogramViewer).toHaveBeenCalledWith('audio-1'); }); + it('cmd-click adds a second regular region without range fill', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'select-midi-region' })); + fireEvent.click(screen.getByRole('button', { name: 'meta-select-second-midi-region' })); + + expect(storeState.selectedRegionIds).toEqual(['region-1', 'region-2']); + expect(storeState.activeRegionId).toBe('region-2'); + }); + + it('cmd-click on an already selected regular region removes it', () => { + storeState.selectedRegionIds = ['region-1', 'region-2']; + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'meta-select-second-midi-region' })); + + expect(storeState.selectedRegionIds).toEqual(['region-1']); + }); + + it('shift-click on the same track selects the full in-between range and keeps the clicked region primary', () => { + const middleMidiRegion = new KGMidiRegion('region-1b', '1', 0, 'Region 1B', 4, 4); + midiTrack.setRegions([midiRegion, middleMidiRegion, anotherMidiRegion]); + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'select-midi-region' })); + fireEvent.click(screen.getByRole('button', { name: 'shift-select-second-midi-region' })); + + expect(storeState.selectedRegionIds).toEqual(['region-1', 'region-1b', 'region-2']); + + midiTrack.setRegions([midiRegion, anotherMidiRegion]); + }); + + it('shift-click with no same-track anchor falls back to single selection', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'shift-select-second-midi-region' })); + + expect(storeState.selectedRegionIds).toEqual(['region-2']); + }); + + it('shift-click preserves already selected regions on other regular tracks', () => { + const middleMidiRegion = new KGMidiRegion('region-1b', '1', 0, 'Region 1B', 4, 4); + midiTrack.setRegions([midiRegion, middleMidiRegion, anotherMidiRegion]); + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'select-midi-region' })); + fireEvent.click(screen.getByRole('button', { name: 'meta-select-audio-region' })); + fireEvent.click(screen.getByRole('button', { name: 'shift-select-second-midi-region' })); + + expect(storeState.selectedRegionIds).toEqual(['audio-1', 'region-1', 'region-1b', 'region-2']); + + midiTrack.setRegions([midiRegion, anotherMidiRegion]); + }); + + it('shift-plus-cmd-click uses additive-toggle behavior instead of range selection', () => { + const middleMidiRegion = new KGMidiRegion('region-1b', '1', 0, 'Region 1B', 4, 4); + midiTrack.setRegions([midiRegion, middleMidiRegion, anotherMidiRegion]); + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'select-midi-region' })); + fireEvent.click(screen.getByRole('button', { name: 'shift-meta-select-second-midi-region' })); + + expect(storeState.selectedRegionIds).toEqual(['region-1', 'region-2']); + + midiTrack.setRegions([midiRegion, anotherMidiRegion]); + }); + it('explicit close still clears piano roll visibility and active region', () => { storeState.showPianoRoll = true; storeState.activeRegionId = 'region-1'; @@ -352,6 +439,123 @@ describe('MainContent', () => { expect((executeCommandMock.mock.calls[1][0] as { startBeat?: number }).startBeat).toBe(5); }); + it('selecting a global region clears regular-region selection', () => { + const globalTracks = createDefaultGlobalTracks(); + const chordTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Chord); + chordTrack?.setRegions([ + new KGChordRegion('chord-1', chordTrack.getId(), chordTrack.getTrackIndex(), 'Am', 0, 4), + ]); + storeState.globalTracks = globalTracks; + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'select-midi-region' })); + fireEvent.click(screen.getByRole('button', { name: 'Show global tracks' })); + fireEvent.click(screen.getByText('Am')); + + expect(storeState.selectedRegionIds).toEqual(['chord-1']); + + storeState.globalTracks = createDefaultGlobalTracks(); + }); + + it('shift-click across globals ranges only within the same global lane and keeps the clicked region primary', () => { + const globalTracks = createDefaultGlobalTracks(); + const chordTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Chord); + chordTrack?.setRegions([ + new KGChordRegion('chord-1', chordTrack.getId(), chordTrack.getTrackIndex(), 'Am', 0, 4), + new KGChordRegion('chord-2', chordTrack.getId(), chordTrack.getTrackIndex(), 'F', 4, 4), + new KGChordRegion('chord-3', chordTrack.getId(), chordTrack.getTrackIndex(), 'G', 8, 4), + ]); + storeState.globalTracks = globalTracks; + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Show global tracks' })); + fireEvent.click(screen.getByText('Am')); + fireEvent.click(screen.getByText('G'), { shiftKey: true }); + + expect(storeState.selectedRegionIds).toEqual(['chord-1', 'chord-2', 'chord-3']); + + storeState.globalTracks = createDefaultGlobalTracks(); + }); + + it('shift-clicking from the first chord region to the last chord region selects the full chord range', () => { + const globalTracks = createDefaultGlobalTracks(); + const chordTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Chord); + chordTrack?.setRegions([ + new KGChordRegion('chord-1', chordTrack.getId(), chordTrack.getTrackIndex(), 'Am', 0, 4), + new KGChordRegion('chord-2', chordTrack.getId(), chordTrack.getTrackIndex(), 'F', 4, 4), + new KGChordRegion('chord-3', chordTrack.getId(), chordTrack.getTrackIndex(), 'G', 8, 4), + ]); + storeState.globalTracks = globalTracks; + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Show global tracks' })); + + const firstChord = screen.getByText('Am'); + const lastChord = screen.getByText('G'); + + fireEvent.mouseDown(firstChord, { clientX: 10, clientY: 10, button: 0 }); + fireEvent.mouseUp(window, { clientX: 10, clientY: 10 }); + fireEvent.click(firstChord); + + fireEvent.mouseDown(lastChord, { clientX: 10, clientY: 10, button: 0, shiftKey: true }); + fireEvent.mouseUp(window, { clientX: 10, clientY: 10, shiftKey: true }); + fireEvent.click(lastChord, { shiftKey: true }); + + expect(storeState.selectedRegionIds).toEqual(['chord-1', 'chord-2', 'chord-3']); + + storeState.globalTracks = createDefaultGlobalTracks(); + }); + + it('cmd-clicking a region in a different global lane clears the previous global selection', () => { + const globalTracks = createDefaultGlobalTracks(); + const chordTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Chord); + const tempoTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Tempo); + chordTrack?.setRegions([ + new KGChordRegion('chord-1', chordTrack.getId(), chordTrack.getTrackIndex(), 'Am', 0, 4), + ]); + tempoTrack?.setRegions([ + new KGTempoRegion('tempo-1', tempoTrack.getId(), tempoTrack.getTrackIndex(), 128, 0, 4, 4), + ]); + storeState.globalTracks = globalTracks; + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Show global tracks' })); + fireEvent.click(screen.getByText('Am')); + fireEvent.click(screen.getByText('128 BPM'), { metaKey: true }); + + expect(storeState.selectedRegionIds).toEqual(['tempo-1']); + + storeState.globalTracks = createDefaultGlobalTracks(); + }); + + it('shift-clicking a region in a different global lane clears the previous global selection', () => { + const globalTracks = createDefaultGlobalTracks(); + const chordTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Chord); + const tempoTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Tempo); + chordTrack?.setRegions([ + new KGChordRegion('chord-1', chordTrack.getId(), chordTrack.getTrackIndex(), 'Am', 0, 4), + ]); + tempoTrack?.setRegions([ + new KGTempoRegion('tempo-1', tempoTrack.getId(), tempoTrack.getTrackIndex(), 128, 0, 4, 4), + new KGTempoRegion('tempo-2', tempoTrack.getId(), tempoTrack.getTrackIndex(), 140, 4, 4, 4), + ]); + storeState.globalTracks = globalTracks; + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Show global tracks' })); + fireEvent.click(screen.getByText('Am')); + fireEvent.click(screen.getByText('140 BPM'), { shiftKey: true }); + + expect(storeState.selectedRegionIds).toEqual(['tempo-2']); + + storeState.globalTracks = createDefaultGlobalTracks(); + }); + it('uses split-insert chord command when the playhead is inside an existing chord region', () => { const globalTracks = createDefaultGlobalTracks(); const chordTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Chord); @@ -439,4 +643,46 @@ describe('MainContent', () => { expect(executeCommandMock).toHaveBeenCalledTimes(1); }); + + it('creates new marker regions with a one-bar default length', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Show global tracks' })); + fireEvent.click(screen.getByRole('button', { name: 'Add Marker global track item' })); + + expect(executeCommandMock).toHaveBeenCalledTimes(1); + expect((executeCommandMock.mock.calls[0][0] as { preferredLength?: number }).preferredLength).toBe( + storeState.timeSignature.numerator + ); + }); + + it('keeps a double-clicked global region editor visible past the sticky left panel', async () => { + const globalTracks = createDefaultGlobalTracks(); + const chordTrack = globalTracks.find(track => track.getType() === GlobalTrackType.Chord); + chordTrack?.setRegions([ + new KGChordRegion('chord-1', chordTrack.getId(), chordTrack.getTrackIndex(), 'Am', 40, 16), + ]); + storeState.globalTracks = globalTracks; + + const requestAnimationFrameSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((callback: FrameRequestCallback) => { + callback(0); + return 1; + }); + + const { container } = render(); + const mainContent = container.querySelector('.main-content') as HTMLDivElement; + Object.defineProperty(mainContent, 'scrollWidth', { configurable: true, value: 4000 }); + Object.defineProperty(mainContent, 'clientWidth', { configurable: true, value: 900 }); + mainContent.scrollLeft = 500; + + fireEvent.click(screen.getByRole('button', { name: 'Show global tracks' })); + fireEvent.doubleClick(screen.getByText('Am')); + + await waitFor(() => { + expect(mainContent.scrollLeft).toBe(188); + }); + + requestAnimationFrameSpy.mockRestore(); + storeState.globalTracks = createDefaultGlobalTracks(); + }); }); diff --git a/src/components/MainContent.tsx b/src/components/MainContent.tsx index 6f98476..6b0d682 100644 --- a/src/components/MainContent.tsx +++ b/src/components/MainContent.tsx @@ -70,6 +70,12 @@ const GLOBAL_TRACKS: GlobalTrackDefinition[] = [ { id: 'chord', label: 'Chord' }, ]; +const DEFAULT_REGION_CLICK_OPTIONS: RegionClickOptions = { + shiftKey: false, + metaKey: false, + ctrlKey: false, +}; + const MainContent: React.FC = ({ onTrackClick = () => { } // Default to empty function if not provided }) => { @@ -417,6 +423,57 @@ const MainContent: React.FC = ({ useProjectStore.setState({ mainContentScrollRequest: null }); }, [mainContentScrollRequest, timeSignature]); + useEffect(() => { + const editingRegionId = editingGlobalRegionId + ?? editingTempoRegionId + ?? editingKeySignatureRegionId + ?? editingChordRegionId; + if (!editingRegionId) { + return; + } + + const container = mainContentRef.current; + if (!container) { + return; + } + + const editingRegion = findProjectRegionById(editingRegionId); + if (!(editingRegion instanceof KGGlobalRegion)) { + return; + } + + const barWidth = TOOLBAR_CONSTANTS.BASE_BAR_WIDTH * barWidthMultiplier; + const leftInset = (parseInt( + getComputedStyle(document.documentElement).getPropertyValue('--track-info-panel-width'), + 10 + ) || 200) + 12; + const regionStartBeat = editingRegion instanceof KGTempoRegion || editingRegion instanceof KGKeySignatureRegion + ? editingRegion.getStartBar() * timeSignature.numerator + : editingRegion.getStartFromBeat(); + const regionStartPixel = (regionStartBeat / timeSignature.numerator) * barWidth; + const minimumVisiblePixel = container.scrollLeft + leftInset; + + if (regionStartPixel >= minimumVisiblePixel) { + return; + } + + const targetScrollLeft = Math.max(0, regionStartPixel - leftInset); + requestAnimationFrame(() => { + if (!mainContentRef.current) { + return; + } + mainContentRef.current.scrollLeft = targetScrollLeft; + }); + }, [ + barWidthMultiplier, + editingChordRegionId, + editingGlobalRegionId, + editingKeySignatureRegionId, + editingTempoRegionId, + findProjectRegionById, + timeSignature.numerator, + ]); + // Effect to verify track updates useEffect(() => { // Check for pending updates @@ -514,7 +571,7 @@ const MainContent: React.FC = ({ if (!regionExists) return; pendingAutoSelectionRegionIdRef.current = null; - selectRegion(pendingRegionId, { shiftKey: false }, regions); + selectRegion(pendingRegionId, DEFAULT_REGION_CLICK_OPTIONS, regions); }, [regions]); // Handle track name edit @@ -783,9 +840,137 @@ const MainContent: React.FC = ({ } }; + const isAdditiveSelection = (options: RegionClickOptions) => options.metaKey || options.ctrlKey; + + const getOrderedTrackRegionIds = useCallback((trackId: string) => ( + regions + .filter(region => region.trackId === trackId) + .sort((left, right) => { + if (left.barNumber !== right.barNumber) { + return left.barNumber - right.barNumber; + } + + if (left.length !== right.length) { + return left.length - right.length; + } + + return left.id.localeCompare(right.id); + }) + .map(region => region.id) + ), [regions]); + + const getOrderedGlobalRegionIds = useCallback((regionType: GlobalTrackDefinition['id']) => ( + globalTracks + .flatMap(globalTrack => globalTrack.getRegions()) + .filter((region): region is KGGlobalRegion => { + if (regionType === 'tempo') { + return region instanceof KGTempoRegion; + } + + if (regionType === 'signature') { + return region instanceof KGKeySignatureRegion; + } + + if (regionType === 'chord') { + return region instanceof KGChordRegion; + } + + return region instanceof KGMarkerRegion; + }) + .sort((left, right) => { + const leftStart = left instanceof KGTempoRegion || left instanceof KGKeySignatureRegion + ? left.getStartBar() + : Math.round(left.getStartFromBeat() / timeSignature.numerator); + const rightStart = right instanceof KGTempoRegion || right instanceof KGKeySignatureRegion + ? right.getStartBar() + : Math.round(right.getStartFromBeat() / timeSignature.numerator); + + if (leftStart !== rightStart) { + return leftStart - rightStart; + } + + const leftLength = left instanceof KGTempoRegion || left instanceof KGKeySignatureRegion + ? left.getLengthBars() + : left.getLength(); + const rightLength = right instanceof KGTempoRegion || right instanceof KGKeySignatureRegion + ? right.getLengthBars() + : right.getLength(); + + if (leftLength !== rightLength) { + return leftLength - rightLength; + } + + return left.getId().localeCompare(right.getId()); + }) + .map(region => region.getId()) + ), [globalTracks, timeSignature.numerator]); + + const appendPrimarySelection = (orderedIds: string[], primaryRegionId: string) => { + const deduped = orderedIds.filter(id => id !== primaryRegionId); + return [...deduped, primaryRegionId]; + }; + + const buildSameTrackRangeSelection = ( + existingRegularSelectionIds: string[], + anchorRegionId: string, + targetRegionId: string, + orderedTrackRegionIds: string[] + ) => { + const anchorIndex = orderedTrackRegionIds.indexOf(anchorRegionId); + const targetIndex = orderedTrackRegionIds.indexOf(targetRegionId); + if (anchorIndex === -1 || targetIndex === -1) { + return [targetRegionId]; + } + + const [startIndex, endIndex] = anchorIndex <= targetIndex + ? [anchorIndex, targetIndex] + : [targetIndex, anchorIndex]; + const rangeIds = orderedTrackRegionIds.slice(startIndex, endIndex + 1); + const rangeIdSet = new Set(rangeIds); + const preservedOtherTrackIds = existingRegularSelectionIds.filter(selectedId => !rangeIdSet.has(selectedId)); + return appendPrimarySelection([...preservedOtherTrackIds, ...rangeIds], targetRegionId); + }; + + const buildSameLaneRangeSelection = ( + existingGlobalSelectionIds: string[], + anchorRegionId: string, + targetRegionId: string, + orderedLaneRegionIds: string[] + ) => { + const anchorIndex = orderedLaneRegionIds.indexOf(anchorRegionId); + const targetIndex = orderedLaneRegionIds.indexOf(targetRegionId); + if (anchorIndex === -1 || targetIndex === -1) { + return [targetRegionId]; + } + + const [startIndex, endIndex] = anchorIndex <= targetIndex + ? [anchorIndex, targetIndex] + : [targetIndex, anchorIndex]; + const rangeIds = orderedLaneRegionIds.slice(startIndex, endIndex + 1); + const laneIdSet = new Set(orderedLaneRegionIds); + const preservedOtherLaneIds = existingGlobalSelectionIds.filter(selectedId => !laneIdSet.has(selectedId)); + return appendPrimarySelection([...preservedOtherLaneIds, ...rangeIds], targetRegionId); + }; + + const getGlobalRegionLaneType = (region: KGGlobalRegion): GlobalTrackDefinition['id'] => { + if (region instanceof KGTempoRegion) { + return 'tempo'; + } + + if (region instanceof KGKeySignatureRegion) { + return 'signature'; + } + + if (region instanceof KGChordRegion) { + return 'chord'; + } + + return 'marker'; + }; + const selectRegion = ( regionId: string, - options: RegionClickOptions = { shiftKey: false }, + options: RegionClickOptions = DEFAULT_REGION_CLICK_OPTIONS, regionsToSearch?: RegionUI[] ) => { const regionsToUse = regionsToSearch || regions; @@ -814,35 +999,91 @@ const MainContent: React.FC = ({ } const regularSelectedRegionIds = selectedRegionIds.filter(selectedId => !isGlobalRegionId(selectedId)); - const orderedSelection = options.shiftKey - ? (selectedRegionIds.includes(regionId) + const additiveSelection = isAdditiveSelection(options); + let orderedSelection: string[]; + + if (additiveSelection) { + orderedSelection = regularSelectedRegionIds.includes(regionId) ? regularSelectedRegionIds.filter(id => id !== regionId) - : [...regularSelectedRegionIds, regionId]) - : [regionId]; + : appendPrimarySelection([...regularSelectedRegionIds, regionId], regionId); + } else if (options.shiftKey) { + const sameTrackSelectedIds = regularSelectedRegionIds.filter(selectedId => { + const selectedRegion = regionsToUse.find(candidate => candidate.id === selectedId); + return selectedRegion?.trackId === region.trackId; + }); + const anchorRegionId = [...sameTrackSelectedIds].reverse().find(selectedId => selectedId !== regionId) ?? null; + + if (!anchorRegionId) { + orderedSelection = [regionId]; + } else { + orderedSelection = buildSameTrackRangeSelection( + regularSelectedRegionIds, + anchorRegionId, + regionId, + getOrderedTrackRegionIds(region.trackId) + ); + } + } else { + orderedSelection = [regionId]; + } applyRegionSelection(orderedSelection); }; - const selectGlobalRegion = (regionId: string, options: RegionClickOptions = { shiftKey: false }) => { + const selectGlobalRegion = (regionId: string, options: RegionClickOptions = DEFAULT_REGION_CLICK_OPTIONS) => { const globalSelectedRegionIds = selectedRegionIds.filter(selectedId => isGlobalRegionId(selectedId)); - const orderedSelection = options.shiftKey - ? (globalSelectedRegionIds.includes(regionId) - ? globalSelectedRegionIds.filter(id => id !== regionId) - : [...globalSelectedRegionIds, regionId]) - : [regionId]; + const globalRegion = findProjectRegionById(regionId); + if (!(globalRegion instanceof KGGlobalRegion)) { + return; + } + + const targetLaneType = getGlobalRegionLaneType(globalRegion); + const sameLaneSelectedRegionIds = globalSelectedRegionIds.filter(selectedId => { + const selectedRegion = findProjectRegionById(selectedId); + return selectedRegion instanceof KGGlobalRegion && getGlobalRegionLaneType(selectedRegion) === targetLaneType; + }); + + const additiveSelection = isAdditiveSelection(options); + let orderedSelection: string[]; + + if (additiveSelection) { + orderedSelection = sameLaneSelectedRegionIds.includes(regionId) + ? sameLaneSelectedRegionIds.filter(id => id !== regionId) + : appendPrimarySelection([...sameLaneSelectedRegionIds, regionId], regionId); + } else if (options.shiftKey) { + const orderedLaneRegionIds = getOrderedGlobalRegionIds(targetLaneType); + + const laneRegionIdSet = new Set(orderedLaneRegionIds); + const sameLaneSelectedIds = sameLaneSelectedRegionIds.filter(selectedId => laneRegionIdSet.has(selectedId)); + const anchorRegionId = [...sameLaneSelectedIds].reverse().find(selectedId => selectedId !== regionId) ?? null; + + if (!anchorRegionId) { + orderedSelection = [regionId]; + } else { + orderedSelection = buildSameLaneRangeSelection( + sameLaneSelectedRegionIds, + anchorRegionId, + regionId, + orderedLaneRegionIds + ); + } + } else { + orderedSelection = [regionId]; + } applyRegionSelection(orderedSelection); }; - const handleRegionLassoSelection = (regionIds: string[], options: RegionClickOptions = { shiftKey: false }) => { + const handleRegionLassoSelection = (regionIds: string[], options: RegionClickOptions = DEFAULT_REGION_CLICK_OPTIONS) => { const orderedRegionIds = regionIds.filter(regionId => regions.some(region => region.id === regionId)); const regularSelectedRegionIds = selectedRegionIds.filter(selectedId => !isGlobalRegionId(selectedId)); - const orderedSelection = options.shiftKey + const additiveSelection = isAdditiveSelection(options); + const orderedSelection = additiveSelection ? orderedRegionIds.reduce((nextSelection, regionId) => { if (nextSelection.includes(regionId)) { return nextSelection.filter(id => id !== regionId); } - return [...nextSelection, regionId]; + return appendPrimarySelection([...nextSelection, regionId], regionId); }, [...regularSelectedRegionIds]) : orderedRegionIds; @@ -859,7 +1100,7 @@ const MainContent: React.FC = ({ return; } - handleRegionLassoSelection([], { shiftKey: false }); + handleRegionLassoSelection([], DEFAULT_REGION_CLICK_OPTIONS); }; const handleRegionLassoCommit = () => { @@ -867,7 +1108,7 @@ const MainContent: React.FC = ({ }; // Handle region single click: selection only (no piano roll opening) - const handleRegionClick = (regionId: string, options: RegionClickOptions = { shiftKey: false }) => { + const handleRegionClick = (regionId: string, options: RegionClickOptions = DEFAULT_REGION_CLICK_OPTIONS) => { if (DEBUG_MODE.MAIN_CONTENT) { console.log(`Region clicked in MainContent (selection only): ${regionId}`); } @@ -900,7 +1141,7 @@ const MainContent: React.FC = ({ } // Reuse selection logic - handleRegionClick(regionId, { shiftKey: false }); + handleRegionClick(regionId, DEFAULT_REGION_CLICK_OPTIONS); // Activate and show piano roll in midi-edit mode openMidiPianoRoll(regionId); @@ -908,7 +1149,7 @@ const MainContent: React.FC = ({ // Handle spectrogram viewer open const handleOpenSpectrogram = (regionId: string) => { - handleRegionClick(regionId, { shiftKey: false }); + handleRegionClick(regionId, DEFAULT_REGION_CLICK_OPTIONS); openSpectrogramViewer(regionId); }; @@ -1002,7 +1243,7 @@ const MainContent: React.FC = ({ const normalizedStartBeat = Math.max(0, Math.round(requestedStartBeat)); const occupiedRegion = markerRegions.find(region => region.getStartFromBeat() === normalizedStartBeat); if (occupiedRegion) { - selectGlobalRegion(occupiedRegion.getId(), { shiftKey: false }); + selectGlobalRegion(occupiedRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); beginEditingGlobalRegion(occupiedRegion.getId()); return; } @@ -1010,7 +1251,7 @@ const MainContent: React.FC = ({ try { const command = new CreateGlobalMarkerRegionCommand( normalizedStartBeat, - 8 * timeSignature.numerator, + timeSignature.numerator, DEFAULT_MARKER_REGION_NAME ); KGCore.instance().executeCommand(command); @@ -1021,7 +1262,7 @@ const MainContent: React.FC = ({ return; } - selectGlobalRegion(createdRegion.getId(), { shiftKey: false }); + selectGlobalRegion(createdRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); setEditingGlobalRegionId(createdRegion.getId()); setEditingGlobalRegionText(createdRegion.getName()); } catch (error) { @@ -1057,7 +1298,7 @@ const MainContent: React.FC = ({ const normalizedStartBar = Math.max(0, Math.min(requestedStartBar, maxBars - 1)); const existingRegionAtStart = signatureRegions.find(region => region.getStartBar() === normalizedStartBar); if (existingRegionAtStart) { - selectGlobalRegion(existingRegionAtStart.getId(), { shiftKey: false }); + selectGlobalRegion(existingRegionAtStart.getId(), DEFAULT_REGION_CLICK_OPTIONS); beginEditingKeySignatureRegion(existingRegionAtStart.getId()); return; } @@ -1072,7 +1313,7 @@ const MainContent: React.FC = ({ return; } - selectGlobalRegion(createdRegion.getId(), { shiftKey: false }); + selectGlobalRegion(createdRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); setEditingKeySignatureRegionId(createdRegion.getId()); } catch (error) { console.error('Error creating key signature region:', error); @@ -1142,7 +1383,7 @@ const MainContent: React.FC = ({ const normalizedStartBar = Math.max(0, Math.min(requestedStartBar, maxBars - 1)); const existingRegionAtStart = tempoRegions.find(region => region.getStartBar() === normalizedStartBar); if (existingRegionAtStart) { - selectGlobalRegion(existingRegionAtStart.getId(), { shiftKey: false }); + selectGlobalRegion(existingRegionAtStart.getId(), DEFAULT_REGION_CLICK_OPTIONS); beginEditingTempoRegion(existingRegionAtStart.getId()); return; } @@ -1158,7 +1399,7 @@ const MainContent: React.FC = ({ return; } - selectGlobalRegion(createdRegion.getId(), { shiftKey: false }); + selectGlobalRegion(createdRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); setEditingTempoRegionId(createdRegion.getId()); setEditingTempoText(createdRegion.getBpm().toString()); } catch (error) { @@ -1189,7 +1430,7 @@ const MainContent: React.FC = ({ && normalizedStartBeat < region.getStartFromBeat() + region.getLength() )); if (occupiedRegion) { - selectGlobalRegion(occupiedRegion.getId(), { shiftKey: false }); + selectGlobalRegion(occupiedRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); beginEditingChordRegion(occupiedRegion.getId()); return; } @@ -1204,7 +1445,7 @@ const MainContent: React.FC = ({ return; } - selectGlobalRegion(createdRegion.getId(), { shiftKey: false }); + selectGlobalRegion(createdRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); setEditingChordRegionId(createdRegion.getId()); } catch (error) { console.error('Error creating chord region:', error); @@ -1229,19 +1470,19 @@ const MainContent: React.FC = ({ const createdRegion = command.getCreatedRegion(); if (!createdRegion) { if (occupiedRegion) { - selectGlobalRegion(occupiedRegion.getId(), { shiftKey: false }); + selectGlobalRegion(occupiedRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); beginEditingChordRegion(occupiedRegion.getId()); } return null; } - selectGlobalRegion(createdRegion.getId(), { shiftKey: false }); + selectGlobalRegion(createdRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); setEditingChordRegionId(createdRegion.getId()); return createdRegion; } catch (error) { console.error('Error creating chord region at exact beat:', error); if (occupiedRegion) { - selectGlobalRegion(occupiedRegion.getId(), { shiftKey: false }); + selectGlobalRegion(occupiedRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); beginEditingChordRegion(occupiedRegion.getId()); } return null; @@ -1278,7 +1519,7 @@ const MainContent: React.FC = ({ .find(region => region.getStartFromBeat() < currentStartBeat); if (targetRegion) { - selectGlobalRegion(targetRegion.getId(), { shiftKey: false }); + selectGlobalRegion(targetRegion.getId(), DEFAULT_REGION_CLICK_OPTIONS); setEditingChordRegionId(targetRegion.getId()); return; } diff --git a/src/components/global-track/GlobalChordLane.test.tsx b/src/components/global-track/GlobalChordLane.test.tsx index b19c655..b006b34 100644 --- a/src/components/global-track/GlobalChordLane.test.tsx +++ b/src/components/global-track/GlobalChordLane.test.tsx @@ -29,6 +29,7 @@ describe('GlobalChordLane', () => { onResizeRegion={vi.fn()} onChangeChord={vi.fn()} onOpenPopup={onOpenPopup} + onTabNavigate={vi.fn()} /> ); @@ -54,6 +55,7 @@ describe('GlobalChordLane', () => { onResizeRegion={vi.fn()} onChangeChord={vi.fn()} onOpenPopup={vi.fn()} + onTabNavigate={vi.fn()} /> ); @@ -95,6 +97,7 @@ describe('GlobalChordLane', () => { onResizeRegion={vi.fn()} onChangeChord={vi.fn()} onOpenPopup={vi.fn()} + onTabNavigate={vi.fn()} /> ); @@ -117,4 +120,31 @@ describe('GlobalChordLane', () => { expect(onMoveRegion).toHaveBeenCalledWith('chord-1', 2); }); + + it('passes modifier state through region selection clicks', () => { + const onSelectRegion = vi.fn(); + + render( + + ); + + fireEvent.click(screen.getByText('Cmaj7'), { metaKey: true, shiftKey: true }); + + expect(onSelectRegion).toHaveBeenCalledWith('chord-1', { shiftKey: true, metaKey: true, ctrlKey: false }); + }); }); diff --git a/src/components/global-track/GlobalChordLane.tsx b/src/components/global-track/GlobalChordLane.tsx index 8de3611..a2265f6 100644 --- a/src/components/global-track/GlobalChordLane.tsx +++ b/src/components/global-track/GlobalChordLane.tsx @@ -27,6 +27,11 @@ type ResizeEdge = 'start' | 'end' | null; const REGION_EDGE_HITBOX_PX = 8; const DRAG_THRESHOLD_PX = 4; +const getRegionClickOptions = (event: Pick): RegionClickOptions => ({ + shiftKey: event.shiftKey, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, +}); const GlobalChordLane: React.FC = ({ chordRegions, @@ -48,6 +53,7 @@ const GlobalChordLane: React.FC = ({ const [previewBeats, setPreviewBeats] = useState>({}); const [hoverEdges, setHoverEdges] = useState>({}); const [isModifierPressed, setIsModifierPressed] = useState(false); + const suppressClickSelectionRef = useRef(false); const interactionRef = useRef<{ mode: 'drag' | 'resize' | null; regionId: string; @@ -193,7 +199,8 @@ const GlobalChordLane: React.FC = ({ if (!interaction.moved) { setPreviewBeats({}); - onSelectRegion(interaction.regionId, { shiftKey: event.shiftKey }); + suppressClickSelectionRef.current = true; + onSelectRegion(interaction.regionId, getRegionClickOptions(event)); return; } @@ -283,7 +290,7 @@ const GlobalChordLane: React.FC = ({ onDoubleClick={(event) => { event.preventDefault(); event.stopPropagation(); - onSelectRegion(region.getId(), { shiftKey: false }); + onSelectRegion(region.getId(), { shiftKey: false, metaKey: false, ctrlKey: false }); onOpenPopup(region.getId()); }} onMouseMove={(event) => { @@ -334,7 +341,11 @@ const GlobalChordLane: React.FC = ({ }} onClick={(event) => { event.stopPropagation(); - onSelectRegion(region.getId(), { shiftKey: event.shiftKey }); + if (suppressClickSelectionRef.current) { + suppressClickSelectionRef.current = false; + return; + } + onSelectRegion(region.getId(), getRegionClickOptions(event)); }} > {region.getSymbol()} diff --git a/src/components/global-track/GlobalKeySignatureLane.tsx b/src/components/global-track/GlobalKeySignatureLane.tsx index 66a48a6..b906970 100644 --- a/src/components/global-track/GlobalKeySignatureLane.tsx +++ b/src/components/global-track/GlobalKeySignatureLane.tsx @@ -26,6 +26,11 @@ type ResizeEdge = 'start' | 'end' | null; const REGION_EDGE_HITBOX_PX = 8; const DRAG_THRESHOLD_PX = 4; +const getRegionClickOptions = (event: Pick): RegionClickOptions => ({ + shiftKey: event.shiftKey, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, +}); const GlobalKeySignatureLane: React.FC = ({ signatureRegions, @@ -45,6 +50,7 @@ const GlobalKeySignatureLane: React.FC = ({ const [previewBars, setPreviewBars] = useState>({}); const [hoverEdges, setHoverEdges] = useState>({}); const [isModifierPressed, setIsModifierPressed] = useState(false); + const suppressClickSelectionRef = useRef(false); const interactionRef = useRef<{ mode: 'resize' | null; regionId: string; @@ -223,7 +229,8 @@ const GlobalKeySignatureLane: React.FC = ({ setPreviewBars({}); if (!shouldResize) { - onSelectRegion(interaction.regionId, { shiftKey: event.shiftKey }); + suppressClickSelectionRef.current = true; + onSelectRegion(interaction.regionId, getRegionClickOptions(event)); return; } @@ -326,7 +333,11 @@ const GlobalKeySignatureLane: React.FC = ({ }} onClick={(event) => { event.stopPropagation(); - onSelectRegion(region.getId(), { shiftKey: event.shiftKey }); + if (suppressClickSelectionRef.current) { + suppressClickSelectionRef.current = false; + return; + } + onSelectRegion(region.getId(), getRegionClickOptions(event)); }} onDoubleClick={(event) => { event.preventDefault(); diff --git a/src/components/global-track/GlobalMarkerLane.tsx b/src/components/global-track/GlobalMarkerLane.tsx index 2a4d837..d952871 100644 --- a/src/components/global-track/GlobalMarkerLane.tsx +++ b/src/components/global-track/GlobalMarkerLane.tsx @@ -26,6 +26,11 @@ type ResizeEdge = 'start' | 'end' | null; const REGION_EDGE_HITBOX_PX = 8; const DRAG_THRESHOLD_PX = 4; +const getRegionClickOptions = (event: Pick): RegionClickOptions => ({ + shiftKey: event.shiftKey, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, +}); const GlobalMarkerLane: React.FC = ({ markerRegions, @@ -186,7 +191,7 @@ const GlobalMarkerLane: React.FC = ({ if (!interaction.moved) { setPreviewBeats({}); - onSelectRegion(interaction.regionId, { shiftKey: event.shiftKey }); + onSelectRegion(interaction.regionId, getRegionClickOptions(event)); return; } @@ -263,7 +268,7 @@ const GlobalMarkerLane: React.FC = ({ }} onDoubleClick={(event) => { event.stopPropagation(); - onSelectRegion(region.getId(), { shiftKey: false }); + onSelectRegion(region.getId(), { shiftKey: false, metaKey: false, ctrlKey: false }); onBeginEdit(region.getId()); }} onMouseMove={(event) => { diff --git a/src/components/global-track/GlobalTempoLane.test.tsx b/src/components/global-track/GlobalTempoLane.test.tsx index 517cea8..e95c083 100644 --- a/src/components/global-track/GlobalTempoLane.test.tsx +++ b/src/components/global-track/GlobalTempoLane.test.tsx @@ -120,4 +120,30 @@ describe('GlobalTempoLane', () => { expect(container.querySelector('.global-marker-region.global-tempo-region')).toBeInTheDocument(); }); + + it('passes modifier state through region selection clicks', () => { + const onSelectRegion = vi.fn(); + + render( + + ); + + fireEvent.click(screen.getByText('128 BPM'), { ctrlKey: true }); + + expect(onSelectRegion).toHaveBeenCalledWith('tempo-1', { shiftKey: false, metaKey: false, ctrlKey: true }); + }); }); diff --git a/src/components/global-track/GlobalTempoLane.tsx b/src/components/global-track/GlobalTempoLane.tsx index 51f3b66..9bf3dc1 100644 --- a/src/components/global-track/GlobalTempoLane.tsx +++ b/src/components/global-track/GlobalTempoLane.tsx @@ -24,6 +24,11 @@ type ResizeEdge = 'start' | 'end' | null; const REGION_EDGE_HITBOX_PX = 8; const DRAG_THRESHOLD_PX = 4; +const getRegionClickOptions = (event: Pick): RegionClickOptions => ({ + shiftKey: event.shiftKey, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, +}); const GlobalTempoLane: React.FC = ({ tempoRegions, @@ -44,6 +49,7 @@ const GlobalTempoLane: React.FC = ({ const [previewBars, setPreviewBars] = useState>({}); const [hoverEdges, setHoverEdges] = useState>({}); const [isModifierPressed, setIsModifierPressed] = useState(false); + const suppressClickSelectionRef = useRef(false); const interactionRef = useRef<{ mode: 'resize' | null; regionId: string; @@ -222,7 +228,8 @@ const GlobalTempoLane: React.FC = ({ setPreviewBars({}); if (!shouldResize) { - onSelectRegion(interaction.regionId, { shiftKey: event.shiftKey }); + suppressClickSelectionRef.current = true; + onSelectRegion(interaction.regionId, getRegionClickOptions(event)); return; } @@ -328,12 +335,16 @@ const GlobalTempoLane: React.FC = ({ }} onClick={(event) => { event.stopPropagation(); - onSelectRegion(region.getId(), { shiftKey: event.shiftKey }); + if (suppressClickSelectionRef.current) { + suppressClickSelectionRef.current = false; + return; + } + onSelectRegion(region.getId(), getRegionClickOptions(event)); }} onDoubleClick={(event) => { event.preventDefault(); event.stopPropagation(); - onSelectRegion(region.getId(), { shiftKey: false }); + onSelectRegion(region.getId(), { shiftKey: false, metaKey: false, ctrlKey: false }); onBeginEdit(region.getId()); }} > diff --git a/src/components/interfaces.ts b/src/components/interfaces.ts index 4f7367c..e76a18b 100644 --- a/src/components/interfaces.ts +++ b/src/components/interfaces.ts @@ -25,6 +25,8 @@ export interface RegionPreviewContentStyle { export interface RegionClickOptions { shiftKey: boolean; + metaKey: boolean; + ctrlKey: boolean; } // Define resize action types diff --git a/src/components/track/Region.css b/src/components/track/Region.css index 1de1aeb..29f8927 100644 --- a/src/components/track/Region.css +++ b/src/components/track/Region.css @@ -15,10 +15,6 @@ will-change: transform; } -.track-region:hover { - box-shadow: 0 0 0 1px #7a9bba; -} - .track-region.selected { /* box-shadow: 0 0 0 2px #ffffff, 0 0 8px rgba(255, 255, 255, 0.3); */ /* let's only apply a border */ border-color: #ffffff; @@ -99,10 +95,6 @@ border-color: #4a8b5a; } -.track-region.audio-region:hover { - box-shadow: 0 0 0 1px #6aab7a; -} - .track-region.audio-region.selected { border-color: #ffffff; } diff --git a/src/components/track/RegionItem.test.tsx b/src/components/track/RegionItem.test.tsx index 7164275..1eb0ebc 100644 --- a/src/components/track/RegionItem.test.tsx +++ b/src/components/track/RegionItem.test.tsx @@ -72,7 +72,7 @@ describe('RegionItem', () => { fireEvent.mouseMove(document, { clientX: 102, clientY: 102 }); fireEvent.mouseUp(document, { clientX: 102, clientY: 102 }); - expect(onClick).toHaveBeenCalledWith('midi-1', { shiftKey: false }); + expect(onClick).toHaveBeenCalledWith('midi-1', { shiftKey: false, metaKey: false, ctrlKey: false }); expect(onDragStart).not.toHaveBeenCalled(); expect(onDrag).not.toHaveBeenCalled(); expect(onDragEnd).not.toHaveBeenCalled(); @@ -88,7 +88,20 @@ describe('RegionItem', () => { fireEvent.mouseDown(region!, { clientX: 100, clientY: 100, shiftKey: true }); fireEvent.mouseUp(document, { clientX: 100, clientY: 100, shiftKey: true }); - expect(onClick).toHaveBeenCalledWith('midi-1', { shiftKey: true }); + expect(onClick).toHaveBeenCalledWith('midi-1', { shiftKey: true, metaKey: false, ctrlKey: false }); + }); + + it('passes cmd-click state through the region click callback', () => { + const onClick = vi.fn(); + const { container } = renderRegion({ onClick }); + const region = container.querySelector('.track-region'); + + expect(region).toBeTruthy(); + + fireEvent.mouseDown(region!, { clientX: 100, clientY: 100, metaKey: true }); + fireEvent.mouseUp(document, { clientX: 100, clientY: 100, metaKey: true }); + + expect(onClick).toHaveBeenCalledWith('midi-1', { shiftKey: false, metaKey: true, ctrlKey: false }); }); it('starts a drag after crossing the movement threshold', () => { diff --git a/src/components/track/RegionItem.tsx b/src/components/track/RegionItem.tsx index 2d9ce37..bfcc619 100644 --- a/src/components/track/RegionItem.tsx +++ b/src/components/track/RegionItem.tsx @@ -14,6 +14,11 @@ import { KGCore } from '../../core/KGCore'; import { beatRangeToSeconds } from '../../util/globalTrackUtil'; const DRAG_START_THRESHOLD_PX = 4; +const getRegionClickOptions = (event: Pick): RegionClickOptions => ({ + shiftKey: event.shiftKey, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, +}); interface RegionItemProps { id: string; @@ -481,7 +486,7 @@ const RegionItem: React.FC = ({ if (DEBUG_MODE.REGION_ITEM) { console.log(`REGION CLICKED (pencil mode): regionId=${id}`); } - onClick(id, { shiftKey: e.shiftKey }); + onClick(id, getRegionClickOptions(e)); } return; } @@ -608,7 +613,7 @@ const RegionItem: React.FC = ({ if (DEBUG_MODE.REGION_ITEM) { console.log(`REGION CLICKED: regionId=${id}`); } - onClick(id, { shiftKey: e.shiftKey }); + onClick(id, getRegionClickOptions(e)); } isPendingDragRef.current = false; @@ -708,7 +713,7 @@ const RegionItem: React.FC = ({ if (onOpenPianoRoll) { onOpenPianoRoll(id); } else if (onClick) { - onClick(id, { shiftKey: e.shiftKey }); + onClick(id, getRegionClickOptions(e)); } }} aria-label="Open piano roll" diff --git a/src/components/track/TrackGridItem.tsx b/src/components/track/TrackGridItem.tsx index b1a84a7..51df9c6 100644 --- a/src/components/track/TrackGridItem.tsx +++ b/src/components/track/TrackGridItem.tsx @@ -8,7 +8,6 @@ import TrackAutomationLane from './TrackAutomationLane'; import type { RegionClickOptions, RegionPreviewContentStyle, RegionUI, ResizeAction } from '../interfaces'; import { REGION_CONSTANTS, DEBUG_MODE } from '../../constants'; import { KGMainContentState } from '../../core/state/KGMainContentState'; -import { isModifierKeyPressed } from '../../util/osUtil'; import { useProjectStore } from '../../stores/projectStore'; interface RegionResizePreviewBaseline { @@ -200,28 +199,23 @@ const TrackGridItem: React.FC = ({ }; }, [gridContainerRef]); - // Track modifier key state for cursor feedback + // Track tool state for cursor feedback. useEffect(() => { - const handleKeyDown = (e: KeyboardEvent) => { - if (isModifierKeyPressed(e)) { - setIsModifierPressed(true); - } - }; - - const handleKeyUp = (e: KeyboardEvent) => { - if (!isModifierKeyPressed(e)) { - setIsModifierPressed(false); - } + const syncCursorState = () => { + setIsModifierPressed(KGMainContentState.instance().getActiveTool() === 'pencil'); }; // Add global event listeners - window.addEventListener('keydown', handleKeyDown); - window.addEventListener('keyup', handleKeyUp); + window.addEventListener('keydown', syncCursorState); + window.addEventListener('keyup', syncCursorState); + window.addEventListener('focus', syncCursorState); + syncCursorState(); // Cleanup listeners on unmount return () => { - window.removeEventListener('keydown', handleKeyDown); - window.removeEventListener('keyup', handleKeyUp); + window.removeEventListener('keydown', syncCursorState); + window.removeEventListener('keyup', syncCursorState); + window.removeEventListener('focus', syncCursorState); }; }, []); @@ -785,7 +779,7 @@ const TrackGridItem: React.FC = ({ onOpenPianoRoll(regionId); } else if (onRegionClick) { // Fallback to legacy behavior - onRegionClick(regionId, { shiftKey: false }); + onRegionClick(regionId, { shiftKey: false, metaKey: false, ctrlKey: false }); } }} onOpenSpectrogram={audioRegion ? (regionId) => { diff --git a/src/components/track/TrackGridPanel.test.tsx b/src/components/track/TrackGridPanel.test.tsx index af80b75..b3ef39e 100644 --- a/src/components/track/TrackGridPanel.test.tsx +++ b/src/components/track/TrackGridPanel.test.tsx @@ -4,6 +4,9 @@ import { fireEvent, render } from '@testing-library/react'; import TrackGridPanel from './TrackGridPanel'; import { createMockMidiRegion, createMockMidiTrack } from '../../test/utils/mock-data'; +const executeCommandMock = vi.fn(); +const getCreatedRegionMock = vi.fn(); + vi.mock('../../stores/projectStore', () => ({ useProjectStore: (selector?: (state: { selectedRegionIds: string[], @@ -26,6 +29,36 @@ vi.mock('../common', () => ({ FileImportModal: () => null, })); +vi.mock('../../core/KGCore', () => ({ + KGCore: { + instance: () => ({ + executeCommand: executeCommandMock, + }), + }, +})); + +vi.mock('../../util/miscUtil', () => ({ + generateNewRegionName: () => 'New Region', +})); + +vi.mock('../../core/commands', async () => { + const actual = await vi.importActual('../../core/commands'); + return { + ...actual, + CreateRegionCommand: { + fromBarCoordinates: vi.fn((trackId: string, trackIndex: number, barNumber: number) => ({ + getCreatedRegion: () => getCreatedRegionMock() ?? createMockMidiRegion({ + id: 'created-region', + trackId, + trackIndex, + startFromBeat: (barNumber - 1) * 4, + length: 4, + }), + })), + }, + }; +}); + describe('TrackGridPanel lasso selection', () => { beforeAll(() => { Object.defineProperty(HTMLCanvasElement.prototype, 'getContext', { @@ -57,6 +90,7 @@ describe('TrackGridPanel lasso selection', () => { trackB.setTrackIndex(1); const onRegionLassoSelection = vi.fn(); + const onRegionCreated = vi.fn(); const view = render( { dragOverTrackIndex={null} selectedRegionId={null} projectName="Test" - onRegionCreated={vi.fn()} + onRegionCreated={onRegionCreated} onRegionLassoSelection={onRegionLassoSelection} /> ); @@ -90,11 +124,13 @@ describe('TrackGridPanel lasso selection', () => { toJSON: () => ({}), }); - return { ...view, onRegionLassoSelection }; + return { ...view, onRegionLassoSelection, onRegionCreated }; }; beforeEach(() => { vi.restoreAllMocks(); + executeCommandMock.mockReset(); + getCreatedRegionMock.mockReset(); }); it('selects intersecting regions across multiple track rows', () => { @@ -105,7 +141,7 @@ describe('TrackGridPanel lasso selection', () => { fireEvent.mouseMove(document, { clientX: 130, clientY: 200 }); fireEvent.mouseUp(document, { clientX: 130, clientY: 200 }); - expect(onRegionLassoSelection).toHaveBeenCalledWith(['region-a', 'region-b'], { shiftKey: false }); + expect(onRegionLassoSelection).toHaveBeenCalledWith(['region-a', 'region-b'], { shiftKey: false, metaKey: false, ctrlKey: false }); }); it('moves the release-point region to the end of the lasso selection order', () => { @@ -116,7 +152,7 @@ describe('TrackGridPanel lasso selection', () => { fireEvent.mouseMove(document, { clientX: 20, clientY: 20 }); fireEvent.mouseUp(document, { clientX: 20, clientY: 20 }); - expect(onRegionLassoSelection).toHaveBeenCalledWith(['region-b', 'region-a'], { shiftKey: false }); + expect(onRegionLassoSelection).toHaveBeenCalledWith(['region-b', 'region-a'], { shiftKey: false, metaKey: false, ctrlKey: false }); }); it('uses the closest intersected region as primary when release is outside all regions', () => { @@ -127,7 +163,7 @@ describe('TrackGridPanel lasso selection', () => { fireEvent.mouseMove(document, { clientX: 150, clientY: 170 }); fireEvent.mouseUp(document, { clientX: 150, clientY: 170 }); - expect(onRegionLassoSelection).toHaveBeenCalledWith(['region-a', 'region-b'], { shiftKey: false }); + expect(onRegionLassoSelection).toHaveBeenCalledWith(['region-a', 'region-b'], { shiftKey: false, metaKey: false, ctrlKey: false }); }); it('clears selection on a plain empty-space click', () => { @@ -137,6 +173,36 @@ describe('TrackGridPanel lasso selection', () => { fireEvent.mouseDown(firstTrackGrid, { clientX: 10, clientY: 10, button: 0 }); fireEvent.mouseUp(document, { clientX: 11, clientY: 11 }); - expect(onRegionLassoSelection).toHaveBeenCalledWith([], { shiftKey: false }); + expect(onRegionLassoSelection).toHaveBeenCalledWith([], { shiftKey: false, metaKey: false, ctrlKey: false }); + }); + + it('does not create a region when ctrl-clicking an existing region', () => { + const { container, onRegionCreated } = renderPanel(); + const region = container.querySelector('[data-region-id="region-a"]') as HTMLDivElement; + + fireEvent.mouseDown(region, { clientX: 20, clientY: 20, button: 0, ctrlKey: true }); + fireEvent.mouseUp(document, { clientX: 20, clientY: 20, ctrlKey: true }); + fireEvent.click(region, { ctrlKey: true }); + + expect(onRegionCreated).not.toHaveBeenCalled(); + }); + + it('creates a region when ctrl-clicking empty track space', async () => { + const { container, onRegionCreated } = renderPanel(); + const firstTrackGrid = container.querySelector('[data-test-id="track-grid-1"]') as HTMLDivElement; + getCreatedRegionMock.mockReturnValue(createMockMidiRegion({ + id: 'created-region', + trackId: '1', + trackIndex: 0, + startFromBeat: 12, + length: 4, + name: 'New Region', + })); + + fireEvent.click(firstTrackGrid, { clientX: 140, clientY: 20, ctrlKey: true }); + + await vi.waitFor(() => { + expect(onRegionCreated).toHaveBeenCalledTimes(1); + }); }); }); diff --git a/src/components/track/TrackGridPanel.tsx b/src/components/track/TrackGridPanel.tsx index 93644e4..eaacb28 100644 --- a/src/components/track/TrackGridPanel.tsx +++ b/src/components/track/TrackGridPanel.tsx @@ -20,6 +20,12 @@ import * as Tone from 'tone'; import { useProjectStore } from '../../stores/projectStore'; import { getAudioRegionDisplayLengthBeats } from '../../util/globalTrackUtil'; +const getRegionClickOptions = (event: Pick): RegionClickOptions => ({ + shiftKey: event.shiftKey, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, +}); + interface TrackGridPanelProps { tracks: KGTrack[]; regions: RegionUI[]; @@ -130,7 +136,7 @@ const TrackGridPanel: React.FC = ({ if (isClick) { if (!isLassoShiftPressedRef.current) { - onRegionLassoSelection?.([], { shiftKey: false }); + onRegionLassoSelection?.([], getRegionClickOptions({ shiftKey: false, metaKey: false, ctrlKey: false })); } } else { const containerWidth = gridContainerRef.current.clientWidth; @@ -194,7 +200,11 @@ const TrackGridPanel: React.FC = ({ intersectedRegionIds.push(primaryRegionId); } - onRegionLassoSelection?.(intersectedRegionIds, { shiftKey: isLassoShiftPressedRef.current }); + onRegionLassoSelection?.(intersectedRegionIds, { + shiftKey: isLassoShiftPressedRef.current, + metaKey: false, + ctrlKey: false, + }); onRegionLassoCommit?.(); } @@ -390,12 +400,23 @@ const TrackGridPanel: React.FC = ({ createRegionAtPosition(e, trackIndex); }; - // Handle single click on track grid for pencil mode or modifier+click + // Handle single click on track grid for pencil mode or modifier-click on empty space. const handleTrackGridClick = (e: React.MouseEvent, trackIndex: number) => { - // Create region on single click in pencil mode OR when modifier key is pressed - if (KGMainContentState.instance().getActiveTool() === 'pencil' || isModifierKeyPressed(e)) { - createRegionAtPosition(e, trackIndex); + if (!(e.target instanceof HTMLElement)) { + return; } + + const clickedInsideRegion = e.target.closest('.track-region'); + if (clickedInsideRegion) { + return; + } + + const isPencilMode = KGMainContentState.instance().getActiveTool() === 'pencil'; + if (!isPencilMode && !isModifierKeyPressed(e)) { + return; + } + + createRegionAtPosition(e, trackIndex); }; // Handle region resize during drag