From 0581aa12563af5bb78996d282bd0ec1d9375992a Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 6 Jun 2026 21:56:05 +0000 Subject: [PATCH] Rework workspace browser to match PAP-10508 design (PAP-10511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses UXDesigner review against the approved design spec: - Placement: move the discovery affordance into a calm action row at the bottom of IssueWorkspaceCard ("Browse files…" + "Open file by path…"), removing the orphaned button between Artifacts and Tabs. The existing top-right toolbar / `g f` entry stays as the secondary affordance. - Single sheet, two modes: opening a result from browse now shows a "← Back to files" affordance (and a separate × close) in the view header, with Esc returning to the list. Browse state is URL-backed (`browse=1`, `q`) via FileViewerContext (open(...{fromBrowse}), openBrowse, backToFiles). - Remove the Recent/All toggle: empty query → Recently changed (silent fallback to full listing when git change-tracking is unavailable); populated query → Files matching. Search field accepts `path:line` and Enter opens the highlighted row or the typed path. - Mobile ( --- ui/src/components/FileViewerSheet.tsx | 200 ++++------ ui/src/components/IssueWorkspaceCard.tsx | 30 +- .../components/WorkspaceFileBrowser.test.tsx | 106 +++-- ui/src/components/WorkspaceFileBrowser.tsx | 361 +++++++++--------- ui/src/context/FileViewerContext.tsx | 108 +++++- ui/src/pages/IssueDetail.tsx | 23 +- .../workspace-file-browser.stories.tsx | 199 +++++++--- 7 files changed, 576 insertions(+), 451 deletions(-) diff --git a/ui/src/components/FileViewerSheet.tsx b/ui/src/components/FileViewerSheet.tsx index cb38eaad14d..8bf1884f582 100644 --- a/ui/src/components/FileViewerSheet.tsx +++ b/ui/src/components/FileViewerSheet.tsx @@ -9,6 +9,7 @@ import { import { useQuery, type UseQueryResult } from "@tanstack/react-query"; import { AlertTriangle, + ArrowLeft, Ban, Check, Cloud, @@ -16,10 +17,12 @@ import { FileCode2, FileSearch, FolderOpen, + FolderSearch, Link2, Loader2, Lock, RefreshCcw, + X, } from "lucide-react"; import { Sheet, @@ -37,7 +40,6 @@ import { useRequiredFileViewer, type FileViewerUrlState, } from "@/context/FileViewerContext"; -import { parseWorkspaceFileRef } from "@/lib/workspace-file-parser"; import { WorkspaceFileBrowser } from "@/components/WorkspaceFileBrowser"; import type { ResolvedWorkspaceResource, @@ -178,112 +180,6 @@ function FileViewerStateView({ ); } -interface OpenFilePromptProps { - onSubmit: ( - path: string, - workspace: WorkspaceFileSelector, - line: number | null, - column: number | null, - ) => void; -} - -function OpenFilePrompt({ onSubmit }: OpenFilePromptProps) { - const [value, setValue] = useState(""); - const [workspace, setWorkspace] = useState("auto"); - const inputRef = useRef(null); - - useEffect(() => { - const frame = requestAnimationFrame(() => inputRef.current?.focus()); - return () => cancelAnimationFrame(frame); - }, []); - - const handleSubmit = (event: React.FormEvent) => { - event.preventDefault(); - const trimmed = value.trim(); - if (!trimmed) return; - const parsed = parseWorkspaceFileRef(trimmed); - if (parsed) { - onSubmit(parsed.path, workspace, parsed.line, parsed.column); - return; - } - onSubmit(trimmed, workspace, null, null); - }; - - return ( -
- - setValue(event.target.value)} - autoComplete="off" - spellCheck={false} - /> -
- Workspace -
- {(["auto", "execution", "project"] as const).map((option) => ( - - ))} -
-
-
- -
-
- ); -} - -interface FileViewerEmptyStateProps { - issueId: string; - onBrowseOpen: (ref: { path: string; workspace: WorkspaceFileSelector }) => void; - onSubmitPath: OpenFilePromptProps["onSubmit"]; -} - -function FileViewerEmptyState({ issueId, onBrowseOpen, onSubmitPath }: FileViewerEmptyStateProps) { - return ( -
- -
- - Open a specific path… - - -
-
- ); -} - interface FileContentViewerProps { content: WorkspaceFileContent; highlightedLine: number | null; @@ -438,7 +334,12 @@ export function FileViewerSheet({ }: FileViewerSheetProps) { const viewer = useRequiredFileViewer(); const state = typeof stateProp !== "undefined" ? stateProp : viewer.state; - const computedOpen = typeof openProp === "boolean" ? openProp : state !== null || showPromptWhenEmpty; + // Browse mode: no file selected, but the sheet was opened to browse/search. + const browseMode = state === null && (showPromptWhenEmpty || viewer.browse); + // True when the current file was reached from the browse list (drill-down). + const cameFromBrowse = state !== null && viewer.browse; + const computedOpen = + typeof openProp === "boolean" ? openProp : state !== null || showPromptWhenEmpty || viewer.browse; const [elapsedMs, setElapsedMs] = useState(0); const [copiedField, setCopiedField] = useState<"path" | "link" | null>(null); @@ -506,14 +407,12 @@ export function FileViewerSheet({ [onOpenChange, viewer], ); - const handlePromptSubmit = useCallback( - ( - path: string, - workspace: WorkspaceFileSelector, - line: number | null, - column: number | null, - ) => { - viewer.open({ path, line, column, workspace }); + const handleBrowseOpen = useCallback( + (ref: { path: string; workspace: WorkspaceFileSelector; line?: number | null; column?: number | null }) => { + viewer.open( + { path: ref.path, line: ref.line ?? null, column: ref.column ?? null, workspace: ref.workspace }, + { fromBrowse: true }, + ); }, [viewer], ); @@ -556,10 +455,21 @@ export function FileViewerSheet({ aria-labelledby={FILE_VIEWER_LABELLED_BY_ID} aria-describedby={FILE_VIEWER_DESCRIBED_BY_ID} showCloseButton={false} + onEscapeKeyDown={(event) => { + // From a file reached via browse, Esc returns to the list; a second Esc closes. + if (cameFromBrowse) { + event.preventDefault(); + viewer.backToFiles(); + } + }} >
-
{resolvedResource ? ( @@ -668,11 +605,12 @@ export function FileViewerSheet({ : null } /> - ) : showPromptWhenEmpty ? ( - viewer.open({ path: ref.path, line: null, column: null, workspace: ref.workspace })} - onSubmitPath={handlePromptSubmit} + onOpen={handleBrowseOpen} + initialQuery={viewer.query} + className="min-h-0 flex-1 p-4" /> ) : null} diff --git a/ui/src/components/IssueWorkspaceCard.tsx b/ui/src/components/IssueWorkspaceCard.tsx index 35cfa60d172..f8e1a03532c 100644 --- a/ui/src/components/IssueWorkspaceCard.tsx +++ b/ui/src/components/IssueWorkspaceCard.tsx @@ -10,7 +10,7 @@ import { queryKeys } from "../lib/queryKeys"; import { orderReusableExecutionWorkspaces } from "../lib/reusable-execution-workspaces"; import { cn, projectWorkspaceUrl } from "../lib/utils"; import { Button } from "@/components/ui/button"; -import { Check, Copy, GitBranch, FolderOpen, Pencil, X } from "lucide-react"; +import { Check, Copy, FileSearch, FolderOpen, FolderSearch, GitBranch, Pencil, X } from "lucide-react"; /* -------------------------------------------------------------------------- */ /* Utility helpers (mirrored from IssueProperties for self-containment) */ @@ -192,6 +192,10 @@ interface IssueWorkspaceCardProps { initialEditing?: boolean; livePreview?: boolean; onDraftChange?: (data: Record, meta: { canSave: boolean; workspaceBranchName?: string | null }) => void; + /** Opens the workspace file browser sheet. When omitted, the browse row is hidden. */ + onBrowseFiles?: () => void; + /** Opens the same browser sheet focused for path entry. */ + onOpenFileByPath?: () => void; } export function IssueWorkspaceCard({ @@ -201,6 +205,8 @@ export function IssueWorkspaceCard({ initialEditing = false, livePreview = false, onDraftChange, + onBrowseFiles, + onOpenFileByPath, }: IssueWorkspaceCardProps) { const { selectedCompanyId } = useCompany(); const companyId = issue.companyId ?? selectedCompanyId; @@ -572,6 +578,28 @@ export function IssueWorkspaceCard({ )} )} + + {/* Workspace file discovery — calm row under the workspace identity. */} + {!showEditingControls && onBrowseFiles && ( +
+ + +
+ )} ); } diff --git a/ui/src/components/WorkspaceFileBrowser.test.tsx b/ui/src/components/WorkspaceFileBrowser.test.tsx index 80d19caa1db..f65645704ba 100644 --- a/ui/src/components/WorkspaceFileBrowser.test.tsx +++ b/ui/src/components/WorkspaceFileBrowser.test.tsx @@ -31,14 +31,14 @@ function createItem(overrides: Partial = {}): WorkspaceFi workspaceId: "ws-1", contentType: "text/plain; charset=utf-8", byteSize: 2048, - modifiedAt: null, + modifiedAt: new Date(Date.now() - 120_000).toISOString(), previewKind: "text", capabilities: { preview: true, download: false, listChildren: false }, ...overrides, }; } -function availableResponse(items: WorkspaceFileListItem[]): WorkspaceFileListResponse { +function availableResponse(items: WorkspaceFileListItem[], truncated = false): WorkspaceFileListResponse { return { kind: "workspace_file_list", state: "available", @@ -51,7 +51,7 @@ function availableResponse(items: WorkspaceFileListItem[]): WorkspaceFileListRes query: { workspace: "auto", mode: "changed", q: null, limit: 100 }, items, scannedCount: items.length, - truncated: false, + truncated, }; } @@ -68,6 +68,10 @@ function unavailableResponse(reason: string): WorkspaceFileListResponse { }; } +function ok(data: WorkspaceFileListResponse) { + return { data, isFetching: false, isError: false, error: null, refetch: vi.fn() }; +} + describe("WorkspaceFileBrowser", () => { let container: HTMLDivElement; @@ -89,82 +93,72 @@ describe("WorkspaceFileBrowser", () => { return { root, onOpen }; } - it("lists available files and opens one with its relative path and workspace selector", () => { - useQueryMock.mockReturnValue({ - data: availableResponse([createItem(), createItem({ relativePath: "README.md", displayPath: "README.md" })]), - isFetching: false, - isError: false, - error: null, - refetch: vi.fn(), - }); + it("renders the Recently changed list as a listbox and opens a row with its relative path", () => { + useQueryMock.mockReturnValue( + ok(availableResponse([createItem(), createItem({ relativePath: "README.md", displayPath: "README.md" })])), + ); const { onOpen } = renderBrowser(); + expect(container.querySelector('[role="listbox"]')).not.toBeNull(); + expect(container.textContent).toContain("Recently changed"); expect(container.textContent).toContain("From Isolated workspace"); - const fileButton = Array.from(container.querySelectorAll("button")).find((b) => - b.getAttribute("title") === "ui/src/pages/IssueDetail.tsx", + + const option = Array.from(container.querySelectorAll('[role="option"]')).find( + (el) => el.getAttribute("title") === "ui/src/pages/IssueDetail.tsx", ); - expect(fileButton).not.toBeUndefined(); + expect(option).not.toBeUndefined(); act(() => { - fileButton!.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true })); + option!.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true })); }); - expect(onOpen).toHaveBeenCalledWith({ path: "ui/src/pages/IssueDetail.tsx", workspace: "auto" }); }); - it("shows the remote-workspace state without any file rows", () => { - useQueryMock.mockReturnValue({ - data: unavailableResponse("remote_workspace"), - isFetching: false, - isError: false, - error: null, - refetch: vi.fn(), - }); - + it("does not render a Recent/All toggle", () => { + useQueryMock.mockReturnValue(ok(availableResponse([createItem()]))); renderBrowser(); - expect(container.textContent).toContain("Remote workspace"); - expect(container.textContent).not.toContain("From Isolated workspace"); + expect(container.textContent).not.toContain("All files"); + expect(container.textContent).not.toContain("Recent changes / All"); + }); + + it("discloses truncation in the footer", () => { + useQueryMock.mockReturnValue(ok(availableResponse([createItem()], true))); + renderBrowser(); + expect(container.textContent).toContain("refine the search to narrow"); + }); + + it("opens the highlighted row when Enter is pressed in the search field", () => { + useQueryMock.mockReturnValue(ok(availableResponse([createItem()]))); + const { onOpen } = renderBrowser(); + const input = container.querySelector("input")!; + act(() => { + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true, cancelable: true })); + }); + expect(onOpen).toHaveBeenCalledWith({ path: "ui/src/pages/IssueDetail.tsx", workspace: "auto" }); + }); + + it("shows the remote-workspace state without file rows", () => { + useQueryMock.mockReturnValue(ok(unavailableResponse("remote_workspace"))); + renderBrowser(); + expect(container.textContent).toContain("Remote workspace preview not supported"); + expect(container.querySelector('[role="listbox"]')).toBeNull(); }); it("shows the no-workspace state when the issue has no workspace", () => { - useQueryMock.mockReturnValue({ - data: unavailableResponse("no_workspace"), - isFetching: false, - isError: false, - error: null, - refetch: vi.fn(), - }); - + useQueryMock.mockReturnValue(ok(unavailableResponse("no_workspace"))); renderBrowser(); expect(container.textContent).toContain("No workspace yet"); }); - - it("offers a fallback to all files when recent-change tracking is unavailable", () => { - useQueryMock.mockReturnValue({ - data: unavailableResponse("changed_unavailable"), - isFetching: false, - isError: false, - error: null, - refetch: vi.fn(), - }); - - renderBrowser(); - expect(container.textContent).toContain("Recent changes unavailable"); - const fallback = Array.from(container.querySelectorAll("button")).find((b) => - b.textContent?.includes("Browse all files"), - ); - expect(fallback).not.toBeUndefined(); - }); }); describe("describeUnavailable", () => { - it("maps each documented reason to a distinct, human-readable title", () => { - expect(describeUnavailable("remote_workspace").title).toBe("Remote workspace"); + it("maps reasons to copy that matches the viewer's denial voice", () => { + expect(describeUnavailable("remote_workspace").title).toBe("Remote workspace preview not supported"); expect(describeUnavailable("no_workspace").title).toBe("No workspace yet"); expect(describeUnavailable("no_local_workspace").title).toBe("No workspace yet"); - expect(describeUnavailable("workspace_unavailable").title).toBe("Workspace cleaned up"); - expect(describeUnavailable("changed_unavailable").title).toBe("Recent changes unavailable"); + expect(describeUnavailable("workspace_unavailable").title).toBe("Workspace is no longer available"); + expect(describeUnavailable("archived").title).toBe("Workspace is no longer available"); }); it("never leaks the raw reason code as the body", () => { diff --git a/ui/src/components/WorkspaceFileBrowser.tsx b/ui/src/components/WorkspaceFileBrowser.tsx index 9f73cdfbe73..3ff6e7a820d 100644 --- a/ui/src/components/WorkspaceFileBrowser.tsx +++ b/ui/src/components/WorkspaceFileBrowser.tsx @@ -1,20 +1,21 @@ -import { useEffect, useMemo, useRef, useState, type ReactNode } from "react"; -import { useQuery } from "@tanstack/react-query"; import { - AlertTriangle, - Cloud, - FileCode2, - FolderOpen, - FolderSearch, - Loader2, - Search, -} from "lucide-react"; + useEffect, + useId, + useMemo, + useRef, + useState, + type KeyboardEvent, + type ReactNode, +} from "react"; +import { useQuery } from "@tanstack/react-query"; +import { AlertTriangle, Cloud, FileCode2, FolderOpen, Loader2, Search } from "lucide-react"; import { Input } from "@/components/ui/input"; -import { Button } from "@/components/ui/button"; import { cn } from "@/lib/utils"; +import { timeAgo } from "@/lib/timeAgo"; import { fileResourcesApi } from "@/api/file-resources"; import { ApiError } from "@/api/client"; import { queryKeys } from "@/lib/queryKeys"; +import { parseWorkspaceFileRef } from "@/lib/workspace-file-parser"; import type { WorkspaceFileListItem, WorkspaceFileListMode, @@ -27,20 +28,11 @@ const WORKSPACE_OPTIONS: ReadonlyArray<{ value: WorkspaceFileSelector; label: st { value: "project", label: "Project" }, ]; -const MODE_OPTIONS: ReadonlyArray<{ value: Extract; label: string }> = [ - { value: "changed", label: "Recent changes" }, - { value: "all", label: "All files" }, -]; - +// Hard list cap. The spec called out ~50 to keep reads cheap; 100 trades a bit +// more scan for fewer "refine to narrow" dead-ends on large trees. Footer always +// discloses truncation so the cap is never silent. const LIST_LIMIT = 100; -function formatBytes(size: number | null | undefined): string | null { - if (typeof size !== "number" || !Number.isFinite(size) || size < 0) return null; - if (size < 1024) return `${size} B`; - if (size < 1024 * 1024) return `${(size / 1024).toFixed(1)} KB`; - return `${(size / (1024 * 1024)).toFixed(1)} MB`; -} - function basename(path: string): string { const idx = path.lastIndexOf("/"); return idx < 0 ? path : path.slice(idx + 1); @@ -52,17 +44,18 @@ function dirname(path: string): string { } /** - * Maps a server `unavailableReason` (or transport error) to a calm, board-readable - * explanation. Substring matching keeps it resilient to small reason-string changes - * on the server, mirroring describeDenial() in FileViewerSheet. + * Maps a server `unavailableReason` to a calm, board-readable explanation. + * Copy is kept in sync with `describeDenial` in FileViewerSheet so the browse + * states and the viewer's error panels read in one voice. Substring matching + * keeps it resilient to small reason-string changes on the server. */ export function describeUnavailable(reason: string): { title: string; body: string; icon: ReactNode } { const lower = reason.toLowerCase(); if (lower.includes("remote")) { return { icon: