From 74b2d5f5d70883b132544bb228b96d10f0bb3981 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:12:31 +0800 Subject: [PATCH] fix(vcs): prevent Git log refresh flicker and graph seams (#269) --- src/features/vcs/GitLog.module.css | 3 +- src/features/vcs/GitLog.tsx | 13 ++-- src/features/vcs/__tests__/multiRoot.test.ts | 75 ++++++++++++++++++++ src/features/vcs/logStore.ts | 42 ++++++----- 4 files changed, 110 insertions(+), 23 deletions(-) diff --git a/src/features/vcs/GitLog.module.css b/src/features/vcs/GitLog.module.css index ddc457e83..088c07f34 100644 --- a/src/features/vcs/GitLog.module.css +++ b/src/features/vcs/GitLog.module.css @@ -173,7 +173,8 @@ display: flex; align-items: center; gap: 6px; - overflow: hidden; + /* Let graph endpoints overlap adjacent rows; text is clipped by .subject. */ + overflow: visible; } .colAuthor { flex: none; diff --git a/src/features/vcs/GitLog.tsx b/src/features/vcs/GitLog.tsx index d934fa1e2..3844cef72 100644 --- a/src/features/vcs/GitLog.tsx +++ b/src/features/vcs/GitLog.tsx @@ -178,7 +178,7 @@ function LogView() { .then((all) => { if (!live) return; const signature = JSON.stringify(all); - if (allRefs.current !== null && allRefs.current !== signature) void reloadLog(); + if (allRefs.current !== null && allRefs.current !== signature) void reloadLog({ preserve: true }); allRefs.current = signature; }) .catch(() => undefined); @@ -215,7 +215,8 @@ function LogView() { const refs = refsSignature(branch, branches) + JSON.stringify(useVcs.getState().heads); const previous = lastRefs.current; lastRefs.current = refs; - if (previous !== null && previous !== refs && useGitLog.getState().commits.length) void reloadLog(); + if (previous !== null && previous !== refs && useGitLog.getState().commits.length) + void reloadLog({ preserve: true }); }, [version]); return ( @@ -742,7 +743,7 @@ function FilterBar() { onClear={() => setLogFilters({ path: null })} /> - void reloadLog()} /> + void reloadLog({ preserve: true })} /> +
{" "} Loading… @@ -1075,7 +1076,7 @@ const CommitRow = memo(function CommitRow({ role="option" aria-selected={selected} className={clsx(styles.row, onBranch && styles.currentBranch, selected && styles.selected)} - style={{ transform: `translateY(${top}px)` }} + style={{ top }} onMouseDown={(e) => { if (e.shiftKey) e.preventDefault(); // no text selection on range clicks onPick(commitKey(c), e); @@ -1132,6 +1133,7 @@ export function GraphCell({ row, head }: { row: GraphRow; head?: boolean }) { key={`u${i}`} d={seg(x(s.from), 0, x(s.to), cy) + (s.arrow ? arrowHead(x(s.from), 0, -1) : "")} stroke={laneColor(s.color)} + strokeLinecap={s.dashed ? "butt" : "square"} strokeDasharray={s.dashed ? "2 2" : undefined} /> ))} @@ -1140,6 +1142,7 @@ export function GraphCell({ row, head }: { row: GraphRow; head?: boolean }) { key={`d${i}`} d={seg(x(s.from), cy, x(s.to), ROW) + (s.arrow ? arrowHead(x(s.to), ROW, 1) : "")} stroke={laneColor(s.color)} + strokeLinecap={s.dashed ? "butt" : "square"} strokeDasharray={s.dashed ? "2 2" : undefined} /> ))} diff --git a/src/features/vcs/__tests__/multiRoot.test.ts b/src/features/vcs/__tests__/multiRoot.test.ts index 9efee34a6..5abd172eb 100644 --- a/src/features/vcs/__tests__/multiRoot.test.ts +++ b/src/features/vcs/__tests__/multiRoot.test.ts @@ -149,3 +149,78 @@ describe("multi-root Git log", () => { } }); }); + +describe("Git log background refresh", () => { + it("keeps history and selection visible until the replacement is ready", async () => { + useVcs.setState({ roots: ["/project/front"] }); + log.mockResolvedValue({ status: "ok", data: [commit("old")] }); + await reloadLog(); + const before = useGitLog.getState(); + selectCommit(commitKey(before.commits[0]!)); + let finish!: (value: unknown) => void; + log.mockImplementation( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + const refresh = reloadLog({ preserve: true }); + await vi.waitFor(() => expect(finish).toBeDefined()); + expect(useGitLog.getState().commits).toBe(before.commits); + expect(useGitLog.getState().rows).toBe(before.rows); + expect(useGitLog.getState().loading).toBe(true); + finish({ status: "ok", data: [commit("new"), commit("old")] }); + await refresh; + expect(useGitLog.getState().commits.map((c) => c.id)).toEqual(["new", "old"]); + expect(useGitLog.getState().rows).toHaveLength(2); + expect(useGitLog.getState().selected).toBe(commitKey(before.commits[0]!)); + expect(useGitLog.getState().loading).toBe(false); + }); + + it("refreshes the loaded range and resumes paging at the replacement's end", async () => { + useVcs.setState({ roots: ["/project/front"] }); + const history = Array.from({ length: PAGE * 3 }, (_, i) => commit(String(i))); + log.mockImplementation(async (_repo, q) => ({ status: "ok", data: history.slice(q.skip, q.skip + q.limit) })); + await reloadLog(); + await loadMore(); + history.unshift(commit("new")); + await reloadLog({ preserve: true }); + expect(useGitLog.getState().commits).toHaveLength(PAGE * 2); + expect(log.mock.lastCall?.[1]).toMatchObject({ skip: 0, limit: PAGE * 2 }); + await loadMore(); + expect(log.mock.lastCall?.[1]).toMatchObject({ skip: PAGE * 2, limit: PAGE }); + expect(useGitLog.getState().commits.map((c) => c.id)).toEqual(history.slice(0, PAGE * 3).map((c) => c.id)); + }); + + it("retains the current history if refreshing fails", async () => { + useVcs.setState({ roots: ["/project/front"] }); + log.mockResolvedValue({ status: "ok", data: [commit("old")] }); + await reloadLog(); + const before = useGitLog.getState(); + log.mockRejectedValue(new Error("unavailable")); + await reloadLog({ preserve: true }); + expect(useGitLog.getState().commits).toBe(before.commits); + expect(useGitLog.getState().rows).toBe(before.rows); + expect(useGitLog.getState().error).toContain("unavailable"); + expect(useGitLog.getState().loading).toBe(false); + }); + + it("ignores a refresh that finishes after the filters have changed", async () => { + useVcs.setState({ roots: ["/project/front"] }); + let finish!: (value: unknown) => void; + log.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + const refresh = reloadLog({ preserve: true }); + await vi.waitFor(() => expect(finish).toBeDefined()); + useGitLog.setState({ filters: { ...emptyFilters(), text: "filtered" } }); + log.mockResolvedValue({ status: "ok", data: [commit("filtered")] }); + await reloadLog(); + finish({ status: "ok", data: [commit("stale")] }); + await refresh; + expect(useGitLog.getState().commits.map((c) => c.id)).toEqual(["filtered"]); + }); +}); diff --git a/src/features/vcs/logStore.ts b/src/features/vcs/logStore.ts index de7395510..8a34fdaa7 100644 --- a/src/features/vcs/logStore.ts +++ b/src/features/vcs/logStore.ts @@ -318,13 +318,11 @@ export function setLogFilters(patch: Partial) { void reloadLog(); } -/** Drops loaded commits and loads the first page. */ -export function reloadLog(): Promise { +/** Reloads history, optionally keeping the current view until its replacement arrives. */ +export function reloadLog({ preserve = false }: { preserve?: boolean } = {}): Promise { useGitLog.setState((s) => ({ - commits: [], - rows: [], - graph: emptyGraphState(), - done: false, + ...(preserve ? {} : { commits: [], rows: [], graph: emptyGraphState(), done: false }), + loading: true, error: null, generation: s.generation + 1, })); @@ -334,7 +332,7 @@ export function reloadLog(): Promise { void currentUser().then((user) => { if (useGitLog.getState().generation === generation) useGitLog.setState({ user }); }); - return loadMore(); + return loadPage(true); } async function loadOutgoing() { @@ -376,16 +374,21 @@ export function isPushed(id: string): boolean { } export function loadMore(): Promise { + return loadPage(false); +} + +function loadPage(replace: boolean): Promise { const s = useGitLog.getState(); - if (s.done) return Promise.resolve(); + if (!replace && s.done) return Promise.resolve(); if (loadPromise) return loadPromise; const gen = s.generation; const f = s.filters; // Settings › Version Control › Limit history to N rows (file / folder history). const vcs = workflowSettings().vcs; const cap = f.path && vcs.limitHistory ? vcs.historyLimit : Infinity; - if (s.commits.length >= cap) { - useGitLog.setState({ done: true }); + const loaded = replace ? 0 : s.commits.length; + if (loaded >= cap) { + useGitLog.setState({ done: true, loading: false }); return Promise.resolve(); } useGitLog.setState({ loading: true }); @@ -402,7 +405,7 @@ export function loadMore(): Promise { : useVcs.getState().roots.length ? useVcs.getState().roots : [currentRepo()]; - const limit = Math.min(PAGE, cap - s.commits.length); + const limit = Math.min(replace ? Math.max(PAGE, s.commits.length) : PAGE, cap - loaded); const refs = await branchesByRepo(roots.filter((r): r is string => !!r)); const knownRefs = new Set( Object.values(refs) @@ -430,7 +433,7 @@ export function loadMore(): Promise { textRegex: f.regex, textMatchCase: f.matchCase, range: null, - skip: s.commits.filter((c) => commitRepo(c) === repo).length, + skip: replace ? 0 : s.commits.filter((c) => commitRepo(c) === repo).length, limit, order: o.sort, firstParent: o.firstParent, @@ -455,16 +458,21 @@ export function loadMore(): Promise { const page = mergeRepoLogs(streams, limit); const cur = useGitLog.getState(); if (cur.generation !== gen) return; - const { rows, state } = computeGraph(page.map(graphCommit), cur.graph); + if (replace && failures.length === results.length && cur.commits.length) { + useGitLog.setState({ error: failures.join("\n") }); + return; + } + const { rows, state } = computeGraph(page.map(graphCommit), replace ? emptyGraphState() : cur.graph); useGitLog.setState({ - commits: cur.commits.concat(page), - rows: cur.rows.concat(rows), + commits: replace ? page : cur.commits.concat(page), + rows: replace ? rows : cur.rows.concat(rows), graph: state, - done: page.length < PAGE || cur.commits.length + page.length >= cap, + done: page.length < limit || loaded + page.length >= cap, error: failures.join("\n") || null, }); } catch (e) { - if (useGitLog.getState().generation === gen) useGitLog.setState({ error: errorMessage(e), done: true }); + if (useGitLog.getState().generation === gen) + useGitLog.setState({ error: errorMessage(e), done: replace && s.commits.length ? s.done : true }); } finally { if (useGitLog.getState().generation === gen) { useGitLog.setState({ loading: false });