refactor: update review components to support global hunk offsets and total review hunks

- Introduced global hunk offset and total review hunks properties in ChangeReviewDialog, ContinuousScrollView, FileSectionDiff, and CodeMirrorDiffView components to enhance the review experience.
- Updated related components to utilize these new properties for better tracking and display of review progress across multiple files.
- Improved floating toolbar functionality in CodeMirrorDiffView to reflect the current hunk index and total count, enhancing user interaction during code reviews.
This commit is contained in:
iliya 2026-03-13 17:02:38 +02:00
parent afcb0fcc1a
commit b76be0f634
5 changed files with 373 additions and 316 deletions

View file

@ -616,15 +616,14 @@ function buildTaskStatusProtocol(teamName: string): string {
Do NOT comment on trivial coordination messages. Only comment when the information is valuable context for the task. Do NOT comment on trivial coordination messages. Only comment when the information is valuable context for the task.
9. When sending a message about a specific task, include its short display label like #<displayId> in your SendMessage summary field for traceability. 9. When sending a message about a specific task, include its short display label like #<displayId> in your SendMessage summary field for traceability.
10. In ALL human-facing or teammate-facing message text, when you mention a task reference, ALWAYS write it with a leading # (for example: #abcd1234, not abcd1234 or "task abcd1234"). 10. In ALL human-facing or teammate-facing message text, when you mention a task reference, ALWAYS write it with a leading # (for example: #abcd1234, not abcd1234 or "task abcd1234").
11. In ALL human-facing or teammate-facing message text, when you mention a teammate, ALWAYS write their name with a leading @ (for example: @alice, not alice). When you mention another team, also use @ (for example: @signal-ops, not signal-ops). 11. Review workflow clarity (IMPORTANT):
12. Review workflow clarity (IMPORTANT):
- The work task (e.g. #1) is the thing that must end up APPROVED after review. - The work task (e.g. #1) is the thing that must end up APPROVED after review.
- If you are reviewing work for task #X, run review_approve/review_request_changes on #X (the work task). - If you are reviewing work for task #X, run review_approve/review_request_changes on #X (the work task).
- Do NOT approve a separate "review task" (e.g. #2 created just to ask for a review) that will put the wrong task into APPROVED. - Do NOT approve a separate "review task" (e.g. #2 created just to ask for a review) that will put the wrong task into APPROVED.
- Typical flow: - Typical flow:
a) Owner finishes work on #X -> task_complete #X a) Owner finishes work on #X -> task_complete #X
b) Reviewer accepts -> review_approve #X b) Reviewer accepts -> review_approve #X
13. CLARIFICATION PROTOCOL (CRITICAL MANDATORY): 12. CLARIFICATION PROTOCOL (CRITICAL MANDATORY):
When you are blocked and need information to continue a task, you MUST do ALL steps below skipping the board update or comment breaks traceability: When you are blocked and need information to continue a task, you MUST do ALL steps below skipping the board update or comment breaks traceability:
a) STEP 1 FIRST, set the clarification flag with MCP tool task_set_clarification: a) STEP 1 FIRST, set the clarification flag with MCP tool task_set_clarification:
{ teamName: "${teamName}", taskId: "<taskId>", value: "lead" } { teamName: "${teamName}", taskId: "<taskId>", value: "lead" }
@ -636,10 +635,10 @@ function buildTaskStatusProtocol(teamName: string): string {
If the lead replies via SendMessage instead, clear the flag yourself once you have the answer: If the lead replies via SendMessage instead, clear the flag yourself once you have the answer:
{ teamName: "${teamName}", taskId: "<taskId>", value: "clear" } { teamName: "${teamName}", taskId: "<taskId>", value: "clear" }
e) Do NOT set clarification to "user" yourself only the team lead escalates to the user. e) Do NOT set clarification to "user" yourself only the team lead escalates to the user.
14. DEPENDENCY AWARENESS: 13. DEPENDENCY AWARENESS:
When your task has blockedBy dependencies, check if they are completed before starting. When your task has blockedBy dependencies, check if they are completed before starting.
When you complete a task that blocks others, mention this in your completion message so blocked teammates can proceed. When you complete a task that blocks others, mention this in your completion message so blocked teammates can proceed.
15. TASK QUEUE DISCIPLINE: 14. TASK QUEUE DISCIPLINE:
- Use task_briefing as a compact queue view of your assigned tasks. - Use task_briefing as a compact queue view of your assigned tasks.
- task_briefing may include full description/comments only for in_progress tasks; needsFix/pending/review/completed entries may be minimal on purpose. - task_briefing may include full description/comments only for in_progress tasks; needsFix/pending/review/completed entries may be minimal on purpose.
- Finish existing in_progress tasks first. - Finish existing in_progress tasks first.
@ -835,7 +834,7 @@ Communication protocol (CRITICAL — you are running headless, no one sees your
- Do NOT spam other teams, and do NOT use cross-team messaging for trivial FYIs that do not require action, coordination, or domain knowledge. - Do NOT spam other teams, and do NOT use cross-team messaging for trivial FYIs that do not require action, coordination, or domain knowledge.
Message formatting: Message formatting:
- When mentioning teammates by name in messages and text output, always use @ prefix (e.g. @alice, @bob) for UI highlighting. Do NOT use @ in tool parameters (recipient, owner, etc.) those require plain names. - When mentioning teammates by name in messages and text output, always use @ prefix (e.g. @alice, @bob) for UI highlighting. When mentioning another team, also use @ (e.g. @signal-ops). Do NOT use @ in tool parameters (recipient, owner, etc.) those require plain names.
${agentBlockPolicy} ${agentBlockPolicy}
${membersFooter}`; ${membersFooter}`;

View file

@ -10,10 +10,10 @@ import { useViewedFiles } from '@renderer/hooks/useViewedFiles';
import { cn } from '@renderer/lib/utils'; import { cn } from '@renderer/lib/utils';
import { useStore } from '@renderer/store'; import { useStore } from '@renderer/store';
import { getFileHunkCount, REVIEW_INSTANT_APPLY } from '@renderer/store/slices/changeReviewSlice'; import { getFileHunkCount, REVIEW_INSTANT_APPLY } from '@renderer/store/slices/changeReviewSlice';
import { type TaskChangeRequestOptions } from '@renderer/utils/taskChangeRequest';
import { buildSelectionAction } from '@renderer/utils/buildSelectionAction'; import { buildSelectionAction } from '@renderer/utils/buildSelectionAction';
import { buildSelectionInfo, SELECTION_DEBOUNCE_MS } from '@renderer/utils/codemirrorSelectionInfo'; import { buildSelectionInfo, SELECTION_DEBOUNCE_MS } from '@renderer/utils/codemirrorSelectionInfo';
import { sortItemsAsTree } from '@renderer/utils/fileTreeBuilder'; import { sortItemsAsTree } from '@renderer/utils/fileTreeBuilder';
import { type TaskChangeRequestOptions } from '@renderer/utils/taskChangeRequest';
import { ChevronDown, Clock, X } from 'lucide-react'; import { ChevronDown, Clock, X } from 'lucide-react';
import { acceptAllChunks, computeChunkIndexAtPos, rejectAllChunks } from './CodeMirrorDiffUtils'; import { acceptAllChunks, computeChunkIndexAtPos, rejectAllChunks } from './CodeMirrorDiffUtils';
@ -34,11 +34,11 @@ import type {
} from '@shared/types'; } from '@shared/types';
import type { EditorSelectionAction, EditorSelectionInfo } from '@shared/types/editor'; import type { EditorSelectionAction, EditorSelectionInfo } from '@shared/types/editor';
type RecentHunkUndoAction = { interface RecentHunkUndoAction {
filePath: string; filePath: string;
originalIndex: number; originalIndex: number;
at: number; at: number;
}; }
interface ChangeReviewDialogProps { interface ChangeReviewDialogProps {
open: boolean; open: boolean;
@ -648,6 +648,16 @@ export const ChangeReviewDialog = ({
getFileHunkCount(filePath, fallbackSnippetsLength, fileChunkCounts) getFileHunkCount(filePath, fallbackSnippetsLength, fileChunkCounts)
); );
const reviewHunkOrder = useMemo(() => {
const offsets: Record<string, number> = {};
let total = 0;
for (const file of sortedFiles) {
offsets[file.filePath] = total;
total += getFileHunkCount(file.filePath, file.snippets.length, fileChunkCounts);
}
return { offsets, total };
}, [sortedFiles, fileChunkCounts]);
const toggleCollapsedFile = useCallback((filePath: string) => { const toggleCollapsedFile = useCallback((filePath: string) => {
setCollapsedFiles((prev) => { setCollapsedFiles((prev) => {
const next = new Set(prev); const next = new Set(prev);
@ -1234,6 +1244,8 @@ export const ChangeReviewDialog = ({
memberName={memberName} memberName={memberName}
fetchFileContent={fetchFileContent} fetchFileContent={fetchFileContent}
onSelectionChange={onEditorAction ? handleSelectionChange : undefined} onSelectionChange={onEditorAction ? handleSelectionChange : undefined}
globalHunkOffsets={reviewHunkOrder.offsets}
totalReviewHunks={reviewHunkOrder.total}
/> />
{selectionInfo && onEditorAction && ( {selectionInfo && onEditorAction && (
<EditorSelectionMenu <EditorSelectionMenu

View file

@ -51,6 +51,10 @@ interface CodeMirrorDiffViewProps {
portionSize?: number; portionSize?: number;
/** Called when text selection changes (for floating action menu) */ /** Called when text selection changes (for floating action menu) */
onSelectionChange?: (info: EditorSelectionInfo | null) => void; onSelectionChange?: (info: EditorSelectionInfo | null) => void;
/** Global hunk offset for this file in the review order */
globalHunkOffset?: number;
/** Total hunk count across all review files */
totalReviewHunks?: number;
} }
/** Compute hunk index for the chunk at a given position (B-side / modified doc). /** Compute hunk index for the chunk at a given position (B-side / modified doc).
@ -200,11 +204,18 @@ export const CodeMirrorDiffView = ({
usePortionCollapse = false, usePortionCollapse = false,
portionSize = 100, portionSize = 100,
onSelectionChange, onSelectionChange,
globalHunkOffset = 0,
totalReviewHunks,
}: CodeMirrorDiffViewProps): React.ReactElement => { }: CodeMirrorDiffViewProps): React.ReactElement => {
const rootRef = useRef<HTMLDivElement>(null);
const containerRef = useRef<HTMLDivElement>(null); const containerRef = useRef<HTMLDivElement>(null);
const viewRef = useRef<EditorView | null>(null); const viewRef = useRef<EditorView | null>(null);
const endSentinelRef = useRef<HTMLDivElement>(null); const endSentinelRef = useRef<HTMLDivElement>(null);
const lastPointerRef = useRef<{ x: number; y: number } | null>(null); const lastPointerRef = useRef<{ x: number; y: number } | null>(null);
const floatingToolbarRef = useRef<HTMLDivElement>(null);
const floatingNavRef = useRef<HTMLDivElement>(null);
const floatingCounterRef = useRef<HTMLSpanElement>(null);
const activeChunkIndexRef = useRef<number | null>(null);
// Local ref to hold externalViewRef for syncing via useEffect // Local ref to hold externalViewRef for syncing via useEffect
const externalViewRefHolder = useRef(externalViewRef); const externalViewRefHolder = useRef(externalViewRef);
@ -251,6 +262,225 @@ export const CodeMirrorDiffView = ({
collapseRef.current = { enabled: collapseUnchangedProp, margin: collapseMargin }; collapseRef.current = { enabled: collapseUnchangedProp, margin: collapseMargin };
}, [collapseUnchangedProp, collapseMargin]); }, [collapseUnchangedProp, collapseMargin]);
const hideFloatingToolbar = useCallback(() => {
const toolbar = floatingToolbarRef.current;
if (!toolbar) return;
toolbar.style.display = 'none';
activeChunkIndexRef.current = null;
}, []);
const positionFloatingToolbar = useCallback((view: EditorView, clientY: number) => {
const toolbar = floatingToolbarRef.current;
const root = rootRef.current;
if (!toolbar || !root) return;
const rootRect = root.getBoundingClientRect();
const scrollerRect = view.scrollDOM.getBoundingClientRect();
const toolbarWidth = toolbar.offsetWidth || 200;
const toolbarHeight = toolbar.offsetHeight || 28;
const margin = 12;
const left = scrollerRect.right - rootRect.left - toolbarWidth - margin;
const clampedTop = Math.max(
scrollerRect.top,
Math.min(clientY - toolbarHeight / 2, scrollerRect.bottom - toolbarHeight)
);
toolbar.style.left = `${left}px`;
toolbar.style.top = `${clampedTop - rootRect.top}px`;
}, []);
const resolveDeletedChunkIndex = useCallback(
(deletedChunk: Element, view: EditorView): number => {
try {
return computeHunkIndexAtPos(view.state, view.posAtDOM(deletedChunk));
} catch {
return -1;
}
},
[]
);
const findHoveredChunkIndex = useCallback(
(clientX: number, clientY: number, view: EditorView): number => {
const hoveredElement = document.elementFromPoint(clientX, clientY);
if (!hoveredElement) return -1;
if (hoveredElement.closest('[data-review-floating-toolbar="true"]')) {
return activeChunkIndexRef.current ?? -1;
}
const deletedChunk = hoveredElement.closest('.cm-deletedChunk');
if (deletedChunk) {
return resolveDeletedChunkIndex(deletedChunk, view);
}
if (
!hoveredElement.closest(
'.cm-changedLine, .cm-insertedLine, .cm-inlineChangedLine, .cm-changedText, .cm-deletedText'
)
) {
return -1;
}
const pos = view.posAtCoords({ x: clientX, y: clientY });
if (pos === null) return -1;
const chunks = getChunks(view.state);
if (!chunks) return -1;
for (let i = 0; i < chunks.chunks.length; i++) {
const chunk = chunks.chunks[i];
const chunkEnd = Math.min(view.state.doc.length, chunk.endB);
if (pos >= chunk.fromB && pos <= chunkEnd) {
return i;
}
}
return -1;
},
[resolveDeletedChunkIndex]
);
const updateFloatingToolbar = useCallback(
(
view: EditorView,
clientY: number,
options?: { clientX?: number; followCursor?: boolean }
): void => {
if (!showMergeControls) {
hideFloatingToolbar();
return;
}
const toolbar = floatingToolbarRef.current;
const nav = floatingNavRef.current;
const counter = floatingCounterRef.current;
const chunks = getChunks(view.state);
if (!toolbar || !chunks || chunks.chunks.length === 0) {
hideFloatingToolbar();
return;
}
let activeIndex =
options?.clientX !== undefined ? findHoveredChunkIndex(options.clientX, clientY, view) : -1;
if (activeIndex < 0) {
hideFloatingToolbar();
return;
}
activeIndex = Math.max(0, Math.min(activeIndex, chunks.chunks.length - 1));
activeChunkIndexRef.current = activeIndex;
if (counter) {
const displayIndex = globalHunkOffset + activeIndex + 1;
const displayTotal = totalReviewHunks ?? chunks.chunks.length;
counter.textContent = `${displayIndex} of ${displayTotal}`;
}
if (nav) {
nav.style.display = chunks.chunks.length > 1 ? '' : 'none';
}
toolbar.style.display = 'flex';
const scrollerRect = view.scrollDOM.getBoundingClientRect();
const targetY = options?.followCursor
? clientY
: (scrollerRect.top + scrollerRect.bottom) / 2;
positionFloatingToolbar(view, targetY);
},
[
findHoveredChunkIndex,
globalHunkOffset,
hideFloatingToolbar,
positionFloatingToolbar,
showMergeControls,
totalReviewHunks,
]
);
const actOnActiveChunk = useCallback(
(decision: 'accept' | 'reject') => {
const view = viewRef.current;
const activeChunkIndex = activeChunkIndexRef.current;
if (!view || activeChunkIndex === null) return;
const chunks = getChunks(view.state);
const chunk = chunks?.chunks[activeChunkIndex];
if (!chunk) return;
if (decision === 'accept') {
acceptChunk(view, chunk.fromB);
onAcceptRef.current?.(activeChunkIndex);
} else {
rejectChunk(view, chunk.fromB);
onRejectRef.current?.(activeChunkIndex);
}
scrollToNextChunk();
requestAnimationFrame(() => {
const scrollerRect = view.scrollDOM.getBoundingClientRect();
updateFloatingToolbar(view, (scrollerRect.top + scrollerRect.bottom) / 2);
});
},
[scrollToNextChunk, updateFloatingToolbar]
);
const moveBetweenChunks = useCallback(
(direction: 'prev' | 'next') => {
const view = viewRef.current;
if (!view) return;
if (direction === 'prev') {
goToPreviousChunk(view);
} else {
goToNextChunk(view);
}
requestAnimationFrame(() => {
const scrollerRect = view.scrollDOM.getBoundingClientRect();
updateFloatingToolbar(view, (scrollerRect.top + scrollerRect.bottom) / 2);
});
},
[updateFloatingToolbar]
);
useEffect(() => {
if (!showMergeControls) return;
const repositionToolbar = (): void => {
const view = viewRef.current;
const root = rootRef.current;
const toolbar = floatingToolbarRef.current;
if (!view || !root || !toolbar || toolbar.style.display === 'none') return;
const pointer = lastPointerRef.current;
const rootRect = root.getBoundingClientRect();
const pointerInsideRoot =
pointer &&
pointer.x >= rootRect.left &&
pointer.x <= rootRect.right &&
pointer.y >= rootRect.top &&
pointer.y <= rootRect.bottom;
if (pointerInsideRoot) {
updateFloatingToolbar(view, pointer.y, {
clientX: pointer.x,
followCursor: true,
});
return;
}
const scrollerRect = view.scrollDOM.getBoundingClientRect();
updateFloatingToolbar(view, (scrollerRect.top + scrollerRect.bottom) / 2);
};
window.addEventListener('scroll', repositionToolbar, true);
window.addEventListener('resize', repositionToolbar);
return () => {
window.removeEventListener('scroll', repositionToolbar, true);
window.removeEventListener('resize', repositionToolbar);
};
}, [showMergeControls, updateFloatingToolbar]);
/** Build unified merge view extension. Extracted for dynamic compartment reconfigure. */ /** Build unified merge view extension. Extracted for dynamic compartment reconfigure. */
const buildMergeExtension = useCallback( const buildMergeExtension = useCallback(
(collapse: boolean, margin: number): Extension => { (collapse: boolean, margin: number): Extension => {
@ -261,12 +491,8 @@ export const CodeMirrorDiffView = ({
syntaxHighlightDeletions: true, syntaxHighlightDeletions: true,
}; };
// IMPORTANT: @codemirror/merge shows accept/reject buttons by default. // We render our own floating merge toolbar outside CodeMirror's DeletionWidget DOM.
// When our UI chooses to hide merge controls (e.g. "Missing on disk" preview), mergeConfig.mergeControls = false;
// explicitly disable them rather than relying on default behavior.
if (!showMergeControls) {
mergeConfig.mergeControls = false;
}
if (collapse && !usePortionCollapse) { if (collapse && !usePortionCollapse) {
mergeConfig.collapseUnchanged = { mergeConfig.collapseUnchanged = {
@ -275,120 +501,9 @@ export const CodeMirrorDiffView = ({
}; };
} }
if (showMergeControls) {
// NOTE: We intentionally do NOT use the `action` callback from @codemirror/merge.
// CM's DeletionWidget caches DOM via a global WeakMap keyed by chunk.changes.
// When EditorView is recreated (e.g. from cached initialState), toDOM() returns
// the OLD cached DOM whose `action` closure references the DESTROYED view.
// Instead, we call acceptChunk/rejectChunk directly with viewRef.current.
//
// CM calls mergeControls twice per chunk: 'accept' first, 'reject' second.
// Both elements go into `.cm-chunkButtons`. We return the full toolbar for
// 'accept' and a hidden span for 'reject'.
mergeConfig.mergeControls = (type, _action) => {
if (type === 'reject') {
const empty = document.createElement('span');
empty.style.display = 'none';
return empty;
}
// --- Full toolbar for 'accept' ---
const toolbar = document.createElement('div');
toolbar.className = 'cm-merge-toolbar';
// Navigation section (hidden by default, shown if >1 chunks)
const nav = document.createElement('div');
nav.className = 'cm-merge-nav';
nav.style.display = 'none';
const prevBtn = document.createElement('button');
prevBtn.className = 'cm-merge-nav-btn';
prevBtn.textContent = '\u2227';
prevBtn.title = 'Previous chunk';
prevBtn.onmousedown = (e) => {
e.preventDefault();
const v = viewRef.current;
if (v) goToPreviousChunk(v);
};
const counter = document.createElement('span');
counter.className = 'cm-merge-nav-counter';
const nextBtn = document.createElement('button');
nextBtn.className = 'cm-merge-nav-btn';
nextBtn.textContent = '\u2228';
nextBtn.title = 'Next chunk';
nextBtn.onmousedown = (e) => {
e.preventDefault();
const v = viewRef.current;
if (v) goToNextChunk(v);
};
nav.append(prevBtn, counter, nextBtn);
toolbar.append(nav);
// Helper: create button with label + kbd shortcut
const makeBtn = (cls: string, label: string, shortcut: string): HTMLButtonElement => {
const btn = document.createElement('button');
btn.className = cls;
btn.append(document.createTextNode(label + ' '));
const kbd = document.createElement('kbd');
kbd.textContent = shortcut;
btn.append(kbd);
return btn;
};
// Undo button (reject action)
const undoBtn = makeBtn('cm-merge-undo', 'Undo', '\u2318N');
undoBtn.title = 'Reject change (⌘N)';
undoBtn.onmousedown = (e) => {
e.preventDefault();
const v = viewRef.current;
if (v) {
const pos = v.posAtDOM(toolbar);
const idx = computeHunkIndexAtPos(v.state, pos);
rejectChunk(v, pos);
onRejectRef.current?.(idx);
scrollToNextChunk();
}
};
toolbar.append(undoBtn);
// Keep button (accept action)
const keepBtn = makeBtn('cm-merge-keep', 'Keep', '\u2318Y');
keepBtn.title = 'Accept change (⌘Y)';
keepBtn.onmousedown = (e) => {
e.preventDefault();
const v = viewRef.current;
if (v) {
const pos = v.posAtDOM(toolbar);
const idx = computeHunkIndexAtPos(v.state, pos);
acceptChunk(v, pos);
onAcceptRef.current?.(idx);
scrollToNextChunk();
}
};
toolbar.append(keepBtn);
// Deferred: compute chunk index + show nav if >1 chunks
requestAnimationFrame(() => {
const v = viewRef.current;
if (!v) return;
const chunks = getChunks(v.state);
if (!chunks || chunks.chunks.length <= 1) return;
const pos = v.posAtDOM(toolbar);
const idx = computeHunkIndexAtPos(v.state, pos);
counter.textContent = `${idx + 1} of ${chunks.chunks.length}`;
nav.style.display = '';
});
return toolbar;
};
}
return unifiedMergeView(mergeConfig); return unifiedMergeView(mergeConfig);
}, },
[original, showMergeControls, scrollToNextChunk, usePortionCollapse] [original, usePortionCollapse]
); );
const buildExtensions = useCallback(() => { const buildExtensions = useCallback(() => {
@ -465,191 +580,11 @@ export const CodeMirrorDiffView = ({
}) })
); );
// Merge toolbar: always visible for nearest chunk, follows cursor when hovering on chunk // External merge toolbar: follows cursor without depending on CodeMirror's widget DOM.
if (showMergeControls) { if (showMergeControls) {
// Helper: pin chunkButtons to right edge of visible viewport, accounting for horizontal scroll.
// Uses getBoundingClientRect() so the offset from gutters / CM content padding is handled exactly.
const pinToViewportRight = (btnContainer: HTMLElement, scroller: Element): void => {
const scrollerRect = scroller.getBoundingClientRect();
const chunkEl = btnContainer.parentElement;
if (!chunkEl) return;
const chunkRect = chunkEl.getBoundingClientRect();
const btnWidth = btnContainer.offsetWidth || 200;
const margin = 12;
const { style } = btnContainer;
// left is relative to .cm-deletedChunk — so we compute from scroller's right edge
style.left = `${scrollerRect.right - chunkRect.left - btnWidth - margin}px`;
style.right = 'auto';
};
// Helper: position a chunkButtons container so it's below the change block,
// but clamped to the visible viewport if that would be off-screen.
const positionAtBottom = (chunkEl: Element, scroller: Element): void => {
const btnContainer = chunkEl.querySelector<HTMLElement>('.cm-chunkButtons');
if (!btnContainer) return;
const parentRect = chunkEl.getBoundingClientRect();
const scrollerRect = scroller.getBoundingClientRect();
// "below block" = 100% of parent height
let targetY = parentRect.bottom;
const tbHeight = btnContainer.offsetHeight || 28;
// Clamp: if bottom edge would go below visible area, pin to viewport bottom
if (targetY + tbHeight > scrollerRect.bottom) {
targetY = scrollerRect.bottom - tbHeight;
}
btnContainer.style.top = `${targetY - parentRect.top}px`;
pinToViewportRight(btnContainer, scroller);
};
const positionAtCursor = (chunkEl: Element, clientY: number, scroller: Element): void => {
const btnContainer = chunkEl.querySelector<HTMLElement>('.cm-chunkButtons');
if (!btnContainer) return;
const parentRect = chunkEl.getBoundingClientRect();
const scrollerRect = scroller.getBoundingClientRect();
const tbHeight = btnContainer.offsetHeight || 28;
let targetY = clientY - tbHeight / 2;
// Clamp to viewport
if (targetY + tbHeight > scrollerRect.bottom) {
targetY = scrollerRect.bottom - tbHeight;
}
if (targetY < scrollerRect.top) {
targetY = scrollerRect.top;
}
btnContainer.style.top = `${targetY - parentRect.top}px`;
pinToViewportRight(btnContainer, scroller);
};
interface RenderedChunkControl {
chunkIndex: number;
chunkEl: HTMLElement;
toolbar: HTMLElement;
}
const getRenderedChunkControls = (view: EditorView): RenderedChunkControl[] => {
const toolbars = view.dom.querySelectorAll<HTMLElement>('.cm-merge-toolbar');
const controls: RenderedChunkControl[] = [];
toolbars.forEach((toolbar) => {
const chunkEl = toolbar.closest<HTMLElement>('.cm-deletedChunk');
if (!chunkEl) return;
const pos = view.posAtDOM(toolbar);
controls.push({
chunkIndex: computeHunkIndexAtPos(view.state, pos),
chunkEl,
toolbar,
});
});
return controls;
};
const resolveChunkIndexFromDeletedChunk = (
deletedChunk: Element,
view: EditorView
): number => {
const toolbar = deletedChunk.querySelector<HTMLElement>('.cm-merge-toolbar');
if (!toolbar) return -1;
return computeHunkIndexAtPos(view.state, view.posAtDOM(toolbar));
};
// Find which chunk index the pointer is directly over, including inline deleted text.
const findHoveredChunkIndex = (
clientX: number,
clientY: number,
view: EditorView
): number => {
const el = document.elementFromPoint(clientX, clientY);
if (!el) return -1;
const deletedChunk = el.closest('.cm-deletedChunk');
if (deletedChunk) {
return resolveChunkIndexFromDeletedChunk(deletedChunk, view);
}
if (
el.closest(
'.cm-changedLine, .cm-insertedLine, .cm-inlineChangedLine, .cm-changedText, .cm-deletedText'
)
) {
const allChunks = getChunks(view.state);
if (!allChunks) return -1;
const pos = view.posAtCoords({ x: clientX, y: clientY });
if (pos !== null) {
for (let i = 0; i < allChunks.chunks.length; i++) {
const chunk = allChunks.chunks[i];
if (pos >= chunk.fromB && pos <= chunk.toB) return i;
}
}
}
return -1;
};
const findNearestRenderedChunk = (
clientY: number,
renderedControls: RenderedChunkControl[]
): RenderedChunkControl | null => {
let bestMatch: RenderedChunkControl | null = null;
let bestDist = Infinity;
renderedControls.forEach((control) => {
const rect = control.chunkEl.getBoundingClientRect();
const centerY = (rect.top + rect.bottom) / 2;
const dist = Math.abs(clientY - centerY);
if (dist < bestDist) {
bestDist = dist;
bestMatch = control;
}
});
return bestMatch;
};
const updateActiveToolbar = (
view: EditorView,
clientY: number,
options?: { clientX?: number; followCursor?: boolean }
): void => {
const allChunks = getChunks(view.state);
if (!allChunks || allChunks.chunks.length === 0) return;
const renderedControls = getRenderedChunkControls(view);
if (renderedControls.length === 0) return;
const hoveredChunkIndex =
options?.clientX !== undefined
? findHoveredChunkIndex(options.clientX, clientY, view)
: -1;
const activeControl =
renderedControls.find((control) => control.chunkIndex === hoveredChunkIndex) ??
findNearestRenderedChunk(clientY, renderedControls);
renderedControls.forEach((control) => {
control.toolbar.classList.toggle(
'cm-merge-toolbar-active',
activeControl?.chunkIndex === control.chunkIndex
);
});
if (!activeControl) return;
if (options?.followCursor && hoveredChunkIndex >= 0) {
positionAtCursor(activeControl.chunkEl, clientY, view.scrollDOM);
} else {
positionAtBottom(activeControl.chunkEl, view.scrollDOM);
}
};
extensions.push( extensions.push(
EditorView.domEventHandlers({ EditorView.domEventHandlers({
mousemove(event, view) { mouseleave() {
lastPointerRef.current = { x: event.clientX, y: event.clientY };
updateActiveToolbar(view, event.clientY, {
clientX: event.clientX,
followCursor: true,
});
return false;
},
mouseleave(_event, view) {
lastPointerRef.current = null;
// Keep active toolbar visible, reposition to "below block"
const activeToolbar = view.dom.querySelector('.cm-merge-toolbar-active');
if (activeToolbar) {
const chunkEl = activeToolbar.closest('.cm-deletedChunk');
if (chunkEl) positionAtBottom(chunkEl, view.scrollDOM);
}
return false; return false;
}, },
scroll(_event, view) { scroll(_event, view) {
@ -665,7 +600,7 @@ export const CodeMirrorDiffView = ({
? pointer.y ? pointer.y
: (scrollerRect.top + scrollerRect.bottom) / 2; : (scrollerRect.top + scrollerRect.bottom) / 2;
updateActiveToolbar(view, targetY, { updateFloatingToolbar(view, targetY, {
clientX: pointerInsideScroller ? pointer.x : undefined, clientX: pointerInsideScroller ? pointer.x : undefined,
followCursor: Boolean(pointerInsideScroller), followCursor: Boolean(pointerInsideScroller),
}); });
@ -677,12 +612,10 @@ export const CodeMirrorDiffView = ({
// Ensure at least one toolbar is visible (initial load + after accept/reject) // Ensure at least one toolbar is visible (initial load + after accept/reject)
extensions.push( extensions.push(
EditorView.updateListener.of((update) => { EditorView.updateListener.of((update) => {
if (update.view.dom.querySelector('.cm-merge-toolbar-active')) return;
requestAnimationFrame(() => { requestAnimationFrame(() => {
const v = update.view; const v = update.view;
if (v.dom.querySelector('.cm-merge-toolbar-active')) return;
const scrollerRect = v.scrollDOM.getBoundingClientRect(); const scrollerRect = v.scrollDOM.getBoundingClientRect();
updateActiveToolbar(v, (scrollerRect.top + scrollerRect.bottom) / 2); updateFloatingToolbar(v, (scrollerRect.top + scrollerRect.bottom) / 2);
}); });
}) })
); );
@ -709,7 +642,15 @@ export const CodeMirrorDiffView = ({
); );
return extensions; return extensions;
}, [readOnly, showMergeControls, buildMergeExtension, usePortionCollapse, portionSize, original]); }, [
readOnly,
showMergeControls,
buildMergeExtension,
usePortionCollapse,
portionSize,
original,
updateFloatingToolbar,
]);
useEffect(() => { useEffect(() => {
if (!containerRef.current) return; if (!containerRef.current) return;
@ -739,6 +680,7 @@ export const CodeMirrorDiffView = ({
return () => { return () => {
clearTimeout(debounceTimer.current); clearTimeout(debounceTimer.current);
hideFloatingToolbar();
view.destroy(); view.destroy();
viewRef.current = null; viewRef.current = null;
if (extRef) { if (extRef) {
@ -748,7 +690,7 @@ export const CodeMirrorDiffView = ({
onViewChangeRef.current?.(null); onViewChangeRef.current?.(null);
}; };
// We intentionally rebuild the entire editor when key props change // We intentionally rebuild the entire editor when key props change
}, [original, modified, buildExtensions, initialState]); }, [original, modified, buildExtensions, initialState, hideFloatingToolbar]);
// Inject language extension via compartment after editor creation // Inject language extension via compartment after editor creation
useEffect(() => { useEffect(() => {
@ -824,8 +766,100 @@ export const CodeMirrorDiffView = ({
}, [onFullyViewed]); }, [onFullyViewed]);
return ( return (
<div className="flex flex-col" style={{ maxHeight }}> <div
ref={rootRef}
role="presentation"
className="relative flex flex-col"
style={{ maxHeight }}
onMouseMove={(e) => {
lastPointerRef.current = { x: e.clientX, y: e.clientY };
const view = viewRef.current;
if (!view) return;
updateFloatingToolbar(view, e.clientY, {
clientX: e.clientX,
followCursor: true,
});
}}
onMouseLeave={() => {
lastPointerRef.current = null;
hideFloatingToolbar();
}}
>
<div ref={containerRef} className="flex-1 overflow-hidden rounded-lg border border-border" /> <div ref={containerRef} className="flex-1 overflow-hidden rounded-lg border border-border" />
{showMergeControls && (
<div
ref={floatingToolbarRef}
data-review-floating-toolbar="true"
className="pointer-events-none absolute z-20 hidden items-center gap-0.5"
>
<div
ref={floatingNavRef}
className="pointer-events-auto flex items-center overflow-hidden rounded-md border border-border bg-surface-raised"
style={{ display: 'none' }}
>
<button
type="button"
className="px-2 py-[3px] text-[13px] leading-5 text-text-secondary transition-colors hover:bg-[var(--diff-merge-nav-hover-bg)]"
title="Previous chunk"
onMouseDown={(e) => {
e.preventDefault();
moveBetweenChunks('prev');
}}
>
{'\u2227'}
</button>
<span
ref={floatingCounterRef}
className="whitespace-nowrap px-1 text-xs text-text-secondary"
/>
<button
type="button"
className="px-2 py-[3px] text-[13px] leading-5 text-text-secondary transition-colors hover:bg-[var(--diff-merge-nav-hover-bg)]"
title="Next chunk"
onMouseDown={(e) => {
e.preventDefault();
moveBetweenChunks('next');
}}
>
{'\u2228'}
</button>
</div>
<button
type="button"
className="pointer-events-auto rounded px-2.5 py-[3px] text-xs font-medium leading-5 transition-colors hover:[background-color:var(--diff-merge-undo-hover-bg)]"
style={{
color: 'var(--diff-merge-undo-color)',
backgroundColor: 'var(--diff-merge-undo-bg)',
border: '1px solid var(--diff-merge-undo-border)',
}}
title="Reject change (⌘N)"
onMouseDown={(e) => {
e.preventDefault();
actOnActiveChunk('reject');
}}
>
{'Undo '}
<kbd className="ml-1 text-[10px] text-text-muted">{'\u2318N'}</kbd>
</button>
<button
type="button"
className="pointer-events-auto rounded px-2.5 py-[3px] text-xs font-medium leading-5 transition-colors hover:[background-color:var(--diff-merge-keep-hover-bg)]"
style={{
color: 'var(--diff-merge-keep-color)',
backgroundColor: 'var(--diff-merge-keep-bg)',
border: '1px solid var(--diff-merge-keep-border)',
}}
title="Accept change (⌘Y)"
onMouseDown={(e) => {
e.preventDefault();
actOnActiveChunk('accept');
}}
>
{'Keep '}
<kbd className="ml-1 text-[10px] text-[var(--diff-merge-keep-kbd)]">{'\u2318Y'}</kbd>
</button>
</div>
)}
{/* Invisible sentinel for auto-viewed detection */} {/* Invisible sentinel for auto-viewed detection */}
<div ref={endSentinelRef} className="h-px shrink-0" /> <div ref={endSentinelRef} className="h-px shrink-0" />
</div> </div>

View file

@ -60,6 +60,8 @@ interface ContinuousScrollViewProps {
filePath: string filePath: string
) => Promise<void>; ) => Promise<void>;
onSelectionChange?: (info: EditorSelectionInfo | null) => void; onSelectionChange?: (info: EditorSelectionInfo | null) => void;
globalHunkOffsets?: Record<string, number>;
totalReviewHunks?: number;
} }
export const ContinuousScrollView = ({ export const ContinuousScrollView = ({
@ -95,6 +97,8 @@ export const ContinuousScrollView = ({
memberName, memberName,
fetchFileContent, fetchFileContent,
onSelectionChange, onSelectionChange,
globalHunkOffsets,
totalReviewHunks,
}: ContinuousScrollViewProps): React.ReactElement => { }: ContinuousScrollViewProps): React.ReactElement => {
const setFileChunkCount = useStore((s) => s.setFileChunkCount); const setFileChunkCount = useStore((s) => s.setFileChunkCount);
const [localCollapsedFiles, setLocalCollapsedFiles] = useState<Set<string>>(() => new Set()); const [localCollapsedFiles, setLocalCollapsedFiles] = useState<Set<string>>(() => new Set());
@ -268,6 +272,8 @@ export const ContinuousScrollView = ({
autoViewed={autoViewed} autoViewed={autoViewed}
isViewed={isViewed} isViewed={isViewed}
onSelectionChange={onSelectionChange} onSelectionChange={onSelectionChange}
globalHunkOffset={globalHunkOffsets?.[filePath] ?? 0}
totalReviewHunks={totalReviewHunks}
/> />
) : ( ) : (
<FileSectionPlaceholder fileName={file.relativePath} /> <FileSectionPlaceholder fileName={file.relativePath} />

View file

@ -24,6 +24,8 @@ interface FileSectionDiffProps {
autoViewed: boolean; autoViewed: boolean;
isViewed: boolean; isViewed: boolean;
onSelectionChange?: (info: EditorSelectionInfo | null) => void; onSelectionChange?: (info: EditorSelectionInfo | null) => void;
globalHunkOffset?: number;
totalReviewHunks?: number;
} }
export const FileSectionDiff = ({ export const FileSectionDiff = ({
@ -40,6 +42,8 @@ export const FileSectionDiff = ({
autoViewed, autoViewed,
isViewed, isViewed,
onSelectionChange, onSelectionChange,
globalHunkOffset = 0,
totalReviewHunks,
}: FileSectionDiffProps): React.ReactElement => { }: FileSectionDiffProps): React.ReactElement => {
const localEditorViewRef = useRef<EditorView | null>(null); const localEditorViewRef = useRef<EditorView | null>(null);
const sentinelRef = useRef<HTMLDivElement>(null); const sentinelRef = useRef<HTMLDivElement>(null);
@ -148,6 +152,8 @@ export const FileSectionDiff = ({
? (info) => onSelectionChange(info ? { ...info, filePath: file.filePath } : null) ? (info) => onSelectionChange(info ? { ...info, filePath: file.filePath } : null)
: undefined : undefined
} }
globalHunkOffset={globalHunkOffset}
totalReviewHunks={totalReviewHunks}
/> />
</DiffErrorBoundary> </DiffErrorBoundary>
<div ref={sentinelRef} className="h-1 shrink-0" /> <div ref={sentinelRef} className="h-1 shrink-0" />