mirror of
https://github.com/stablyai/orca.git
synced 2026-10-07 00:02:29 +00:00
Fix word wrap for both panes in side-by-side diffs
Forward wrapping to both diff panes through the existing editor option path and clean up listeners. Co-authored-by: Wooseong Kim <innocarpe@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
co-authored by
Wooseong Kim
Cursor
Neil
parent
8919f3d6aa
commit
a5f28f265c
@@ -1,3 +1,5 @@
|
||||
import { useEffect, useLayoutEffect, useRef } from 'react'
|
||||
import type { editor } from 'monaco-editor'
|
||||
import { lazyWithRetry as lazy } from '@/lib/lazy-with-retry'
|
||||
import { AlertCircle, RefreshCw } from 'lucide-react'
|
||||
import { DiffEditor, type DiffOnMount } from '@monaco-editor/react'
|
||||
@@ -10,7 +12,10 @@ import { translate } from '@/i18n/i18n'
|
||||
import { LargeDiffFallback } from './LargeDiffFallback'
|
||||
import { LargeDiffLoadPrompt } from './LargeDiffLoadPrompt'
|
||||
import { buildDiffEditorWhitespaceOptions } from './diff-editor-whitespace-options'
|
||||
import { buildDiffEditorWordWrapOptions } from './diff-editor-word-wrap-options'
|
||||
import {
|
||||
buildDiffEditorWordWrapOptions,
|
||||
syncDiffEditorOriginalWordWrap
|
||||
} from './diff-editor-word-wrap-options'
|
||||
import { monacoFindOptions } from './monaco-find-options'
|
||||
import { installDiffEditorShiftWheelScroll } from './diff-editor-shift-wheel-scroll'
|
||||
|
||||
@@ -58,12 +63,57 @@ export function DiffSectionBody({
|
||||
onMount
|
||||
}: DiffSectionBodyProps): React.JSX.Element {
|
||||
const renderLimit = section.largeDiffRenderLimit?.limited ? section.largeDiffRenderLimit : null
|
||||
const handleEditorMount: DiffOnMount = (editor, monaco) => {
|
||||
const cleanupShiftWheelScroll = installDiffEditorShiftWheelScroll(editor)
|
||||
editor.onDidDispose(cleanupShiftWheelScroll)
|
||||
onMount(editor, monaco)
|
||||
const diffEditorRef = useRef<editor.IStandaloneDiffEditor | null>(null)
|
||||
const wordWrapOptionsSubRef = useRef<{ dispose: () => void } | null>(null)
|
||||
const wordWrapMountFrameRef = useRef(0)
|
||||
const diffWordWrapRef = useRef(diffWordWrap)
|
||||
useLayoutEffect(() => {
|
||||
diffWordWrapRef.current = diffWordWrap
|
||||
}, [diffWordWrap])
|
||||
const handleEditorMount: DiffOnMount = (diffEditor, monaco) => {
|
||||
diffEditorRef.current = diffEditor
|
||||
const cleanupShiftWheelScroll = installDiffEditorShiftWheelScroll(diffEditor)
|
||||
diffEditor.getModifiedEditor().onDidDispose(() => {
|
||||
cleanupShiftWheelScroll()
|
||||
if (diffEditorRef.current !== diffEditor) {
|
||||
return
|
||||
}
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = null
|
||||
diffEditorRef.current = null
|
||||
})
|
||||
// Why: Monaco applies the inline-layout wrap override after mount, once width is known.
|
||||
wordWrapMountFrameRef.current = requestAnimationFrame(() => {
|
||||
if (diffEditorRef.current !== diffEditor) {
|
||||
return
|
||||
}
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = syncDiffEditorOriginalWordWrap(
|
||||
diffEditor,
|
||||
diffWordWrapRef.current
|
||||
)
|
||||
})
|
||||
onMount(diffEditor, monaco)
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
const diffEditor = diffEditorRef.current
|
||||
if (!diffEditor) {
|
||||
return () => {
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
}
|
||||
}
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = syncDiffEditorOriginalWordWrap(diffEditor, diffWordWrap)
|
||||
return () => {
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = null
|
||||
}
|
||||
}, [diffWordWrap, sideBySide])
|
||||
|
||||
return (
|
||||
<div
|
||||
className={cn('relative', useIntrinsicImageHeight && 'overflow-visible')}
|
||||
|
||||
@@ -0,0 +1,182 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { act, cleanup, render } from '@testing-library/react'
|
||||
import { useEffect, useRef, type ComponentProps } from 'react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { DiffSectionBody } from './DiffSectionBody'
|
||||
|
||||
const { mountedEditors, syncWordWrap, cleanupShiftWheel, createEditor } = vi.hoisted(() => {
|
||||
function createEditor() {
|
||||
const listeners = new Set<() => void>()
|
||||
const modified = {
|
||||
onDidDispose: (listener: () => void) => {
|
||||
listeners.add(listener)
|
||||
return { dispose: () => listeners.delete(listener) }
|
||||
}
|
||||
}
|
||||
return {
|
||||
editor: {
|
||||
getModifiedEditor: () => modified,
|
||||
onDidDispose: vi.fn(() => ({ dispose: vi.fn() }))
|
||||
},
|
||||
disposeModified: () => listeners.forEach((listener) => listener())
|
||||
}
|
||||
}
|
||||
const mountedEditors: ReturnType<typeof createEditor>[] = []
|
||||
return {
|
||||
mountedEditors,
|
||||
syncWordWrap: vi.fn(() => ({ dispose: vi.fn() })),
|
||||
cleanupShiftWheel: vi.fn(),
|
||||
createEditor
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@monaco-editor/react', () => ({
|
||||
DiffEditor: ({
|
||||
onMount
|
||||
}: {
|
||||
onMount: (editor: (typeof mountedEditors)[number]['editor']) => void
|
||||
}) => {
|
||||
const mount = useRef(onMount)
|
||||
useEffect(() => {
|
||||
const instance = createEditor()
|
||||
mountedEditors.push(instance)
|
||||
let disposed = false
|
||||
// Monaco's React wrapper mounts asynchronously, after the parent's first effect.
|
||||
queueMicrotask(() => {
|
||||
if (!disposed) {
|
||||
mount.current(instance.editor)
|
||||
}
|
||||
})
|
||||
return () => {
|
||||
disposed = true
|
||||
instance.disposeModified()
|
||||
}
|
||||
}, [])
|
||||
return <div data-testid="monaco-diff" />
|
||||
}
|
||||
}))
|
||||
vi.mock('./diff-editor-word-wrap-options', () => ({
|
||||
buildDiffEditorWordWrapOptions: () => ({}),
|
||||
syncDiffEditorOriginalWordWrap: syncWordWrap
|
||||
}))
|
||||
vi.mock('./diff-editor-shift-wheel-scroll', () => ({
|
||||
installDiffEditorShiftWheelScroll: () => cleanupShiftWheel
|
||||
}))
|
||||
vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
|
||||
|
||||
const props: ComponentProps<typeof DiffSectionBody> = {
|
||||
section: {
|
||||
key: 'README.md',
|
||||
path: 'README.md',
|
||||
status: 'M',
|
||||
originalContent: 'original',
|
||||
modifiedContent: 'modified',
|
||||
collapsed: false,
|
||||
loading: false,
|
||||
dirty: false,
|
||||
diffResult: null,
|
||||
largeDiffRenderLimit: null
|
||||
},
|
||||
index: 0,
|
||||
sectionBodyHeight: 300,
|
||||
useIntrinsicImageHeight: false,
|
||||
isBranchMode: false,
|
||||
sideBySide: true,
|
||||
isDark: false,
|
||||
language: 'markdown',
|
||||
modelPathBase: 'wrap-lifecycle',
|
||||
isEditable: false,
|
||||
diffEditorFontSize: 13,
|
||||
diffWordWrap: true,
|
||||
onRetrySection: vi.fn(),
|
||||
onLoadDeferredSection: vi.fn(),
|
||||
onSaveLimitedDiff: vi.fn(),
|
||||
onMount: vi.fn()
|
||||
}
|
||||
const frames = new Map<number, FrameRequestCallback>()
|
||||
|
||||
beforeEach(() => {
|
||||
mountedEditors.length = 0
|
||||
syncWordWrap.mockClear()
|
||||
cleanupShiftWheel.mockClear()
|
||||
frames.clear()
|
||||
let nextFrame = 1
|
||||
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => {
|
||||
const id = nextFrame++
|
||||
frames.set(id, callback)
|
||||
return id
|
||||
})
|
||||
vi.stubGlobal('cancelAnimationFrame', (id: number) => frames.delete(id))
|
||||
})
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
async function finishMount(): Promise<void> {
|
||||
await act(async () => {
|
||||
await Promise.resolve()
|
||||
})
|
||||
}
|
||||
function runMountFrame(): void {
|
||||
const queued = [...frames.values()]
|
||||
frames.clear()
|
||||
act(() => queued.forEach((callback) => callback(0)))
|
||||
}
|
||||
|
||||
describe('combined diff word-wrap lifecycle', () => {
|
||||
it('cancels the pending mount frame when the section returns to loading', async () => {
|
||||
const view = render(<DiffSectionBody {...props} />)
|
||||
await finishMount()
|
||||
expect(frames.size).toBe(1)
|
||||
|
||||
view.rerender(<DiffSectionBody {...props} section={{ ...props.section, loading: true }} />)
|
||||
|
||||
expect(frames.size).toBe(0)
|
||||
expect(syncWordWrap).not.toHaveBeenCalled()
|
||||
expect(cleanupShiftWheel).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('disposes synchronization and ignores preference changes while the editor is absent', async () => {
|
||||
const view = render(<DiffSectionBody {...props} />)
|
||||
await finishMount()
|
||||
runMountFrame()
|
||||
const subscription = syncWordWrap.mock.results[0]?.value
|
||||
expect(subscription).toBeDefined()
|
||||
|
||||
view.rerender(<DiffSectionBody {...props} section={{ ...props.section, loading: true }} />)
|
||||
expect(subscription?.dispose).toHaveBeenCalledOnce()
|
||||
|
||||
view.rerender(
|
||||
<DiffSectionBody
|
||||
{...props}
|
||||
diffWordWrap={false}
|
||||
section={{ ...props.section, loading: true }}
|
||||
/>
|
||||
)
|
||||
expect(syncWordWrap).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('uses the current wrap preference when a loading section remounts its editor', async () => {
|
||||
const view = render(<DiffSectionBody {...props} />)
|
||||
await finishMount()
|
||||
runMountFrame()
|
||||
const oldEditor = mountedEditors[0]
|
||||
|
||||
view.rerender(
|
||||
<DiffSectionBody
|
||||
{...props}
|
||||
diffWordWrap={false}
|
||||
section={{ ...props.section, loading: true }}
|
||||
/>
|
||||
)
|
||||
view.rerender(<DiffSectionBody {...props} diffWordWrap={false} />)
|
||||
await finishMount()
|
||||
expect(frames.size).toBe(1)
|
||||
act(() => oldEditor?.disposeModified())
|
||||
expect(frames.size).toBe(1)
|
||||
runMountFrame()
|
||||
|
||||
expect(syncWordWrap).toHaveBeenLastCalledWith(mountedEditors[1]?.editor, false)
|
||||
})
|
||||
})
|
||||
@@ -21,7 +21,10 @@ import { useDiffViewerFirstChangeAutoScroll } from './useDiffViewerFirstChangeAu
|
||||
import { getDiffViewerLargeDiffSaveAction } from './diff-viewer-large-diff-save-action'
|
||||
import type { DiffViewerProps } from './diff-viewer-props'
|
||||
import { buildDiffEditorWhitespaceOptions } from './diff-editor-whitespace-options'
|
||||
import { buildDiffEditorWordWrapOptions } from './diff-editor-word-wrap-options'
|
||||
import {
|
||||
buildDiffEditorWordWrapOptions,
|
||||
syncDiffEditorOriginalWordWrap
|
||||
} from './diff-editor-word-wrap-options'
|
||||
import { buildDiffEditorHideUnchangedOptions } from './diff-editor-hide-unchanged-options'
|
||||
import { useDiffEditorRegistration } from './diff-navigation-context'
|
||||
import { preserveDiffViewStateAcrossModelSwaps } from './diff-model-swap-view-state'
|
||||
@@ -67,10 +70,17 @@ export default function DiffViewer({
|
||||
)
|
||||
const terminalFontSize = settings?.terminalFontSize ?? 13,
|
||||
diffEditorFontSize = computeDiffEditorFontSize(terminalFontSize, editorFontZoomLevel)
|
||||
const diffWordWrap = settings?.diffWordWrap
|
||||
|
||||
const diffEditorRef = useRef<editor.IStandaloneDiffEditor | null>(null)
|
||||
const { registerDiffEditor, unregisterDiffEditor } = useDiffEditorRegistration()
|
||||
const lineNumberOptionsSubRef = useRef<{ dispose: () => void } | null>(null)
|
||||
const wordWrapOptionsSubRef = useRef<{ dispose: () => void } | null>(null)
|
||||
const wordWrapMountFrameRef = useRef(0)
|
||||
const diffWordWrapRef = useRef(diffWordWrap)
|
||||
useLayoutEffect(() => {
|
||||
diffWordWrapRef.current = diffWordWrap
|
||||
}, [diffWordWrap])
|
||||
const [modifiedEditor, setModifiedEditor] = useState<editor.ICodeEditor | null>(null)
|
||||
|
||||
const renderLimit = useMemo(
|
||||
@@ -161,6 +171,9 @@ export default function DiffViewer({
|
||||
// Why: on fallback transition, drop stale Monaco refs so decorators/save handlers don't talk to disposed UI.
|
||||
lineNumberOptionsSubRef.current?.dispose()
|
||||
lineNumberOptionsSubRef.current = null
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = null
|
||||
// Why: capture before nulling so we unregister the exact instance (identity guard no-ops a stale dispose).
|
||||
const fallenBackEditor = diffEditorRef.current
|
||||
diffEditorRef.current = null
|
||||
@@ -193,11 +206,35 @@ export default function DiffViewer({
|
||||
(diffEditor, monaco) => {
|
||||
diffEditorRef.current = diffEditor
|
||||
registerDiffEditor(diffEditor)
|
||||
// Why: Monaco applies the inline-layout wrap override after mount, once width is known.
|
||||
wordWrapMountFrameRef.current = requestAnimationFrame(() => {
|
||||
if (diffEditorRef.current !== diffEditor) {
|
||||
return
|
||||
}
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = syncDiffEditorOriginalWordWrap(
|
||||
diffEditor,
|
||||
diffWordWrapRef.current
|
||||
)
|
||||
})
|
||||
lineNumberOptionsSubRef.current?.dispose()
|
||||
lineNumberOptionsSubRef.current = applyDiffEditorLineNumberOptions(diffEditor, sideBySide)
|
||||
|
||||
const originalEditor = diffEditor.getOriginalEditor()
|
||||
const modifiedEditor = diffEditor.getModifiedEditor()
|
||||
modifiedEditor.onDidDispose(() => {
|
||||
if (diffEditorRef.current !== diffEditor) {
|
||||
return
|
||||
}
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
lineNumberOptionsSubRef.current?.dispose()
|
||||
lineNumberOptionsSubRef.current = null
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = null
|
||||
unregisterDiffEditor(diffEditor)
|
||||
diffEditorRef.current = null
|
||||
setModifiedEditor(null)
|
||||
})
|
||||
diffEditor.onDidDispose(preserveDiffViewStateAcrossModelSwaps(diffEditor).dispose)
|
||||
|
||||
setupCopy(originalEditor, monaco, filePath, propsRef)
|
||||
@@ -237,14 +274,6 @@ export default function DiffViewer({
|
||||
} else {
|
||||
diffEditor.focus()
|
||||
}
|
||||
|
||||
diffEditor.onDidDispose(() => {
|
||||
lineNumberOptionsSubRef.current?.dispose()
|
||||
lineNumberOptionsSubRef.current = null
|
||||
diffEditorRef.current = null
|
||||
unregisterDiffEditor(diffEditor)
|
||||
setModifiedEditor(null)
|
||||
})
|
||||
},
|
||||
[editable, setupCopy, modelKey, filePath, sideBySide, registerDiffEditor, unregisterDiffEditor]
|
||||
)
|
||||
@@ -263,17 +292,25 @@ export default function DiffViewer({
|
||||
}, [modelKey])
|
||||
|
||||
useEffect(() => {
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
const diffEditor = diffEditorRef.current
|
||||
if (!diffEditor) {
|
||||
return
|
||||
return () => {
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
}
|
||||
}
|
||||
lineNumberOptionsSubRef.current?.dispose()
|
||||
lineNumberOptionsSubRef.current = applyDiffEditorLineNumberOptions(diffEditor, sideBySide)
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = syncDiffEditorOriginalWordWrap(diffEditor, diffWordWrap)
|
||||
return () => {
|
||||
cancelAnimationFrame(wordWrapMountFrameRef.current)
|
||||
lineNumberOptionsSubRef.current?.dispose()
|
||||
lineNumberOptionsSubRef.current = null
|
||||
wordWrapOptionsSubRef.current?.dispose()
|
||||
wordWrapOptionsSubRef.current = null
|
||||
}
|
||||
}, [sideBySide])
|
||||
}, [diffWordWrap, sideBySide])
|
||||
|
||||
return (
|
||||
<div className="flex flex-col flex-1 min-h-0">
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { act, cleanup, render } from '@testing-library/react'
|
||||
import { useEffect, useRef } from 'react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import DiffViewer from './DiffViewer'
|
||||
import { DiffNavigationProvider, useDiffNavigation } from './diff-navigation-context'
|
||||
import { getLargeDiffRenderLimitFromCounts } from './large-diff-render-limit'
|
||||
|
||||
const fixture = vi.hoisted(() => {
|
||||
function createEditor() {
|
||||
const disposals = new Set<() => void>()
|
||||
const modifiedEditor = {
|
||||
onDidDispose: (callback: () => void) => {
|
||||
disposals.add(callback)
|
||||
return { dispose: () => disposals.delete(callback) }
|
||||
}
|
||||
}
|
||||
const disposeUpdate = vi.fn()
|
||||
const editor = {
|
||||
getOriginalEditor: () => ({}),
|
||||
getModifiedEditor: () => modifiedEditor,
|
||||
onDidDispose: vi.fn(() => ({ dispose: vi.fn() })),
|
||||
onDidUpdateDiff: () => ({ dispose: disposeUpdate }),
|
||||
getLineChanges: () => [{}],
|
||||
goToDiff: vi.fn(),
|
||||
saveViewState: () => null,
|
||||
focus: vi.fn()
|
||||
}
|
||||
return { editor, disposeUpdate, dispose: () => disposals.forEach((callback) => callback()) }
|
||||
}
|
||||
const editors: ReturnType<typeof createEditor>[] = []
|
||||
const state = {
|
||||
settings: { diffWordWrap: true },
|
||||
editorFontZoomLevel: 0,
|
||||
addDiffComment: vi.fn(),
|
||||
deleteDiffComment: vi.fn(),
|
||||
updateDiffComment: vi.fn(),
|
||||
scrollToDiffCommentId: null,
|
||||
setScrollToDiffCommentId: vi.fn()
|
||||
}
|
||||
return { createEditor, editors, state }
|
||||
})
|
||||
|
||||
vi.mock('@monaco-editor/react', () => ({
|
||||
DiffEditor: ({
|
||||
onMount
|
||||
}: {
|
||||
onMount: (editor: ReturnType<typeof fixture.createEditor>['editor']) => void
|
||||
}) => {
|
||||
const mount = useRef(onMount)
|
||||
useEffect(() => {
|
||||
const instance = fixture.createEditor()
|
||||
fixture.editors.push(instance)
|
||||
let disposed = false
|
||||
queueMicrotask(() => {
|
||||
if (!disposed) {
|
||||
mount.current(instance.editor)
|
||||
}
|
||||
})
|
||||
return () => {
|
||||
disposed = true
|
||||
instance.dispose()
|
||||
}
|
||||
}, [])
|
||||
return <div />
|
||||
}
|
||||
}))
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: <T,>(selector: (state: typeof fixture.state) => T) => selector(fixture.state)
|
||||
}))
|
||||
vi.mock('@/store/worktree-diff-comments-selector', () => ({
|
||||
selectWorktreeDiffComments: () => undefined
|
||||
}))
|
||||
vi.mock('@/lib/monaco-setup', () => ({
|
||||
monaco: { Uri: { parse: (path: string) => path }, editor: { getModel: () => null } }
|
||||
}))
|
||||
vi.mock('./useContextualCopySetup', () => ({
|
||||
useContextualCopySetup: () => ({ setupCopy: vi.fn(), toastNode: null })
|
||||
}))
|
||||
vi.mock('../diff-comments/useDiffCommentDecorator', () => ({ useDiffCommentDecorator: vi.fn() }))
|
||||
vi.mock('./useDiffViewerFirstChangeAutoScroll', () => ({
|
||||
useDiffViewerFirstChangeAutoScroll: vi.fn()
|
||||
}))
|
||||
vi.mock('@/hooks/use-document-dark-theme', () => ({ useDocumentDarkTheme: () => false }))
|
||||
vi.mock('./diff-editor-line-number-options', () => ({
|
||||
applyDiffEditorLineNumberOptions: () => ({ dispose: vi.fn() })
|
||||
}))
|
||||
vi.mock('./diff-editor-word-wrap-options', () => ({
|
||||
buildDiffEditorWordWrapOptions: () => ({}),
|
||||
syncDiffEditorOriginalWordWrap: () => ({ dispose: vi.fn() })
|
||||
}))
|
||||
vi.mock('./diff-model-swap-view-state', () => ({
|
||||
preserveDiffViewStateAcrossModelSwaps: () => ({ dispose: vi.fn() })
|
||||
}))
|
||||
vi.mock('./editor-shortcuts', () => ({ installMonacoDiffChangeNavigationShortcut: () => vi.fn() }))
|
||||
vi.mock('./LargeDiffFallback', () => ({ LargeDiffFallback: () => <div>Large diff</div> }))
|
||||
|
||||
function NavigationProbe(): React.JSX.Element {
|
||||
const navigation = useDiffNavigation()
|
||||
return (
|
||||
<button disabled={navigation.changeCount === 0} onClick={navigation.goToNextDiff}>
|
||||
Next change ({navigation.changeCount})
|
||||
</button>
|
||||
)
|
||||
}
|
||||
|
||||
function Surface({ limited }: { limited: boolean }): React.JSX.Element {
|
||||
return (
|
||||
<DiffNavigationProvider>
|
||||
<NavigationProbe />
|
||||
<DiffViewer
|
||||
modelKey="wrap-lifecycle"
|
||||
originalContent="original"
|
||||
modifiedContent="modified"
|
||||
language="markdown"
|
||||
filePath="README.md"
|
||||
relativePath="README.md"
|
||||
sideBySide
|
||||
largeDiffRenderLimit={getLargeDiffRenderLimitFromCounts({
|
||||
originalLineCount: limited ? 120_001 : 1,
|
||||
modifiedLineCount: 1,
|
||||
originalCharacterCount: 8,
|
||||
modifiedCharacterCount: 8
|
||||
})}
|
||||
/>
|
||||
</DiffNavigationProvider>
|
||||
)
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
fixture.editors.length = 0
|
||||
})
|
||||
|
||||
describe('file diff word-wrap lifecycle', () => {
|
||||
it('unregisters navigation when inner disposal precedes the large-diff fallback effect', async () => {
|
||||
const view = render(<Surface limited={false} />)
|
||||
await act(async () => {
|
||||
await Promise.resolve()
|
||||
})
|
||||
const mounted = fixture.editors[0]
|
||||
const next = view.getByRole('button', { name: 'Next change (1)' })
|
||||
expect(next.hasAttribute('disabled')).toBe(false)
|
||||
act(() => next.click())
|
||||
expect(mounted?.editor.goToDiff).toHaveBeenCalledWith('next')
|
||||
|
||||
view.rerender(<Surface limited />)
|
||||
|
||||
expect(view.getByRole('button', { name: 'Next change (0)' }).hasAttribute('disabled')).toBe(
|
||||
true
|
||||
)
|
||||
expect(mounted?.disposeUpdate).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
@@ -1,13 +1,166 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildDiffEditorWordWrapOptions } from './diff-editor-word-wrap-options'
|
||||
// @vitest-environment happy-dom
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { editor } from 'monaco-editor'
|
||||
import {
|
||||
buildDiffEditorWordWrapOptions,
|
||||
syncDiffEditorOriginalWordWrap
|
||||
} from './diff-editor-word-wrap-options'
|
||||
|
||||
describe('buildDiffEditorWordWrapOptions', () => {
|
||||
it('keeps long diff lines unwrapped by default', () => {
|
||||
expect(buildDiffEditorWordWrapOptions(undefined)).toEqual({ wordWrap: 'off' })
|
||||
expect(buildDiffEditorWordWrapOptions(false)).toEqual({ wordWrap: 'off' })
|
||||
expect(buildDiffEditorWordWrapOptions(undefined)).toEqual({
|
||||
wordWrap: 'off',
|
||||
diffWordWrap: 'off'
|
||||
})
|
||||
expect(buildDiffEditorWordWrapOptions(false)).toEqual({
|
||||
wordWrap: 'off',
|
||||
diffWordWrap: 'off'
|
||||
})
|
||||
})
|
||||
|
||||
it('enables Monaco diff word wrapping when the diff preference is on', () => {
|
||||
expect(buildDiffEditorWordWrapOptions(true)).toEqual({ wordWrap: 'on' })
|
||||
it('enables Monaco diff word wrapping on both panes when the diff preference is on', () => {
|
||||
expect(buildDiffEditorWordWrapOptions(true)).toEqual({
|
||||
wordWrap: 'on',
|
||||
diffWordWrap: 'on'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('syncDiffEditorOriginalWordWrap', () => {
|
||||
function fakeEditor() {
|
||||
const listeners = new Set<() => void>()
|
||||
let options: editor.IEditorOptions = {}
|
||||
const editorStub = {
|
||||
updateOptions: vi.fn((next: editor.IEditorOptions) => {
|
||||
options = { ...options, ...next }
|
||||
}),
|
||||
getRawOptions: () => options,
|
||||
onDidChangeConfiguration: (listener: () => void) => {
|
||||
listeners.add(listener)
|
||||
return {
|
||||
dispose: () => {
|
||||
listeners.delete(listener)
|
||||
}
|
||||
}
|
||||
},
|
||||
emitDidChangeConfiguration: () => {
|
||||
listeners.forEach((listener) => listener())
|
||||
}
|
||||
}
|
||||
return editorStub
|
||||
}
|
||||
|
||||
function monacoDiffHost(sideBySide: boolean): HTMLElement {
|
||||
const host = document.createElement('div')
|
||||
const widget = document.createElement('div')
|
||||
widget.classList.add('monaco-diff-editor')
|
||||
widget.classList.toggle('side-by-side', sideBySide)
|
||||
host.append(widget)
|
||||
return host
|
||||
}
|
||||
|
||||
function fakeDiffEditor(root = monacoDiffHost(true)): {
|
||||
diffEditor: Parameters<typeof syncDiffEditorOriginalWordWrap>[0]
|
||||
original: ReturnType<typeof fakeEditor>
|
||||
modified: ReturnType<typeof fakeEditor>
|
||||
} {
|
||||
const original = fakeEditor()
|
||||
const modified = fakeEditor()
|
||||
return {
|
||||
original,
|
||||
modified,
|
||||
diffEditor: {
|
||||
getOriginalEditor: () => original,
|
||||
getModifiedEditor: () => modified,
|
||||
getContainerDomNode: () => root
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
it('clears the original pane override that stays off after Monaco leaves inline layout', () => {
|
||||
const { diffEditor, original, modified } = fakeDiffEditor()
|
||||
|
||||
syncDiffEditorOriginalWordWrap(diffEditor, true)
|
||||
|
||||
expect(original.updateOptions).toHaveBeenCalledWith({
|
||||
wordWrap: 'on',
|
||||
wordWrapOverride2: 'inherit'
|
||||
})
|
||||
expect(modified.updateOptions).toHaveBeenCalledWith({ wordWrap: 'on' })
|
||||
})
|
||||
|
||||
it('keeps both panes unwrapped when the preference is off', () => {
|
||||
const { diffEditor, original, modified } = fakeDiffEditor()
|
||||
|
||||
syncDiffEditorOriginalWordWrap(diffEditor, false)
|
||||
|
||||
expect(original.updateOptions).toHaveBeenCalledWith({
|
||||
wordWrap: 'off',
|
||||
wordWrapOverride2: 'off'
|
||||
})
|
||||
expect(modified.updateOptions).toHaveBeenCalledWith({ wordWrap: 'off' })
|
||||
})
|
||||
|
||||
it('reapplies the original pane wrap after Monaco clears it, and stops after dispose', async () => {
|
||||
const root = monacoDiffHost(true)
|
||||
expect(root.classList.contains('side-by-side')).toBe(false)
|
||||
const { diffEditor, original } = fakeDiffEditor(root)
|
||||
const disposable = syncDiffEditorOriginalWordWrap(diffEditor, true)
|
||||
original.updateOptions.mockClear()
|
||||
|
||||
original.updateOptions({ wordWrapOverride2: 'off' })
|
||||
original.emitDidChangeConfiguration()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(original.getRawOptions().wordWrapOverride2).toBe('inherit')
|
||||
|
||||
original.updateOptions.mockClear()
|
||||
disposable.dispose()
|
||||
original.updateOptions({ wordWrapOverride2: 'off' })
|
||||
original.emitDidChangeConfiguration()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(original.getRawOptions().wordWrapOverride2).toBe('off')
|
||||
expect(original.updateOptions).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('leaves the hidden original pane unwrapped while Monaco is inline', async () => {
|
||||
const root = monacoDiffHost(false)
|
||||
const { diffEditor, original } = fakeDiffEditor(root)
|
||||
|
||||
syncDiffEditorOriginalWordWrap(diffEditor, true)
|
||||
|
||||
expect(original.getRawOptions().wordWrapOverride2).toBe('off')
|
||||
|
||||
original.updateOptions({ wordWrapOverride2: 'inherit' })
|
||||
original.emitDidChangeConfiguration()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(original.getRawOptions().wordWrapOverride2).toBe('off')
|
||||
})
|
||||
|
||||
it('observes the settled layout after an inline-to-side-by-side transition', async () => {
|
||||
const host = monacoDiffHost(false)
|
||||
const { diffEditor, original, modified } = fakeDiffEditor(host)
|
||||
const disposable = syncDiffEditorOriginalWordWrap(diffEditor, true)
|
||||
|
||||
original.emitDidChangeConfiguration()
|
||||
modified.emitDidChangeConfiguration()
|
||||
host.querySelector('.monaco-diff-editor')?.classList.add('side-by-side')
|
||||
await Promise.resolve()
|
||||
|
||||
expect(original.getRawOptions().wordWrapOverride2).toBe('inherit')
|
||||
disposable.dispose()
|
||||
})
|
||||
|
||||
it('cancels a queued update when the editor is disposed', async () => {
|
||||
const { diffEditor, original } = fakeDiffEditor()
|
||||
const disposable = syncDiffEditorOriginalWordWrap(diffEditor, true)
|
||||
original.updateOptions({ wordWrapOverride2: 'off' })
|
||||
original.emitDidChangeConfiguration()
|
||||
disposable.dispose()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(original.getRawOptions().wordWrapOverride2).toBe('off')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,9 +1,96 @@
|
||||
import type { editor } from 'monaco-editor'
|
||||
|
||||
export function diffEditorWordWrapMode(diffWordWrap: boolean | undefined): 'on' | 'off' {
|
||||
return diffWordWrap === true ? 'on' : 'off'
|
||||
}
|
||||
|
||||
export function buildDiffEditorWordWrapOptions(
|
||||
diffWordWrap: boolean | undefined
|
||||
): Pick<editor.IStandaloneDiffEditorConstructionOptions, 'wordWrap'> {
|
||||
): Pick<editor.IStandaloneDiffEditorConstructionOptions, 'wordWrap' | 'diffWordWrap'> {
|
||||
const wrap = diffEditorWordWrapMode(diffWordWrap)
|
||||
return {
|
||||
wordWrap: diffWordWrap === true ? 'on' : 'off'
|
||||
wordWrap: wrap,
|
||||
// Why: `wordWrap` alone reaches the modified pane; the original pane follows `diffWordWrap`.
|
||||
diffWordWrap: wrap
|
||||
}
|
||||
}
|
||||
|
||||
type Disposable = { dispose: () => void }
|
||||
|
||||
type WordWrapEditor = Pick<editor.ICodeEditor, 'getRawOptions' | 'updateOptions'> & {
|
||||
onDidChangeConfiguration: (listener: () => void) => Disposable
|
||||
}
|
||||
|
||||
type WordWrapDiffEditor = Pick<editor.IStandaloneDiffEditor, 'getContainerDomNode'> & {
|
||||
getOriginalEditor: () => WordWrapEditor
|
||||
getModifiedEditor: () => WordWrapEditor
|
||||
}
|
||||
|
||||
function diffEditorIsSideBySide(diffEditor: WordWrapDiffEditor): boolean {
|
||||
const host = diffEditor.getContainerDomNode?.()
|
||||
if (!host) {
|
||||
return true
|
||||
}
|
||||
// Why: createDiffEditor's container never receives the class. Monaco appends
|
||||
// `div.monaco-diff-editor` and toggles `side-by-side` on that child.
|
||||
const widget = host.classList.contains('monaco-diff-editor')
|
||||
? host
|
||||
: host.querySelector?.('.monaco-diff-editor')
|
||||
if (!widget) {
|
||||
return true
|
||||
}
|
||||
return widget.classList.contains('side-by-side')
|
||||
}
|
||||
|
||||
export function syncDiffEditorOriginalWordWrap(
|
||||
diffEditor: WordWrapDiffEditor,
|
||||
diffWordWrap: boolean | undefined
|
||||
): Disposable {
|
||||
const originalEditor = diffEditor.getOriginalEditor()
|
||||
const modifiedEditor = diffEditor.getModifiedEditor()
|
||||
let disposed = false
|
||||
let scheduled = false
|
||||
|
||||
const apply = (): void => {
|
||||
if (disposed) {
|
||||
return
|
||||
}
|
||||
const wrap = diffEditorWordWrapMode(diffWordWrap)
|
||||
// Why: inline layout hides the original editor and sets wordWrapOverride2 to off.
|
||||
// Side-by-side only writes wordWrapOverride1, so the off value sticks after a widen (#24199).
|
||||
const showOriginal = diffEditorIsSideBySide(diffEditor)
|
||||
const override = wrap === 'on' && showOriginal ? 'inherit' : 'off'
|
||||
const originalOptions = originalEditor.getRawOptions()
|
||||
if (originalOptions.wordWrap !== wrap || originalOptions.wordWrapOverride2 !== override) {
|
||||
originalEditor.updateOptions({ wordWrap: wrap, wordWrapOverride2: override })
|
||||
}
|
||||
if (modifiedEditor.getRawOptions().wordWrap !== wrap) {
|
||||
modifiedEditor.updateOptions({ wordWrap: wrap })
|
||||
}
|
||||
}
|
||||
|
||||
// Why: Monaco updates the inner editor and the side-by-side class in the same turn.
|
||||
// Applying on the next microtask sees the class after that turn settles.
|
||||
const schedule = (): void => {
|
||||
if (disposed || scheduled) {
|
||||
return
|
||||
}
|
||||
scheduled = true
|
||||
queueMicrotask(() => {
|
||||
scheduled = false
|
||||
apply()
|
||||
})
|
||||
}
|
||||
|
||||
apply()
|
||||
const originalSub = originalEditor.onDidChangeConfiguration(schedule)
|
||||
const modifiedSub = modifiedEditor.onDidChangeConfiguration(schedule)
|
||||
|
||||
return {
|
||||
dispose: () => {
|
||||
disposed = true
|
||||
originalSub.dispose()
|
||||
modifiedSub.dispose()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { writeFileSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import {
|
||||
cleanupGoldenWorktree,
|
||||
createGoldenWorktree,
|
||||
openGoldenSourceControl
|
||||
} from './helpers/golden-source-control'
|
||||
import { waitForSessionReady } from './helpers/store'
|
||||
|
||||
type DiffSurface = 'file' | 'combined'
|
||||
|
||||
async function toggleWordWrap(page: Page, surface: DiffSurface): Promise<void> {
|
||||
if (surface === 'combined') {
|
||||
await page.getByRole('button', { name: /^Wrap (On|Off)$/ }).click()
|
||||
return
|
||||
}
|
||||
await page.getByRole('button', { name: 'More actions', exact: true }).click()
|
||||
await page.getByRole('menuitemcheckbox', { name: 'Word Wrap', exact: true }).click()
|
||||
}
|
||||
|
||||
async function expectWrappedParagraphs(page: Page): Promise<void> {
|
||||
for (const side of ['original', 'modified']) {
|
||||
const pane = page.locator(`.${side}-in-monaco-diff-editor`)
|
||||
await expect
|
||||
.poll(() => pane.locator('.view-line').count(), {
|
||||
message: `${side} paragraphs should occupy multiple wrapped rows`
|
||||
})
|
||||
.toBeGreaterThan(10)
|
||||
await expect
|
||||
.poll(() =>
|
||||
pane.evaluate((element) => {
|
||||
const viewport = element.querySelector('.monaco-scrollable-element')
|
||||
if (!viewport) {
|
||||
throw new Error('Missing Monaco text viewport')
|
||||
}
|
||||
const textWidth = Math.max(
|
||||
...Array.from(
|
||||
element.querySelectorAll('.view-line > span'),
|
||||
(line) => line.getBoundingClientRect().width
|
||||
)
|
||||
)
|
||||
return textWidth - viewport.getBoundingClientRect().width
|
||||
})
|
||||
)
|
||||
.toBeLessThanOrEqual(1)
|
||||
}
|
||||
}
|
||||
|
||||
for (const surface of ['file', 'combined'] as const) {
|
||||
test(`${surface} diff word wrap applies to both panes after toggles and narrow inline layout`, async ({
|
||||
orcaPage,
|
||||
testRepoPath,
|
||||
registerPostElectronShutdownCleanup
|
||||
}, testInfo) => {
|
||||
const fixture = createGoldenWorktree(testRepoPath, `diff-word-wrap-${surface}`)
|
||||
registerPostElectronShutdownCleanup(async () => cleanupGoldenWorktree(testRepoPath, fixture))
|
||||
const paragraph =
|
||||
'This long Markdown paragraph compares the original and modified panes at their own widths. '.repeat(
|
||||
9
|
||||
)
|
||||
const readmePath = path.join(fixture.worktreePath, 'README.md')
|
||||
const content = `# Diff word wrap\n\n${paragraph}Original paragraph end.\n\n${paragraph}Second original paragraph end.\n`
|
||||
writeFileSync(readmePath, content)
|
||||
execFileSync('git', ['add', 'README.md'], { cwd: fixture.worktreePath, stdio: 'pipe' })
|
||||
execFileSync('git', ['commit', '-m', 'Seed long Markdown paragraphs'], {
|
||||
cwd: fixture.worktreePath,
|
||||
stdio: 'pipe'
|
||||
})
|
||||
writeFileSync(
|
||||
readmePath,
|
||||
content.replaceAll('original', 'modified').replaceAll('Original', 'Modified')
|
||||
)
|
||||
|
||||
await orcaPage.setViewportSize({ width: 1600, height: 850 })
|
||||
await waitForSessionReady(orcaPage)
|
||||
await orcaPage.evaluate(async () => {
|
||||
await window.__store?.getState().updateSettings({
|
||||
diffDefaultView: 'side-by-side',
|
||||
diffWordWrap: false
|
||||
})
|
||||
})
|
||||
await openGoldenSourceControl(orcaPage, testRepoPath, fixture)
|
||||
const changes = orcaPage.getByRole('button', { name: /^Changes \d+$/ }).locator('..')
|
||||
await (
|
||||
surface === 'combined'
|
||||
? changes.getByRole('button', { name: 'View all', exact: true })
|
||||
: changes
|
||||
.locator('../..')
|
||||
.locator('[data-testid="source-control-entry"]')
|
||||
.filter({ hasText: 'README.md' })
|
||||
).click()
|
||||
await orcaPage.evaluate(() => window.__store?.getState().setRightSidebarOpen(false))
|
||||
const diff = orcaPage.locator('.monaco-diff-editor')
|
||||
await expect(diff).toHaveClass(/side-by-side/)
|
||||
await toggleWordWrap(orcaPage, surface)
|
||||
await orcaPage.screenshot({ path: testInfo.outputPath('wrap-on.png') })
|
||||
await expectWrappedParagraphs(orcaPage)
|
||||
|
||||
await orcaPage.setViewportSize({ width: 1000, height: 850 })
|
||||
await expect(diff).not.toHaveClass(/side-by-side/)
|
||||
await orcaPage.setViewportSize({ width: 1600, height: 850 })
|
||||
await expect(diff).toHaveClass(/side-by-side/)
|
||||
await expectWrappedParagraphs(orcaPage)
|
||||
|
||||
await toggleWordWrap(orcaPage, surface)
|
||||
for (const side of ['original', 'modified']) {
|
||||
await expect
|
||||
.poll(() => orcaPage.locator(`.${side}-in-monaco-diff-editor .view-line`).count())
|
||||
.toBeLessThanOrEqual(6)
|
||||
}
|
||||
await toggleWordWrap(orcaPage, surface)
|
||||
await expectWrappedParagraphs(orcaPage)
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user