From 19c68079fb0a51e30d54d9878b5b0a5b5f1c64a5 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Mon, 23 Feb 2026 12:43:37 +0000 Subject: [PATCH 1/2] fix: auto-select full hunk on immersive J/K navigation --- .../CodeReview/ImmersiveReviewView.tsx | 54 +++++++++++++++++-- .../components/shared/DiffRenderer.tsx | 18 ++++++- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx b/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx index 7e498c5543..9965f61def 100644 --- a/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx +++ b/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx @@ -357,6 +357,7 @@ export const ImmersiveReviewView: React.FC = (props) = const scrollContainerRef = useRef(null); const notesSidebarRef = useRef(null); const hunkJumpRef = useRef(false); + const pendingJumpSelectAllHunkIdRef = useRef(null); const { api } = useAPI(); const { @@ -436,6 +437,7 @@ export const ImmersiveReviewView: React.FC = (props) = } if (!selectedHunkId || !currentFileHunks.some((hunk) => hunk.id === selectedHunkId)) { + pendingJumpSelectAllHunkIdRef.current = null; onSelectHunk(currentFileHunks[0].id); } }, [currentFileHunks, selectedHunkId, onSelectHunk]); @@ -741,12 +743,27 @@ export const ImmersiveReviewView: React.FC = (props) = // Keep cursor and selection aligned to the selected hunk when hunk navigation changes. useEffect(() => { - if (!selectedHunkRange) { + const resolvedSelectedHunkId = selectedHunk?.id ?? null; + + if (!selectedHunkRange || !resolvedSelectedHunkId) { + pendingJumpSelectAllHunkIdRef.current = null; setActiveLineIndex(null); setSelectedLineRange(null); return; } + const shouldSelectEntireHunk = pendingJumpSelectAllHunkIdRef.current === resolvedSelectedHunkId; + if (shouldSelectEntireHunk) { + pendingJumpSelectAllHunkIdRef.current = null; + const hunkAnchorLine = selectedHunkRange.firstModifiedIndex ?? selectedHunkRange.startIndex; + setActiveLineIndex(hunkAnchorLine); + setSelectedLineRange({ + startIndex: selectedHunkRange.startIndex, + endIndex: selectedHunkRange.endIndex, + }); + return; + } + setActiveLineIndex((previousLineIndex) => { if ( previousLineIndex !== null && @@ -800,6 +817,7 @@ export const ImmersiveReviewView: React.FC = (props) = candidatePath = nextPath; const fileHunks = sortHunksInFileOrder(getFileHunks(hunks, candidatePath)); if (fileHunks.length > 0) { + pendingJumpSelectAllHunkIdRef.current = null; hunkJumpRef.current = true; onSelectHunk(fileHunks[0].id); return; @@ -828,8 +846,10 @@ export const ImmersiveReviewView: React.FC = (props) = } } + const targetHunkId = currentFileHunks[nextIdx].id; + pendingJumpSelectAllHunkIdRef.current = targetHunkId; hunkJumpRef.current = true; - onSelectHunk(currentFileHunks[nextIdx].id); + onSelectHunk(targetHunkId); }, [currentFileHunks, selectedHunkId, onSelectHunk] ); @@ -842,6 +862,7 @@ export const ImmersiveReviewView: React.FC = (props) = } const targetHunkId = findReviewHunkId(review, fileHunks) ?? fileHunks[0].id; + pendingJumpSelectAllHunkIdRef.current = null; hunkJumpRef.current = true; onSelectHunk(targetHunkId); // Force scroll effect to re-fire even when activeLineIndex is unchanged @@ -927,6 +948,7 @@ export const ImmersiveReviewView: React.FC = (props) = if (resolved.hunk.id !== currentSelectedHunkId) { // Record the in-flight hunk switch so mismatch guards do not clear // this composer request before onSelectHunk propagates. + pendingJumpSelectAllHunkIdRef.current = null; pendingComposerHunkSwitchRef.current = { fromHunkId: currentSelectedHunkId, toHunkId: resolved.hunk.id, @@ -1012,6 +1034,7 @@ export const ImmersiveReviewView: React.FC = (props) = const lineHunkId = overlayData.lineHunkIds[nextIndex]; if (lineHunkId && lineHunkId !== selectedHunkIdRef.current) { + pendingJumpSelectAllHunkIdRef.current = null; onSelectHunk(lineHunkId); } }, @@ -1022,6 +1045,7 @@ export const ImmersiveReviewView: React.FC = (props) = (lineIndex: number, shiftKey: boolean) => { const resolvedHunk = findHunkAtLine(lineIndex, overlayData, currentFileHunks); if (resolvedHunk && selectedHunkIdRef.current !== resolvedHunk.hunk.id) { + pendingJumpSelectAllHunkIdRef.current = null; onSelectHunk(resolvedHunk.hunk.id); } @@ -1320,7 +1344,30 @@ export const ImmersiveReviewView: React.FC = (props) = return; } - if (activeLineIndex !== null && lineIndexForScroll === activeLineIndex) { + const normalizedSelectedLineRange = selectedLineRange + ? { + startIndex: Math.min(selectedLineRange.startIndex, selectedLineRange.endIndex), + endIndex: Math.max(selectedLineRange.startIndex, selectedLineRange.endIndex), + } + : null; + const hasMultiLineSelection = Boolean( + normalizedSelectedLineRange && + normalizedSelectedLineRange.endIndex > normalizedSelectedLineRange.startIndex + ); + const isActiveLineInsideSelection = Boolean( + normalizedSelectedLineRange && + activeLineIndex !== null && + activeLineIndex >= normalizedSelectedLineRange.startIndex && + activeLineIndex <= normalizedSelectedLineRange.endIndex + ); + const shouldRenderActiveLineOutline = + activeLineIndex !== null && + lineIndexForScroll === activeLineIndex && + !(hasMultiLineSelection && isActiveLineInsideSelection); + + // For full-hunk keyboard selections (J/K), suppress the separate active-line + // ring so the range highlight reads as one unified selection. + if (shouldRenderActiveLineOutline) { lineElement.style.outline = ACTIVE_LINE_OUTLINE; lineElement.style.outlineOffset = "-1px"; highlightedLineElementRef.current = lineElement; @@ -1353,6 +1400,7 @@ export const ImmersiveReviewView: React.FC = (props) = overlayData.content, revealTargetLineIndex, scrollNonce, + selectedLineRange, ]); useEffect(() => { diff --git a/src/browser/components/shared/DiffRenderer.tsx b/src/browser/components/shared/DiffRenderer.tsx index afcfaa6a43..f7e0ea3163 100644 --- a/src/browser/components/shared/DiffRenderer.tsx +++ b/src/browser/components/shared/DiffRenderer.tsx @@ -1283,6 +1283,16 @@ export const SelectableDiffRenderer = React.memo( const rangeSelectionHighlight = "inset 0 0 0 100vmax hsl(from var(--color-review-accent) h s l / 0.12)"; const activeLineHighlight = "inset 0 0 0 1px hsl(from var(--color-review-accent) h s l / 0.45)"; + const normalizedSelectedLineRange = selectedLineRange + ? { + startIndex: Math.min(selectedLineRange.startIndex, selectedLineRange.endIndex), + endIndex: Math.max(selectedLineRange.startIndex, selectedLineRange.endIndex), + } + : null; + const hasMultiLineExternalSelection = Boolean( + normalizedSelectedLineRange && + normalizedSelectedLineRange.endIndex > normalizedSelectedLineRange.startIndex + ); return ( ( > {highlightedLineData.map((lineInfo, displayIndex) => { const isComposerSelected = isLineInSelection(displayIndex, renderSelection); - const isRangeSelected = isLineInSelection(displayIndex, selectedLineRange); + const isRangeSelected = isLineInSelection(displayIndex, normalizedSelectedLineRange); const isActiveLine = activeLineIndex === displayIndex; + // When a multi-line selection is active (e.g. immersive J/K full-hunk selection), + // let the range highlight own the visual state so the cursor doesn't appear detached. + const shouldRenderActiveLineHighlight = + isActiveLine && !(hasMultiLineExternalSelection && isRangeSelected); const isInReviewRange = reviewRangeByLineIndex[displayIndex] ?? false; const baseCodeBg = getDiffLineBackground(lineInfo.type); const codeBg = applyReviewRangeOverlay(baseCodeBg, isInReviewRange); @@ -1311,7 +1325,7 @@ export const SelectableDiffRenderer = React.memo( } else if (isRangeSelected) { lineShadows.push(rangeSelectionHighlight); } - if (isActiveLine) { + if (shouldRenderActiveLineHighlight) { lineShadows.push(activeLineHighlight); } From 74f7aab5ba4a94a35c2ce85d26b6f3a6d2d1edf5 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 24 Feb 2026 13:54:50 +0000 Subject: [PATCH 2/2] fix: guard immersive Escape handler against open modal dialogs Add isDialogOpen() check before Escape handling in capture-phase keydown listener. Since capture-phase listeners fire before bubble-phase stopPropagation from dialog onKeyDown, pressing Escape could exit immersive review while a modal (command palette, secrets dialog, etc.) was active. The guard skips all immersive shortcuts when a [role=dialog][aria-modal=true] element is present. --- .../RightSidebar/CodeReview/ImmersiveReviewView.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx b/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx index 9965f61def..5384a10b7c 100644 --- a/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx +++ b/src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx @@ -27,7 +27,12 @@ import { getAdjacentFilePath, getFileHunks, } from "@/browser/utils/review/navigation"; -import { isEditableElement, KEYBINDS, matchesKeybind } from "@/browser/utils/ui/keybinds"; +import { + isDialogOpen, + isEditableElement, + KEYBINDS, + matchesKeybind, +} from "@/browser/utils/ui/keybinds"; import { stopKeyboardPropagation } from "@/browser/utils/events"; import { buildReadFileScript, processFileContents } from "@/browser/utils/fileExplorer"; import { @@ -1185,6 +1190,11 @@ export const ImmersiveReviewView: React.FC = (props) = // Don't intercept when typing in editable elements if (isEditableElement(e.target)) return; + // Don't intercept Escape (or any shortcut) while a modal dialog is open. + // This handler runs in capture phase, so bubble-phase stopPropagation + // from dialog onKeyDown can't block it; check the DOM directly. + if (isDialogOpen()) return; + // Esc: exit immersive if (matchesKeybind(e, KEYBINDS.CANCEL)) { e.preventDefault();