From f3d4a904b8f8260505f7a7274b86b6d8a1788a1a Mon Sep 17 00:00:00 2001 From: Wooseong Kim Date: Fri, 2 Oct 2026 07:19:01 +0900 Subject: [PATCH 1/2] fix(diff): keep the collapsed section header in view Collapsing the section the viewport is inside left the scroll anchor's within-section offset pointing at a body that no longer exists, so restore landed on later sections. Pin that anchor to the header before the collapse flips. Collapsing or expanding any other section keeps its anchor. Fixes #20665 --- .../combined-diff/CombinedDiffViewer.tsx | 9 +++-- ...mbined-diff-collapse-scroll-anchor.test.ts | 36 +++++++++++++++++++ .../combined-diff-collapse-scroll-anchor.ts | 24 +++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.test.ts create mode 100644 src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.ts diff --git a/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx index 7596e823eac1..00ebe11d4420 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 { pinScrollAnchorWhenCollapsingSection } 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,7 +164,11 @@ export default function CombinedDiffViewer({ const toggleSection = useCallback( (index: number) => { - const shouldLoadAfterExpand = registry.sectionsRef.current[index]?.collapsed ?? false + const section = registry.sectionsRef.current[index] + const shouldLoadAfterExpand = section?.collapsed ?? false + // Why: zero the within-section offset before the collapsed flag flips, so the + // structural restore that follows pins this header instead of a body offset. + pinScrollAnchorWhenCollapsingSection(restore.scrollAnchorRef.current, section) setSections((prev) => prev.map((s, i) => (i === index ? { ...s, collapsed: !s.collapsed } : s)) ) @@ -171,7 +176,7 @@ export default function CombinedDiffViewer({ 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..a5a288ef2bc4 --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest' +import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor' +import { pinScrollAnchorWhenCollapsingSection } 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('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..d42315c8710e --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff/scroll-viewport/combined-diff-collapse-scroll-anchor.ts @@ -0,0 +1,24 @@ +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 +} From c20289d7b255b8278bbd34314dc93835833b2a72 Mon Sep 17 00:00:00 2001 From: Wooseong Kim Date: Fri, 2 Oct 2026 07:51:10 +0900 Subject: [PATCH 2/2] test(diff): pin the collapsed header before the section state updates The viewer asks for that plan before it publishes the collapsed flag, and the regression fails if the header pin happens later. Co-authored-by: Cursor --- .../combined-diff/CombinedDiffViewer.tsx | 18 +++++++-------- ...mbined-diff-collapse-scroll-anchor.test.ts | 23 ++++++++++++++++++- .../combined-diff-collapse-scroll-anchor.ts | 20 ++++++++++++++++ 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx index 00ebe11d4420..4c8640617f3d 100644 --- a/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx @@ -20,7 +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 { pinScrollAnchorWhenCollapsingSection } from './scroll-viewport/combined-diff-collapse-scroll-anchor' +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' @@ -164,15 +164,13 @@ export default function CombinedDiffViewer({ const toggleSection = useCallback( (index: number) => { - const section = registry.sectionsRef.current[index] - const shouldLoadAfterExpand = section?.collapsed ?? false - // Why: zero the within-section offset before the collapsed flag flips, so the - // structural restore that follows pins this header instead of a body offset. - pinScrollAnchorWhenCollapsingSection(restore.scrollAnchorRef.current, section) - 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) } }, 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 index a5a288ef2bc4..df18917b85b4 100644 --- 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 @@ -1,6 +1,9 @@ import { describe, expect, it } from 'vitest' import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor' -import { pinScrollAnchorWhenCollapsingSection } from './combined-diff-collapse-scroll-anchor' +import { + pinScrollAnchorWhenCollapsingSection, + planCombinedDiffSectionToggle +} from './combined-diff-collapse-scroll-anchor' function anchor(offset: number): NonNullable { return { key: 'src/app.ts', offset, scrollTop: 2_000 } @@ -27,6 +30,24 @@ describe('pinScrollAnchorWhenCollapsingSection', () => { 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) 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 index d42315c8710e..2ce4739b2986 100644 --- 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 @@ -22,3 +22,23 @@ export function pinScrollAnchorWhenCollapsingSection( } 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 + ) + } +}