Review UI refinements: separator styling and header layout (#683)

* Collapse viewed files in all-files review * Style diff hunk separators to be slimmer and subtler Reduce separator height from 32px to 24px, use theme-integrated semi-transparent backgrounds via --diffs-bg-separator-override, and fade text/buttons with opacity for a less visually heavy bar. * Refine review header layout Move sidebar toggles to far right, options menu to their left. Add divider between file tree button and repo label. Remove worktree pill from header (still accessible via menu). * Fix gutter drag selecting only one line Switch from deprecated enableHoverUtility + renderHoverUtility to enableGutterUtility + onGutterUtilityClick. The old API never entered Pierre's gutterSelecting mode, so dragging the gutter button always reported a single line. The new API receives the full drag range. Closes #679
2026-09-14 14:17:26 +08:00 · 2026-05-07 18:54:06 -04:00
parent 71492ec880
commit 0e4d301925
6 changed files with 115 additions and 124 deletions
@@ -1712,30 +1712,25 @@ const ReviewApp: React.FC = () => {
        <header className="py-1 flex items-center justify-between px-2 md:px-4 border-b border-border/50 bg-card/50 backdrop-blur-xl z-50">
          <div className="min-w-0 flex items-center gap-2 md:gap-3 -ml-1.5 md:-ml-3">
            {shouldShowFileTree && (
-              <button
-                onClick={() => setIsFileTreeOpen(prev => !prev)}
-                className={`p-1 rounded-md transition-all focus-visible:outline-none ${
-                  isFileTreeOpen
-                    ? 'text-primary'
-                    : 'text-muted-foreground hover:text-foreground hover:bg-muted'
-                }`}
-                title={isFileTreeOpen ? 'Hide file tree' : 'Show file tree'}
-              >
-                <svg className="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}>
-                  <path strokeLinecap="round" strokeLinejoin="round" d="M3 7v10a2 2 0 002 2h14a2 2 0 002-2V9a2 2 0 00-2-2h-6l-2-2H5a2 2 0 00-2 2z" />
-                </svg>
-              </button>
+              <>
+                <button
+                  onClick={() => setIsFileTreeOpen(prev => !prev)}
+                  className={`p-1 rounded-md transition-all focus-visible:outline-none ${
+                    isFileTreeOpen
+                      ? 'text-primary'
+                      : 'text-muted-foreground hover:text-foreground hover:bg-muted'
+                  }`}
+                  title={isFileTreeOpen ? 'Hide file tree' : 'Show file tree'}
+                >
+                  <svg className="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}>
+                    <path strokeLinecap="round" strokeLinejoin="round" d="M3 7v10a2 2 0 002 2h14a2 2 0 002-2V9a2 2 0 00-2-2h-6l-2-2H5a2 2 0 00-2 2z" />
+                  </svg>
+                </button>
+                <div className="w-px h-5 bg-border/50 mx-1 hidden md:block" />
+              </>
            )}
            {prMetadata ? (
              <div className="min-w-0 flex items-center gap-2 md:gap-3">
-                {(gitContext || agentCwd) && (
-                  <button
-                    onClick={() => setShowWorktreeDialog(true)}
-                    className="text-[10px] font-medium text-primary/80 bg-primary/10 hover:bg-primary/20 px-1.5 py-0.5 rounded transition-colors cursor-pointer"
-                  >
-                    worktree
-                  </button>
-                )}
                <span
                  className="text-xs text-muted-foreground/60 inline-flex items-center gap-1 whitespace-nowrap"
                >
@@ -1983,6 +1978,18 @@ const ReviewApp: React.FC = () => {

            <div className="w-px h-5 bg-border/50 mx-1 hidden md:block" />

+            <ReviewHeaderMenu
+              onOpenSettings={() => setOpenSettingsMenu(true)}
+              onOpenExport={() => setShowExportModal(true)}
+              onToggleFileTree={() => setIsFileTreeOpen(prev => !prev)}
+              onToggleSidebar={() => reviewSidebar.isOpen ? reviewSidebar.close() : reviewSidebar.open()}
+              isFileTreeOpen={isFileTreeOpen}
+              isSidebarOpen={reviewSidebar.isOpen}
+              appVersion={appVersion}
+            />
+
+            <div className="w-px h-5 bg-border/50 mx-1 hidden md:block" />
+
            {/* Sidebar tab toggles */}
            <button
              onClick={() => reviewSidebar.toggleTab('annotations')}
@@ -2034,18 +2041,6 @@ const ReviewApp: React.FC = () => {
                )}
              </button>
            )}
-
-            <div className="w-px h-5 bg-border/50 mx-1 hidden md:block" />
-
-            <ReviewHeaderMenu
-              onOpenSettings={() => setOpenSettingsMenu(true)}
-              onOpenExport={() => setShowExportModal(true)}
-              onToggleFileTree={() => setIsFileTreeOpen(prev => !prev)}
-              onToggleSidebar={() => reviewSidebar.isOpen ? reviewSidebar.close() : reviewSidebar.open()}
-              isFileTreeOpen={isFileTreeOpen}
-              isSidebarOpen={reviewSidebar.isOpen}
-              appVersion={appVersion}
-            />
          </div>
        </header>

@@ -154,6 +154,24 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
    });
  }, [activeFilePath]);

