From ff8accba87006f70a4d4fde3f68e850c973f07f6 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Fri, 15 May 2026 18:46:43 -0700
Subject: [PATCH] Fix full-height image diffs in combined view

Preserve intrinsic image height for binary image diffs in combined View all mode while keeping text diffs on measured Monaco heights.
---
 .../src/components/editor/DiffSectionItem.tsx | 28 +++----
 .../src/components/editor/ImageDiffViewer.tsx | 57 ++++++++++---
 .../src/components/editor/ImageViewer.tsx     | 37 ++++++--
 .../editor/diff-section-layout.test.ts        | 84 +++++++++++++++++++
 .../components/editor/diff-section-layout.ts  | 38 +++++++++
 5 files changed, 210 insertions(+), 34 deletions(-)
 create mode 100644 src/renderer/src/components/editor/diff-section-layout.test.ts
 create mode 100644 src/renderer/src/components/editor/diff-section-layout.ts

diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx
index 351bb9631..b4e6dea45 100644
--- a/src/renderer/src/components/editor/DiffSectionItem.tsx
+++ b/src/renderer/src/components/editor/DiffSectionItem.tsx
@@ -23,8 +23,10 @@ import { getDiffCommentPopoverTop } from '../diff-comments/diff-comment-popover-
 import { applyDiffEditorLineNumberOptions } from './diff-editor-line-number-options'
 import { computeLineStats } from './diff-line-stats'
 import { DiffSectionHeader } from './DiffSectionHeader'
+import { getDiffSectionBodyHeight, isIntrinsicHeightImageDiff } from './diff-section-layout'
 import type { DiffSection } from './diff-section-types'
 import type { DiffComment } from '../../../../shared/types'
+import { cn } from '@/lib/utils'
 import { isDiffComment } from '@/lib/diff-comment-compat'
 
 const ImageDiffViewer = lazy(() => import('./ImageDiffViewer'))
