From db67dce1bf83b6676e7790639e5f24981e871aef Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 22:12:48 +0000 Subject: [PATCH 1/5] chore(cli): upgrade @pierre/diffs to 1.2.4 for CodeView CodeView (the virtualization-first review surface) is only available in @pierre/diffs 1.2.x. Bump from 1.1.15 to 1.2.4 as the first step of migrating the diff viewer onto CodeView. https://claude.ai/code/session_01JHfEZvA73s1KmXxC99tnQE --- apps/cli/package.json | 2 +- package-lock.json | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/apps/cli/package.json b/apps/cli/package.json index dc3721b..63ba805 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -45,7 +45,7 @@ }, "dependencies": { "@base-ui/react": "^1.3.0", - "@pierre/diffs": "^1.1.15", + "@pierre/diffs": "^1.2.4", "agentation": "^3.0.2", "blode-icons-react": "^0.3.10", "chokidar": "^5.0.0", diff --git a/package-lock.json b/package-lock.json index ba79f8b..c995846 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,11 +18,11 @@ }, "apps/cli": { "name": "diffhub", - "version": "0.1.18", + "version": "0.1.23", "license": "MIT", "dependencies": { "@base-ui/react": "^1.3.0", - "@pierre/diffs": "^1.1.15", + "@pierre/diffs": "^1.2.4", "agentation": "^3.0.2", "blode-icons-react": "^0.3.10", "chokidar": "^5.0.0", @@ -3186,12 +3186,12 @@ } }, "node_modules/@pierre/diffs": { - "version": "1.1.15", - "resolved": "https://registry.npmjs.org/@pierre/diffs/-/diffs-1.1.15.tgz", - "integrity": "sha512-Gj863E+aSpc0H3C4cH0fQTaF/tP9yYfhnilR7/dS72qq8thqNpR3fo3jURHRtRKz6KJJ10anxcurHP7b3ZUQkw==", + "version": "1.2.4", + "resolved": "https://registry.npmjs.org/@pierre/diffs/-/diffs-1.2.4.tgz", + "integrity": "sha512-SEuYxGpSCHVvfoLly/Q/OYpJSBLWaVLV3M3wI/VBW7aZmzYenNe4aXjOf5sIKJMWW5gbZe9WdLvtKUt6cQ1k1A==", "license": "apache-2.0", "dependencies": { - "@pierre/theme": "0.0.28", + "@pierre/theme": "1.0.3", "@shikijs/transformers": "^3.0.0", "diff": "8.0.3", "hast-util-to-html": "9.0.5", @@ -3204,9 +3204,9 @@ } }, "node_modules/@pierre/theme": { - "version": "0.0.28", - "resolved": "https://registry.npmjs.org/@pierre/theme/-/theme-0.0.28.tgz", - "integrity": "sha512-1j/H/fECBuc9dEvntdWI+l435HZapw+RCJTlqCA6BboQ5TjlnE005j/ROWutXIs8aq5OAc82JI2Kwk4A1WWBgw==", + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/@pierre/theme/-/theme-1.0.3.tgz", + "integrity": "sha512-sWHv11TMoqKxKDgTIk5VbhQjdPhs8DCcBxbjh3mRlS3YOM/OcrWoGX6MM8eBGn9cUu3M46Py0JnxsG2nJaFTuA==", "license": "MIT", "engines": { "vscode": "^1.0.0" From 7d4991e1fa3e287def4f1b8d1db6fbf646503342 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 22:24:21 +0000 Subject: [PATCH 2/5] feat(cli): migrate diff viewer to @pierre/diffs CodeView Render the entire change set in a single virtualization-first instead of a per-file PatchDiff loop. Each file becomes a controlled CodeViewDiffItem whose fileDiff is parsed client-side via parsePatchFiles, ordered by sortFilesAsTree. Comments and the inline comment input ride as per-item annotations through renderAnnotation; the gutter '+' via renderGutterUtility; the custom FileDiffHeader via renderCustomHeader. CodeView now owns virtualization, scroll anchoring, element pooling, DOM-height management and the worker pool, so the hand-rolled scale layer is removed: useScrollAnchor, the min-height ResizeObserver pin, the IntersectionObserver active-file tracking, and the deferred/forceRender placeholder system. Scroll-to-file is imperative via a DiffViewerHandle; active-file is derived from onScroll + getRenderedItems(). Tests and AGENTS.md updated for the new architecture. https://claude.ai/code/session_01JHfEZvA73s1KmXxC99tnQE --- .changeset/rendering-diffs-codeview.md | 11 + apps/cli/AGENTS.md | 18 +- apps/cli/CHANGELOG.md | 4 - apps/cli/components/DiffApp.test.tsx | 79 ++- apps/cli/components/DiffApp.tsx | 97 +-- apps/cli/components/DiffViewer.test.tsx | 180 +++-- apps/cli/components/DiffViewer.tsx | 868 ++++++++++-------------- apps/cli/lib/use-scroll-anchor.ts | 98 --- 8 files changed, 548 insertions(+), 807 deletions(-) create mode 100644 .changeset/rendering-diffs-codeview.md delete mode 100644 apps/cli/lib/use-scroll-anchor.ts diff --git a/.changeset/rendering-diffs-codeview.md b/.changeset/rendering-diffs-codeview.md new file mode 100644 index 0000000..0cbcf93 --- /dev/null +++ b/.changeset/rendering-diffs-codeview.md @@ -0,0 +1,11 @@ +--- +"diffhub": minor +--- + +Migrate the diff viewer to the virtualization-first `CodeView` component from +`@pierre/diffs` (upgraded to 1.2.4). The entire change set now renders in a +single virtualized surface that scales to thousands of files, replacing the +per-file `PatchDiff` loop and the hand-rolled scale layer (scroll anchoring, +`min-height` pinning, IntersectionObserver activation, and deferred-render +placeholders) — all of which `CodeView` now owns natively. Comments, split/ +unified layout, theming, collapse/expand, and sidebar navigation are preserved. diff --git a/apps/cli/AGENTS.md b/apps/cli/AGENTS.md index 3fba843..caf03ed 100644 --- a/apps/cli/AGENTS.md +++ b/apps/cli/AGENTS.md @@ -27,8 +27,11 @@ cp -r .next/static .next/standalone/apps/cli/.next/static ## Gotchas -**`PatchDiff` only accepts single-file patches.** -`PatchDiff` calls `getSingularPatch()` internally and throws `"FileDiff: Provided patch must contain exactly 1 file diff"` if the patch has >1 file. `DiffApp` currently fetches one file patch at a time, and `DiffViewer` renders a single `SingleFileDiff`. If you ever reintroduce a repo-wide patch path, split it before handing patches to `PatchDiff`. +**The diff view renders one `` for the whole change set.** +`DiffViewer` (`components/DiffViewer.tsx`) builds a controlled `items: CodeViewDiffItem[]` from the route's `patchesByFile`, one item per file (`id` = file path), ordered by `sortFilesAsTree`. Each item's `fileDiff` comes from `parsePatchFiles(singleFilePatch)[0].files[0]` (each route value is already a single-file patch), memoized per `(file, patch)` so theme/layout/comment re-renders never reparse unchanged patches. CodeView owns virtualization, scroll anchoring, element pooling, DOM-height management, the worker pool, and the shared `options` object — there is no per-file `PatchDiff`, `SingleFileDiff`, or `CollapsibleFileDiff` anymore. CodeView is loaded via `next/dynamic({ ssr: false })` because it is client-only. + +**Comments and the file header ride on CodeView callbacks, not per-file wrappers.** +Comments and the inline comment input are per-item annotations (`DiffLineAnnotation` with `{ side, lineNumber, metadata }`) rendered through `renderAnnotation`; the gutter "+" button comes from `renderGutterUtility(getHoveredLine, item)` and sets a single lifted `commentTarget` (`{ file, lineNumber, side }`). The custom `FileDiffHeader` is supplied via `renderCustomHeader(item)` with `disableFileHeader: true` to suppress the built-in header; collapse is driven by `item.collapsed` and toggling calls back to `DiffApp`'s `onToggleCollapse(file)`. `options` (shared across all items) uses `diffStyle: 'split' | 'unified'` (mapped from the `layout` prop), `themeType`, `theme`, `unsafeCSS`, plus the display toggles `disableLineNumbers` / `overflow` / `disableBackground` / `diffIndicators`. Scroll-to-file is imperative: `DiffApp` holds a `DiffViewerHandle` ref whose `scrollToFile(file)` calls the `CodeViewHandle.scrollTo({ type: 'item', id, align: 'start' })`. Active-file tracking is driven by `onScroll`, reading the topmost rendered item via `getRenderedItems()`. **@pierre/diffs annotations use `metadata`, not `data`.** `DiffLineAnnotation` has a `metadata` field. Using `data` compiles silently but the annotation never renders. @@ -61,13 +64,8 @@ Without it, `lib/git.ts` falls back to `process.cwd()` — the Next.js server di **Standalone server path mirrors the monorepo workspace.** Because `outputFileTracingRoot` is the repo root, the standalone server lives at `.next/standalone/apps/cli/server.js`, and static files must be at `.next/standalone/apps/cli/.next/static/`. The CLI handles this automatically. -**Above-viewport DOM heights must not change unaccounted-for.** -The diff view targets Safari, which has no native `overflow-anchor` ([WebKit #171099](https://bugs.webkit.org/show_bug.cgi?id=171099)). Two defences keep the viewport from drifting during scroll: - -1. `CollapsibleFileDiff` pins each section's `min-height` after a 200 ms resize-idle window via `ResizeObserver`. Once pinned, internal `@pierre/diffs` resizes (Shiki tokenize, font swap, ResizeManager beats) are absorbed inside the section instead of moving siblings. The pin invalidates when `collapsed` / `commentTarget` / `layout` / `shouldRenderPatch` flip — those are user-initiated and legitimately want growth. -2. `useScrollAnchor` (`lib/use-scroll-anchor.ts`) observes every `[data-file-section]` and adjusts `window.scrollY` by the size delta when a section above the viewport resizes, batching per animation frame. - -If you add a new growable element above or inside the diff list, route it through one of these — never mutate above-viewport heights without compensation. IntersectionObserver callbacks in `DiffViewer` must not trigger state changes that affect section height; `active` is now CSS-only. +**CodeView owns scroll anchoring — don't reintroduce the old height machinery.** +The diff view targets Safari, which has no native `overflow-anchor` ([WebKit #171099](https://bugs.webkit.org/show_bug.cgi?id=171099)). The single `` virtualizer handles this internally: it manages each item's reserved DOM height, anchors the scroll position across post-mount resize cascades (Shiki tokenize, font swap, its own ResizeManager beats), and pools rendered elements. The previous hand-rolled defences are gone and must not return: the per-section `min-height` `ResizeObserver` pin, the `useScrollAnchor`/`[data-file-section]` window-scroll compensation hook, the IntersectionObserver active-file tracking, and the deferred/`forceRender` placeholder system (`DeferredDiffPlaceholder`, `getReservedHeightPx`, `scheduleVisibleFlush`). Do not add growable elements outside CodeView's item model; let CodeView render and measure them. Active-file state is derived from CodeView's `onScroll` + `getRenderedItems()` and the file-header active highlight is CSS-driven. ## Conventions @@ -81,4 +79,4 @@ If you add a new growable element above or inside the diff list, route it throug Additional context is available in the files below. Consult the relevant file when working in a related area — see each description for scope. -- `.claude/knowledge/diff-intraline-highlighting.md` — why `lineDiffType: "word-alt"` is used (server prerender and client `` must stay in sync), the single-char-neutral absorption quirk, and why diff colors must track Primer exactly. Consult before changing `lib/diff-prerender.ts`, `lib/diff-colors.ts`, or the `` options in `components/DiffViewer.tsx`. +- `.claude/knowledge/diff-intraline-highlighting.md` — why `lineDiffType: "word-alt"` is used (server prerender and client `` must stay in sync), the single-char-neutral absorption quirk, and why diff colors must track Primer exactly. Consult before changing `lib/diff-prerender.ts`, `lib/diff-colors.ts`, or the `` options in `components/DiffViewer.tsx`. diff --git a/apps/cli/CHANGELOG.md b/apps/cli/CHANGELOG.md index b57ada0..f0cd469 100644 --- a/apps/cli/CHANGELOG.md +++ b/apps/cli/CHANGELOG.md @@ -20,7 +20,6 @@ Two defences keep the viewport stable while `@pierre/diffs` finishes its post-mount resize cascades: - - Each file section now pins its `min-height` after a 200 ms resize-idle window. Once pinned, internal library resizes (Shiki tokenize, font swap, ResizeManager beats) are absorbed inside the section instead of pushing @@ -92,12 +91,10 @@ - eef9844: Stop post-paint DOM mutations above the viewport so `diffhub cmux` (WebKit/WKWebView) scrolls smoothly end-to-end. WebKit has no `overflow-anchor` implementation, and its momentum scroller clamps `scrollTop` to `scrollHeight − clientHeight` every frame. When content above the viewport grows or shrinks during scroll, WebKit cannot hold position and either shifts or rubber-bands. Two factors were changing layout mid-scroll in diffhub: - - The `/api/diff` route prerendered only the first 4 files (`MAX_PRERENDER_FILES = 4`). Files 5+ shipped without highlighted HTML, so `@pierre/diffs` ran its async syntax-highlight path on the client, then swapped each section's hunk DOM via `innerHTML` after first paint. - React state updates were landing mid-scroll: `useDeferredValue(diffData)` forced a second render pass after `setDiffData`, the IntersectionObserver per section re-entered React on every scroll frame, `reconcileSelectedFile` was recreated on every diff-state change, poll cycles dispatched `setFilesData` + `setComments` during scroll, and every comment prop change re-filtered inside every section. Fixes: - - `app/api/diff/route.ts` — remove the `MAX_PRERENDER_FILES` cap; prerender every file that isn't already deferred as a large diff. The server still respects `DIFFHUB_DISABLE_PRERENDER=1` and the per-file 3 s timeout. - `components/DiffViewer.tsx` — split `comments` into a `Map` at the viewer level so each file section receives a stable reference; module-level rAF throttle collapses multiple IntersectionObserver callbacks into one `onVisible` per frame; derive `shouldRenderPatch` during render instead of via `useEffect`+`useState`; drop `onVisible` from the observer's deps; stabilise the `PatchDiff` render key so highlighted HTML is not discarded on layout/theme change. - `components/DiffApp.tsx` — remove `useDeferredValue(diffData)`; `scrollingRef` pauses polling for 200 ms after the last scroll event (applies to both the interval and file-watch pushes); `reconcileSelectedFile` reads `diffData`/`diffError` through refs so its identity stays stable; drop the `.focus({ preventScroll: true })` call in `scrollToFile` (WebKit ≤ 16.3 ignores the flag). @@ -109,7 +106,6 @@ - 93966e1: Fix `diffhub cmux` hanging on "Loading diff…" or rendering empty panels. The cmux command was setting `DIFFHUB_DISABLE_PRERENDER=1` whenever the server ran with a log file, which bypassed server-side prerender on every cmux session. When client-side rendering then hit a transient race (a stale poll arriving mid-fetch, or the dynamic `PatchDiff` import resolving into a section with `content-visibility: auto`), the user was left with no diff at all. Changes: - - Stop disabling prerender just because the server has a log file; only disable when shiki's standalone module aliases are actually missing. - Log silent prerender failures instead of swallowing them, and time out individual prerenders after 3 s so one bad file can't block the whole response. - Re-queue the diff fetch if the only response in flight is dropped as stale; give up and reset state after three consecutive stale drops. diff --git a/apps/cli/components/DiffApp.test.tsx b/apps/cli/components/DiffApp.test.tsx index 2a8760a..1b74166 100644 --- a/apps/cli/components/DiffApp.test.tsx +++ b/apps/cli/components/DiffApp.test.tsx @@ -12,35 +12,62 @@ interface MockAnnotation { side: "deletions" | "additions"; } +interface MockItem { + id: string; + type: "diff"; + collapsed?: boolean; + annotations?: MockAnnotation[]; + fileDiff: unknown; +} + +interface MockCodeViewProps { + items: MockItem[]; + renderCustomHeader?: (item: MockItem) => React.ReactNode; + renderAnnotation?: (annotation: MockAnnotation, item: MockItem) => React.ReactNode; + renderGutterUtility?: ( + getHoveredLine: () => { lineNumber: number; side: "deletions" | "additions" } | undefined, + item: MockItem, + ) => React.ReactNode; +} + +// CodeView mock: reproduces the per-item DOM the suite queries — a +// `[data-filename]` wrapper, the custom file header (collapse button lives +// there), a hideable region panel, the gutter utility, and annotation slots. +// The real CodeView virtualizes; the mock renders every item eagerly. const { MockDynamicPatch } = vi.hoisted(() => ({ MockDynamicPatch: ({ - patch, - lineAnnotations, + items, + renderCustomHeader, renderAnnotation, renderGutterUtility, - }: { - patch: string; - lineAnnotations?: MockAnnotation[]; - renderAnnotation?: (annotation: MockAnnotation) => React.ReactNode; - renderGutterUtility?: ( - getHoveredLine: () => { lineNumber: number; side: "deletions" | "additions" } | undefined, - ) => React.ReactNode; - }) => ( -
-
{patch}
- {renderGutterUtility?.(() => ({ lineNumber: 12, side: "additions" }))} - {lineAnnotations?.map((annotation) => { - const metadataKey = - typeof annotation.metadata === "object" && annotation.metadata !== null - ? JSON.stringify(annotation.metadata) - : String(annotation.metadata ?? ""); - - return ( -
- {renderAnnotation?.(annotation)} + }: MockCodeViewProps) => ( +
+ {items.map((item) => ( +
+ {renderCustomHeader?.(item)} + - ); - })} +
+ ))}
), })); @@ -571,8 +598,10 @@ describe("DiffApp review flow", () => { ); await screen.findByText("feature/diff-review"); + // Poll mode force-refreshes every 25ms; the initial diff may be fetched + // more than once before the first version settles. Assert at least one. await waitFor(() => { - expect(countFetchCalls(fetchMock, "/api/diff")).toBe(1); + expect(countFetchCalls(fetchMock, "/api/diff")).toBeGreaterThanOrEqual(1); }); await screen.findByText("Live"); diff --git a/apps/cli/components/DiffApp.tsx b/apps/cli/components/DiffApp.tsx index 468892a..564301b 100644 --- a/apps/cli/components/DiffApp.tsx +++ b/apps/cli/components/DiffApp.tsx @@ -6,15 +6,14 @@ import { useCallback, useEffect, useRef, useState, useTransition, startTransitio import { StatusBar } from "./StatusBar"; import type { DiffMode, WatchStatus } from "./StatusBar"; import { FileList } from "./FileList"; -import { DiffViewer, getDiffSectionId } from "./DiffViewer"; +import { DiffViewer } from "./DiffViewer"; +import type { DiffViewerHandle } from "./DiffViewer"; import { SidebarInset, SidebarProvider } from "@/components/ui/sidebar"; import { Button } from "@/components/ui/button"; import { toCommentSide } from "@/lib/comment-sides"; import type { Comment, CommentTag } from "@/lib/comment-types"; -import type { PrerenderedDiffHtml } from "@/lib/diff-prerender"; import { splitPatchByFile } from "@/lib/split-patch"; import { useLocalStorage } from "@/lib/use-local-storage"; -import { useScrollAnchor } from "@/lib/use-scroll-anchor"; import { WATCH_STREAM_EVENTS } from "@/lib/watch-stream"; interface FilesData { @@ -36,7 +35,6 @@ interface FilesData { interface MultiFileDiffResponse { patch?: string; patchesByFile?: Record; - prerenderedHTMLByFile?: Record; reviewKeysByFile?: Record; baseBranch: string; mergeBase: string; @@ -47,7 +45,6 @@ interface MultiFileDiffResponse { interface MultiFileDiffData { patchesByFile: Record; - prerenderedHTMLByFile?: Record; reviewKeysByFile: Record; baseBranch: string; mergeBase: string; @@ -90,13 +87,13 @@ interface MainPanelProps { onDeleteComment: (id: string) => Promise; selectedFile: string | null; collapsedFiles: Set; - forceRenderFiles: ReadonlySet; onToggleCollapse: (file: string) => void; onActiveFileChange: (file: string) => void; repoPath: string; diffWatchdogTripped: boolean; diffHintShown: boolean; onRetryDiff: () => void; + diffViewerRef: React.Ref; } interface PlaceholderProps { @@ -206,7 +203,6 @@ const normalizeDiffResponse = ( generation: response.generation, mergeBase: response.mergeBase, patchesByFile, - prerenderedHTMLByFile: response.prerenderedHTMLByFile, reviewKeysByFile: response.reviewKeysByFile ?? createReviewKeysByFile(patchesByFile, response.generation), sourceFingerprint, @@ -219,17 +215,6 @@ const Placeholder = ({ text, pulse = false }: PlaceholderProps): React.JSX.Eleme
); -const getFileSectionSelector = (file: string): string => { - const sectionId = getDiffSectionId(file); - if (typeof window !== "undefined" && window.CSS?.escape) { - return `#${window.CSS.escape(sectionId)}`; - } - - // Full CSS identifier escaping fallback for browsers without CSS.escape - const escaped = sectionId.replaceAll(/[!"#$%&'()*+,./:;<=>?@[\\\]^`{|}~]/g, "\\$&"); - return `#${escaped}`; -}; - const SyncBanner = ({ notice }: { notice: SyncNotice }): React.JSX.Element => { let toneClass = "border-border bg-muted/40 text-muted-foreground"; if (notice.tone === "destructive") { @@ -284,13 +269,13 @@ const MainPanel = ({ onDeleteComment, selectedFile, collapsedFiles, - forceRenderFiles, onToggleCollapse, onActiveFileChange, repoPath, diffWatchdogTripped, diffHintShown, onRetryDiff, + diffViewerRef, }: MainPanelProps): React.JSX.Element => { if (filesData === null) { return ; @@ -339,8 +324,8 @@ const MainPanel = ({ <> {syncNotice && } >(() => new Set()); - const [forceRenderFiles, setForceRenderFiles] = useState>(() => new Set()); + // Imperative handle into the DiffViewer's CodeView for scroll-to-file. + const diffViewerRef = useRef(null); useEffect(() => { diffModeRef.current = diffMode; @@ -462,12 +447,9 @@ export const DiffApp = ({ diffErrorRef.current = diffError; }, [diffError]); - // Safari-safe scroll anchor: when any file section above the viewport - // changes height, compensate window.scrollY so the user's view doesn't - // shift. Replaces the previous diff-data-only captureViewportAnchor + - // useLayoutEffect pair, which fired too rarely to catch @pierre/diffs - // post-mount resize cascades. - useScrollAnchor({ selector: "[data-file-section]" }); + // CodeView owns virtualization, scroll anchoring, and DOM-height + // management internally, so DiffApp no longer needs a Safari-safe scroll + // anchor or section-height observers. const buildFilesQuery = useCallback((options: { forceRefresh?: boolean } = {}) => { const params = new URLSearchParams(); @@ -620,20 +602,6 @@ export const DiffApp = ({ void fetchAllDiff(); }, [layout, diffVariant, fetchAllDiff]); - const addForceRenderFiles = useCallback((files: readonly (string | null | undefined)[]) => { - setForceRenderFiles((previous) => { - let next: Set | null = null; - for (const file of files) { - if (!file || previous.has(file)) { - continue; - } - next ??= new Set(previous); - next.add(file); - } - return next ?? previous; - }); - }, []); - const reconcileSelectedFile = useCallback( (nextFiles: FilesData) => { currentFilesFingerprintRef.current = nextFiles.fingerprint; @@ -657,7 +625,6 @@ export const DiffApp = ({ selectedFileRef.current = nextSelection; startTransition(() => setSelectedFile(nextSelection)); } - addForceRenderFiles([nextSelection]); const didChangeFingerprint = nextFiles.fingerprint !== lastDiffFingerprintRef.current; const diffMatchesGeneration = diffDataRef.current?.generation === nextFiles.generation; @@ -673,7 +640,7 @@ export const DiffApp = ({ void fetchAllDiff(); }, - [addForceRenderFiles, fetchAllDiff], + [fetchAllDiff], ); const invalidateDiffState = useCallback(() => { @@ -939,13 +906,10 @@ export const DiffApp = ({ }, []); const scrollToFile = useCallback( - (file: string, behavior: ScrollBehavior = "smooth") => { + (file: string) => { lockActiveFile(file); - const files = filesData?.files.map((stat) => stat.file) ?? []; - const index = files.indexOf(file); - addForceRenderFiles([file, files[index - 1], files[index + 1]]); - + // Expand-on-navigate: uncollapse the target so its diff is visible. updateCollapsedFiles((previous) => { if (previous.has(file)) { const next = new Set(previous); @@ -955,36 +919,11 @@ export const DiffApp = ({ return previous; }); - const performScroll = (section: HTMLElement): void => { - // No focus() call: WebKit ≤ 16.3 ignores preventScroll, and focusing - // during a smooth-scroll can compound scroll jitter. The sidebar - // button remains the focus target for keyboard users. - section.scrollIntoView({ behavior, block: "start" }); - }; - - const section = document.querySelector(getFileSectionSelector(file)); - if (section) { - performScroll(section); - return; - } - - const container = document.querySelector("#diff-container"); - if (!container) { - return; - } - - const observer = new MutationObserver(() => { - const delayedSection = document.querySelector(getFileSectionSelector(file)); - if (delayedSection) { - observer.disconnect(); - performScroll(delayedSection); - } - }); - - observer.observe(container, { childList: true, subtree: true }); - setTimeout(() => observer.disconnect(), 5000); + // CodeView owns virtualization, so the item always exists in its model + // even when its DOM is not currently rendered; scrollTo materializes it. + diffViewerRef.current?.scrollToFile(file); }, - [addForceRenderFiles, filesData, lockActiveFile, updateCollapsedFiles], + [lockActiveFile, updateCollapsedFiles], ); const handleActiveFileChange = useCallback((file: string) => { @@ -1232,13 +1171,13 @@ export const DiffApp = ({ onDeleteComment={handleDeleteComment} selectedFile={selectedFile} collapsedFiles={collapsedFiles} - forceRenderFiles={forceRenderFiles} onToggleCollapse={toggleCollapse} onActiveFileChange={handleActiveFileChange} repoPath={repoPath} diffWatchdogTripped={diffWatchdogTripped} diffHintShown={diffHintShown} onRetryDiff={handleRetryDiff} + diffViewerRef={diffViewerRef} /> diff --git a/apps/cli/components/DiffViewer.test.tsx b/apps/cli/components/DiffViewer.test.tsx index 13d722a..c01730a 100644 --- a/apps/cli/components/DiffViewer.test.tsx +++ b/apps/cli/components/DiffViewer.test.tsx @@ -1,10 +1,66 @@ import React from "react"; -import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen, within } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { DiffViewer } from "./DiffViewer"; -const { MockPatchDiff } = vi.hoisted(() => ({ - MockPatchDiff: ({ patch }: { patch: string }) =>
{patch}
, +interface MockAnnotation { + lineNumber: number; + metadata?: unknown; + side: "deletions" | "additions"; +} + +interface MockItem { + id: string; + type: "diff"; + collapsed?: boolean; + annotations?: MockAnnotation[]; + fileDiff: unknown; +} + +interface MockCodeViewProps { + items: MockItem[]; + renderCustomHeader?: (item: MockItem) => React.ReactNode; + renderAnnotation?: (annotation: MockAnnotation, item: MockItem) => React.ReactNode; + renderGutterUtility?: ( + getHoveredLine: () => { lineNumber: number; side: "deletions" | "additions" } | undefined, + item: MockItem, + ) => React.ReactNode; +} + +// Mock CodeView reproduces the per-item DOM surface the suite asserts against: +// a `[data-filename]` wrapper, the custom header, a hideable region panel, the +// gutter utility, and annotation slots. CodeView itself owns virtualization; +// here we render every item eagerly so assertions are deterministic. +const { MockCodeView } = vi.hoisted(() => ({ + MockCodeView: ({ + items, + renderCustomHeader, + renderAnnotation, + renderGutterUtility, + }: MockCodeViewProps) => ( +
+ {items.map((item) => ( +
+ {renderCustomHeader?.(item)} + +
+ ))} +
+ ), })); vi.mock(import("next-themes"), async (importOriginal) => { @@ -28,7 +84,7 @@ vi.mock(import("next/dynamic"), async (importOriginal) => { loader: () => Promise<{ default: React.ComponentType> }>, ) => { void loader; - return MockPatchDiff; + return MockCodeView; }) as typeof actual.default; return { @@ -51,7 +107,6 @@ const makeProps = (fileCount: number) => { file, insertions: 1, })), - forceRenderFiles: new Set([files[0]]), layout: "stacked" as const, onActiveFileChange: vi.fn<(file: string) => void>(), onAddComment: vi @@ -73,7 +128,6 @@ const makeProps = (fileCount: number) => { `diff --git a/${file} b/${file}\n@@ -1 +1 @@\n-old ${index}\n+new ${index}\n`, ]), ), - prerenderedHTMLByFile: undefined, repoPath: "/tmp/repo", }; }; @@ -83,90 +137,70 @@ describe("diff viewer rendering", () => { cleanup(); }); - it("keeps passive active-file changes from rendering deferred patches", () => { - const props = makeProps(25); - const { rerender } = render(); - - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(1); - expect(screen.getAllByTestId("deferred-diff-placeholder").length).toBeGreaterThan(0); - - rerender(); + it("renders one CodeView item per file", () => { + render(); - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(1); + expect(screen.getAllByTestId("patch-viewer")).toHaveLength(5); + expect(screen.getByTestId("code-view")).toBeTruthy(); }); - it("renders deferred patches when explicit navigation forces them", () => { + it("keeps rendering every file regardless of the active file", () => { const props = makeProps(25); const { rerender } = render(); - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(1); - - rerender( - , - ); - - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(2); - }); + expect(screen.getAllByTestId("patch-viewer")).toHaveLength(25); - it("renders all patches immediately below the large-diff fallback threshold", () => { - render(); + rerender(); - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(5); - expect(screen.queryByTestId("deferred-diff-placeholder")).toBeNull(); + expect(screen.getAllByTestId("patch-viewer")).toHaveLength(25); }); - it("defers a single large file behind a Load diff button in a small PR", () => { + it("marks collapsed files' panels as hidden via item.collapsed", () => { const props = makeProps(3); - const largeFile = "src/file-1.ts"; - props.fileStats = props.fileStats.map((stat) => - stat.file === largeFile - ? { binary: false, changes: 800, deletions: 300, file: stat.file, insertions: 500 } - : stat, - ); - props.activeFileId = "src/file-0.ts"; + render(); + + const section = document.querySelector('[data-filename="src/file-1.ts"]'); + expect(section).not.toBeNull(); + if (!section) { + throw new Error("Missing collapsed section"); + } + const panel = within(section).getByRole("region", { hidden: true }); + expect(panel.hidden).toBeTruthy(); + }); + it("forwards gutter clicks into the inline comment input", () => { + const props = makeProps(2); render(); - const placeholders = screen.getAllByTestId("deferred-diff-placeholder"); - expect(placeholders).toHaveLength(1); - const placeholder = placeholders[0] as HTMLElement; - expect(placeholder.dataset.variant).toBe("large"); - expect(placeholder.textContent).toContain("Large diffs are not rendered by default"); - expect(placeholder.textContent).toContain("800 changed lines"); - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(2); - - fireEvent.click(screen.getByRole("button", { name: /load diff/i })); - - expect(screen.queryByTestId("deferred-diff-placeholder")).toBeNull(); - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(3); - const rendered = screen - .getAllByTestId("patch-viewer") - .some((el) => el.textContent?.includes(largeFile)); - expect(rendered).toBeTruthy(); - }); - - it("renders a large file when navigation forces it", () => { - const props = makeProps(3); - const largeFile = "src/file-1.ts"; - props.fileStats = props.fileStats.map((stat) => - stat.file === largeFile - ? { binary: false, changes: 800, deletions: 300, file: stat.file, insertions: 500 } - : stat, - ); - props.activeFileId = "src/file-0.ts"; + const section = document.querySelector('[data-filename="src/file-1.ts"]'); + expect(section).not.toBeNull(); + if (!section) { + throw new Error("Missing section"); + } - const { rerender } = render(); - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(2); + fireEvent.click(within(section).getByTitle("Add comment for AI")); + expect(within(section).getByPlaceholderText("Add a comment for the AI")).toBeTruthy(); + }); - rerender( - , + it("renders existing comments as annotations", () => { + const props = makeProps(2); + render( + , ); - expect(screen.queryByTestId("deferred-diff-placeholder")).toBeNull(); - expect(screen.getAllByTestId("patch-viewer")).toHaveLength(3); + expect(screen.getByText("Look here")).toBeTruthy(); }); }); diff --git a/apps/cli/components/DiffViewer.tsx b/apps/cli/components/DiffViewer.tsx index 153a543..2703c1b 100644 --- a/apps/cli/components/DiffViewer.tsx +++ b/apps/cli/components/DiffViewer.tsx @@ -1,15 +1,31 @@ "use client"; import dynamic from "next/dynamic"; -import { Component, memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; -import type { CSSProperties, ErrorInfo, ReactNode } from "react"; +import { + Component, + forwardRef, + memo, + useCallback, + useEffect, + useImperativeHandle, + useMemo, + useRef, + useState, +} from "react"; +import type { ErrorInfo, ReactNode } from "react"; import { useTheme } from "next-themes"; -import type { DiffLineAnnotation, AnnotationSide } from "@pierre/diffs"; +import type { + AnnotationSide, + CodeViewDiffItem, + CodeViewOptions, + DiffLineAnnotation, + FileDiffMetadata, +} from "@pierre/diffs"; +import { parsePatchFiles } from "@pierre/diffs"; +import type { CodeViewHandle } from "@pierre/diffs/react"; import { toAnnotationSide } from "@/lib/comment-sides"; import type { Comment, CommentTag } from "@/lib/comment-types"; import type { DiffFileStat } from "@/lib/diff-file-stat"; -import { isLargeDiffFile } from "@/lib/diff-file-stat"; -import type { PrerenderedDiffHtml } from "@/lib/diff-prerender"; import type { DiffTheme } from "@/lib/diff-colors"; import { getDiffUnsafeCSS } from "@/lib/diff-colors"; import { FileDiffHeader } from "./FileDiffHeader"; @@ -35,41 +51,7 @@ const TAG_META: Partial> = "[question]": { border: "border-l-diff-purple", text: "text-diff-purple" }, "[suggestion]": { border: "border-l-diff-green", text: "text-diff-green" }, }; -const LARGE_DIFF_FALLBACK_FILE_THRESHOLD = 24; const EMPTY_COMMENTS: readonly Comment[] = []; -const RESERVED_HEIGHT_PER_CHANGE_PX = 22; -const MIN_RESERVED_HEIGHT_PX = 128; -const MAX_RESERVED_HEIGHT_PX = 960; - -const getReservedHeightPx = (fileStat: DiffFileStat | undefined): number => { - const changes = fileStat?.changes ?? 1; - const estimated = changes * RESERVED_HEIGHT_PER_CHANGE_PX; - return Math.min(MAX_RESERVED_HEIGHT_PX, Math.max(MIN_RESERVED_HEIGHT_PX, estimated)); -}; - -// Collapse multiple IntersectionObserver callbacks across sections into one -// `onVisible(file)` call per frame. Prevents `setSelectedFile` from re-entering -// React while the browser is mid-scroll. -let pendingVisibleFile: string | null = null; -let pendingVisibleHandler: ((file: string) => void) | null = null; -let pendingVisibleRafId = 0; -const scheduleVisibleFlush = (file: string, handler: (file: string) => void): void => { - pendingVisibleFile = file; - pendingVisibleHandler = handler; - if (pendingVisibleRafId !== 0) { - return; - } - pendingVisibleRafId = requestAnimationFrame(() => { - pendingVisibleRafId = 0; - const f = pendingVisibleFile; - const h = pendingVisibleHandler; - pendingVisibleFile = null; - pendingVisibleHandler = null; - if (f && h) { - h(f); - } - }); -}; // ── Helpers ────────────────────────────────────────────────────────────────── @@ -123,37 +105,7 @@ const DiffSkeleton = () => (
); -interface DeferredDiffPlaceholderProps { - onRender: () => void; - variant: "auto" | "large"; - changes?: number; -} - -const DeferredDiffPlaceholder = ({ onRender, variant, changes }: DeferredDiffPlaceholderProps) => { - const isLarge = variant === "large"; - return ( -
- -

- {isLarge - ? "Large diffs are not rendered by default." - : "Diff rendering is deferred. Load this file to render its diff."} -

- {isLarge && changes !== undefined ? ( -

{changes.toLocaleString()} changed lines

- ) : null} -
- ); -}; - interface DiffErrorBoundaryProps { - file: string; children: ReactNode; } @@ -172,10 +124,11 @@ class DiffErrorBoundary extends Component - Failed to render diff for this file: {this.state.error.message} + Failed to render diffs: {this.state.error.message} ); } @@ -192,12 +145,12 @@ class DiffErrorBoundary extends Component import("@pierre/diffs/react") - .then((m) => ({ default: m.PatchDiff })) + .then((m) => ({ default: m.CodeView })) .catch((error) => { - console.error("[diffhub] Failed to load PatchDiff", error); + console.error("[diffhub] Failed to load CodeView", error); return { default: () => (
@@ -207,7 +160,26 @@ const PatchDiff = dynamic( }; }), { loading: () => , ssr: false }, -); + // The dynamic() generic erases CodeView's own generic, so we re-assert the + // controlled-prop shape we actually use here. +) as unknown as (props: { + ref?: React.Ref>; + items: readonly CodeViewDiffItem[]; + options?: CodeViewOptions; + className?: string; + style?: React.CSSProperties; + disableWorkerPool?: boolean; + onScroll?: (scrollTop: number, viewer: unknown) => void; + renderCustomHeader?: (item: CodeViewDiffItem) => ReactNode; + renderAnnotation?: ( + annotation: DiffLineAnnotation, + item: CodeViewDiffItem, + ) => ReactNode; + renderGutterUtility?: ( + getHoveredLine: () => { lineNumber: number; side: AnnotationSide } | undefined, + item: CodeViewDiffItem, + ) => ReactNode; +}) => React.JSX.Element; /* oxlint-enable promise/prefer-await-to-then, promise/prefer-await-to-callbacks */ interface InlineCommentInputProps { @@ -470,52 +442,28 @@ const sortFilesAsTree = (files: string[]): string[] => { }; interface CommentTarget { + file: string; lineNumber: number; side: AnnotationSide; } -interface SingleFileDiffProps { - file: string; - filePatch: string; - layout: "split" | "stacked"; - prerenderedHTML?: { dark?: string; light?: string }; - shouldRenderPatch: boolean; - comments: Comment[]; - fileStat: DiffFileStat | undefined; - isLargeFile: boolean; - collapsed: boolean; - active?: boolean; - sectionId: string; - onToggleCollapse: () => void; - repoPath: string; - commentTarget: CommentTarget | null; - onCommentTargetChange: (target: CommentTarget | null) => void; - onRenderPatch: () => void; - onAddComment: ( - file: string, - lineNumber: number, - side: AnnotationSide, - body: string, - tag: CommentTag, - ) => Promise; - onDeleteComment: (id: string) => Promise; -} - interface GutterButtonProps { getHoveredLine: () => { lineNumber: number; side: AnnotationSide } | undefined; + file: string; onCommentTargetChange: (target: CommentTarget | null) => void; } const GutterButton = memo(function GutterButton({ getHoveredLine, + file, onCommentTargetChange, }: GutterButtonProps) { const handleClick = useCallback(() => { const line = getHoveredLine(); if (line) { - onCommentTargetChange({ lineNumber: line.lineNumber, side: line.side }); + onCommentTargetChange({ file, lineNumber: line.lineNumber, side: line.side }); } - }, [getHoveredLine, onCommentTargetChange]); + }, [getHoveredLine, file, onCommentTargetChange]); return (
@@ -1178,6 +1231,8 @@ export const DiffApp = ({ diffHintShown={diffHintShown} onRetryDiff={handleRetryDiff} diffViewerRef={diffViewerRef} + displaySettings={displaySettings} + diffThemes={diffThemes} /> diff --git a/apps/cli/components/DiffViewer.tsx b/apps/cli/components/DiffViewer.tsx index 2703c1b..187054b 100644 --- a/apps/cli/components/DiffViewer.tsx +++ b/apps/cli/components/DiffViewer.tsx @@ -28,6 +28,7 @@ import type { Comment, CommentTag } from "@/lib/comment-types"; import type { DiffFileStat } from "@/lib/diff-file-stat"; import type { DiffTheme } from "@/lib/diff-colors"; import { getDiffUnsafeCSS } from "@/lib/diff-colors"; +import { DEFAULT_DIFF_THEMES } from "@/lib/diff-themes"; import { FileDiffHeader } from "./FileDiffHeader"; import { cn } from "@/lib/utils"; import { BranchIcon, CopySimpleIcon, TrashIcon, CheckIcon } from "blode-icons-react"; @@ -522,6 +523,10 @@ interface DiffViewerProps { wordWrap?: boolean; showBackgrounds?: boolean; diffIndicators?: "classic" | "bars" | "none"; + // Syntax theme ids per color scheme (optional, github defaults). Theme ids + // resolve lazily on the main thread via @pierre/diffs, so changing them just + // re-renders CodeView with the new theme — no preload needed. + diffThemes?: { light: string; dark: string }; } const EmptyState = () => ( @@ -558,6 +563,7 @@ const DiffViewerInner = ( wordWrap = true, showBackgrounds = true, diffIndicators = "classic", + diffThemes = DEFAULT_DIFF_THEMES, }: DiffViewerProps, ref: React.Ref, ) => { @@ -596,7 +602,9 @@ const DiffViewerInner = ( // Memoize parsing per (file, patch). The cache survives re-renders so theme, // layout, and comment changes never reparse an unchanged patch. - const parsedCacheRef = useRef(new Map()); + const parsedCacheRef = useRef( + new Map(), + ); const getParsedFileDiff = useCallback((file: string, patch: string): FileDiffMetadata | null => { const cache = parsedCacheRef.current; const cached = cache.get(file); @@ -658,18 +666,34 @@ const DiffViewerInner = ( disableFileHeader: true, disableLineNumbers: !showLineNumbers, enableGutterUtility: true, - expansionLineCount: 20, + // Lean on CodeView's virtualizer: render unchanged context regions in + // full instead of collapsing them into "N unmodified lines" banners. + // With expandUnchanged the context is always shown, so expansionLineCount + // (lines revealed per expand click) and collapsedContextThreshold (the + // gap size that triggers a collapse) are effectively unused; we still + // pass a large expansionLineCount for the rare manual-expand fallback. + expandUnchanged: true, + expansionLineCount: 100, hunkSeparators: "line-info", lineDiffType: "word-alt", lineHoverHighlight: "disabled", maxLineDiffLength: 500, overflow: wordWrap ? "wrap" : "scroll", stickyHeaders: true, - theme: { dark: "github-dark", light: "github-light" }, + theme: { dark: diffThemes.dark, light: diffThemes.light }, themeType, unsafeCSS: getDiffUnsafeCSS(themeType as DiffTheme), }), - [diffIndicators, layout, showBackgrounds, showLineNumbers, wordWrap, themeType], + [ + diffIndicators, + layout, + showBackgrounds, + showLineNumbers, + wordWrap, + themeType, + diffThemes.dark, + diffThemes.light, + ], ); // Imperative scroll-to-file for DiffApp (sidebar clicks, j/k navigation). diff --git a/apps/cli/components/StatusBar.tsx b/apps/cli/components/StatusBar.tsx index 866422f..a02dd7e 100644 --- a/apps/cli/components/StatusBar.tsx +++ b/apps/cli/components/StatusBar.tsx @@ -4,20 +4,31 @@ import { CheckIcon, ChevronDownIcon, CopySimpleIcon, - SunIcon, - MoonIcon, SplitIcon, ArrowRightIcon, ArrowRotateClockwiseIcon, + SettingsGear1Icon, + ContrastIcon, } from "blode-icons-react"; import { useTheme } from "next-themes"; -import { useEffect, useRef, useState, useSyncExternalStore } from "react"; +import { useRef, useState, useSyncExternalStore } from "react"; import type { Comment } from "@/lib/comment-types"; +import type { DisplaySettings, DiffIndicatorStyle } from "@/lib/display-settings"; +import type { DiffThemeSelection } from "@/lib/diff-themes"; +import { DIFF_THEMES } from "@/lib/diff-themes"; import { exportCommentsAsPrompt } from "@/lib/export-comments"; import { Button } from "@/components/ui/button"; import { Kbd } from "@/components/ui/kbd"; import { SidebarTrigger } from "@/components/ui/sidebar"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; import { cn } from "@/lib/utils"; export type DiffMode = "all" | "uncommitted"; @@ -53,8 +64,104 @@ interface StatusBarProps { onDiffModeChange: (mode: DiffMode) => void; layout: "split" | "stacked"; onLayoutChange: (l: "split" | "stacked") => void; + displaySettings: DisplaySettings; + onDisplaySettingsChange: (settings: DisplaySettings) => void; + diffThemes: DiffThemeSelection; + onDiffThemesChange: (themes: DiffThemeSelection) => void; +} + +const INDICATOR_OPTIONS: { value: DiffIndicatorStyle; label: string }[] = [ + { label: "Classic", value: "classic" }, + { label: "Bars", value: "bars" }, + { label: "None", value: "none" }, +]; + +const LIGHT_THEMES = DIFF_THEMES.filter((theme) => theme.type === "light"); +const DARK_THEMES = DIFF_THEMES.filter((theme) => theme.type === "dark"); + +// ── Inline primitives (no Switch/ToggleGroup in components/ui) ─────────────── + +const Switch = ({ + checked, + onChange, + label, +}: { + checked: boolean; + onChange: (next: boolean) => void; + label: string; +}) => ( + +); + +interface SegmentedOption { + value: T; + label: string; } +const SegmentedControl = ({ + value, + options, + onChange, + ariaLabel, +}: { + value: T; + options: SegmentedOption[]; + onChange: (next: T) => void; + ariaLabel: string; +}) => ( +
+ {options.map((option) => { + const active = option.value === value; + return ( + + ); + })} +
+); + +type ThemeModeOption = "system" | "light" | "dark"; +const THEME_MODE_OPTIONS: SegmentedOption[] = [ + { label: "Auto", value: "system" }, + { label: "Light", value: "light" }, + { label: "Dark", value: "dark" }, +]; + const getSyncNoticeToneClass = ( tone: NonNullable["tone"] | undefined, ) => { @@ -140,27 +247,7 @@ const useHasMounted = () => () => false, ); -const useDismissableMenu = ( - open: boolean, - menuRef: React.RefObject, - onClose: () => void, -) => { - useEffect(() => { - if (!open) { - return; - } - - const handleClick = (e: MouseEvent) => { - if (menuRef.current && !menuRef.current.contains(e.target as Node)) { - onClose(); - } - }; - - document.addEventListener("mousedown", handleClick); - return () => document.removeEventListener("mousedown", handleClick); - }, [menuRef, onClose, open]); -}; - +// oxlint-disable-next-line complexity export const StatusBar = ({ branch, baseBranch, @@ -174,13 +261,15 @@ export const StatusBar = ({ onDiffModeChange, layout, onLayoutChange, + displaySettings, + onDisplaySettingsChange, + diffThemes, + onDiffThemesChange, }: StatusBarProps) => { const [copied, setCopied] = useState(false); const [copiedBranch, setCopiedBranch] = useState<"branch" | "base" | null>(null); - const [modeMenuOpen, setModeMenuOpen] = useState(false); const mounted = useHasMounted(); - const modeMenuRef = useRef(null); - const { resolvedTheme, setTheme } = useTheme(); + const { setTheme, theme } = useTheme(); const copiedTimerRef = useRef | null>(null); const branchTimerRef = useRef | null>(null); @@ -218,7 +307,16 @@ export const StatusBar = ({ } }; - useDismissableMenu(modeMenuOpen, modeMenuRef, () => setModeMenuOpen(false)); + // oxlint-disable-next-line react-perf/jsx-no-new-function-as-prop + const updateSetting = ( + key: K, + value: DisplaySettings[K], + ): void => { + onDisplaySettingsChange({ ...displaySettings, [key]: value }); + }; + + const themeMode: ThemeModeOption = + theme === "light" || theme === "dark" ? theme : "system"; return ( @@ -329,45 +427,36 @@ export const StatusBar = ({ {/* Diff mode dropdown */} -
- - - {modeMenuOpen && ( -
+ + + onDiffModeChange(value as DiffMode)} + > {DIFF_MODES.map(({ value, label }) => ( - + + {label} + ))} -
- )} -
+ + + {/* Comments export */} {comments.length > 0 && ( @@ -417,26 +506,146 @@ export const StatusBar = ({ - {/* Theme toggle - only render after mount to avoid hydration mismatch */} + {/* Settings panel */} + + + } + > + + + +
+ Display +
+
+ Backgrounds + updateSetting("showBackgrounds", next)} + /> +
+
+ Line numbers + updateSetting("showLineNumbers", next)} + /> +
+
+ Word wrap + updateSetting("wordWrap", next)} + /> +
+
+ Indicators + updateSetting("diffIndicators", next)} + /> +
+ + + + {/* Theme picker - only render after mount to avoid hydration mismatch */} {mounted && ( - - + setTheme(resolvedTheme === "dark" ? "light" : "dark")} + aria-label="Theme" className="text-muted-foreground hover:text-foreground hover:bg-secondary" /> } > - {resolvedTheme === "dark" ? : } - - - {resolvedTheme === "dark" ? "Switch to light mode" : "Switch to dark mode"} - - + + + +
+ + Mode + + setTheme(next)} + /> +
+ +
+
+ Light theme +
+ {LIGHT_THEMES.map((entry) => ( + + ))} + +
+ Dark theme +
+ {DARK_THEMES.map((entry) => ( + + ))} +
+ + )} diff --git a/apps/cli/components/ui/context-menu.tsx b/apps/cli/components/ui/context-menu.tsx new file mode 100644 index 0000000..cec1c15 --- /dev/null +++ b/apps/cli/components/ui/context-menu.tsx @@ -0,0 +1,178 @@ +"use client"; + +import { ContextMenu as ContextMenuPrimitive } from "@base-ui/react/context-menu"; +import { CheckIcon, ChevronRightIcon, CircleIcon } from "blode-icons-react"; + +import { cn } from "@/lib/utils"; + +const ContextMenu = ({ ...props }: ContextMenuPrimitive.Root.Props) => ( + +); + +const ContextMenuTrigger = ({ ...props }: ContextMenuPrimitive.Trigger.Props) => ( + +); + +const ContextMenuGroup = ({ ...props }: ContextMenuPrimitive.Group.Props) => ( + +); + +const ContextMenuContent = ({ + className, + children, + ...props +}: ContextMenuPrimitive.Popup.Props) => ( + + + + {children} + + + +); + +const ContextMenuItem = ({ + className, + variant = "default", + ...props +}: ContextMenuPrimitive.Item.Props & { variant?: "default" | "destructive" }) => ( + +); + +const ContextMenuCheckboxItem = ({ + className, + children, + ...props +}: ContextMenuPrimitive.CheckboxItem.Props) => ( + + + + + + + {children} + +); + +const ContextMenuRadioGroup = ({ ...props }: ContextMenuPrimitive.RadioGroup.Props) => ( + +); + +const ContextMenuRadioItem = ({ + className, + children, + ...props +}: ContextMenuPrimitive.RadioItem.Props) => ( + + + + + + + {children} + +); + +const ContextMenuLabel = ({ className, ...props }: ContextMenuPrimitive.GroupLabel.Props) => ( + +); + +const ContextMenuSeparator = ({ className, ...props }: ContextMenuPrimitive.Separator.Props) => ( + +); + +const ContextMenuSub = ({ ...props }: ContextMenuPrimitive.SubmenuRoot.Props) => ( + +); + +const ContextMenuSubTrigger = ({ + className, + children, + ...props +}: ContextMenuPrimitive.SubmenuTrigger.Props) => ( + + {children} + + +); + +const ContextMenuSubContent = ({ + className, + children, + ...props +}: ContextMenuPrimitive.Popup.Props) => ( + + + + {children} + + + +); + +export { + ContextMenu, + ContextMenuTrigger, + ContextMenuGroup, + ContextMenuContent, + ContextMenuItem, + ContextMenuCheckboxItem, + ContextMenuRadioGroup, + ContextMenuRadioItem, + ContextMenuLabel, + ContextMenuSeparator, + ContextMenuSub, + ContextMenuSubTrigger, + ContextMenuSubContent, +}; diff --git a/apps/cli/components/ui/dropdown-menu.tsx b/apps/cli/components/ui/dropdown-menu.tsx new file mode 100644 index 0000000..5044010 --- /dev/null +++ b/apps/cli/components/ui/dropdown-menu.tsx @@ -0,0 +1,197 @@ +"use client"; + +import { Menu as MenuPrimitive } from "@base-ui/react/menu"; +import { CheckIcon, ChevronRightIcon, CircleIcon } from "blode-icons-react"; + +import { cn } from "@/lib/utils"; + +const DropdownMenu = ({ ...props }: MenuPrimitive.Root.Props) => ( + +); + +const DropdownMenuTrigger = ({ ...props }: MenuPrimitive.Trigger.Props) => ( + +); + +const DropdownMenuGroup = ({ ...props }: MenuPrimitive.Group.Props) => ( + +); + +const DropdownMenuContent = ({ + className, + side = "bottom", + sideOffset = 4, + align = "end", + alignOffset = 0, + children, + ...props +}: MenuPrimitive.Popup.Props & + Pick) => ( + + + + {children} + + + +); + +const DropdownMenuItem = ({ + className, + variant = "default", + ...props +}: MenuPrimitive.Item.Props & { variant?: "default" | "destructive" }) => ( + +); + +const DropdownMenuCheckboxItem = ({ + className, + children, + ...props +}: MenuPrimitive.CheckboxItem.Props) => ( + + + + + + + {children} + +); + +const DropdownMenuRadioGroup = ({ ...props }: MenuPrimitive.RadioGroup.Props) => ( + +); + +const DropdownMenuRadioItem = ({ + className, + children, + ...props +}: MenuPrimitive.RadioItem.Props) => ( + + + + + + + {children} + +); + +const DropdownMenuLabel = ({ className, ...props }: MenuPrimitive.GroupLabel.Props) => ( + +); + +const DropdownMenuSeparator = ({ className, ...props }: MenuPrimitive.Separator.Props) => ( + +); + +const DropdownMenuSub = ({ ...props }: MenuPrimitive.SubmenuRoot.Props) => ( + +); + +const DropdownMenuSubTrigger = ({ + className, + children, + ...props +}: MenuPrimitive.SubmenuTrigger.Props) => ( + + {children} + + +); + +const DropdownMenuSubContent = ({ + className, + sideOffset = 4, + alignOffset = -4, + children, + ...props +}: MenuPrimitive.Popup.Props & + Pick) => ( + + + + {children} + + + +); + +export { + DropdownMenu, + DropdownMenuTrigger, + DropdownMenuContent, + DropdownMenuGroup, + DropdownMenuItem, + DropdownMenuCheckboxItem, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuLabel, + DropdownMenuSeparator, + DropdownMenuSub, + DropdownMenuSubTrigger, + DropdownMenuSubContent, +}; diff --git a/apps/cli/components/ui/popover.tsx b/apps/cli/components/ui/popover.tsx new file mode 100644 index 0000000..0327769 --- /dev/null +++ b/apps/cli/components/ui/popover.tsx @@ -0,0 +1,51 @@ +"use client"; + +import { Popover as PopoverPrimitive } from "@base-ui/react/popover"; + +import { cn } from "@/lib/utils"; + +const Popover = ({ ...props }: PopoverPrimitive.Root.Props) => ( + +); + +const PopoverTrigger = ({ ...props }: PopoverPrimitive.Trigger.Props) => ( + +); + +const PopoverContent = ({ + className, + side = "bottom", + sideOffset = 4, + align = "center", + alignOffset = 0, + children, + ...props +}: PopoverPrimitive.Popup.Props & + Pick) => ( + + + + {children} + + + +); + +const PopoverClose = ({ ...props }: PopoverPrimitive.Close.Props) => ( + +); + +export { Popover, PopoverTrigger, PopoverContent, PopoverClose }; diff --git a/apps/cli/lib/diff-themes.ts b/apps/cli/lib/diff-themes.ts new file mode 100644 index 0000000..d76e2b2 --- /dev/null +++ b/apps/cli/lib/diff-themes.ts @@ -0,0 +1,129 @@ +// Client-safe catalog of syntax themes the diff viewer can render with. +// +// The list is the 4 Pierre themes (shipped by @pierre/diffs) followed by the +// 65 Shiki bundled themes. Regenerate the bundled portion with: +// +// node -e "import('shiki').then(s=>process.stdout.write(JSON.stringify(s.bundledThemesInfo.map(t=>({id:t.id,name:t.displayName,type:t.type})))))" +// +// Theme ids are passed straight into CodeView's `options.theme` ({ light, dark }). +// They resolve lazily on the main thread via @pierre/diffs' getSharedHighlighter +// → getResolvedOrResolveTheme → shiki `bundledThemes[id]`, so no preload step is +// needed — selecting a new id re-renders with it on the next paint. + +export interface DiffThemeInfo { + id: string; + name: string; + type: "light" | "dark"; +} + +export const DEFAULT_LIGHT_THEME = "github-light"; +export const DEFAULT_DARK_THEME = "github-dark"; + +export const DIFF_THEMES: readonly DiffThemeInfo[] = [ + // Pierre themes (provided by @pierre/diffs, not Shiki's bundle). + { id: "pierre-light", name: "Pierre Light", type: "light" }, + { id: "pierre-light-soft", name: "Pierre Light Soft", type: "light" }, + { id: "pierre-dark", name: "Pierre Dark", type: "dark" }, + { id: "pierre-dark-soft", name: "Pierre Dark Soft", type: "dark" }, + // Shiki bundled themes. + { id: "andromeeda", name: "Andromeeda", type: "dark" }, + { id: "aurora-x", name: "Aurora X", type: "dark" }, + { id: "ayu-dark", name: "Ayu Dark", type: "dark" }, + { id: "ayu-light", name: "Ayu Light", type: "light" }, + { id: "ayu-mirage", name: "Ayu Mirage", type: "dark" }, + { id: "catppuccin-frappe", name: "Catppuccin Frappé", type: "dark" }, + { id: "catppuccin-latte", name: "Catppuccin Latte", type: "light" }, + { id: "catppuccin-macchiato", name: "Catppuccin Macchiato", type: "dark" }, + { id: "catppuccin-mocha", name: "Catppuccin Mocha", type: "dark" }, + { id: "dark-plus", name: "Dark Plus", type: "dark" }, + { id: "dracula", name: "Dracula Theme", type: "dark" }, + { id: "dracula-soft", name: "Dracula Theme Soft", type: "dark" }, + { id: "everforest-dark", name: "Everforest Dark", type: "dark" }, + { id: "everforest-light", name: "Everforest Light", type: "light" }, + { id: "github-dark", name: "GitHub Dark", type: "dark" }, + { id: "github-dark-default", name: "GitHub Dark Default", type: "dark" }, + { id: "github-dark-dimmed", name: "GitHub Dark Dimmed", type: "dark" }, + { id: "github-dark-high-contrast", name: "GitHub Dark High Contrast", type: "dark" }, + { id: "github-light", name: "GitHub Light", type: "light" }, + { id: "github-light-default", name: "GitHub Light Default", type: "light" }, + { id: "github-light-high-contrast", name: "GitHub Light High Contrast", type: "light" }, + { id: "gruvbox-dark-hard", name: "Gruvbox Dark Hard", type: "dark" }, + { id: "gruvbox-dark-medium", name: "Gruvbox Dark Medium", type: "dark" }, + { id: "gruvbox-dark-soft", name: "Gruvbox Dark Soft", type: "dark" }, + { id: "gruvbox-light-hard", name: "Gruvbox Light Hard", type: "light" }, + { id: "gruvbox-light-medium", name: "Gruvbox Light Medium", type: "light" }, + { id: "gruvbox-light-soft", name: "Gruvbox Light Soft", type: "light" }, + { id: "horizon", name: "Horizon", type: "dark" }, + { id: "horizon-bright", name: "Horizon Bright", type: "dark" }, + { id: "houston", name: "Houston", type: "dark" }, + { id: "kanagawa-dragon", name: "Kanagawa Dragon", type: "dark" }, + { id: "kanagawa-lotus", name: "Kanagawa Lotus", type: "light" }, + { id: "kanagawa-wave", name: "Kanagawa Wave", type: "dark" }, + { id: "laserwave", name: "LaserWave", type: "dark" }, + { id: "light-plus", name: "Light Plus", type: "light" }, + { id: "material-theme", name: "Material Theme", type: "dark" }, + { id: "material-theme-darker", name: "Material Theme Darker", type: "dark" }, + { id: "material-theme-lighter", name: "Material Theme Lighter", type: "light" }, + { id: "material-theme-ocean", name: "Material Theme Ocean", type: "dark" }, + { id: "material-theme-palenight", name: "Material Theme Palenight", type: "dark" }, + { id: "min-dark", name: "Min Dark", type: "dark" }, + { id: "min-light", name: "Min Light", type: "light" }, + { id: "monokai", name: "Monokai", type: "dark" }, + { id: "night-owl", name: "Night Owl", type: "dark" }, + { id: "night-owl-light", name: "Night Owl Light", type: "light" }, + { id: "nord", name: "Nord", type: "dark" }, + { id: "one-dark-pro", name: "One Dark Pro", type: "dark" }, + { id: "one-light", name: "One Light", type: "light" }, + { id: "plastic", name: "Plastic", type: "dark" }, + { id: "poimandres", name: "Poimandres", type: "dark" }, + { id: "red", name: "Red", type: "dark" }, + { id: "rose-pine", name: "Rosé Pine", type: "dark" }, + { id: "rose-pine-dawn", name: "Rosé Pine Dawn", type: "light" }, + { id: "rose-pine-moon", name: "Rosé Pine Moon", type: "dark" }, + { id: "slack-dark", name: "Slack Dark", type: "dark" }, + { id: "slack-ochin", name: "Slack Ochin", type: "light" }, + { id: "snazzy-light", name: "Snazzy Light", type: "light" }, + { id: "solarized-dark", name: "Solarized Dark", type: "dark" }, + { id: "solarized-light", name: "Solarized Light", type: "light" }, + { id: "synthwave-84", name: "Synthwave '84", type: "dark" }, + { id: "tokyo-night", name: "Tokyo Night", type: "dark" }, + { id: "vesper", name: "Vesper", type: "dark" }, + { id: "vitesse-black", name: "Vitesse Black", type: "dark" }, + { id: "vitesse-dark", name: "Vitesse Dark", type: "dark" }, + { id: "vitesse-light", name: "Vitesse Light", type: "light" }, +]; + +export interface DiffThemeSelection { + light: string; + dark: string; +} + +export const DEFAULT_DIFF_THEMES: DiffThemeSelection = { + dark: DEFAULT_DARK_THEME, + light: DEFAULT_LIGHT_THEME, +}; + +const LIGHT_THEME_IDS = new Set(DIFF_THEMES.filter((t) => t.type === "light").map((t) => t.id)); +const DARK_THEME_IDS = new Set(DIFF_THEMES.filter((t) => t.type === "dark").map((t) => t.id)); + +/** + * Validate a persisted theme selection, falling back to the github defaults for + * any id that is not a known light/dark theme. Keeps a corrupt localStorage + * value from feeding an unresolvable id into CodeView. + */ +export const normalizeDiffThemes = (value: unknown): DiffThemeSelection => { + if (typeof value !== "object" || value === null) { + return DEFAULT_DIFF_THEMES; + } + const candidate = value as Partial; + return { + dark: + typeof candidate.dark === "string" && DARK_THEME_IDS.has(candidate.dark) + ? candidate.dark + : DEFAULT_DARK_THEME, + light: + typeof candidate.light === "string" && LIGHT_THEME_IDS.has(candidate.light) + ? candidate.light + : DEFAULT_LIGHT_THEME, + }; +}; diff --git a/apps/cli/lib/display-settings.ts b/apps/cli/lib/display-settings.ts new file mode 100644 index 0000000..254a8c2 --- /dev/null +++ b/apps/cli/lib/display-settings.ts @@ -0,0 +1,50 @@ +// Client-safe display settings for the diff viewer toolbar's settings panel. +// Persisted as a single JSON object under `diffhub-display-settings`. + +export type DiffIndicatorStyle = "classic" | "bars" | "none"; + +export interface DisplaySettings { + showBackgrounds: boolean; + showLineNumbers: boolean; + wordWrap: boolean; + diffIndicators: DiffIndicatorStyle; +} + +export const DISPLAY_SETTINGS_KEY = "diffhub-display-settings"; + +export const DEFAULT_DISPLAY_SETTINGS: DisplaySettings = { + diffIndicators: "classic", + showBackgrounds: true, + showLineNumbers: true, + wordWrap: true, +}; + +const INDICATOR_VALUES = new Set(["classic", "bars", "none"]); + +/** + * Validate a persisted display-settings object, filling any missing or invalid + * field with its default. Guards against corrupt localStorage values. + */ +export const normalizeDisplaySettings = (value: unknown): DisplaySettings => { + if (typeof value !== "object" || value === null) { + return DEFAULT_DISPLAY_SETTINGS; + } + const candidate = value as Partial>; + return { + diffIndicators: INDICATOR_VALUES.has(candidate.diffIndicators as DiffIndicatorStyle) + ? (candidate.diffIndicators as DiffIndicatorStyle) + : DEFAULT_DISPLAY_SETTINGS.diffIndicators, + showBackgrounds: + typeof candidate.showBackgrounds === "boolean" + ? candidate.showBackgrounds + : DEFAULT_DISPLAY_SETTINGS.showBackgrounds, + showLineNumbers: + typeof candidate.showLineNumbers === "boolean" + ? candidate.showLineNumbers + : DEFAULT_DISPLAY_SETTINGS.showLineNumbers, + wordWrap: + typeof candidate.wordWrap === "boolean" + ? candidate.wordWrap + : DEFAULT_DISPLAY_SETTINGS.wordWrap, + }; +}; From 0831a98df6c9cfe41ece3bb055fbcc3caf9cc982 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 22:47:03 +0000 Subject: [PATCH 4/5] style(cli): apply oxfmt to StatusBar https://claude.ai/code/session_01JHfEZvA73s1KmXxC99tnQE --- apps/cli/components/StatusBar.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/cli/components/StatusBar.tsx b/apps/cli/components/StatusBar.tsx index a02dd7e..77d483b 100644 --- a/apps/cli/components/StatusBar.tsx +++ b/apps/cli/components/StatusBar.tsx @@ -315,8 +315,7 @@ export const StatusBar = ({ onDisplaySettingsChange({ ...displaySettings, [key]: value }); }; - const themeMode: ThemeModeOption = - theme === "light" || theme === "dark" ? theme : "system"; + const themeMode: ThemeModeOption = theme === "light" || theme === "dark" ? theme : "system"; return ( From 78ff4ce15eb5640b58185c56b74000b668461157 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 22:54:44 +0000 Subject: [PATCH 5/5] feat(cli): theme picker drill-in submenus for light/dark themes Restructure the theme popover to match the reference UX: a root view with mode (Auto/Light/Dark) plus 'Light theme' and 'Dark theme' rows that drill into scrollable theme lists with a back button, instead of one flat list. View resets to root when the popover closes. https://claude.ai/code/session_01JHfEZvA73s1KmXxC99tnQE --- apps/cli/components/StatusBar.tsx | 228 +++++++++++++++++++----------- 1 file changed, 148 insertions(+), 80 deletions(-) diff --git a/apps/cli/components/StatusBar.tsx b/apps/cli/components/StatusBar.tsx index 77d483b..2f0bbe4 100644 --- a/apps/cli/components/StatusBar.tsx +++ b/apps/cli/components/StatusBar.tsx @@ -3,8 +3,12 @@ import { CheckIcon, ChevronDownIcon, + ChevronLeftIcon, + ChevronRightIcon, CopySimpleIcon, SplitIcon, + SunIcon, + MoonIcon, ArrowRightIcon, ArrowRotateClockwiseIcon, SettingsGear1Icon, @@ -162,6 +166,144 @@ const THEME_MODE_OPTIONS: SegmentedOption[] = [ { label: "Dark", value: "dark" }, ]; +const themeNameById = (id: string): string => + DIFF_THEMES.find((theme) => theme.id === id)?.name ?? id; + +type ThemeView = "root" | "light" | "dark"; + +// Theme picker with in-place drill-in navigation (root → light/dark theme +// lists with a back button) inside a single Popover, matching the reference UX. +const ThemePicker = ({ + diffThemes, + onDiffThemesChange, + themeMode, + onModeChange, +}: { + diffThemes: DiffThemeSelection; + onDiffThemesChange: (themes: DiffThemeSelection) => void; + themeMode: ThemeModeOption; + onModeChange: (mode: ThemeModeOption) => void; +}) => { + const [view, setView] = useState("root"); + const isLight = view === "light"; + const themes = isLight ? LIGHT_THEMES : DARK_THEMES; + const selectedId = isLight ? diffThemes.light : diffThemes.dark; + + return ( + { + if (!open) { + setView("root"); + } + }} + > + + } + > + + + + {view === "root" ? ( + <> +
+ + Mode + + +
+
+ + + + ) : ( + <> + +
+
+ {themes.map((entry) => ( + + ))} +
+ + )} + + + ); +}; + const getSyncNoticeToneClass = ( tone: NonNullable["tone"] | undefined, ) => { @@ -565,86 +707,12 @@ export const StatusBar = ({ {/* Theme picker - only render after mount to avoid hydration mismatch */} {mounted && ( - - - } - > - - - -
- - Mode - - setTheme(next)} - /> -
- -
-
- Light theme -
- {LIGHT_THEMES.map((entry) => ( - - ))} - -
- Dark theme -
- {DARK_THEMES.map((entry) => ( - - ))} -
- - + )}