shelley: Add hover highlighting and comment indicator to modal diff viewer

Philip Zeyliger created 4 months ago

Prompt: The hovering looks BEAUTIFUL in the patch tool diffs. Can you also make it work for the modal diff viewer we have?

- Apply same hover highlighting pattern to DiffViewer component
- Show comment indicator glyph only in comment mode (not edit mode)
- Clear hover decorations when switching to edit mode
- Enable glyph margin on non-mobile devices
- Use separate CSS class names (diff-viewer-*) to avoid conflicts

Change summary

ui/src/components/DiffViewer.tsx | 51 +++++++++++++++++++++++++++++++++
ui/src/styles.css                | 32 +++++++++++++++++++++
2 files changed, 82 insertions(+), 1 deletion(-)

Detailed changes

ui/src/components/DiffViewer.tsx

@@ -113,6 +113,7 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
   const monacoRef = useRef<typeof Monaco | null>(null);
   const commentInputRef = useRef<HTMLTextAreaElement>(null);
   const modeRef = useRef<ViewMode>(mode);
+  const hoverDecorationsRef = useRef<string[]>([]);
 
   // Keep modeRef in sync with mode state and update editor options
   useEffect(() => {
@@ -252,7 +253,7 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
       minimap: { enabled: false },
       scrollBeyondLastLine: true, // Enable scroll past end for mobile floating buttons
       wordWrap: "on",
-      glyphMargin: false, // No glyph margin - click on lines to comment
+      glyphMargin: !isMobile, // Enable glyph margin for comment indicator on hover
       lineDecorationsWidth: isMobile ? 0 : 10,
       lineNumbersMinChars: isMobile ? 0 : 3,
       quickSuggestions: false,
@@ -346,6 +347,54 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
       });
     }
 
+    // Add hover highlighting with comment indicator (comment mode only)
+    let lastHoveredLine = -1;
+    modifiedEditor.onMouseMove((e: Monaco.editor.IEditorMouseEvent) => {
+      // Only show hover effects in comment mode
+      if (modeRef.current !== "comment") {
+        if (hoverDecorationsRef.current.length > 0) {
+          hoverDecorationsRef.current = modifiedEditor.deltaDecorations(
+            hoverDecorationsRef.current,
+            [],
+          );
+        }
+        return;
+      }
+
+      const position = e.target.position;
+      const lineNumber = position?.lineNumber ?? -1;
+
+      if (lineNumber === lastHoveredLine) return;
+      lastHoveredLine = lineNumber;
+
+      if (lineNumber > 0) {
+        hoverDecorationsRef.current = modifiedEditor.deltaDecorations(hoverDecorationsRef.current, [
+          {
+            range: new monaco.Range(lineNumber, 1, lineNumber, 1),
+            options: {
+              isWholeLine: true,
+              className: "diff-viewer-line-hover",
+              glyphMarginClassName: "diff-viewer-comment-glyph",
+            },
+          },
+        ]);
+      } else {
+        hoverDecorationsRef.current = modifiedEditor.deltaDecorations(
+          hoverDecorationsRef.current,
+          [],
+        );
+      }
+    });
+
+    // Clear decorations when mouse leaves editor
+    modifiedEditor.onMouseLeave(() => {
+      lastHoveredLine = -1;
+      hoverDecorationsRef.current = modifiedEditor.deltaDecorations(
+        hoverDecorationsRef.current,
+        [],
+      );
+    });
+
     // Add content change listener for auto-save
     contentChangeDisposableRef.current?.dispose();
     contentChangeDisposableRef.current = modifiedEditor.onDidChangeModelContent(() => {

ui/src/styles.css

@@ -3009,6 +3009,38 @@ svg {
   width: 100%;
 }
 
+/* Diff viewer line hover highlighting (via decoration API) */
+.diff-viewer-editor .monaco-editor .diff-viewer-line-hover {
+  background-color: rgba(37, 99, 235, 0.08) !important;
+}
+
+.dark .diff-viewer-editor .monaco-editor .diff-viewer-line-hover {
+  background-color: rgba(96, 165, 250, 0.12) !important;
+}
+
+/* Comment indicator glyph in margin for diff viewer */
+.diff-viewer-editor .monaco-editor .diff-viewer-comment-glyph {
+  background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='14' height='14' viewBox='0 0 24 24' fill='none' stroke='%236b7280' stroke-width='2' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='M21 15a2 2 0 0 1-2 2H7l-4 4V5a2 2 0 0 1 2-2h14a2 2 0 0 1 2 2z'%3E%3C/path%3E%3Cline x1='9' y1='10' x2='15' y2='10'%3E%3C/line%3E%3C/svg%3E");
+  background-size: 14px 14px;
+  background-repeat: no-repeat;
+  background-position: center;
+  opacity: 0.6;
+  cursor: pointer;
+}
+
+.diff-viewer-editor .monaco-editor .diff-viewer-comment-glyph:hover {
+  opacity: 1;
+}
+
+.dark .diff-viewer-editor .monaco-editor .diff-viewer-comment-glyph {
+  background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='14' height='14' viewBox='0 0 24 24' fill='none' stroke='%239ca3af' stroke-width='2' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='M21 15a2 2 0 0 1-2 2H7l-4 4V5a2 2 0 0 1 2-2h14a2 2 0 0 1 2 2z'%3E%3C/path%3E%3Cline x1='9' y1='10' x2='15' y2='10'%3E%3C/line%3E%3C/svg%3E");
+}
+
+/* Make diff viewer Monaco lines clickable with pointer cursor */
+.diff-viewer-editor .monaco-editor .view-lines {
+  cursor: pointer;
+}
+
 .diff-viewer-comment-badge {
   position: absolute;
   bottom: 1rem;