Fix diff view scrolling using virtualized scroll anchors (#6190)
* Restore combined diff scroll position using virtualized scroll anchors Fixes scroll jumping and incorrect restoration in virtualized combined diff views by tracking scroll position via a stable row anchor (key and offset) rather than a fragile raw scrollTop. - Track active row anchors across tab switches and component remounts - Prevent programmatic scroll events from writing incorrect anchors - Avoid redundant state updates and re-renders when focusing an already focused group * fix: address review findings
This commit is contained in:
Jinjing
GitHub
parent
d14498a111
commit
b053e1ac60
|
|
@ -8,6 +8,11 @@ import React, { useState, useEffect, useCallback, useRef, useLayoutEffect } from
|
|||
import { useVirtualizer } from '@tanstack/react-virtual'
|
||||
import type { editor as monacoEditor } from 'monaco-editor'
|
||||
import { useAppStore } from '@/store'
|
||||
import {
|
||||
useVirtualizedScrollAnchor,
|
||||
type VirtualizedScrollAnchor
|
||||
} from '@/hooks/useVirtualizedScrollAnchor'
|
||||
import { getVirtualizedScrollAnchorForOffset } from '@/hooks/virtualized-scroll-anchor-recording'
|
||||
import { joinPath } from '@/lib/path'
|
||||
import { detectLanguage } from '@/lib/language-detect'
|
||||
import { setWithLRU } from '@/lib/scroll-cache'
|
||||
|
|
@ -50,7 +55,10 @@ import {
|
|||
createCombinedDiffSectionIndexMap,
|
||||
handleCombinedDiffFileTreeNavigation
|
||||
} from './CombinedDiffFileTree'
|
||||
import { getCombinedDiffFileTreeSectionKey } from './combined-diff-file-tree-model'
|
||||
import {
|
||||
getCombinedDiffFileTreeSectionKey,
|
||||
type CombinedDiffFileTreeMode
|
||||
} from './combined-diff-file-tree-model'
|
||||
import {
|
||||
ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT,
|
||||
type EditorPathMutationTarget
|
||||
|
|
@ -92,6 +100,7 @@ type CombinedDiffScrollThumb = {
|
|||
|
||||
const combinedDiffViewStateCache = new Map<string, CachedCombinedDiffViewState>()
|
||||
const combinedDiffScrollTopCache = new Map<string, number>()
|
||||
const combinedDiffScrollAnchorCache = new Map<string, VirtualizedScrollAnchor>()
|
||||
|
||||
function buildCombinedGitStatusSignature(
|
||||
sections: readonly { path: string }[],
|
||||
|
|
@ -183,6 +192,26 @@ function getInitialCombinedDiffFileTreeCollapsed(
|
|||
return combinedDiffFileTreeCollapsedPreference ?? combinedDiffFileTreeVisibleByDefault !== true
|
||||
}
|
||||
|
||||
function cachedCombinedDiffSectionsMatchEntries({
|
||||
entries,
|
||||
sections,
|
||||
treeMode
|
||||
}: {
|
||||
entries: readonly (GitStatusEntry | GitBranchChangeEntry)[]
|
||||
sections: readonly DiffSection[]
|
||||
treeMode: CombinedDiffFileTreeMode
|
||||
}): boolean {
|
||||
return (
|
||||
sections.length === entries.length &&
|
||||
sections.every((section, index) => {
|
||||
const entry = entries[index]
|
||||
return (
|
||||
entry !== undefined && section.key === getCombinedDiffFileTreeSectionKey(treeMode, entry)
|
||||
)
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
export default function CombinedDiffViewer({
|
||||
file,
|
||||
viewStateKey
|
||||
|
|
@ -259,7 +288,14 @@ export default function CombinedDiffViewer({
|
|||
top: 0,
|
||||
height: COMBINED_DIFF_SCROLLBAR_THUMB_MIN_HEIGHT
|
||||
})
|
||||
const pendingRestoreScrollTopRef = useRef<number | null>(null)
|
||||
const scrollOffsetRef = useRef(combinedDiffScrollTopCache.get(viewStateKey) ?? 0)
|
||||
const scrollAnchorRef = useRef<VirtualizedScrollAnchor>(
|
||||
combinedDiffScrollAnchorCache.get(viewStateKey) ?? null
|
||||
)
|
||||
const latestDomScrollAnchorRef = useRef<VirtualizedScrollAnchor>(
|
||||
combinedDiffScrollAnchorCache.get(viewStateKey) ?? null
|
||||
)
|
||||
const directScrollInputUntilRef = useRef(0)
|
||||
const activeScrollbarDragCleanupRef = useRef<CombinedDiffScrollbarDragCleanup | null>(null)
|
||||
const loadedIndicesRef = useRef<Set<number>>(new Set())
|
||||
const loadingIndicesRef = useRef<Set<number>>(new Set())
|
||||
|
|
@ -295,6 +331,15 @@ export default function CombinedDiffViewer({
|
|||
setScrollThumb({ visible: true, top, height })
|
||||
}, [])
|
||||
|
||||
const markDirectScrollInput = useCallback((): void => {
|
||||
directScrollInputUntilRef.current = window.performance.now() + 250
|
||||
}, [])
|
||||
|
||||
const hasDirectScrollInput = useCallback(
|
||||
() => window.performance.now() < directScrollInputUntilRef.current,
|
||||
[]
|
||||
)
|
||||
|
||||
const clearNotesCopiedResetTimer = useCallback((): void => {
|
||||
if (notesCopiedResetTimerRef.current !== null) {
|
||||
window.clearTimeout(notesCopiedResetTimerRef.current)
|
||||
|
|
@ -464,15 +509,23 @@ export default function CombinedDiffViewer({
|
|||
// Why: switching tabs or worktrees unmounts this viewer through the shared
|
||||
// editor surface above it. Cache the rendered combined-diff state by the
|
||||
// visible pane key so remounting can restore loaded sections and scroll
|
||||
// position instead of flashing back to "Loading..." and forcing the user to
|
||||
// find their place again.
|
||||
useEffect(() => {
|
||||
// position before the remounted surface paints at the top.
|
||||
useLayoutEffect(() => {
|
||||
const cached = combinedDiffViewStateCache.get(viewStateKey)
|
||||
const canRestoreSnapshotSectionsByKey =
|
||||
hasUncommittedEntriesSnapshot &&
|
||||
cached !== undefined &&
|
||||
cachedCombinedDiffSectionsMatchEntries({
|
||||
entries,
|
||||
sections: cached.sections,
|
||||
treeMode
|
||||
})
|
||||
const canRestoreCachedSections =
|
||||
cached &&
|
||||
cached.entrySignature === entrySignature &&
|
||||
(cached.gitStatusSignature ?? '') ===
|
||||
buildCombinedGitStatusSignature(cached.sections, gitStatusEntries) &&
|
||||
(cached.entrySignature === entrySignature || canRestoreSnapshotSectionsByKey) &&
|
||||
(!shouldAutoReloadFromGitStatus ||
|
||||
(cached.gitStatusSignature ?? '') ===
|
||||
buildCombinedGitStatusSignature(cached.sections, gitStatusEntries)) &&
|
||||
(cached.sections.length > 0 || entries.length === 0)
|
||||
if (canRestoreCachedSections && cached) {
|
||||
const collapsedPreference = combinedDiffCollapsedPreference
|
||||
|
|
@ -490,12 +543,15 @@ export default function CombinedDiffViewer({
|
|||
cached.loadedIndices.filter((index) => !restoredSections[index]?.loading)
|
||||
)
|
||||
loadingIndicesRef.current.clear()
|
||||
pendingRestoreScrollTopRef.current =
|
||||
combinedDiffScrollTopCache.get(viewStateKey) ?? cached.scrollTop
|
||||
scrollOffsetRef.current = combinedDiffScrollTopCache.get(viewStateKey) ?? cached.scrollTop
|
||||
scrollAnchorRef.current = combinedDiffScrollAnchorCache.get(viewStateKey) ?? null
|
||||
latestDomScrollAnchorRef.current = scrollAnchorRef.current
|
||||
return
|
||||
}
|
||||
|
||||
pendingRestoreScrollTopRef.current = combinedDiffScrollTopCache.get(viewStateKey) ?? null
|
||||
scrollOffsetRef.current = combinedDiffScrollTopCache.get(viewStateKey) ?? 0
|
||||
scrollAnchorRef.current = combinedDiffScrollAnchorCache.get(viewStateKey) ?? null
|
||||
latestDomScrollAnchorRef.current = scrollAnchorRef.current
|
||||
setSections(
|
||||
entries.map((entry) => ({
|
||||
key: getCombinedDiffFileTreeSectionKey(treeMode, entry),
|
||||
|
|
@ -521,7 +577,15 @@ export default function CombinedDiffViewer({
|
|||
loadSchedulerRef.current.reset()
|
||||
generationRef.current += 1
|
||||
setGeneration((prev) => prev + 1)
|
||||
}, [entries, entrySignature, file.diffSource, gitStatusEntries, treeMode, viewStateKey])
|
||||
}, [
|
||||
entries,
|
||||
entrySignature,
|
||||
gitStatusEntries,
|
||||
hasUncommittedEntriesSnapshot,
|
||||
shouldAutoReloadFromGitStatus,
|
||||
treeMode,
|
||||
viewStateKey
|
||||
])
|
||||
|
||||
const loadSectionNow = useCallback(
|
||||
async (index: number) => {
|
||||
|
|
@ -771,6 +835,7 @@ export default function CombinedDiffViewer({
|
|||
})
|
||||
},
|
||||
overscan: COMBINED_DIFF_OVERSCAN,
|
||||
initialOffset: () => scrollOffsetRef.current,
|
||||
getItemKey: (index) => {
|
||||
const section = sections[index]
|
||||
if (!section) {
|
||||
|
|
@ -779,6 +844,103 @@ export default function CombinedDiffViewer({
|
|||
return `${section.key}:${section.collapsed ? 'collapsed' : 'expanded'}:${generation}`
|
||||
}
|
||||
})
|
||||
const combinedDiffTotalSize = virtualizer.getTotalSize()
|
||||
const getCombinedDiffSectionKey = useCallback((section: DiffSection): string => section.key, [])
|
||||
const getCombinedDiffSectionElementKey = useCallback(
|
||||
(element: Element): string | null =>
|
||||
element instanceof HTMLElement ? (element.dataset.combinedDiffSectionKey ?? null) : null,
|
||||
[]
|
||||
)
|
||||
const recordCombinedDiffVirtualScrollAnchor = useCallback(
|
||||
(scrollTop: number): void => {
|
||||
scrollAnchorRef.current = getVirtualizedScrollAnchorForOffset({
|
||||
getRowKey: getCombinedDiffSectionKey,
|
||||
rows: sectionsRef.current,
|
||||
scrollTop,
|
||||
virtualItems: virtualizer.getVirtualItems()
|
||||
})
|
||||
latestDomScrollAnchorRef.current = null
|
||||
},
|
||||
[getCombinedDiffSectionKey, virtualizer]
|
||||
)
|
||||
const recordCombinedDiffDomScrollAnchor = useCallback((): boolean => {
|
||||
const container = scrollContainerRef.current
|
||||
if (!container) {
|
||||
return false
|
||||
}
|
||||
|
||||
const containerRect = container.getBoundingClientRect()
|
||||
const visibleRows = Array.from(
|
||||
container.querySelectorAll<HTMLElement>('[data-combined-diff-section-row]')
|
||||
)
|
||||
.map((row) => {
|
||||
const key = row.dataset.combinedDiffSectionKey
|
||||
if (!key || !row.isConnected) {
|
||||
return null
|
||||
}
|
||||
const rect = row.getBoundingClientRect()
|
||||
if (
|
||||
rect.height <= 0 ||
|
||||
rect.bottom <= containerRect.top ||
|
||||
rect.top >= containerRect.bottom
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return { key, rect }
|
||||
})
|
||||
.filter((row): row is { key: string; rect: DOMRect } => row !== null)
|
||||
.sort((a, b) => a.rect.top - b.rect.top)
|
||||
|
||||
const firstVisible = visibleRows[0]
|
||||
if (!firstVisible) {
|
||||
return false
|
||||
}
|
||||
|
||||
const anchor: NonNullable<VirtualizedScrollAnchor> = {
|
||||
fallbackKeys: visibleRows.slice(1).map((row) => row.key),
|
||||
key: firstVisible.key,
|
||||
offset: Math.min(
|
||||
firstVisible.rect.height,
|
||||
Math.max(0, containerRect.top - firstVisible.rect.top)
|
||||
)
|
||||
}
|
||||
scrollAnchorRef.current = anchor
|
||||
latestDomScrollAnchorRef.current = anchor
|
||||
return true
|
||||
}, [])
|
||||
const writeCombinedDiffScrollAnchor = useCallback((): void => {
|
||||
const anchor = scrollAnchorRef.current
|
||||
if (anchor) {
|
||||
setWithLRU(combinedDiffScrollAnchorCache, viewStateKey, anchor)
|
||||
} else {
|
||||
combinedDiffScrollAnchorCache.delete(viewStateKey)
|
||||
}
|
||||
}, [viewStateKey])
|
||||
const persistCombinedDiffScrollAnchor = useCallback(
|
||||
(refreshDomAnchor = true): void => {
|
||||
if (refreshDomAnchor) {
|
||||
recordCombinedDiffDomScrollAnchor()
|
||||
}
|
||||
writeCombinedDiffScrollAnchor()
|
||||
},
|
||||
[recordCombinedDiffDomScrollAnchor, writeCombinedDiffScrollAnchor]
|
||||
)
|
||||
|
||||
useVirtualizedScrollAnchor({
|
||||
anchorRef: scrollAnchorRef,
|
||||
getItemElementKey: getCombinedDiffSectionElementKey,
|
||||
getRowKey: getCombinedDiffSectionKey,
|
||||
hasDirectScrollInput,
|
||||
itemElementSelector: '[data-combined-diff-section-row]',
|
||||
recordAnchorOnCleanup: false,
|
||||
recordAnchorOnScroll: false,
|
||||
rows: sections,
|
||||
scrollElementRef: scrollContainerRef,
|
||||
shouldSkipRestore: hasDirectScrollInput,
|
||||
scrollOffsetRef,
|
||||
totalSize: combinedDiffTotalSize,
|
||||
virtualizer
|
||||
})
|
||||
|
||||
useLayoutEffect(() => {
|
||||
// Why: inline vs side-by-side can change Monaco content heights across
|
||||
|
|
@ -824,13 +986,18 @@ export default function CombinedDiffViewer({
|
|||
)
|
||||
const handleTreeNavigate = useCallback(
|
||||
(entry: GitStatusEntry | GitBranchChangeEntry) => {
|
||||
markDirectScrollInput()
|
||||
const navigatedIndex = handleCombinedDiffFileTreeNavigation({
|
||||
mode: treeMode,
|
||||
entry,
|
||||
sections: sectionsRef.current,
|
||||
sectionIndexByKey,
|
||||
toggleSection,
|
||||
scrollToIndex: (index) => virtualizer.scrollToIndex(index, { align: 'start' })
|
||||
scrollToIndex: (index) => {
|
||||
scrollAnchorRef.current = null
|
||||
latestDomScrollAnchorRef.current = null
|
||||
virtualizer.scrollToIndex(index, { align: 'start' })
|
||||
}
|
||||
})
|
||||
if (navigatedIndex !== null) {
|
||||
// Why: tree navigation is also the user's explicit "show me this diff"
|
||||
|
|
@ -845,6 +1012,7 @@ export default function CombinedDiffViewer({
|
|||
},
|
||||
[
|
||||
entrySignature,
|
||||
markDirectScrollInput,
|
||||
requestCombinedDiffSectionReload,
|
||||
sectionIndexByKey,
|
||||
toggleSection,
|
||||
|
|
@ -1095,20 +1263,82 @@ export default function CombinedDiffViewer({
|
|||
|
||||
const cached = combinedDiffViewStateCache.get(viewStateKey)
|
||||
if (cached && cached.entrySignature === entrySignature) {
|
||||
pendingRestoreScrollTopRef.current =
|
||||
combinedDiffScrollTopCache.get(viewStateKey) ?? cached.scrollTop
|
||||
scrollOffsetRef.current = combinedDiffScrollTopCache.get(viewStateKey) ?? cached.scrollTop
|
||||
}
|
||||
|
||||
const updateCachedScrollPosition = (): void => {
|
||||
let anchorIdleTimerId: number | null = null
|
||||
let anchorFrameId: number | null = null
|
||||
const cancelScheduledAnchorPersist = (): void => {
|
||||
if (anchorIdleTimerId !== null) {
|
||||
window.clearTimeout(anchorIdleTimerId)
|
||||
anchorIdleTimerId = null
|
||||
}
|
||||
if (anchorFrameId !== null) {
|
||||
window.cancelAnimationFrame(anchorFrameId)
|
||||
anchorFrameId = null
|
||||
}
|
||||
}
|
||||
const scheduleSettledAnchorPersist = (): void => {
|
||||
cancelScheduledAnchorPersist()
|
||||
anchorIdleTimerId = window.setTimeout(() => {
|
||||
anchorIdleTimerId = null
|
||||
if (hasDirectScrollInput()) {
|
||||
// Why: the first idle timer can fire while wheel input is still
|
||||
// active and TanStack may be showing a transitional virtual window.
|
||||
scheduleSettledAnchorPersist()
|
||||
return
|
||||
}
|
||||
anchorFrameId = window.requestAnimationFrame(() => {
|
||||
anchorFrameId = null
|
||||
persistCombinedDiffScrollAnchor()
|
||||
})
|
||||
}, 150)
|
||||
}
|
||||
|
||||
const updateCachedScrollPosition = ({
|
||||
recordDomAnchor,
|
||||
scheduleSettled,
|
||||
scrollTop,
|
||||
writeAnchor
|
||||
}: {
|
||||
recordDomAnchor: boolean
|
||||
scheduleSettled: boolean
|
||||
scrollTop: number
|
||||
writeAnchor: boolean
|
||||
}): void => {
|
||||
const existing = combinedDiffViewStateCache.get(viewStateKey)
|
||||
setWithLRU(combinedDiffScrollTopCache, viewStateKey, container.scrollTop)
|
||||
scrollOffsetRef.current = scrollTop
|
||||
setWithLRU(combinedDiffScrollTopCache, viewStateKey, scrollTop)
|
||||
if (writeAnchor) {
|
||||
if (recordDomAnchor) {
|
||||
persistCombinedDiffScrollAnchor()
|
||||
} else {
|
||||
writeCombinedDiffScrollAnchor()
|
||||
}
|
||||
}
|
||||
if (scheduleSettled) {
|
||||
scheduleSettledAnchorPersist()
|
||||
}
|
||||
updateCombinedDiffScrollbar()
|
||||
if (!existing || existing.entrySignature !== entrySignature) {
|
||||
return
|
||||
}
|
||||
setWithLRU(combinedDiffViewStateCache, viewStateKey, {
|
||||
...existing,
|
||||
scrollTop: container.scrollTop
|
||||
scrollTop
|
||||
})
|
||||
}
|
||||
const handleScroll = (): void => {
|
||||
if (!hasDirectScrollInput()) {
|
||||
updateCombinedDiffScrollbar()
|
||||
return
|
||||
}
|
||||
recordCombinedDiffVirtualScrollAnchor(container.scrollTop)
|
||||
updateCachedScrollPosition({
|
||||
recordDomAnchor: false,
|
||||
scheduleSettled: true,
|
||||
scrollTop: container.scrollTop,
|
||||
writeAnchor: true
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -1119,54 +1349,63 @@ export default function CombinedDiffViewer({
|
|||
updateCombinedDiffScrollbar()
|
||||
const resizeObserver = new ResizeObserver(updateCombinedDiffScrollbar)
|
||||
resizeObserver.observe(container)
|
||||
container.addEventListener('scroll', updateCachedScrollPosition)
|
||||
container.addEventListener('scroll', handleScroll)
|
||||
return () => {
|
||||
updateCachedScrollPosition()
|
||||
cancelScheduledAnchorPersist()
|
||||
if (latestDomScrollAnchorRef.current) {
|
||||
scrollAnchorRef.current = latestDomScrollAnchorRef.current
|
||||
}
|
||||
updateCachedScrollPosition({
|
||||
recordDomAnchor: false,
|
||||
scheduleSettled: false,
|
||||
scrollTop: scrollOffsetRef.current,
|
||||
writeAnchor: true
|
||||
})
|
||||
resizeObserver.disconnect()
|
||||
container.removeEventListener('scroll', updateCachedScrollPosition)
|
||||
container.removeEventListener('scroll', handleScroll)
|
||||
}
|
||||
}, [entrySignature, sections.length, updateCombinedDiffScrollbar, viewStateKey])
|
||||
}, [
|
||||
entrySignature,
|
||||
hasDirectScrollInput,
|
||||
persistCombinedDiffScrollAnchor,
|
||||
recordCombinedDiffVirtualScrollAnchor,
|
||||
sections.length,
|
||||
updateCombinedDiffScrollbar,
|
||||
writeCombinedDiffScrollAnchor,
|
||||
viewStateKey
|
||||
])
|
||||
|
||||
useLayoutEffect(() => {
|
||||
updateCombinedDiffScrollbar()
|
||||
}, [sectionHeights, sections, updateCombinedDiffScrollbar])
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const container = scrollContainerRef.current
|
||||
const targetScrollTop = pendingRestoreScrollTopRef.current
|
||||
if (!container || targetScrollTop === null) {
|
||||
if (!container || container.scrollTop <= 0) {
|
||||
return
|
||||
}
|
||||
|
||||
let frameId = 0
|
||||
let attempts = 0
|
||||
|
||||
const restoreScrollPosition = (): void => {
|
||||
const liveContainer = scrollContainerRef.current
|
||||
const liveTarget = pendingRestoreScrollTopRef.current
|
||||
if (!liveContainer || liveTarget === null) {
|
||||
let frameId: number | null = null
|
||||
const timerId = window.setTimeout(() => {
|
||||
if (!container.isConnected || hasDirectScrollInput()) {
|
||||
return
|
||||
}
|
||||
frameId = window.requestAnimationFrame(() => {
|
||||
frameId = null
|
||||
persistCombinedDiffScrollAnchor()
|
||||
})
|
||||
}, 300)
|
||||
|
||||
const maxScrollTop = Math.max(0, liveContainer.scrollHeight - liveContainer.clientHeight)
|
||||
const nextScrollTop = Math.min(liveTarget, maxScrollTop)
|
||||
liveContainer.scrollTop = nextScrollTop
|
||||
setWithLRU(combinedDiffScrollTopCache, viewStateKey, nextScrollTop)
|
||||
|
||||
if (Math.abs(liveContainer.scrollTop - liveTarget) <= 1 || maxScrollTop >= liveTarget) {
|
||||
pendingRestoreScrollTopRef.current = null
|
||||
return
|
||||
}
|
||||
|
||||
attempts += 1
|
||||
if (attempts < 30) {
|
||||
frameId = window.requestAnimationFrame(restoreScrollPosition)
|
||||
return () => {
|
||||
window.clearTimeout(timerId)
|
||||
if (frameId !== null) {
|
||||
window.cancelAnimationFrame(frameId)
|
||||
}
|
||||
}
|
||||
|
||||
restoreScrollPosition()
|
||||
return () => window.cancelAnimationFrame(frameId)
|
||||
}, [sectionHeights, sections, viewStateKey])
|
||||
}, [
|
||||
hasDirectScrollInput,
|
||||
persistCombinedDiffScrollAnchor,
|
||||
sectionHeights,
|
||||
sections,
|
||||
updateCombinedDiffScrollbar
|
||||
])
|
||||
|
||||
const openAlternateDiff = useCallback(() => {
|
||||
if (!file.combinedAlternate) {
|
||||
|
|
@ -1193,6 +1432,7 @@ export default function CombinedDiffViewer({
|
|||
}
|
||||
|
||||
event.preventDefault()
|
||||
markDirectScrollInput()
|
||||
const track = event.currentTarget
|
||||
const thumb =
|
||||
event.target instanceof HTMLElement
|
||||
|
|
@ -1231,6 +1471,7 @@ export default function CombinedDiffViewer({
|
|||
|
||||
const handlePointerMove = (moveEvent: PointerEvent): void => {
|
||||
moveEvent.preventDefault()
|
||||
markDirectScrollInput()
|
||||
container.scrollTop = getScrollTopForPointer(moveEvent.clientY, grabOffset)
|
||||
updateCombinedDiffScrollbar()
|
||||
}
|
||||
|
|
@ -1248,7 +1489,7 @@ export default function CombinedDiffViewer({
|
|||
})
|
||||
activeScrollbarDragCleanupRef.current = cleanupPointerDrag
|
||||
},
|
||||
[cleanupActiveScrollbarDrag, updateCombinedDiffScrollbar]
|
||||
[cleanupActiveScrollbarDrag, markDirectScrollInput, updateCombinedDiffScrollbar]
|
||||
)
|
||||
|
||||
const handleCopyNotes = useCallback(async (): Promise<void> => {
|
||||
|
|
@ -1592,12 +1833,11 @@ export default function CombinedDiffViewer({
|
|||
<div
|
||||
ref={setScrollContainerRef}
|
||||
className="combined-diff-scroll-container h-full overflow-auto pr-5 scrollbar-editor"
|
||||
onWheel={markDirectScrollInput}
|
||||
onTouchMove={markDirectScrollInput}
|
||||
>
|
||||
{skippedConflictNotice}
|
||||
<div
|
||||
className="relative w-full"
|
||||
style={{ height: `${virtualizer.getTotalSize()}px` }}
|
||||
>
|
||||
<div className="relative w-full" style={{ height: `${combinedDiffTotalSize}px` }}>
|
||||
{virtualizer.getVirtualItems().map((virtualItem) => {
|
||||
const section = sections[virtualItem.index]
|
||||
if (!section) {
|
||||
|
|
@ -1608,6 +1848,8 @@ export default function CombinedDiffViewer({
|
|||
<div
|
||||
key={virtualItem.key}
|
||||
data-index={virtualItem.index}
|
||||
data-combined-diff-section-row
|
||||
data-combined-diff-section-key={section.key}
|
||||
ref={virtualizer.measureElement}
|
||||
className="absolute left-0 top-0 w-full"
|
||||
// Why: `top` preserves sticky file headers inside each row;
|
||||
|
|
|
|||
|
|
@ -2,7 +2,10 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
|
|||
|
||||
function createReactHookHarness() {
|
||||
const refs: { current: unknown }[] = []
|
||||
const effects: { deps: readonly unknown[] | undefined }[] = []
|
||||
const effects: {
|
||||
deps: readonly unknown[] | undefined
|
||||
effect: () => void | (() => void)
|
||||
}[] = []
|
||||
let refIndex = 0
|
||||
|
||||
return {
|
||||
|
|
@ -13,8 +16,8 @@ function createReactHookHarness() {
|
|||
effects,
|
||||
react: {
|
||||
useCallback: <T extends (...args: never[]) => unknown>(callback: T): T => callback,
|
||||
useLayoutEffect: (_effect: () => void | (() => void), deps?: readonly unknown[]) => {
|
||||
effects.push({ deps })
|
||||
useLayoutEffect: (effect: () => void | (() => void), deps?: readonly unknown[]) => {
|
||||
effects.push({ deps, effect })
|
||||
},
|
||||
useMemo: <T>(factory: () => T): T => factory(),
|
||||
useRef: <T>(initialValue: T): { current: T } => {
|
||||
|
|
@ -69,4 +72,99 @@ describe('useVirtualizedScrollAnchor listener effect dependencies', () => {
|
|||
expect(initialDeps).toEqual([scrollElementRef, scrollOffsetRef])
|
||||
expect(nextDeps).toEqual(initialDeps)
|
||||
})
|
||||
|
||||
it('keeps the target anchor while measured fallback restores a transitional window', async () => {
|
||||
const harness = createReactHookHarness()
|
||||
vi.doMock('react', () => harness.react)
|
||||
const { useVirtualizedScrollAnchor } = await import('./useVirtualizedScrollAnchor')
|
||||
|
||||
const anchorRef = { current: { key: 'row-1', offset: 3358 } }
|
||||
const scrollElementRef = {
|
||||
current: {
|
||||
clientHeight: 880,
|
||||
scrollHeight: 30_000,
|
||||
scrollTop: 0,
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn()
|
||||
}
|
||||
}
|
||||
const scrollOffsetRef = { current: 0 }
|
||||
const virtualizer = {
|
||||
getVirtualItems: () => [
|
||||
{ index: 8, start: 0, end: 30_000 },
|
||||
{ index: 1, start: 758, end: 4512 }
|
||||
],
|
||||
isScrolling: false,
|
||||
scrollToIndex: vi.fn()
|
||||
}
|
||||
|
||||
harness.beginRender()
|
||||
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
|
||||
useVirtualizedScrollAnchor({
|
||||
anchorRef,
|
||||
getRowKey: (row) => row,
|
||||
rows: Array.from({ length: 9 }, (_, index) => `row-${index}`),
|
||||
scrollElementRef,
|
||||
scrollOffsetRef,
|
||||
totalSize: 30_000,
|
||||
virtualizer
|
||||
} as never)
|
||||
|
||||
harness.effects[1]?.effect()
|
||||
|
||||
expect(scrollElementRef.current.scrollTop).toBe(4116)
|
||||
expect(anchorRef.current).toEqual({ key: 'row-1', offset: 3358 })
|
||||
})
|
||||
|
||||
it('can ignore generic scroll anchor recording while preserving the saved anchor', async () => {
|
||||
const harness = createReactHookHarness()
|
||||
vi.doMock('react', () => harness.react)
|
||||
const { useVirtualizedScrollAnchor } = await import('./useVirtualizedScrollAnchor')
|
||||
|
||||
const capturedScrollHandler: { current: (() => void) | null } = { current: null }
|
||||
const savedAnchor = { key: 'row-1', offset: 3358 }
|
||||
const anchorRef = { current: savedAnchor }
|
||||
const scrollElementRef = {
|
||||
current: {
|
||||
clientHeight: 880,
|
||||
scrollHeight: 30_000,
|
||||
scrollTop: 746,
|
||||
addEventListener: vi.fn((eventName: string, handler: () => void) => {
|
||||
if (eventName === 'scroll') {
|
||||
capturedScrollHandler.current = handler
|
||||
}
|
||||
}),
|
||||
removeEventListener: vi.fn()
|
||||
}
|
||||
}
|
||||
const scrollOffsetRef = { current: 0 }
|
||||
const virtualizer = {
|
||||
getVirtualItems: () => [
|
||||
{ index: 0, start: 0, end: 3_000 },
|
||||
{ index: 1, start: 3_000, end: 7_000 }
|
||||
],
|
||||
isScrolling: false,
|
||||
scrollToIndex: vi.fn()
|
||||
}
|
||||
|
||||
harness.beginRender()
|
||||
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
|
||||
useVirtualizedScrollAnchor({
|
||||
anchorRef,
|
||||
getRowKey: (row) => row,
|
||||
recordAnchorOnScroll: false,
|
||||
rows: ['row-0', 'row-1'],
|
||||
scrollElementRef,
|
||||
scrollOffsetRef,
|
||||
totalSize: 30_000,
|
||||
virtualizer
|
||||
} as never)
|
||||
|
||||
harness.effects[0]?.effect()
|
||||
expect(capturedScrollHandler.current).not.toBeNull()
|
||||
capturedScrollHandler.current?.()
|
||||
|
||||
expect(scrollOffsetRef.current).toBe(0)
|
||||
expect(anchorRef.current).toBe(savedAnchor)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -8,6 +8,10 @@ import {
|
|||
} from 'react'
|
||||
import type { Virtualizer } from '@tanstack/react-virtual'
|
||||
import { shouldCancelVirtualizedScrollOffsetRestore } from './virtualizedScrollOffsetRestore'
|
||||
import {
|
||||
findVirtualizedDomScrollAnchor,
|
||||
getVirtualizedScrollAnchorForOffset
|
||||
} from './virtualized-scroll-anchor-recording'
|
||||
|
||||
export type VirtualizedScrollAnchor = {
|
||||
fallbackKeys?: readonly string[]
|
||||
|
|
@ -27,6 +31,10 @@ type UseVirtualizedScrollAnchorOptions<
|
|||
getRowKey: (row: TRow) => string
|
||||
hasDirectScrollInput?: () => boolean
|
||||
itemElementSelector?: string
|
||||
recordAnchorOnCleanup?: boolean
|
||||
// Why: some callers record user scroll anchors outside this hook; passive
|
||||
// programmatic scroll events during remount must not teach them a transient row.
|
||||
recordAnchorOnScroll?: boolean
|
||||
rows: readonly TRow[]
|
||||
scrollElementRef: RefObject<TScrollElement | null>
|
||||
scrollOffsetRef: MutableRefObject<number>
|
||||
|
|
@ -53,6 +61,8 @@ export function useVirtualizedScrollAnchor<
|
|||
getRowKey,
|
||||
hasDirectScrollInput,
|
||||
itemElementSelector,
|
||||
recordAnchorOnCleanup = true,
|
||||
recordAnchorOnScroll = true,
|
||||
rows,
|
||||
scrollElementRef,
|
||||
scrollOffsetRef,
|
||||
|
|
@ -68,64 +78,14 @@ export function useVirtualizedScrollAnchor<
|
|||
return indexByKey
|
||||
}, [getRowKey, rows])
|
||||
|
||||
const findDomAnchor = useCallback(
|
||||
(scrollElement: TScrollElement) => {
|
||||
if (!itemElementSelector || !getItemElementKey) {
|
||||
return null
|
||||
}
|
||||
const scrollRect = scrollElement.getBoundingClientRect()
|
||||
type DomAnchorItem = { element: TItemElement; key: string; rect: DOMRect }
|
||||
const visibleItems = Array.from(
|
||||
scrollElement.querySelectorAll<TItemElement>(itemElementSelector)
|
||||
)
|
||||
.map((element) => {
|
||||
const key = getItemElementKey(element)
|
||||
if (!key || !rowIndexByKey.has(key) || !element.isConnected) {
|
||||
return null
|
||||
}
|
||||
const rect = element.getBoundingClientRect()
|
||||
if (rect.height <= 0 || rect.bottom <= scrollRect.top || rect.top >= scrollRect.bottom) {
|
||||
return null
|
||||
}
|
||||
return { element, key, rect }
|
||||
})
|
||||
.filter((item): item is DomAnchorItem => item != null)
|
||||
.sort((a, b) => a.rect.top - b.rect.top)
|
||||
|
||||
const [firstVisible] = visibleItems
|
||||
if (!firstVisible) {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
fallbackKeys: visibleItems.slice(1).map((item) => item.key),
|
||||
key: firstVisible.key,
|
||||
offset: Math.min(
|
||||
firstVisible.rect.height,
|
||||
Math.max(0, scrollRect.top - firstVisible.rect.top)
|
||||
)
|
||||
}
|
||||
},
|
||||
[getItemElementKey, itemElementSelector, rowIndexByKey]
|
||||
)
|
||||
|
||||
const recordVirtualScrollAnchor = useCallback(
|
||||
(scrollTop: number) => {
|
||||
const virtualItems = virtualizer.getVirtualItems()
|
||||
const firstVisible = virtualItems.find((item) => item.end > scrollTop)
|
||||
const row = firstVisible ? rows[firstVisible.index] : undefined
|
||||
if (!firstVisible || !row) {
|
||||
anchorRef.current = null
|
||||
return
|
||||
}
|
||||
anchorRef.current = {
|
||||
fallbackKeys: virtualItems
|
||||
.slice(virtualItems.indexOf(firstVisible) + 1)
|
||||
.map((item) => rows[item.index])
|
||||
.filter((row): row is TRow => row != null)
|
||||
.map(getRowKey),
|
||||
key: getRowKey(row),
|
||||
offset: Math.max(0, scrollTop - firstVisible.start)
|
||||
}
|
||||
anchorRef.current = getVirtualizedScrollAnchorForOffset({
|
||||
getRowKey,
|
||||
rows,
|
||||
scrollTop,
|
||||
virtualItems: virtualizer.getVirtualItems()
|
||||
})
|
||||
},
|
||||
[anchorRef, getRowKey, rows, virtualizer]
|
||||
)
|
||||
|
|
@ -133,17 +93,36 @@ export function useVirtualizedScrollAnchor<
|
|||
const recordScrollAnchor = useCallback(
|
||||
(scrollTop: number) => {
|
||||
const scrollElement = scrollElementRef.current
|
||||
if (scrollElement) {
|
||||
const domAnchor = findDomAnchor(scrollElement)
|
||||
if (scrollElement && itemElementSelector && getItemElementKey) {
|
||||
const domAnchor = findVirtualizedDomScrollAnchor<TItemElement>({
|
||||
getItemElementKey,
|
||||
itemElementSelector,
|
||||
rowIndexByKey,
|
||||
scrollElement
|
||||
})
|
||||
if (domAnchor) {
|
||||
anchorRef.current = domAnchor
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
recordVirtualScrollAnchor(scrollTop)
|
||||
anchorRef.current = getVirtualizedScrollAnchorForOffset({
|
||||
getRowKey,
|
||||
rows,
|
||||
scrollTop,
|
||||
virtualItems: virtualizer.getVirtualItems()
|
||||
})
|
||||
},
|
||||
[anchorRef, findDomAnchor, recordVirtualScrollAnchor, scrollElementRef]
|
||||
[
|
||||
anchorRef,
|
||||
getItemElementKey,
|
||||
getRowKey,
|
||||
itemElementSelector,
|
||||
rowIndexByKey,
|
||||
rows,
|
||||
scrollElementRef,
|
||||
virtualizer
|
||||
]
|
||||
)
|
||||
|
||||
// Why: row changes must not re-register the scroll listener; cleanup records
|
||||
|
|
@ -154,6 +133,10 @@ export function useVirtualizedScrollAnchor<
|
|||
recordVirtualScrollAnchorRef.current = recordVirtualScrollAnchor
|
||||
const hasDirectScrollInputRef = useRef(hasDirectScrollInput)
|
||||
hasDirectScrollInputRef.current = hasDirectScrollInput
|
||||
const recordAnchorOnCleanupRef = useRef(recordAnchorOnCleanup)
|
||||
recordAnchorOnCleanupRef.current = recordAnchorOnCleanup
|
||||
const recordAnchorOnScrollRef = useRef(recordAnchorOnScroll)
|
||||
recordAnchorOnScrollRef.current = recordAnchorOnScroll
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const el = scrollElementRef.current
|
||||
|
|
@ -216,18 +199,29 @@ export function useVirtualizedScrollAnchor<
|
|||
// user's real position until the intended offset is reachable.
|
||||
if (el.scrollTop === targetOffset) {
|
||||
restoring = false
|
||||
recordCurrentAnchor()
|
||||
if (recordAnchorOnScrollRef.current) {
|
||||
recordCurrentAnchor()
|
||||
} else {
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
}
|
||||
return
|
||||
}
|
||||
if (el.scrollHeight - el.clientHeight >= targetOffset) {
|
||||
el.scrollTop = targetOffset
|
||||
if (el.scrollTop === targetOffset) {
|
||||
restoring = false
|
||||
recordCurrentAnchor()
|
||||
if (recordAnchorOnScrollRef.current) {
|
||||
recordCurrentAnchor()
|
||||
} else {
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
}
|
||||
}
|
||||
}
|
||||
return
|
||||
}
|
||||
if (!recordAnchorOnScrollRef.current) {
|
||||
return
|
||||
}
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
recordVirtualScrollAnchorRef.current(el.scrollTop)
|
||||
scheduleRecordAnchor()
|
||||
|
|
@ -237,8 +231,10 @@ export function useVirtualizedScrollAnchor<
|
|||
el.addEventListener(VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT, recordCurrentAnchor)
|
||||
return () => {
|
||||
cancelScheduledRecord()
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
recordScrollAnchorRef.current(el.scrollTop)
|
||||
if (recordAnchorOnCleanupRef.current) {
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
recordScrollAnchorRef.current(el.scrollTop)
|
||||
}
|
||||
el.removeEventListener(VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT, recordCurrentAnchor)
|
||||
el.removeEventListener('scroll', onScroll)
|
||||
}
|
||||
|
|
@ -250,9 +246,10 @@ export function useVirtualizedScrollAnchor<
|
|||
if (!anchor || !el) {
|
||||
return
|
||||
}
|
||||
if (virtualizer.isScrolling) {
|
||||
if (virtualizer.isScrolling && hasDirectScrollInputRef.current?.() === true) {
|
||||
// Why: remeasurement during wheel scrolling can change totalSize. Restoring
|
||||
// the anchor in that window writes scrollTop and fights the user's wheel.
|
||||
// Programmatic scrolls during remount still need anchor correction.
|
||||
return
|
||||
}
|
||||
if (shouldSkipRestore?.()) {
|
||||
|
|
@ -287,7 +284,11 @@ export function useVirtualizedScrollAnchor<
|
|||
const desiredTop = scrollRect.top - offset
|
||||
const delta = rect.top - desiredTop
|
||||
if (Math.abs(delta) > 1) {
|
||||
// Why: this scroll write is still part of restore; keep the target
|
||||
// anchor until a later layout confirms the intended row is in place.
|
||||
el.scrollTop += delta
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
return true
|
||||
}
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
recordScrollAnchor(el.scrollTop)
|
||||
|
|
@ -304,8 +305,9 @@ export function useVirtualizedScrollAnchor<
|
|||
if (Math.abs(el.scrollTop - nextScrollTop) > 1) {
|
||||
el.scrollTop = nextScrollTop
|
||||
}
|
||||
// Why: measured fallback can run while TanStack's virtual window is
|
||||
// transitional, so recording here can replace the target with a wrong row.
|
||||
scrollOffsetRef.current = el.scrollTop
|
||||
recordScrollAnchor(el.scrollTop)
|
||||
return true
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,75 @@
|
|||
import type { VirtualizedScrollAnchor } from './useVirtualizedScrollAnchor'
|
||||
|
||||
type VirtualScrollItem = {
|
||||
end: number
|
||||
index: number
|
||||
start: number
|
||||
}
|
||||
|
||||
export function findVirtualizedDomScrollAnchor<TItemElement extends Element>({
|
||||
getItemElementKey,
|
||||
itemElementSelector,
|
||||
rowIndexByKey,
|
||||
scrollElement
|
||||
}: {
|
||||
getItemElementKey: (element: TItemElement) => string | null
|
||||
itemElementSelector: string
|
||||
rowIndexByKey: ReadonlyMap<string, number>
|
||||
scrollElement: Element
|
||||
}): NonNullable<VirtualizedScrollAnchor> | null {
|
||||
const scrollRect = scrollElement.getBoundingClientRect()
|
||||
type DomAnchorItem = { key: string; rect: DOMRect }
|
||||
const visibleItems = Array.from(scrollElement.querySelectorAll<TItemElement>(itemElementSelector))
|
||||
.map((element) => {
|
||||
const key = getItemElementKey(element)
|
||||
if (!key || !rowIndexByKey.has(key) || !element.isConnected) {
|
||||
return null
|
||||
}
|
||||
const rect = element.getBoundingClientRect()
|
||||
if (rect.height <= 0 || rect.bottom <= scrollRect.top || rect.top >= scrollRect.bottom) {
|
||||
return null
|
||||
}
|
||||
return { key, rect }
|
||||
})
|
||||
.filter((item): item is DomAnchorItem => item != null)
|
||||
.sort((a, b) => a.rect.top - b.rect.top)
|
||||
|
||||
const [firstVisible] = visibleItems
|
||||
if (!firstVisible) {
|
||||
return null
|
||||
}
|
||||
|
||||
return {
|
||||
fallbackKeys: visibleItems.slice(1).map((item) => item.key),
|
||||
key: firstVisible.key,
|
||||
offset: Math.min(firstVisible.rect.height, Math.max(0, scrollRect.top - firstVisible.rect.top))
|
||||
}
|
||||
}
|
||||
|
||||
export function getVirtualizedScrollAnchorForOffset<TRow>({
|
||||
getRowKey,
|
||||
rows,
|
||||
scrollTop,
|
||||
virtualItems
|
||||
}: {
|
||||
getRowKey: (row: TRow) => string
|
||||
rows: readonly TRow[]
|
||||
scrollTop: number
|
||||
virtualItems: readonly VirtualScrollItem[]
|
||||
}): VirtualizedScrollAnchor {
|
||||
const firstVisible = virtualItems.find((item) => item.end > scrollTop)
|
||||
const row = firstVisible ? rows[firstVisible.index] : undefined
|
||||
if (!firstVisible || !row) {
|
||||
return null
|
||||
}
|
||||
|
||||
return {
|
||||
fallbackKeys: virtualItems
|
||||
.slice(virtualItems.indexOf(firstVisible) + 1)
|
||||
.map((item) => rows[item.index])
|
||||
.filter((row): row is TRow => row != null)
|
||||
.map(getRowKey),
|
||||
key: getRowKey(row),
|
||||
offset: Math.max(0, scrollTop - firstVisible.start)
|
||||
}
|
||||
}
|
||||
|
|
@ -851,6 +851,48 @@ describe('TabsSlice', () => {
|
|||
// on that group's active terminal tab. Without this, the bell lingers
|
||||
// until the tab is clicked a second time.
|
||||
describe('focusGroup', () => {
|
||||
it('does not broadcast active-surface writes when the focused group is already current', () => {
|
||||
const editorFileId = '/tmp/feature/src/main.ts'
|
||||
const tab = store.getState().createUnifiedTab(WT, 'editor', {
|
||||
id: 'editor-tab-1',
|
||||
entityId: editorFileId,
|
||||
label: 'main.ts'
|
||||
})
|
||||
const groupId = store.getState().groupsByWorktree[WT][0].id
|
||||
store.setState({
|
||||
activeWorktreeId: WT,
|
||||
openFiles: [makeOpenFile({ id: editorFileId, worktreeId: WT })],
|
||||
activeGroupIdByWorktree: { [WT]: groupId },
|
||||
activeFileId: editorFileId,
|
||||
activeFileIdByWorktree: { [WT]: editorFileId },
|
||||
activeBrowserTabId: null,
|
||||
activeBrowserTabIdByWorktree: { [WT]: null },
|
||||
activeTabId: null,
|
||||
activeTabIdByWorktree: { [WT]: null },
|
||||
activeTabType: 'editor',
|
||||
activeTabTypeByWorktree: { [WT]: 'editor' },
|
||||
groupsByWorktree: {
|
||||
[WT]: [
|
||||
{
|
||||
...store.getState().groupsByWorktree[WT][0],
|
||||
activeTabId: tab.id
|
||||
}
|
||||
]
|
||||
}
|
||||
})
|
||||
const before = store.getState()
|
||||
const listener = vi.fn()
|
||||
const unsubscribe = store.subscribe(listener)
|
||||
|
||||
store.getState().focusGroup(WT, groupId)
|
||||
unsubscribe()
|
||||
|
||||
expect(listener).not.toHaveBeenCalled()
|
||||
expect(store.getState().activeGroupIdByWorktree).toBe(before.activeGroupIdByWorktree)
|
||||
expect(store.getState().activeFileIdByWorktree).toBe(before.activeFileIdByWorktree)
|
||||
expect(store.getState().activeTabTypeByWorktree).toBe(before.activeTabTypeByWorktree)
|
||||
})
|
||||
|
||||
// Why: focusGroup is fired on every pointerdown within a split group's
|
||||
// chrome (onPointerDown + onFocusCapture in TabGroupPanel). Clearing the
|
||||
// tab-level bell here is fine — the user is now looking at this group.
|
||||
|
|
|
|||
|
|
@ -534,6 +534,44 @@ function buildActiveSurfacePatch(
|
|||
}
|
||||
}
|
||||
|
||||
function activeSurfacePatchMatchesState(
|
||||
state: Pick<
|
||||
AppState,
|
||||
| 'activeBrowserTabId'
|
||||
| 'activeBrowserTabIdByWorktree'
|
||||
| 'activeFileId'
|
||||
| 'activeFileIdByWorktree'
|
||||
| 'activeTabId'
|
||||
| 'activeTabIdByWorktree'
|
||||
| 'activeTabType'
|
||||
| 'activeTabTypeByWorktree'
|
||||
>,
|
||||
worktreeId: string,
|
||||
patch: Pick<
|
||||
AppState,
|
||||
| 'activeBrowserTabId'
|
||||
| 'activeBrowserTabIdByWorktree'
|
||||
| 'activeFileId'
|
||||
| 'activeFileIdByWorktree'
|
||||
| 'activeTabId'
|
||||
| 'activeTabIdByWorktree'
|
||||
| 'activeTabType'
|
||||
| 'activeTabTypeByWorktree'
|
||||
>
|
||||
): boolean {
|
||||
return (
|
||||
state.activeBrowserTabId === patch.activeBrowserTabId &&
|
||||
state.activeBrowserTabIdByWorktree[worktreeId] ===
|
||||
patch.activeBrowserTabIdByWorktree[worktreeId] &&
|
||||
state.activeFileId === patch.activeFileId &&
|
||||
state.activeFileIdByWorktree[worktreeId] === patch.activeFileIdByWorktree[worktreeId] &&
|
||||
state.activeTabId === patch.activeTabId &&
|
||||
state.activeTabIdByWorktree[worktreeId] === patch.activeTabIdByWorktree[worktreeId] &&
|
||||
state.activeTabType === patch.activeTabType &&
|
||||
state.activeTabTypeByWorktree[worktreeId] === patch.activeTabTypeByWorktree[worktreeId]
|
||||
)
|
||||
}
|
||||
|
||||
export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set, get) => ({
|
||||
unifiedTabsByWorktree: {},
|
||||
renamingTabId: null,
|
||||
|
|
@ -1171,10 +1209,13 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
|
|||
|
||||
focusGroup: (worktreeId, groupId) =>
|
||||
set((state) => {
|
||||
const nextActiveGroupIdByWorktree = {
|
||||
...state.activeGroupIdByWorktree,
|
||||
[worktreeId]: groupId
|
||||
}
|
||||
const groupAlreadyFocused = state.activeGroupIdByWorktree[worktreeId] === groupId
|
||||
const nextActiveGroupIdByWorktree = groupAlreadyFocused
|
||||
? state.activeGroupIdByWorktree
|
||||
: {
|
||||
...state.activeGroupIdByWorktree,
|
||||
[worktreeId]: groupId
|
||||
}
|
||||
// Why: focusing a split group surfaces whichever terminal tab is already
|
||||
// active in that group, so the tab-level bell is no longer needed.
|
||||
//
|
||||
|
|
@ -1185,6 +1226,9 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
|
|||
// before the user ever sees the tab. All current callers only fire for
|
||||
// the active worktree, but this guard prevents future misuse.
|
||||
if (state.activeWorktreeId !== worktreeId) {
|
||||
if (groupAlreadyFocused) {
|
||||
return state
|
||||
}
|
||||
return {
|
||||
activeGroupIdByWorktree: nextActiveGroupIdByWorktree
|
||||
}
|
||||
|
|
@ -1214,8 +1258,23 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
|
|||
return changed ? copy : state.unreadTerminalTabs
|
||||
})()
|
||||
: state.unreadTerminalTabs
|
||||
const activeSurfacePatch = buildActiveSurfacePatch(
|
||||
{
|
||||
...state,
|
||||
activeGroupIdByWorktree: nextActiveGroupIdByWorktree
|
||||
},
|
||||
worktreeId,
|
||||
groupId
|
||||
)
|
||||
if (
|
||||
groupAlreadyFocused &&
|
||||
nextUnreadTerminalTabs === state.unreadTerminalTabs &&
|
||||
activeSurfacePatchMatchesState(state, worktreeId, activeSurfacePatch)
|
||||
) {
|
||||
return state
|
||||
}
|
||||
return {
|
||||
activeGroupIdByWorktree: nextActiveGroupIdByWorktree,
|
||||
...(groupAlreadyFocused ? {} : { activeGroupIdByWorktree: nextActiveGroupIdByWorktree }),
|
||||
// Why: only write unreadTerminalTabs back into state when it actually
|
||||
// changed. The IIFE above returns state.unreadTerminalTabs by reference
|
||||
// on no-op; preserving that reference via conditional spread keeps
|
||||
|
|
@ -1224,14 +1283,7 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
|
|||
...(nextUnreadTerminalTabs !== state.unreadTerminalTabs
|
||||
? { unreadTerminalTabs: nextUnreadTerminalTabs }
|
||||
: {}),
|
||||
...buildActiveSurfacePatch(
|
||||
{
|
||||
...state,
|
||||
activeGroupIdByWorktree: nextActiveGroupIdByWorktree
|
||||
},
|
||||
worktreeId,
|
||||
groupId
|
||||
)
|
||||
...activeSurfacePatch
|
||||
}
|
||||
}),
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,403 @@
|
|||
import { execFileSync } from 'child_process'
|
||||
import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'fs'
|
||||
import os from 'os'
|
||||
import path from 'path'
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { waitForSessionReady } from './helpers/store'
|
||||
|
||||
type CombinedDiffScrollRepo = {
|
||||
repoPath: string
|
||||
}
|
||||
|
||||
type ViewportAnchor = {
|
||||
key: string
|
||||
index: number
|
||||
top: number
|
||||
bottom: number
|
||||
scrollTop: number
|
||||
scrollHeight: number
|
||||
clientHeight: number
|
||||
}
|
||||
|
||||
type ScrollProbeSample = {
|
||||
scrollHeight: number
|
||||
scrollTop: number
|
||||
}
|
||||
|
||||
const FILE_COUNT = 18
|
||||
const ADDED_LINES_PER_FILE = 180
|
||||
|
||||
function runGit(repoPath: string, args: string[]): void {
|
||||
execFileSync('git', args, { cwd: repoPath, stdio: 'pipe' })
|
||||
}
|
||||
|
||||
function buildBaseFile(fileIndex: number): string {
|
||||
return `${Array.from(
|
||||
{ length: 20 },
|
||||
(_, lineIndex) => `export const base_${fileIndex}_${lineIndex} = ${lineIndex}`
|
||||
).join('\n')}\n`
|
||||
}
|
||||
|
||||
function buildModifiedFile(fileIndex: number): string {
|
||||
const added = Array.from(
|
||||
{ length: ADDED_LINES_PER_FILE },
|
||||
(_, lineIndex) => `export const changed_${fileIndex}_${lineIndex} = ${fileIndex + lineIndex}`
|
||||
).join('\n')
|
||||
return `${buildBaseFile(fileIndex)}${added}\n`
|
||||
}
|
||||
|
||||
function createCombinedDiffScrollRepo(): CombinedDiffScrollRepo {
|
||||
const repoPath = realpathSync(mkdtempSync(path.join(os.tmpdir(), 'orca-combined-diff-scroll-')))
|
||||
runGit(repoPath, ['init'])
|
||||
runGit(repoPath, ['config', 'user.email', 'e2e@test.local'])
|
||||
runGit(repoPath, ['config', 'user.name', 'E2E Test'])
|
||||
|
||||
const srcDir = path.join(repoPath, 'src')
|
||||
mkdirSync(srcDir, { recursive: true })
|
||||
for (let index = 0; index < FILE_COUNT; index += 1) {
|
||||
writeFileSync(
|
||||
path.join(srcDir, `scroll-${String(index).padStart(2, '0')}.ts`),
|
||||
buildBaseFile(index)
|
||||
)
|
||||
}
|
||||
runGit(repoPath, ['add', '-A'])
|
||||
runGit(repoPath, ['commit', '-m', 'Initial combined diff scroll fixture'])
|
||||
|
||||
for (let index = 0; index < FILE_COUNT; index += 1) {
|
||||
writeFileSync(
|
||||
path.join(srcDir, `scroll-${String(index).padStart(2, '0')}.ts`),
|
||||
buildModifiedFile(index)
|
||||
)
|
||||
}
|
||||
|
||||
return { repoPath }
|
||||
}
|
||||
|
||||
async function addAndActivateRepo(page: Page, repoPath: string): Promise<string> {
|
||||
const repoId = await page.evaluate(async (pathToRepo: string) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
throw new Error('window.__store is not available')
|
||||
}
|
||||
|
||||
const addedRepo = await store.getState().addRepoPath(pathToRepo)
|
||||
if (!addedRepo) {
|
||||
throw new Error(`isolated combined-diff repo not found: ${pathToRepo}`)
|
||||
}
|
||||
return addedRepo.id
|
||||
}, repoPath)
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
() =>
|
||||
page.evaluate(async (targetRepoId: string) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
return 0
|
||||
}
|
||||
await store.getState().fetchWorktrees(targetRepoId)
|
||||
return store.getState().worktreesByRepo[targetRepoId]?.length ?? 0
|
||||
}, repoId),
|
||||
{
|
||||
timeout: 30_000,
|
||||
message: 'isolated combined-diff worktree did not load'
|
||||
}
|
||||
)
|
||||
.toBeGreaterThan(0)
|
||||
|
||||
return page.evaluate(
|
||||
({ targetRepoId, pathToRepo }) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
throw new Error('window.__store is not available')
|
||||
}
|
||||
|
||||
const state = store.getState()
|
||||
const worktrees = state.worktreesByRepo[targetRepoId] ?? []
|
||||
const worktree = worktrees.find((entry) => entry.path === pathToRepo) ?? worktrees[0]
|
||||
if (!worktree) {
|
||||
throw new Error(`isolated combined-diff worktree not found: ${pathToRepo}`)
|
||||
}
|
||||
state.setActiveRepo(targetRepoId)
|
||||
state.setActiveWorktree(worktree.id)
|
||||
return worktree.id
|
||||
},
|
||||
{ targetRepoId: repoId, pathToRepo: repoPath }
|
||||
)
|
||||
}
|
||||
|
||||
async function openCombinedDiff(page: Page, worktreeId: string, repoPath: string): Promise<string> {
|
||||
return page.evaluate(
|
||||
async ({ wId, pathToRepo }) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
throw new Error('window.__store is not available')
|
||||
}
|
||||
const state = store.getState()
|
||||
const status = await window.api.git.status({ worktreePath: pathToRepo })
|
||||
const entries = status.entries.filter((entry) => entry.area === 'unstaged')
|
||||
if (entries.length < 2) {
|
||||
throw new Error(`expected multiple unstaged entries, received ${entries.length}`)
|
||||
}
|
||||
state.setGitStatus(wId, status)
|
||||
state.openAllDiffs(wId, pathToRepo, undefined, 'unstaged', entries)
|
||||
|
||||
const nextState = store.getState()
|
||||
const activeGroupId = nextState.activeGroupIdByWorktree[wId]
|
||||
const activeFileId = nextState.activeFileId
|
||||
const tab = (nextState.unifiedTabsByWorktree[wId] ?? []).find(
|
||||
(candidate) => candidate.groupId === activeGroupId && candidate.entityId === activeFileId
|
||||
)
|
||||
if (!tab) {
|
||||
throw new Error('combined diff tab was not created')
|
||||
}
|
||||
return tab.id
|
||||
},
|
||||
{ wId: worktreeId, pathToRepo: repoPath }
|
||||
)
|
||||
}
|
||||
|
||||
async function scrollCombinedDiffDeep(page: Page): Promise<void> {
|
||||
await page.evaluate(() => {
|
||||
const container = document.querySelector<HTMLElement>('.combined-diff-scroll-container')
|
||||
if (!container) {
|
||||
throw new Error('combined diff scroll container not found')
|
||||
}
|
||||
const target = Math.min(7_000, Math.max(0, container.scrollHeight - container.clientHeight - 1))
|
||||
container.dispatchEvent(
|
||||
new WheelEvent('wheel', { bubbles: true, cancelable: true, deltaY: target })
|
||||
)
|
||||
container.scrollTop = target
|
||||
container.dispatchEvent(new Event('scroll', { bubbles: true }))
|
||||
})
|
||||
}
|
||||
|
||||
async function readViewportAnchor(page: Page): Promise<ViewportAnchor | null> {
|
||||
return page.evaluate(() => {
|
||||
const container = document.querySelector<HTMLElement>('.combined-diff-scroll-container')
|
||||
if (!container) {
|
||||
return null
|
||||
}
|
||||
const containerRect = container.getBoundingClientRect()
|
||||
const visibleRows = Array.from(
|
||||
container.querySelectorAll<HTMLElement>('[data-combined-diff-section-row]')
|
||||
)
|
||||
.map((row) => {
|
||||
const rect = row.getBoundingClientRect()
|
||||
const key = row.dataset.combinedDiffSectionKey
|
||||
const index = Number(row.dataset.index)
|
||||
if (
|
||||
!key ||
|
||||
!Number.isFinite(index) ||
|
||||
rect.height <= 0 ||
|
||||
rect.bottom <= containerRect.top ||
|
||||
rect.top >= containerRect.bottom
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
key,
|
||||
index,
|
||||
top: rect.top - containerRect.top,
|
||||
bottom: rect.bottom - containerRect.top,
|
||||
scrollTop: container.scrollTop,
|
||||
scrollHeight: container.scrollHeight,
|
||||
clientHeight: container.clientHeight
|
||||
}
|
||||
})
|
||||
.filter((row): row is ViewportAnchor => row !== null)
|
||||
.sort((a, b) => a.top - b.top)
|
||||
return visibleRows[0] ?? null
|
||||
})
|
||||
}
|
||||
|
||||
async function waitForStableViewportAnchor(page: Page): Promise<ViewportAnchor> {
|
||||
const startedAt = Date.now()
|
||||
let lastSignature = ''
|
||||
let stableSamples = 0
|
||||
let lastAnchor: ViewportAnchor | null = null
|
||||
|
||||
while (Date.now() - startedAt < 15_000) {
|
||||
const anchor = await readViewportAnchor(page)
|
||||
if (anchor) {
|
||||
const signature = `${anchor.key}:${Math.round(anchor.top)}:${Math.round(
|
||||
anchor.bottom
|
||||
)}:${Math.round(anchor.scrollHeight)}`
|
||||
if (signature === lastSignature) {
|
||||
stableSamples += 1
|
||||
if (stableSamples >= 2) {
|
||||
return anchor
|
||||
}
|
||||
} else {
|
||||
lastSignature = signature
|
||||
stableSamples = 0
|
||||
}
|
||||
lastAnchor = anchor
|
||||
}
|
||||
await page.waitForTimeout(100)
|
||||
}
|
||||
|
||||
throw new Error(`combined diff viewport anchor did not settle: ${JSON.stringify(lastAnchor)}`)
|
||||
}
|
||||
|
||||
async function startCombinedDiffScrollProbe(page: Page): Promise<void> {
|
||||
await page.evaluate(() => {
|
||||
type CombinedDiffScrollProbe = {
|
||||
samples: ScrollProbeSample[]
|
||||
stop: () => void
|
||||
}
|
||||
const targetWindow = window as typeof window & {
|
||||
__combinedDiffScrollProbe?: CombinedDiffScrollProbe
|
||||
}
|
||||
targetWindow.__combinedDiffScrollProbe?.stop()
|
||||
|
||||
const container = document.querySelector<HTMLElement>('.combined-diff-scroll-container')
|
||||
if (!container) {
|
||||
throw new Error('combined diff scroll container not found')
|
||||
}
|
||||
|
||||
const samples: ScrollProbeSample[] = []
|
||||
const record = (): void => {
|
||||
samples.push({
|
||||
scrollHeight: container.scrollHeight,
|
||||
scrollTop: container.scrollTop
|
||||
})
|
||||
}
|
||||
container.addEventListener('scroll', record, { passive: true })
|
||||
record()
|
||||
targetWindow.__combinedDiffScrollProbe = {
|
||||
samples,
|
||||
stop: () => container.removeEventListener('scroll', record)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
async function stopCombinedDiffScrollProbe(page: Page): Promise<ScrollProbeSample[]> {
|
||||
return page.evaluate(() => {
|
||||
type CombinedDiffScrollProbe = {
|
||||
samples: ScrollProbeSample[]
|
||||
stop: () => void
|
||||
}
|
||||
const targetWindow = window as typeof window & {
|
||||
__combinedDiffScrollProbe?: CombinedDiffScrollProbe
|
||||
}
|
||||
const probe = targetWindow.__combinedDiffScrollProbe
|
||||
if (!probe) {
|
||||
return []
|
||||
}
|
||||
probe.stop()
|
||||
delete targetWindow.__combinedDiffScrollProbe
|
||||
return probe.samples
|
||||
})
|
||||
}
|
||||
|
||||
async function wheelCombinedDiffDown(page: Page): Promise<ScrollProbeSample[]> {
|
||||
const container = page.locator('.combined-diff-scroll-container')
|
||||
const box = await container.boundingBox()
|
||||
if (!box) {
|
||||
throw new Error('combined diff scroll container bounds not found')
|
||||
}
|
||||
|
||||
await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2)
|
||||
await startCombinedDiffScrollProbe(page)
|
||||
for (let index = 0; index < 12; index += 1) {
|
||||
await page.mouse.wheel(0, 520)
|
||||
await page.waitForTimeout(35)
|
||||
}
|
||||
await page.waitForTimeout(400)
|
||||
return stopCombinedDiffScrollProbe(page)
|
||||
}
|
||||
|
||||
function getLargestBackwardScrollJump(samples: readonly ScrollProbeSample[]): number {
|
||||
let largestBackwardJump = 0
|
||||
for (let index = 1; index < samples.length; index += 1) {
|
||||
largestBackwardJump = Math.max(
|
||||
largestBackwardJump,
|
||||
samples[index - 1].scrollTop - samples[index].scrollTop
|
||||
)
|
||||
}
|
||||
return largestBackwardJump
|
||||
}
|
||||
|
||||
async function clickVisibleDiffLine(page: Page): Promise<void> {
|
||||
const linePoint = await page.evaluate(() => {
|
||||
const container = document.querySelector<HTMLElement>('.combined-diff-scroll-container')
|
||||
if (!container) {
|
||||
throw new Error('combined diff scroll container not found')
|
||||
}
|
||||
const containerRect = container.getBoundingClientRect()
|
||||
const visibleLine = Array.from(
|
||||
container.querySelectorAll<HTMLElement>('.monaco-diff-editor .view-line')
|
||||
).find((line) => {
|
||||
const rect = line.getBoundingClientRect()
|
||||
return (
|
||||
rect.height > 0 &&
|
||||
rect.bottom > containerRect.top &&
|
||||
rect.top < containerRect.bottom &&
|
||||
rect.right > containerRect.left &&
|
||||
rect.left < containerRect.right
|
||||
)
|
||||
})
|
||||
if (!visibleLine) {
|
||||
throw new Error('visible combined diff line not found')
|
||||
}
|
||||
const rect = visibleLine.getBoundingClientRect()
|
||||
return {
|
||||
x: rect.left + Math.min(12, Math.max(1, rect.width / 2)),
|
||||
y: rect.top + rect.height / 2
|
||||
}
|
||||
})
|
||||
|
||||
await page.mouse.click(linePoint.x, linePoint.y)
|
||||
}
|
||||
|
||||
test.describe('Combined diff scroll restore', () => {
|
||||
test.describe.configure({ mode: 'serial' })
|
||||
test.use({ seedTestRepo: false })
|
||||
|
||||
test('keeps the visible section anchored after switching tabs', async ({ orcaPage }) => {
|
||||
await waitForSessionReady(orcaPage)
|
||||
const fixture = createCombinedDiffScrollRepo()
|
||||
|
||||
try {
|
||||
const worktreeId = await addAndActivateRepo(orcaPage, fixture.repoPath)
|
||||
const diffTabId = await openCombinedDiff(orcaPage, worktreeId, fixture.repoPath)
|
||||
await expect(orcaPage.locator('.combined-diff-scroll-container')).toBeVisible()
|
||||
await expect(orcaPage.getByText(`${FILE_COUNT} changed files`)).toBeVisible()
|
||||
|
||||
await scrollCombinedDiffDeep(orcaPage)
|
||||
await waitForStableViewportAnchor(orcaPage)
|
||||
const activeScrollSamples = await wheelCombinedDiffDown(orcaPage)
|
||||
expect(activeScrollSamples.length).toBeGreaterThan(2)
|
||||
expect(getLargestBackwardScrollJump(activeScrollSamples)).toBeLessThan(120)
|
||||
|
||||
const beforeSwitch = await waitForStableViewportAnchor(orcaPage)
|
||||
expect(beforeSwitch.index).toBeGreaterThan(0)
|
||||
|
||||
await orcaPage.evaluate((wId) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
throw new Error('window.__store is not available')
|
||||
}
|
||||
store.getState().createTab(wId)
|
||||
}, worktreeId)
|
||||
await expect(orcaPage.locator('.combined-diff-scroll-container')).toHaveCount(0)
|
||||
|
||||
await orcaPage.locator(`[data-tab-id="${diffTabId}"]`).click({ force: true })
|
||||
await expect(orcaPage.locator('.combined-diff-scroll-container')).toBeVisible()
|
||||
const afterSwitch = await waitForStableViewportAnchor(orcaPage)
|
||||
|
||||
expect(afterSwitch.key).toBe(beforeSwitch.key)
|
||||
expect(Math.abs(afterSwitch.top - beforeSwitch.top)).toBeLessThan(80)
|
||||
|
||||
await clickVisibleDiffLine(orcaPage)
|
||||
const afterLineClick = await waitForStableViewportAnchor(orcaPage)
|
||||
|
||||
expect(afterLineClick.key).toBe(afterSwitch.key)
|
||||
expect(Math.abs(afterLineClick.top - afterSwitch.top)).toBeLessThan(80)
|
||||
} finally {
|
||||
rmSync(fixture.repoPath, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
Loading…
Reference in New Issue