From 5d0734e005d275bded6f13b7ab08ea74c45fcf9e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 24 Mar 2026 09:04:14 +0000 Subject: [PATCH] fix(site): diff viewer virtualizer buffer fix and styling polish (#23462) --- .../ai-elements/tool/EditFilesTool.tsx | 3 +- site/src/components/ai-elements/tool/Tool.tsx | 4 +- .../ai-elements/tool/WriteFileTool.tsx | 3 +- .../components/ai-elements/tool/utils.test.ts | 2 +- site/src/components/ai-elements/tool/utils.ts | 75 +++++++++++++------ .../components/DiffViewer/DiffViewer.tsx | 46 ++++++++---- 6 files changed, 90 insertions(+), 43 deletions(-) diff --git a/site/src/components/ai-elements/tool/EditFilesTool.tsx b/site/src/components/ai-elements/tool/EditFilesTool.tsx index 11825c4900..23ead707b1 100644 --- a/site/src/components/ai-elements/tool/EditFilesTool.tsx +++ b/site/src/components/ai-elements/tool/EditFilesTool.tsx @@ -15,6 +15,7 @@ import { DIFFS_FONT_STYLE, type EditFilesFileEntry, getDiffViewerOptions, + stripNoNewline, type ToolStatus, } from "./utils"; @@ -94,7 +95,7 @@ export const EditFilesTool: React.FC<{ scrollBarClassName="w-1.5" > diff --git a/site/src/components/ai-elements/tool/Tool.tsx b/site/src/components/ai-elements/tool/Tool.tsx index e62e4a3f3d..e5e87a7ea4 100644 --- a/site/src/components/ai-elements/tool/Tool.tsx +++ b/site/src/components/ai-elements/tool/Tool.tsx @@ -35,6 +35,7 @@ import { mapSubagentStatusToToolStatus, parseArgs, parseEditFilesArgs, + stripNoNewline, type ToolStatus, toProviderLabel, } from "./utils"; @@ -449,8 +450,9 @@ const GenericToolRenderer: FC = ({ scrollBarClassName="w-1.5" > ) : fileContent ? ( diff --git a/site/src/components/ai-elements/tool/WriteFileTool.tsx b/site/src/components/ai-elements/tool/WriteFileTool.tsx index 8bf8e93c1e..bfdc176319 100644 --- a/site/src/components/ai-elements/tool/WriteFileTool.tsx +++ b/site/src/components/ai-elements/tool/WriteFileTool.tsx @@ -14,6 +14,7 @@ import { ToolCollapsible } from "./ToolCollapsible"; import { DIFFS_FONT_STYLE, getDiffViewerOptions, + stripNoNewline, type ToolStatus, } from "./utils"; @@ -73,7 +74,7 @@ export const WriteFileTool: React.FC<{ scrollBarClassName="w-1.5" > diff --git a/site/src/components/ai-elements/tool/utils.test.ts b/site/src/components/ai-elements/tool/utils.test.ts index aa0b2c6a06..a8525f1c49 100644 --- a/site/src/components/ai-elements/tool/utils.test.ts +++ b/site/src/components/ai-elements/tool/utils.test.ts @@ -278,7 +278,7 @@ describe("getDiffViewerOptions", () => { expect(opts.theme).toBe("github-dark-high-contrast"); expect(opts.diffStyle).toBe("unified"); expect(opts.diffIndicators).toBe("bars"); - expect(opts.overflow).toBe("scroll"); + expect(opts.overflow).toBe("wrap"); expect(opts.unsafeCSS).toBe(diffViewerCSS); }); diff --git a/site/src/components/ai-elements/tool/utils.ts b/site/src/components/ai-elements/tool/utils.ts index d1e6bcdd1a..b11901ccac 100644 --- a/site/src/components/ai-elements/tool/utils.ts +++ b/site/src/components/ai-elements/tool/utils.ts @@ -166,6 +166,7 @@ const SELECTION_OVERRIDE_CSS = [ " --diffs-bg-selection-override: hsl(var(--content-link) / 0.08);", " --diffs-bg-selection-number-override: hsl(var(--content-link) / 0.13);", " --diffs-selection-number-fg: hsl(var(--content-link));", + " --diffs-gap-style: 1px solid hsl(var(--border-default));", "}", // Direct rules that override both context and change-line // selection backgrounds so every selected line looks the same. @@ -209,24 +210,29 @@ const SEPARATOR_CSS = [ "[data-unified] [data-separator='line-info'] [data-separator-wrapper] {", " padding-inline: 0 !important;", "}", - // The first separator in a file just says "N unmodified - // lines" before the first hunk — that's obvious context - // that adds no value, so hide it entirely. - "[data-separator='line-info'][data-separator-first] {", - " display: none !important;", - "}", - // Thin single border and muted text so collapsed-line - // indicators read as a quiet hint, not a landmark. + + // Centered text with horizontal rules on either side: + // ────── N unmodified lines ────── "[data-separator='line-info'] {", " height: 28px !important;", - " border-top: 1px solid hsl(var(--border-default));", - " border-bottom: 1px solid hsl(var(--border-default));", "}", "[data-separator-content] {", + " display: flex !important;", + " align-items: center !important;", + " gap: 12px !important;", + " overflow: visible !important;", + " height: auto !important;", " font-size: 11px !important;", " color: hsl(var(--content-secondary)) !important;", " opacity: 0.8;", "}", + "[data-separator-content]::before,", + "[data-separator-content]::after {", + " content: '' !important;", + " flex: 1 !important;", + " height: 1px !important;", + " background: hsl(var(--border-default)) !important;", + "}", ].join(" "); // Shared header styling applied to all diff viewers (both the @@ -238,8 +244,8 @@ const DIFF_HEADER_CSS = [ "[data-diffs-header] {", " font-size: 13px;", " min-height: 32px !important;", - " padding-block: 0 !important;", - " padding-inline: 12px !important;", + " padding-block: 8px !important;", + " padding-inline: 10px 6px !important;", " border-bottom: 1px solid hsl(var(--border-default));", "}", @@ -283,7 +289,12 @@ const DIFF_HEADER_CSS = [ // Stat counts styled as compact pill badges. "[data-diffs-header] [data-metadata] {", " flex-direction: row-reverse;", + " align-items: stretch;", " gap: 0 !important;", + " padding: 0;", + " border: 1px solid hsl(var(--border-default));", + " border-radius: 3px;", + " overflow: hidden;", "}", "[data-diffs-header] [data-additions-count],", "[data-diffs-header] [data-deletions-count] {", @@ -291,8 +302,8 @@ const DIFF_HEADER_CSS = [ " font-size: 12px;", " font-weight: 500;", " line-height: 20px;", - " padding-inline: 6px;", - " border-radius: 3px;", + " padding-inline: 4px;", + " border-radius: 0;", "}", "[data-diffs-header] [data-additions-count] {", " color: hsl(var(--git-added-bright)) !important;", @@ -302,14 +313,6 @@ const DIFF_HEADER_CSS = [ " color: hsl(var(--git-deleted-bright)) !important;", " background-color: hsl(var(--surface-git-deleted));", "}", - // Joined badge: flatten touching inner edges. DOM order is - // [deletions][additions]; row-reverse puts additions left. - "[data-deletions-count] + [data-additions-count] {", - " border-radius: 3px 0 0 3px;", - "}", - "[data-deletions-count]:has(+ [data-additions-count]) {", - " border-radius: 0 3px 3px 0;", - "}", ].join(" "); export const diffViewerCSS = [ @@ -319,6 +322,11 @@ export const diffViewerCSS = [ // tint and word-level emphasis highlights remain visible. "pre, [data-line]:not([data-selected-line]):not([data-line-type='change-addition']):not([data-line-type='change-deletion']), [data-diffs-header] { background-color: transparent !important; }", "[data-diffs-header] { border-left: 1px solid var(--border); }", + // The library reserves a 6 px horizontal scrollbar track on + // [data-code] via overflow: scroll clip. In wrap mode lines + // never overflow, so hide the track to remove the phantom gap. + "[data-code] { scrollbar-width: none !important; }", + "[data-code]::-webkit-scrollbar { height: 0 !important; }", DIFF_HEADER_CSS, SELECTION_OVERRIDE_CSS, SEPARATOR_CSS, @@ -329,13 +337,34 @@ export function getDiffViewerOptions(isDark: boolean) { return { diffStyle: "unified" as const, diffIndicators: "bars" as const, - overflow: "scroll" as const, + overflow: "wrap" as const, themeType: (isDark ? "dark" : "light") as "dark" | "light", theme: isDark ? "github-dark-high-contrast" : "github-light", unsafeCSS: diffViewerCSS, }; } +/** + * Returns a shallow clone of the diff with the no-EOF-newline + * flags cleared on every hunk so the renderer never emits the + * "No newline at end of file" row. Use for inline tool diffs + * where the indicator is visual noise. + */ +export function stripNoNewline(fileDiff: FileDiffMetadata): FileDiffMetadata { + const needsStrip = fileDiff.hunks.some( + (h) => h.noEOFCRDeletions || h.noEOFCRAdditions, + ); + if (!needsStrip) return fileDiff; + return { + ...fileDiff, + hunks: fileDiff.hunks.map((h) => ({ + ...h, + noEOFCRDeletions: false, + noEOFCRAdditions: false, + })), + }; +} + export function getFileViewerOptions(isDark: boolean) { return { overflow: "scroll" as const, diff --git a/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx b/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx index 387e81eb95..900d9a7a2d 100644 --- a/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx +++ b/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx @@ -3,6 +3,7 @@ import type { DiffLineAnnotation, FileDiffMetadata, SelectedLineRange, + VirtualFileMetrics, } from "@pierre/diffs"; import { Virtualizer } from "@pierre/diffs"; import { FileDiff, VirtualizerContext } from "@pierre/diffs/react"; @@ -114,6 +115,7 @@ const STICKY_HEADER_CSS = [ "[data-diffs-header] {", " position: sticky; top: 0; z-index: 10;", " background-color: hsl(var(--surface-secondary)) !important;", + " padding-block: 0 !important;", "}", ].join(" "); @@ -143,15 +145,31 @@ const FILE_TREE_WIDTH = 300; // ------------------------------------------------------------------- /** - * Estimated height per line in the diff viewer (px). Derived from - * the --diffs-font-size (11px) and --diffs-line-height (1.5) - * values set via DIFFS_FONT_STYLE, plus 1px for the border/gap. + * Estimated height per line in the diff viewer (px). The + * library's shadow DOM applies box-sizing: border-box to all + * elements, and code lines have no padding or border, so the + * rendered height equals the CSS line-height: 11px × 1.5 = 16.5. */ -const LINE_HEIGHT_PX = 17.5; +const LINE_HEIGHT_PX = 16.5; /** Height of the file header row rendered by @pierre/diffs. */ const HEADER_HEIGHT_PX = 36; +/** + * Metrics that tell the @pierre/diffs virtualizer how tall each + * element actually is after our CSS overrides. Without these the + * library falls back to its built-in defaults (20 px lines, + * 44 px headers, 32 px separators) which are larger than our + * custom styling, causing visible blank buffers in the viewport. + */ +const VIRTUALIZER_METRICS: VirtualFileMetrics = { + hunkLineCount: 50, + lineHeight: LINE_HEIGHT_PX, + diffHeaderHeight: 32, // 32 px min-height (border-box includes borders) + hunkSeparatorHeight: 28, // height: 28px !important in SEPARATOR_CSS + fileGap: 2, // padding-bottom: max(0, gap-block - 6px) = 2px +}; + /** * Estimate the rendered pixel height of a file diff so the * placeholder occupies roughly the same space. This keeps the @@ -357,8 +375,7 @@ const DiffScrollContainer: FC<{ children: ReactNode; className?: string; diffViewportRef: React.RefObject; - onViewportHeight: (height: number) => void; -}> = ({ children, className, diffViewportRef, onViewportHeight }) => { +}> = ({ children, className, diffViewportRef }) => { const [virtualizer] = useState(() => new Virtualizer()); // useCallback is required for correctness: in React 19 an @@ -377,18 +394,12 @@ const DiffScrollContainer: FC<{ diffViewportRef.current = viewport; virtualizer.setup(viewport); - onViewportHeight(viewport.clientHeight); - const ro = new ResizeObserver(([entry]) => { - onViewportHeight(entry.contentRect.height); - }); - ro.observe(viewport); return () => { - ro.disconnect(); virtualizer.cleanUp(); diffViewportRef.current = null; }; }, - [virtualizer, diffViewportRef, onViewportHeight], + [virtualizer, diffViewportRef], ); return ( @@ -476,6 +487,7 @@ const LazyFileDiff = memo<{ = ({ } }, [scrollToFile, onScrollToFileComplete]); - const [viewportHeight, setViewportHeight] = useState(0); // --------------------------------------------------------------- // Loading state // --------------------------------------------------------------- @@ -820,7 +831,6 @@ export const DiffViewer: FC = ({ )} = ({
setFileRef(fileDiff.name, el)} - style={isLast ? { minHeight: viewportHeight } : undefined} + className={ + i > 0 + ? "border-0 border-t border-solid border-border-default" + : undefined + } >