diff --git a/src/components/NoteList.test.tsx b/src/components/NoteList.test.tsx index 34371a10..fa817e3a 100644 --- a/src/components/NoteList.test.tsx +++ b/src/components/NoteList.test.tsx @@ -288,27 +288,27 @@ describe('NoteList click behavior', () => { expect(noopSelect).not.toHaveBeenCalled() }) - it('Cmd+Click toggles multi-select on a note', () => { + it('Cmd+Click calls onSelectNote (opens in new tab)', () => { render() fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) + expect(noopSelect).toHaveBeenCalledWith(mockEntries[0]) expect(noopReplace).not.toHaveBeenCalled() - expect(screen.getByTestId('multi-selected-item')).toBeInTheDocument() - expect(screen.getByTestId('bulk-action-bar')).toBeInTheDocument() }) - it('Ctrl+Click toggles multi-select (Windows/Linux)', () => { + it('Ctrl+Click calls onSelectNote (Windows/Linux)', () => { render() fireEvent.click(screen.getByText('Build Laputa App'), { ctrlKey: true }) + expect(noopSelect).toHaveBeenCalledWith(mockEntries[0]) expect(noopReplace).not.toHaveBeenCalled() - expect(screen.getByTestId('multi-selected-item')).toBeInTheDocument() }) - it('Cmd+Click on entity pinned card toggles multi-select', () => { + it('Cmd+Click on entity pinned card calls onSelectNote', () => { render( ) const titles = screen.getAllByText('Build Laputa App') fireEvent.click(titles[titles.length - 1], { metaKey: true }) + expect(noopSelect).toHaveBeenCalledWith(mockEntries[0]) expect(noopReplace).not.toHaveBeenCalled() }) @@ -996,40 +996,10 @@ describe('NoteList — multi-select', () => { noopReplace.mockClear() }) - it('Cmd+Click selects multiple notes', () => { - render() - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) - fireEvent.click(screen.getByText('Facebook Ads Strategy'), { metaKey: true }) - const selected = screen.getAllByTestId('multi-selected-item') - expect(selected).toHaveLength(2) - }) - - it('Cmd+Click toggles off an already-selected note', () => { - render() - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) - expect(screen.getAllByTestId('multi-selected-item')).toHaveLength(1) - // Toggle off - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) - expect(screen.queryByTestId('multi-selected-item')).not.toBeInTheDocument() - expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() - }) - - it('regular click clears multi-select and opens note', () => { - render() - // Multi-select two notes - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) - fireEvent.click(screen.getByText('Facebook Ads Strategy'), { metaKey: true }) - expect(screen.getAllByTestId('multi-selected-item')).toHaveLength(2) - // Regular click clears selection and opens note - fireEvent.click(screen.getByText('Matteo Cellini')) - expect(screen.queryByTestId('multi-selected-item')).not.toBeInTheDocument() - expect(noopReplace).toHaveBeenCalledWith(mockEntries[2]) - }) - it('Shift+Click selects a range of notes', () => { render() - // First Cmd+Click to set anchor - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) + // Regular click to set anchor + fireEvent.click(screen.getByText('Build Laputa App')) // Shift+Click to select range fireEvent.click(screen.getByText('Matteo Cellini'), { shiftKey: true }) // Should select all notes between Build Laputa App and Matteo Cellini (inclusive) @@ -1037,10 +1007,34 @@ describe('NoteList — multi-select', () => { expect(selected.length).toBeGreaterThanOrEqual(2) }) + it('regular click clears multi-select and opens note', () => { + render() + // Select range via Shift+click + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) + expect(screen.getAllByTestId('multi-selected-item').length).toBeGreaterThanOrEqual(1) + // Regular click clears selection and opens note + fireEvent.click(screen.getByText('Matteo Cellini')) + expect(screen.queryByTestId('multi-selected-item')).not.toBeInTheDocument() + expect(noopReplace).toHaveBeenCalledWith(mockEntries[2]) + }) + + it('Cmd+Click clears multi-select and opens in new tab', () => { + render() + // Select range via Shift+click + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) + expect(screen.getAllByTestId('multi-selected-item').length).toBeGreaterThanOrEqual(1) + // Cmd+click clears selection and opens in new tab + fireEvent.click(screen.getByText('Matteo Cellini'), { metaKey: true }) + expect(screen.queryByTestId('multi-selected-item')).not.toBeInTheDocument() + expect(noopSelect).toHaveBeenCalledWith(mockEntries[2]) + }) + it('shows bulk action bar with correct count', () => { render() - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) - fireEvent.click(screen.getByText('Facebook Ads Strategy'), { metaKey: true }) + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) expect(screen.getByTestId('bulk-action-bar')).toBeInTheDocument() expect(screen.getByText('2 selected')).toBeInTheDocument() }) @@ -1048,30 +1042,65 @@ describe('NoteList — multi-select', () => { it('bulk archive calls onBulkArchive and clears selection', () => { const onBulkArchive = vi.fn() render() - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) fireEvent.click(screen.getByTestId('bulk-archive-btn')) - expect(onBulkArchive).toHaveBeenCalledWith([mockEntries[0].path]) + expect(onBulkArchive).toHaveBeenCalledWith([mockEntries[0].path, mockEntries[1].path]) expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() }) it('bulk trash calls onBulkTrash and clears selection', () => { const onBulkTrash = vi.fn() render() - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) fireEvent.click(screen.getByTestId('bulk-trash-btn')) - expect(onBulkTrash).toHaveBeenCalledWith([mockEntries[0].path]) + expect(onBulkTrash).toHaveBeenCalledWith([mockEntries[0].path, mockEntries[1].path]) expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() }) it('clear button on bulk action bar clears selection', () => { render() - fireEvent.click(screen.getByText('Build Laputa App'), { metaKey: true }) + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) expect(screen.getByTestId('bulk-action-bar')).toBeInTheDocument() fireEvent.click(screen.getByTestId('bulk-clear-btn')) expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() expect(screen.queryByTestId('multi-selected-item')).not.toBeInTheDocument() }) + it('Cmd+E archives selected notes when multiselect is active', () => { + const onBulkArchive = vi.fn() + render() + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) + expect(screen.getByTestId('bulk-action-bar')).toBeInTheDocument() + fireEvent.keyDown(window, { key: 'e', metaKey: true }) + expect(onBulkArchive).toHaveBeenCalledWith([mockEntries[0].path, mockEntries[1].path]) + expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() + }) + + it('Cmd+Backspace trashes selected notes when multiselect is active', () => { + const onBulkTrash = vi.fn() + render() + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) + expect(screen.getByTestId('bulk-action-bar')).toBeInTheDocument() + fireEvent.keyDown(window, { key: 'Backspace', metaKey: true }) + expect(onBulkTrash).toHaveBeenCalledWith([mockEntries[0].path, mockEntries[1].path]) + expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() + }) + + it('Cmd+Delete trashes selected notes when multiselect is active', () => { + const onBulkTrash = vi.fn() + render() + fireEvent.click(screen.getByText('Build Laputa App')) + fireEvent.click(screen.getByText('Facebook Ads Strategy'), { shiftKey: true }) + fireEvent.keyDown(window, { key: 'Delete', metaKey: true }) + expect(onBulkTrash).toHaveBeenCalledWith([mockEntries[0].path, mockEntries[1].path]) + expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() + }) + it('no bulk action bar when nothing is selected', () => { render() expect(screen.queryByTestId('bulk-action-bar')).not.toBeInTheDocument() diff --git a/src/components/NoteList.tsx b/src/components/NoteList.tsx index 22f6fc14..fc8568bf 100644 --- a/src/components/NoteList.tsx +++ b/src/components/NoteList.tsx @@ -197,15 +197,16 @@ function countExpiredTrash(entries: VaultEntry[]): number { // --- Click routing --- -type MultiSelectActions = { toggle: (path: string) => void; selectRange: (path: string) => void; clear: () => void; setAnchor: (path: string) => void } +type MultiSelectActions = { selectRange: (path: string) => void; clear: () => void; setAnchor: (path: string) => void } function routeNoteClick( entry: VaultEntry, e: React.MouseEvent, onReplaceActiveTab: (entry: VaultEntry) => void, + onSelectNote: (entry: VaultEntry) => void, multiSelect: MultiSelectActions, ) { if (e.shiftKey) { multiSelect.selectRange(entry.path) } - else if (e.metaKey || e.ctrlKey) { multiSelect.toggle(entry.path) } + else if (e.metaKey || e.ctrlKey) { multiSelect.clear(); onSelectNote(entry) } else { multiSelect.clear(); multiSelect.setAnchor(entry.path); onReplaceActiveTab(entry) } } @@ -276,7 +277,7 @@ function useNoteListData({ entries, selection, allContent, query, listSort, list const defaultGetNoteStatus = (): NoteStatus => 'clean' -function NoteListInner({ entries, selection, selectedNote, allContent, modifiedFiles, getNoteStatus, sidebarCollapsed, onReplaceActiveTab, onCreateNote, onBulkArchive, onBulkTrash }: NoteListProps) { +function NoteListInner({ entries, selection, selectedNote, allContent, modifiedFiles, getNoteStatus, sidebarCollapsed, onSelectNote, onReplaceActiveTab, onCreateNote, onBulkArchive, onBulkTrash }: NoteListProps) { const [search, setSearch] = useState('') const [searchVisible, setSearchVisible] = useState(false) const [collapsedGroups, setCollapsedGroups] = useState>(new Set()) @@ -314,10 +315,23 @@ function NoteListInner({ entries, selection, selectedNote, allContent, modifiedF useEffect(() => { multiSelect.clear() }, [selection]) // eslint-disable-line react-hooks/exhaustive-deps -- clear on selection change only const handleClickNote = useCallback((entry: VaultEntry, e: React.MouseEvent) => { - routeNoteClick(entry, e, onReplaceActiveTab, multiSelect) - }, [onReplaceActiveTab, multiSelect]) + routeNoteClick(entry, e, onReplaceActiveTab, onSelectNote, multiSelect) + }, [onReplaceActiveTab, onSelectNote, multiSelect]) - // Keyboard: Escape to clear, Cmd+A to select all + const handleBulkArchive = useCallback(() => { + const paths = [...multiSelect.selectedPaths] + multiSelect.clear() + onBulkArchive?.(paths) + }, [multiSelect, onBulkArchive]) + + const handleBulkTrash = useCallback(() => { + const paths = [...multiSelect.selectedPaths] + multiSelect.clear() + onBulkTrash?.(paths) + }, [multiSelect, onBulkTrash]) + + // Keyboard: Escape to clear, Cmd+A to select all, Cmd+E/Cmd+Delete for bulk actions + // Uses capture phase so bulk shortcuts preempt the global single-note handler useEffect(() => { function handleKeyDown(e: KeyboardEvent) { if (e.key === 'Escape' && multiSelect.isMultiSelecting) { @@ -332,22 +346,21 @@ function NoteListInner({ entries, selection, selectedNote, allContent, modifiedF multiSelect.selectAll() } } + if (multiSelect.isMultiSelecting && (e.metaKey || e.ctrlKey)) { + if (e.key === 'e') { + e.preventDefault() + e.stopPropagation() + handleBulkArchive() + } else if (e.key === 'Backspace' || e.key === 'Delete') { + e.preventDefault() + e.stopPropagation() + handleBulkTrash() + } + } } - window.addEventListener('keydown', handleKeyDown) - return () => window.removeEventListener('keydown', handleKeyDown) - }, [multiSelect, isEntityView]) - - const handleBulkArchive = useCallback(() => { - const paths = [...multiSelect.selectedPaths] - multiSelect.clear() - onBulkArchive?.(paths) - }, [multiSelect, onBulkArchive]) - - const handleBulkTrash = useCallback(() => { - const paths = [...multiSelect.selectedPaths] - multiSelect.clear() - onBulkTrash?.(paths) - }, [multiSelect, onBulkTrash]) + window.addEventListener('keydown', handleKeyDown, true) + return () => window.removeEventListener('keydown', handleKeyDown, true) + }, [multiSelect, isEntityView, handleBulkArchive, handleBulkTrash]) const renderItem = useCallback((entry: VaultEntry) => (