From b76be0f6342ff2a6faa50a1976c740042b219a5c Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 13 Mar 2026 17:02:38 +0200 Subject: [PATCH] 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. --- .../services/team/TeamProvisioningService.ts | 11 +- .../team/review/ChangeReviewDialog.tsx | 18 +- .../team/review/CodeMirrorDiffView.tsx | 648 +++++++++--------- .../team/review/ContinuousScrollView.tsx | 6 + .../team/review/FileSectionDiff.tsx | 6 + 5 files changed, 373 insertions(+), 316 deletions(-) diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 5b16cbc3..889bae9d 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -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. 9. When sending a message about a specific task, include its short display label like # 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"). -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). -12. Review workflow clarity (IMPORTANT): +11. Review workflow clarity (IMPORTANT): - 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). - 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: a) Owner finishes work on #X -> task_complete #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: a) STEP 1 — FIRST, set the clarification flag with MCP tool task_set_clarification: { teamName: "${teamName}", 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: { teamName: "${teamName}", taskId: "", value: "clear" } 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 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. - 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. @@ -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. 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} ${membersFooter}`; diff --git a/src/renderer/components/team/review/ChangeReviewDialog.tsx b/src/renderer/components/team/review/ChangeReviewDialog.tsx index ed3a0d12..b0f09a6d 100644 --- a/src/renderer/components/team/review/ChangeReviewDialog.tsx +++ b/src/renderer/components/team/review/ChangeReviewDialog.tsx @@ -10,10 +10,10 @@ import { useViewedFiles } from '@renderer/hooks/useViewedFiles'; import { cn } from '@renderer/lib/utils'; import { useStore } from '@renderer/store'; import { getFileHunkCount, REVIEW_INSTANT_APPLY } from '@renderer/store/slices/changeReviewSlice'; -import { type TaskChangeRequestOptions } from '@renderer/utils/taskChangeRequest'; import { buildSelectionAction } from '@renderer/utils/buildSelectionAction'; import { buildSelectionInfo, SELECTION_DEBOUNCE_MS } from '@renderer/utils/codemirrorSelectionInfo'; import { sortItemsAsTree } from '@renderer/utils/fileTreeBuilder'; +import { type TaskChangeRequestOptions } from '@renderer/utils/taskChangeRequest'; import { ChevronDown, Clock, X } from 'lucide-react'; import { acceptAllChunks, computeChunkIndexAtPos, rejectAllChunks } from './CodeMirrorDiffUtils'; @@ -34,11 +34,11 @@ import type { } from '@shared/types'; import type { EditorSelectionAction, EditorSelectionInfo } from '@shared/types/editor'; -type RecentHunkUndoAction = { +interface RecentHunkUndoAction { filePath: string; originalIndex: number; at: number; -}; +} interface ChangeReviewDialogProps { open: boolean; @@ -648,6 +648,16 @@ export const ChangeReviewDialog = ({ getFileHunkCount(filePath, fallbackSnippetsLength, fileChunkCounts) ); + const reviewHunkOrder = useMemo(() => { + const offsets: Record = {}; + 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) => { setCollapsedFiles((prev) => { const next = new Set(prev); @@ -1234,6 +1244,8 @@ export const ChangeReviewDialog = ({ memberName={memberName} fetchFileContent={fetchFileContent} onSelectionChange={onEditorAction ? handleSelectionChange : undefined} + globalHunkOffsets={reviewHunkOrder.offsets} + totalReviewHunks={reviewHunkOrder.total} /> {selectionInfo && onEditorAction && ( 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). @@ -200,11 +204,18 @@ export const CodeMirrorDiffView = ({ usePortionCollapse = false, portionSize = 100, onSelectionChange, + globalHunkOffset = 0, + totalReviewHunks, }: CodeMirrorDiffViewProps): React.ReactElement => { + const rootRef = useRef(null); const containerRef = useRef(null); const viewRef = useRef(null); const endSentinelRef = useRef(null); const lastPointerRef = useRef<{ x: number; y: number } | null>(null); + const floatingToolbarRef = useRef(null); + const floatingNavRef = useRef(null); + const floatingCounterRef = useRef(null); + const activeChunkIndexRef = useRef(null); // Local ref to hold externalViewRef for syncing via useEffect const externalViewRefHolder = useRef(externalViewRef); @@ -251,6 +262,225 @@ export const CodeMirrorDiffView = ({ collapseRef.current = { enabled: collapseUnchangedProp, margin: 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. */ const buildMergeExtension = useCallback( (collapse: boolean, margin: number): Extension => { @@ -261,12 +491,8 @@ export const CodeMirrorDiffView = ({ syntaxHighlightDeletions: true, }; - // IMPORTANT: @codemirror/merge shows accept/reject buttons by default. - // When our UI chooses to hide merge controls (e.g. "Missing on disk" preview), - // explicitly disable them rather than relying on default behavior. - if (!showMergeControls) { - mergeConfig.mergeControls = false; - } + // We render our own floating merge toolbar outside CodeMirror's DeletionWidget DOM. + mergeConfig.mergeControls = false; if (collapse && !usePortionCollapse) { 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); }, - [original, showMergeControls, scrollToNextChunk, usePortionCollapse] + [original, usePortionCollapse] ); 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) { - // 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('.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('.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('.cm-merge-toolbar'); - const controls: RenderedChunkControl[] = []; - toolbars.forEach((toolbar) => { - const chunkEl = toolbar.closest('.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('.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( EditorView.domEventHandlers({ - mousemove(event, view) { - 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); - } + mouseleave() { return false; }, scroll(_event, view) { @@ -665,7 +600,7 @@ export const CodeMirrorDiffView = ({ ? pointer.y : (scrollerRect.top + scrollerRect.bottom) / 2; - updateActiveToolbar(view, targetY, { + updateFloatingToolbar(view, targetY, { clientX: pointerInsideScroller ? pointer.x : undefined, followCursor: Boolean(pointerInsideScroller), }); @@ -677,12 +612,10 @@ export const CodeMirrorDiffView = ({ // Ensure at least one toolbar is visible (initial load + after accept/reject) extensions.push( EditorView.updateListener.of((update) => { - if (update.view.dom.querySelector('.cm-merge-toolbar-active')) return; requestAnimationFrame(() => { const v = update.view; - if (v.dom.querySelector('.cm-merge-toolbar-active')) return; 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; - }, [readOnly, showMergeControls, buildMergeExtension, usePortionCollapse, portionSize, original]); + }, [ + readOnly, + showMergeControls, + buildMergeExtension, + usePortionCollapse, + portionSize, + original, + updateFloatingToolbar, + ]); useEffect(() => { if (!containerRef.current) return; @@ -739,6 +680,7 @@ export const CodeMirrorDiffView = ({ return () => { clearTimeout(debounceTimer.current); + hideFloatingToolbar(); view.destroy(); viewRef.current = null; if (extRef) { @@ -748,7 +690,7 @@ export const CodeMirrorDiffView = ({ onViewChangeRef.current?.(null); }; // 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 useEffect(() => { @@ -824,8 +766,100 @@ export const CodeMirrorDiffView = ({ }, [onFullyViewed]); return ( -
+
{ + 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(); + }} + >
+ {showMergeControls && ( +
+
+ + + +
+ + +
+ )} {/* Invisible sentinel for auto-viewed detection */}
diff --git a/src/renderer/components/team/review/ContinuousScrollView.tsx b/src/renderer/components/team/review/ContinuousScrollView.tsx index 912baab6..fde5a09f 100644 --- a/src/renderer/components/team/review/ContinuousScrollView.tsx +++ b/src/renderer/components/team/review/ContinuousScrollView.tsx @@ -60,6 +60,8 @@ interface ContinuousScrollViewProps { filePath: string ) => Promise; onSelectionChange?: (info: EditorSelectionInfo | null) => void; + globalHunkOffsets?: Record; + totalReviewHunks?: number; } export const ContinuousScrollView = ({ @@ -95,6 +97,8 @@ export const ContinuousScrollView = ({ memberName, fetchFileContent, onSelectionChange, + globalHunkOffsets, + totalReviewHunks, }: ContinuousScrollViewProps): React.ReactElement => { const setFileChunkCount = useStore((s) => s.setFileChunkCount); const [localCollapsedFiles, setLocalCollapsedFiles] = useState>(() => new Set()); @@ -268,6 +272,8 @@ export const ContinuousScrollView = ({ autoViewed={autoViewed} isViewed={isViewed} onSelectionChange={onSelectionChange} + globalHunkOffset={globalHunkOffsets?.[filePath] ?? 0} + totalReviewHunks={totalReviewHunks} /> ) : ( diff --git a/src/renderer/components/team/review/FileSectionDiff.tsx b/src/renderer/components/team/review/FileSectionDiff.tsx index 324316ff..86f1fd7f 100644 --- a/src/renderer/components/team/review/FileSectionDiff.tsx +++ b/src/renderer/components/team/review/FileSectionDiff.tsx @@ -24,6 +24,8 @@ interface FileSectionDiffProps { autoViewed: boolean; isViewed: boolean; onSelectionChange?: (info: EditorSelectionInfo | null) => void; + globalHunkOffset?: number; + totalReviewHunks?: number; } export const FileSectionDiff = ({ @@ -40,6 +42,8 @@ export const FileSectionDiff = ({ autoViewed, isViewed, onSelectionChange, + globalHunkOffset = 0, + totalReviewHunks, }: FileSectionDiffProps): React.ReactElement => { const localEditorViewRef = useRef(null); const sentinelRef = useRef(null); @@ -148,6 +152,8 @@ export const FileSectionDiff = ({ ? (info) => onSelectionChange(info ? { ...info, filePath: file.filePath } : null) : undefined } + globalHunkOffset={globalHunkOffset} + totalReviewHunks={totalReviewHunks} />