дзеркало
https://github.com/l0ng-ai/openidea.git
синхронізовано 2026-11-09 11:15:40 +02:00
fix(vcs): 修复 Git 对比定位与选区异常,优化跳转反馈 (#2093)
Switching the compared file did not select it in the changes tree, a Git status refresh reset the tree's selection and collapsed folders, chunk jumps moved abruptly, and a single click after a jump could select several lines because WebKit scrolls to the old DOM selection on focus. The tree now follows the active diff without taking focus and keeps its state across refreshes. Next and previous chunk scroll smoothly with a short fade on entering a diff, both honouring reduced motion. The caret is placed before focusing on a click. Sibling covered: delayed scroll events from the synchronized pane no longer interrupt a smooth jump. Co-authored-by: PLZY <[email protected]>
Цей коміт міститься в:
1 джерело
9a4ef162bb
коміт
d5e7747803
6 files changed
+341
-14
No files matched your search
@@ -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<string>();
|
||||
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<string | null>(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<string>();
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
@@ -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 ?? [];
|
||||
|
||||
@@ -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<typeof import("@/workbench/contributions")>()),
|
||||
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<typeof import("@/platform/ipc")>();
|
||||
@@ -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(<CommitToolWindow />);
|
||||
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(<CommitToolWindow />);
|
||||
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(<CommitToolWindow />);
|
||||
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(<CommitToolWindow />);
|
||||
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(<CommitToolWindow />);
|
||||
act(() => useVcs.setState({ status: [modified("/p/new.ts")] }));
|
||||
expect(useCommit.getState().selection).toEqual(["/p/new.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();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -165,6 +165,8 @@ export class UnalignedPanes {
|
||||
b: { top: null, left: null },
|
||||
};
|
||||
private sync: boolean;
|
||||
private scrolling: Side | null = null;
|
||||
private scrollTimer: ReturnType<typeof setTimeout> | 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 =
|
||||
|
||||
Reference in new issue
Заблокувати користувача