diff --git a/packages/client/src/right-panel/CodeMenuFrame.tsx b/packages/client/src/right-panel/CodeMenuFrame.tsx index 7accc9b3..cb653c3e 100644 --- a/packages/client/src/right-panel/CodeMenuFrame.tsx +++ b/packages/client/src/right-panel/CodeMenuFrame.tsx @@ -11,6 +11,7 @@ import { CodeContextMenu, type CodeContextMenuController, } from "../ui/CodeContextMenu"; +import type { LineRef } from "../ui/lineRef"; import { type LineSelection, useLineSelection } from "../ui/useLineSelection"; export type CodeMenuFrameProps = { @@ -25,19 +26,57 @@ export type CodeMenuFrameProps = { * controller so a terminal `path:line` click drives both the * Pierre highlight AND the right-click menu's "Copy path:N" item. */ initialSelectedLines?: SelectedLineRange | null; + /** When provided, adds an "Open :" entry to the context + * menu that dispatches the selected ref to the host (typically a + * call to `openInCodeTab`). Omit for viewers where "open" is a + * no-op (the file is already on screen at line precision). */ + onOpen?: (ref: LineRef) => void; }; +/** Walk the contextmenu event's composed path (which pierces Pierre's + * open shadow DOM, where `event.target` would otherwise be retargeted + * to the shadow host) and return the line number from the first + * element carrying `data-column-number`. Returns null when the + * right-click landed outside any gutter line — empty area, scrollbar, + * decoration row — so the host can skip opening a menu entirely. */ +function lineFromContextMenu(event: MouseEvent): number | null { + for (const node of event.composedPath()) { + if (!(node instanceof Element)) continue; + const raw = node.getAttribute("data-column-number"); + if (raw === null) continue; + const n = Number(raw); + if (Number.isFinite(n) && n >= 1) return n; + } + return null; +} + export const CodeMenuFrame: Component = (props) => { let menuCtrl: CodeContextMenuController | undefined; const selection = useLineSelection(() => props.path, { initialRange: () => props.initialSelectedLines, + onOpen: () => props.onOpen, }); return (
el.addEventListener("contextmenu", (e) => menuCtrl?.open(e))} + ref={(el) => + el.addEventListener("contextmenu", (e) => { + // Right-click on a gutter line is the single entry point for + // the context menu: it both selects the line and opens the + // menu in one gesture. Right-clicks elsewhere (whitespace, + // scrollbar, decoration row) clear the range and produce no + // menu — `buildItems` returns empty when no range is set, + // so `menuCtrl.open` short-circuits without preventing the + // browser default. + const line = lineFromContextMenu(e); + selection.handleSelect( + line === null ? null : { start: line, end: line }, + ); + menuCtrl?.open(e); + }) + } class="h-full w-full" > {props.children(selection)} diff --git a/packages/client/src/right-panel/CodeTab.tsx b/packages/client/src/right-panel/CodeTab.tsx index 961de824..ee89b2ff 100644 --- a/packages/client/src/right-panel/CodeTab.tsx +++ b/packages/client/src/right-panel/CodeTab.tsx @@ -41,8 +41,12 @@ import { } from "../ui/pierreTheme"; import { resolveLineRefPath } from "../ui/lineRef"; import BrowseFileView from "./BrowseFileView"; -import { type CodeOpenRequest, pendingCodeOpen } from "./codeNavigation"; import CodeMenuFrame from "./CodeMenuFrame"; +import { + openInCodeTab, + type OpenInCodeTabRequest, + pendingOpen, +} from "./openInCodeTab"; import { projectFileTreeSearch } from "./fileSearch"; import FileSearchInput from "./FileSearchInput"; import ModeChipPicker, { type ModeOption } from "./ModeChipPicker"; @@ -176,57 +180,66 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => { // absolute path or `null`, never the empty string that would alias // null. const resetKey = createMemo(() => `${repoPath() ?? ""}::${view()}`); + + /** The resetKey effect runs BEFORE the pendingOpen effect by + * registration order. When a navigation request is about to land in + * the new (repo, mode) — same repoRoot, target mode equals the + * freshly-ticked `view()`, and the request hasn't already been + * consumed by `handled()` — the resetKey effect must skip its clear, + * or the pendingOpen effect would null what we're about to set. + * This predicate names the cross-effect temporal coupling so the + * guard isn't a wall of inline conjunctions a future editor has to + * re-derive. The `batch()` in `openInCodeTab` ensures both writes + * (`view` and `pendingOpen`) commit before either effect fires; the + * registration-order discipline survives ABOVE that. */ + const isPendingOpenAboutToLand = (): boolean => { + const req = pendingOpen(); + return ( + req !== null && + req.repoRoot === repoPath() && + req.targetMode === view() && + handled()?.request !== req + ); + }; + createEffect( on( resetKey, () => { setSearchQuery(""); - // Skip the selectedPath clear when an incoming request is - // about to land in the new mode — the resetKey effect runs - // before the pendingCodeOpen effect (registration order), and - // an unconditional clear would null what we're about to set. - // Reading `req.targetMode` (not `view()`) makes the guard - // robust to user-driven mode flips that race the click. - const req = pendingCodeOpen(); - if ( - req && - req.repoRoot === repoPath() && - req.targetMode === view() && - handled()?.request !== req - ) { - return; - } + if (isPendingOpenAboutToLand()) return; setSelectedPath(null); }, { defer: true }, ), ); - // Consume-once record for the latest pendingCodeOpen tick. Holds - // the full request object (reference identity discriminates two - // structurally-identical clicks — `requestCodeOpen` mints a fresh + // Consume-once record for the latest pendingOpen tick. Holds the + // full request object (reference identity discriminates two + // structurally-identical clicks — `openInCodeTab` mints a fresh // object per call) alongside the resolved path. Storing the // request here lets `selectedRange` derive its value without // re-running `resolveLineRefPath` (single resolution site per // request) and lets `resetKey` know whether a pending request // has already been applied. const [handled, setHandled] = createSignal<{ - request: CodeOpenRequest; + request: OpenInCodeTabRequest; resolvedPath: string | null; } | null>(null); - // Honor terminal file-ref clicks. The effect waits for the live - // `fsListAll` stream to settle so resolution can validate against - // a complete file list — otherwise a request fired during boot - // would toast "not found" on a path that just hasn't been - // enumerated yet. The terminal click handler is the sole site that - // flips the panel to browse mode; this effect only sets + // Honor every `openInCodeTab` request — terminal file-ref clicks, + // right-click "Open path:N" entries, and any future producer. The + // effect waits for the live `fsListAll` stream to settle so + // resolution can validate against a complete file list — otherwise + // a request fired during boot would toast "not found" on a path + // that just hasn't been enumerated yet. `openInCodeTab` flips the + // panel to browse mode itself; this effect only sets // `selectedPath`. The `resetKey` effect above guards against // clearing selectedPath when this effect is about to set it. createEffect( on( () => { - const req = pendingCodeOpen(); + const req = pendingOpen(); const paths = treePaths(); const isPending = allPaths.pending(); return { req, repo: repoPath(), paths, isPending }; @@ -263,7 +276,7 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => { // // No `equals` override: two clicks on the same `path:line` produce // structurally identical `{start, end}` but distinct request - // objects (`requestCodeOpen` mints a fresh one per call), so the + // objects (`openInCodeTab` mints a fresh one per call), so the // memo emits a fresh value on every click. Pierre's // `InteractionManager.setSelection` re-renders when the selection // is "dirty" — and tearing down the gutter (panel collapse, @@ -275,7 +288,7 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => { start: number; end: number; } | null>(() => { - const req = pendingCodeOpen(); + const req = pendingOpen(); if (!req) return null; const h = handled(); if (!h || h.request !== req || h.resolvedPath === null) return null; @@ -538,7 +551,19 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => { {(d) => ( - + { + // Diff paths are repo-relative; cwd is irrelevant. + const repo = repoPath(); + if (repo === null) return; + openInCodeTab({ + ref, + repoRoot: repo, + targetMode: "browse", + }); + }} + > {(selection) => ( // `` is the scroll container — // `` consumes its context and diff --git a/packages/client/src/right-panel/codeNavigation.ts b/packages/client/src/right-panel/codeNavigation.ts deleted file mode 100644 index e77179dc..00000000 --- a/packages/client/src/right-panel/codeNavigation.ts +++ /dev/null @@ -1,39 +0,0 @@ -/** Cross-component code-tab navigation requests. - * - * Terminal link-click handlers (and any future caller that needs to - * open a file from outside the right panel) publish a request here; - * `CodeTab` observes the signal and reacts. Latest request wins; - * callers don't need to clear it. Each call mints a fresh request - * object, so two clicks on the same `path:line` are distinct by - * reference — that's what lets `CodeTab` tell them apart even when - * their `ref` content matches. */ - -import { createSignal } from "solid-js"; -import type { LineRef } from "../ui/lineRef"; - -export interface CodeOpenRequest { - /** Parsed `path:line[-end]` the user clicked. */ - ref: LineRef; - /** Per-terminal git repo root that `ref.path` is relative to (when - * relative). Absolute paths beneath this root are also accepted — - * the resolver normalizes both shapes. */ - repoRoot: string; - /** Terminal cwd at the time of click. Drives the "user typed - * `bar.ts:42` while standing in a subdirectory of the repo" case; - * undefined falls back to repo-relative interpretation only. */ - cwd: string | undefined; - /** Code-tab mode the request expects to land in. Today only - * `"browse"` makes sense (highlighting a line in a diff doesn't - * generalize), but encoding the assumption on the type lets the - * consumer guard explicitly instead of relying on the click - * handler having pre-called `openCodeBrowser` in the right order. */ - targetMode: "browse"; -} - -const [pending, setPending] = createSignal(null); - -export const pendingCodeOpen = pending; - -export function requestCodeOpen(req: CodeOpenRequest): void { - setPending(req); -} diff --git a/packages/client/src/right-panel/fileSearch.ts b/packages/client/src/right-panel/fileSearch.ts index d0156bbd..da14c833 100644 --- a/packages/client/src/right-panel/fileSearch.ts +++ b/packages/client/src/right-panel/fileSearch.ts @@ -13,6 +13,8 @@ * the directories the wrapper should ensure are open so matches don't * hide behind a collapsed parent on first paint. */ +import { ancestorDirectoryPaths } from "@kolu/solid-pierre"; + type FileTreeSearchProjection = { projectedPaths: string[]; expandedAncestors: string[]; @@ -39,21 +41,6 @@ function pathContainsTokensInOrder( return true; } -/** Pierre uses `getAncestorDirectoryPaths` internally to drive - * expansion in `hide-non-matches` mode. Mirror that exact shape so - * the wrapper's expansion request reaches every dir Pierre infers - * from the projected paths. */ -function ancestorDirectoryPaths(path: string): string[] { - const normalized = path.endsWith("/") ? path.slice(0, -1) : path; - if (normalized.length === 0) return []; - const segments = normalized.split("/"); - const out: string[] = []; - for (let i = 1; i < segments.length; i += 1) { - out.push(`${segments.slice(0, i).join("/")}/`); - } - return out; -} - export function projectFileTreeSearch( paths: string[], query: string, diff --git a/packages/client/src/right-panel/openInCodeTab.ts b/packages/client/src/right-panel/openInCodeTab.ts new file mode 100644 index 00000000..d9368898 --- /dev/null +++ b/packages/client/src/right-panel/openInCodeTab.ts @@ -0,0 +1,59 @@ +/** Front door for "open this file:line in the Code tab". Every producer + * — terminal-link click, right-click "Open path:N" context-menu entry, + * future surfaces — calls `openInCodeTab(...)` instead of writing the + * preferences patch and pending-request signal separately. The function + * encapsulates the paired writes (panel-uncollapse + tab + browse-mode + + * pending request) so the SolidJS effect-ordering invariant lives here, + * not at every call site. + * + * Latest request wins; callers don't clear it. Each call mints a fresh + * request object — two clicks on the same `path:line` are distinct by + * reference, which is what lets `CodeTab` tell them apart even when + * their `ref` content matches and re-paint the highlight. */ + +import type { CodeTabView } from "kolu-common/surface"; +import { batch, createSignal } from "solid-js"; +import type { LineRef } from "../ui/lineRef"; +import { useRightPanel } from "./useRightPanel"; + +export interface OpenInCodeTabRequest { + /** Parsed `path:line[-end]` to navigate to. The path is interpreted + * relative to `repoRoot` (or, when present, cwd-relative under + * `repoRoot`) by `CodeTab` via `resolveLineRefPath`. */ + ref: LineRef; + /** Per-terminal git repo root that `ref.path` is relative to (when + * relative). Absolute paths beneath this root are also accepted — + * the resolver normalizes both shapes. */ + repoRoot: string; + /** Terminal cwd at the time of the request. Drives the "user typed + * `bar.ts:42` while standing in a subdirectory of the repo" case; + * undefined falls back to repo-relative interpretation only. */ + cwd?: string; + /** Which Code-tab sub-mode the request expects to land in. + * Producers that don't track an authoring mode pass `"browse"`. */ + targetMode: CodeTabView; +} + +// Module-level singleton. Right-panel state is a singleton in Kolu — +// one panel, one CodeTab — and the navigation request is meant for +// the unique consumer. If kolu ever mounts multiple CodeTab instances +// (split panels, multi-window), this signal must move into a +// SolidJS context or scope to a per-panel store, otherwise concurrent +// consumers will race on each other's pending requests. +const [pending, setPending] = createSignal(null); + +export const pendingOpen = pending; + +/** Open the right panel's Code tab at `req.targetMode` showing `req.ref`. + * The two reactive writes (preferences patch + pending-request signal) + * are wrapped in `batch()` so SolidJS defers dependent effects until + * both have committed. Without the batch, the preferences optimistic + * update ticks `view()` first, fires `CodeTab`'s `resetKey` effect + * when `pendingOpen()` is still null, the guard fails, and the + * selectedPath the user navigated to gets cleared. */ +export function openInCodeTab(req: OpenInCodeTabRequest): void { + batch(() => { + useRightPanel().openCodeAt(req.targetMode); + setPending(req); + }); +} diff --git a/packages/client/src/right-panel/useRightPanel.ts b/packages/client/src/right-panel/useRightPanel.ts index 5a408760..7a3a8118 100644 --- a/packages/client/src/right-panel/useRightPanel.ts +++ b/packages/client/src/right-panel/useRightPanel.ts @@ -42,18 +42,26 @@ export function useRightPanel() { ...(mode !== undefined && { codeMode: mode }), }, }), - /** Atomic "open the code browser at this file" — uncollapse the - * panel, switch to Code, force browse mode. Single patch so the - * UI ticks once instead of three times when callers need all - * three transitions together. */ - openCodeBrowser: () => + /** Atomic "open the Code tab at `mode`" — uncollapse the panel, + * switch to Code, set the requested sub-mode. Single preferences + * patch so the UI ticks once instead of three times when callers + * need all three transitions together. Skips the patch when the + * panel is already in the target state (every diff→browse and + * browse→browse `openInCodeTab` would otherwise round-trip a + * three-field preferences write to the server). */ + openCodeAt: (mode: CodeTabView) => { + const cur = rp(); + if (!cur.collapsed && cur.activeTab === "code" && cur.codeMode === mode) { + return; + } updatePreferences({ rightPanel: { collapsed: false, activeTab: "code", - codeMode: "browse", + codeMode: mode, }, - }), + }); + }, /** Change the sub-mode within the Code tab. */ setCodeMode: (mode: CodeTabView) => updatePreferences({ rightPanel: { codeMode: mode } }), diff --git a/packages/client/src/terminal/Terminal.tsx b/packages/client/src/terminal/Terminal.tsx index 3312ad26..fe7be8d1 100644 --- a/packages/client/src/terminal/Terminal.tsx +++ b/packages/client/src/terminal/Terminal.tsx @@ -41,8 +41,7 @@ import { streamCall } from "@kolu/surface/solid"; import { client } from "../wire"; import { isExpectedCleanupError } from "../rpc/streamCleanup"; import { createScrollLock } from "../scrollLock"; -import { requestCodeOpen } from "../right-panel/codeNavigation"; -import { useRightPanel } from "../right-panel/useRightPanel"; +import { openInCodeTab } from "../right-panel/openInCodeTab"; import { preferences } from "../wire"; import { isTouch } from "../useMobile"; import { createFileRefLinkProvider } from "./fileRefLinkProvider"; @@ -163,7 +162,6 @@ const Terminal: Component<{ const [searchAddon, setSearchAddon] = createSignal(null); const scrollLock = createScrollLock(() => preferences().scrollLock); const terminalStore = useTerminalStore(); - const rightPanel = useRightPanel(); let fitRaf = 0; /** Debounce fit() to one call per animation frame — ResizeObserver fires rapidly. */ @@ -471,16 +469,7 @@ const Terminal: Component<{ const meta = terminalStore.getMetadata(props.terminalId); const repoRoot = meta?.git?.repoRoot ?? null; if (!repoRoot) return; - // Issue both writes in the same DOM-event tick: the - // panel-mode change and the click request invalidate - // CodeTab's `resetKey` and `pendingCodeOpen` effects - // together, and Solid runs them in registration order - // (resetKey clears, then pendingCodeOpen sets), so - // `selectedPath` lands on the click's target. Moving - // `openCodeBrowser` inside CodeTab's effect would - // invert that ordering and null the path. - rightPanel.openCodeBrowser(); - requestCodeOpen({ + openInCodeTab({ ref, repoRoot, cwd: meta?.cwd, diff --git a/packages/client/src/ui/CodeContextMenu.tsx b/packages/client/src/ui/CodeContextMenu.tsx index bb76619a..0fc864f9 100644 --- a/packages/client/src/ui/CodeContextMenu.tsx +++ b/packages/client/src/ui/CodeContextMenu.tsx @@ -7,14 +7,32 @@ * line selection — items can return null to omit themselves. */ import { type Component, createSignal, For, onCleanup, Show } from "solid-js"; -import { Portal } from "solid-js/web"; +import { Dynamic, Portal } from "solid-js/web"; import { toast } from "solid-sonner"; +import { match } from "ts-pattern"; -export type CodeContextMenuItem = { - label: string; - /** Returned text gets copied; success toast names the item. */ - textToCopy: string; -}; +/** Two verbs over the same selection noun: copy a string to the clipboard, + * or invoke an action callback. The discriminator keeps the dispatch + * explicit so adding a third verb (e.g. "share") doesn't tempt the + * handler into reading every field on every item. Every item carries + * a leading icon — verbs like "Copy" and "Open" share the same label + * shape (` path:N`) and disambiguate poorly when only the first + * word differs; the glyph makes the verb readable at a glance. */ +export type CodeContextMenuItem = + | { + kind: "copy"; + label: string; + icon: Component<{ class?: string }>; + /** Text written to the clipboard; success toast names the item. */ + textToCopy: string; + } + | { + kind: "action"; + label: string; + icon: Component<{ class?: string }>; + /** Fired on click. The item is closed regardless of completion. */ + onActivate: () => void; + }; export type CodeContextMenuController = { /** Bind to a host element's `oncontextmenu`. */ @@ -67,10 +85,15 @@ export const CodeContextMenu: Component<{ }); const handleItem = (item: CodeContextMenuItem) => { - navigator.clipboard - .writeText(item.textToCopy) - .then(() => toast.success(`Copied: ${item.textToCopy}`)) - .catch((err: Error) => toast.error(`Failed to copy: ${err.message}`)); + match(item) + .with({ kind: "copy" }, ({ textToCopy }) => { + navigator.clipboard + .writeText(textToCopy) + .then(() => toast.success(`Copied: ${textToCopy}`)) + .catch((err: Error) => toast.error(`Failed to copy: ${err.message}`)); + }) + .with({ kind: "action" }, ({ onActivate }) => onActivate()) + .exhaustive(); close(); }; @@ -88,10 +111,14 @@ export const CodeContextMenu: Component<{ )} diff --git a/packages/client/src/ui/Icons.tsx b/packages/client/src/ui/Icons.tsx index bb18d7fa..47187d10 100644 --- a/packages/client/src/ui/Icons.tsx +++ b/packages/client/src/ui/Icons.tsx @@ -126,6 +126,39 @@ export const DiffBranchIcon: Component<{ class?: string }> = (props) => ( ); +export const CopyIcon: Component<{ class?: string }> = (props) => ( + +); + +export const OpenIcon: Component<{ class?: string }> = (props) => ( + +); + export const CloseIcon: Component<{ class?: string }> = (props) => ( void; /** Bind to ``. Returns "Copy path" plus, when - * a line is selected, "Copy :" with the rendered ref. */ + * a line is selected, "Copy :" with the rendered ref — + * and, when `options.onOpen` is provided, an "Open :" + * action entry that dispatches to the host. */ buildItems: () => CodeContextMenuItem[]; }; @@ -34,6 +37,14 @@ export interface LineSelectionOptions { * effect below pushes it into the controller's range, which means * the right-click menu and the Pierre highlight stay in sync. */ initialRange?: Accessor; + /** Accessor — re-read at `buildItems()` time. Returning a handler + * adds an "Open :" entry to the menu while a range is + * selected; returning undefined omits it. Accessor (not bare + * callback) so a host whose prop arrives later in the lifecycle + * still flips the item in once it's available. The host is + * responsible for routing the ref through the Code-tab open + * dispatcher. */ + onOpen?: Accessor<((ref: LineRef) => void) | undefined>; } export function useLineSelection( @@ -61,13 +72,40 @@ export function useLineSelection( range, handleSelect: (r) => setRange(r), buildItems: () => { + // The menu is line-anchored: every entry refers to a specific + // `path:line` ref, so a build without a selected range returns + // empty. ``'s `open()` short-circuits on empty + // items, which means the right-click on whitespace produces no + // menu — the path-only menu was actionable noise (just one + // "Copy path" item, redundant with the in-menu Copy entries + // that ALSO copy the path bundled with the line). + const r = range(); + if (!r) return []; + // Snapshot `path()` once so label, copy text, and onActivate + // capture all agree. Reading the accessor inside `onActivate` + // would resolve at click time and could disagree with the + // rendered label if Pierre remounts to a different file between + // menu-open and click. + const p = path(); + const refStr = formatLineRef(p, r.start, r.end); const items: CodeContextMenuItem[] = [ - { label: "Copy path", textToCopy: path() }, + { kind: "copy", label: "Copy path", icon: CopyIcon, textToCopy: p }, + { + kind: "copy", + label: `Copy ${refStr}`, + icon: CopyIcon, + textToCopy: refStr, + }, ]; - const r = range(); - if (r) { - const ref = formatLineRef(path(), r.start, r.end); - items.push({ label: `Copy ${ref}`, textToCopy: ref }); + const onOpen = options.onOpen?.(); + if (onOpen) { + items.push({ + kind: "action", + label: `Open ${refStr}`, + icon: OpenIcon, + onActivate: () => + onOpen({ path: p, startLine: r.start, endLine: r.end }), + }); } return items; }, diff --git a/packages/solid-pierre/src/FileTree.tsx b/packages/solid-pierre/src/FileTree.tsx index 86734d86..dce74dca 100644 --- a/packages/solid-pierre/src/FileTree.tsx +++ b/packages/solid-pierre/src/FileTree.tsx @@ -28,6 +28,23 @@ type FileTreeOptions = ConstructorParameters[0]; type Composition = NonNullable; type FileTreeContextMenu = NonNullable; +/** Directory paths that contain `path`, formatted with the trailing + * slash Pierre uses for folder keys (`src/`, `src/right-panel/`). + * Tolerates an input that already carries a trailing slash (folder + * path) by stripping it before splitting. Mirrors the shape Pierre's + * internal `getAncestorDirectoryPaths` walks so the result can be + * fed back as `initialExpandedPaths` without surprises. */ +export function ancestorDirectoryPaths(path: string): string[] { + const normalized = path.endsWith("/") ? path.slice(0, -1) : path; + if (normalized.length === 0) return []; + const segments = normalized.split("/").filter(Boolean); + const out: string[] = []; + for (let i = 1; i < segments.length; i += 1) { + out.push(`${segments.slice(0, i).join("/")}/`); + } + return out; +} + export type FileTreeProps = { paths: string[]; gitStatus?: GitStatusEntry[]; @@ -80,12 +97,33 @@ export const FileTree: Component = (props) => { // membership in this set is a reliable file-vs-folder discriminator. const fileSet = createMemo(() => new Set(props.paths)); + // Known limitation: Pierre's vanilla `FileTree` doesn't expose a + // public `scrollToPath`. Its `scrollFocusedRowIntoView` (in + // `dist/render/FileTreeView.js`) is gated on `shouldOwnDomFocus`, + // which is set only by user-initiated handlers (row click, keyboard + // nav). Programmatic selection via `initialSelectedPaths` / + // `getItem(path)?.select()` marks the row `aria-selected="true"` but + // does NOT scroll the virtualizer to reveal it. For small trees the + // row happens to sit in the visible window; for large worktrees the + // selected row stays virtualized off-screen until the user scrolls. + // Filed upstream: https://github.com/pierrecomputer/pierre/issues/676 onMount(() => { try { + // Snapshot read of `props.selectedPath` for `initialExpandedPaths`. + // The deferred resetPaths effect below reads it reactively for + // subsequent changes — Pierre doesn't expose a hook to re-feed + // `initialExpandedPaths` after the constructor, so initial and + // reactive paths are unavoidably two sites. + const selectedAncestors = props.selectedPath + ? ancestorDirectoryPaths(props.selectedPath) + : []; tree = new FileTreeClass({ paths: props.paths, initialExpansion: props.initialExpansion ?? "closed", - initialExpandedPaths: props.expandPaths, + initialExpandedPaths: [ + ...(props.expandPaths ?? []), + ...selectedAncestors, + ], flattenEmptyDirectories: props.flattenEmptyDirectories ?? true, stickyFolders: props.stickyFolders ?? true, icons: props.icons, @@ -110,15 +148,28 @@ export const FileTree: Component = (props) => { // `resetPaths` takes the new path inventory and the directories to // open in one call (Pierre's `FileTreeResetOptions.initialExpandedPaths`). - // Tracking both inputs in the same effect means a paths-and-ancestors + // Tracking the inputs in the same effect means a paths-and-ancestors // swap lands atomically — no second effect, no ordering invariant - // between "rebuild tree" and "open ancestors". + // between "rebuild tree" and "open ancestors". The selected path's + // ancestors are merged in too: when an external caller drives selection + // (e.g. a terminal `path:line` click resolving into a nested file), + // the parents must be expanded for the row to be visible. Pierre's + // public surface doesn't expose `expandDirectory` directly, so the + // expand-on-select is routed through this same `resetPaths` call. createEffect( on( - [() => props.paths, () => props.expandPaths], - ([paths, expandPaths]) => { + [ + () => props.paths, + () => props.expandPaths, + () => props.selectedPath ?? null, + ], + ([paths, expandPaths, selectedPath]) => { try { - tree?.resetPaths(paths, { initialExpandedPaths: expandPaths }); + const ancestors = selectedPath + ? ancestorDirectoryPaths(selectedPath) + : []; + const expanded = [...(expandPaths ?? []), ...ancestors]; + tree?.resetPaths(paths, { initialExpandedPaths: expanded }); } catch (e) { props.onError(toError(e)); } diff --git a/packages/solid-pierre/src/index.ts b/packages/solid-pierre/src/index.ts index 9254b353..7edb3f2c 100644 --- a/packages/solid-pierre/src/index.ts +++ b/packages/solid-pierre/src/index.ts @@ -3,7 +3,7 @@ * and routes Pierre throws through a required `onError` prop so silent * failures can't escape into a blank pane. */ -export { FileTree } from "./FileTree"; +export { ancestorDirectoryPaths, FileTree } from "./FileTree"; export type { FileTreeProps } from "./FileTree"; export { FileDiff } from "./FileDiff"; export type { FileDiffProps } from "./FileDiff"; diff --git a/packages/tests/features/code-tab.feature b/packages/tests/features/code-tab.feature index a5a8bf9e..b7719a94 100644 --- a/packages/tests/features/code-tab.feature +++ b/packages/tests/features/code-tab.feature @@ -288,7 +288,7 @@ Feature: Code tab (review + browse) # ── Pierre file/diff viewer right-click menu (Copy path:line) ── - Scenario: Right-click on file content with a selected line copies "path:line" + Scenario: Right-click on a file content line copies "path:line" When I run "git init /tmp/kolu-browse-ctx && cd /tmp/kolu-browse-ctx" And I run "printf 'alpha\nbeta\ngamma\n' > letters.txt" And I run "git add . && git commit -m init" @@ -296,8 +296,7 @@ Feature: Code tab (review + browse) And I click the Code tab mode "browse" When I click the file "letters.txt" in the file browser Then the file content should contain "beta" - When I click the line number 2 in the file content - And I right-click the file content + When I right-click line 2 in the file content And I click the context menu item "Copy letters.txt:2" Then the clipboard should contain "letters.txt:2" @@ -322,19 +321,38 @@ Feature: Code tab (review + browse) And the Code tab should list a changed file "file-b.txt" When I click the changed file "file-a.txt" in the Code tab Then the diff view should contain "a-one" - When I click the line number 1 in the diff view - And I right-click the diff view + When I right-click line 1 in the diff view And I click the context menu item "Copy file-a.txt:1" Then the clipboard should contain "file-a.txt:1" When I click the changed file "file-b.txt" in the Code tab Then the diff view should contain "b-one" - When I click the line number 1 in the diff view - And I right-click the diff view - Then the context menu items should be "Copy path | Copy file-b.txt:1" + When I right-click line 1 in the diff view + Then the context menu items should be "Copy path | Copy file-b.txt:1 | Open file-b.txt:1" When I click the context menu item "Copy file-b.txt:1" Then the clipboard should contain "file-b.txt:1" And the clipboard should not contain "file-a.txt" + # ── Right-click "Open" jumps from diff to full file (#881 phase 0) ── + # Reviewing a diff and wanting full-file context at the same line was + # previously two manual steps: copy `path:N`, switch to browse mode, + # paste-navigate. The "Open path:N" context-menu entry dispatches via + # the same `openInCodeTab` front door the terminal-link click uses. + Scenario: Right-click "Open path:N" in diff view jumps to browse at that line + When I run "git init /tmp/kolu-open-from-diff && cd /tmp/kolu-open-from-diff" + And I run "git commit --allow-empty -m init" + And I run "mkdir -p docs && printf 'first\nsecond\nthird\n' > docs/notes.txt" + And I click the Code tab + Then the Code tab should list a changed file "docs/notes.txt" + When I click the changed file "docs/notes.txt" in the Code tab + Then the diff view should contain "second" + When I right-click line 2 in the diff view + And I click the context menu item "Open docs/notes.txt:2" + Then the Code tab mode should be "browse" + And the selected file should show content "second" + And line 2 should be selected in the file content + And the file browser should show a file "docs/notes.txt" + And the file "docs/notes.txt" should be selected in the file browser + # ── Live updates: filesystem changes propagate without manual refresh ── # The Code view subscribes to a watcher that observes four axes (HEAD, # reflog, index, working tree) and pushes snapshot updates whenever any diff --git a/packages/tests/step_definitions/code_tab_steps.ts b/packages/tests/step_definitions/code_tab_steps.ts index d8d5e948..9eb746e6 100644 --- a/packages/tests/step_definitions/code_tab_steps.ts +++ b/packages/tests/step_definitions/code_tab_steps.ts @@ -114,13 +114,19 @@ When( // diff viewer (`DIFF_VIEW`) wrap the same Pierre primitive, so the // gutter selector and mouse dance are identical — only the host // element's CSS root changes. -async function clickLineGutterIn(world: KoluWorld, root: string, line: number) { +// +// Poll the bounding box because Pierre's `VirtualizedFileDiff` is keyed +// on path; switching files makes the element pass `waitFor(visible)` +// and then return a null bounding box on the very next call as the +// virtualizer re-measures. +async function interactWithGutterLine( + world: KoluWorld, + root: string, + line: number, + button: "left" | "right", +): Promise { const lineEl = world.page.locator(`${root} [data-column-number="${line}"]`); await lineEl.first().waitFor({ state: "visible", timeout: POLL_TIMEOUT }); - // Switching files re-mounts Pierre's `VirtualizedFileDiff` (FileDiff - // is keyed on path), so the line element can pass `waitFor(visible)` - // and then return a null bounding box on the very next call as the - // virtualizer re-measures. Poll until the box is stable. const box = await pollFor({ observe: () => lineEl.first().boundingBox(), isDone: (b) => !!b && b.width > 0 && b.height > 0, @@ -133,39 +139,42 @@ async function clickLineGutterIn(world: KoluWorld, root: string, line: number) { }); if (!box) throw new Error("unreachable: pollFor returned without box"); await world.page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); - await world.page.mouse.down(); - await world.page.mouse.up(); - await world.waitForFrame(); -} - -async function rightClickViewRoot(world: KoluWorld, root: string) { - const view = world.page.locator(root); - await view.waitFor({ state: "visible", timeout: POLL_TIMEOUT }); - await view.click({ button: "right" }); + await world.page.mouse.down({ button }); + await world.page.mouse.up({ button }); await world.waitForFrame(); } When( "I click the line number {int} in the file content", async function (this: KoluWorld, line: number) { - await clickLineGutterIn(this, FILE_VIEW, line); + await interactWithGutterLine(this, FILE_VIEW, line, "left"); }, ); -When("I right-click the file content", async function (this: KoluWorld) { - await rightClickViewRoot(this, FILE_VIEW); -}); +// Right-click on a gutter line: `CodeMenuFrame`'s contextmenu handler +// reads the line from `event.composedPath()` and opens a 3-item menu +// scoped to that line. Selection + menu-open in one gesture. When( - "I click the line number {int} in the diff view", + "I right-click line {int} in the diff view", async function (this: KoluWorld, line: number) { - await clickLineGutterIn(this, DIFF_VIEW, line); + await interactWithGutterLine(this, DIFF_VIEW, line, "right"); }, ); -When("I right-click the diff view", async function (this: KoluWorld) { - await rightClickViewRoot(this, DIFF_VIEW); -}); +When( + "I right-click line {int} in the file content", + async function (this: KoluWorld, line: number) { + await interactWithGutterLine(this, FILE_VIEW, line, "right"); + }, +); + +When( + "I click the line number {int} in the diff view", + async function (this: KoluWorld, line: number) { + await interactWithGutterLine(this, DIFF_VIEW, line, "left"); + }, +); // Pierre marks selected gutter + content rows with `data-selected-line` // (see @pierre/diffs InteractionManager.renderSelectedLines). The gutter @@ -363,6 +372,22 @@ Then( }, ); +// Pierre marks selected rows with `aria-selected="true"` (and a boolean +// `data-item-selected` that may serialize as `""` or `"true"` depending +// on the renderer — `aria-selected` is the reliable string form). The +// row must also be VISIBLE — collapsed-ancestor descendants fail +// `state: "visible"` even when marked selected, so this step implicitly +// verifies ancestor expansion too. +Then( + "the file {string} should be selected in the file browser", + async function (this: KoluWorld, path: string) { + const item = this.page.locator( + `${TREE} [data-item-path="${path}"][data-item-type="file"][aria-selected="true"]:not([data-file-tree-sticky-row])`, + ); + await item.waitFor({ state: "visible", timeout: POLL_TIMEOUT }); + }, +); + Then( "the Code tab content should show the select hint {string}", async function (this: KoluWorld, expected: string) {