diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index cef5efde6..cb4c1b0fe 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -546,6 +546,10 @@ export default function CombinedDiffViewer({ measuredContentHeight: sectionHeights[index], originalContent: section.originalContent, modifiedContent: section.modifiedContent, + changedLineCount: + section.added === undefined && section.removed === undefined + ? undefined + : (section.added ?? 0) + (section.removed ?? 0), useIntrinsicImageHeight: isIntrinsicHeightImageDiff(section.diffResult) }) }, diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx index b32c2b912..9ed91d76f 100644 --- a/src/renderer/src/components/editor/DiffSectionItem.tsx +++ b/src/renderer/src/components/editor/DiffSectionItem.tsx @@ -242,6 +242,15 @@ export function DiffSectionItem({ section.status ] ) + const changedLineCount = useMemo(() => { + if (lineStats) { + return lineStats.added + lineStats.removed + } + if (section.added === undefined && section.removed === undefined) { + return undefined + } + return (section.added ?? 0) + (section.removed ?? 0) + }, [lineStats, section.added, section.removed]) // Why: image diffs need document-flow height in the combined view; the text // fallback only knows line counts and would squash screenshots into one row. const useIntrinsicImageHeight = isIntrinsicHeightImageDiff(section.diffResult) @@ -249,6 +258,7 @@ export function DiffSectionItem({ measuredContentHeight: sectionHeight, originalContent: section.originalContent, modifiedContent: section.modifiedContent, + changedLineCount, useIntrinsicImageHeight }) @@ -258,6 +268,10 @@ export function DiffSectionItem({ lineNumberOptionsSubRef.current = applyDiffEditorLineNumberOptions(editor, sideBySide) const modified = editor.getModifiedEditor() + // Why: measuring before Monaco computes hidden unchanged regions records + // full-file height, making virtualized combined diffs jump as rows remount. + let diffLayoutReady = false + let pendingHeightFrame: number | null = null const updateHeight = (): void => { const contentHeight = editor.getModifiedEditor().getContentHeight() setSectionHeights((prev) => { @@ -267,8 +281,28 @@ export function DiffSectionItem({ return { ...prev, [index]: contentHeight } }) } - modified.onDidContentSizeChange(updateHeight) - updateHeight() + const requestHeightUpdate = (): void => { + if (pendingHeightFrame !== null) { + return + } + pendingHeightFrame = window.requestAnimationFrame(() => { + pendingHeightFrame = null + updateHeight() + }) + } + const markDiffLayoutReady = (): void => { + diffLayoutReady = true + requestHeightUpdate() + } + const contentSizeSub = modified.onDidContentSizeChange(() => { + if (diffLayoutReady) { + requestHeightUpdate() + } + }) + const diffUpdateSub = editor.onDidUpdateDiff(markDiffLayoutReady) + if (editor.getLineChanges() !== null) { + markDiffLayoutReady() + } setModifiedEditor(modified) // Why: Monaco disposes inner editors when the DiffEditor container is @@ -277,6 +311,12 @@ export function DiffSectionItem({ // methods on a disposed editor instance, and avoids `popover` pointing // at a line in an editor that no longer exists. modified.onDidDispose(() => { + contentSizeSub.dispose() + diffUpdateSub.dispose() + if (pendingHeightFrame !== null) { + window.cancelAnimationFrame(pendingHeightFrame) + pendingHeightFrame = null + } lineNumberOptionsSubRef.current?.dispose() lineNumberOptionsSubRef.current = null diffEditorRef.current = null diff --git a/src/renderer/src/components/editor/diff-section-layout.test.ts b/src/renderer/src/components/editor/diff-section-layout.test.ts index a8b91bb5e..bc10ad45c 100644 --- a/src/renderer/src/components/editor/diff-section-layout.test.ts +++ b/src/renderer/src/components/editor/diff-section-layout.test.ts @@ -29,6 +29,37 @@ describe('diff section layout', () => { ).toBe(76) }) + it('uses changed-line count before Monaco reports collapsed diff height', () => { + const largeUnchangedFile = Array.from({ length: 10_000 }, (_, index) => `line ${index}`).join( + '\n' + ) + + expect( + getDiffSectionBodyHeight({ + measuredContentHeight: undefined, + originalContent: largeUnchangedFile, + modifiedContent: `${largeUnchangedFile}\nchanged`, + changedLineCount: 1, + useIntrinsicImageHeight: false + }) + ).toBe(266) + }) + + it('caps unmeasured text diffs without changed-line stats', () => { + const largeUnchangedFile = Array.from({ length: 10_000 }, (_, index) => `line ${index}`).join( + '\n' + ) + + expect( + getDiffSectionBodyHeight({ + measuredContentHeight: undefined, + originalContent: largeUnchangedFile, + modifiedContent: `${largeUnchangedFile}\nchanged`, + useIntrinsicImageHeight: false + }) + ).toBe(1539) + }) + it('keeps empty text sections visible', () => { expect( getDiffSectionBodyHeight({ @@ -93,11 +124,29 @@ describe('diff section layout', () => { measuredContentHeight: undefined, originalContent: 'one', modifiedContent: 'one\ntwo\nthree', + changedLineCount: 2, useIntrinsicImageHeight: false }) ).toBe(104) }) + it('uses changed-line count for large virtualized expanded sections', () => { + const largeUnchangedFile = Array.from({ length: 10_000 }, (_, index) => `line ${index}`).join( + '\n' + ) + + expect( + getDiffSectionEstimatedHeight({ + collapsed: false, + measuredContentHeight: undefined, + originalContent: largeUnchangedFile, + modifiedContent: `${largeUnchangedFile}\nchanged`, + changedLineCount: 1, + useIntrinsicImageHeight: false + }) + ).toBe(294) + }) + it('estimates collapsed virtualized sections as header-only rows', () => { expect( getDiffSectionEstimatedHeight({ diff --git a/src/renderer/src/components/editor/diff-section-layout.ts b/src/renderer/src/components/editor/diff-section-layout.ts index a51aca7b2..0e12a432b 100644 --- a/src/renderer/src/components/editor/diff-section-layout.ts +++ b/src/renderer/src/components/editor/diff-section-layout.ts @@ -4,11 +4,14 @@ const DIFF_LINE_HEIGHT = 19 const DIFF_SECTION_PADDING_HEIGHT = 19 const MIN_DIFF_SECTION_BODY_HEIGHT = 60 const DIFF_SECTION_HEADER_HEIGHT = 28 +const DIFF_UNCHANGED_CONTEXT_LINE_ESTIMATE = 12 +const MAX_UNMEASURED_TEXT_BODY_LINES = 80 type DiffSectionBodyHeightInput = { measuredContentHeight: number | undefined originalContent: string modifiedContent: string + changedLineCount?: number useIntrinsicImageHeight: boolean } @@ -20,6 +23,7 @@ export function getDiffSectionBodyHeight({ measuredContentHeight, originalContent, modifiedContent, + changedLineCount, useIntrinsicImageHeight }: DiffSectionBodyHeightInput): number | undefined { if (useIntrinsicImageHeight) { @@ -30,11 +34,24 @@ export function getDiffSectionBodyHeight({ return measuredContentHeight + DIFF_SECTION_PADDING_HEIGHT } + const fullLineCount = Math.max( + originalContent.split('\n').length, + modifiedContent.split('\n').length + ) + const estimatedLineCount = + changedLineCount !== undefined + ? Math.min( + fullLineCount, + Math.max(2, changedLineCount + DIFF_UNCHANGED_CONTEXT_LINE_ESTIMATE) + ) + : Math.min(fullLineCount, MAX_UNMEASURED_TEXT_BODY_LINES) + + // Why: combined diffs hide unchanged regions inside Monaco. Before Monaco + // reports its collapsed content height, sizing from full file length makes + // large files flash open and forces the virtualizer to jump on scroll. return Math.max( MIN_DIFF_SECTION_BODY_HEIGHT, - Math.max(originalContent.split('\n').length, modifiedContent.split('\n').length) * - DIFF_LINE_HEIGHT + - DIFF_SECTION_PADDING_HEIGHT + estimatedLineCount * DIFF_LINE_HEIGHT + DIFF_SECTION_PADDING_HEIGHT ) } @@ -43,6 +60,7 @@ export function getDiffSectionEstimatedHeight({ measuredContentHeight, originalContent, modifiedContent, + changedLineCount, useIntrinsicImageHeight }: DiffSectionBodyHeightInput & { collapsed: boolean }): number { if (collapsed) { @@ -55,6 +73,7 @@ export function getDiffSectionEstimatedHeight({ measuredContentHeight, originalContent, modifiedContent, + changedLineCount, useIntrinsicImageHeight }) ?? MIN_DIFF_SECTION_BODY_HEIGHT) )