From 374bd67dd852bec4c6f1e8f8e327e1abf9fd9f10 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 20 Aug 2026 15:42:56 -0700 Subject: [PATCH 1/3] Add tests for Files selection adoption Cover single, shift-range, and select-all/indeterminate selection on the Files table, the bulk action bar's appearance and Escape-clear, parity between the bulk action bar's and the shell context menu's operation sets, and that page actions live in the StageTopBar slot rather than the body. --- .../web/src/shell/context-menu/items.test.tsx | 43 ++++ .../src/shell/context-menu/targets.test.ts | 20 ++ apps/web/test/library-page-selection.test.tsx | 200 ++++++++++++++++++ 3 files changed, 263 insertions(+) create mode 100644 apps/web/test/library-page-selection.test.tsx diff --git a/apps/web/src/shell/context-menu/items.test.tsx b/apps/web/src/shell/context-menu/items.test.tsx index 6e5983d74..462b53cf8 100644 --- a/apps/web/src/shell/context-menu/items.test.tsx +++ b/apps/web/src/shell/context-menu/items.test.tsx @@ -8,6 +8,7 @@ const toastMock = spyOnReactUiToast(); import { shellContextMenuFor } from "./items"; import type { ShellContextMenuActions } from "./items"; import type { ShellContextMenuTarget } from "./targets"; +import { LIBRARY_BULK_OPERATION_IDS } from "../library-artifacts"; function itemIds(entries: readonly ContextMenuEntry[]): readonly string[] { return entries @@ -152,6 +153,48 @@ describe("shellContextMenuFor: insights-run", () => { }); }); +describe("shellContextMenuFor: artifact", () => { + test("a single right-clicked file offers exactly the Files bulk action bar's operation set", () => { + const target: ShellContextMenuTarget = { + type: "artifact", + id: "art_1", + ids: ["art_1"], + }; + const menu = shellContextMenuFor(target, actions()); + // Parity, not eyeballing: the context menu and the bulk action bar are + // driven off the exact same constant (CL-6423). + expect(itemIds(menu.entries)).toEqual([...LIBRARY_BULK_OPERATION_IDS]); + expect(findItem(menu.entries, "copy-link").label).toBe("Copy link"); + }); + + test("right-clicking inside a multi-select still offers the same operation set, pluralized", () => { + const target: ShellContextMenuTarget = { + type: "artifact", + id: "art_2", + ids: ["art_1", "art_2", "art_3"], + }; + const menu = shellContextMenuFor(target, actions()); + expect(itemIds(menu.entries)).toEqual([...LIBRARY_BULK_OPERATION_IDS]); + expect(findItem(menu.entries, "copy-link").label).toBe("Copy 3 links"); + }); + + test("copy-link writes every acted-on file's canonical link, newline-joined", async () => { + const target: ShellContextMenuTarget = { + type: "artifact", + id: "art_1", + ids: ["art_1", "art_2"], + }; + const menu = shellContextMenuFor(target, actions()); + findItem(menu.entries, "copy-link").onSelect(); + await Promise.resolve(); + await Promise.resolve(); + expect(navigator.clipboard.writeText).toHaveBeenCalledWith( + `${window.location.origin}/files/a/art_1\n${window.location.origin}/files/a/art_2`, + ); + expect(toastMock).toHaveBeenCalledWith("2 links copied"); + }); +}); + describe("shellContextMenuFor: account", () => { test("offers settings and sign-out, never a bare destructive gesture", () => { const menu = shellContextMenuFor({ type: "account" }, actions()); diff --git a/apps/web/src/shell/context-menu/targets.test.ts b/apps/web/src/shell/context-menu/targets.test.ts index bebfbffe3..507bf6b35 100644 --- a/apps/web/src/shell/context-menu/targets.test.ts +++ b/apps/web/src/shell/context-menu/targets.test.ts @@ -83,6 +83,26 @@ describe("SHELL_CONTEXT_MENU_TARGETS", () => { expect(resolve(container)).toEqual({ type: "account" }); }); + test("resolves an artifact row, defaulting ids to just its own id", () => { + const container = mount('
'); + expect(resolve(container)).toEqual({ + type: "artifact", + id: "art_1", + ids: ["art_1"], + }); + }); + + test("resolves an artifact row inside an active multi-select as every selected id", () => { + const container = mount( + '
', + ); + expect(resolve(container)).toEqual({ + type: "artifact", + id: "art_2", + ids: ["art_1", "art_2", "art_3"], + }); + }); + test("falls back to the shell target for anything unmatched", () => { const container = mount('
'); expect(resolve(container.querySelector("#plain"))).toEqual( diff --git a/apps/web/test/library-page-selection.test.tsx b/apps/web/test/library-page-selection.test.tsx new file mode 100644 index 000000000..d38e22619 --- /dev/null +++ b/apps/web/test/library-page-selection.test.tsx @@ -0,0 +1,200 @@ +// CL-6423: Files adopts @corbits/react-ui's selection system. These tests +// drive `LibraryPage` directly (it is a pure, uncontrolled-selection +// component outside `LibraryRoute`) so selection state, the bulk action +// bar, and the top-nav action placement can all be asserted without a +// network layer. + +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; + +import { spyOnReactUiToast } from "./react-ui-toast-mock"; +import { LibraryPage } from "../src/pages/library-page"; +import { LIBRARY_BULK_OPERATION_IDS } from "../src/shell/library-artifacts"; + +const toastMock = spyOnReactUiToast(); + +const artifacts = [ + { + id: "art_1", + title: "Alpha", + kind: "document", + ownerName: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }, + { + id: "art_2", + title: "Bravo", + kind: "document", + ownerName: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }, + { + id: "art_3", + title: "Charlie", + kind: "document", + ownerName: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }, +]; + +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + Object.defineProperty(navigator, "clipboard", { + configurable: true, + value: { writeText: mock(() => Promise.resolve()) }, + }); + toastMock.mockClear(); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); +}); + +function render() { + act(() => { + root.render(); + }); +} + +function checkboxFor(title: string): HTMLButtonElement { + const row = [...container.querySelectorAll("tbody tr")].find((candidate) => + candidate.textContent?.includes(title), + ); + if (row === undefined) throw new Error(`no row for "${title}"`); + const button = row.querySelector('button[role="checkbox"]'); + if (button === null) throw new Error(`no checkbox for "${title}"`); + return button as HTMLButtonElement; +} + +function headerCheckbox(): HTMLButtonElement { + const button = container.querySelector('thead button[role="checkbox"]'); + if (button === null) throw new Error("no header checkbox"); + return button as HTMLButtonElement; +} + +function click( + button: HTMLButtonElement, + modifiers: { shiftKey?: boolean } = {}, +) { + act(() => { + button.dispatchEvent( + new MouseEvent("click", { + bubbles: true, + shiftKey: modifiers.shiftKey ?? false, + }), + ); + }); +} + +function bulkActionBar(): Element | null { + return container.querySelector('[role="group"][aria-label="Bulk actions"]'); +} + +describe("LibraryPage selection", () => { + test("clicking a row's checkbox selects just that row", () => { + render(); + click(checkboxFor("Alpha")); + expect(checkboxFor("Alpha").getAttribute("aria-checked")).toBe("true"); + expect(checkboxFor("Bravo").getAttribute("aria-checked")).toBe("false"); + }); + + test("shift-click ranges from the last plain toggle through the clicked row", () => { + render(); + click(checkboxFor("Alpha")); + click(checkboxFor("Charlie"), { shiftKey: true }); + expect(checkboxFor("Alpha").getAttribute("aria-checked")).toBe("true"); + expect(checkboxFor("Bravo").getAttribute("aria-checked")).toBe("true"); + expect(checkboxFor("Charlie").getAttribute("aria-checked")).toBe("true"); + }); + + test("the header checkbox is indeterminate with a partial selection and selects all on click", () => { + render(); + click(checkboxFor("Alpha")); + expect(headerCheckbox().getAttribute("aria-checked")).toBe("mixed"); + + click(headerCheckbox()); + expect(checkboxFor("Alpha").getAttribute("aria-checked")).toBe("true"); + expect(checkboxFor("Bravo").getAttribute("aria-checked")).toBe("true"); + expect(checkboxFor("Charlie").getAttribute("aria-checked")).toBe("true"); + expect(headerCheckbox().getAttribute("aria-checked")).toBe("true"); + }); + + test("clicking the fully-selected header checkbox again clears the selection", () => { + render(); + click(headerCheckbox()); + click(headerCheckbox()); + expect(bulkActionBar()).toBeNull(); + }); + + test("the bulk action bar appears once something is selected, and clears with the selection", () => { + render(); + expect(bulkActionBar()).toBeNull(); + + click(checkboxFor("Alpha")); + const bar = bulkActionBar(); + expect(bar).not.toBeNull(); + expect(bar?.textContent).toContain("1 selected"); + + act(() => { + window.dispatchEvent( + new KeyboardEvent("keydown", { key: "Escape", bubbles: true }), + ); + }); + expect(bulkActionBar()).toBeNull(); + }); + + test("the bulk action bar's action list is exactly the real, already-implemented operations", () => { + render(); + click(checkboxFor("Alpha")); + click(checkboxFor("Bravo"), { shiftKey: false }); + const ids = [...container.querySelectorAll("[data-bulk-action]")].map( + (node) => node.getAttribute("data-bulk-action"), + ); + expect(ids).toEqual([...LIBRARY_BULK_OPERATION_IDS]); + }); + + test("the bulk copy-link action copies every selected file's canonical link", async () => { + render(); + click(checkboxFor("Alpha")); + click(checkboxFor("Bravo"), { shiftKey: true }); + const button = container.querySelector( + '[data-bulk-action="copy-link"]', + ) as HTMLButtonElement; + act(() => button.click()); + await Promise.resolve(); + await Promise.resolve(); + expect(navigator.clipboard.writeText).toHaveBeenCalledWith( + `${window.location.origin}/files/a/art_1\n${window.location.origin}/files/a/art_2`, + ); + expect(toastMock).toHaveBeenCalledWith("2 links copied"); + }); +}); + +describe("LibraryPage top-nav action placement", () => { + test("primary page actions render inside the StageTopBar action slot, not the page body", () => { + act(() => { + root.render( + undefined} />, + ); + }); + const topBarActions = container.querySelector( + '[data-testid="stage-top-bar-actions"]', + ); + expect(topBarActions).not.toBeNull(); + expect(topBarActions?.textContent).toContain("Upload"); + + const clone = container.cloneNode(true) as HTMLElement; + clone.querySelector('[data-testid="stage-top-bar"]')?.remove(); + expect(clone.textContent).not.toContain("Upload"); + }); +}); From 5c49437a2cfd76561e7f0960b70d444f76ef7475 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 20 Aug 2026 15:43:11 -0700 Subject: [PATCH 2/3] Files: adopt the shared selection system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Files table becomes the reference adopter of @corbits/react-ui's selection system: a hover-reveal SelectionCheckbox column with an indeterminate select-all header, useListSelection for shift-click range and cmd/ctrl-click toggle selection, and a BulkActionBar carrying the one operation the artifacts surface actually supports today — copying files' canonical /files/a/:id links. The shell context menu gains an artifact target driven off the same operation constant, so a right-click on a row (or inside a multi-select) offers exactly the bulk bar's operation set. --- apps/web/src/pages/library-page.tsx | 145 +++++++++++++++++---- apps/web/src/shell/context-menu/items.tsx | 33 +++++ apps/web/src/shell/context-menu/targets.ts | 26 +++- apps/web/src/shell/library-artifacts.ts | 34 +++++ 4 files changed, 212 insertions(+), 26 deletions(-) diff --git a/apps/web/src/pages/library-page.tsx b/apps/web/src/pages/library-page.tsx index 7887a5bc2..3fac9fd1e 100644 --- a/apps/web/src/pages/library-page.tsx +++ b/apps/web/src/pages/library-page.tsx @@ -1,4 +1,5 @@ import { + BulkActionBar, Button, LibrarySearchInput, Menu, @@ -7,6 +8,7 @@ import { MenuTrigger, PageShell, RichEmptyState, + SelectionCheckbox, Skeleton, Table, TableBody, @@ -18,8 +20,13 @@ import { artifactKindLabel, formatRelativeTime, toast, + useListSelection, +} from "@corbits/react-ui"; +import type { + SelectionCheckboxState, + UseListSelectionResult, + ViewMode, } from "@corbits/react-ui"; -import type { ViewMode } from "@corbits/react-ui"; import { ArtifactCard, ArtifactRenderer, @@ -33,8 +40,15 @@ import { } from "@corbits/artifact-ui"; import type { ArtifactSort, ArtifactSummary } from "@corbits/artifact-ui"; import { useQueryClient } from "@tanstack/react-query"; -import { ArrowsDownUp, ArrowSquareOut, Stack, X } from "@corbits/icons"; +import { + ArrowsDownUp, + ArrowSquareOut, + LinkSimple as LinkIcon, + Stack, + X, +} from "@corbits/icons"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import type { MouseEvent as ReactMouseEvent } from "react"; import { describeApiError, ListSkeleton, @@ -49,7 +63,7 @@ import { useAPIQuery, type ArtifactDetail, } from "../api"; -import { rowActivationProps } from "../activatable-row"; +import { isRowActivationKey } from "../activatable-row"; import { useBench } from "../bench-context"; import { readLastWorkbenchId } from "../last-workbench"; import { @@ -63,7 +77,10 @@ import { tenantKeys } from "../query-client"; import { useBenchActivity } from "../shell/bench-activity"; import { artifactUploadToast, + copyArtifactLinks, + copyArtifactLinksToastLabel, isArtifactsUnavailableStatus, + LIBRARY_BULK_OPERATION_IDS, mapArtifactListToSummaries, uploadArtifactFiles, } from "../shell/library-artifacts"; @@ -79,16 +96,38 @@ function ArtifactRows({ now, selectedId, onSelect, + selection, }: { readonly artifacts: readonly ArtifactSummary[]; readonly now: number | undefined; readonly selectedId: string | null; readonly onSelect: (id: string) => void; + readonly selection: UseListSelectionResult; }) { + const allSelected = + artifacts.length > 0 && selection.selectedCount === artifacts.length; + const headerChecked: SelectionCheckboxState = + selection.selectedCount === 0 + ? false + : allSelected + ? true + : "indeterminate"; + return ( + + + allSelected ? selection.clear() : selection.selectAll() + } + rowLabel="all files" + ariaLabel="Select all files" + className="opacity-100" + /> + Title Kind Owner @@ -96,28 +135,59 @@ function ArtifactRows({ - {artifacts.map((artifact) => ( - onSelect(artifact.id))} - > - {artifact.title} - - {artifactKindLabel(artifact.kind)} - - - {artifact.ownerName ?? "—"} - - - {formatRelativeTime( - artifact.updatedAt ?? artifact.createdAt, - now, - )} - - - ))} + {artifacts.map((artifact) => { + const isSelected = selection.isSelected(artifact.id); + const selectionIds = + isSelected && selection.selectedCount > 1 + ? [...selection.selectedIds] + : [artifact.id]; + return ( + { + if (event.shiftKey || event.metaKey || event.ctrlKey) { + selection.toggle(artifact.id, { shiftKey: event.shiftKey }); + return; + } + onSelect(artifact.id); + }} + onKeyDown={(event) => { + if (!isRowActivationKey(event.key)) return; + event.preventDefault(); + onSelect(artifact.id); + }} + > + event.stopPropagation()}> + + selection.toggle(artifact.id, modifiers) + } + rowLabel={artifact.title} + /> + + {artifact.title} + + {artifactKindLabel(artifact.kind)} + + + {artifact.ownerName ?? "—"} + + + {formatRelativeTime( + artifact.updatedAt ?? artifact.createdAt, + now, + )} + + + ); + })}
); @@ -297,6 +367,12 @@ export function LibraryPage({ [artifacts, activeQuery, sort, onQueryChange], ); + const visibleIds = useMemo( + () => visible.map((artifact) => artifact.id), + [visible], + ); + const selection = useListSelection({ ids: visibleIds }); + const openPicker = useCallback(() => { if (uploading === true) return; fileInputRef.current?.click(); @@ -447,6 +523,7 @@ export function LibraryPage({ now={now} selectedId={activeSelected} onSelect={(id) => select(id)} + selection={selection} /> ) : ( @@ -479,6 +556,24 @@ export function LibraryPage({ ) : null} + + + ); } diff --git a/apps/web/src/shell/context-menu/items.tsx b/apps/web/src/shell/context-menu/items.tsx index 78c808d52..e6063442a 100644 --- a/apps/web/src/shell/context-menu/items.tsx +++ b/apps/web/src/shell/context-menu/items.tsx @@ -26,6 +26,10 @@ import { } from "@corbits/icons"; import { toast } from "@corbits/react-ui"; +import { + copyArtifactLinks, + copyArtifactLinksToastLabel, +} from "../library-artifacts"; import { workbenchPath } from "../../workbench-path"; import { requestWorkbenchRename } from "../../workbench-rename-events"; import { requestOpenCommandPalette } from "../../command-palette-events"; @@ -179,6 +183,33 @@ function insightsRunMenu( }; } +/** + * Same operation set the Files bulk action bar offers (CL-6423): copy + * every acted-on file's canonical link. `target.ids` is either the single + * right-clicked row, or the whole active selection when the row is part of + * one — see `SHELL_CONTEXT_MENU_TARGETS`. + */ +function artifactMenu( + target: Extract, +): ContextMenu { + const count = target.ids.length; + return { + entries: [ + contextMenuItem({ + id: "copy-link", + label: count > 1 ? `Copy ${count} links` : "Copy link", + icon: , + onSelect: () => { + void copyArtifactLinks(target.ids).then( + () => toast(copyArtifactLinksToastLabel(count)), + () => toast("Couldn't copy the link"), + ); + }, + }), + ], + }; +} + function accountMenu(actions: ShellContextMenuActions): ContextMenu { return { label: "Account", @@ -240,6 +271,8 @@ export function shellContextMenuFor( return routineMenu(target, actions); case "insights-run": return insightsRunMenu(target, actions); + case "artifact": + return artifactMenu(target); case "account": return accountMenu(actions); case "shell": diff --git a/apps/web/src/shell/context-menu/targets.ts b/apps/web/src/shell/context-menu/targets.ts index 9e3fb601b..359b95fdd 100644 --- a/apps/web/src/shell/context-menu/targets.ts +++ b/apps/web/src/shell/context-menu/targets.ts @@ -19,7 +19,18 @@ export type ShellContextMenuTarget = readonly handle: string; } | { readonly type: "routine"; readonly id: string; readonly name: string } - | { readonly type: "insights-run"; readonly id: string }; + | { readonly type: "insights-run"; readonly id: string } + | { + readonly type: "artifact"; + readonly id: string; + /** + * The ids this menu acts on: the row's own id alone, or — when the + * right-clicked row is part of a multi-row selection — every + * selected id, so a right-click inside an active selection offers + * the exact same operation set as the bulk action bar (CL-6423). + */ + readonly ids: readonly string[]; + }; function attr(element: Element, name: string): string | null { const value = element.getAttribute(name); @@ -78,6 +89,19 @@ export const SHELL_CONTEXT_MENU_TARGETS: readonly TargetDefinition { + const id = attr(element, "data-ctx-artifact"); + if (id === null) return null; + const selectedIds = attr(element, "data-ctx-artifact-selected-ids"); + const ids = + selectedIds === null + ? [id] + : selectedIds.split(",").filter((candidate) => candidate !== ""); + return { type: "artifact", id, ids: ids.length > 0 ? ids : [id] }; + }, + }, ]; export const SHELL_CONTEXT_MENU_FALLBACK: ShellContextMenuTarget = { diff --git a/apps/web/src/shell/library-artifacts.ts b/apps/web/src/shell/library-artifacts.ts index 69ec2d124..5ff4a5b9d 100644 --- a/apps/web/src/shell/library-artifacts.ts +++ b/apps/web/src/shell/library-artifacts.ts @@ -10,6 +10,8 @@ import type { ArtifactSummary } from "@corbits/artifact-ui"; import { ApiQueryError, UnauthenticatedError } from "@corbits/api-query"; +import { FILES_PATH_PREFIX } from "../path-ids"; + /** List row from the hub artifacts surface (content omitted). */ export type ArtifactListRow = { readonly id: string; @@ -110,3 +112,35 @@ export function artifactUploadToast(names: readonly string[]): string { ? `Uploaded · ${only}` : `Uploaded ${names.length} files`; } + +/** + * The bulk/context-menu operation set this file adopts from the shared + * selection system (CL-6423) — deliberately just the one real, already- + * shippable operation: every other candidate (delete, move, rename, + * download) has no backend route or store method behind it yet (see + * `packages/artifacts-hub/src/routes.ts` and `@corbits/artifacts`' + * `ArtifactStore`), so wiring a button for any of them would be exactly the + * dead/no-op control this adoption is required to avoid. `BulkActionBar` + * and the shell context menu's `artifact` target both read this same + * constant, which is what the parity test asserts against. + */ +export const LIBRARY_BULK_OPERATION_IDS = ["copy-link"] as const; + +/** `/files/a/:id` (CL-6015) — the one canonical deep link a file has. */ +export function libraryArtifactDeepLink(id: string): string { + return `${FILES_PATH_PREFIX}/a/${encodeURIComponent(id)}`; +} + +/** Copies one or more files' canonical links, newline-joined, to the + * clipboard — the same `copyLink` idiom already used for workbenches, + * routines, and insight runs, extended to a whole selection. */ +export async function copyArtifactLinks(ids: readonly string[]): Promise { + const urls = ids.map( + (id) => `${window.location.origin}${libraryArtifactDeepLink(id)}`, + ); + await navigator.clipboard.writeText(urls.join("\n")); +} + +export function copyArtifactLinksToastLabel(count: number): string { + return count === 1 ? "Link copied" : `${count} links copied`; +} From 2dab4936cfc91b1e905e8303a044b1f0678ccdde Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 20 Aug 2026 16:34:07 -0700 Subject: [PATCH 3/3] Files: review fixes for the shared selection adoption - Additive multi-select is cmd-click on Mac (ctrl-click stays the context-menu gesture there); ctrl-click still adds on other platforms, via a shared isAdditiveSelectClick helper. - Bulk copy-link and the context menu's selected-ids now emit in visible row order, so a bottom-up selection no longer copies links out of order. - Switching between rows and grid views clears the selection, so the bulk bar can't float over a view with no checkboxes. - The copy-link action label is one count-aware helper shared by the bulk bar and the context menu. --- apps/web/src/activatable-row.test.ts | 25 ++++++- apps/web/src/activatable-row.ts | 24 +++++++ apps/web/src/pages/library-page.tsx | 38 +++++++++-- apps/web/src/shell/context-menu/items.tsx | 3 +- apps/web/src/shell/library-artifacts.test.ts | 18 ++++- apps/web/src/shell/library-artifacts.ts | 6 ++ apps/web/test/library-page-selection.test.tsx | 67 ++++++++++++++++++- 7 files changed, 172 insertions(+), 9 deletions(-) diff --git a/apps/web/src/activatable-row.test.ts b/apps/web/src/activatable-row.test.ts index 335283aec..47aabf4e7 100644 --- a/apps/web/src/activatable-row.test.ts +++ b/apps/web/src/activatable-row.test.ts @@ -1,6 +1,29 @@ import { describe, expect, test } from "bun:test"; -import { isRowActivationKey, rowActivationProps } from "./activatable-row"; +import { + isAdditiveSelectClick, + isRowActivationKey, + rowActivationProps, +} from "./activatable-row"; + +describe("isAdditiveSelectClick", () => { + // The test DOM reports a Darwin platform, so these exercise the Mac rules. + test("cmd-click is additive", () => { + expect(isAdditiveSelectClick({ metaKey: true, ctrlKey: false })).toBe(true); + }); + + test("ctrl-click is not additive on Mac (it's the context-menu gesture)", () => { + expect(isAdditiveSelectClick({ metaKey: false, ctrlKey: true })).toBe( + false, + ); + }); + + test("a plain click is not additive", () => { + expect(isAdditiveSelectClick({ metaKey: false, ctrlKey: false })).toBe( + false, + ); + }); +}); describe("isRowActivationKey", () => { test("Enter and Space activate", () => { diff --git a/apps/web/src/activatable-row.ts b/apps/web/src/activatable-row.ts index 6ee5e33fe..a896ad127 100644 --- a/apps/web/src/activatable-row.ts +++ b/apps/web/src/activatable-row.ts @@ -8,6 +8,30 @@ export function isRowActivationKey(key: string): boolean { return key === "Enter" || key === " "; } +/** + * Whether a click's modifiers mean "add this row to the selection" rather + * than "activate/replace". Cmd-click is the additive gesture on every + * platform; Ctrl-click only joins in on non-Mac, because on Mac Ctrl-click + * is the native context-menu gesture — the browser can fire `click` and + * `contextmenu` from the same physical click, and treating Ctrl as additive + * there would silently toggle the very row the context menu is about to + * act on. + */ +export function isAdditiveSelectClick(event: { + readonly metaKey: boolean; + readonly ctrlKey: boolean; +}): boolean { + return event.metaKey || (!isMacPlatform() && event.ctrlKey); +} + +function isMacPlatform(): boolean { + // Browsers report "MacIntel"; happy-dom (our test DOM) reports + // "X11; Darwin arm64" — both are the same Ctrl-click-is-context-menu OS. + return ( + typeof navigator !== "undefined" && /mac|darwin/i.test(navigator.platform) + ); +} + export function rowActivationProps(onSelect: () => void) { return { role: "button" as const, diff --git a/apps/web/src/pages/library-page.tsx b/apps/web/src/pages/library-page.tsx index 3fac9fd1e..a12450e91 100644 --- a/apps/web/src/pages/library-page.tsx +++ b/apps/web/src/pages/library-page.tsx @@ -63,7 +63,7 @@ import { useAPIQuery, type ArtifactDetail, } from "../api"; -import { isRowActivationKey } from "../activatable-row"; +import { isAdditiveSelectClick, isRowActivationKey } from "../activatable-row"; import { useBench } from "../bench-context"; import { readLastWorkbenchId } from "../last-workbench"; import { @@ -78,6 +78,7 @@ import { useBenchActivity } from "../shell/bench-activity"; import { artifactUploadToast, copyArtifactLinks, + copyArtifactLinksActionLabel, copyArtifactLinksToastLabel, isArtifactsUnavailableStatus, LIBRARY_BULK_OPERATION_IDS, @@ -112,6 +113,14 @@ function ArtifactRows({ : allSelected ? true : "indeterminate"; + // `useListSelection` hands back ids in toggle/insertion order, not row + // order — a bottom-up shift-select would otherwise join/copy links out of + // visible order. Sort against this row order before handing ids to any + // bulk operation (copy links here, the context menu's `ids` below). + const visibleOrder = useMemo( + () => new Map(artifacts.map((artifact, index) => [artifact.id, index])), + [artifacts], + ); return ( @@ -139,7 +148,10 @@ function ArtifactRows({ const isSelected = selection.isSelected(artifact.id); const selectionIds = isSelected && selection.selectedCount > 1 - ? [...selection.selectedIds] + ? [...selection.selectedIds].sort( + (a, b) => + (visibleOrder.get(a) ?? 0) - (visibleOrder.get(b) ?? 0), + ) : [artifact.id]; return ( { - if (event.shiftKey || event.metaKey || event.ctrlKey) { + if (event.shiftKey || isAdditiveSelectClick(event)) { selection.toggle(artifact.id, { shiftKey: event.shiftKey }); return; } @@ -371,8 +383,22 @@ export function LibraryPage({ () => visible.map((artifact) => artifact.id), [visible], ); + // A row filtered out of `visibleIds` drops out of `selection.selectedIds` + // immediately (the hook reconciles against `ids` on every read) but + // `useListSelection` keeps it in its own internal state, so the row comes + // back selected if the filter that hid it is cleared. Deliberate: it + // matches Finder/Sheets ("clearing a filter doesn't lose your picks") and + // needs no bookkeeping here. const selection = useListSelection({ ids: visibleIds }); + // Rows and cards render selection differently — only rows has checkboxes + // — so a selection made in one view has nothing to anchor to in the + // other. Clearing on view change is simpler than teaching the card view + // its own checkboxes for a selection UI it doesn't otherwise need. + useEffect(() => { + selection.clear(); + }, [viewMode, selection.clear]); + const openPicker = useCallback(() => { if (uploading === true) return; fileInputRef.current?.click(); @@ -563,7 +589,9 @@ export function LibraryPage({ variant="outline" data-bulk-action={LIBRARY_BULK_OPERATION_IDS[0]} onClick={() => { - const ids = [...selection.selectedIds]; + const ids = [...selection.selectedIds].sort( + (a, b) => visibleIds.indexOf(a) - visibleIds.indexOf(b), + ); void copyArtifactLinks(ids).then( () => toast(copyArtifactLinksToastLabel(ids.length)), () => toast("Couldn't copy the link"), @@ -571,7 +599,7 @@ export function LibraryPage({ }} >