diff --git a/src/features/vcs/CommitView.tsx b/src/features/vcs/CommitView.tsx index e6498a6f8..156c8124b 100644 --- a/src/features/vcs/CommitView.tsx +++ b/src/features/vcs/CommitView.tsx @@ -4,7 +4,16 @@ */ import { useWorkflowSettings } from "@/platform/workflowSettings"; import { formatBinding, isMacHost, macStroke, normalizeBinding, strokeOf } from "@/platform/keymap"; -import { useCallback, useEffect, useMemo, useRef, useState, type MouseEvent, type PointerEvent } from "react"; +import { + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, + type MouseEvent, + type PointerEvent, +} from "react"; import { tx } from "@/platform/i18n"; import clsx from "clsx"; import { executeAction } from "@/platform/actions"; @@ -33,7 +42,7 @@ import { type TreeHandle, T, } from "@/ui"; -import { editorService, useMenuSlotItems } from "@/workbench/contributions"; +import { editorService, maybeEditorService, type EditorInput, useMenuSlotItems } from "@/workbench/contributions"; import { evaluateWhen, useContextKeys } from "@/platform/context"; import { useToolWindowLayout } from "@/workbench/toolwindows/store"; import { @@ -70,7 +79,7 @@ import { import { pushAfterCommit } from "./commitUi"; import { rebaseAfterFixup } from "./rebase"; import { stashTabOf, stashTabs, type ContentTab } from "./gitTabs"; -import { diffInput, openDiff, withChain, workingDiffData } from "./diffInput"; +import { DIFF_KIND, diffInput, openDiff, withChain, workingDiffData } from "./diffInput"; import { openDiffDialog } from "./diffDialog"; import { DiffEditor } from "./DiffEditor"; import { KIND_COLOR, KIND_LABEL, fileCount, formatTime, stashLabel, stripSlash } from "./model"; @@ -527,15 +536,51 @@ export function ChangesTree({ mode }: { mode: "commit" | "local" | "dialog" }) { return [m, up] as const; }, [roots]); - /** Select Opened File: reveals the focused editor's file in the tree. */ + const [nodeToReveal, setNodeToReveal] = useState<{ key: string } | null>(null); + const revealNode = useCallback( + (node: ChangeNode) => { + const ancestors = new Set(); + for (let key = parents.get(node.key); key; key = parents.get(key)) ancestors.add(key); + setCollapsed((current) => new Set([...current].filter((key) => !ancestors.has(key)))); + setNodeToReveal({ key: node.key }); + }, + [parents], + ); + + const followedDiff = useRef(null); + useEffect(() => { + const editor = maybeEditorService(); + const followDiff = (input: EditorInput | null) => { + if (input?.kind !== DIFF_KIND) { + followedDiff.current = null; + return; + } + if (input.key === followedDiff.current) return; + const node = [...byKey.values()].find((node) => { + if (node.type !== "file" || node.path !== input.path) return false; + const side = stagingSide(node.group, node.files[0]!); + return diffInput(workingDiffData(node.path!, undefined, side)).key === input.key; + }); + if (node) { + followedDiff.current = input.key; + revealNode(node); + } else { + followedDiff.current = null; + } + }; + followDiff(editor?.activeInput?.() ?? null); + return editor?.onActiveInputChange?.(followDiff); + }, [byKey, revealNode]); + + // Reveal only after the ancestors have expanded, without moving keyboard focus. + useLayoutEffect(() => { + if (nodeToReveal) treeRef.current?.reveal(nodeToReveal.key); + }, [nodeToReveal]); + const selectOpened = () => { const path = editorService().activePath(); - const node = path ? [...byKey.values()].find((n) => n.type === "file" && n.path === path) : undefined; - if (!node) return; - const ancestors: string[] = []; - for (let k = parents.get(node.key); k; k = parents.get(k)) ancestors.push(k); - setCollapsed((c) => new Set([...c].filter((k) => !ancestors.includes(k)))); - requestAnimationFrame(() => treeRef.current?.reveal(node.key)); + const node = path ? [...byKey.values()].find((node) => node.type === "file" && node.path === path) : undefined; + if (node) revealNode(node); }; const expanded = useMemo(() => { const s = new Set(); diff --git a/src/features/vcs/DiffEditor.module.css b/src/features/vcs/DiffEditor.module.css index cb0ba6729..041e32327 100644 --- a/src/features/vcs/DiffEditor.module.css +++ b/src/features/vcs/DiffEditor.module.css @@ -118,6 +118,9 @@ flex-direction: column; overflow: hidden; } +.host:not([hidden]) { + animation: diff-enter var(--dur) var(--ease) both; +} .host[hidden] { display: none; } @@ -346,3 +349,18 @@ font-size: var(--fs-small); pointer-events: none; } + +@keyframes diff-enter { + from { + opacity: 0; + } + to { + opacity: 1; + } +} + +@media (prefers-reduced-motion: reduce) { + .host:not([hidden]) { + animation: none; + } +} diff --git a/src/features/vcs/DiffEditor.tsx b/src/features/vcs/DiffEditor.tsx index 32a3c9013..2ac253588 100644 --- a/src/features/vcs/DiffEditor.tsx +++ b/src/features/vcs/DiffEditor.tsx @@ -385,6 +385,26 @@ export function DiffEditor({ focus: () => { focusedSide.current = side; }, + mousedown: (event, view) => { + if ( + view.hasFocus || + event.button !== 0 || + event.detail !== 1 || + event.shiftKey || + event.altKey || + event.metaKey || + event.ctrlKey || + !view.contentDOM.contains(event.target as Node) + ) + return false; + const pos = view.posAtCoords({ x: event.clientX, y: event.clientY }); + if (pos === null) return false; + // WebKit may scroll to the old DOM selection on focus. Place the caret first so + // the default mouse handler measures the same position before and after focusing. + view.dispatch({ selection: { anchor: pos }, userEvent: "select.pointer" }); + view.focus(); + return false; + }, }); const collapseUnchanged = opts.collapse ? { margin: 3, minSize: 6 } : undefined; const diffConfig = ideaDiffConfig(opts.ignore, opts.highlight); @@ -649,7 +669,29 @@ export function DiffEditor({ } pendingFile.current = 0; setNavHint(null); - (dir === 1 ? goToNextChunk : goToPreviousChunk)({ state: v.state, dispatch: (tr) => v.dispatch(tr) }); + (dir === 1 ? goToNextChunk : goToPreviousChunk)({ + state: v.state, + dispatch: (tr) => { + v.dispatch({ selection: tr.selection, userEvent: "select.byChunk" }); + const scroller = views.merge && !views.panes ? views.merge.dom : v.scrollDOM; + v.requestMeasure({ + key: step, + read: () => { + const documentTop = v.documentTop - scroller.getBoundingClientRect().top + scroller.scrollTop; + const lineTop = v.lineBlockAt(v.state.selection.main.head).top; + return documentTop + lineTop - scroller.clientHeight / 3; + }, + write: (top) => { + const options: ScrollToOptions = { + top: Math.max(0, top), + behavior: window.matchMedia("(prefers-reduced-motion: reduce)").matches ? "instant" : "smooth", + }; + if (views.panes) views.panes.scrollTo(onA ? "a" : "b", options); + else scroller.scrollTo(options); + }, + }); + }, + }); if (v !== views.b) { // Mirror the caret into the right pane so the counter follows. const chunks = getChunks(v.state)?.chunks ?? []; diff --git a/src/features/vcs/__tests__/commitPreviewDiff.test.tsx b/src/features/vcs/__tests__/commitPreviewDiff.test.tsx index 6820d4573..02f58e848 100644 --- a/src/features/vcs/__tests__/commitPreviewDiff.test.tsx +++ b/src/features/vcs/__tests__/commitPreviewDiff.test.tsx @@ -1,5 +1,28 @@ import { act, fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { EditorInput } from "@/workbench/contributions"; + +const editor = vi.hoisted(() => ({ + input: null as EditorInput | null, + follow: null as ((input: EditorInput | null) => void) | null, +})); +beforeEach(() => { + editor.input = null; + editor.follow = null; +}); + +vi.mock("@/workbench/contributions", async (original) => ({ + ...(await original()), + maybeEditorService: () => ({ + activeInput: () => editor.input, + onActiveInputChange: (follow: typeof editor.follow) => { + editor.follow = follow; + return () => { + editor.follow = null; + }; + }, + }), +})); vi.mock("@/platform/ipc", async (importOriginal) => { const real = await importOriginal(); @@ -20,6 +43,7 @@ vi.mock("../diffInput", async (original) => ({ import { CommitToolWindow } from "../CommitView"; import { setCommitDiffPreview, useCommit } from "../commitStore"; +import { diffInput, workingDiffData } from "../diffInput"; import { useVcs } from "../store"; /** The splitter between the changes and the preview: the only vertical one in the layout. */ @@ -141,3 +165,84 @@ describe("Commit tool window Preview Diff (#1599)", () => { expect(screen.queryByTestId("diff-preview")).toBeNull(); }); }); + +it("follows the active working diff without following other revisions or taking focus", () => { + useCommit.setState({ commitDiffPreview: false, selection: [] }); + useVcs.setState({ + available: true, + root: "/p", + repo: "/p", + roots: ["/p"], + status: [modified("/p/a.ts"), modified("/p/b.ts")], + }); + render(); + const message = screen.getByPlaceholderText("Commit Message"); + message.focus(); + act(() => editor.follow?.(diffInput(workingDiffData("/p/b.ts")))); + expect(useCommit.getState().selection).toEqual(["/p/b.ts"]); + expect(screen.getByRole("treeitem", { name: /b.ts/ })).toHaveAttribute("aria-selected", "true"); + expect(document.activeElement).toBe(message); + act(() => + editor.follow?.( + diffInput({ path: "/p/a.ts", left: { type: "rev", rev: "old" }, right: { type: "rev", rev: "new" } }), + ), + ); + expect(useCommit.getState().selection).toEqual(["/p/b.ts"]); +}); + +describe("following the active diff across Git status refreshes", () => { + beforeEach(() => { + editor.input = diffInput(workingDiffData("/p/b.ts")); + useCommit.setState({ commitDiffPreview: false, selection: [], groupByDir: false, groupByStaging: false }); + useVcs.setState({ + available: true, + root: "/p", + repo: "/p", + roots: ["/p"], + status: [modified("/p/a.ts"), modified("/p/b.ts")], + }); + }); + + it("keeps a manually selected file when status refreshes with the same active diff", () => { + render(); + expect(useCommit.getState().selection).toEqual(["/p/b.ts"]); + selectFirstFile(); + expect(useCommit.getState().selection).toEqual(["/p/a.ts"]); + act(() => useVcs.setState({ status: [modified("/p/a.ts"), modified("/p/b.ts")] })); + expect(useCommit.getState().selection).toEqual(["/p/a.ts"]); + act(() => editor.follow?.(editor.input)); + expect(useCommit.getState().selection).toEqual(["/p/a.ts"]); + }); + + it("keeps a manually collapsed group across status refreshes", () => { + render(); + const tree = screen.getByRole("tree", { name: "Changes" }); + tree.focus(); + fireEvent.keyDown(tree, { key: "Home" }); + fireEvent.keyDown(tree, { key: "ArrowLeft" }); + expect(tree.querySelector('[role="treeitem"][aria-expanded="false"]')).toBeTruthy(); + expect(screen.queryByRole("treeitem", { name: /b.ts/ })).toBeNull(); + act(() => useVcs.setState({ status: [modified("/p/a.ts"), modified("/p/b.ts")] })); + expect(screen.queryByRole("treeitem", { name: /b.ts/ })).toBeNull(); + }); + + it.each([ + null, + diffInput({ path: "/p/b.ts", left: { type: "rev", rev: "old" }, right: { type: "rev", rev: "new" } }), + ])("follows the same diff again after switching away from it (%#)", (other) => { + render(); + selectFirstFile(); + act(() => editor.follow?.(other)); + act(() => editor.follow?.(editor.input)); + expect(useCommit.getState().selection).toEqual(["/p/b.ts"]); + }); +}); + +it("follows a newly opened diff when its Git status arrives later", () => { + editor.input = diffInput(workingDiffData("/p/new.ts")); + useCommit.setState({ commitDiffPreview: false, selection: [] }); + useVcs.setState({ available: true, root: "/p", repo: "/p", roots: ["/p"], status: [] }); + render(); + act(() => useVcs.setState({ status: [modified("/p/new.ts")] })); + expect(useCommit.getState().selection).toEqual(["/p/new.ts"]); +}); diff --git a/src/features/vcs/__tests__/diffScroll.test.ts b/src/features/vcs/__tests__/diffScroll.test.ts index 1668509f8..30bf85a9a 100644 --- a/src/features/vcs/__tests__/diffScroll.test.ts +++ b/src/features/vcs/__tests__/diffScroll.test.ts @@ -1,7 +1,8 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { Chunk, MergeView } from "@codemirror/merge"; import { Text } from "@codemirror/state"; -import { bandAt, bandPath, transferY, type Band } from "../diffScroll"; +import type { EditorView } from "@codemirror/view"; +import { bandAt, bandPath, transferY, UnalignedPanes, type Band } from "../diffScroll"; const doc = (s: string) => Text.of(s.split("\n")); const LINE = 20; @@ -95,3 +96,78 @@ describe("unaligned side-by-side scrolling", () => { expect(typeof (MergeView.prototype as unknown as { measure: unknown }).measure).toBe("function"); }); }); + +describe("animated synchronized scrolling", () => { + function panes(sync = true) { + const root = document.createElement("div"); + const text = doc(Array.from({ length: 200 }, () => "line").join("\n")); + const view = () => { + const wrapper = root.appendChild(document.createElement("div")); + const dom = wrapper.appendChild(document.createElement("div")); + const scrollDOM = dom.appendChild(document.createElement("div")); + Object.defineProperty(scrollDOM, "scrollHeight", { value: 4000 }); + scrollDOM.scrollTo = vi.fn(); + return { + dom, + scrollDOM, + state: { doc: text }, + get documentTop() { + return -scrollDOM.scrollTop; + }, + lineBlockAt: (pos: number) => ({ bottom: text.lineAt(pos).number * LINE }), + } as unknown as EditorView; + }; + const a = view(); + const b = view(); + const controller = new UnalignedPanes({ sync }); + controller.attach({ a, b, chunks: [] } as unknown as MergeView); + const scroll = (view: EditorView, top: number) => { + view.scrollDOM.scrollTop = top; + view.scrollDOM.dispatchEvent(new Event("scroll")); + }; + return { a, b, controller, scroll }; + } + + it.each(["a", "b"] as const)("ignores delayed follower layout scrolls while %s animates", (side) => { + vi.useFakeTimers(); + const frame = vi.spyOn(window, "requestAnimationFrame").mockReturnValue(1); + const p = panes(); + try { + const leader = p[side]; + const follower = p[side === "a" ? "b" : "a"]; + p.controller.scrollTo(side, { top: 2000, behavior: "smooth" }); + p.scroll(leader, 1000); + expect(follower.scrollDOM.scrollTop).toBe(1000); + p.scroll(follower, 200); + expect(leader.scrollDOM.scrollTop).toBe(1000); + expect(follower.scrollDOM.scrollTop).toBe(1000); + p.scroll(leader, 2000); + expect(follower.scrollDOM.scrollTop).toBe(2000); + vi.advanceTimersByTime(150); + p.scroll(follower, 300); + expect(leader.scrollDOM.scrollTop).toBe(300); + } finally { + p.controller.destroy(); + frame.mockRestore(); + vi.useRealTimers(); + } + }); + + it.each(["wheel", "pointerdown"])("lets %s interrupt an animated jump", (event) => { + vi.useFakeTimers(); + const frame = vi.spyOn(window, "requestAnimationFrame").mockReturnValue(1); + const p = panes(); + try { + p.controller.scrollTo("a", { top: 2000, behavior: "smooth" }); + p.scroll(p.a, 1000); + p.b.scrollDOM.dispatchEvent(new Event(event)); + expect(p.a.scrollDOM.scrollTo).toHaveBeenLastCalledWith({ top: 1000, behavior: "instant" }); + p.scroll(p.b, 400); + expect(p.a.scrollDOM.scrollTop).toBe(400); + } finally { + p.controller.destroy(); + frame.mockRestore(); + vi.useRealTimers(); + } + }); +}); diff --git a/src/features/vcs/diffScroll.ts b/src/features/vcs/diffScroll.ts index 476663016..7afcf536b 100644 --- a/src/features/vcs/diffScroll.ts +++ b/src/features/vcs/diffScroll.ts @@ -165,6 +165,8 @@ export class UnalignedPanes { b: { top: null, left: null }, }; private sync: boolean; + private scrolling: Side | null = null; + private scrollTimer: ReturnType | undefined; private frame = -1; private resize: ResizeObserver | null = null; private readonly cleanup: (() => void)[] = []; @@ -187,12 +189,20 @@ export class UnalignedPanes { for (const side of ["a", "b"] as const) { const el = this.view(side).scrollDOM; const onScroll = () => this.scrolled(side); + const onPointer = () => this.interruptScroll(); el.addEventListener("scroll", onScroll, { passive: true }); - this.cleanup.push(() => el.removeEventListener("scroll", onScroll)); + el.addEventListener("wheel", onPointer, { passive: true }); + el.addEventListener("pointerdown", onPointer); + this.cleanup.push(() => { + el.removeEventListener("scroll", onScroll); + el.removeEventListener("wheel", onPointer); + el.removeEventListener("pointerdown", onPointer); + }); } // The divider scrolls the right pane (and, when synchronized, through it the left one). const onWheel = (e: WheelEvent) => { e.preventDefault(); + this.interruptScroll(); mv.b.scrollDOM.scrollBy(e.deltaX, e.deltaY); }; this.divider.addEventListener("wheel", onWheel, { passive: false }); @@ -213,11 +223,35 @@ export class UnalignedPanes { /** Turns synchronized scrolling on (bringing the left pane in line) or off. */ setSync(on: boolean) { if (on === this.sync) return; + this.interruptScroll(); this.sync = on; this.resync(); } + scrollTo(side: Side, options: ScrollToOptions) { + this.interruptScroll(); + this.scrolling = this.sync && options.behavior === "smooth" ? side : null; + this.releaseScroll(); + this.view(side).scrollDOM.scrollTo(options); + } + + private releaseScroll() { + clearTimeout(this.scrollTimer); + if (this.scrolling) this.scrollTimer = setTimeout(() => (this.scrolling = null), 150); + } + + private interruptScroll() { + const side = this.scrolling; + this.scrolling = null; + clearTimeout(this.scrollTimer); + if (side && this.mv) { + const el = this.view(side).scrollDOM; + el.scrollTo({ top: el.scrollTop, behavior: "instant" }); + } + } + destroy() { + clearTimeout(this.scrollTimer); if (this.frame >= 0) cancelAnimationFrame(this.frame); this.resize?.disconnect(); for (const f of this.cleanup) f(); @@ -230,6 +264,13 @@ export class UnalignedPanes { } private scrolled(side: Side) { + if (this.scrolling) { + // A follower's delayed layout scroll must not cancel the leader's animation. + this.follow(this.scrolling); + this.releaseScroll(); + this.schedule(); + return; + } const el = this.view(side).scrollDOM; const moved = this.moved[side]; const echo =