Skip to content

Commit 8c52449

Browse files
authored
🤖 fix: drag-select review diff lines via indicator (#1133)
Adds drag-to-select for review notes when dragging on the +/- indicator column. - Mouse down on indicator starts selection; dragging across indicators extends it - Selection stops on mouseup/blur - Updated tooltip text and added an integration test covering drag selection Validation: - make static-check _Generated with `mux`_
1 parent e6de168 commit 8c52449

File tree

2 files changed

+159
-4
lines changed

2 files changed

+159
-4
lines changed

src/browser/components/shared/DiffRenderer.tsx

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,31 @@ interface DiffIndicatorProps {
175175
type: DiffLineType;
176176
/** Render review button overlay on hover */
177177
reviewButton?: React.ReactNode;
178+
/** When provided, enables drag-to-select behavior in SelectableDiffRenderer */
179+
onMouseDown?: React.MouseEventHandler<HTMLSpanElement>;
180+
onMouseEnter?: React.MouseEventHandler<HTMLSpanElement>;
181+
isInteractive?: boolean;
182+
lineIndex?: number;
178183
}
179184

180-
const DiffIndicator: React.FC<DiffIndicatorProps> = ({ type, reviewButton }) => (
181-
<span className="relative inline-block w-4 shrink-0 text-center select-none">
185+
const DiffIndicator: React.FC<DiffIndicatorProps> = ({
186+
type,
187+
reviewButton,
188+
onMouseDown,
189+
onMouseEnter,
190+
isInteractive,
191+
lineIndex,
192+
}) => (
193+
<span
194+
data-diff-indicator={true}
195+
data-line-index={lineIndex}
196+
className={cn(
197+
"relative inline-block w-4 shrink-0 text-center select-none",
198+
isInteractive && "cursor-pointer"
199+
)}
200+
onMouseDown={onMouseDown}
201+
onMouseEnter={onMouseEnter}
202+
>
182203
<span
183204
className={cn("transition-opacity", reviewButton && "group-hover:opacity-0")}
184205
style={{ color: getIndicatorColor(type) }}
@@ -657,6 +678,23 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
657678
searchConfig,
658679
enableHighlighting = true,
659680
}) => {
681+
const dragAnchorRef = React.useRef<number | null>(null);
682+
const [isDragging, setIsDragging] = React.useState(false);
683+
684+
React.useEffect(() => {
685+
const stopDragging = () => {
686+
setIsDragging(false);
687+
dragAnchorRef.current = null;
688+
};
689+
690+
window.addEventListener("mouseup", stopDragging);
691+
window.addEventListener("blur", stopDragging);
692+
693+
return () => {
694+
window.removeEventListener("mouseup", stopDragging);
695+
window.removeEventListener("blur", stopDragging);
696+
};
697+
}, []);
660698
const { theme } = useTheme();
661699
const [selection, setSelection] = React.useState<LineSelection | null>(null);
662700

@@ -742,6 +780,34 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
742780
};
743781
}, [lineData, showLineNumbers]);
744782

