diff --git a/src/app/components/WorksheetExplorer.test.tsx b/src/app/components/WorksheetExplorer.test.tsx index 33680d5..0267bcd 100644 --- a/src/app/components/WorksheetExplorer.test.tsx +++ b/src/app/components/WorksheetExplorer.test.tsx @@ -420,4 +420,257 @@ describe('WorksheetExplorer', () => { expect(await screen.findByDisplayValue('Second Worksheet')).toBeVisible() }) }) + describe('selecting several worksheets', () => { + // Explicit sort orders, so the rows sit in the order the tests name them: + // the list falls back to newest-first for worksheets without one. + const threeWorksheets: WorksheetDto[] = [ + { ...testWorksheet, sortOrder: 0 }, + { ...secondWorksheet, sortOrder: 1 }, + { ...testWorksheet, id: 'ws-789', name: 'Third Worksheet', sortOrder: 2 } + ] + + // What the sidebar acts on together, read off the rows rather than the + // store: the highlight is the only thing telling the user what a drag or a + // delete is about to take with it. + function selectedNames(): string[] { + return screen + .getAllByRole('button') + .filter((button) => button.dataset.selected === 'true') + .map((button) => button.textContent ?? '') + } + + it('adds a row to the selection on a command-click, without opening it', () => { + const { store } = renderWithProviders(, { + databases: [], + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.click( + screen.getByRole('button', { name: 'Second Worksheet' }), + { + metaKey: true + } + ) + + // The open worksheet joins the selection it started: a command-click adds + // a row, so it cannot be the thing that drops the row you were on. + expect({ + openWorksheetId: store.getState().tabs.activeWorksheetId, + selected: selectedNames() + }).toEqual({ + openWorksheetId: 'ws-123', + selected: ['Test Worksheet', 'Second Worksheet'] + }) + }) + + // `mod` is ⌘ on macOS and Ctrl everywhere else, and the click handler is + // the one place in the sidebar that has to know both. + it('adds a row on a control-click too', () => { + renderWithProviders(, { + databases: [], + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.click( + screen.getByRole('button', { name: 'Second Worksheet' }), + { + ctrlKey: true + } + ) + + expect(selectedNames()).toEqual(['Test Worksheet', 'Second Worksheet']) + }) + + it('takes a selected row back out on a second command-click', () => { + renderWithProviders(, { + databases: [], + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + const row = screen.getByRole('button', { name: 'Second Worksheet' }) + + fireEvent.click(row, { metaKey: true }) + fireEvent.click(row, { metaKey: true }) + + expect(selectedNames()).toEqual(['Test Worksheet']) + }) + + it('selects the range from the open row on a shift-click', () => { + const { store } = renderWithProviders(, { + databases: [], + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.click(screen.getByRole('button', { name: 'Third Worksheet' }), { + shiftKey: true + }) + + expect({ + openWorksheetId: store.getState().tabs.activeWorksheetId, + selected: selectedNames() + }).toEqual({ + openWorksheetId: 'ws-123', + selected: ['Test Worksheet', 'Second Worksheet', 'Third Worksheet'] + }) + }) + + it('drops the selection and opens the worksheet on a plain click', () => { + const { store } = renderWithProviders(, { + databases: [], + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.click(screen.getByRole('button', { name: 'Third Worksheet' }), { + shiftKey: true + }) + fireEvent.click(screen.getByRole('button', { name: 'Second Worksheet' })) + + expect({ + openWorksheetId: store.getState().tabs.activeWorksheetId, + selected: selectedNames() + }).toEqual({ + openWorksheetId: 'ws-456', + selected: ['Second Worksheet'] + }) + }) + + describe('deleting the selection', () => { + const selectingTwo = { + worksheetSelection: { anchorId: 'ws-123', ids: ['ws-123', 'ws-456'] } + } + + it('deletes every selected worksheet after one confirmation', async () => { + const user = userEvent.setup() + + vi.mocked(apiClient.deleteWorksheet).mockResolvedValue(undefined) + + renderWithProviders(, { + databases: [], + editor: selectingTwo, + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.contextMenu(screen.getByText('Second Worksheet')) + + await user.click( + await screen.findByRole('menuitem', { name: 'Delete 2 worksheets' }) + ) + + expect(await screen.findByText('Delete 2 worksheets?')).toBeVisible() + expect(apiClient.deleteWorksheet).not.toHaveBeenCalled() + + await user.click(screen.getByRole('button', { name: 'Delete' })) + + await waitFor(() => { + expect(vi.mocked(apiClient.deleteWorksheet).mock.calls).toEqual([ + ['ws-123'], + ['ws-456'] + ]) + }) + + expect(await screen.findByText('Deleted 2 worksheets')).toBeVisible() + }) + + // Renaming three worksheets at once means nothing, so the menu does not + // offer it rather than quietly renaming the one that was right-clicked. + it('offers no rename while several rows are selected', async () => { + renderWithProviders(, { + databases: [], + editor: selectingTwo, + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.contextMenu(screen.getByText('Second Worksheet')) + + await screen.findByRole('menuitem', { name: 'Delete 2 worksheets' }) + + expect( + screen.queryByRole('menuitem', { name: 'Rename' }) + ).not.toBeInTheDocument() + }) + + // Right-clicking outside the selection is how a file manager behaves: + // the row you pointed at becomes the selection, and the menu acts on it. + it('acts on the row alone when it is not part of the selection', async () => { + const user = userEvent.setup() + + vi.mocked(apiClient.deleteWorksheet).mockResolvedValue(undefined) + + renderWithProviders(, { + databases: [], + editor: selectingTwo, + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.contextMenu(screen.getByText('Third Worksheet')) + + await user.click( + await screen.findByRole('menuitem', { name: 'Delete' }) + ) + await user.click(await screen.findByRole('button', { name: 'Delete' })) + + await waitFor(() => { + expect(vi.mocked(apiClient.deleteWorksheet).mock.calls).toEqual([ + ['ws-789'] + ]) + }) + }) + + // Same rule as the single row: the app is built around always having a + // worksheet open, and the list endpoint would recreate a default one. + it('disables the delete that would empty the list', async () => { + renderWithProviders(, { + databases: [], + editor: selectingTwo, + openWorksheetId: 'ws-123', + worksheets: [testWorksheet, secondWorksheet] + }) + + fireEvent.contextMenu(screen.getByText('Second Worksheet')) + + expect( + await screen.findByRole('menuitem', { name: 'Delete 2 worksheets' }) + ).toHaveAttribute('aria-disabled', 'true') + }) + + it('says how many of them failed when only some are deleted', async () => { + const user = userEvent.setup() + + vi.mocked(apiClient.deleteWorksheet).mockImplementation( + async (worksheetId: string) => { + if (worksheetId === 'ws-456') { + throw new Error('Database is locked') + } + } + ) + + renderWithProviders(, { + databases: [], + editor: selectingTwo, + openWorksheetId: 'ws-123', + worksheets: threeWorksheets + }) + + fireEvent.contextMenu(screen.getByText('Second Worksheet')) + + await user.click( + await screen.findByRole('menuitem', { name: 'Delete 2 worksheets' }) + ) + await user.click(screen.getByRole('button', { name: 'Delete' })) + + expect( + await screen.findByText('Failed to delete 1 of 2 worksheets') + ).toBeVisible() + expect(await screen.findByText('Database is locked')).toBeVisible() + }) + }) + }) }) diff --git a/src/app/components/WorksheetExplorer.tsx b/src/app/components/WorksheetExplorer.tsx index 48bec36..b99175d 100644 --- a/src/app/components/WorksheetExplorer.tsx +++ b/src/app/components/WorksheetExplorer.tsx @@ -17,6 +17,7 @@ import { useOpenWorksheet } from '../hooks/use-worksheet-commands' import { useWorksheetRename } from '../hooks/use-worksheet-rename' +import { useWorksheetSelection } from '../hooks/use-worksheet-selection' import { cn } from '../lib/utils' import { useAppDispatch, useAppSelector } from '../store' import { worksheetSearchQueryUpdated } from '../store/editor-slice' @@ -32,34 +33,76 @@ import { SearchInput } from './SearchInput' import { WorksheetNameInput } from './WorksheetNameInput' import { WorksheetDto } from '@/glue/worksheets' +// One worksheet is named; several are counted. The count is what makes a +// multi-row delete safe to confirm — "Delete 3 worksheets?" is the only place +// the user learns the menu meant more than the row they right-clicked. +function describeWorksheets(worksheets: WorksheetDto[]): string { + return worksheets.length === 1 + ? `"${worksheets[0].name}"` + : `${worksheets.length} worksheets` +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : 'Unknown error' +} + // Deleting takes the editor content with it, so it asks first via an action // toast — ignoring it is a safe no. Mirrors the database explorer. -function useConfirmedWorksheetDeletion(): (worksheet: WorksheetDto) => void { +function useConfirmedWorksheetDeletion(): (worksheets: WorksheetDto[]) => void { const deleteWorksheet = useDeleteWorksheet() return useCallback( - (worksheet: WorksheetDto) => { - toast(`Delete "${worksheet.name}"?`, { + (worksheets: WorksheetDto[]) => { + if (worksheets.length === 0) { + return + } + + const described = describeWorksheets(worksheets) + + toast(`Delete ${described}?`, { action: { label: 'Delete', onClick: () => { - deleteWorksheet.mutate(worksheet.id, { - onError: (error) => { - const message = - error instanceof Error ? error.message : 'Unknown error' - - toast.error('Failed to delete worksheet', { - description: message - }) - }, - onSuccess: () => { - toast.success(`Deleted "${worksheet.name}"`) + // `allSettled`, so one row that will not go does not strand the + // others: every delete is attempted and the toast afterwards says + // how many made it. The selection needs no clearing — it is pruned + // to the rows that still exist on the next render. + const deletions = Promise.allSettled( + worksheets.map((worksheet) => + deleteWorksheet.mutateAsync(worksheet.id) + ) + ) + + void deletions.then((results) => { + const failures = results.filter( + (result) => result.status === 'rejected' + ) + + if (failures.length === 0) { + toast.success(`Deleted ${described}`) + + return } + + const description = errorMessage(failures[0].reason) + + if (worksheets.length === 1) { + toast.error('Failed to delete worksheet', { description }) + + return + } + + toast.error( + `Failed to delete ${failures.length} of ${worksheets.length} worksheets`, + { description } + ) }) } }, description: - 'This worksheet and its editor content will be removed. Query history is kept.' + worksheets.length === 1 + ? 'This worksheet and its editor content will be removed. Query history is kept.' + : 'These worksheets and their editor content will be removed. Query history is kept.' }) }, [deleteWorksheet] @@ -106,15 +149,65 @@ export function WorksheetExplorer(): ReactElement { const filteredWorksheetIds = filteredWorksheets.map( (worksheet) => worksheet.id ) + const worksheetIds = worksheets.data.map((worksheet) => worksheet.id) + + const selection = useWorksheetSelection(worksheetIds) + // The reorder runs over the whole list; the filtered rows on screen are a // subsequence of it, and asking for the indicator by id is what lets those // two lists differ without any index having to line up. - const { dndContextProps, dropIndicatorFor } = useListReorder({ + const { dndContextProps, dropIndicatorFor, isMoving } = useListReorder({ axis: 'vertical', - ids: worksheets.data.map((worksheet) => worksheet.id), - onReorder: reorderWorksheets.mutate + ids: worksheetIds, + onReorder: reorderWorksheets.mutate, + selectedIds: selection.ids }) + // Command- and shift-click pick rows out; a plain click is still what opens + // one. Modified clicks deliberately leave the open worksheet where it is — + // picking rows out to move them should not swap the editor under the user. + const handleRowClick = ( + event: React.MouseEvent, + worksheetId: string + ): void => { + // ⌘ on macOS, Ctrl everywhere else. + if (event.metaKey || event.ctrlKey) { + selection.toggle(worksheetId) + + return + } + + if (event.shiftKey) { + selection.extend(worksheetId) + + return + } + + selection.replace(worksheetId) + handleSelectWorksheet(worksheetId) + } + + // Right-clicking outside the selection makes that row the selection, the way + // a file manager does, so the menu can never act on rows the user is not + // pointing at. + const handleRowContextMenu = (worksheetId: string): void => { + if (!selection.isSelected(worksheetId)) { + selection.replace(worksheetId) + } + } + + const selectedWorksheets = worksheets.data.filter((worksheet) => + selection.isSelected(worksheet.id) + ) + + // A row inside a multi-row selection opens a menu about the whole selection; + // anything else is about itself, including the single row a selection has + // shrunk to. + const deleteTargetsFor = (worksheet: WorksheetDto): WorksheetDto[] => + selectedWorksheets.length > 1 && selection.isSelected(worksheet.id) + ? selectedWorksheets + : [worksheet] + // Worksheets show which database they run against, so the names are looked // up once per render instead of per row. const databaseNames = new Map( @@ -173,6 +266,7 @@ export function WorksheetExplorer(): ReactElement { ) : ( 1} databaseName={ worksheet.databaseId ? databaseNames.get(worksheet.databaseId) : undefined } + deleteTargets={deleteTargetsFor(worksheet)} isOpen={worksheet.id === openWorksheetId} + isSelected={selection.isSelected(worksheet.id)} + remainingWorksheetCount={worksheets.data.length} worksheet={worksheet} + onContextMenu={handleRowContextMenu} onDelete={handleDeleteWorksheet} onDoubleClick={startEditing} - onSelect={handleSelectWorksheet} + onSelect={handleRowClick} /> )} @@ -215,6 +312,13 @@ export function WorksheetExplorer(): ReactElement { interface WorksheetRowProps { children: ReactNode dropIndicator: DropIndicator + /** + * Whether this row is travelling with the drag. Not `useSortable`'s + * `isDragging`, which only knows about the row under the cursor: a selection + * dragged as a group carries rows the cursor never touched, and dimming all + * of them is the only thing that says what is being carried. + */ + isMoving: boolean isSortingDisabled: boolean worksheetId: string } @@ -227,16 +331,19 @@ interface WorksheetRowProps { function WorksheetRow({ children, dropIndicator, + isMoving, isSortingDisabled, worksheetId }: WorksheetRowProps): ReactElement { - const { isDragging, listeners, setNodeRef, transform, transition } = - useSortable({ disabled: isSortingDisabled, id: worksheetId }) + const { listeners, setNodeRef, transform, transition } = useSortable({ + disabled: isSortingDisabled, + id: worksheetId + }) return (
@@ -253,36 +360,52 @@ function WorksheetRow({ } interface WorksheetListItemProps { - // False for the last remaining worksheet: the app is built around always - // having one open, and the list endpoint would just recreate a default. - canDelete: boolean databaseName?: string + /** What this row's menu acts on: itself, or the selection it belongs to. */ + deleteTargets: WorksheetDto[] isOpen: boolean + isSelected: boolean + /** How many worksheets there are, which is what caps a delete. */ + remainingWorksheetCount: number worksheet: WorksheetDto - onDelete: (worksheet: WorksheetDto) => void + onContextMenu: (worksheetId: string) => void + onDelete: (worksheets: WorksheetDto[]) => void onDoubleClick: (worksheet: WorksheetDto) => void - onSelect: (worksheetId: string) => void + onSelect: (event: React.MouseEvent, worksheetId: string) => void } function WorksheetListItem({ - canDelete, databaseName, + deleteTargets, isOpen, + isSelected, + remainingWorksheetCount, worksheet, + onContextMenu, onDelete, onDoubleClick, onSelect }: WorksheetListItemProps): ReactElement { + // The app is built around always having a worksheet open, and the list + // endpoint would just recreate a default one. + const canDelete = remainingWorksheetCount > deleteTargets.length + + // Renaming several rows at once means nothing, so the menu drops the item + // rather than quietly renaming whichever one was right-clicked. + const isRenamable = deleteTargets.length === 1 + return (