Skip to content

🤖 fix: remove review line tooltip #3091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ammar-agent wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −120
Open

🤖 fix: remove review line tooltip #3091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 6 additions & 114 deletions src/browser/features/Shared/DiffRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
/**
* DiffRenderer - Shared diff rendering component
* Used by FileEditToolCall for read-only diff display.
* ReviewPanel uses SelectableDiffRenderer for interactive line selection.
* ReviewPanel and ImmersiveReviewView both use SelectableDiffRenderer for
* interactive line selection.
*/

import React, { useEffect, useMemo, useState } from "react";
import { createPortal } from "react-dom";
import { stopKeyboardPropagation } from "@/browser/utils/events";
import { cn } from "@/common/lib/utils";
import { getLanguageFromPath } from "@/common/utils/git/languageDetector";
import { useOverflowDetection } from "@/browser/hooks/useOverflowDetection";
import { MessageSquare } from "lucide-react";
import { TOOLTIP_SURFACE_CLASSNAME } from "@/browser/components/Tooltip/Tooltip";
import { InlineReviewNote, type ReviewActionCallbacks } from "./InlineReviewNote";
import { groupDiffLines } from "@/browser/utils/highlighting/diffChunking";
import { useTheme, type ThemeMode } from "@/browser/contexts/ThemeContext";
Expand Down Expand Up @@ -672,15 +671,6 @@ interface LineSelection {
endIndex: number;
}

interface TooltipAnchorRect {
left: number;
top: number;
width: number;
height: number;
}

const REVIEW_COMMENT_TOOLTIP = "Add review comment (Shift-click or drag to select range)";

// CSS class for diff line wrapper - used by arbitrary selector in CommentButton
const SELECTABLE_DIFF_LINE_CLASS = "selectable-diff-line";

Expand Down Expand Up @@ -1029,49 +1019,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
const [selection, setSelection] = React.useState<LineSelection | null>(null);
const [selectionInitialNoteText, setSelectionInitialNoteText] = React.useState("");

const reviewTooltipTriggerRef = React.useRef<HTMLButtonElement | null>(null);
const [reviewTooltipAnchorRect, setReviewTooltipAnchorRect] =
React.useState<TooltipAnchorRect | null>(null);

const hideReviewTooltip = React.useCallback((trigger?: HTMLButtonElement | null) => {
if (trigger && reviewTooltipTriggerRef.current !== trigger) {
return;
}

reviewTooltipTriggerRef.current = null;
setReviewTooltipAnchorRect(null);
}, []);

const syncReviewTooltipAnchor = React.useCallback(() => {
const trigger = reviewTooltipTriggerRef.current;
if (!trigger?.isConnected) {
hideReviewTooltip();
return;
}

const { left, top, width, height } = trigger.getBoundingClientRect();
setReviewTooltipAnchorRect((previousRect) => {
if (
previousRect?.left === left &&
previousRect?.top === top &&
previousRect?.width === width &&
previousRect?.height === height
) {
return previousRect;
}

return { left, top, width, height };
});
}, [hideReviewTooltip]);

const showReviewTooltip = React.useCallback(
(trigger: HTMLButtonElement) => {
reviewTooltipTriggerRef.current = trigger;
syncReviewTooltipAnchor();
},
[syncReviewTooltipAnchor]
);

const flushPendingDragSelection = React.useCallback(() => {
const anchorIndex = dragAnchorRef.current;
const pendingLineIndex = pendingDragLineIndexRef.current;
Expand Down Expand Up @@ -1139,34 +1086,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
};
}, []);

React.useEffect(() => {
if (!reviewTooltipAnchorRect) {
return;
}

const handleViewportChange = () => {
syncReviewTooltipAnchor();
};

window.addEventListener("resize", handleViewportChange);
window.addEventListener("scroll", handleViewportChange, true);

return () => {
window.removeEventListener("resize", handleViewportChange);
window.removeEventListener("scroll", handleViewportChange, true);
};
}, [reviewTooltipAnchorRect, syncReviewTooltipAnchor]);