+  const collapseFile = useCallback((filePath: string) => {
+    if (!collapsedFiles.has(filePath)) {
+      collapseHistory.current.push(filePath);
+      setCollapsedFiles(prev => {
+        const next = new Set(prev);
+        next.add(filePath);
+        return next;
+      });
+    }
+    if (activeFilePath === filePath) setActiveFilePath(null);
+  }, [activeFilePath, collapsedFiles]);
+
+  const toggleViewedAndCollapse = useCallback((filePath: string) => {
+    const isCurrentlyViewed = viewedFiles.has(filePath);
+    onToggleViewed?.(filePath);
+    if (!isCurrentlyViewed) collapseFile(filePath);
+  }, [collapseFile, onToggleViewed, viewedFiles]);
+
  const setHeaderRef = useCallback((path: string, el: HTMLDivElement | null) => {
    if (el) headerRefs.current.set(path, el);
    else headerRefs.current.delete(path);
@@ -243,17 +261,7 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({

      if (e.key === 'v' && currentPath) {
        e.preventDefault();
-        const isCurrentlyViewed = viewedFiles.has(currentPath);
-        onToggleViewed?.(currentPath);
-        if (!isCurrentlyViewed) {
-          collapseHistory.current.push(currentPath);
-          setCollapsedFiles(prev => {
-            const next = new Set(prev);
-            next.add(currentPath);
-            return next;
-          });
-          setActiveFilePath(null);
-        }
+        toggleViewedAndCollapse(currentPath);
        return;
      }

@@ -287,7 +295,7 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
    };
    window.addEventListener('keydown', handler);
    return () => window.removeEventListener('keydown', handler);
-  }, [isActive, sortedFiles, collapsedFiles, activeFilePath, toggleCollapse, viewedFiles, onToggleViewed, canStageFiles, onStage, onAddFileComment]);
+  }, [isActive, sortedFiles, collapsedFiles, activeFilePath, toggleCollapse, toggleViewedAndCollapse, canStageFiles, onStage, onAddFileComment]);

  // Click-and-drag line selection in diff content
  useEffect(() => {
@@ -391,7 +399,7 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
                filePath={file.path}
                patch={file.patch}
                isViewed={viewedFiles.has(file.path)}
-                onToggleViewed={onToggleViewed ? () => onToggleViewed(file.path) : undefined}
+                onToggleViewed={onToggleViewed ? () => toggleViewedAndCollapse(file.path) : undefined}
                isStaged={stagedFiles?.has(file.path)}
                isStaging={stagingFile === file.path}
                onStage={onStage ? () => onStage(file.path) : undefined}
@@ -437,7 +445,15 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
                  disableBackground,
                  hunkSeparators: 'line-info',
                  enableLineSelection: true,
-                  enableHoverUtility: true,
+                  enableGutterUtility: true,
+                  onGutterUtilityClick: (range: SelectedLineRange) => {
+                    if (activeFilePath === file.path) {
+                      toolbarHostRef.current?.handleLineSelectionEnd(range);
+                    } else {
+                      pendingToolbarRange.current = range;
+                      setActiveFilePath(file.path);
+                    }
+                  },
                  onLineSelectionEnd: (range: SelectedLineRange | null) => {
                    if (range) {
                      if (activeFilePath === file.path) {
@@ -464,25 +480,6 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
                    />
                  );
                }}
-                renderHoverUtility={(getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => (
-                  <button
-                    className="hover-add-comment"
-                    onClick={(e) => {
-                      e.stopPropagation();
-                      const line = getHoveredLine();
-                      if (!line) return;
-                      const range = { start: line.lineNumber, end: line.lineNumber, side: line.side };
-                      if (activeFilePath === file.path) {
-                        toolbarHostRef.current?.handleLineSelectionEnd(range);
-                      } else {
-                        pendingToolbarRange.current = range;
-                        setActiveFilePath(file.path);
-                      }
-                    }}
-                  >
-                    +
-                  </button>
-                )}
              />
            )}
          </div>
@@ -37,8 +37,8 @@ interface PierreDiffContentProps {
  mergedAnnotations: DiffLineAnnotation<DiffAnnotationMetadata>[];
  pendingSelection: SelectedLineRange | null;
  onLineSelectionEnd: (range: SelectedLineRange | null) => void;
+  onGutterUtilityClick: (range: SelectedLineRange) => void;
  renderAnnotation: (annotation: { side: string; lineNumber: number; metadata?: DiffAnnotationMetadata }) => React.ReactNode;
-  renderHoverUtility: (getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => React.ReactNode;
  onTokenClick?: (props: DiffTokenEventBaseProps, event: MouseEvent) => void;
  onTokenEnter?: (props: DiffTokenEventBaseProps, event: PointerEvent) => void;
  onTokenLeave?: (props: DiffTokenEventBaseProps, event: PointerEvent) => void;
@@ -57,8 +57,8 @@ const PierreDiffContent = React.memo(({
  mergedAnnotations,
  pendingSelection,
  onLineSelectionEnd,
+  onGutterUtilityClick,
  renderAnnotation,
-  renderHoverUtility,
  onTokenClick,
  onTokenEnter,
  onTokenLeave,
@@ -79,7 +79,8 @@ const PierreDiffContent = React.memo(({
        disableBackground,
        hunkSeparators: 'line-info',
        enableLineSelection: true,
-        enableHoverUtility: true,
+        enableGutterUtility: true,
+        onGutterUtilityClick,
        onLineSelectionEnd,
        onTokenClick,
        onTokenEnter,
@@ -88,7 +89,6 @@ const PierreDiffContent = React.memo(({
      lineAnnotations={mergedAnnotations}
      selectedLines={pendingSelection || undefined}
      renderAnnotation={renderAnnotation}
-      renderHoverUtility={renderHoverUtility}
    />
  );
 }, (prev, next) => (
@@ -107,8 +107,8 @@ const PierreDiffContent = React.memo(({
  prev.mergedAnnotations === next.mergedAnnotations &&
  prev.pendingSelection === next.pendingSelection &&
  prev.onLineSelectionEnd === next.onLineSelectionEnd &&
+  prev.onGutterUtilityClick === next.onGutterUtilityClick &&
  prev.renderAnnotation === next.renderAnnotation &&
-  prev.renderHoverUtility === next.renderHoverUtility &&
  prev.onTokenClick === next.onTokenClick &&
  prev.onTokenEnter === next.onTokenEnter &&
  prev.onTokenLeave === next.onTokenLeave
@@ -494,29 +494,8 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
    );
  }, [filePath, onSelectAnnotation, handleEdit, onDeleteAnnotation, onClickAIMarker]);

-  // Render hover utility (+ button).
-  // Pierre manages visibility imperatively — it inserts/removes the slot
-  // container on line hover. We must always return a button; calling
-  // getHoveredLine() to gate rendering would return stale data because
-  // Pierre's hover state changes don't trigger React re-renders.
-  const renderHoverUtility = useCallback((getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => {
-    return (
-      <button
-        className="hover-add-comment"
-        onClick={(e) => {
-          e.stopPropagation();
-          const line = getHoveredLine();
-          if (!line) return;
-          toolbarHostRef.current?.handleLineSelectionEnd({
-            start: line.lineNumber,
-            end: line.lineNumber,
-            side: line.side,
-          });
-        }}
-      >
-        +
-      </button>
-    );
+  const handleGutterUtilityClick = useCallback((range: SelectedLineRange) => {
+    toolbarHostRef.current?.handleLineSelectionEnd(range);
  }, []);

  useEffect(() => {
@@ -613,8 +592,8 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
              mergedAnnotations={mergedAnnotations}
              pendingSelection={pendingSelection}
              onLineSelectionEnd={handlePierreLineSelectionEnd}
+              onGutterUtilityClick={handleGutterUtilityClick}
              renderAnnotation={renderAnnotation}
-              renderHoverUtility={renderHoverUtility}
              onTokenClick={handleTokenClick}
              onTokenEnter={handleTokenEnter}
              onTokenLeave={handleTokenLeave}
@@ -16,7 +16,6 @@ interface LazyFileDiffProps {
  annotations: DiffLineAnnotation<DiffAnnotationMetadata>[];
  selectedLines: SelectedLineRange | undefined;
  renderAnnotation: (annotation: DiffLineAnnotation<DiffAnnotationMetadata>) => React.ReactNode;
-  renderHoverUtility: (getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => React.ReactNode;
 }

 function estimateHeight(fileDiff: FileDiffMetadata, diffStyle: 'split' | 'unified'): number {
@@ -35,7 +34,6 @@ export const LazyFileDiff: React.FC<LazyFileDiffProps> = ({
  annotations,
  selectedLines,
  renderAnnotation,
-  renderHoverUtility,
 }) => {
  const [mounted, setMounted] = useState(forceMount);
  const sentinelRef = useRef<HTMLDivElement>(null);
@@ -114,7 +112,6 @@ export const LazyFileDiff: React.FC<LazyFileDiffProps> = ({
        lineAnnotations={annotations}
        selectedLines={selectedLines}
        renderAnnotation={renderAnnotation}
-        renderHoverUtility={renderHoverUtility}
      />
    </div>
  );
@@ -68,6 +68,9 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin
        --diffs-dark-bg: ${bg}; --diffs-light-bg: ${bg}; --diffs-dark: ${fg}; --diffs-light: ${fg};
      }
      pre, code { background-color: ${bg} !important; }
+      :host { --diffs-bg-separator-override: color-mix(in srgb, ${fg} 8%, ${bg}); }
+      [data-separator='line-info'], [data-separator='line-info-basic'] { height: 24px !important; }
+      [data-separator='line-info'] { margin-block: 4px !important; }
    `};
  });

@@ -77,6 +80,8 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin
      const bg = styles.getPropertyValue('--background').trim();
      const fg = styles.getPropertyValue('--foreground').trim();
      const muted = styles.getPropertyValue('--muted').trim();
+      const mutedFg = styles.getPropertyValue('--muted-foreground').trim();
+      const border = styles.getPropertyValue('--border').trim();
      const primary = styles.getPropertyValue('--primary').trim();
      if (!bg || !fg) return;

@@ -120,6 +125,50 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin
            text-underline-offset: 2px;
            cursor: pointer;
          }
+
+          /* Separator bars — slimmer, semi-transparent, integrated with theme */
+          :host {
+            --diffs-bg-separator-override: color-mix(in srgb, ${border || fg} 25%, ${bg});
+          }
+          [data-separator='line-info'],
+          [data-separator='line-info-basic'] {
+            height: 24px !important;
+          }
+          [data-separator='line-info'] {
+            margin-block: 4px !important;
+          }
+          [data-separator-content] {
+            font-size: 11px !important;
+            color: ${mutedFg || fg} !important;
+            opacity: 0.7;
+          }
+          [data-separator-content]:hover {
+            opacity: 1;
+          }
+          [data-expand-button] {
+            min-width: 24px !important;
+            color: ${mutedFg || fg} !important;
+            opacity: 0.5;
+          }
+          [data-expand-button]:hover {
+            color: ${fg} !important;
+            opacity: 1;
+          }
+          [data-expand-index] [data-separator-wrapper] {
+            grid-template-columns: 24px auto !important;
+          }
+          [data-expand-index] [data-separator-wrapper][data-separator-multi-button] {
+            grid-template-columns: 24px 24px auto !important;
+          }
+          @media (pointer: fine) {
+            [data-separator='line-info'] [data-separator-wrapper] {
+              grid-template-columns: 26px auto !important;
+            }
+            [data-separator='line-info'] [data-separator-wrapper][data-separator-multi-button] {
+              grid-template-columns: 26px 26px auto !important;
+            }
+          }
+
          ${fontCSS}
        `,
      });
@@ -241,32 +241,6 @@ diffs-container {
  background: oklch(from var(--primary) l c h / 0.3) !important;
 }

-/* Hover utility button — matches Pierre's [data-utility-button] sizing */
-.hover-add-comment {
-  appearance: none;
-  display: flex;
-  align-items: center;
-  justify-content: center;
-  width: 1lh;
-  height: 1lh;
-  font-size: var(--diffs-font-size, 13px);
-  line-height: var(--diffs-line-height, 20px);
-  border: none;
-  border-radius: 4px;
-  background-color: var(--diffs-modified-base);
-  color: var(--diffs-bg);
-  fill: currentColor;
-  cursor: pointer;
-  position: relative;
-  z-index: 4;
-  padding: 0;
-  margin-right: calc(1ch - 1lh);
-}
-
-.hover-add-comment:hover {
-  filter: brightness(1.2);
-}
-
 /* File tree styling */
 .file-tree-item {
  display: flex;