783+
const startDragSelection = React.useCallback(
784+
(lineIndex: number, shiftKey: boolean) => {
785+
if (!onReviewNote) {
786+
return;
787+
}
788+
789+
// Notify parent that this hunk should become active
790+
onLineClick?.();
791+
792+
const anchor = shiftKey && selection ? selection.startIndex : lineIndex;
793+
dragAnchorRef.current = anchor;
794+
setIsDragging(true);
795+
setSelection({ startIndex: anchor, endIndex: lineIndex });
796+
},
797+
[onLineClick, onReviewNote, selection]
798+
);
799+
800+
const updateDragSelection = React.useCallback(
801+
(lineIndex: number) => {
802+
if (!isDragging || dragAnchorRef.current === null) {
803+
return;
804+
}
805+
806+
setSelection({ startIndex: dragAnchorRef.current, endIndex: lineIndex });
807+
},
808+
[isDragging]
809+
);
810+
745811
const handleCommentButtonClick = (lineIndex: number, shiftKey: boolean) => {
746812
// Notify parent that this hunk should become active
747813
onLineClick?.();
@@ -797,6 +863,7 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
797863
<React.Fragment key={displayIndex}>
798864
<div
799865
className={cn(SELECTABLE_DIFF_LINE_CLASS, "flex w-full relative cursor-text group")}
866+
data-selected={isSelected ? "true" : "false"}
800867
style={{
801868
background: isSelected
802869
? "hsl(from var(--color-review-accent) h s l / 0.16)"
@@ -819,9 +886,22 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
819886
>
820887
<DiffIndicator
821888
type={lineInfo.type}
889+
lineIndex={displayIndex}
890+
isInteractive={Boolean(onReviewNote)}
891+
onMouseDown={(e) => {
892+
if (!onReviewNote) return;
893+
if (e.button !== 0) return;
894+
e.preventDefault();
895+
e.stopPropagation();
896+
startDragSelection(displayIndex, e.shiftKey);
897+
}}
898+
onMouseEnter={() => {
899+
if (!onReviewNote) return;
900+
updateDragSelection(displayIndex);
901+
}}
822902
reviewButton={
823903
onReviewNote && (
824-
<Tooltip>
904+
<Tooltip {...(selection || isDragging ? { open: false } : {})}>
825905
<TooltipTrigger asChild>
826906
<button
827907
className="pointer-events-none absolute inset-0 flex items-center justify-center rounded-sm text-[var(--color-review-accent)]/60 opacity-0 transition-opacity group-hover:pointer-events-auto group-hover:opacity-100 hover:text-[var(--color-review-accent)] active:scale-90"
@@ -837,7 +917,7 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
837917
<TooltipContent side="bottom" align="start">
838918
Add review comment
839919
<br />
840-
(Shift-click to select range)
920+
(Shift-click or drag to select range)
841921
</TooltipContent>
842922
</Tooltip>
843923
)
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import React from "react";
2+
import { afterEach, beforeEach, describe, expect, mock, test, type Mock } from "bun:test";
3+
import { GlobalWindow } from "happy-dom";
4+
import { cleanup, fireEvent, render, waitFor } from "@testing-library/react";
5+
6+
import { ThemeProvider } from "@/browser/contexts/ThemeContext";
7+
import { TooltipProvider } from "@/browser/components/ui/tooltip";
8+
import { SelectableDiffRenderer } from "./DiffRenderer";
9+
10+
describe("SelectableDiffRenderer drag selection", () => {
11+
let onReviewNote: Mock<(data: unknown) => void>;
12+
13+
beforeEach(() => {
14+
globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis;
15+
globalThis.document = globalThis.window.document;
16+
17+
onReviewNote = mock(() => undefined);
18+
});
19+
20+
afterEach(() => {
21+
cleanup();
22+
globalThis.window = undefined as unknown as Window & typeof globalThis;
23+
globalThis.document = undefined as unknown as Document;
24+
});
25+
26+
test("dragging on the indicator column selects a line range", async () => {
27+
const content = "+const a = 1;\n+const b = 2;\n+const c = 3;";
28+
29+
const { container, getByPlaceholderText } = render(
30+
<ThemeProvider forcedTheme="dark">
31+
<TooltipProvider>
32+
<SelectableDiffRenderer
33+
content={content}
34+
filePath="src/test.ts"
35+
onReviewNote={onReviewNote}
36+
maxHeight="none"
37+
enableHighlighting={false}
38+
/>
39+
</TooltipProvider>
40+
</ThemeProvider>
41+
);
42+
43+
await waitFor(() => {
44+
const indicators = container.querySelectorAll('[data-diff-indicator="true"]');
45+
expect(indicators.length).toBe(3);
46+
});
47+
48+
const indicators = Array.from(
49+
container.querySelectorAll<HTMLSpanElement>('[data-diff-indicator="true"]')
50+
);
51+
52+
fireEvent.mouseDown(indicators[0], { button: 0 });
53+
fireEvent.mouseEnter(indicators[2]);
54+
fireEvent.mouseUp(window);
55+
56+
const textarea = (await waitFor(() =>
57+
getByPlaceholderText(/Add a review note/i)
58+
)) as HTMLTextAreaElement;
59+
60+
await waitFor(() => {
61+
const selectedLines = Array.from(
62+
container.querySelectorAll<HTMLElement>('.selectable-diff-line[data-selected="true"]')
63+
);
64+
expect(selectedLines.length).toBe(3);
65+
66+
const allLines = Array.from(container.querySelectorAll<HTMLElement>(".selectable-diff-line"));
67+
expect(allLines.length).toBe(3);
68+
69+
// Input should render *after* the last selected line (line 2).
70+
const inputWrapper = allLines[2]?.nextElementSibling;
71+
expect(inputWrapper).toBeTruthy();
72+
expect(inputWrapper?.querySelector("textarea")).toBe(textarea);
73+
});
74+
});
75+
});

0 commit comments

Comments
 (0)