Add Previous/Next change navigation buttons to the Diff View (#6668)

* feat(diff): add Previous/Next change navigation buttons to Diff View

Add up/down navigation buttons to the single-file Diff View toolbar so
users can jump between change regions (hunks) without scrolling, matching
IntelliJ/Android Studio. The buttons bridge the Monaco diff editor to the
header via an instance-scoped DiffNavigationProvider and call Monaco's
goToDiff('previous'|'next'); they disable when the file has no changes.

Closes #6215

Co-authored-by: Orca <help@stably.ai>

* test(diff): cover stale onDidUpdateDiff guard in fast-swap

Address CodeRabbit nitpick: prove that an update fired from a replaced
editor is ignored after a new editor registers (the subscription is
disposed on re-register). The fake editor's dispose now clears its
callback, matching real Monaco subscription semantics.

Co-authored-by: Orca <help@stably.ai>

* fix(diff): isolate navigation registration updates

* fix(diff): use arrows for change navigation

* fix(editor): tighten header action spacing

---------

Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Brennan Benson <brennanbenson@Brennans-MacBook-Pro.local>
This commit is contained in:
Brennan Benson 2026-07-12 14:59:18 -07:00 committed by GitHub
parent d509ee7abe
commit b93ec86fef
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 403 additions and 59 deletions

View File

@ -970,7 +970,8 @@
.editor-header {
display: flex;
align-items: center;
gap: 12px;
/* Keep adjacent actions aligned with the app's compact toolbar spacing. */
gap: 4px;
min-height: 36px;
padding: 6px 14px;
background: var(--editor-surface);

View File

@ -24,6 +24,7 @@ import { useDiffViewerLargeDiffLifecycle } from './useDiffViewerLargeDiffLifecyc
import { getDiffViewerLargeDiffSaveAction } from './diff-viewer-large-diff-save-action'
import type { DiffViewerProps } from './diff-viewer-props'
import { buildDiffEditorWordWrapOptions } from './diff-editor-word-wrap-options'
import { useDiffEditorRegistration } from './diff-navigation-context'
export default function DiffViewer({
modelKey,
@ -72,6 +73,7 @@ export default function DiffViewer({
(settings?.theme === 'system' && window.matchMedia('(prefers-color-scheme: dark)').matches)
const diffEditorRef = useRef<editor.IStandaloneDiffEditor | null>(null)
const { registerDiffEditor, unregisterDiffEditor } = useDiffEditorRegistration()
const diffBodyRef = useRef<HTMLDivElement | null>(null)
const lineNumberOptionsSubRef = useRef<{ dispose: () => void } | null>(null)
const [modifiedEditor, setModifiedEditor] = useState<editor.ICodeEditor | null>(null)
@ -244,10 +246,16 @@ export default function DiffViewer({
// must not keep comment decorators or save handlers talking to disposed UI.
lineNumberOptionsSubRef.current?.dispose()
lineNumberOptionsSubRef.current = null
// Why: capture before nulling so we unregister the exact instance the
// navigator may still hold (identity guard no-ops a stale dispose).
const fallenBackEditor = diffEditorRef.current
diffEditorRef.current = null
if (fallenBackEditor) {
unregisterDiffEditor(fallenBackEditor)
}
setModifiedEditor(null)
setPopover(null)
}, [])
}, [unregisterDiffEditor])
const handleSubmitComment = async (body: string): Promise<void> => {
if (!popover) {
@ -307,6 +315,7 @@ export default function DiffViewer({
const handleMount: DiffOnMount = useCallback(
(diffEditor, monaco) => {
diffEditorRef.current = diffEditor
registerDiffEditor(diffEditor)
lineNumberOptionsSubRef.current?.dispose()
lineNumberOptionsSubRef.current = applyDiffEditorLineNumberOptions(diffEditor, sideBySide)
@ -363,11 +372,12 @@ export default function DiffViewer({
lineNumberOptionsSubRef.current?.dispose()
lineNumberOptionsSubRef.current = null
diffEditorRef.current = null
unregisterDiffEditor(diffEditor)
setModifiedEditor(null)
setPopover(null)
})
},
[editable, setupCopy, modelKey, filePath, sideBySide]
[editable, setupCopy, modelKey, filePath, sideBySide, registerDiffEditor, unregisterDiffEditor]
)
// Why: VS Code snapshots diff view state on deactivation, not on scroll events.

View File

@ -10,6 +10,7 @@ import { requestEditorFileSave } from './editor-autosave'
import { exportActiveMarkdownToPdf } from './export-active-markdown'
import type { EditorToggleValue } from './EditorViewToggle'
import { EditorPanelShell } from './EditorPanelShell'
import { DiffNavigationProvider } from './diff-navigation-context'
import { canUseChangesModeForFile } from './editor-panel-file-mode'
import { getEditorPanelRenderModel } from './editor-panel-render-model'
import { useClosedEditorTabCleanup } from './useClosedEditorTabCleanup'
@ -351,56 +352,59 @@ function EditorPanelInner({
markdownTableOfContentsVisible[markdownDocumentStateFileId] ?? false
return (
<EditorPanelShell
panelRef={setPanelRef}
activeFile={activeFile}
activeViewStateId={activeViewStateId}
model={model}
copiedPathVisible={copiedPathToast?.fileId === activeFile.id}
showMarkdownTableOfContents={isMarkdownTableOfContentsVisible}
canShowMarkdownFrontmatterToggle={canShowMarkdownFrontmatterToggle}
markdownFrontmatterVisible={isMarkdownFrontmatterVisible}
sideBySide={sideBySide}
openFiles={openFiles}
fileContents={fileContents}
diffContents={diffContents}
editorDrafts={editorDrafts}
pendingEditorReveal={pendingEditorReveal}
renameDialogFile={renameDialogFile}
renameError={renameError}
disableRenameBrowse={disableRenameBrowse}
onCopyPath={() => void handleCopyPath()}
onOpenDiffTargetFile={handleOpenDiffTargetFile}
onOpenPreviewToSide={handleOpenPreviewToSide}
onOpenMarkdownPreview={handleOpenMarkdownPreview}
onOpenContainingFolder={handleOpenContainingFolder}
onToggleSideBySide={() => setSideBySide((prev) => !prev)}
onEditorToggleChange={handleEditorToggleChange}
onToggleMarkdownTableOfContents={() =>
setMarkdownTableOfContentsVisible(
markdownDocumentStateFileId,
!isMarkdownTableOfContentsVisible
)
}
onToggleMarkdownFrontmatter={() =>
setMarkdownFrontmatterVisible(markdownDocumentStateFileId, !isMarkdownFrontmatterVisible)
}
onExportMarkdownToPdf={() =>
void exportActiveMarkdownToPdf({ fileId: activeFile.id, root: panelRef.current })
}
onContentChange={handleContentChange}
onContentChangeForFile={handleContentChangeForFile}
onDirtyStateHint={handleDirtyStateHint}
onSave={handleSave}
onSaveForFile={handleSaveForFile}
onReloadContent={reloadContent}
onCloseMarkdownTableOfContents={() =>
setMarkdownTableOfContentsVisible(markdownDocumentStateFileId, false)
}
onCloseRenameDialog={closeRenameDialog}
onRenameConfirm={handleRenameConfirm}
markdownAnnotationsEnabled={markdownAnnotationsEnabled}
/>
// Why: each split pane needs an isolated bridge between its diff editor and header controls.
<DiffNavigationProvider>
<EditorPanelShell
panelRef={setPanelRef}
activeFile={activeFile}
activeViewStateId={activeViewStateId}
model={model}
copiedPathVisible={copiedPathToast?.fileId === activeFile.id}
showMarkdownTableOfContents={isMarkdownTableOfContentsVisible}
canShowMarkdownFrontmatterToggle={canShowMarkdownFrontmatterToggle}
markdownFrontmatterVisible={isMarkdownFrontmatterVisible}
sideBySide={sideBySide}
openFiles={openFiles}
fileContents={fileContents}
diffContents={diffContents}
editorDrafts={editorDrafts}
pendingEditorReveal={pendingEditorReveal}
renameDialogFile={renameDialogFile}
renameError={renameError}
disableRenameBrowse={disableRenameBrowse}
onCopyPath={() => void handleCopyPath()}
onOpenDiffTargetFile={handleOpenDiffTargetFile}
onOpenPreviewToSide={handleOpenPreviewToSide}
onOpenMarkdownPreview={handleOpenMarkdownPreview}
onOpenContainingFolder={handleOpenContainingFolder}
onToggleSideBySide={() => setSideBySide((prev) => !prev)}
onEditorToggleChange={handleEditorToggleChange}
onToggleMarkdownTableOfContents={() =>
setMarkdownTableOfContentsVisible(
markdownDocumentStateFileId,
!isMarkdownTableOfContentsVisible
)
}
onToggleMarkdownFrontmatter={() =>
setMarkdownFrontmatterVisible(markdownDocumentStateFileId, !isMarkdownFrontmatterVisible)
}
onExportMarkdownToPdf={() =>
void exportActiveMarkdownToPdf({ fileId: activeFile.id, root: panelRef.current })
}
onContentChange={handleContentChange}
onContentChangeForFile={handleContentChangeForFile}
onDirtyStateHint={handleDirtyStateHint}
onSave={handleSave}
onSaveForFile={handleSaveForFile}
onReloadContent={reloadContent}
onCloseMarkdownTableOfContents={() =>
setMarkdownTableOfContentsVisible(markdownDocumentStateFileId, false)
}
onCloseRenameDialog={closeRenameDialog}
onRenameConfirm={handleRenameConfirm}
markdownAnnotationsEnabled={markdownAnnotationsEnabled}
/>
</DiffNavigationProvider>
)
}

View File

@ -1,5 +1,5 @@
import { useMemo } from 'react'
import { Columns2, Eye, FileText, ListTree, Rows2 } from 'lucide-react'
import { ArrowDown, ArrowUp, Columns2, Eye, FileText, ListTree, Rows2 } from 'lucide-react'
import { useAppStore } from '@/store'
import { selectWorktreeDiffCommentsOrEmpty } from '@/store/worktree-diff-comments-selector'
import type { OpenFile } from '@/store/slices/editor'
@ -14,6 +14,7 @@ import { DiffNotesSendMenu } from './DiffNotesSendMenu'
import { EditorPanelMarkdownActionsMenu } from './EditorPanelMarkdownActionsMenu'
import { translate } from '@/i18n/i18n'
import { EditorPanelHeaderPath } from './EditorPanelHeaderPath'
import { useDiffNavigation } from './diff-navigation-context'
type EditorPanelHeaderProps = {
activeFile: OpenFile
@ -92,6 +93,7 @@ export function EditorPanelHeader({
() => diffComments.filter((comment) => comment.filePath === activeFile.relativePath),
[activeFile.relativePath, diffComments]
)
const { changeCount, goToPreviousDiff, goToNextDiff } = useDiffNavigation()
return (
<div className="editor-header">
@ -203,6 +205,52 @@ export function EditorPanelHeader({
</Tooltip>
</TooltipProvider>
)}
{isDiffSurface && (
<TooltipProvider delayDuration={300}>
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
className="p-1 rounded hover:bg-accent text-muted-foreground hover:text-foreground transition-colors flex-shrink-0 disabled:opacity-50 disabled:hover:bg-transparent disabled:hover:text-muted-foreground"
onClick={goToPreviousDiff}
aria-label={translate(
'auto.components.editor.EditorPanelHeader.2076ecfc9c',
'Previous change'
)}
disabled={changeCount === 0}
>
<ArrowUp size={14} />
</button>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={4}>
{translate('auto.components.editor.EditorPanelHeader.2076ecfc9c', 'Previous change')}
</TooltipContent>
</Tooltip>
</TooltipProvider>
)}
{isDiffSurface && (
<TooltipProvider delayDuration={300}>
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
className="p-1 rounded hover:bg-accent text-muted-foreground hover:text-foreground transition-colors flex-shrink-0 disabled:opacity-50 disabled:hover:bg-transparent disabled:hover:text-muted-foreground"
onClick={goToNextDiff}
aria-label={translate(
'auto.components.editor.EditorPanelHeader.631dab0df3',
'Next change'
)}
disabled={changeCount === 0}
>
<ArrowDown size={14} />
</button>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={4}>
{translate('auto.components.editor.EditorPanelHeader.631dab0df3', 'Next change')}
</TooltipContent>
</Tooltip>
</TooltipProvider>
)}
{hasEditorToggle && (
<EditorViewToggle
value={effectiveToggleValue}

View File

@ -0,0 +1,156 @@
// @vitest-environment happy-dom
import { act } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { editor } from 'monaco-editor'
import {
DiffNavigationProvider,
useDiffEditorRegistration,
useDiffNavigation,
type DiffEditorRegistrationContextValue,
type DiffNavigationContextValue
} from './diff-navigation-context'
type FakeDiffEditor = editor.IStandaloneDiffEditor & {
setLineChanges: (count: number) => void
fireUpdate: () => void
goToDiff: ReturnType<typeof vi.fn>
disposeUpdate: ReturnType<typeof vi.fn>
}
function createFakeEditor(initialCount: number): FakeDiffEditor {
let count = initialCount
let updateCallback: (() => void) | null = null
const disposeUpdate = vi.fn(() => {
updateCallback = null
})
const editor = {
getLineChanges: () => (count > 0 ? Array.from({ length: count }, () => ({})) : []),
goToDiff: vi.fn(),
onDidUpdateDiff: (cb: () => void) => {
updateCallback = cb
return {
dispose: disposeUpdate
}
},
setLineChanges: (next: number) => {
count = next
},
fireUpdate: () => updateCallback?.(),
disposeUpdate
} as unknown as FakeDiffEditor
return editor
}
let captured: DiffNavigationContextValue | null = null
let registration: DiffEditorRegistrationContextValue | null = null
let registrationRenderCount = 0
function Probe(): null {
captured = useDiffNavigation()
return null
}
function RegistrationProbe(): null {
registration = useDiffEditorRegistration()
registrationRenderCount += 1
return null
}
describe('DiffNavigationProvider', () => {
let container: HTMLDivElement | null = null
let root: Root | null = null
function mount(): void {
container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
act(() => {
root?.render(
<DiffNavigationProvider>
<Probe />
<RegistrationProbe />
</DiffNavigationProvider>
)
})
}
afterEach(() => {
if (root) {
act(() => root?.unmount())
}
container?.remove()
container = null
root = null
captured = null
registration = null
registrationRenderCount = 0
})
it('exposes the change count and routes nav actions to the registered editor', () => {
mount()
const editor = createFakeEditor(3)
act(() => registration?.registerDiffEditor(editor))
expect(captured?.changeCount).toBe(3)
act(() => captured?.goToNextDiff())
expect(editor.goToDiff).toHaveBeenCalledWith('next')
act(() => captured?.goToPreviousDiff())
expect(editor.goToDiff).toHaveBeenCalledWith('previous')
})
it('re-renders when onDidUpdateDiff flips the count 0 -> N (count is state)', () => {
mount()
const editor = createFakeEditor(0)
act(() => registration?.registerDiffEditor(editor))
expect(captured?.changeCount).toBe(0)
act(() => {
editor.setLineChanges(2)
editor.fireUpdate()
})
expect(captured?.changeCount).toBe(2)
expect(registrationRenderCount).toBe(1)
})
it('ignores a stale unregister for an editor that is no longer current (identity guard)', () => {
mount()
const oldEditor = createFakeEditor(1)
const newEditor = createFakeEditor(4)
// Fast-swap: new editor registers before the old one's dispose fires.
act(() => registration?.registerDiffEditor(oldEditor))
act(() => registration?.registerDiffEditor(newEditor))
expect(captured?.changeCount).toBe(4)
expect(oldEditor.disposeUpdate).toHaveBeenCalledOnce()
// A stale update from the old editor must not flip the count back: registering
// the new editor disposed the old subscription, so its callback no longer fires.
act(() => {
oldEditor.setLineChanges(9)
oldEditor.fireUpdate()
})
expect(captured?.changeCount).toBe(4)
act(() => registration?.unregisterDiffEditor(oldEditor))
// New editor's count is intact and nav still routes to it.
expect(captured?.changeCount).toBe(4)
act(() => captured?.goToNextDiff())
expect(newEditor.goToDiff).toHaveBeenCalledWith('next')
expect(oldEditor.goToDiff).not.toHaveBeenCalled()
})
it('disposes the active diff update subscription when the provider unmounts', () => {
mount()
const editor = createFakeEditor(1)
act(() => registration?.registerDiffEditor(editor))
act(() => root?.unmount())
expect(editor.disposeUpdate).toHaveBeenCalledOnce()
root = null
})
})

View File

@ -0,0 +1,115 @@
import { createContext, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'
import type { editor } from 'monaco-editor'
export type DiffEditorRegistrationContextValue = {
registerDiffEditor: (editor: editor.IStandaloneDiffEditor) => void
unregisterDiffEditor: (editor: editor.IStandaloneDiffEditor) => void
}
export type DiffNavigationContextValue = {
goToPreviousDiff: () => void
goToNextDiff: () => void
changeCount: number
}
const noop = (): void => {}
// Why: registration stays separate from changeCount so diff recomputation only
// rerenders the header controls, not the heavy Monaco DiffViewer consumer.
const DiffEditorRegistrationContext = createContext<DiffEditorRegistrationContextValue>({
registerDiffEditor: noop,
unregisterDiffEditor: noop
})
const DiffNavigationContext = createContext<DiffNavigationContextValue>({
goToPreviousDiff: noop,
goToNextDiff: noop,
changeCount: 0
})
function countChanges(diffEditor: editor.IStandaloneDiffEditor): number {
return diffEditor.getLineChanges()?.length ?? 0
}
export function DiffNavigationProvider({
children
}: {
children: React.ReactNode
}): React.JSX.Element {
const editorRef = useRef<editor.IStandaloneDiffEditor | null>(null)
const updateSubRef = useRef<{ dispose: () => void } | null>(null)
// Why: changeCount must be state, not a ref — the header is a sibling consumer
// and only re-renders (enabling the buttons) when the value object identity
// changes on the 0 -> N flip once the diff computation lands.
const [changeCount, setChangeCount] = useState(0)
const registerDiffEditor = useCallback((diffEditor: editor.IStandaloneDiffEditor) => {
editorRef.current = diffEditor
// Hold at most one update subscription; replace any prior editor's.
updateSubRef.current?.dispose()
updateSubRef.current = diffEditor.onDidUpdateDiff(() => {
// Why: ignore updates from an editor that is no longer current so a stale
// subscription in the fast-swap case can't write a wrong count.
if (editorRef.current === diffEditor) {
setChangeCount(countChanges(diffEditor))
}
})
setChangeCount(countChanges(diffEditor))
}, [])
const unregisterDiffEditor = useCallback((diffEditor: editor.IStandaloneDiffEditor) => {
// Why: identity guard for the fast-swap race — a stale dispose carrying the
// old editor must not wipe a freshly-registered new one.
if (editorRef.current !== diffEditor) {
return
}
updateSubRef.current?.dispose()
updateSubRef.current = null
editorRef.current = null
setChangeCount(0)
}, [])
const goToPreviousDiff = useCallback(() => {
editorRef.current?.goToDiff('previous')
}, [])
const goToNextDiff = useCallback(() => {
editorRef.current?.goToDiff('next')
}, [])
useEffect(() => {
return () => {
updateSubRef.current?.dispose()
updateSubRef.current = null
}
}, [])
const registrationValue = useMemo(
() => ({ registerDiffEditor, unregisterDiffEditor }),
[registerDiffEditor, unregisterDiffEditor]
)
const navigationValue = useMemo(
() => ({
goToPreviousDiff,
goToNextDiff,
changeCount
}),
[goToPreviousDiff, goToNextDiff, changeCount]
)
return (
<DiffEditorRegistrationContext.Provider value={registrationValue}>
<DiffNavigationContext.Provider value={navigationValue}>
{children}
</DiffNavigationContext.Provider>
</DiffEditorRegistrationContext.Provider>
)
}
export function useDiffEditorRegistration(): DiffEditorRegistrationContextValue {
return useContext(DiffEditorRegistrationContext)
}
export function useDiffNavigation(): DiffNavigationContextValue {
return useContext(DiffNavigationContext)
}

View File

@ -11436,7 +11436,9 @@
"c98ce191da": "This diff has no modified-side file to open",
"9b80bbe1de": "Open file tab",
"f0fd4174b5": "Open file tab to use rich markdown editing",
"a10d9b8337": "Open file"
"a10d9b8337": "Open file",
"2076ecfc9c": "Previous change",
"631dab0df3": "Next change"
},
"EditorPanelMarkdownActionsMenu": {
"3e0ce48c24": "Export as PDF",

View File

@ -11436,7 +11436,9 @@
"c98ce191da": "Este diff no tiene ningún archivo del lado modificado para abrir",
"9b80bbe1de": "Abrir pestaña de archivo",
"f0fd4174b5": "Abre la pestaña de archivo para usar la edición enriquecida de Markdown",
"a10d9b8337": "Abrir archivo"
"a10d9b8337": "Abrir archivo",
"2076ecfc9c": "Previous change",
"631dab0df3": "Next change"
},
"EditorPanelMarkdownActionsMenu": {
"3e0ce48c24": "Exportar como PDF",

View File

@ -11436,7 +11436,9 @@
"c98ce191da": "この差分には開くための変更側ファイルがありません",
"9b80bbe1de": "ファイルタブを開く",
"f0fd4174b5": "ファイル タブを開いてリッチ markdown 編集を使用する",
"a10d9b8337": "ファイルを開く"
"a10d9b8337": "ファイルを開く",
"2076ecfc9c": "Previous change",
"631dab0df3": "Next change"
},
"EditorPanelMarkdownActionsMenu": {
"3e0ce48c24": "PDFとしてエクスポート",

View File

@ -11436,7 +11436,9 @@
"c98ce191da": "이 차이점에는 열 수 있는 수정된 측면 파일이 없습니다.",
"9b80bbe1de": "파일 탭 열기",
"f0fd4174b5": "풍부한 markdown 편집을 사용하려면 파일 탭을 엽니다.",
"a10d9b8337": "파일 열기"
"a10d9b8337": "파일 열기",
"2076ecfc9c": "Previous change",
"631dab0df3": "Next change"
},
"EditorPanelMarkdownActionsMenu": {
"3e0ce48c24": "PDF로 내보내기",

View File

@ -11436,7 +11436,9 @@
"c98ce191da": "此差异没有可打开的修改端文件",
"9b80bbe1de": "打开文件选项卡",
"f0fd4174b5": "打开文件选项卡以使用丰富的 Markdown 编辑",
"a10d9b8337": "打开文件"
"a10d9b8337": "打开文件",
"2076ecfc9c": "Previous change",
"631dab0df3": "Next change"
},
"EditorPanelMarkdownActionsMenu": {
"3e0ce48c24": "导出为 PDF",