@@ -217,6 +219,15 @@ export function DiffSectionItem({
         : computeLineStats(section.originalContent, section.modifiedContent, section.status),
     [section.loading, section.originalContent, section.modifiedContent, section.status]
   )
+  // 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)
+  const sectionBodyHeight = getDiffSectionBodyHeight({
+    measuredContentHeight: sectionHeight,
+    originalContent: section.originalContent,
+    modifiedContent: section.modifiedContent,
+    useIntrinsicImageHeight
+  })
 
   const handleOpenInEditor = (e: React.MouseEvent): void => {
     e.stopPropagation()
@@ -292,20 +303,8 @@ export function DiffSectionItem({
 
       {!section.collapsed && (
         <div
-          className="relative"
-          style={{
-            height: sectionHeight
-              ? sectionHeight + 19
-              : Math.max(
-                  60,
-                  Math.max(
-                    section.originalContent.split('\n').length,
-                    section.modifiedContent.split('\n').length
-                  ) *
-                    19 +
-                    19
-                )
-          }}
+          className={cn('relative', useIntrinsicImageHeight && 'overflow-visible')}
+          style={sectionBodyHeight === undefined ? undefined : { height: sectionBodyHeight }}
         >
           {popover && (
             // Why: key by lineNumber so the popover remounts when the anchor
@@ -332,6 +331,7 @@ export function DiffSectionItem({
                 filePath={section.path}
                 mimeType={section.diffResult.mimeType}
                 sideBySide={sideBySide}
+                layout={useIntrinsicImageHeight ? 'intrinsic' : 'fill'}
               />
             ) : (
               <div className="flex h-full items-center justify-center px-6 text-center">
diff --git a/src/renderer/src/components/editor/ImageDiffViewer.tsx b/src/renderer/src/components/editor/ImageDiffViewer.tsx
index 2d2622702..9b6b4d849 100644
--- a/src/renderer/src/components/editor/ImageDiffViewer.tsx
+++ b/src/renderer/src/components/editor/ImageDiffViewer.tsx
@@ -1,4 +1,5 @@
 import { type JSX } from 'react'
+import { cn } from '@/lib/utils'
 import ImageViewer from './ImageViewer'
 
 type ImageDiffViewerProps = {
@@ -7,24 +8,39 @@ type ImageDiffViewerProps = {
   filePath: string
   mimeType?: string
   sideBySide: boolean
+  layout?: 'fill' | 'intrinsic'
 }
 
 function ImageDiffPane({
   label,
   content,
   filePath,
-  mimeType
+  mimeType,
+  layout
 }: {
   label: string
   content: string
   filePath: string
   mimeType?: string
+  layout: 'fill' | 'intrinsic'
 }): JSX.Element {
+  const isIntrinsicLayout = layout === 'intrinsic'
+
   if (!content) {
     return (
-      <div className="flex h-full min-h-0 flex-col overflow-hidden rounded-md bg-muted/10">
+      <div
+        className={cn(
+          'flex min-h-0 flex-col overflow-hidden rounded-md bg-muted/10',
+          isIntrinsicLayout ? 'h-auto' : 'h-full'
+        )}
+      >
         <div className="px-3 py-2 text-xs font-medium text-muted-foreground">{label}</div>
-        <div className="flex flex-1 items-center justify-center bg-muted/20 p-6 text-sm text-muted-foreground">
+        <div
+          className={cn(
+            'flex items-center justify-center bg-muted/20 p-6 text-sm text-muted-foreground',
+            isIntrinsicLayout ? 'min-h-32' : 'flex-1'
+          )}
+        >
           No preview
         </div>
       </div>
@@ -32,10 +48,15 @@ function ImageDiffPane({
   }
 
   return (
-    <div className="flex h-full min-h-0 flex-col overflow-hidden rounded-md bg-muted/10">
+    <div
+      className={cn(
+        'flex min-h-0 flex-col overflow-hidden rounded-md bg-muted/10',
+        isIntrinsicLayout ? 'h-auto' : 'h-full'
+      )}
+    >
       <div className="px-3 py-2 text-xs font-medium text-muted-foreground">{label}</div>
-      <div className="min-h-0 flex-1">
-        <ImageViewer content={content} filePath={filePath} mimeType={mimeType} />
+      <div className={cn('min-h-0', isIntrinsicLayout ? 'flex-none' : 'flex-1')}>
+        <ImageViewer content={content} filePath={filePath} mimeType={mimeType} layout={layout} />
       </div>
     </div>
   )
@@ -46,22 +67,30 @@ export default function ImageDiffViewer({
   modifiedContent,
   filePath,
   mimeType,
-  sideBySide
+  sideBySide,
+  layout = 'fill'
 }: ImageDiffViewerProps): JSX.Element {
+  const isIntrinsicLayout = layout === 'intrinsic'
   // Why: in inline (single-column) mode the grid defaults to equal row
   // heights, which squishes each preview into half the panel. Using
   // minmax(32rem, 1fr) ensures content panes are tall enough to show a
   // full page, and overflow-y-auto lets the user scroll between them.
   // Empty "No preview" panes collapse to auto height.
-  const gridRowStyle = !sideBySide
-    ? {
-        gridTemplateRows: `${originalContent ? 'minmax(32rem, 1fr)' : 'auto'} ${modifiedContent ? 'minmax(32rem, 1fr)' : 'auto'}`
-      }
-    : undefined
+  const gridRowStyle =
+    !sideBySide && !isIntrinsicLayout
+      ? {
+          gridTemplateRows: `${originalContent ? 'minmax(32rem, 1fr)' : 'auto'} ${modifiedContent ? 'minmax(32rem, 1fr)' : 'auto'}`
+        }
+      : undefined
 
   return (
     <div
-      className={`grid h-full min-h-0 gap-3 p-3 ${sideBySide ? 'grid-cols-2' : 'grid-cols-1 overflow-y-auto'}`}
+      className={cn(
+        'grid min-h-0 gap-3 p-3',
+        isIntrinsicLayout ? 'h-auto' : 'h-full',
+        sideBySide ? 'grid-cols-2' : 'grid-cols-1',
+        !sideBySide && !isIntrinsicLayout && 'overflow-y-auto'
+      )}
       style={gridRowStyle}
     >
       <ImageDiffPane
@@ -69,12 +98,14 @@ export default function ImageDiffViewer({
         content={originalContent}
         filePath={filePath}
         mimeType={mimeType}
+        layout={layout}
       />
       <ImageDiffPane
         label="Modified"
         content={modifiedContent}
         filePath={filePath}
         mimeType={mimeType}
+        layout={layout}
       />
     </div>
   )
diff --git a/src/renderer/src/components/editor/ImageViewer.tsx b/src/renderer/src/components/editor/ImageViewer.tsx
index 15e1abf89..e8b9a55a9 100644
--- a/src/renderer/src/components/editor/ImageViewer.tsx
+++ b/src/renderer/src/components/editor/ImageViewer.tsx
@@ -1,6 +1,7 @@
 import { Image as ImageIcon, RotateCcw, X, ZoomIn, ZoomOut } from 'lucide-react'
 import { type JSX, useEffect, useMemo, useState } from 'react'
 import { Dialog, DialogContent, DialogDescription, DialogTitle } from '@/components/ui/dialog'
+import { cn } from '@/lib/utils'
 import PdfViewer from './PdfViewer'
 
 const FALLBACK_IMAGE_MIME_TYPE = 'image/png'
@@ -12,12 +13,14 @@ type ImageViewerProps = {
   content: string
   filePath: string
   mimeType?: string
+  layout?: 'fill' | 'intrinsic'
 }
 
 export default function ImageViewer({
   content,
   filePath,
-  mimeType = FALLBACK_IMAGE_MIME_TYPE
+  mimeType = FALLBACK_IMAGE_MIME_TYPE,
+  layout = 'fill'
 }: ImageViewerProps): JSX.Element {
   const [imageError, setImageError] = useState(false)
   const [isPopupOpen, setIsPopupOpen] = useState(false)
@@ -29,6 +32,7 @@ export default function ImageViewer({
   const filename = useMemo(() => filePath.split(/[/\\]/).pop() || filePath, [filePath])
   const cleanedContent = useMemo(() => content.replace(/\s/g, ''), [content])
   const isPdf = mimeType === 'application/pdf'
+  const isIntrinsicLayout = layout === 'intrinsic'
   const [previewUrl, setPreviewUrl] = useState<string | null>(null)
   const estimatedSize = useMemo(() => {
     const bytes = Math.floor((cleanedContent.length * 3) / 4)
@@ -70,7 +74,12 @@ export default function ImageViewer({
 
   if (imageError) {
     return (
-      <div className="flex h-full flex-col items-center justify-center gap-3 bg-muted/20 p-8 text-sm text-muted-foreground">
+      <div
+        className={cn(
+          'flex flex-col items-center justify-center gap-3 bg-muted/20 p-8 text-sm text-muted-foreground',
+          isIntrinsicLayout ? 'min-h-64' : 'h-full'
+        )}
+      >
         <ImageIcon size={40} />
         <div>Failed to load file preview</div>
         <div className="max-w-md break-all text-center text-xs">{filename}</div>
@@ -80,7 +89,12 @@ export default function ImageViewer({
 
   if (!previewUrl) {
     return (
-      <div className="flex items-center justify-center h-full text-muted-foreground text-sm">
+      <div
+        className={cn(
+          'flex items-center justify-center text-muted-foreground text-sm',
+          isIntrinsicLayout ? 'min-h-64' : 'h-full'
+        )}
+      >
         Loading preview...
       </div>
     )
@@ -88,20 +102,29 @@ export default function ImageViewer({
 
   return (
     <>
-      <div className="flex h-full min-h-0 flex-col">
+      <div className={cn('flex min-h-0 flex-col', isIntrinsicLayout ? 'h-auto' : 'h-full')}>
         <div
-          className="flex flex-1 items-center justify-center overflow-auto bg-muted/20 p-4 cursor-pointer"
+          className={cn(
+            'flex justify-center bg-muted/20 p-4 cursor-pointer',
+            isIntrinsicLayout ? 'items-start overflow-visible' : 'flex-1 items-center overflow-auto'
+          )}
           onClick={() => setIsPopupOpen(true)}
           title="Open image in popup"
         >
           <div
-            className="flex items-center justify-center"
+            className={cn(
+              'flex justify-center',
+              isIntrinsicLayout ? 'max-w-full items-start' : 'items-center'
+            )}
             style={{ transform: `scale(${zoom})`, transformOrigin: 'center center' }}
           >
             <img
               src={previewUrl}
               alt={filename}
-              className="max-h-full max-w-full object-contain"
+              className={cn(
+                'max-w-full object-contain',
+                isIntrinsicLayout ? 'block h-auto max-h-none' : 'max-h-full'
+              )}
               onLoad={(event) => {
                 const img = event.currentTarget
                 setImageDimensions({ width: img.naturalWidth, height: img.naturalHeight })
diff --git a/src/renderer/src/components/editor/diff-section-layout.test.ts b/src/renderer/src/components/editor/diff-section-layout.test.ts
new file mode 100644
index 000000000..c86857a8b
--- /dev/null
+++ b/src/renderer/src/components/editor/diff-section-layout.test.ts
@@ -0,0 +1,84 @@
+import { describe, expect, it } from 'vitest'
+import { getDiffSectionBodyHeight, isIntrinsicHeightImageDiff } from './diff-section-layout'
+import type { GitDiffResult } from '../../../../shared/types'
+
+describe('diff section layout', () => {
+  it('uses Monaco measured content height for text diffs', () => {
+    expect(
+      getDiffSectionBodyHeight({
+        measuredContentHeight: 120,
+        originalContent: '',
+        modifiedContent: '',
+        useIntrinsicImageHeight: false
+      })
+    ).toBe(139)
+  })
+
+  it('falls back to line-count height before Monaco has mounted', () => {
+    expect(
+      getDiffSectionBodyHeight({
+        measuredContentHeight: undefined,
+        originalContent: 'one',
+        modifiedContent: 'one\ntwo\nthree',
+        useIntrinsicImageHeight: false
+      })
+    ).toBe(76)
+  })
+
+  it('keeps empty text sections visible', () => {
+    expect(
+      getDiffSectionBodyHeight({
+        measuredContentHeight: undefined,
+        originalContent: '',
+        modifiedContent: '',
+        useIntrinsicImageHeight: false
+      })
+    ).toBe(60)
+  })
+
+  it('treats zero measured height as not laid out yet', () => {
+    expect(
+      getDiffSectionBodyHeight({
+        measuredContentHeight: 0,
+        originalContent: '',
+        modifiedContent: '',
+        useIntrinsicImageHeight: false
+      })
+    ).toBe(60)
+  })
+
+  it('lets image diffs use intrinsic height in combined diff sections', () => {
+    expect(
+      getDiffSectionBodyHeight({
+        measuredContentHeight: undefined,
+        originalContent: '',
+        modifiedContent: '',
+        useIntrinsicImageHeight: true
+      })
+    ).toBeUndefined()
+  })
+
+  it('only treats real image MIME types as intrinsic-height previews', () => {
+    const pngDiff: GitDiffResult = {
+      kind: 'binary',
+      originalContent: '',
+      modifiedContent: 'base64',
+      originalIsBinary: false,
+      modifiedIsBinary: true,
+      isImage: true,
+      mimeType: 'image/png'
+    }
+    const pdfDiff: GitDiffResult = {
+      kind: 'binary',
+      originalContent: '',
+      modifiedContent: 'base64',
+      originalIsBinary: false,
+      modifiedIsBinary: true,
+      isImage: true,
+      mimeType: 'application/pdf'
+    }
+
+    expect(isIntrinsicHeightImageDiff(pngDiff)).toBe(true)
+    expect(isIntrinsicHeightImageDiff(pdfDiff)).toBe(false)
+  })
+})
diff --git a/src/renderer/src/components/editor/diff-section-layout.ts b/src/renderer/src/components/editor/diff-section-layout.ts
new file mode 100644
index 000000000..4191872ae
--- /dev/null
+++ b/src/renderer/src/components/editor/diff-section-layout.ts
@@ -0,0 +1,38 @@
+import type { GitDiffResult } from '../../../../shared/types'
+
+const DIFF_LINE_HEIGHT = 19
+const DIFF_SECTION_PADDING_HEIGHT = 19
+const MIN_DIFF_SECTION_BODY_HEIGHT = 60
+
+type DiffSectionBodyHeightInput = {
+  measuredContentHeight: number | undefined
+  originalContent: string
+  modifiedContent: string
+  useIntrinsicImageHeight: boolean
+}
+
+export function isIntrinsicHeightImageDiff(diffResult: GitDiffResult | null | undefined): boolean {
+  return diffResult?.kind === 'binary' && diffResult.mimeType?.startsWith('image/') === true
+}
+
+export function getDiffSectionBodyHeight({
+  measuredContentHeight,
+  originalContent,
+  modifiedContent,
+  useIntrinsicImageHeight
+}: DiffSectionBodyHeightInput): number | undefined {
+  if (useIntrinsicImageHeight) {
+    return undefined
+  }
+
+  if (measuredContentHeight !== undefined && measuredContentHeight > 0) {
+    return measuredContentHeight + DIFF_SECTION_PADDING_HEIGHT
+  }
+
+  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
+  )
+}