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 <img width="465" height="353" alt="Unified view before" src="https://github.com/user-attachments/assets/0c061f25-f20a-4127-a85d-aee1161611c7" /> ### Unified view after <img width="634" height="240" alt="Unified view after" src="https://github.com/user-attachments/assets/f2dfd952-89ed-4fdd-83db-a05f19f023b2" /> ### Split view before <img width="596" height="335" alt="Split view before" src="https://github.com/user-attachments/assets/09bfbe41-9438-4801-b181-49a9d19d5bb8" /> ### Split view after <img width="640" height="338" alt="Split view after" src="https://github.com/user-attachments/assets/fc3618ef-474f-4217-bb21-5ffd53eb4e01" /> <!-- If you want to replace these screenshots later, keep the four sections above and swap the image URLs. --> ## 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 <i@shantur.com>
This commit is contained in:
VooDisss
@@ -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<number | null>(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<HTMLElement>(".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(() => {
|
||||
|
||||
@@ -384,6 +384,7 @@ const GitChangesTab: Component<GitChangesTabProps> = (props) => {
|
||||
onContextModeChange={props.onContextModeChange}
|
||||
onWordWrapModeChange={props.onWordWrapModeChange}
|
||||
/>
|
||||
|
||||
</>
|
||||
}
|
||||
list={{ panel: renderGroupedList, overlay: renderGroupedList }}
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user