From a0acbc5625f051004aef6391730b2f62793c4c3d Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Fri, 21 Aug 2026 11:33:16 +0200 Subject: [PATCH] feat(worksheets): select several rows and drag them together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar could only ever move one worksheet at a time. Command- or control-click picks rows out, shift-click takes a range from the anchor, and grabbing any selected row now carries the whole group to the drop point — gathering a non-contiguous selection into a block that keeps its list order. With nothing picked out the open worksheet stands in for the selection, so the first modified click adds to the row you were on rather than silently dropping it. Modified clicks deliberately leave the editor alone: picking rows out to move them should not swap what you are looking at. The context menu follows the selection — "Delete 3 worksheets" behind one confirmation, Rename gone while it means nothing, and the same never-empty-the-list guard as a single row. A delete that only half works says how many of them failed instead of stranding the rest. `moveIds` and the selection math are pure and tested on their own; `useListReorder` takes the selection as an option, so the tab strip and the database explorer keep their single-row drag untouched. --- src/app/components/WorksheetExplorer.test.tsx | 253 ++++++++++++++++++ src/app/components/WorksheetExplorer.tsx | 207 +++++++++++--- src/app/hooks/use-list-reorder.test.ts | 133 ++++++++- src/app/hooks/use-list-reorder.ts | 67 +++-- src/app/hooks/use-worksheet-selection.ts | 79 ++++++ src/app/list-move.test.ts | 81 ++++++ src/app/list-move.ts | 45 ++++ src/app/list-selection.test.ts | 112 ++++++++ src/app/list-selection.ts | 107 ++++++++ src/app/store/editor-slice.test.ts | 48 +++- src/app/store/editor-slice.ts | 31 ++- src/app/test-utils.tsx | 3 +- 12 files changed, 1100 insertions(+), 66 deletions(-) create mode 100644 src/app/hooks/use-worksheet-selection.ts create mode 100644 src/app/list-move.test.ts create mode 100644 src/app/list-move.ts create mode 100644 src/app/list-selection.test.ts create mode 100644 src/app/list-selection.ts 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 (