diff --git a/ui/src/components/PatchTool.tsx b/ui/src/components/PatchTool.tsx index f1c1ad59e5d8290ed0708d94e63777d5fe5e5ca9..f9b849eadbcd4e43e990ff1878e7466fb46d460b 100644 --- a/ui/src/components/PatchTool.tsx +++ b/ui/src/components/PatchTool.tsx @@ -3,15 +3,41 @@ import type * as Monaco from "monaco-editor"; import { LLMContent } from "../types"; import { isDarkModeActive } from "../services/theme"; -// Feature flag for Monaco diff view (set localStorage.setItem('shelley-use-monaco-diff', 'true') to enable) +// LocalStorage keys for preferences +const STORAGE_KEY_MONACO_ENABLED = "shelley-use-monaco-diff"; +const STORAGE_KEY_SIDE_BY_SIDE = "shelley-diff-side-by-side"; + +// Feature flag for Monaco diff view function useMonacoDiff(): boolean { try { - return localStorage.getItem("shelley-use-monaco-diff") === "true"; + return localStorage.getItem(STORAGE_KEY_MONACO_ENABLED) === "true"; } catch { return false; } } +// Get saved side-by-side preference (default: true for desktop) +function getSideBySidePreference(): boolean { + try { + const stored = localStorage.getItem(STORAGE_KEY_SIDE_BY_SIDE); + if (stored !== null) { + return stored === "true"; + } + // Default to side-by-side on desktop, inline on mobile + return window.innerWidth >= 768; + } catch { + return window.innerWidth >= 768; + } +} + +function setSideBySidePreference(value: boolean): void { + try { + localStorage.setItem(STORAGE_KEY_SIDE_BY_SIDE, value ? "true" : "false"); + } catch { + // Ignore storage errors + } +} + // Display data structure from the patch tool interface PatchDisplayData { path: string; @@ -72,13 +98,7 @@ function loadMonaco(): Promise { } // Simple diff view component (default) -function SimpleDiffView({ - displayData, - executionTime, -}: { - displayData: PatchDisplayData | null; - executionTime?: string; -}) { +function SimpleDiffView({ displayData }: { displayData: PatchDisplayData | null }) { // Get diff text from displayData or fall back to empty const diff = displayData?.diff || ""; @@ -86,35 +106,27 @@ function SimpleDiffView({ const lines = diff ? diff.split("\n") : []; return ( -
- {executionTime && ( -
- Diff: - {executionTime} -
- )} -
-        {lines.map((line, idx) => {
-          // Determine line type for styling
-          let className = "patch-diff-line";
-          if (line.startsWith("+") && !line.startsWith("+++")) {
-            className += " patch-diff-addition";
-          } else if (line.startsWith("-") && !line.startsWith("---")) {
-            className += " patch-diff-deletion";
-          } else if (line.startsWith("@@")) {
-            className += " patch-diff-hunk";
-          } else if (line.startsWith("---") || line.startsWith("+++")) {
-            className += " patch-diff-header";
-          }
+    
+      {lines.map((line, idx) => {
+        // Determine line type for styling
+        let className = "patch-diff-line";
+        if (line.startsWith("+") && !line.startsWith("+++")) {
+          className += " patch-diff-addition";
+        } else if (line.startsWith("-") && !line.startsWith("---")) {
+          className += " patch-diff-deletion";
+        } else if (line.startsWith("@@")) {
+          className += " patch-diff-hunk";
+        } else if (line.startsWith("---") || line.startsWith("+++")) {
+          className += " patch-diff-header";
+        }
 
-          return (
-            
- {line || " "} -
- ); - })} -
-
+ return ( +
+ {line || " "} +
+ ); + })} + ); } @@ -122,26 +134,32 @@ function SimpleDiffView({ function MonacoDiffView({ displayData, isMobile, + sideBySide, onCommentTextChange, filename, }: { displayData: PatchDisplayData; isMobile: boolean; + sideBySide: boolean; onCommentTextChange?: (text: string) => void; filename: string; }) { const [monacoLoaded, setMonacoLoaded] = useState(false); + const [isVisible, setIsVisible] = useState(false); + const [editorHeight, setEditorHeight] = useState(100); const [showCommentDialog, setShowCommentDialog] = useState<{ line: number; selectedText?: string; } | null>(null); const [commentText, setCommentText] = useState(""); + const containerRef = useRef(null); const editorContainerRef = useRef(null); const editorRef = useRef(null); const monacoRef = useRef(null); const commentInputRef = useRef(null); const hoverDecorationsRef = useRef([]); + const heightSetRef = useRef(false); const modelsRef = useRef<{ original: Monaco.editor.ITextModel | null; modified: Monaco.editor.ITextModel | null; @@ -150,23 +168,58 @@ function MonacoDiffView({ modified: null, }); - // Load Monaco + // Intersection observer for lazy loading useEffect(() => { - if (!monacoLoaded) { - loadMonaco() - .then((monaco) => { - monacoRef.current = monaco; - setMonacoLoaded(true); - }) - .catch((err) => { - console.error("Failed to load Monaco:", err); - }); + const container = containerRef.current; + if (!container) return; + + const observer = new IntersectionObserver( + (entries) => { + for (const entry of entries) { + if (entry.isIntersecting) { + setIsVisible(true); + // Once visible, we don't need to observe anymore + observer.disconnect(); + } + } + }, + { + rootMargin: "100px", // Start loading a bit before it's visible + threshold: 0, + }, + ); + + observer.observe(container); + + return () => observer.disconnect(); + }, []); + + // Load Monaco only when visible + useEffect(() => { + if (!isVisible || monacoLoaded) return; + + loadMonaco() + .then((monaco) => { + monacoRef.current = monaco; + setMonacoLoaded(true); + }) + .catch((err) => { + console.error("Failed to load Monaco:", err); + }); + }, [isVisible, monacoLoaded]); + + // Update side-by-side mode when prop changes + useEffect(() => { + if (editorRef.current) { + editorRef.current.updateOptions({ renderSideBySide: sideBySide }); + // Reset height flag to allow recalculation after mode change + heightSetRef.current = false; } - }, [monacoLoaded]); + }, [sideBySide]); - // Create Monaco editor when data is ready + // Create Monaco editor when data is ready and visible useEffect(() => { - if (!monacoLoaded || !editorContainerRef.current || !monacoRef.current) { + if (!monacoLoaded || !isVisible || !editorContainerRef.current || !monacoRef.current) { return; } @@ -186,6 +239,9 @@ function MonacoDiffView({ modelsRef.current.modified = null; } + // Reset height tracking for new editor + heightSetRef.current = false; + // Get language from file extension const ext = "." + (displayData.path.split(".").pop()?.toLowerCase() || ""); const languages = monaco.languages.getLanguages(); @@ -212,13 +268,13 @@ function MonacoDiffView({ const modifiedModel = monaco.editor.createModel(displayData.newContent, language, modifiedUri); modelsRef.current = { original: originalModel, modified: modifiedModel }; - // Create diff editor + // Create diff editor with collapsed unchanged regions const diffEditor = monaco.editor.createDiffEditor(editorContainerRef.current, { theme: isDarkModeActive() ? "vs-dark" : "vs", readOnly: true, originalEditable: false, automaticLayout: true, - renderSideBySide: !isMobile, + renderSideBySide: sideBySide, enableSplitViewResizing: true, renderIndicators: true, renderMarginRevertIcon: false, @@ -236,6 +292,19 @@ function MonacoDiffView({ contextmenu: false, links: false, folding: !isMobile, + // Hide unchanged regions to show only edited sections + hideUnchangedRegions: { + enabled: true, + revealLineCount: 2, // Show 2 lines of context around changes + minimumLineCount: 3, // Hide regions with 3+ unchanged lines + contextLineCount: 2, // Context lines to show when expanding + }, + // Disable scrollbar when content fits + scrollbar: { + vertical: "auto", + horizontal: "auto", + alwaysConsumeMouseWheel: false, + }, }); diffEditor.setModel({ @@ -245,10 +314,33 @@ function MonacoDiffView({ editorRef.current = diffEditor; - // Add click handler for commenting if callback is provided - if (onCommentTextChange) { + // Function to update height - only do this once to avoid scroll disruption + const updateHeight = () => { + if (heightSetRef.current) return; + const modifiedEditor = diffEditor.getModifiedEditor(); + const contentHeight = modifiedEditor.getContentHeight(); + + if (contentHeight > 0) { + // Add small buffer, no max height - let it expand fully + const newHeight = Math.max(60, contentHeight + 4); + heightSetRef.current = true; + setEditorHeight(newHeight); + } + }; + + // Update height after diff is computed + // Monaco needs time to compute the diff and layout + const heightUpdateTimer = setTimeout(updateHeight, 200); + // Also listen for content size change (fires when diff is computed) + const modifiedEditor = diffEditor.getModifiedEditor(); + const contentSizeDisposable = modifiedEditor.onDidContentSizeChange(() => { + updateHeight(); + }); + + // Add click handler for commenting if callback is provided + if (onCommentTextChange) { const openCommentDialog = (lineNumber: number) => { const model = modifiedEditor.getModel(); const selection = modifiedEditor.getSelection(); @@ -323,6 +415,8 @@ function MonacoDiffView({ // Cleanup function return () => { + clearTimeout(heightUpdateTimer); + contentSizeDisposable.dispose(); if (editorRef.current) { editorRef.current.dispose(); editorRef.current = null; @@ -336,7 +430,7 @@ function MonacoDiffView({ modelsRef.current.modified = null; } }; - }, [monacoLoaded, displayData, isMobile, onCommentTextChange]); + }, [monacoLoaded, isVisible, displayData, isMobile, onCommentTextChange, sideBySide]); // Update Monaco theme when dark mode changes useEffect(() => { @@ -385,24 +479,25 @@ function MonacoDiffView({ setCommentText(""); }, [showCommentDialog, commentText, onCommentTextChange, filename]); - // Calculate editor height based on content - const getEditorHeight = () => { - const lineCount = Math.max( - displayData.oldContent.split("\n").length, - displayData.newContent.split("\n").length, - ); - // Clamp between 100px and 400px, with 18px per line - const height = Math.min(400, Math.max(100, lineCount * 18 + 20)); - return `${height}px`; - }; - return ( - <> -
+
+ {/* Monaco editor container */} + {!isVisible ? ( +
+ Scroll to load diff... +
+ ) : !monacoLoaded ? ( +
+
+ Loading editor... +
+ ) : ( +
+ )} {/* Comment dialog */} {showCommentDialog && onCommentTextChange && ( @@ -443,7 +538,71 @@ function MonacoDiffView({
)} - +
+ ); +} + +// Side-by-side toggle icon component +function DiffModeToggle({ sideBySide, onToggle }: { sideBySide: boolean; onToggle: () => void }) { + return ( + ); } @@ -452,13 +611,13 @@ function PatchTool({ isRunning, toolResult, hasError, - executionTime, display, onCommentTextChange, }: PatchToolProps) { // Default to collapsed for errors (since agents typically recover), expanded otherwise const [isExpanded, setIsExpanded] = useState(!hasError); const [isMobile, setIsMobile] = useState(window.innerWidth < 768); + const [sideBySide, setSideBySide] = useState(() => !isMobile && getSideBySidePreference()); // Check feature flag for Monaco diff view const useMonaco = useMonacoDiff(); @@ -472,6 +631,13 @@ function PatchTool({ return () => window.removeEventListener("resize", handleResize); }, []); + // Toggle side-by-side mode + const toggleSideBySide = useCallback(() => { + const newValue = !sideBySide; + setSideBySide(newValue); + setSideBySidePreference(newValue); + }, [sideBySide]); + // Extract path from toolInput const path = typeof toolInput === "object" && @@ -502,6 +668,10 @@ function PatchTool({ // Extract filename from path or diff headers const filename = displayData?.path || path || "patch"; + // Show toggle only for Monaco view on desktop when expanded and complete + const showDiffToggle = + useMonaco && !isMobile && isExpanded && isComplete && !hasError && displayData; + return (
✗} {isComplete && !hasError && }
- + + + + +
{isExpanded && (
{isComplete && !hasError && displayData && (
- {executionTime && ( -
- Diff: - {executionTime} -
- )} - {useMonaco ? ( ) : ( - + )}
)} {isComplete && hasError && (
-
- Error: - {executionTime && {executionTime}} -
{errorMessage || "Patch failed"}
)} diff --git a/ui/src/styles.css b/ui/src/styles.css index 5f416a2edb36ad35973450d48d885958ed00be4d..dacffa8cbdcb1418f14f66b3fb3b887ae3899d53 100644 --- a/ui/src/styles.css +++ b/ui/src/styles.css @@ -1188,6 +1188,13 @@ button { flex-shrink: 0; } +.patch-tool-header-controls { + display: flex; + align-items: center; + gap: 0.25rem; + flex-shrink: 0; +} + .patch-tool-toggle { background: none; border: none; @@ -1204,6 +1211,26 @@ button { color: var(--text-primary); } +.patch-tool-diff-mode-toggle { + background: none; + border: 1px solid var(--border); + border-radius: 0.25rem; + cursor: pointer; + padding: 0.25rem; + display: flex; + align-items: center; + justify-content: center; + color: var(--text-secondary); + flex-shrink: 0; + transition: all 0.15s ease; +} + +.patch-tool-diff-mode-toggle:hover { + color: var(--text-primary); + background: var(--bg-secondary); + border-color: var(--text-tertiary); +} + .patch-tool-details { padding: 0 1rem 0.75rem 1rem; display: flex; @@ -1295,6 +1322,41 @@ button { font-style: italic; } +/* Patch Tool Monaco Container */ +.patch-tool-monaco-container { + display: flex; + flex-direction: column; + gap: 0.25rem; +} + +/* Patch Tool Monaco Placeholder (for lazy loading) */ +.patch-tool-monaco-placeholder { + display: flex; + align-items: center; + justify-content: center; + gap: 0.5rem; + background: var(--bg-tertiary); + border: 1px solid var(--border); + border-radius: 0.25rem; + color: var(--text-secondary); + font-size: 0.75rem; +} + +.spinner-small { + width: 14px; + height: 14px; + border: 2px solid var(--border); + border-top-color: var(--primary); + border-radius: 50%; + animation: spin 0.8s linear infinite; +} + +@keyframes spin { + to { + transform: rotate(360deg); + } +} + /* Patch Tool Monaco Editor */ .patch-tool-monaco-editor { border: 1px solid var(--border);