mirror of
https://github.com/coder/coder.git
synced 2026-09-23 05:43:53 +08:00
fix(site): diff viewer virtualizer buffer fix and styling polish (#23462)
This commit is contained in:
@@ -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"
|
||||
>
|
||||
<FileDiff
|
||||
fileDiff={diff}
|
||||
fileDiff={stripNoNewline(diff)}
|
||||
options={getDiffViewerOptions(isDark)}
|
||||
style={DIFFS_FONT_STYLE}
|
||||
/>
|
||||
|
||||
@@ -35,6 +35,7 @@ import {
|
||||
mapSubagentStatusToToolStatus,
|
||||
parseArgs,
|
||||
parseEditFilesArgs,
|
||||
stripNoNewline,
|
||||
type ToolStatus,
|
||||
toProviderLabel,
|
||||
} from "./utils";
|
||||
@@ -449,8 +450,9 @@ const GenericToolRenderer: FC<ToolRendererProps> = ({
|
||||
scrollBarClassName="w-1.5"
|
||||
>
|
||||
<FileDiff
|
||||
fileDiff={writeFileDiff}
|
||||
fileDiff={stripNoNewline(writeFileDiff)}
|
||||
options={getDiffViewerOptions(isDark)}
|
||||
style={DIFFS_FONT_STYLE}
|
||||
/>
|
||||
</ScrollArea>
|
||||
) : fileContent ? (
|
||||
|
||||
@@ -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"
|
||||
>
|
||||
<FileDiff
|
||||
fileDiff={diff}
|
||||
fileDiff={stripNoNewline(diff)}
|
||||
options={getDiffViewerOptions(isDark)}
|
||||
style={DIFFS_FONT_STYLE}
|
||||
/>
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<HTMLElement | null>;
|
||||
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<{
|
||||
<FileDiff
|
||||
fileDiff={fileDiff}
|
||||
options={options}
|
||||
metrics={VIRTUALIZER_METRICS}
|
||||
style={DIFFS_FONT_STYLE}
|
||||
lineAnnotations={lineAnnotations}
|
||||
renderAnnotation={renderAnnotationProp}
|
||||
@@ -751,7 +763,6 @@ export const DiffViewer: FC<DiffViewerProps> = ({
|
||||
}
|
||||
}, [scrollToFile, onScrollToFileComplete]);
|
||||
|
||||
const [viewportHeight, setViewportHeight] = useState(0);
|
||||
// ---------------------------------------------------------------
|
||||
// Loading state
|
||||
// ---------------------------------------------------------------
|
||||
@@ -820,7 +831,6 @@ export const DiffViewer: FC<DiffViewerProps> = ({
|
||||
)}
|
||||
<DiffScrollContainer
|
||||
diffViewportRef={diffViewportRef}
|
||||
onViewportHeight={setViewportHeight}
|
||||
className={cn(
|
||||
"min-w-0 flex-1",
|
||||
showTree &&
|
||||
@@ -833,7 +843,11 @@ export const DiffViewer: FC<DiffViewerProps> = ({
|
||||
<div
|
||||
key={fileDiff.name}
|
||||
ref={(el) => setFileRef(fileDiff.name, el)}
|
||||
style={isLast ? { minHeight: viewportHeight } : undefined}
|
||||
className={
|
||||
i > 0
|
||||
? "border-0 border-t border-solid border-border-default"
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
<LazyFileDiff
|
||||
fileDiff={fileDiff}
|
||||
|
||||
Reference in New Issue
Block a user