diff --git a/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx index 7596e823eac1..4c8640617f3d 100644 --- a/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx @@ -20,6 +20,7 @@ import { useCombinedDiffSectionRevalidation } from './load-sections/use-combined import { useCombinedDiffViewPersist } from './remember-view/use-combined-diff-view-persist' import { useCombinedDiffViewRestore } from './remember-view/use-combined-diff-view-restore' import { useCombinedDiffDirectScrollInput } from './scroll-viewport/use-combined-diff-direct-scroll-input' +import { planCombinedDiffSectionToggle } from './scroll-viewport/combined-diff-collapse-scroll-anchor' import { useCombinedDiffScrollAnchors } from './scroll-viewport/use-combined-diff-scroll-anchors' import { useCombinedDiffScrollPersistence } from './scroll-viewport/use-combined-diff-scroll-persistence' import { useCombinedDiffScrollbar } from './scroll-viewport/use-combined-diff-scrollbar' @@ -163,15 +164,17 @@ export default function CombinedDiffViewer({ const toggleSection = useCallback( (index: number) => { - const shouldLoadAfterExpand = registry.sectionsRef.current[index]?.collapsed ?? false - setSections((prev) => - prev.map((s, i) => (i === index ? { ...s, collapsed: !s.collapsed } : s)) - ) - if (shouldLoadAfterExpand) { + const plan = planCombinedDiffSectionToggle({ + index, + sections: registry.sectionsRef.current, + anchor: restore.scrollAnchorRef.current + }) + setSections((prev) => plan.next(prev)) + if (plan.shouldLoadAfterExpand) { registry.loadSchedulerRef.current.request(index) } }, - [registry.loadSchedulerRef, registry.sectionsRef] + [registry.loadSchedulerRef, registry.sectionsRef, restore.scrollAnchorRef] ) const treeNavigation = useCombinedDiffTreeNavigation({ diff --git a/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.test.ts b/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.test.ts new file mode 100644 index 000000000000..df18917b85b4 --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor' +import { + pinScrollAnchorWhenCollapsingSection, + planCombinedDiffSectionToggle +} from './combined-diff-collapse-scroll-anchor' + +function anchor(offset: number): NonNullable { + return { key: 'src/app.ts', offset, scrollTop: 2_000 } +} + +describe('pinScrollAnchorWhenCollapsingSection', () => { + it('pins the header when the anchored section is the one being collapsed', () => { + const current = anchor(1_800) + pinScrollAnchorWhenCollapsingSection(current, { collapsed: false, key: 'src/app.ts' }) + expect(current.offset).toBe(0) + expect(current.scrollTop).toBe(2_000) + expect(current.key).toBe('src/app.ts') + }) + + it('leaves the anchor when that section is expanding', () => { + const current = anchor(0) + pinScrollAnchorWhenCollapsingSection(current, { collapsed: true, key: 'src/app.ts' }) + expect(current.offset).toBe(0) + }) + + it('leaves the anchor when a different section collapses', () => { + const current = anchor(400) + pinScrollAnchorWhenCollapsingSection(current, { collapsed: false, key: 'src/other.ts' }) + expect(current.offset).toBe(400) + }) + + it('pins a mid-section anchor before the collapsed flag is published', () => { + const current = anchor(1_800) + const open = { collapsed: false, key: 'src/app.ts', marker: 'body' } + const plan = planCombinedDiffSectionToggle({ + index: 0, + sections: [open], + anchor: current + }) + + expect(current.offset).toBe(0) + expect(current.scrollTop).toBe(2_000) + expect(plan.shouldLoadAfterExpand).toBe(false) + + const published = plan.next([open]) + expect(published[0]?.collapsed).toBe(true) + expect(published[0]?.marker).toBe('body') + }) + + it('does nothing without an anchor or a section', () => { + expect(() => pinScrollAnchorWhenCollapsingSection(null, undefined)).not.toThrow() + const current = anchor(40) + pinScrollAnchorWhenCollapsingSection(current, undefined) + expect(current.offset).toBe(40) + }) +}) diff --git a/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.ts b/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.ts new file mode 100644 index 000000000000..2ce4739b2986 --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.ts @@ -0,0 +1,44 @@ +import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor' + +type CollapsibleSection = { + collapsed: boolean + key: string +} + +/** + * Collapsing the section the viewport is inside drops that section's body. + * The recorded offset still points into the body, so restore lands on later + * sections. Pin the header (offset 0) and leave scrollTop alone: restore + * treats a matching scrollTop as "the user has not moved" and then applies + * the offset. Expanding, or collapsing some other section, keeps the anchor + * so a later section stays where it was. + */ +export function pinScrollAnchorWhenCollapsingSection( + anchor: VirtualizedScrollAnchor, + section: CollapsibleSection | undefined +): void { + if (!anchor || !section || section.collapsed || anchor.key !== section.key) { + return + } + anchor.offset = 0 +} + +export function planCombinedDiffSectionToggle(input: { + index: number + sections: readonly T[] + anchor: VirtualizedScrollAnchor +}): { + next: (sections: readonly T[]) => T[] + shouldLoadAfterExpand: boolean +} { + const section = input.sections[input.index] + const shouldLoadAfterExpand = section?.collapsed ?? false + pinScrollAnchorWhenCollapsingSection(input.anchor, section) + return { + shouldLoadAfterExpand, + next: (sections) => + sections.map((row, index) => + index === input.index ? { ...row, collapsed: !row.collapsed } : row + ) + } +}