Fix scrollbar click-drag on large diffs

benvinegar · benvinegar · commit aefe483e6065 · 2026-03-25T14:00:07.000-04:00
- Fix stale closure bug: use refs instead of state for drag tracking
- Add track click support: click above/below thumb to page up/down
- Add MIN_THUMB_HEIGHT=2 for easier grabbing
- Remove zIndex that was blocking mouse wheel events

Fixes drag interaction on large diffs (1000+ lines) where state updates
were creating stale closures in the event handlers.
diff --git a/src/ui/components/scrollbar/VerticalScrollbar.tsx b/src/ui/components/scrollbar/VerticalScrollbar.tsx
@@ -12,6 +12,7 @@ import type { AppTheme } from "../../themes";
 
 const HIDE_DELAY_MS = 2000;
 const SCROLLBAR_WIDTH = 1;
+const MIN_THUMB_HEIGHT = 2;
 
 export interface VerticalScrollbarHandle {
   show: () => void;
@@ -32,9 +33,10 @@ interface VerticalScrollbarProps {
 export const VerticalScrollbar = forwardRef<VerticalScrollbarHandle, VerticalScrollbarProps>(
   function VerticalScrollbar({ scrollRef, contentHeight, theme, height, onActivity }, ref) {
     const [isVisible, setIsVisible] = useState(false);
-    const [isDragging, setIsDragging] = useState(false);
-    const [dragStartY, setDragStartY] = useState(0);
-    const [dragStartScroll, setDragStartScroll] = useState(0);
+    const [isDraggingState, setIsDraggingState] = useState(false);
+    const isDraggingRef = useRef(false);
+    const dragStartYRef = useRef(0);
+    const dragStartScrollRef = useRef(0);
     const hideTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
 
     const show = useCallback(() => {
@@ -43,12 +45,12 @@ export const VerticalScrollbar = forwardRef<VerticalScrollbarHandle, VerticalScr
         clearTimeout(hideTimeoutRef.current);
       }
       hideTimeoutRef.current = setTimeout(() => {
-        if (!isDragging) {
+        if (!isDraggingRef.current) {
           setIsVisible(false);
         }
       }, HIDE_DELAY_MS);
       onActivity?.();
-    }, [isDragging, onActivity]);
+    }, [onActivity]);
 
     useImperativeHandle(ref, () => ({ show }), [show]);
 
@@ -67,7 +69,7 @@ export const VerticalScrollbar = forwardRef<VerticalScrollbarHandle, VerticalScr
     // Calculate thumb metrics
     const trackHeight = viewportHeight;
     const scrollRatio = viewportHeight / contentHeight;
-    const thumbHeight = Math.max(SCROLLBAR_WIDTH, Math.floor(trackHeight * scrollRatio));
+    const thumbHeight = Math.max(MIN_THUMB_HEIGHT, Math.floor(trackHeight * scrollRatio));
     const maxThumbY = trackHeight - thumbHeight;
 
     const scrollTop = scrollRef.current?.scrollTop ?? 0;
@@ -79,31 +81,59 @@ export const VerticalScrollbar = forwardRef<VerticalScrollbarHandle, VerticalScr
       if (event.button !== 0) return;
 
       const currentScrollTop = scrollRef.current?.scrollTop ?? 0;
-      setIsDragging(true);
-      setDragStartY(event.y);
-      setDragStartScroll(currentScrollTop);
+      isDraggingRef.current = true;
+      setIsDraggingState(true);
+      dragStartYRef.current = event.y;
+      dragStartScrollRef.current = currentScrollTop;
       show();
       event.preventDefault();
       event.stopPropagation();
     };
 
     const handleMouseDrag = (event: TuiMouseEvent) => {
-      if (!isDragging) return;
+      if (!isDraggingRef.current) {
+        return;
+      }
 
-      const deltaY = event.y - dragStartY;
+      const deltaY = event.y - dragStartYRef.current;
       const pixelsPerRow = maxThumbY / maxScroll;
       const scrollDelta = deltaY / pixelsPerRow;
-      const newScrollTop = Math.max(0, Math.min(maxScroll, dragStartScroll + scrollDelta));
+      const newScrollTop = Math.max(
+        0,
+        Math.min(maxScroll, dragStartScrollRef.current + scrollDelta),
+      );
 
       scrollRef.current?.scrollTo(newScrollTop);
       show();
       event.preventDefault();
       event.stopPropagation();
     };
 
+    const handleTrackClick = (event: TuiMouseEvent) => {
+      if (event.button !== 0) return;
+
+      // Calculate where on the track was clicked
+      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);
+      }
+
+      show();
+      event.preventDefault();
+      event.stopPropagation();
+    };
+
     const handleMouseUp = (event?: TuiMouseEvent) => {
-      if (!isDragging) return;
-      setIsDragging(false);
+      if (!isDraggingRef.current) return;
+      isDraggingRef.current = false;
+      setIsDraggingState(false);
       // Restart hide timer
       if (hideTimeoutRef.current) {
         clearTimeout(hideTimeoutRef.current);
@@ -128,7 +158,6 @@ export const VerticalScrollbar = forwardRef<VerticalScrollbarHandle, VerticalScr
           width: SCROLLBAR_WIDTH,
           height: trackHeight,
           backgroundColor: theme.panel,
-          zIndex: 10,
         }}
       >
         {/* Track background */}
@@ -141,6 +170,7 @@ export const VerticalScrollbar = forwardRef<VerticalScrollbarHandle, VerticalScr
             height: trackHeight,
             backgroundColor: theme.border,
           }}
+          onMouseDown={handleTrackClick}
         />
         {/* Thumb */}
         <box
@@ -150,7 +180,7 @@ export const VerticalScrollbar = forwardRef<VerticalScrollbarHandle, VerticalScr
             left: 0,
             width: SCROLLBAR_WIDTH,
             height: thumbHeight,
-            backgroundColor: isDragging ? theme.accent : theme.accentMuted,
+            backgroundColor: isDraggingState ? theme.accent : theme.accentMuted,
           }}
           onMouseDown={handleMouseDown}
           onMouseDrag={handleMouseDrag}
