diff --git a/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx b/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx index 326b05907b..bf4a497254 100644 --- a/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx +++ b/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx @@ -36,7 +36,7 @@ export interface TrackPageTileProps { onSelect: ( fileId: FileId, pageId: string, - modifiers: { shift: boolean; toggle: boolean }, + modifiers: { shift: boolean }, ) => void; onRotate: (pageIds: string[], delta: number) => void; onDelete: (pageIds: string[]) => void; @@ -81,10 +81,7 @@ function TrackPageTileImpl({ const handleClick = useCallback( (event: React.MouseEvent) => { - onSelect(trackFileId, page.id, { - shift: event.shiftKey, - toggle: event.metaKey || event.ctrlKey, - }); + onSelect(trackFileId, page.id, { shift: event.shiftKey }); }, [onSelect, trackFileId, page.id], ); @@ -120,7 +117,7 @@ function TrackPageTileImpl({ onKeyDown={(event) => { if (event.key === "Enter" || event.key === " ") { event.preventDefault(); - onSelect(trackFileId, page.id, { shift: false, toggle: true }); + onSelect(trackFileId, page.id, { shift: false }); } }} > @@ -130,9 +127,7 @@ function TrackPageTileImpl({ aria-label={t("pageTracks.selectPage", "Select page {{number}}", { number: position, })} - onChange={() => - onSelect(trackFileId, page.id, { shift: false, toggle: true }) - } + onChange={() => onSelect(trackFileId, page.id, { shift: false })} /> diff --git a/frontend/editor/src/core/components/pageTracks/hooks/useTrackSelection.ts b/frontend/editor/src/core/components/pageTracks/hooks/useTrackSelection.ts index 61fede8af8..bfa692b6cc 100644 --- a/frontend/editor/src/core/components/pageTracks/hooks/useTrackSelection.ts +++ b/frontend/editor/src/core/components/pageTracks/hooks/useTrackSelection.ts @@ -3,8 +3,8 @@ import { FileId } from "@app/types/file"; import { TrackWorkspace, allPages } from "@app/components/pageTracks/types"; export interface PageClickModifiers { + /** Extend the selection from the last clicked page in the same track. */ shift: boolean; - toggle: boolean; } export interface TrackSelectionHook { @@ -57,6 +57,12 @@ export function useTrackSelection( } }, [livePageIds]); + /** + * A click toggles the page in or out of the selection, so pages accumulate + * without a modifier: picking a set to rotate or move is the whole job here, + * and replace-on-click would make anything past the first page a fight. + * Shift extends from the last clicked page instead. + */ const selectPage = useCallback( (fileId: FileId, pageId: string, modifiers: PageClickModifiers) => { const trackPages = workspaceRef.current.tracks[fileId]?.pages ?? []; @@ -75,25 +81,18 @@ export function useTrackSelection( range.forEach((id) => next.add(id)); return next; }); + // Anchor stays put so repeated shift-clicks re-extend from it. return; } } anchorRef.current = { fileId, pageId }; - - if (modifiers.toggle) { - setSelectedIds((prev) => { - const next = new Set(prev); - if (next.has(pageId)) next.delete(pageId); - else next.add(pageId); - return next; - }); - return; - } - - setSelectedIds((prev) => - prev.size === 1 && prev.has(pageId) ? new Set() : new Set([pageId]), - ); + setSelectedIds((prev) => { + const next = new Set(prev); + if (next.has(pageId)) next.delete(pageId); + else next.add(pageId); + return next; + }); }, [], ); diff --git a/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts b/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts index e4315d900c..884b9d378b 100644 --- a/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts @@ -311,4 +311,53 @@ test.describe("Page Editor tracks", () => { await page.mouse.up(); expect(await readRotations(rotated, 4)).toEqual([0, 90, 180, 270]); }); + + test("clicking pages accumulates the selection instead of replacing it", async ({ + page, + }) => { + await openPageEditor(page); + const rotated = track(page, "rotated-pages.pdf"); + const tiles = rotated.locator("[data-page-id]"); + await expect(tiles).toHaveCount(4, { timeout: 30_000 }); + const selected = rotated.locator('[data-page-id][data-selected="true"]'); + + await tiles.nth(0).click(); + await expect(selected).toHaveCount(1); + + // The second click must ADD, not move the selection onto page 2. + await tiles.nth(1).click(); + await expect(selected).toHaveCount(2); + + // Clicking a selected page takes it back out again. + await tiles.nth(1).click(); + await expect(selected).toHaveCount(1); + await expect(tiles.nth(0)).toHaveAttribute("data-selected", "true"); + + // Shift extends from the last clicked page across the whole run. + await tiles.nth(3).click({ modifiers: ["Shift"] }); + await expect(selected).toHaveCount(4); + + // A selection spanning tracks is allowed too. + const sample = track(page, "sample.pdf"); + await sample.locator("[data-page-id]").first().click(); + await expect( + page.locator('[data-page-id][data-selected="true"]'), + ).toHaveCount(5); + }); + + test("a bar action applies to every page the clicks accumulated", async ({ + page, + }) => { + await openPageEditor(page); + const rotated = track(page, "rotated-pages.pdf"); + const tiles = rotated.locator("[data-page-id]"); + expect(await readRotations(rotated, 4)).toEqual([0, 90, 270, 180]); + + await tiles.nth(0).click(); + await tiles.nth(2).click(); + await page.getByRole("button", { name: "Rotate right" }).first().click(); + + // Only the two clicked pages turn; the ones in between are untouched. + expect(await readRotations(rotated, 4)).toEqual([90, 90, 0, 180]); + }); });