From 82192f23be5ceeba9f7e1a49cf6f1a02a564d2c6 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Thu, 16 Apr 2026 00:38:43 +0200 Subject: [PATCH] feat: simplify bulk action bar --- src/App.tsx | 4 +- src/components/BulkActionBar.test.tsx | 60 +++++++++++- src/components/BulkActionBar.tsx | 95 ++++++++++++------ src/components/NoteList.behavior.test.tsx | 1 + src/components/NoteList.tsx | 16 ++- src/components/note-list/NoteListLayout.tsx | 46 ++++++--- src/hooks/useBulkActions.test.ts | 102 +++++++++++++++----- src/hooks/useBulkActions.ts | 48 +++++++-- 8 files changed, 289 insertions(+), 83 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 74bf2ed7..9dce7e44 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -666,7 +666,7 @@ function App() { return [...builtIn, ...Array.from(customFields).sort()] }, [vault.entries]) - const bulkActions = useBulkActions(entryActions, setToastMessage) + const bulkActions = useBulkActions(entryActions, vault.entries, setToastMessage) // Raw-toggle ref: Editor registers its handleToggleRaw here so the command palette can call it const rawToggleRef = useRef<() => void>(() => {}) @@ -888,7 +888,7 @@ function App() { {effectiveSelection.kind === 'filter' && effectiveSelection.filter === 'pulse' ? ( setViewMode('all')} /> ) : ( - diffToggleRef.current()} onOpenDeletedNote={handleOpenDeletedNote} inboxNoteListProperties={vaultConfig.inbox?.noteListProperties ?? null} onUpdateInboxNoteListProperties={handleUpdateInboxNoteListProperties} views={vault.views} visibleNotesRef={visibleNotesRef} /> + diffToggleRef.current()} onOpenDeletedNote={handleOpenDeletedNote} inboxNoteListProperties={vaultConfig.inbox?.noteListProperties ?? null} onUpdateInboxNoteListProperties={handleUpdateInboxNoteListProperties} views={vault.views} visibleNotesRef={visibleNotesRef} /> )} diff --git a/src/components/BulkActionBar.test.tsx b/src/components/BulkActionBar.test.tsx index 7b9b16c0..eff6fc32 100644 --- a/src/components/BulkActionBar.test.tsx +++ b/src/components/BulkActionBar.test.tsx @@ -5,15 +5,20 @@ import { BulkActionBar } from './BulkActionBar' describe('BulkActionBar', () => { const defaultProps = { count: 3, + onOrganize: vi.fn(), onArchive: vi.fn(), onDelete: vi.fn(), onClear: vi.fn(), } - it('shows Archive and Delete buttons in normal view', () => { + it('shows icon-only organize, archive, and delete buttons in normal view', () => { render() + expect(screen.getByTestId('bulk-organize-btn')).toBeInTheDocument() expect(screen.getByTestId('bulk-archive-btn')).toBeInTheDocument() expect(screen.getByTestId('bulk-delete-btn')).toBeInTheDocument() + expect(screen.queryByText('Organize')).not.toBeInTheDocument() + expect(screen.queryByText('Archive')).not.toBeInTheDocument() + expect(screen.queryByText('Delete')).not.toBeInTheDocument() }) it('shows selected count', () => { @@ -21,13 +26,64 @@ describe('BulkActionBar', () => { expect(screen.getByText('5 selected')).toBeInTheDocument() }) - it('shows Unarchive and Delete buttons in archived view', () => { + it('shows organize, unarchive, and delete buttons in archived view', () => { render() + expect(screen.getByTestId('bulk-organize-btn')).toBeInTheDocument() expect(screen.getByTestId('bulk-unarchive-btn')).toBeInTheDocument() expect(screen.getByTestId('bulk-delete-btn')).toBeInTheDocument() expect(screen.queryByTestId('bulk-archive-btn')).not.toBeInTheDocument() }) + it('exposes accessible names and a destructive variant for icon-only actions', () => { + render() + expect(screen.getByRole('button', { name: 'Organize selected notes' })).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Archive selected notes' })).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Permanently delete selected notes' })).toHaveAttribute('data-variant', 'destructive') + expect(screen.getByRole('button', { name: 'Clear selection' })).toBeInTheDocument() + }) + + it('keeps the icon-only controls focusable and activatable', () => { + const onOrganize = vi.fn() + const onArchive = vi.fn() + const onDelete = vi.fn() + const onClear = vi.fn() + + render( + , + ) + + const organizeButton = screen.getByRole('button', { name: 'Organize selected notes' }) + const archiveButton = screen.getByRole('button', { name: 'Archive selected notes' }) + const deleteButton = screen.getByRole('button', { name: 'Permanently delete selected notes' }) + const clearButton = screen.getByRole('button', { name: 'Clear selection' }) + + organizeButton.focus() + expect(organizeButton).toHaveFocus() + fireEvent.click(organizeButton) + expect(onOrganize).toHaveBeenCalledTimes(1) + + archiveButton.focus() + expect(archiveButton).toHaveFocus() + fireEvent.click(archiveButton) + expect(onArchive).toHaveBeenCalledTimes(1) + + deleteButton.focus() + expect(deleteButton).toHaveFocus() + fireEvent.click(deleteButton) + expect(onDelete).toHaveBeenCalledTimes(1) + + clearButton.focus() + expect(clearButton).toHaveFocus() + fireEvent.click(clearButton) + expect(onClear).toHaveBeenCalledTimes(1) + }) + it('calls onUnarchive when Unarchive button clicked in archived view', () => { const onUnarchive = vi.fn() render() diff --git a/src/components/BulkActionBar.tsx b/src/components/BulkActionBar.tsx index 707c7c9d..50127bd5 100644 --- a/src/components/BulkActionBar.tsx +++ b/src/components/BulkActionBar.tsx @@ -1,45 +1,74 @@ import { memo } from 'react' -import { Archive, ArrowCounterClockwise, Trash, X } from '@phosphor-icons/react' +import { Archive, ArrowCounterClockwise, CheckCircle, Trash, X } from '@phosphor-icons/react' +import { Button } from '@/components/ui/button' interface BulkActionBarProps { count: number isArchivedView?: boolean + onOrganize?: () => void onArchive: () => void onDelete: () => void onUnarchive?: () => void onClear: () => void } -const actionBtnStyle = { padding: '5px 10px', borderRadius: 6, background: 'rgba(255,255,255,0.12)', color: 'inherit', fontSize: 12, fontWeight: 500 } as const -const destructiveBtnStyle = { padding: '5px 10px', borderRadius: 6, background: 'rgba(224,62,62,0.2)', color: 'var(--destructive)', fontSize: 12, fontWeight: 500 } as const +interface BulkActionButtonProps { + ariaLabel: string + children: React.ReactNode + destructive?: boolean + onClick?: () => void + testId: string +} + +function BulkActionButton({ ariaLabel, children, destructive = false, onClick, testId }: BulkActionButtonProps) { + return ( + + ) +} + +function renderPrimaryActions( + isArchivedView: boolean, + onOrganize: (() => void) | undefined, + onArchive: () => void, + onDelete: () => void, + onUnarchive: (() => void) | undefined, +) { + const archiveLabel = isArchivedView ? 'Unarchive selected notes' : 'Archive selected notes' + const archiveIcon = isArchivedView ? : + const archiveHandler = isArchivedView ? onUnarchive : onArchive -function renderArchivedActions(onUnarchive: (() => void) | undefined, onDelete: () => void) { return ( <> - - + + + + + {archiveIcon} + + + + ) } -function renderDefaultActions(onArchive: () => void, onDelete: () => void) { - return ( - <> - - - - ) -} - -function BulkActionBarInner({ count, isArchivedView, onArchive, onDelete, onUnarchive, onClear }: BulkActionBarProps) { +function BulkActionBarInner({ count, isArchivedView, onOrganize, onArchive, onDelete, onUnarchive, onClear }: BulkActionBarProps) { return (
{count} selected -
- {isArchivedView ? renderArchivedActions(onUnarchive, onDelete) - : renderDefaultActions(onArchive, onDelete)} - + +
) diff --git a/src/components/NoteList.behavior.test.tsx b/src/components/NoteList.behavior.test.tsx index 56943098..9927fa5e 100644 --- a/src/components/NoteList.behavior.test.tsx +++ b/src/components/NoteList.behavior.test.tsx @@ -150,6 +150,7 @@ describe('NoteList multi-select', () => { }) it.each([ + { label: 'organizes via button', prop: 'onBulkOrganize', trigger: () => fireEvent.click(screen.getByTestId('bulk-organize-btn')) }, { label: 'archives via button', prop: 'onBulkArchive', trigger: () => fireEvent.click(screen.getByTestId('bulk-archive-btn')) }, { label: 'deletes via button', prop: 'onBulkDeletePermanently', trigger: () => fireEvent.click(screen.getByTestId('bulk-delete-btn')) }, { label: 'archives via Cmd+E', prop: 'onBulkArchive', trigger: () => fireEvent.keyDown(window, { key: 'e', metaKey: true }) }, diff --git a/src/components/NoteList.tsx b/src/components/NoteList.tsx index b183e340..c812ca15 100644 --- a/src/components/NoteList.tsx +++ b/src/components/NoteList.tsx @@ -1,10 +1,20 @@ -import { memo } from 'react' +import { memo, useCallback } from 'react' import { NoteListLayout } from './note-list/NoteListLayout' import { useNoteListModel, type NoteListProps } from './note-list/useNoteListModel' -function NoteListInner(props: NoteListProps) { +type NoteListInnerProps = NoteListProps & { + onBulkOrganize?: (paths: string[]) => void +} + +function NoteListInner({ onBulkOrganize, ...props }: NoteListInnerProps) { const model = useNoteListModel(props) - return + const handleBulkOrganize = useCallback(() => { + const paths = [...model.multiSelect.selectedPaths] + model.multiSelect.clear() + onBulkOrganize?.(paths) + }, [model.multiSelect, onBulkOrganize]) + + return } export const NoteList = memo(NoteListInner) diff --git a/src/components/note-list/NoteListLayout.tsx b/src/components/note-list/NoteListLayout.tsx index 97e8368f..a4bad06a 100644 --- a/src/components/note-list/NoteListLayout.tsx +++ b/src/components/note-list/NoteListLayout.tsx @@ -4,7 +4,32 @@ import { NoteListHeader } from './NoteListHeader' import { EntityView, ListView } from './NoteListViews' import type { useNoteListModel } from './useNoteListModel' -type NoteListLayoutProps = ReturnType +type NoteListLayoutProps = ReturnType & { + handleBulkOrganize?: () => void +} + +function MultiSelectBar({ + multiSelect, + isArchivedView, + handleBulkOrganize, + handleBulkArchive, + handleBulkDeletePermanently, + handleBulkUnarchive, +}: Pick) { + if (!multiSelect.isMultiSelecting) return null + + return ( + + ) +} export function NoteListLayout({ title, @@ -43,6 +68,7 @@ export function NoteListLayout({ filterCounts, onNoteListFilterChange, multiSelect, + handleBulkOrganize, handleBulkArchive, handleBulkDeletePermanently, handleBulkUnarchive, @@ -115,16 +141,14 @@ export function NoteListLayout({ /> )} - {multiSelect.isMultiSelecting && ( - - )} + {contextMenuNode} {dialogNode} diff --git a/src/hooks/useBulkActions.test.ts b/src/hooks/useBulkActions.test.ts index 374e88dc..34b275b2 100644 --- a/src/hooks/useBulkActions.test.ts +++ b/src/hooks/useBulkActions.test.ts @@ -1,52 +1,79 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { renderHook, act } from '@testing-library/react' import { useBulkActions } from './useBulkActions' +import { makeEntry } from '../test-utils/noteListTestUtils' describe('useBulkActions', () => { + const paths = { + a: '/vault/a.md', + b: '/vault/b.md', + c: '/vault/c.md', + } as const + let handleArchiveNote: ReturnType + let handleToggleOrganized: ReturnType let setToastMessage: ReturnType beforeEach(() => { handleArchiveNote = vi.fn().mockResolvedValue(undefined) + handleToggleOrganized = vi.fn().mockResolvedValue(undefined) setToastMessage = vi.fn() }) - function renderBulkActions() { + function renderBulkActions(organizedPaths: string[] = []) { + const entries = [ + makeEntry({ path: paths.a, organized: organizedPaths.includes(paths.a) }), + makeEntry({ path: paths.b, organized: organizedPaths.includes(paths.b) }), + makeEntry({ path: paths.c, organized: organizedPaths.includes(paths.c) }), + ] + return renderHook(() => useBulkActions( - { handleArchiveNote }, + { handleArchiveNote, handleToggleOrganized }, + entries, setToastMessage, ), ) } + async function runAction( + action: 'handleBulkArchive' | 'handleBulkOrganize', + selectedPaths: string[], + organizedPaths: string[] = [], + ) { + const { result } = renderBulkActions(organizedPaths) + + await act(async () => { + await result.current[action](selectedPaths) + }) + } + + function expectSuccessfulCalls(handler: ReturnType, successfulPaths: string[]) { + expect(handler).toHaveBeenCalledTimes(successfulPaths.length) + for (const path of successfulPaths) { + expect(handler).toHaveBeenCalledWith(path) + } + } + // --- handleBulkArchive --- describe('handleBulkArchive', () => { it('archives each path and shows plural toast for multiple notes', async () => { - const { result } = renderBulkActions() - await act(async () => { - await result.current.handleBulkArchive(['/vault/a.md', '/vault/b.md']) - }) - expect(handleArchiveNote).toHaveBeenCalledTimes(2) - expect(handleArchiveNote).toHaveBeenCalledWith('/vault/a.md') - expect(handleArchiveNote).toHaveBeenCalledWith('/vault/b.md') + await runAction('handleBulkArchive', [paths.a, paths.b]) + + expectSuccessfulCalls(handleArchiveNote, [paths.a, paths.b]) expect(setToastMessage).toHaveBeenCalledWith('2 notes archived') }) it('shows singular toast when one note archived', async () => { - const { result } = renderBulkActions() - await act(async () => { - await result.current.handleBulkArchive(['/vault/a.md']) - }) + await runAction('handleBulkArchive', [paths.a]) + expect(setToastMessage).toHaveBeenCalledWith('1 note archived') }) it('does not show toast when empty array given', async () => { - const { result } = renderBulkActions() - await act(async () => { - await result.current.handleBulkArchive([]) - }) + await runAction('handleBulkArchive', []) + expect(handleArchiveNote).not.toHaveBeenCalled() expect(setToastMessage).not.toHaveBeenCalled() }) @@ -56,21 +83,46 @@ describe('useBulkActions', () => { .mockResolvedValueOnce(undefined) // /vault/a.md succeeds .mockRejectedValueOnce(new Error('fail')) // /vault/b.md fails .mockResolvedValueOnce(undefined) // /vault/c.md succeeds - const { result } = renderBulkActions() - await act(async () => { - await result.current.handleBulkArchive(['/vault/a.md', '/vault/b.md', '/vault/c.md']) - }) + + await runAction('handleBulkArchive', [paths.a, paths.b, paths.c]) + expect(handleArchiveNote).toHaveBeenCalledTimes(3) expect(setToastMessage).toHaveBeenCalledWith('2 notes archived') }) it('shows no toast when all paths fail', async () => { handleArchiveNote.mockRejectedValue(new Error('fail')) - const { result } = renderBulkActions() - await act(async () => { - await result.current.handleBulkArchive(['/vault/a.md', '/vault/b.md']) - }) + + await runAction('handleBulkArchive', [paths.a, paths.b]) + expect(setToastMessage).not.toHaveBeenCalled() }) }) + + describe('handleBulkOrganize', () => { + it('organizes only notes that are not already organized', async () => { + await runAction('handleBulkOrganize', [paths.a, paths.b, paths.c], [paths.b]) + + expectSuccessfulCalls(handleToggleOrganized, [paths.a, paths.c]) + expect(setToastMessage).toHaveBeenCalledWith('2 notes organized') + }) + + it('shows no toast when all selected notes are already organized', async () => { + await runAction('handleBulkOrganize', [paths.a, paths.b], [paths.a, paths.b]) + + expect(handleToggleOrganized).not.toHaveBeenCalled() + expect(setToastMessage).not.toHaveBeenCalled() + }) + + it('skips failed organize actions and reports only successes', async () => { + handleToggleOrganized + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('fail')) + + await runAction('handleBulkOrganize', [paths.a, paths.b]) + + expect(handleToggleOrganized).toHaveBeenCalledTimes(2) + expect(setToastMessage).toHaveBeenCalledWith('1 note organized') + }) + }) }) diff --git a/src/hooks/useBulkActions.ts b/src/hooks/useBulkActions.ts index 8ed10585..74befe0f 100644 --- a/src/hooks/useBulkActions.ts +++ b/src/hooks/useBulkActions.ts @@ -1,21 +1,53 @@ -import { useCallback } from 'react' +import { useCallback, useMemo } from 'react' +import type { VaultEntry } from '../types' interface BulkEntryActions { handleArchiveNote: (path: string) => Promise + handleToggleOrganized: (path: string) => Promise +} + +function formatBulkToast(count: number, label: string) { + return `${count} note${count > 1 ? 's' : ''} ${label}` +} + +async function runBulkAction(paths: string[], action: (path: string) => Promise) { + let ok = 0 + for (const path of paths) { + try { + if (await action(path)) ok++ + } catch { + // Error toast already shown by the underlying action. + } + } + return ok } export function useBulkActions( entryActions: BulkEntryActions, + entries: VaultEntry[], setToastMessage: (msg: string | null) => void, ) { + const organizedPathSet = useMemo( + () => new Set(entries.filter((entry) => entry.organized).map((entry) => entry.path)), + [entries], + ) + const handleBulkArchive = useCallback(async (paths: string[]) => { - let ok = 0 - for (const path of paths) { - try { await entryActions.handleArchiveNote(path); ok++ } - catch { /* error toast already shown by flushBeforeAction */ } - } - if (ok > 0) setToastMessage(`${ok} note${ok > 1 ? 's' : ''} archived`) + const ok = await runBulkAction(paths, async (path) => { + await entryActions.handleArchiveNote(path) + return true + }) + if (ok > 0) setToastMessage(formatBulkToast(ok, 'archived')) }, [entryActions, setToastMessage]) - return { handleBulkArchive } + const handleBulkOrganize = useCallback(async (paths: string[]) => { + const ok = await runBulkAction(paths, async (path) => { + if (organizedPathSet.has(path)) return false + await entryActions.handleToggleOrganized(path) + return true + }) + if (ok > 0) setToastMessage(formatBulkToast(ok, 'organized')) + }, [entryActions, organizedPathSet, setToastMessage]) + + return { handleBulkArchive, handleBulkOrganize } }