React.useEffect(() => {
if (!reviewTooltipTriggerRef.current) {
return;
}

// File/hunk switches can remove the hovered trigger during a normal React render without any
// scroll/resize event, so resync here too to avoid leaving a stale floating tooltip behind.
syncReviewTooltipAnchor();
});

const { theme } = useTheme();

const lastExternalSelectionRequestIdRef = React.useRef<number | null>(null);
Expand Down Expand Up @@ -1368,8 +1287,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
return;
}

hideReviewTooltip();

// Notify parent that this hunk should become active.
onLineClick?.();
onLineIndexSelect?.(lineIndex, shiftKey);
Expand All @@ -1396,7 +1313,7 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
return { startIndex: anchor, endIndex: lineIndex };
});
},
[hideReviewTooltip, onLineClick, onLineIndexSelect, onReviewNote, renderSelectionStartIndex]
[onLineClick, onLineIndexSelect, onReviewNote, renderSelectionStartIndex]
);

const updateDragSelection = React.useCallback(
Expand All @@ -1413,8 +1330,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
);

const handleCommentButtonClick = (lineIndex: number, shiftKey: boolean) => {
hideReviewTooltip();

// Keep immersive cursor/hunk selection in sync with inline comment actions.
onLineClick?.();
onLineIndexSelect?.(lineIndex, shiftKey);
Expand Down Expand Up @@ -1573,17 +1488,13 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
reviewButton={
onReviewNote && (
<>
{/* Regular review can mount thousands of diff lines at once, so keep
one shared tooltip anchored to the active button instead of mounting
a full Radix tooltip tree for every individual line. */}
{/* Both review panes share SelectableDiffRenderer, so keep the
review action inline without an extra floating tooltip
covering nearby lines. */}
<button
type="button"
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"
style={{ position: "absolute", inset: 0 }}
onMouseEnter={(event) => showReviewTooltip(event.currentTarget)}
onMouseLeave={(event) => hideReviewTooltip(event.currentTarget)}
onFocus={(event) => showReviewTooltip(event.currentTarget)}
onBlur={(event) => hideReviewTooltip(event.currentTarget)}
onClick={(e) => {
e.stopPropagation();
handleCommentButtonClick(displayIndex, e.shiftKey);
Expand Down Expand Up @@ -1645,25 +1556,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
);
})}
</DiffContainer>
{reviewTooltipAnchorRect &&
createPortal(
<div
className={cn(
TOOLTIP_SURFACE_CLASSNAME,
"pointer-events-none fixed z-[10001] border-separator-light"
)}
style={{
left: reviewTooltipAnchorRect.left + reviewTooltipAnchorRect.width + 8,
top: reviewTooltipAnchorRect.top + reviewTooltipAnchorRect.height / 2,
maxWidth: "min(20rem, calc(100vw - 24px))",
transform: "translateY(-50%)",
}}
>
<span className="border-border-medium bg-modal-bg absolute top-1/2 left-0 h-2 w-2 -translate-x-1/2 -translate-y-1/2 rotate-45 border-b border-l" />
{REVIEW_COMMENT_TOOLTIP}
</div>,
document.body
)}
</>
);
}
Expand Down
10 changes: 4 additions & 6 deletions src/browser/features/Shared/SelectableDiffRenderer.dragSelect.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe("SelectableDiffRenderer drag selection", () => {
}
});

test("hovering the review button uses the full custom range-selection tooltip", async () => {
test("hovering the review button does not show a tooltip", async () => {
const content = "+const a = 1;\n+const b = 2;";

const { container } = render(
Expand All @@ -128,11 +128,9 @@ describe("SelectableDiffRenderer drag selection", () => {

fireEvent.mouseEnter(commentButton!);

await waitFor(() => {
expect(document.body.textContent).toContain(
"Add review comment (Shift-click or drag to select range)"
);
});
expect(document.body.textContent).not.toContain(
"Add review comment (Shift-click or drag to select range)"
);
});

test("dragging on the indicator column selects a line range", async () => {
Expand Down
Loading