fix(editor): scroll combined diffs with keyboard navigation (#35004)

* fix(editor): keep combined diff keyboard cursors visible

* fix(editor): retain focused diff sections through height changes

* test(editor): bound combined diff reveal and measurement work

* fix(editor): share combined diff viewport intent across review hosts

* test: verify worktree tree navigation cancels cursor reveal

* test: assert cursor reveal clears before tree scroll

* test: use typed diff instrumentation and fixture convention

* fix: declare implemented diff content size event

* Simplify combined diff cursor reveal and keep focused regression tests

Keep keyboard cursor geometry mapped to the surrounding viewport and sticky header. Remove speculative focused-row retention, deferred following and its manual-input cancellation; restore existing section-height measurement.

Retain split/inline real-Monaco keyboard cases, click-source preservation, and both PR Files hosts covering the review-found missing viewport markers. Drop redundant timing, tree and operation-count fixtures and restore render-range baseline tests.

src/renderer/src/components/editor/DiffSectionBody.tsx

@@ -19,6 +19,7 @@ import {
 } from './diff-editor-word-wrap-options'
 import { monacoFindOptions } from './monaco-find-options'
 import { installDiffEditorShiftWheelScroll } from './diff-editor-shift-wheel-scroll'
+import { installCombinedDiffCursorReveal } from './combined-diff-cursor-reveal'
 import { usePluginSyntaxLanguage } from './use-plugin-syntax-language'
 import {
   PLUGIN_SYNTAX_EDITOR_OPTIONS,
@@ -87,8 +88,10 @@ export function DiffSectionBody({
     diffEditorRef.current = diffEditor
     applyModelLanguage([diffEditor.getModel()?.original, diffEditor.getModel()?.modified], language)
     const cleanupShiftWheelScroll = installDiffEditorShiftWheelScroll(diffEditor)
+    const cleanupCursorReveal = installCombinedDiffCursorReveal(diffEditor)
     diffEditor.getModifiedEditor().onDidDispose(() => {
       cleanupShiftWheelScroll()
+      cleanupCursorReveal()
       if (diffEditorRef.current !== diffEditor) {
         return
       }

src/renderer/src/components/editor/DiffSectionBody.word-wrap-lifecycle.test.tsx

@@ -16,6 +16,7 @@ const { mountedEditors, syncWordWrap, cleanupShiftWheel, createEditor } = vi.hoi
     return {
       editor: {
         getModifiedEditor: () => modified,
+        getContainerDomNode: () => document.createElement('div'),
         getModel: () => null,
         onDidDispose: vi.fn(() => ({ dispose: vi.fn() }))
       },

src/renderer/src/components/editor/DiffSectionHeader.tsx

@@ -1,3 +1,4 @@
+import { combinedDiffHeaderAttributes } from './combined-diff-viewport'
 import { ChevronDown, ChevronRight, ExternalLink, Eye } from 'lucide-react'
 import type { MouseEvent, ReactElement, ReactNode } from 'react'
 import { translate } from '@/i18n/i18n'
@@ -29,6 +30,7 @@ export function DiffSectionHeader({
   const displayPath = splitPathForDisplay(path)
   return (
     <div
+      {...combinedDiffHeaderAttributes}
       className="sticky top-0 z-10 bg-background flex items-center w-full px-3 py-1.5 text-left text-xs hover:bg-accent transition-colors group cursor-pointer"
       onClick={onToggle}
     >

src/renderer/src/components/editor/combined-diff-cursor-reveal.test.tsx

@@ -0,0 +1,196 @@
+// @vitest-environment happy-dom
+import { useLayoutEffect, useRef } from 'react'
+import { act, cleanup, render } from '@testing-library/react'
+import { afterAll, afterEach, expect, it, vi } from 'vitest'
+import * as monaco from 'monaco-editor/esm/vs/editor/editor.api.js'
+import type { DiffOnMount } from '@monaco-editor/react'
+import { installCombinedDiffCursorReveal } from './combined-diff-cursor-reveal'
+import { TooltipProvider } from '@/components/ui/tooltip'
+import {
+  prFilesDiffScrollTopCache,
+  prFilesDiffViewStateCache
+} from '../pull-request-page/cache/files-diff-view'
+import {
+  reviewHostFixture,
+  reviewHostComponents,
+  originalContent,
+  modifiedContent
+} from './combined-diff-review-host.test-fixture'
+
+// Happy DOM has no canvas adapter; drawing does not participate in cursor geometry.
+const canvasContext = Object.getOwnPropertyDescriptor(HTMLCanvasElement.prototype, 'getContext')!
+Object.defineProperty(HTMLCanvasElement.prototype, 'getContext', {
+  configurable: true,
+  value: () => ({
+    clearRect() {},
+    fillRect() {},
+    beginPath() {},
+    moveTo() {},
+    lineTo() {},
+    stroke() {}
+  })
+})
+afterAll(() => Object.defineProperty(HTMLCanvasElement.prototype, 'getContext', canvasContext))
+const disposers: (() => void)[] = []
+let diff: monaco.editor.IStandaloneDiffEditor
+let viewport: HTMLDivElement
+
+function layoutBox(node: HTMLElement, rectangle: () => DOMRect) {
+  node.getBoundingClientRect = rectangle
+  node.getClientRects = () => Object.assign([rectangle()], { item: () => rectangle() })
+}
+function configureViewport(node: HTMLDivElement) {
+  if (viewport === node) {
+    return
+  }
+  viewport = node
+  Object.defineProperties(node, {
+    clientHeight: { value: 300, configurable: true },
+    offsetHeight: { value: 300, configurable: true },
+    offsetWidth: { value: 1000, configurable: true },
+    scrollHeight: { value: 2500, configurable: true }
+  })
+  layoutBox(node, () => new DOMRect(0, 100, 1000, 300))
+}
+function mountEditor(node: HTMLDivElement, options: monaco.editor.IDiffEditorConstructionOptions) {
+  const editor = monaco.editor.createDiffEditor(node, {
+    ...options,
+    automaticLayout: false,
+    dimension: { width: 1000, height: 2200 },
+    lineHeight: 20,
+    fontSize: 13,
+    renderSideBySideInlineBreakpoint: 0,
+    hideUnchangedRegions: { enabled: false }
+  })
+  const original = monaco.editor.createModel(originalContent)
+  const modified = monaco.editor.createModel(modifiedContent)
+  editor.setModel({ original, modified })
+  for (const pane of [editor.getOriginalEditor(), editor.getModifiedEditor()]) {
+    layoutBox(pane.getContainerDomNode(), () => new DOMRect(0, 130 - viewport.scrollTop, 500, 2200))
+  }
+  layoutBox(node, () => new DOMRect(0, 130 - viewport.scrollTop, 1000, 2200))
+  const header = node
+    .closest('[data-combined-diff-section-row]')
+    ?.querySelector<HTMLElement>('[data-combined-diff-section-header]')
+  if (header) {
+    layoutBox(header, () => new DOMRect(0, 100, 1000, 30))
+  }
+  return {
+    editor,
+    dispose: () => {
+      editor.dispose()
+      original.dispose()
+      modified.dispose()
+    }
+  }
+}
+vi.mock('@monaco-editor/react', () => ({
+  DiffEditor: ({
+    onMount,
+    options
+  }: {
+    onMount: DiffOnMount
+    options: monaco.editor.IDiffEditorConstructionOptions
+  }) => {
+    const ref = useRef<HTMLDivElement>(null)
+    const initial = useRef({ onMount, options })
+    useLayoutEffect(() => {
+      if (!ref.current) {
+        throw new Error('Missing editor host')
+      }
+      const mounted = mountEditor(ref.current, initial.current.options)
+      diff = mounted.editor
+      initial.current.onMount(diff, monaco)
+      return mounted.dispose
+    }, [])
+    return <div ref={ref} />
+  }
+}))
+afterEach(() => {
+  cleanup()
+  disposers.splice(0).forEach((dispose) => dispose())
+  prFilesDiffScrollTopCache.clear()
+  prFilesDiffViewStateCache.clear()
+  document.body.replaceChildren()
+  reviewHostFixture.configureViewport = undefined
+})
+
+function assertCursorVisible(pane: monaco.editor.IStandaloneCodeEditor) {
+  const cursor = pane.getScrolledVisiblePosition(pane.getPosition()!)!
+  const top = pane.getContainerDomNode().getBoundingClientRect().top + cursor.top
+  expect(top).toBeGreaterThanOrEqual(130)
+  expect(top + cursor.height).toBeLessThanOrEqual(400)
+}
+function navigate(pane: monaco.editor.IStandaloneCodeEditor) {
+  pane.focus()
+  pane.setPosition({ lineNumber: 1, column: 1 })
+  viewport.scrollTop = 0
+  for (let index = 0; index < 30; index++) {
+    pane.trigger('keyboard', 'cursorDown', null)
+  }
+  expect(pane.getPosition()?.lineNumber).toBe(31)
+  expect(viewport.scrollTop).toBeGreaterThan(0)
+  assertCursorVisible(pane)
+  pane.trigger('keyboard', 'cursorTop', null)
+  assertCursorVisible(pane)
+}
+
+it.each([true, false])(
+  'reveals keyboard navigation in the outer viewport (split=%s)',
+  async (split) => {
+    const outer = document.createElement('div')
+    outer.setAttribute('data-combined-diff-viewport', '')
+    outer.innerHTML =
+      '<div data-combined-diff-section-row><div data-combined-diff-section-header></div><div class="editor-host"></div></div>'
+    document.body.append(outer)
+    configureViewport(outer)
+    const mounted = mountEditor(outer.querySelector<HTMLDivElement>('.editor-host')!, {
+      renderSideBySide: split
+    })
+    disposers.push(mounted.dispose)
+    disposers.push(installCombinedDiffCursorReveal(mounted.editor))
+    await vi.waitFor(() => expect(mounted.editor.getLineChanges()).not.toBeNull())
+    for (const pane of [mounted.editor.getOriginalEditor(), mounted.editor.getModifiedEditor()]) {
+      navigate(pane)
+      const offset = viewport.scrollTop
+      pane.setPosition({ lineNumber: 90, column: 1 }, 'mouse')
+      expect(viewport.scrollTop).toBe(offset)
+      pane.setPosition({ lineNumber: 1, column: 1 }, 'api')
+      expect(viewport.scrollTop).toBe(offset)
+    }
+  }
+)
+
+it.each(['page', 'dialog'] as const)(
+  'reveals keyboard navigation through the actual PR Files %s host',
+  async (host) => {
+    reviewHostFixture.configureViewport = configureViewport
+    const Viewer = reviewHostComponents[host]
+    render(
+      <TooltipProvider>
+        <Viewer
+          files={[
+            { path: 'file.ts', status: 'modified', additions: 1, deletions: 1, isBinary: false }
+          ]}
+          comments={[]}
+          repoPath="/repo"
+          repoId={`reveal-${host}`}
+          prNumber={7}
+          prUrl="https://example.test/pr/7"
+          headSha="head"
+          baseSha="base"
+          pendingViewedPaths={new Set<string>()}
+          onCommentAdded={() => {}}
+          onViewedChange={async () => true}
+        />
+      </TooltipProvider>
+    )
+    await vi.waitFor(() => expect(viewport?.querySelector('.monaco-diff-editor')).toBeTruthy())
+    await vi.waitFor(() => expect(diff.getLineChanges()).not.toBeNull())
+    expect(viewport.hasAttribute('data-combined-diff-viewport')).toBe(true)
+    expect(
+      viewport.querySelector('[data-combined-diff-section-row] [data-combined-diff-section-header]')
+    ).not.toBeNull()
+    act(() => navigate(diff.getModifiedEditor()))
+  }
+)

src/renderer/src/components/editor/combined-diff-cursor-reveal.ts

@@ -0,0 +1,41 @@
+import type { editor } from 'monaco-editor'
+import {
+  COMBINED_DIFF_VIEWPORT_SELECTOR,
+  COMBINED_DIFF_ROW_SELECTOR,
+  COMBINED_DIFF_HEADER_SELECTOR
+} from './combined-diff-viewport'
+import { hasLayoutBox } from '@/hooks/virtualized-hidden-row-measurement'
+
+export function installCombinedDiffCursorReveal(diff: editor.IStandaloneDiffEditor): () => void {
+  const root = diff.getContainerDomNode()
+  const viewport = root.closest<HTMLElement>(COMBINED_DIFF_VIEWPORT_SELECTOR)
+  if (!viewport) {
+    return () => {}
+  }
+  const header = root
+    .closest(COMBINED_DIFF_ROW_SELECTOR)
+    ?.querySelector<HTMLElement>(COMBINED_DIFF_HEADER_SELECTOR)
+  const subscriptions = [diff.getOriginalEditor(), diff.getModifiedEditor()].map((pane) =>
+    pane.onDidChangeCursorPosition((event) => {
+      const node = pane.getContainerDomNode()
+      if (event.source !== 'keyboard' || !node || !hasLayoutBox(node) || !hasLayoutBox(viewport)) {
+        return
+      }
+      const cursor = pane.getScrolledVisiblePosition(event.position)
+      if (!cursor) {
+        return
+      }
+      const viewportTop = viewport.getBoundingClientRect().top + viewport.clientTop
+      const top = node.getBoundingClientRect().top + cursor.top
+      const visibleTop = viewportTop + (header?.getBoundingClientRect().height ?? 0)
+      const visibleBottom = viewportTop + viewport.clientHeight
+      const delta =
+        top < visibleTop ? top - visibleTop : Math.max(0, top + cursor.height - visibleBottom)
+      // Keyboard reveals are user intent; unmarked scrolls update the existing outer anchor.
+      if (delta !== 0) {
+        viewport.scrollTop += delta
+      }
+    })
+  )
+  return () => subscriptions.forEach((subscription) => subscription.dispose())
+}

src/renderer/src/components/editor/combined-diff-review-host.test-fixture.ts

@@ -0,0 +1,83 @@
+import React from 'react'
+import { vi } from 'vitest'
+import type * as VirtualBindings from '@tanstack/react-virtual'
+import type { DiffSection } from './diff-section-types'
+import { PRFilesCombinedDiffViewer as PageViewer } from '../pull-request-page/files/combined-diff-viewer'
+import { PRFilesCombinedDiffViewer as DialogViewer } from '../github-item-dialog/inspect-pull-request/pr-files-combined-diff-viewer'
+
+export const reviewHostFixture: {
+  configureViewport?: (node: HTMLDivElement) => void
+} = {}
+export const reviewHostComponents = { page: PageViewer, dialog: DialogViewer }
+export const originalContent = Array.from({ length: 100 }, (_, i) => `line ${i}`).join('\n')
+export const modifiedContent = originalContent.replace('line 10\n', 'changed\n')
+
+type SectionSetter = React.Dispatch<React.SetStateAction<DiffSection[]>>
+const loadSection = ({ index, setSections }: { index: number; setSections: SectionSetter }) =>
+  setSections((prev) =>
+    prev[index]?.loading
+      ? prev.map((section, row) =>
+          row === index ? { ...section, originalContent, modifiedContent, loading: false } : section
+        )
+      : prev
+  )
+vi.mock('@tanstack/react-virtual', async (importOriginal) => {
+  const actual = await importOriginal<typeof VirtualBindings>()
+  return {
+    ...actual,
+    useVirtualizer: (
+      options: Parameters<typeof actual.useVirtualizer<HTMLDivElement, Element>>[0]
+    ) =>
+      actual.useVirtualizer({
+        ...options,
+        initialRect: { width: 1000, height: 300 },
+        getScrollElement: () => {
+          const node = options.getScrollElement()
+          if (node) {
+            reviewHostFixture.configureViewport?.(node)
+          }
+          return node
+        }
+      })
+  }
+})
+vi.mock('@/store', () => ({
+  useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
+    selector({
+      settings: { theme: 'dark', diffWordWrap: true },
+      editorFontZoomLevel: 0,
+      worktreesByRepo: {},
+      runtimeEnvironments: [],
+      folderWorkspaces: []
+    })
+}))
+vi.mock('@/hooks/use-document-dark-theme', () => ({ useDocumentDarkTheme: () => false }))
+vi.mock('@/lib/monaco-setup', async () => ({
+  monaco: await import('monaco-editor/esm/vs/editor/editor.api.js')
+}))
+vi.mock('../diff-comments/useDiffCommentDecorator', () => ({ useDiffCommentDecorator: () => {} }))
+vi.mock('./combined-diff/browse-files/combined-diff-file-tree', () => ({
+  CombinedDiffFileTree: () => null
+}))
+vi.mock('../pull-request-page/files/toolbar', () => ({ PRFilesDiffToolbar: () => null }))
+vi.mock('../github-item-dialog/inspect-pull-request/pr-files-combined-diff-toolbar', () => ({
+  PRFilesCombinedDiffToolbar: () => null
+}))
+vi.mock('../pull-request-page/files/section-loader', () => ({
+  usePRFileSectionLoader: ({ setSections }: { setSections: SectionSetter }) => ({
+    loadSection: React.useCallback(
+      (index: number) => loadSection({ index, setSections }),
+      [setSections]
+    ),
+    retrySection: () => {},
+    toggleSection: () => {},
+    setAllSectionsCollapsed: () => {}
+  })
+}))
+vi.mock('../github-item-dialog/inspect-pull-request/pr-files-combined-diff-load', () => ({
+  loadPRFilesCombinedDiffSection: (args: Parameters<typeof loadSection>[0]) => loadSection(args),
+  retryPRFilesCombinedDiffSection: () => {},
+  togglePRFilesCombinedDiffSection: () => {},
+  setAllPRFilesCombinedDiffSectionsCollapsed: () => {},
+  addPRFilesCombinedDiffLineComment: () => Promise.resolve(false)
+}))

src/renderer/src/components/editor/combined-diff-viewport.ts

@@ -0,0 +1,6 @@
+export const combinedDiffViewportAttributes = { 'data-combined-diff-viewport': '' } as const
+export const combinedDiffRowAttributes = { 'data-combined-diff-section-row': '' } as const
+export const combinedDiffHeaderAttributes = { 'data-combined-diff-section-header': '' } as const
+export const COMBINED_DIFF_VIEWPORT_SELECTOR = '[data-combined-diff-viewport]'
+export const COMBINED_DIFF_ROW_SELECTOR = '[data-combined-diff-section-row]'
+export const COMBINED_DIFF_HEADER_SELECTOR = '[data-combined-diff-section-header]'

src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-section-list.tsx

@@ -1,3 +1,7 @@
+import {
+  combinedDiffViewportAttributes,
+  combinedDiffRowAttributes
+} from '../../combined-diff-viewport'
 import type React from 'react'
 import { useMemo } from 'react'
 import type { Virtualizer } from '@tanstack/react-virtual'
@@ -82,6 +86,7 @@ export function CombinedDiffSectionList({
   return (
     <div className="relative min-w-0 flex-1">
       <div
+        {...combinedDiffViewportAttributes}
         ref={setScrollContainerRef}
         className="combined-diff-scroll-container h-full overflow-auto pr-5 scrollbar-editor"
         onWheel={markDirectScrollInput}
@@ -99,7 +104,7 @@ export function CombinedDiffSectionList({
               <div
                 key={virtualItem.key}
                 data-index={virtualItem.index}
-                data-combined-diff-section-row
+                {...combinedDiffRowAttributes}
                 data-combined-diff-section-key={section.key}
                 ref={virtualizer.measureElement}
                 className="absolute left-0 top-0 w-full"

src/renderer/src/components/github-item-dialog/inspect-pull-request/pr-files-combined-diff-body.tsx

@@ -1,3 +1,7 @@
+import {
+  combinedDiffViewportAttributes,
+  combinedDiffRowAttributes
+} from '@/components/editor/combined-diff-viewport'
 import React from 'react'
 import type { Virtualizer } from '@tanstack/react-virtual'
 import type { editor as monacoEditor } from 'monaco-editor'
@@ -108,7 +112,11 @@ export function PRFilesCombinedDiffBody({
           onCollapsedChange={setFileTreeCollapsed}
           onNavigate={handleTreeNavigate}
         />
-        <div ref={scrollContainerRef} className="min-w-0 flex-1 overflow-auto scrollbar-editor">
+        <div
+          {...combinedDiffViewportAttributes}
+          ref={scrollContainerRef}
+          className="min-w-0 flex-1 overflow-auto scrollbar-editor"
+        >
           <div className="relative w-full" style={{ height: `${virtualizer.getTotalSize()}px` }}>
             {virtualizer.getVirtualItems().map((virtualItem) => {
               const section = sections[virtualItem.index]
@@ -118,6 +126,7 @@ export function PRFilesCombinedDiffBody({
               return (
                 <div
                   key={virtualItem.key}
+                  {...combinedDiffRowAttributes}
                   data-index={virtualItem.index}
                   ref={virtualizer.measureElement}
                   className="absolute left-0 top-0 w-full"

src/renderer/src/components/pull-request-page/files/combined-diff-viewer.tsx

@@ -1,3 +1,7 @@
+import {
+  combinedDiffViewportAttributes,
+  combinedDiffRowAttributes
+} from '@/components/editor/combined-diff-viewport'
 import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'
 import { useVirtualizer } from '@tanstack/react-virtual'
 import type { editor as monacoEditor } from 'monaco-editor'
@@ -329,7 +333,11 @@ export function PRFilesCombinedDiffViewer({
           onCollapsedChange={setFileTreeCollapsed}
           onNavigate={handleTreeNavigate}
         />
-        <div ref={scrollContainerRef} className="min-w-0 flex-1 overflow-auto scrollbar-editor">
+        <div
+          {...combinedDiffViewportAttributes}
+          ref={scrollContainerRef}
+          className="min-w-0 flex-1 overflow-auto scrollbar-editor"
+        >
           <div className="relative w-full" style={{ height: `${virtualizer.getTotalSize()}px` }}>
             {virtualizer.getVirtualItems().map((virtualItem) => {
               const section = sections[virtualItem.index]
@@ -339,6 +347,7 @@ export function PRFilesCombinedDiffViewer({
               return (
                 <div
                   key={virtualItem.key}
+                  {...combinedDiffRowAttributes}
                   data-index={virtualItem.index}
                   ref={virtualizer.measureElement}
                   className="absolute left-0 top-0 w-full"