mirror of
https://github.com/l0ng-ai/openidea.git
synced 2026-11-08 08:52:12 +00:00
fix(vcs): keep Git Log rows painted while dragging the scrollbar (#1914)
Dragging the Git Log scrollbar through a long history stuttered and showed blank rows: the native scrollbar scrolls asynchronously in WebKit, so the virtual list rendered rows a frame or more after the new offset was painted. The list now uses a scrollbar drawn in the DOM (`Scrollbar` in `src/ui`) that sets the offset from pointer events and notifies scroll listeners at once, so the virtualizer renders the matching rows before the next paint. Rows are keyed by slot so a large jump updates the mounted rows instead of rebuilding them, overscan is lowered, and rows rendered mid-scroll draw their graph without edge hit targets, which come back once scrolling stops. Adds a `gitLogScrollbar` perf scenario with budgets.
This commit is contained in:
1 parent
159c2d6fd2
commit
b99fe1704b
8 files changed
+414
-46
No files matched your search
+73
-46
@@ -6,6 +6,7 @@ import { branchesByRepo, commitKey, commitRepo, graphCommit, type RepoCommit } f
|
||||
import { IssueText } from "./issueLinks";
|
||||
import { BranchFilterPopup } from "./BranchFilterPopup";
|
||||
import {
|
||||
Fragment,
|
||||
memo,
|
||||
useCallback,
|
||||
useContext,
|
||||
@@ -43,7 +44,9 @@ import {
|
||||
type TreeHandle,
|
||||
T,
|
||||
copyText,
|
||||
hideNativeScrollbar,
|
||||
lazyComponent,
|
||||
Scrollbar,
|
||||
} from "@/ui";
|
||||
import { hasPrimaryModifier } from "@/platform/keymap";
|
||||
import { laneColor, reachableFrom, type GraphRow, type GraphSegment } from "./graph";
|
||||
@@ -1398,7 +1401,7 @@ function CommitList() {
|
||||
count: commits.length + (finished ? 0 : 1),
|
||||
getScrollElement: () => scrollRef.current,
|
||||
estimateSize: () => ROW,
|
||||
overscan: 20,
|
||||
overscan: 8,
|
||||
scrollMargin: ROW,
|
||||
});
|
||||
const items = virtualizer.getVirtualItems();
|
||||
@@ -1410,7 +1413,10 @@ function CommitList() {
|
||||
if (!finished && !loading && last && last.index >= commits.length - PAGE) void loadMore(scope ?? undefined);
|
||||
}, [last, commits.length, finished, loading, active, scope]);
|
||||
|
||||
const selectedIndex = selected ? commits.findIndex((c) => commitKey(c) === selected) : -1;
|
||||
const selectedIndex = useMemo(
|
||||
() => (selected ? commits.findIndex((c) => commitKey(c) === selected) : -1),
|
||||
[commits, selected],
|
||||
);
|
||||
useLayoutEffect(() => {
|
||||
if (selectedIndex >= 0) virtualizer.scrollToIndex(selectedIndex, { align: "center" });
|
||||
// Only on explicit reveal requests.
|
||||
@@ -1669,9 +1675,10 @@ function CommitList() {
|
||||
{error}
|
||||
</div>
|
||||
)}
|
||||
<Scrollbar target={scrollRef} contentHeight={virtualizer.getTotalSize()} />
|
||||
<div
|
||||
ref={scrollRef}
|
||||
className={styles.list}
|
||||
className={clsx(styles.list, hideNativeScrollbar)}
|
||||
role="listbox"
|
||||
aria-label={tx("Commits")}
|
||||
aria-multiselectable
|
||||
@@ -1730,7 +1737,9 @@ function CommitList() {
|
||||
);
|
||||
return (
|
||||
<CommitRow
|
||||
key={commitKey(c)}
|
||||
// Keyed by slot, not commit: a scrollbar drag that jumps past every mounted
|
||||
// row then updates the same DOM nodes instead of rebuilding them.
|
||||
key={v.index % items.length}
|
||||
id={`commit-${scope ?? "log"}-${v.index}`}
|
||||
commit={c}
|
||||
graph={rows[v.index]}
|
||||
@@ -1751,6 +1760,7 @@ function CommitList() {
|
||||
targetTitle={targetTitle}
|
||||
columns={columns}
|
||||
bookmarked={!!bookmarks[commitKey(c)]}
|
||||
interactive={!virtualizer.isScrolling}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
@@ -1876,7 +1886,7 @@ function rowRemoteNames(own: readonly string[] | undefined, shared: ReadonlySet<
|
||||
return set;
|
||||
}
|
||||
|
||||
const CommitRow = memo(function CommitRow({
|
||||
function CommitRowView({
|
||||
id,
|
||||
commit: c,
|
||||
graph,
|
||||
@@ -1897,6 +1907,7 @@ const CommitRow = memo(function CommitRow({
|
||||
targetTitle,
|
||||
columns,
|
||||
bookmarked,
|
||||
interactive,
|
||||
}: {
|
||||
id: string;
|
||||
commit: Commit;
|
||||
@@ -1918,6 +1929,8 @@ const CommitRow = memo(function CommitRow({
|
||||
targetTitle: (index: number, key?: string) => string;
|
||||
columns: LogColumn[];
|
||||
bookmarked: boolean;
|
||||
/** False for rows rendered mid-scroll: their graph is drawn without hit targets. */
|
||||
interactive: boolean;
|
||||
}) {
|
||||
const merge = opts.highlightMerges && c.parents.length > 1;
|
||||
const subjectRef = useRef<HTMLSpanElement>(null);
|
||||
@@ -1981,6 +1994,7 @@ const CommitRow = memo(function CommitRow({
|
||||
targetTitle={targetTitle}
|
||||
onFragment={(target, expand, source) => onFragment(source ?? commitKey(c), target, expand)}
|
||||
onNode={() => onFragment(commitKey(c), commitKey(c), false)}
|
||||
interactive={interactive}
|
||||
/>
|
||||
)}
|
||||
{bookmarked && <Icon name="bookmark" size={12} />}
|
||||
@@ -2029,6 +2043,16 @@ const CommitRow = memo(function CommitRow({
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// A row that is already interactive stays so when scrolling starts; only rows
|
||||
// whose content changes mid-scroll are drawn without hit targets.
|
||||
const CommitRow = memo(CommitRowView, (prev, next) => {
|
||||
for (const key in next) {
|
||||
const k = key as keyof typeof next;
|
||||
if (prev[k] !== next[k] && !(k === "interactive" && prev.interactive)) return false;
|
||||
}
|
||||
return true;
|
||||
});
|
||||
|
||||
function x(lane: number) {
|
||||
@@ -2051,6 +2075,7 @@ export function GraphCell({
|
||||
onFragment,
|
||||
targetTitle,
|
||||
onNode,
|
||||
interactive = true,
|
||||
}: {
|
||||
row: GraphRow;
|
||||
head?: boolean;
|
||||
@@ -2062,9 +2087,27 @@ export function GraphCell({
|
||||
onFragment?: (target: string, expand: boolean, source?: string) => void;
|
||||
targetTitle?: (index: number, key?: string) => string;
|
||||
onNode?: () => void;
|
||||
interactive?: boolean;
|
||||
}) {
|
||||
const cy = ROW / 2;
|
||||
const draw = (s: GraphSegment, i: number, up: boolean) => {
|
||||
const fromX = x(s.fromX ?? s.from);
|
||||
const toX = x(s.toX ?? s.to);
|
||||
const path =
|
||||
seg(fromX, up ? 0 : cy, toX, up ? cy : ROW) +
|
||||
(s.arrow ? arrowHead(up ? fromX : toX, up ? 0 : ROW, up ? -1 : 1) : "");
|
||||
const line = (
|
||||
<path
|
||||
key={interactive ? undefined : `${up ? "u" : "d"}${i}`}
|
||||
d={path}
|
||||
stroke={laneColor(s.color)}
|
||||
strokeWidth={s.edge != null && (activeColor === s.edge || highlighted?.has(s.edge)) ? 2.8 : 1.4}
|
||||
pointerEvents="none"
|
||||
strokeLinecap={s.dashed ? "butt" : "square"}
|
||||
strokeDasharray={s.dashed ? "2 2" : undefined}
|
||||
/>
|
||||
);
|
||||
if (!interactive) return line;
|
||||
const target = (up ? row.upTargets : row.downTargets)?.[i];
|
||||
const canFragment =
|
||||
!!target &&
|
||||
@@ -2086,50 +2129,34 @@ export function GraphCell({
|
||||
if (canNavigate) onNavigate?.(s.targetRow!, target);
|
||||
else if (canFragment) onFragment?.(target!, !!s.dashed, s.source);
|
||||
};
|
||||
const fromX = x(s.fromX ?? s.from);
|
||||
const toX = x(s.toX ?? s.to);
|
||||
const path =
|
||||
seg(fromX, up ? 0 : cy, toX, up ? cy : ROW) +
|
||||
(s.arrow ? arrowHead(up ? fromX : toX, up ? 0 : ROW, up ? -1 : 1) : "");
|
||||
return (
|
||||
<g
|
||||
key={`${up ? "u" : "d"}${i}`}
|
||||
role={actionable ? "button" : undefined}
|
||||
tabIndex={actionable ? 0 : undefined}
|
||||
aria-label={actionable ? label : undefined}
|
||||
onMouseEnter={() => onHover?.(s.edge ?? `${s.source ?? ""}\0${target ?? ""}`)}
|
||||
onMouseDown={(e) => e.stopPropagation()}
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
activate();
|
||||
}}
|
||||
onKeyDown={(e) => {
|
||||
if (actionable && (e.key === "Enter" || e.key === " ")) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
activate();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<title>{label}</title>
|
||||
<rect
|
||||
x={Math.min(fromX, toX) - 5}
|
||||
y={up ? 0 : cy}
|
||||
width={Math.max(10, Math.abs(fromX - toX) + 10)}
|
||||
height={cy}
|
||||
fill="transparent"
|
||||
pointerEvents="none"
|
||||
/>
|
||||
<path d={path} stroke="transparent" strokeWidth={10} style={{ pointerEvents: "stroke" }} />
|
||||
<Fragment key={`${up ? "u" : "d"}${i}`}>
|
||||
<path
|
||||
d={path}
|
||||
stroke={laneColor(s.color)}
|
||||
strokeWidth={s.edge != null && (activeColor === s.edge || highlighted?.has(s.edge)) ? 2.8 : 1.4}
|
||||
pointerEvents="none"
|
||||
strokeLinecap={s.dashed ? "butt" : "square"}
|
||||
strokeDasharray={s.dashed ? "2 2" : undefined}
|
||||
/>
|
||||
</g>
|
||||
stroke="transparent"
|
||||
strokeWidth={10}
|
||||
style={{ pointerEvents: "stroke" }}
|
||||
role={actionable ? "button" : undefined}
|
||||
tabIndex={actionable ? 0 : undefined}
|
||||
aria-label={actionable ? label : undefined}
|
||||
onMouseEnter={() => onHover?.(s.edge ?? `${s.source ?? ""}\0${target ?? ""}`)}
|
||||
onMouseDown={(e) => e.stopPropagation()}
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
activate();
|
||||
}}
|
||||
onKeyDown={(e) => {
|
||||
if (actionable && (e.key === "Enter" || e.key === " ")) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
activate();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<title>{label}</title>
|
||||
</path>
|
||||
{line}
|
||||
</Fragment>
|
||||
);
|
||||
};
|
||||
return (
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
/* Same look as the global ::-webkit-scrollbar (global.css). */
|
||||
.track {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
right: 0;
|
||||
width: 10px;
|
||||
z-index: 1;
|
||||
}
|
||||
.thumb {
|
||||
position: absolute;
|
||||
left: 0;
|
||||
right: 0;
|
||||
top: 0;
|
||||
border: 2px solid transparent;
|
||||
border-radius: 6px;
|
||||
background-clip: content-box;
|
||||
background-color: var(--scrollbar-thumb);
|
||||
will-change: transform;
|
||||
}
|
||||
.thumb:hover,
|
||||
.thumb[data-dragging] {
|
||||
background-color: var(--scrollbar-thumb-hover);
|
||||
}
|
||||
/* For the scroller whose vertical scrollbar `Scrollbar` draws. */
|
||||
.hideNative::-webkit-scrollbar:vertical {
|
||||
width: 0;
|
||||
}
|
||||
@@ -0,0 +1,46 @@
|
||||
import { act, render } from "@testing-library/react";
|
||||
import { useRef } from "react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { Scrollbar } from "./Scrollbar";
|
||||
|
||||
function Harness({ onScroll }: { onScroll: (top: number) => void }) {
|
||||
const ref = useRef<HTMLDivElement>(null);
|
||||
return (
|
||||
<div style={{ position: "relative" }}>
|
||||
<Scrollbar target={ref} contentHeight={1000} />
|
||||
<div ref={ref} data-testid="scroller" onScroll={(e) => onScroll(e.currentTarget.scrollTop)} />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
describe("Scrollbar", () => {
|
||||
it("notifies scroll listeners synchronously while the thumb is dragged", () => {
|
||||
const onScroll = vi.fn();
|
||||
const { getByTestId, container } = render(<Harness onScroll={onScroll} />);
|
||||
const scroller = getByTestId("scroller");
|
||||
let top = 0;
|
||||
Object.defineProperties(scroller, {
|
||||
clientHeight: { value: 100 },
|
||||
scrollHeight: { value: 1000 },
|
||||
scrollTop: { get: () => top, set: (v: number) => (top = v) },
|
||||
});
|
||||
scroller.scrollTo = ((options: ScrollToOptions) => {
|
||||
top = Math.max(0, Math.min(900, options.top ?? top));
|
||||
}) as typeof scroller.scrollTo;
|
||||
act(() => {
|
||||
scroller.dispatchEvent(new Event("scroll"));
|
||||
});
|
||||
onScroll.mockClear();
|
||||
|
||||
const track = container.querySelector<HTMLElement>("[aria-hidden]")!;
|
||||
const thumb = track.firstElementChild as HTMLElement;
|
||||
track.setPointerCapture = () => {};
|
||||
thumb.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, button: 0, clientY: 10 }));
|
||||
track.dispatchEvent(new PointerEvent("pointermove", { bubbles: true, clientY: 46 }));
|
||||
|
||||
// A 32px thumb travels 68px of the track for 900px of content: 36px ≈ 476px.
|
||||
expect(top).toBeCloseTo((36 / 68) * 900);
|
||||
expect(onScroll).toHaveBeenCalledTimes(1);
|
||||
expect(onScroll).toHaveBeenLastCalledWith(top);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,111 @@
|
||||
import { useEffect, useLayoutEffect, useRef, type RefObject } from "react";
|
||||
import styles from "./Scrollbar.module.css";
|
||||
|
||||
const MIN_THUMB = 32;
|
||||
|
||||
/** Class for the scroller: hides its native vertical scrollbar, which `Scrollbar` replaces. */
|
||||
export const hideNativeScrollbar = styles.hideNative;
|
||||
|
||||
/**
|
||||
* A vertical scrollbar drawn in the DOM over `target` (place both in a
|
||||
* `position: relative` box). Dragging it sets `scrollTop` from pointer events,
|
||||
* so content rendered on scroll (virtual lists) lands in the same frame. The
|
||||
* native one scrolls asynchronously in WebKit and shows blank rows until the
|
||||
* page catches up.
|
||||
*
|
||||
* `contentHeight` re-measures when the scroller's content grows.
|
||||
*/
|
||||
export function Scrollbar({
|
||||
target,
|
||||
contentHeight,
|
||||
}: {
|
||||
target: RefObject<HTMLElement | null>;
|
||||
contentHeight?: number;
|
||||
}) {
|
||||
const track = useRef<HTMLDivElement>(null);
|
||||
const thumb = useRef<HTMLDivElement>(null);
|
||||
const drag = useRef<{ y: number; top: number } | null>(null);
|
||||
const geometry = useRef<Geometry | null>(null);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
geometry.current = layout(target.current, track.current, thumb.current);
|
||||
}, [target, contentHeight]);
|
||||
useEffect(() => {
|
||||
const el = target.current;
|
||||
if (!el) return;
|
||||
const sync = () => {
|
||||
geometry.current = layout(el, track.current, thumb.current);
|
||||
};
|
||||
el.addEventListener("scroll", sync, { passive: true });
|
||||
const observer = new ResizeObserver(sync);
|
||||
observer.observe(el);
|
||||
return () => {
|
||||
el.removeEventListener("scroll", sync);
|
||||
observer.disconnect();
|
||||
};
|
||||
}, [target]);
|
||||
|
||||
return (
|
||||
<div
|
||||
ref={track}
|
||||
className={styles.track}
|
||||
aria-hidden
|
||||
onPointerDown={(e) => {
|
||||
const el = target.current;
|
||||
if (!el || e.button !== 0) return;
|
||||
e.preventDefault();
|
||||
// A click on the track pages towards it, as the native scrollbar does.
|
||||
if (e.target !== thumb.current) {
|
||||
const { top, bottom } = thumb.current!.getBoundingClientRect();
|
||||
if (e.clientY < top) scrollNow(el, el.scrollTop - el.clientHeight);
|
||||
else if (e.clientY > bottom) scrollNow(el, el.scrollTop + el.clientHeight);
|
||||
return;
|
||||
}
|
||||
drag.current = { y: e.clientY, top: el.scrollTop };
|
||||
thumb.current!.dataset.dragging = "";
|
||||
e.currentTarget.setPointerCapture(e.pointerId);
|
||||
}}
|
||||
onPointerMove={(e) => {
|
||||
const el = target.current;
|
||||
if (!el || !drag.current) return;
|
||||
const g = geometry.current;
|
||||
if (g) scrollNow(el, drag.current.top + ((e.clientY - drag.current.y) / g.range) * g.max);
|
||||
}}
|
||||
onLostPointerCapture={() => {
|
||||
drag.current = null;
|
||||
delete thumb.current?.dataset.dragging;
|
||||
}}
|
||||
>
|
||||
<div ref={thumb} className={styles.thumb} />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
type Geometry = { range: number; max: number };
|
||||
|
||||
/** Positions the track over `el` and the thumb at its scroll offset; null when nothing scrolls. */
|
||||
function layout(el: HTMLElement | null, t: HTMLElement | null, th: HTMLElement | null): Geometry | null {
|
||||
if (!el || !t || !th) return null;
|
||||
const height = el.clientHeight;
|
||||
const max = el.scrollHeight - height;
|
||||
t.style.top = `${el.offsetTop}px`;
|
||||
t.style.height = `${height}px`;
|
||||
t.style.display = max > 0 ? "" : "none";
|
||||
if (max <= 0) return null;
|
||||
const size = Math.max(MIN_THUMB, (height * height) / el.scrollHeight);
|
||||
const range = height - size;
|
||||
th.style.height = `${size}px`;
|
||||
th.style.transform = `translateY(${(Math.min(max, el.scrollTop) / max) * range}px)`;
|
||||
return range > 0 ? { range, max } : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Scrolls `el` and tells its scroll listeners at once. WebKit fires `scroll` for
|
||||
* a script scroll only at a later rendering step and may paint the new offset
|
||||
* first, so a virtual list would show a frame of blank rows mid-drag.
|
||||
*/
|
||||
function scrollNow(el: HTMLElement, top: number) {
|
||||
const before = el.scrollTop;
|
||||
el.scrollTo({ top, behavior: "instant" });
|
||||
if (el.scrollTop !== before) el.dispatchEvent(new Event("scroll"));
|
||||
}
|
||||
@@ -25,6 +25,7 @@ export * from "./Popup";
|
||||
export * from "./Menu";
|
||||
export * from "./Dialog";
|
||||
export * from "./Splitter";
|
||||
export { Scrollbar, hideNativeScrollbar } from "./Scrollbar";
|
||||
export * from "./SourcePreview";
|
||||
export * from "./Feedback";
|
||||
export * from "./HighlightedText";
|
||||
|
||||
Reference in new issue
Block a user