diff --git a/package-lock.json b/package-lock.json index 9d0df41..e548f36 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,6 +8,7 @@ "name": "merge-nb", "version": "0.0.3", "dependencies": { + "@tanstack/react-virtual": "^3.13.4", "@vscode/markdown-it-katex": "^1.1.2", "dompurify": "^3.3.1", "katex": "^0.16.22", @@ -858,6 +859,33 @@ "node": ">=18" } }, + "node_modules/@tanstack/react-virtual": { + "version": "3.13.18", + "resolved": "https://registry.npmjs.org/@tanstack/react-virtual/-/react-virtual-3.13.18.tgz", + "integrity": "sha512-dZkhyfahpvlaV0rIKnvQiVoWPyURppl6w4m9IwMDpuIjcJ1sD9YGWrt0wISvgU7ewACXx2Ct46WPgI6qAD4v6A==", + "license": "MIT", + "dependencies": { + "@tanstack/virtual-core": "3.13.18" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + }, + "peerDependencies": { + "react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0", + "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0" + } + }, + "node_modules/@tanstack/virtual-core": { + "version": "3.13.18", + "resolved": "https://registry.npmjs.org/@tanstack/virtual-core/-/virtual-core-3.13.18.tgz", + "integrity": "sha512-Mx86Hqu1k39icq2Zusq+Ey2J6dDWTjDvEv43PJtRCoEYTLyfaPnxIQ6iy7YAOK0NV/qOEmZQ/uCufrppZxTgcg==", + "license": "MIT", + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + } + }, "node_modules/@tsconfig/node10": { "version": "1.0.12", "resolved": "https://registry.npmjs.org/@tsconfig/node10/-/node10-1.0.12.tgz", @@ -956,7 +984,6 @@ "integrity": "sha512-BkmoP5/FhRYek5izySdkOneRyXYN35I860MFAGupTdebyE66uZaR+bXLHq8k4DirE5DwQi3NuhvRU1jqTVwUrQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -967,7 +994,6 @@ "integrity": "sha512-KkiJeU6VbYbUOp5ITMIc7kBfqlYkKA5KhEHVrGMmUUMt7NeaZg65ojdPk+FtNrBAOXNVM5QM72jnADjM+XVRAQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -1051,7 +1077,6 @@ "integrity": "sha512-4z2nCSBfVIMnbuu8uinj+f0o4qOeggYJLbjpPHka3KH1om7e+H9yLKTYgksTaHcGco+NClhhY2vyO3HsMH1RGw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.55.0", "@typescript-eslint/types": "8.55.0", @@ -1293,7 +1318,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2280,7 +2304,6 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -4701,9 +4724,7 @@ "version": "19.2.4", "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", - "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -4712,7 +4733,6 @@ "version": "19.2.4", "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.4.tgz", "integrity": "sha512-AXJdLo8kgMbimY95O2aKQqsz2iWi9jMgKJhRBAxECE4IFxfcazB2LmzloIoibJI3C12IlY20+KFaLv+71bUJeQ==", - "dev": true, "license": "MIT", "dependencies": { "scheduler": "^0.27.0" @@ -4947,7 +4967,6 @@ "version": "0.27.0", "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.27.0.tgz", "integrity": "sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q==", - "dev": true, "license": "MIT" }, "node_modules/semver": { @@ -5535,7 +5554,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -5720,7 +5738,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" diff --git a/package.json b/package.json index 0ec2cf7..493272d 100644 --- a/package.json +++ b/package.json @@ -155,6 +155,7 @@ "typescript-eslint": "^8.55.0" }, "dependencies": { + "@tanstack/react-virtual": "^3.13.4", "@vscode/markdown-it-katex": "^1.1.2", "dompurify": "^3.3.1", "katex": "^0.16.22", diff --git a/src/web/client/CellContent.tsx b/src/web/client/CellContent.tsx index f85d286..0929e7e 100644 --- a/src/web/client/CellContent.tsx +++ b/src/web/client/CellContent.tsx @@ -10,9 +10,6 @@ import { renderMarkdown } from './markdown'; import { computeLineDiff, type DiffLine } from '../../diffUtils'; import DOMPurify from 'dompurify'; -// Performance tuning constants -const LAZY_PREVIEW_LENGTH = 100; // Characters to show in lazy-loaded markdown preview - interface CellContentProps { cell: NotebookCell | undefined; cellIndex?: number; @@ -24,10 +21,9 @@ interface CellContentProps { showOutputs?: boolean; onDragStart?: (e: React.DragEvent) => void; onDragEnd?: () => void; - isVisible?: boolean; // For lazy rendering optimization } -export function CellContent({ +export function CellContentInner({ cell, cellIndex, side, @@ -38,7 +34,6 @@ export function CellContent({ showOutputs = true, onDragStart, onDragEnd, - isVisible = true, }: CellContentProps): React.ReactElement { if (!cell) { return ( @@ -50,7 +45,7 @@ export function CellContent({ const source = normalizeCellSource(cell.source); const cellType = cell.cell_type; - const encodedCell = encodeURIComponent(JSON.stringify(cell)); + const encodedCell = useMemo(() => encodeURIComponent(JSON.stringify(cell)), [cell]); const cellClasses = [ 'notebook-cell', @@ -58,28 +53,6 @@ export function CellContent({ isConflict && 'has-conflict' ].filter(Boolean).join(' '); - // For non-visible cells, render a minimal placeholder to maintain layout. - // Keep data attributes + drag handlers so tests and drag/drop remain stable. - if (!isVisible && cellType === 'markdown') { - return ( -
-
-
-
{source.length > LAZY_PREVIEW_LENGTH ? `${source.substring(0, LAZY_PREVIEW_LENGTH)}...` : source}
-
-
-
- ); - } - return (
{showOutputs && cellType === 'code' && cell.outputs && cell.outputs.length > 0 && ( - + )}
); @@ -115,7 +88,6 @@ interface MarkdownContentProps { } function MarkdownContent({ source }: MarkdownContentProps): React.ReactElement { - // Memoize HTML rendering to avoid re-parsing on every render const html = useMemo(() => renderMarkdown(source), [source]); return ( @@ -134,7 +106,7 @@ interface DiffContentProps { } function DiffContent({ source, compareSource, side, diffMode }: DiffContentProps): React.ReactElement { - const diff = computeLineDiff(compareSource, source); + const diff = useMemo(() => computeLineDiff(compareSource, source), [compareSource, source]); // Use the right side for display (shows the "new" content with change markers) const diffLines = diff.right; // Filter out empty alignment lines to avoid unnecessary whitespace @@ -236,14 +208,11 @@ function isWhitespaceOnlyLineChange(line: DiffLine): boolean { interface CellOutputsProps { outputs: CellOutput[]; - isVisible?: boolean; } -function CellOutputs({ outputs, isVisible = true }: CellOutputsProps): React.ReactElement { - // Always render the actual outputs to prevent size changes that cause flickering. - // Use CSS visibility/opacity for performance optimization instead of conditional rendering. +function CellOutputs({ outputs }: CellOutputsProps): React.ReactElement { return ( -
+
{outputs.map((output, i) => ( ))} @@ -260,12 +229,14 @@ function OutputItem({ output }: { output: CellOutput }): React.ReactElement | nu if ((output.output_type === 'display_data' || output.output_type === 'execute_result') && output.data) { const data = output.data; - // Try image first + // Use text placeholders for images instead of rendering actual tags. + // This prevents browser decoding overhead, ResizeObserver feedback loops, + // and flickering from invalid/broken image data. if (data['image/png']) { - return output; + return ; } if (data['image/jpeg']) { - return output; + return ; } // HTML @@ -289,3 +260,26 @@ function OutputItem({ output }: { output: CellOutput }): React.ReactElement | nu return null; } + +/** + * Renders a text placeholder for an image output. + * Uses markdown-style format to provide a consistent, stable representation. + * + * @param mimeType - Currently only 'image/png' or 'image/jpeg' are supported and passed by OutputItem + */ +function ImagePlaceholder({ mimeType }: { mimeType: string }): React.ReactElement { + const placeholderText = `![Image: ${mimeType}]`; + // Convert MIME type to user-friendly label for screen readers + const imageType = mimeType === 'image/png' ? 'PNG' : 'JPEG'; + return ( +
+ {placeholderText} +
+ ); +} + +export const CellContent = React.memo(CellContentInner); diff --git a/src/web/client/ConflictResolver.tsx b/src/web/client/ConflictResolver.tsx index db30a9d..7a88770 100644 --- a/src/web/client/ConflictResolver.tsx +++ b/src/web/client/ConflictResolver.tsx @@ -3,7 +3,8 @@ * @description Main React component for the conflict resolution UI. */ -import React, { useState, useCallback, useMemo, useRef, useEffect } from 'react'; +import React, { useState, useRef, useEffect, useCallback, useMemo } from 'react'; +import { useVirtualizer } from '@tanstack/react-virtual'; import { sortByPosition } from '../../positionUtils'; import { normalizeCellSource } from '../../notebookUtils'; import type { @@ -18,12 +19,6 @@ import type { } from './types'; import { MergeRow } from './MergeRow'; -// Virtualization constants -const INITIAL_VISIBLE_ROWS = 20; // Number of rows to render initially -const DEFAULT_ROW_HEIGHT = 200; // Default height for unmeasured rows (fallback) -const VIRTUALIZATION_OVERSCAN_ROWS = 5; // Number of rows to render outside viewport for smooth scrolling -const RESIZE_OBSERVER_DEBOUNCE_MS = 150; // Debounce ResizeObserver updates to prevent rapid state changes -const HEIGHT_CHANGE_THRESHOLD = 5; // Minimum height change (in pixels) to trigger an update const INITIAL_MARK_AS_RESOLVED = true; const INITIAL_RENUMBER_EXECUTION_COUNTS = true; @@ -86,12 +81,9 @@ export function ConflictResolver({ onResolve, onCancel, }: ConflictResolverProps): React.ReactElement { - const initialRows = useMemo(() => { - if (conflict.type === 'semantic' && conflict.semanticConflict) { - return buildMergeRowsFromSemantic(conflict.semanticConflict); - } - return []; - }, [conflict]); + const initialRows = conflict.type === 'semantic' && conflict.semanticConflict + ? buildMergeRowsFromSemantic(conflict.semanticConflict) + : []; const [choices, setChoices] = useState>(new Map()); const [markAsResolved, setMarkAsResolved] = useState(INITIAL_MARK_AS_RESOLVED); @@ -111,17 +103,7 @@ export function ConflictResolver({ })); const [historyOpen, setHistoryOpen] = useState(false); const historyMenuRef = useRef(null); - - // Virtualization state const mainContentRef = useRef(null); - const [visibleRange, setVisibleRange] = useState({ start: 0, end: INITIAL_VISIBLE_ROWS }); - const [scrollTop, setScrollTop] = useState(0); - - // Track actual row heights using ResizeObserver - const [rowHeights, setRowHeights] = useState>(new Map()); - const rowRefs = useRef>(new Map()); - const resizeObserver = useRef(null); - const previousRowsLength = useRef(rows.length); // Cell-level drag state (for dragging cells between/into rows) const [draggedCell, setDraggedCell] = useState(null); @@ -137,6 +119,7 @@ export function ConflictResolver({ const rowsRef = useRef(rows); const markAsResolvedRef = useRef(markAsResolved); const renumberExecutionCountsRef = useRef(renumberExecutionCounts); + const historyRef = useRef(history); useEffect(() => { choicesRef.current = choices; @@ -154,6 +137,10 @@ export function ConflictResolver({ renumberExecutionCountsRef.current = renumberExecutionCounts; }, [renumberExecutionCounts]); + useEffect(() => { + historyRef.current = history; + }, [history]); + const recordHistory = useCallback(( label: string, nextChoices: Map, @@ -209,182 +196,6 @@ export function ConflictResolver({ const isMac = useMemo(() => /Mac|iPod|iPhone|iPad/.test(navigator.platform), []); const undoShortcutLabel = isMac ? 'Cmd+Z' : 'Ctrl+Z'; const redoShortcutLabel = isMac ? 'Cmd+Shift+Z' : 'Ctrl+Shift+Z'; - - // Helper to get row height (actual or default) - const getRowHeight = useCallback((index: number): number => { - return rowHeights.get(index) ?? DEFAULT_ROW_HEIGHT; - }, [rowHeights]); - - // Pre-calculate cumulative heights to optimize scrolling performance (O(1) lookups vs O(N)) - // cumulativeHeights[i] = cumulative height BEFORE row i starts (i.e. top position of row i) - // cumulativeHeights[rows.length] = total height - const cumulativeHeights = useMemo(() => { - const heights = new Array(rows.length + 1).fill(0); - let current = 0; - for (let i = 0; i < rows.length; i++) { - heights[i] = current; - current += (rowHeights.get(i) ?? DEFAULT_ROW_HEIGHT); - } - heights[rows.length] = current; - return heights; - }, [rows.length, rowHeights]); - - // Get total content height - const getTotalHeight = useCallback((): number => { - return cumulativeHeights[rows.length]; - }, [cumulativeHeights, rows.length]); - - // Find row index at a given scroll position using binary search - const getRowIndexAtPosition = useCallback((scrollPos: number): number => { - if (rows.length === 0) return 0; - - // Binary search for the row where cumulativeHeights[i] <= scrollPos < cumulativeHeights[i+1] - let low = 0; - let high = rows.length - 1; - - while (low <= high) { - const mid = Math.floor((low + high) / 2); - const rowTop = cumulativeHeights[mid]; - const rowBottom = cumulativeHeights[mid + 1]; - - if (scrollPos >= rowTop && scrollPos < rowBottom) { - return mid; - } else if (scrollPos < rowTop) { - high = mid - 1; - } else { - low = mid + 1; - } - } - - // Fallback for out of bounds (should return last row if scrolled way past) - if (scrollPos >= cumulativeHeights[rows.length]) return rows.length - 1; - return 0; - }, [cumulativeHeights, rows.length]); - - // Setup ResizeObserver to track actual row heights - useEffect(() => { - let debounceTimer: NodeJS.Timeout | null = null; - let pendingUpdates = new Map(); - - resizeObserver.current = new ResizeObserver((entries) => { - // Collect all pending updates - for (const entry of entries) { - const element = entry.target as HTMLDivElement; - const rowIndex = parseInt(element.dataset.rowIndex ?? '-1', 10); - if (rowIndex >= 0) { - const newHeight = entry.contentRect.height; - if (newHeight > 0) { - pendingUpdates.set(rowIndex, newHeight); - } - } - } - - // Clear existing timer and set a new one - if (debounceTimer) { - clearTimeout(debounceTimer); - } - - debounceTimer = setTimeout(() => { - setRowHeights(prev => { - const newHeights = new Map(prev); - let hasChanges = false; - - for (const [rowIndex, newHeight] of pendingUpdates.entries()) { - const currentHeight = newHeights.get(rowIndex); - // Only update if the height change is significant (above threshold) - if (currentHeight === undefined || Math.abs(currentHeight - newHeight) >= HEIGHT_CHANGE_THRESHOLD) { - newHeights.set(rowIndex, newHeight); - hasChanges = true; - } - } - - pendingUpdates.clear(); - return hasChanges ? newHeights : prev; - }); - }, RESIZE_OBSERVER_DEBOUNCE_MS); - }); - - return () => { - if (debounceTimer) { - clearTimeout(debounceTimer); - } - resizeObserver.current?.disconnect(); - }; - }, []); // Only create observer once - - // Callback to register row refs with ResizeObserver - const registerRowRef = useCallback((index: number, element: HTMLDivElement | null) => { - const prevElement = rowRefs.current.get(index); - - if (prevElement && prevElement !== element) { - resizeObserver.current?.unobserve(prevElement); - rowRefs.current.delete(index); - } - - if (element) { - rowRefs.current.set(index, element); - resizeObserver.current?.observe(element); - } - }, []); - - const getRefCallback = useCallback((index: number) => (element: HTMLDivElement | null) => { - registerRowRef(index, element); - }, [registerRowRef]); - - // Adjust scroll position when rows are deleted to prevent "black spots" - useEffect(() => { - const prevLength = previousRowsLength.current; - const currentLength = rows.length; - - if (currentLength < prevLength && mainContentRef.current) { - // Rows were deleted - adjust scroll position - const element = mainContentRef.current; - const viewportHeight = element.clientHeight; - const totalHeight = getTotalHeight(); - - // If scroll position is now beyond content, scroll to end - if (element.scrollTop > totalHeight - viewportHeight) { - element.scrollTop = Math.max(0, totalHeight - viewportHeight); - } - - // Clean up height measurements for deleted rows - setRowHeights(new Map()); - } - - previousRowsLength.current = currentLength; - }, [rows.length, getTotalHeight]); - - // Handle scroll for virtualization using actual heights - useEffect(() => { - const handleScroll = () => { - if (!mainContentRef.current) return; - - const currentScrollTop = mainContentRef.current.scrollTop; - const viewportHeight = mainContentRef.current.clientHeight; - - // Find start index using actual heights - const rawStartIndex = getRowIndexAtPosition(currentScrollTop); - const startIndex = Math.max(0, rawStartIndex - VIRTUALIZATION_OVERSCAN_ROWS); - - // Find end index using actual heights - const viewportEndPos = currentScrollTop + viewportHeight; - const rawEndIndex = getRowIndexAtPosition(viewportEndPos); - const endIndex = Math.min(rows.length, rawEndIndex + VIRTUALIZATION_OVERSCAN_ROWS + 1); - - setVisibleRange({ start: startIndex, end: endIndex }); - setScrollTop(currentScrollTop); - }; - - const element = mainContentRef.current; - if (element) { - element.addEventListener('scroll', handleScroll); - // Initial calculation - handleScroll(); - - return () => element.removeEventListener('scroll', handleScroll); - } - }, [rows.length, getRowIndexAtPosition]); - useEffect(() => { if (!historyOpen) return; @@ -437,16 +248,17 @@ export function ConflictResolver({ const handleCommitContent = useCallback((index: number) => { const current = choicesRef.current.get(index); if (!current) return; - const lastSnapshot = history.entries[history.index]?.snapshot; + const h = historyRef.current; + const lastSnapshot = h.entries[h.index]?.snapshot; const lastChoice = lastSnapshot?.choices.get(index); if (lastChoice && lastChoice.resolvedContent === current.resolvedContent && lastChoice.choice === current.choice) { return; } recordHistory(`Edit conflict ${index + 1}`, choicesRef.current, rowsRef.current); - }, [history, recordHistory]); + }, [recordHistory]); /** Handle "Accept All" action */ - const handleAcceptAll = useCallback((choice: 'base' | 'current' | 'incoming') => { + const handleAcceptAll = (choice: 'base' | 'current' | 'incoming') => { const next = new Map(choicesRef.current); let didChange = false; conflictRows.forEach(row => { @@ -477,9 +289,9 @@ export function ConflictResolver({ if (!didChange) return; setChoices(next); recordHistory(`Accept all ${choice}`, next, rowsRef.current); - }, [conflictRows, recordHistory]); + }; - const handleToggleRenumberExecutionCounts = useCallback((checked: boolean) => { + const handleToggleRenumberExecutionCounts = (checked: boolean) => { if (checked === renumberExecutionCountsRef.current) return; setRenumberExecutionCounts(checked); recordHistory( @@ -488,9 +300,9 @@ export function ConflictResolver({ rowsRef.current, { renumberExecutionCounts: checked } ); - }, [recordHistory]); + }; - const handleToggleMarkAsResolved = useCallback((checked: boolean) => { + const handleToggleMarkAsResolved = (checked: boolean) => { if (checked === markAsResolvedRef.current) return; setMarkAsResolved(checked); recordHistory( @@ -499,16 +311,16 @@ export function ConflictResolver({ rowsRef.current, { markAsResolved: checked } ); - }, [recordHistory]); + }; - const handleJumpToHistory = useCallback((targetIndex: number) => { + const handleJumpToHistory = (targetIndex: number) => { setHistory(prev => { if (targetIndex === prev.index) return prev; if (targetIndex < 0 || targetIndex >= prev.entries.length) return prev; applySnapshot(prev.entries[targetIndex].snapshot); return { ...prev, index: targetIndex }; }); - }, [applySnapshot]); + }; const handleUndo = useCallback(() => { setHistory(prev => { @@ -548,9 +360,9 @@ export function ConflictResolver({ window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); - }, [enableUndoRedoHotkeys, handleRedo, handleUndo, isEditableTarget, isMac]); + }, [enableUndoRedoHotkeys, handleRedo, handleUndo, isMac, isEditableTarget]); - const handleResolve = useCallback(() => { + const handleResolve = () => { const resolutions: ConflictChoice[] = []; for (const [index, state] of choices) { resolutions.push({ @@ -580,7 +392,7 @@ export function ConflictResolver({ }); onResolve(resolutions, markAsResolved, renumberExecutionCounts, resolvedRows); - }, [choices, markAsResolved, renumberExecutionCounts, onResolve, rows]); + }; // Cell drag handlers const handleCellDragStart = useCallback((rowIndex: number, side: 'base' | 'current' | 'incoming', cell: NotebookCell) => { @@ -685,7 +497,7 @@ export function ConflictResolver({ setDropRowIndex(null); }, []); - const handleRowDragOver = useCallback((e: React.DragEvent, targetIndex: number) => { + const handleRowDragOver = (e: React.DragEvent, targetIndex: number) => { e.preventDefault(); const dragged = draggedRowIndexRef.current; if (dragged === null || dragged === targetIndex) { @@ -693,9 +505,9 @@ export function ConflictResolver({ } // Only update if target changed (prevent excessive re-renders) setDropRowIndex(prev => prev === targetIndex ? prev : targetIndex); - }, []); + }; - const handleRowDrop = useCallback((targetIndex: number) => { + const handleRowDrop = (targetIndex: number) => { const dragged = draggedRowIndexRef.current; if (dragged === null || dragged === targetIndex) { draggedRowIndexRef.current = null; @@ -712,11 +524,18 @@ export function ConflictResolver({ draggedRowIndexRef.current = null; setDraggedRowIndex(null); setDropRowIndex(null); - }, [recordHistory]); + }; const fileName = conflict.filePath.split('/').pop() || 'notebook.ipynb'; const allowCellDrag = true; const allowRowDrag = true; + const rowVirtualizer = useVirtualizer({ + count: rows.length, + getScrollElement: () => mainContentRef.current, + estimateSize: () => 240, + overscan: 6, + }); + const virtualRows = rowVirtualizer.getVirtualItems(); return (
@@ -912,31 +731,34 @@ export function ConflictResolver({
- {/* - Virtualization Strategy: Content Windowing - - We render ALL row wrapper divs in the normal document flow here. - This allows the browser to calculate the correct document height and scrollbar natively, - and allows ResizeObserver to measure the actual height of every row (even "off-screen" ones). - - True virtualization (removing nodes) would require absolute positioning and estimating heights, - which breaks the sticky headers and variable-height content flow. - - Optimization comes from passing `isVisible` to the MergeRow component. - When `isVisible` is false, MergeRow skips rendering expensive content (like syntax highlighting - and diffs), rendering lightweight placeholders instead. - */} -
- {rows.map((row, i) => { - const conflictIdx = row.conflictIndex ?? -1; +
+ {virtualRows.map((virtualRow) => { + const i = virtualRow.index; + const row = rows[i]; + const conflictIdx = row?.conflictIndex ?? -1; const resolutionState = conflictIdx >= 0 ? choices.get(conflictIdx) : undefined; - const isDropTargetRow = dropRowIndex === i; - // Check if row is in visible range - const isVisible = i >= visibleRange.start && i < visibleRange.end; + if (!row) return null; return ( - +
{/* Drop zone before first row */} {i === 0 && draggedRowIndex !== null && (
)} - {/* Row wrapper for height measurement */}
{ if (draggedRowIndexRef.current !== null) handleRowDragOver(e, i); }} onDrop={() => { if (draggedRowIndexRef.current !== null) handleRowDrop(i); }} @@ -971,10 +790,13 @@ export function ConflictResolver({ rowDragEnabled={allowRowDrag} onRowDragStart={allowRowDrag ? handleRowDragStart : undefined} onRowDragEnd={allowRowDrag ? handleRowDragEnd : undefined} - isVisible={isVisible} - // Cell drag props - draggedCell={draggedCell} - dropTarget={dropTarget} + // Cell drag state (primitives for stable memo comparison) + cellDragActive={draggedCell !== null} + cellDragSide={draggedCell?.side} + cellDragSourceRow={draggedCell?.rowIndex} + isRowDropTarget={dropTarget?.rowIndex === i} + rowDropTargetSide={dropTarget?.rowIndex === i ? dropTarget?.side : undefined} + // Cell drag callbacks onCellDragStart={handleCellDragStart} onCellDragEnd={handleCellDragEnd} onCellDragOver={handleCellDragOver} @@ -992,7 +814,7 @@ export function ConflictResolver({ onDragLeave={() => setDropRowIndex(null)} /> )} - +
); })}
diff --git a/src/web/client/MergeRow.tsx b/src/web/client/MergeRow.tsx index 6e872a6..3627519 100644 --- a/src/web/client/MergeRow.tsx +++ b/src/web/client/MergeRow.tsx @@ -9,24 +9,11 @@ * 4. If user changes the selected branch after editing, show a warning */ -import React, { useState, useCallback, useEffect, useMemo } from 'react'; +import React, { useState, useCallback } from 'react'; import type { MergeRow as MergeRowType, NotebookCell, ResolutionChoice } from './types'; import { CellContent } from './CellContent'; import { normalizeCellSource } from '../../notebookUtils'; -/** Data about a cell being dragged */ -interface DraggedCellData { - rowIndex: number; - side: 'base' | 'current' | 'incoming'; - cell: NotebookCell; -} - -/** Data about a potential drop target */ -interface DropTarget { - rowIndex: number; - side: 'base' | 'current' | 'incoming'; -} - /** Resolution state for a cell */ interface ResolutionState { choice: ResolutionChoice; @@ -48,13 +35,16 @@ interface MergeRowProps { showOutputs?: boolean; showBaseColumn?: boolean; enableCellDrag?: boolean; - isVisible?: boolean; // For lazy rendering optimization rowDragEnabled?: boolean; onRowDragStart?: (rowIndex: number) => void; onRowDragEnd?: () => void; - // Cell drag props - draggedCell: DraggedCellData | null; - dropTarget: DropTarget | null; + // Cell drag state (primitives for stable memo comparison) + cellDragActive: boolean; + cellDragSide?: 'base' | 'current' | 'incoming'; + cellDragSourceRow?: number; + isRowDropTarget: boolean; + rowDropTargetSide?: 'base' | 'current' | 'incoming'; + // Cell drag callbacks onCellDragStart: (rowIndex: number, side: 'base' | 'current' | 'incoming', cell: NotebookCell) => void; onCellDragEnd: () => void; onCellDragOver: (e: React.DragEvent, rowIndex: number, side: 'base' | 'current' | 'incoming') => void; @@ -62,7 +52,7 @@ interface MergeRowProps { 'data-testid'?: string; } -export function MergeRow({ +export function MergeRowInner({ row, rowIndex, conflictIndex, @@ -74,12 +64,14 @@ export function MergeRow({ showOutputs = true, showBaseColumn = true, enableCellDrag = true, - isVisible = true, rowDragEnabled = true, onRowDragStart, onRowDragEnd, - draggedCell, - dropTarget, + cellDragActive, + cellDragSide, + cellDragSourceRow, + isRowDropTarget, + rowDropTargetSide, onCellDragStart, onCellDragEnd, onCellDragOver, @@ -93,13 +85,13 @@ export function MergeRow({ const [showWarning, setShowWarning] = useState(false); // Get content for a given choice - const getContentForChoice = useCallback((choice: ResolutionChoice): string => { + const getContentForChoice = (choice: ResolutionChoice): string => { if (choice === 'delete') return ''; const cell = choice === 'base' ? row.baseCell : choice === 'current' ? row.currentCell : row.incomingCell; return cell ? normalizeCellSource(cell.source) : ''; - }, [row]); + }; // Check if content has been modified from the original const isContentModified = resolutionState @@ -140,6 +132,33 @@ export function MergeRow({ onUpdateContent(conflictIndex, e.target.value); }; + // Commit content to history on blur + const handleBlur = () => { + onCommitContent(conflictIndex); + }; + + // Stable drag handlers - useCallback keeps references stable for CellContent memo + const handleBaseCellDragStart = useCallback((e: React.DragEvent) => { + e.dataTransfer.effectAllowed = 'move'; + const src = row.baseCell?.source; + e.dataTransfer.setData('text/plain', src ? (Array.isArray(src) ? src.join('') : src) : ''); + if (row.baseCell) onCellDragStart(rowIndex, 'base', row.baseCell); + }, [onCellDragStart, rowIndex, row.baseCell]); + + const handleCurrentCellDragStart = useCallback((e: React.DragEvent) => { + e.dataTransfer.effectAllowed = 'move'; + const src = row.currentCell?.source; + e.dataTransfer.setData('text/plain', src ? (Array.isArray(src) ? src.join('') : src) : ''); + if (row.currentCell) onCellDragStart(rowIndex, 'current', row.currentCell); + }, [onCellDragStart, rowIndex, row.currentCell]); + + const handleIncomingCellDragStart = useCallback((e: React.DragEvent) => { + e.dataTransfer.effectAllowed = 'move'; + const src = row.incomingCell?.source; + e.dataTransfer.setData('text/plain', src ? (Array.isArray(src) ? src.join('') : src) : ''); + if (row.incomingCell) onCellDragStart(rowIndex, 'incoming', row.incomingCell); + }, [onCellDragStart, rowIndex, row.incomingCell]); + const canDragRow = rowDragEnabled && Boolean(onRowDragStart); const rowDragHandle = canDragRow ? (
@@ -191,10 +209,10 @@ export function MergeRow({ // Check if this cell is a valid drop target const isDropTargetCell = (side: 'base' | 'current' | 'incoming') => { if (!enableCellDrag) return false; - if (!draggedCell) return false; - if (draggedCell.side !== side) return false; // Same column only - if (draggedCell.rowIndex === rowIndex) return false; // Not same row - return dropTarget?.rowIndex === rowIndex && dropTarget?.side === side; + if (!cellDragActive) return false; + if (cellDragSide !== side) return false; // Same column only + if (cellDragSourceRow === rowIndex) return false; // Not same row + return isRowDropTarget && rowDropTargetSide === side; }; // Check if a cell can be dragged (only unmatched cells) @@ -244,37 +262,29 @@ export function MergeRow({ )} {/* Three-way diff view */} -
- {showBaseColumn && ( -
- {row.baseCell ? ( - { - e.dataTransfer.effectAllowed = 'move'; - e.dataTransfer.setData('text/plain', Array.isArray(row.baseCell!.source) ? row.baseCell!.source.join('') : row.baseCell!.source); - onCellDragStart(rowIndex, 'base', row.baseCell!); - } : undefined} - onDragEnd={canDragCell ? () => onCellDragEnd() : undefined} - /> - ) : ( -
{ e.preventDefault(); onCellDragOver(e, rowIndex, 'base'); } : undefined} - onDrop={enableCellDrag ? () => onCellDrop(rowIndex, 'base') : undefined} - > - {getPlaceholderText('base')} -
- )} -
- )} +
+
+ {row.baseCell ? ( + + ) : ( +
{ e.preventDefault(); onCellDragOver(e, rowIndex, 'base'); } : undefined} + onDrop={enableCellDrag ? () => onCellDrop(rowIndex, 'base') : undefined} + > + {getPlaceholderText('base')} +
+ )} +
{row.currentCell ? ( { - e.dataTransfer.effectAllowed = 'move'; - e.dataTransfer.setData('text/plain', Array.isArray(row.currentCell!.source) ? row.currentCell!.source.join('') : row.currentCell!.source); - onCellDragStart(rowIndex, 'current', row.currentCell!); - } : undefined} - onDragEnd={canDragCell ? () => onCellDragEnd() : undefined} + onDragStart={canDragCell ? handleCurrentCellDragStart : undefined} + onDragEnd={canDragCell ? onCellDragEnd : undefined} /> ) : (
{ - e.dataTransfer.effectAllowed = 'move'; - e.dataTransfer.setData('text/plain', Array.isArray(row.incomingCell!.source) ? row.incomingCell!.source.join('') : row.incomingCell!.source); - onCellDragStart(rowIndex, 'incoming', row.incomingCell!); - } : undefined} - onDragEnd={canDragCell ? () => onCellDragEnd() : undefined} + onDragStart={canDragCell ? handleIncomingCellDragStart : undefined} + onDragEnd={canDragCell ? onCellDragEnd : undefined} /> ) : (
onCommitContent(conflictIndex)} + onBlur={handleBlur} placeholder="Enter cell content..." rows={Math.max(5, resolutionState.resolvedContent.split('\n').length + 1)} /> @@ -402,3 +402,4 @@ export function MergeRow({
); } +export const MergeRow = React.memo(MergeRowInner); diff --git a/src/web/client/styles.ts b/src/web/client/styles.ts index 18b04c3..dec4e47 100644 --- a/src/web/client/styles.ts +++ b/src/web/client/styles.ts @@ -252,7 +252,7 @@ body { /* Merge rows */ .merge-row { - margin-bottom: 16px; + margin-bottom: 0; border-radius: 6px; overflow: hidden; position: relative; @@ -459,6 +459,18 @@ body { height: auto; } +.image-placeholder { + color: var(--text-secondary); + font-style: italic; + padding: 8px; + background: var(--bg-secondary); + border: 1px dashed var(--border-color); + border-radius: 4px; + font-family: "SF Mono", Monaco, "Cascadia Code", "Courier New", monospace; + white-space: pre-wrap; + font-size: 12px; +} + /* Auto-resolve banner */ .auto-resolve-banner { background: rgba(78, 201, 176, 0.1); @@ -876,7 +888,6 @@ body { .merge-row.conflict-row { background: rgba(244, 135, 113, 0.05); border-left: 4px solid #f48771; - margin: 8px 0; border-radius: 4px; } @@ -884,7 +895,6 @@ body { .merge-row.unmatched-row { background: rgba(255, 193, 7, 0.08); border-left: 4px solid #ffc107; - margin: 8px 0; border-radius: 4px; } @@ -902,6 +912,10 @@ body { background: rgba(255, 152, 0, 0.08); } +.virtual-row { + padding-bottom: 16px; +} + /* Drop zones for drag and drop */ .drop-zone { min-height: 40px;