Fix scrollbar click-drag on large diffs

[benvinegar]

Summary

Fixes scrollbar click-drag interaction failing on large diffs (1000+ lines).

Changes

1. Fixed stale closure bug - Changed drag tracking from React state to refs. Event handlers now see current values instead of stale state snapshots.

2. Added track click support - Clicking on the scrollbar track (not just the thumb) now scrolls up/down by one viewport.

3. Minimum thumb height - Added MIN_THUMB_HEIGHT=2 so the thumb is easier to grab on large diffs.

4. Removed z-index - Removed zIndex: 10 that was blocking mouse wheel events.

Testing

• Tested on small diffs (working tree changes)
• Tested on large diffs (hunk diff main with 1000+ lines)
• Mouse wheel, click-drag, and track clicking all work correctly

[greptile-apps]

Greptile Summary

This PR fixes the scrollbar click-drag interaction on large diffs by migrating drag-tracking from React state to refs (eliminating stale closure captures), adding track-click support for page-up/down scrolling, enforcing a minimum thumb height of 2, and removing a zIndex: 10 that was blocking mouse wheel events.

Key observations:

• Stale closure fix is correct — isDraggingRef, dragStartYRef, and dragStartScrollRef are now accessed synchronously in event handlers, resolving the root cause of drag failures on large diffs.
• Coordinate frame assumption in handleTrackClick — event.y is compared directly to thumbY (a track-relative offset). This works today because the scrollbar is anchored at top: 0, but if the component is ever embedded at a non-zero y-position the above/below detection will silently produce wrong scrolling. See inline comment.
• Division by zero in drag delta — pixelsPerRow = maxThumbY / maxScroll can evaluate to 0/0 or x/0 when maxThumbY is 0 (e.g. thumb height equals track height), resulting in Infinity and an instantaneous jump to the extremes. See inline comment.
• The dual state+ref pattern for isDragging (isDraggingState for rendering, isDraggingRef for event handlers) is a sensible, well-documented React pattern for this use case.

Confidence Score: 3/5

• Core drag fix is solid, but two edge-case logic issues (coordinate frame assumption, division by zero) should be addressed before merging.
• The primary stale-closure fix is correct and well-executed. However, the new handleTrackClick has a latent coordinate-frame bug that will surface if the component is ever used outside a top-anchored context, and the drag delta calculation has a division-by-zero path when maxThumbY equals 0.
• src/ui/components/scrollbar/VerticalScrollbar.tsx — specifically handleTrackClick (lines 112-131) and handleMouseDrag (line 99).

Important Files Changed

Filename	Overview
src/ui/components/scrollbar/VerticalScrollbar.tsx	Stale-closure drag bug fixed with refs, MIN_THUMB_HEIGHT added, z-index removed, and track-click support added — but the track-click coordinate comparison assumes event.y is track-relative (fragile), and a division-by-zero in the drag delta calculation can occur when maxThumbY is 0.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[MouseDown event] --> B{Target element?}
    B -->|Thumb box| C[handleMouseDown]
    B -->|Track box| D[handleTrackClick]

    C --> E[Set isDraggingRef = true\nSet isDraggingState = true\nStore dragStartY & dragStartScroll]
    E --> F[show - reset hide timer]

    D --> G{clickY vs thumbY?}
    G -->|clickY < thumbY| H[scrollTop -= viewportHeight]
    G -->|clickY >= thumbY + thumbHeight| I[scrollTop += viewportHeight]
    G -->|clickY within thumb| J[No-op]
    H --> K[scrollRef.scrollTo]
    I --> K
    K --> F

    F --> L[MouseDrag event on Thumb]
    L --> M{isDraggingRef?}
    M -->|false| N[return early]
    M -->|true| O[Compute deltaY = event.y - dragStartYRef\nCompute scrollDelta = deltaY / pixelsPerRow]
    O --> P[scrollRef.scrollTo clamp 0..maxScroll]
    P --> F

    F --> Q[MouseUp / MouseDragEnd on Thumb]
    Q --> R{isDraggingRef?}
    R -->|false| S[return early]
    R -->|true| T[Set isDraggingRef = false\nSet isDraggingState = false\nRestart hide timer unconditionally]

Loading

_{Reviews (1): Last reviewed commit: "Fix scrollbar click-drag on large diffs" | Re-trigger Greptile}

[greptile-apps]

Potential coordinate frame mismatch

event.y is compared directly to thumbY, but thumbY is a track-relative offset (0 = top of the scrollbar track), while event.y may be an absolute terminal row coordinate.

If the scrollbar's parent container does not start at terminal row 0 (i.e. the diff viewer has a header row, or is embedded at any non-zero y offset), then clickY will be the absolute screen row while thumbY is relative to the track's top — making the above/below detection incorrect.

For example, if the scrollbar starts at absolute y=5 and the thumb is at thumbY=3 (relative):

• Clicking at absolute y=7 (which is above the thumb at absolute y=8) gives 7 < 3 → false, so no scroll-up happens.

The drag handlers avoid this by using deltas (event.y - dragStartYRef.current), which cancel out any absolute offset. The track-click handler should do the same, or derive an element-relative y from the event.

Suggested change

	const clickY = event.y;
	// If clicked above thumb, scroll up one viewport
	// If clicked below thumb, scroll down one viewport
	if (clickY < thumbY) {
	const newScrollTop = Math.max(0, scrollTop - viewportHeight);
	scrollRef.current?.scrollTo(newScrollTop);
	} else if (clickY >= thumbY + thumbHeight) {
	const newScrollTop = Math.min(maxScroll, scrollTop + viewportHeight);
	scrollRef.current?.scrollTo(newScrollTop);
	}
	const handleTrackClick = (event: TuiMouseEvent) => {
	if (event.button !== 0) return;
	// Use delta relative to the drag-start reference to stay in the same
	// coordinate frame as thumbY (track-relative).
	// For a simple click there is no prior mousedown, so we compute the
	// click position relative to the track top via the scroll element.
	const trackTop = scrollRef.current ? 0 : 0; // placeholder – derive from layout if available
	const clickY = event.y - trackTop;
	if (clickY < thumbY) {
	const newScrollTop = Math.max(0, scrollTop - viewportHeight);
	scrollRef.current?.scrollTo(newScrollTop);
	} else if (clickY >= thumbY + thumbHeight) {
	const newScrollTop = Math.min(maxScroll, scrollTop + viewportHeight);
	scrollRef.current?.scrollTo(newScrollTop);
	}
	show();
	event.preventDefault();
	event.stopPropagation();
	};

At minimum, add a comment documenting that this assumes the track's absolute y origin equals 0 (i.e. top: 0 is always relative to the terminal top), so a future layout change doesn't silently break track-click behaviour.

[greptile-apps]

Division by zero when maxThumbY is 0

pixelsPerRow = maxThumbY / maxScroll. With MIN_THUMB_HEIGHT = 2 and SCROLLBAR_WIDTH = 1, when thumbHeight equals trackHeight (i.e. content barely exceeds the viewport so scrollRatio rounds to 1), maxThumbY becomes 0.

A zero maxThumbY yields pixelsPerRow = 0, and then scrollDelta = deltaY / 0 = Infinity, causing newScrollTop to clamp to maxScroll on any downward drag and to 0 on any upward drag — the thumb teleports rather than tracks.

Guard against it:

Suggested change

	const pixelsPerRow = maxThumbY / maxScroll;
	const scrollDelta = deltaY / pixelsPerRow;
	const pixelsPerRow = maxThumbY > 0 ? maxThumbY / maxScroll : 1;