diff --git a/packages/app/e2e/browser/workspace-content-search.spec.ts b/packages/app/e2e/browser/workspace-content-search.spec.ts index 5c3c539f59..df0c4cedc6 100644 --- a/packages/app/e2e/browser/workspace-content-search.spec.ts +++ b/packages/app/e2e/browser/workspace-content-search.spec.ts @@ -168,15 +168,26 @@ test("opening the same occurrence again navigates the pane it is already showing import { revealExactPathOnHover } from "../support/helpers/workspace-content-search"; -test("pointing at a result reveals its exact path when the row label cannot", async ({ +test("a result's tooltip paints the exact path, however deep", async ({ page, withWorkspace, }, testInfo) => { - const { panel, rows } = await revealExactPathOnHover(page, await withWorkspace()); - await rows.last().hover(); - await panel.screenshot({ path: testInfo.outputPath("hover-exact-path.png") }); - // Pointing must not disturb the keyboard selection or the Enter contract. + const { rows, tip } = await revealExactPathOnHover(page, await withWorkspace()); + await rows.first().hover(); + await expect(tip).toBeVisible(); + await page.waitForTimeout(400); + await page.screenshot({ path: testInfo.outputPath("tooltip-exact-path.png") }); + // Pointing must not move the keyboard selection: Enter still opens the first result. await page.keyboard.press("Enter"); - await expect(panel).toBeHidden(); + await expect(page.getByTestId("command-center-panel")).toBeHidden(); await expect(page.getByTestId("file-source-editor")).toContainText("SHARED_UTIL"); }); + +import { keepDesktopOverlayChrome } from "../support/helpers/workspace-content-search"; + +test("the desktop panel keeps its backdrop, Escape and focus return in every scope", async ({ + page, + withWorkspace, +}) => { + await keepDesktopOverlayChrome(page, await withWorkspace()); +}); diff --git a/packages/app/e2e/support/helpers/workspace-content-search.ts b/packages/app/e2e/support/helpers/workspace-content-search.ts index 53e4d4ed7d..905166db2e 100644 --- a/packages/app/e2e/support/helpers/workspace-content-search.ts +++ b/packages/app/e2e/support/helpers/workspace-content-search.ts @@ -1,6 +1,7 @@ import { mkdir, unlink, writeFile } from "node:fs/promises"; import path from "node:path"; -import { expect, type Page } from "@playwright/test"; +import { expect, type Locator, type Page } from "@playwright/test"; +import { composerLocator } from "./composer"; import type { CreatedWorkspace } from "./with-workspace"; export async function prepareContentSearch(workspace: CreatedWorkspace) { @@ -243,13 +244,17 @@ async function openReopenTarget(page: Page) { } /** - * Two files can share a base name and their nearest parent, so the resting row label cannot name - * them. Pointing at a row, or selecting it, spells the exact workspace-relative path out. + * Two files can share a base name and their nearest parent, and a real path is often wider than + * the result column, so the exact workspace-relative path lives in the tooltip. Being in the DOM + * is not enough: the tooltip has to be the thing painted at its own coordinates. */ export async function revealExactPathOnHover(page: Page, workspace: CreatedWorkspace) { + const deep = + "packages/app/src/command-center/workspace-content-search/internal/deeply/nested/result-path-presentation.ts"; for (const relative of [ "packages/app/src/utils/index.ts", "packages/server/src/utils/index.ts", + deep, ]) { const target = path.join(workspace.repoPath, relative); await mkdir(path.dirname(target), { recursive: true }); @@ -262,17 +267,81 @@ export async function revealExactPathOnHover(page: Page, workspace: CreatedWorks .fill("SHARED_UTIL"); const panel = page.getByTestId("command-center-panel"); - const rows = panel.getByRole("button", { name: /index\.ts:1:\d+/ }); - await expect(rows).toHaveCount(2); - // The accessible name always carries the exact path. - await expect(rows.first()).toHaveAccessibleName(/^packages\/app\/src\/utils\/index\.ts:1:24 /); - await expect(rows.last()).toHaveAccessibleName(/^packages\/server\/src\/utils\/index\.ts:1:24 /); - // The second row is at rest and shows only its nearest parent, which both files share. - await expect(rows.last()).toContainText("…/utils/index.ts:1:24"); + const rows = panel.getByRole("button", { name: /:1:24/ }); + await expect(rows).toHaveCount(3); + const tip = page.getByTestId("content-search-row-path"); - await rows.last().hover(); - await expect(rows.last()).toContainText("packages/server/src/utils/index.ts:1:24"); - await rows.first().hover(); - await expect(rows.first()).toContainText("packages/app/src/utils/index.ts:1:24"); - return { panel, rows }; + async function expectPaintedPath(row: Locator, expected: string) { + await row.hover(); + await expect(tip).toHaveText(expected); + // The reader must actually see it: whatever is painted at the tooltip's own coordinates has + // to be the tooltip, not the panel that used to cover it. + // A tooltip is pointer-transparent, so elementFromPoint reports the row underneath either + // way, and its surface is the same white as the panel. What settles it is whether the pixels + // in its own rectangle change when it opens: if the panel still covers it, they do not. + const box = await tip.boundingBox(); + if (!box) throw new Error("tooltip has no box"); + const clip = { x: box.x, y: box.y, width: box.width, height: box.height }; + const shown = await page.screenshot({ clip }); + await page.mouse.move(2, 2); + await expect(tip).toBeHidden(); + const hidden = await page.screenshot({ clip }); + expect( + shown.equals(hidden), + "the tooltip's own rectangle must change when it opens, or the reader cannot see it", + ).toBe(false); + } + + // Same base name and same nearest parent: the resting labels cannot tell these apart. + const app = rows.filter({ hasText: "…/utils/index.ts:1:24" }).first(); + const server = rows.filter({ hasText: "…/utils/index.ts:1:24" }).last(); + await expect(app).toHaveAccessibleName(/^packages\/app\/src\/utils\/index\.ts:1:24 /); + await expect(server).toHaveAccessibleName(/^packages\/server\/src\/utils\/index\.ts:1:24 /); + await expectPaintedPath(app, "packages/app/src/utils/index.ts:1:24"); + await expectPaintedPath(server, "packages/server/src/utils/index.ts:1:24"); + + // A path far wider than the result column still reads in full. + await expectPaintedPath( + rows.filter({ hasText: "result-path-presentation.ts" }).first(), + `${deep}:1:24`, + ); + return { panel, rows, tip }; +} + +/** + * The overlay chrome the desktop panel owns: a backdrop that dismisses it, Escape that dismisses + * it, and focus that goes back where it came from. Hosting the panel in the shared overlay root + * rather than a native Modal must not change any of it. + */ +export async function keepDesktopOverlayChrome(page: Page, workspace: CreatedWorkspace) { + await writeFile(path.join(workspace.repoPath, "chrome.ts"), 'const a = "CHROME_TARGET";\n'); + await workspace.navigateTo(); + const panel = page.getByTestId("command-center-panel"); + const input = page.getByRole("textbox", { name: "Search saved file contents...", exact: true }); + + // Backdrop dismisses the panel. + const composer = composerLocator(page); + await composer.click(); + await expect(composer).toBeFocused(); + await page.keyboard.press("Meta+Shift+F"); + await expect(input).toBeFocused(); + await page.mouse.click(page.viewportSize()!.width - 12, page.viewportSize()!.height - 12); + await expect(panel).toBeHidden(); + + // Escape dismisses it and hands focus back to the composer it was opened from. + await page.keyboard.press("Meta+Shift+F"); + await input.fill("CHROME_TARGET"); + await expect(panel.getByRole("button", { name: /chrome\.ts:1:/ })).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(panel).toBeHidden(); + await expect(composer).toBeFocused(); + + // Ordinary commands and the Files scope still open over the same chrome. + await page.keyboard.press("Meta+k"); + await expect(page.getByTestId("command-center-input")).toBeFocused(); + await page.keyboard.press("Escape"); + await page.keyboard.press("Meta+p"); + await expect(page.getByRole("textbox", { name: "Search files...", exact: true })).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(panel).toBeHidden(); } diff --git a/packages/app/src/command-center/command-center.tsx b/packages/app/src/command-center/command-center.tsx index f45117c26c..71d646f30e 100644 --- a/packages/app/src/command-center/command-center.tsx +++ b/packages/app/src/command-center/command-center.tsx @@ -11,7 +11,8 @@ import { type NativeSyntheticEvent, type PressableStateCallbackType, } from "react-native"; -import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { memo, useCallback, useEffect, useMemo, useRef, useState, type ReactElement } from "react"; +import { createPortal } from "react-dom"; import { useTranslation } from "react-i18next"; import { Check, ChevronRight, Folder, X } from "lucide-react-native"; import { StyleSheet, withUnistyles } from "react-native-unistyles"; @@ -37,6 +38,7 @@ import { isNative, isWeb } from "@/constants/platform"; import { useAggregatedAgents, type AggregatedAgent } from "@/hooks/use-aggregated-agents"; import { useProjects } from "@/hooks/use-projects"; import { + getOverlayRoot, OverlayLayerProvider, useGlobalWebOverlayLayer, useWebOverlayRegistration, @@ -809,55 +811,72 @@ export function CommandCenter() { ); } if (!state.open) return null; - return ( + const desktopContent = ( - - - - - - {scopeChipLabel ? ( - - ) : null} - - - - {state.scope === "content" ? ( - - ) : ( - <> - {fileSearchError} - - - )} + + + + + {scopeChipLabel ? ( + + ) : null} + + + {state.scope === "content" ? ( + + ) : ( + <> + {fileSearchError} + + + )} - + ); + return hostDesktopOverlay(desktopContent, state.close); +} + +/** + * Desktop web hosts the panel in the shared overlay root rather than React Native Web's `Modal`, + * which paints in a plane of its own above it. Inside that plane an ordinary portal — the tooltip + * on a result row — can never cover the panel however high its own z-index, so the row could not + * show a reader its exact path. `adaptive-modal-sheet` already hosts desktop modals this way; see + * docs/floating-panels.md. Native keeps the Modal. + */ +function hostDesktopOverlay(content: ReactElement, onRequestClose: () => void): ReactElement { + if (isWeb && typeof document !== "undefined") { + return createPortal(content, getOverlayRoot()); + } + return ( + + {content} + + ); } function FileSearchLoadingIndicator({ loading, label }: { loading: boolean; label: string }) { @@ -897,10 +916,11 @@ function ScopeChip({ label, onRemove }: { label: string; onRemove(): void }) { const styles = StyleSheet.create((theme) => ({ overlay: { - flex: 1, + ...StyleSheet.absoluteFillObject, justifyContent: "flex-start", alignItems: "center", paddingTop: theme.spacing[12], + pointerEvents: "auto", }, backdrop: { ...StyleSheet.absoluteFillObject, backgroundColor: "rgba(0, 0, 0, 0.5)" }, contentPanel: { width: 1000, maxWidth: "96%", height: "75%" }, diff --git a/packages/app/src/command-center/workspace-content-search/index.tsx b/packages/app/src/command-center/workspace-content-search/index.tsx index 09942b71d2..50e47cd999 100644 --- a/packages/app/src/command-center/workspace-content-search/index.tsx +++ b/packages/app/src/command-center/workspace-content-search/index.tsx @@ -5,6 +5,7 @@ import { useTranslation } from "react-i18next"; import type { WorkspaceContentMatch } from "@getpaseo/protocol/messages"; import { isWeb } from "@/constants/platform"; import { Button } from "@/components/ui/button"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import { LoadingSpinner } from "@/components/ui/loading-spinner"; import { useActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store"; import { useWorkspaceDirectory } from "@/stores/session-store-hooks"; @@ -234,43 +235,42 @@ function ResultRow({ const file = useMemo(() => describeResultPath(item.path), [item.path]); const location = `${item.path}:${item.line}:${item.columnStart}`; return ( - - {({ hovered }: PressableStateCallbackType & { hovered?: boolean }) => { - // Pointing at a row, or selecting it, spells every parent out; at rest the row shows only - // the nearest one so the column stays scannable. - const parents = hovered || selected ? file.head : file.directory; - return ( - <> - - {line.before} - {line.match} - {line.after} - - {/* The name and coordinates are pinned; only the parent context may shrink, so a deep - path loses its middle rather than the identity the reader is scanning for. Two files - can share a name and their nearest parent, so pointing at a row spells the rest of - the path out. */} - - {parents ? ( - - {parents} - - ) : null} - - {file.name}:{item.line}:{item.columnStart} + // The resting row shows the file and its nearest parent, which two files can share + // (packages/app/src/utils/index.ts and packages/server/src/utils/index.ts). The tooltip is + // where the exact workspace-relative path lives; it is not bounded by the column's width. + + + + + {line.before} + {line.match} + {line.after} + + {/* The name and coordinates are pinned; only the parent context may shrink, so a deep + path loses its middle rather than the identity the reader is scanning for. */} + + {file.directory ? ( + + {file.directory} - - - ); - }} - + ) : null} + + {file.name}:{item.line}:{item.columnStart} + + + + + + {location} + + ); } diff --git a/packages/app/src/command-center/workspace-content-search/internal/result-path.test.ts b/packages/app/src/command-center/workspace-content-search/internal/result-path.test.ts index 36af61bb89..f13261230d 100644 --- a/packages/app/src/command-center/workspace-content-search/internal/result-path.test.ts +++ b/packages/app/src/command-center/workspace-content-search/internal/result-path.test.ts @@ -2,24 +2,13 @@ import { describe, expect, it } from "vitest"; import { describeResultPath } from "./result-path"; describe("describeResultPath", () => { - it("spells out every parent for the row being pointed at", () => { - // Two files can share a name and their nearest parent; only the head tells them apart. - expect(describeResultPath("packages/app/src/utils/index.ts").head).toBe( - "packages/app/src/utils/", - ); - expect(describeResultPath("packages/server/src/utils/index.ts").head).toBe( - "packages/server/src/utils/", - ); - }); - it("keeps a root file whole", () => { - expect(describeResultPath("README.md")).toEqual({ directory: "", head: "", name: "README.md" }); + expect(describeResultPath("README.md")).toEqual({ directory: "", name: "README.md" }); }); it("keeps a shallow path whole", () => { expect(describeResultPath("docs/architecture.md")).toEqual({ directory: "docs/", - head: "docs/", name: "architecture.md", }); }); @@ -28,19 +17,14 @@ describe("describeResultPath", () => { // These two rows are both "index.tsx"; the parents are the only thing telling them apart. expect( describeResultPath("packages/app/src/command-center/workspace-content-search/index.tsx"), - ).toEqual({ - directory: "…/workspace-content-search/", - head: "packages/app/src/command-center/workspace-content-search/", - name: "index.tsx", - }); + ).toEqual({ directory: "…/workspace-content-search/", name: "index.tsx" }); expect(describeResultPath("packages/app/src/file-pane/source/index.tsx")).toEqual({ directory: "…/source/", - head: "packages/app/src/file-pane/source/", name: "index.tsx", }); }); it("keeps a literal backslash in a file name out of the directory", () => { - expect(describeResultPath("a\\b.txt")).toEqual({ directory: "", head: "", name: "a\\b.txt" }); + expect(describeResultPath("a\\b.txt")).toEqual({ directory: "", name: "a\\b.txt" }); }); }); diff --git a/packages/app/src/command-center/workspace-content-search/internal/result-path.ts b/packages/app/src/command-center/workspace-content-search/internal/result-path.ts index 2909ec11a7..c7cb8cef8e 100644 --- a/packages/app/src/command-center/workspace-content-search/internal/result-path.ts +++ b/packages/app/src/command-center/workspace-content-search/internal/result-path.ts @@ -9,8 +9,6 @@ export interface ResultPathParts { /** Directory context immediately above the file, already elided at the head when deeper. */ directory: string; - /** Every directory above the file, for the row the reader is pointing at or has selected. */ - head: string; /** File name, which is never truncated away. */ name: string; } @@ -20,12 +18,8 @@ const PARENTS_KEPT = 1; export function describeResultPath(path: string): ResultPathParts { const segments = path.split("/"); const name = segments.pop() ?? path; - if (segments.length === 0) return { directory: "", head: "", name }; + if (segments.length === 0) return { directory: "", name }; const kept = segments.slice(-PARENTS_KEPT); const elided = segments.length > kept.length; - return { - directory: `${elided ? "…/" : ""}${kept.join("/")}/`, - head: `${segments.join("/")}/`, - name, - }; + return { directory: `${elided ? "…/" : ""}${kept.join("/")}/`, name }; }