From 9d9a6a79ec4eb7840101b677ade81acd450e803f Mon Sep 17 00:00:00 2001 From: VooDisss <41582720+VooDisss@users.noreply.github.com> Date: Fri, 17 Apr 2026 19:04:10 +0300 Subject: [PATCH] Git diff monaco redesign (#304) ## Summary Fixes #303. This PR redesigns the Git Changes Monaco diff gutter so unified and split view both use a more intentional, space-efficient Monaco presentation while preserving Monaco's performance on large diffs. The final behavior includes: - `Compact` and `Normal` gutter modes for Git Changes - dynamic gutter sizing based on actual line-number digit counts - independent original/modified number-column sizing where needed - split-view fixes for both wasted left inset and line-number/sign overlap - persisted gutter-mode selection - localized user-facing labels for the control ## Visual comparison ### Unified view before Unified view before ### Unified view after Unified view after ### Split view before Split view before ### Split view after Split view after ## What changed ### Unified view - added two Git Changes Monaco gutter presentations: - `Compact` - `Normal` - kept compact as the tighter single-column-feel unified gutter - kept normal as the wider Monaco-style unified gutter - made unified gutter sizing respond to actual line-number digit counts instead of fixed assumptions - made normal mode size the visible number columns independently when one side needs more width than the other ### Split view - added dynamic split gutter sizing derived from actual before/after line counts - made split original and modified number columns size independently - fixed the modified-pane overlap where larger line numbers could collide with the `+` lane - fixed the original-pane wasted left inset caused by Monaco reserving an empty original-side glyph-margin lane ### Persistence and UI - persisted the selected gutter mode in preferences so it survives reloads - moved the gutter-mode control out of the Git Changes toolbar and into Appearance settings - renamed the visible settings options to `Compact` and `Normal` ### i18n - removed hardcoded user-facing gutter toggle strings - added localized keys for the gutter control labels and titles used by the Git Changes surface ## Implementation notes - Monaco remains the active Git Changes renderer throughout - gutter sizing logic is centralized in `packages/ui/src/components/file-viewer/monaco-diff-viewer.tsx` - CSS is used only for narrow presentation adjustments such as the 4px left inset and the split original-pane glyph-margin correction - the persisted gutter-mode preference is the source of truth for the selected presentation ## Review focus - unified `Compact` mode should feel tight without clipping or overlap - unified `Normal` mode should remain wider and readable - 3-digit and 4-digit line numbers should not collide with the sign lane - split original pane should no longer show wasted left inset before the first visible number column - split modified pane should not leave conspicuous dead space or collide with the `+` lane as digit counts grow - selected gutter mode should persist after reload --------- Co-authored-by: Shantur Rathore --- .../file-viewer/monaco-diff-viewer.tsx | 131 +++++++++++++++++- .../shell/right-panel/tabs/GitChangesTab.tsx | 1 + packages/ui/src/styles/panels/right-panel.css | 34 +++++ 3 files changed, 162 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/components/file-viewer/monaco-diff-viewer.tsx b/packages/ui/src/components/file-viewer/monaco-diff-viewer.tsx index 3cf2204a..6d4fda3d 100644 --- a/packages/ui/src/components/file-viewer/monaco-diff-viewer.tsx +++ b/packages/ui/src/components/file-viewer/monaco-diff-viewer.tsx @@ -19,12 +19,60 @@ interface MonacoDiffViewerProps { insertContextLabel?: string } +function getLineCount(value: string): number { + if (!value) return 1 + return value.split("\n").length +} + +function getDigitCount(value: number): number { + return String(Math.max(1, value)).length +} + +function getUnifiedGutterSizing(options: { before: string; after: string }) { + const beforeLineCount = getLineCount(options.before) + const afterLineCount = getLineCount(options.after) + const beforeDigitCount = getDigitCount(beforeLineCount) + const afterDigitCount = getDigitCount(afterLineCount) + const maxDigitCount = Math.max(beforeDigitCount, afterDigitCount) + const extraDigits = Math.max(0, maxDigitCount - 2) + const beforeNumberChars = Math.max(2, beforeDigitCount) + const afterNumberChars = Math.max(2, afterDigitCount) + const fourDigitPenalty = Math.max(0, maxDigitCount - 3) + + return { + diffEditorLineNumbersMinChars: Math.max(beforeNumberChars, afterNumberChars), + originalLineNumbersMinChars: beforeNumberChars, + modifiedLineNumbersMinChars: afterNumberChars, + lineDecorationsWidth: 6 + extraDigits * 2 + fourDigitPenalty * 2, + } +} + +function getSplitGutterSizing(options: { before: string; after: string }) { + const beforeLineCount = getLineCount(options.before) + const afterLineCount = getLineCount(options.after) + const beforeDigitCount = getDigitCount(beforeLineCount) + const afterDigitCount = getDigitCount(afterLineCount) + const maxDigitCount = Math.max(beforeDigitCount, afterDigitCount) + const extraDigits = Math.max(0, maxDigitCount - 2) + const beforeNumberChars = Math.max(2, beforeDigitCount) + const afterNumberChars = Math.max(2, afterDigitCount) + const fourDigitPenalty = Math.max(0, maxDigitCount - 3) + + return { + diffEditorLineNumbersMinChars: Math.max(beforeNumberChars, afterNumberChars), + originalLineNumbersMinChars: beforeNumberChars, + modifiedLineNumbersMinChars: afterNumberChars, + lineDecorationsWidth: 8 + extraDigits * 2 + fourDigitPenalty, + } +} + export function MonacoDiffViewer(props: MonacoDiffViewerProps) { const { isDark } = useTheme() let host: HTMLDivElement | undefined let diffEditor: any = null let monaco: any = null + let splitLayoutFrame: number | null = null const [ready, setReady] = createSignal(false) const [hoveredLine, setHoveredLine] = createSignal(null) const [selectedRange, setSelectedRange] = createSignal<{ startLine: number; endLine: number } | null>(null) @@ -55,6 +103,44 @@ export function MonacoDiffViewer(props: MonacoDiffViewerProps) { diffEditor = null } + const clearSplitLayoutVariables = () => { + if (!host) return + host.style.removeProperty("--split-original-line-number-width") + host.style.removeProperty("--split-original-delete-sign-left") + host.style.removeProperty("--split-original-gutter-width") + } + + const syncSplitLayoutVariables = (options: { + viewMode: "split" | "unified" + originalLineNumbersMinChars: number + lineDecorationsWidth: number + }) => { + if (!host) return + if (splitLayoutFrame !== null && typeof window !== "undefined") { + window.cancelAnimationFrame(splitLayoutFrame) + splitLayoutFrame = null + } + if (options.viewMode !== "split" || typeof window === "undefined") { + clearSplitLayoutVariables() + return + } + + splitLayoutFrame = window.requestAnimationFrame(() => { + splitLayoutFrame = null + if (!host) return + const originalLineNumbers = host.querySelector(".editor.original .line-numbers") + const measuredWidth = originalLineNumbers?.getBoundingClientRect().width ?? 0 + const lineNumberWidth = + measuredWidth > 0 ? measuredWidth : Math.max(12, options.originalLineNumbersMinChars * 6) + host.style.setProperty("--split-original-line-number-width", `${lineNumberWidth}px`) + host.style.setProperty("--split-original-delete-sign-left", `${lineNumberWidth}px`) + host.style.setProperty( + "--split-original-gutter-width", + `${lineNumberWidth + options.lineDecorationsWidth}px`, + ) + }) + } + const getModifiedEditor = () => diffEditor?.getModifiedEditor?.() ?? null const getActiveInsertRange = () => { @@ -120,7 +206,7 @@ export function MonacoDiffViewer(props: MonacoDiffViewerProps) { renderWhitespace: "selection", fontSize: 13, wordWrap: props.wordWrap === "on" ? "on" : "off", - glyphMargin: true, + glyphMargin: false, folding: false, // Keep enough gutter space so unified diffs don't overlap `+`/`-` markers. lineNumbersMinChars: 4, @@ -139,6 +225,11 @@ export function MonacoDiffViewer(props: MonacoDiffViewerProps) { onCleanup(() => { cancelled = true + if (splitLayoutFrame !== null && typeof window !== "undefined") { + window.cancelAnimationFrame(splitLayoutFrame) + splitLayoutFrame = null + } + clearSplitLayoutVariables() setReady(false) disposeEditor() }) @@ -149,6 +240,11 @@ export function MonacoDiffViewer(props: MonacoDiffViewerProps) { monaco.editor.setTheme(isDark() ? "vs-dark" : "vs") }) + createEffect(() => { + if (!host) return + host.dataset.viewMode = props.viewMode === "split" ? "split" : "unified" + }) + createEffect(() => { if (!ready() || !monaco || !diffEditor) return const modifiedEditor = diffEditor.getModifiedEditor?.() @@ -222,10 +318,23 @@ export function MonacoDiffViewer(props: MonacoDiffViewerProps) { const viewMode = props.viewMode === "unified" ? "unified" : "split" const contextMode = props.contextMode === "collapsed" ? "collapsed" : "expanded" const wordWrap = props.wordWrap === "on" ? "on" : "off" - + const { before, after } = resolvedContent() + const sizing = + viewMode === "unified" + ? getUnifiedGutterSizing({ before, after }) + : getSplitGutterSizing({ before, after }) + const { + diffEditorLineNumbersMinChars, + originalLineNumbersMinChars, + modifiedLineNumbersMinChars, + lineDecorationsWidth, + } = sizing diffEditor.updateOptions({ renderSideBySide: viewMode === "split", renderSideBySideInlineBreakpoint: 0, + renderIndicators: true, + lineNumbersMinChars: diffEditorLineNumbersMinChars, + lineDecorationsWidth, hideUnchangedRegions: contextMode === "collapsed" ? { enabled: true } @@ -234,16 +343,30 @@ export function MonacoDiffViewer(props: MonacoDiffViewerProps) { }) try { - diffEditor.getOriginalEditor?.()?.updateOptions?.({ wordWrap }) + diffEditor.getOriginalEditor?.()?.updateOptions?.({ + wordWrap, + lineNumbersMinChars: originalLineNumbersMinChars, + lineDecorationsWidth, + }) } catch { // ignore } try { - diffEditor.getModifiedEditor?.()?.updateOptions?.({ wordWrap }) + diffEditor.getModifiedEditor?.()?.updateOptions?.({ + wordWrap, + lineNumbersMinChars: modifiedLineNumbersMinChars, + lineDecorationsWidth, + }) } catch { // ignore } + + syncSplitLayoutVariables({ + viewMode, + originalLineNumbersMinChars, + lineDecorationsWidth, + }) }) createEffect(() => { diff --git a/packages/ui/src/components/instance/shell/right-panel/tabs/GitChangesTab.tsx b/packages/ui/src/components/instance/shell/right-panel/tabs/GitChangesTab.tsx index c7da139c..56249d4e 100644 --- a/packages/ui/src/components/instance/shell/right-panel/tabs/GitChangesTab.tsx +++ b/packages/ui/src/components/instance/shell/right-panel/tabs/GitChangesTab.tsx @@ -384,6 +384,7 @@ const GitChangesTab: Component = (props) => { onContextModeChange={props.onContextModeChange} onWordWrapModeChange={props.onWordWrapModeChange} /> + } list={{ panel: renderGroupedList, overlay: renderGroupedList }} diff --git a/packages/ui/src/styles/panels/right-panel.css b/packages/ui/src/styles/panels/right-panel.css index 3f78fcc6..14bd0d36 100644 --- a/packages/ui/src/styles/panels/right-panel.css +++ b/packages/ui/src/styles/panels/right-panel.css @@ -611,6 +611,40 @@ z-index: 30; } +.file-viewer-content--monaco .monaco-viewer[data-view-mode="unified"] .line-numbers { + text-align: left !important; + padding-left: 4px; +} + +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .line-numbers, +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.modified .line-numbers { + text-align: left !important; + padding-left: 4px; +} + +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .glyph-margin { + width: 0 !important; +} + +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .line-numbers { + left: 0 !important; +} + +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .cldr.delete-sign { + left: var(--split-original-delete-sign-left, 14px) !important; +} + +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .margin, +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .margin-view-zones, +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .margin-view-overlays { + width: var(--split-original-gutter-width, 24px) !important; +} + +.file-viewer-content--monaco .monaco-viewer[data-view-mode="split"] .editor.original .editor-scrollable { + left: var(--split-original-gutter-width, 24px) !important; + width: calc(100% - var(--split-original-gutter-width, 24px)) !important; +} + .file-viewer-empty { @apply flex flex-col items-center justify-center h-full gap-3 text-center; color: var(--text-muted);