diff --git a/src/ui/components/scrollbar/VerticalScrollbar.tsx b/src/ui/components/scrollbar/VerticalScrollbar.tsx index a0c4dde..ae2edce 100644 --- 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( 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 | null>(null); const show = useCallback(() => { @@ -43,12 +45,12 @@ export const VerticalScrollbar = forwardRef { - 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 { - if (!isDragging) return; + if (!isDraggingRef.current) { + return; + } - const deltaY = event.y - dragStartY; - const pixelsPerRow = maxThumbY / maxScroll; + const deltaY = event.y - dragStartYRef.current; + // Guard against division by zero when thumb fills track (maxThumbY = 0) or no scroll needed + const pixelsPerRow = maxThumbY > 0 && maxScroll > 0 ? maxThumbY / maxScroll : 1; 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(); @@ -101,9 +110,33 @@ export const VerticalScrollbar = forwardRef { + if (event.button !== 0) return; + + // Calculate where on the track was clicked + // Note: event.y is relative to the scrollbar container since the component + // is positioned at top: 0. If scrollbar position changes, this needs adjustment. + 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 +161,6 @@ export const VerticalScrollbar = forwardRef {/* Track background */} @@ -141,6 +173,7 @@ export const VerticalScrollbar = forwardRef {/* Thumb */} { }); } }); + + test("thumb drag scrolls content", async () => { + // Create a file with many lines to ensure scrolling + const before = Array.from({ length: 100 }, (_, j) => `line${j + 1}`).join("\n"); + const after = before.replace("line50", "line50modified"); + + const bootstrap: AppBootstrap = { + input: { + kind: "git", + staged: false, + options: { mode: "split" }, + }, + changeset: { + id: "drag-test", + sourceLabel: "repo", + title: "drag test", + files: [createDiffFile("drag", "src/drag.ts", before, after)], + }, + initialMode: "split", + initialTheme: "midnight", + }; + + const setup = await testRender(, { + width: 160, + height: 20, // Small viewport to force scrolling + }); + + try { + await flush(setup); + await act(async () => { + await Bun.sleep(100); + }); + + // Get initial frame - app centers on the hunk at line 50 + const frame1 = setup.captureCharFrame(); + expect(frame1).toContain("line50"); + + // Drag scrollbar thumb down (rightmost column is scrollbar at x=159, y ranges 0-19) + // Thumb should be at some position, drag it down to scroll + await act(async () => { + // Drag from top area of scrollbar down + await setup.mockMouse.drag(159, 2, 159, 10); + await flush(setup); + await Bun.sleep(100); + }); + + // After dragging down, we should see different content + const frame2 = setup.captureCharFrame(); + expect(frame2).toBeTruthy(); + + // Drag back up + await act(async () => { + await setup.mockMouse.drag(159, 10, 159, 2); + await flush(setup); + await Bun.sleep(100); + }); + + const frame3 = setup.captureCharFrame(); + expect(frame3).toBeTruthy(); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("track click scrolls by one viewport", async () => { + // Create a file with many lines to ensure scrolling + const lines = Array.from({ length: 80 }, (_, j) => `line${String(j + 1).padStart(3, "0")}`); + const before = lines.join("\n"); + const after = before.replace("line040", "line040modified"); + + const bootstrap: AppBootstrap = { + input: { + kind: "git", + staged: false, + options: { mode: "split" }, + }, + changeset: { + id: "track-click-test", + sourceLabel: "repo", + title: "track click test", + files: [createDiffFile("track", "src/track.ts", before, after)], + }, + initialMode: "split", + initialTheme: "midnight", + }; + + const setup = await testRender(, { + width: 160, + height: 15, // Viewport of 15 lines + }); + + try { + await flush(setup); + await act(async () => { + await Bun.sleep(100); + }); + + // Get initial content - app centers on the hunk at line 40 + const frame1 = setup.captureCharFrame(); + expect(frame1).toContain("line040"); + + // First scroll down a bit to make scrollbar visible and move thumb down + await act(async () => { + for (let i = 0; i < 5; i++) { + await setup.mockInput.pressArrow("down"); + } + await flush(setup); + await Bun.sleep(100); + }); + + // Click on scrollbar track below thumb to page down + // Scrollbar is at rightmost column (x=159), click near bottom + await act(async () => { + await setup.mockMouse.click(159, 12); + await flush(setup); + await Bun.sleep(100); + }); + + const frame2 = setup.captureCharFrame(); + // Should have scrolled down further after track click + expect(frame2).toBeTruthy(); + + // Click on scrollbar track above thumb to page up + await act(async () => { + await setup.mockMouse.click(159, 2); + await flush(setup); + await Bun.sleep(100); + }); + + const frame3 = setup.captureCharFrame(); + // Should have scrolled back up + expect(frame3).toBeTruthy(); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("handles edge case when content barely exceeds viewport", async () => { + // Create content that's just slightly larger than viewport + // This tests the division-by-zero guard in drag calculations + // Use the same pattern as other tests which work correctly + const before = Array.from( + { length: 25 }, + (_, j) => `export const line${String(j + 1).padStart(2, "0")} = ${j + 1};`, + ).join("\n"); + const after = before.replace("line08 = 8;", "line08 = 999; // modified"); + + const bootstrap: AppBootstrap = { + input: { + kind: "git", + staged: false, + options: { mode: "split" }, + }, + changeset: { + id: "edge-case-test", + sourceLabel: "repo", + title: "edge case test", + files: [createDiffFile("edge", "src/edge.ts", before, after)], + }, + initialMode: "split", + initialTheme: "midnight", + }; + + const setup = await testRender(, { + width: 160, + height: 15, // Small viewport to force scrolling (25 lines of content in 15-line viewport) + }); + + try { + await flush(setup); + await act(async () => { + await Bun.sleep(100); + }); + + // Verify app renders with the hunk visible - look for the modified line + const frame1 = setup.captureCharFrame(); + expect(frame1).toContain("line08"); + + // Try to drag - should not crash with division by zero + await act(async () => { + await setup.mockMouse.drag(159, 0, 159, 5); + await flush(setup); + await Bun.sleep(100); + }); + + // App should still be responsive after drag attempt + const frame2 = setup.captureCharFrame(); + expect(frame2).toBeTruthy(); + + // Try track click - should not crash + await act(async () => { + await setup.mockMouse.click(159, 10); + await flush(setup); + await Bun.sleep(100); + }); + + const frame3 = setup.captureCharFrame(); + expect(frame3).toBeTruthy(); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); });