Skip to content

Commit 0e4d301

Browse files
backnotpropbacknotprop
authored
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
1 parent 71492ec commit 0e4d301

6 files changed

Lines changed: 115 additions & 124 deletions

File tree

‎packages/review-editor/App.tsx‎

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,30 +1712,25 @@ const ReviewApp: React.FC = () => {
17121712
<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">
17131713
<div className="min-w-0 flex items-center gap-2 md:gap-3 -ml-1.5 md:-ml-3">
17141714
{shouldShowFileTree && (
1715-
<button
1716-
onClick={() => setIsFileTreeOpen(prev => !prev)}
1717-
className={`p-1 rounded-md transition-all focus-visible:outline-none ${
1718-
isFileTreeOpen
1719-
? 'text-primary'
1720-
: 'text-muted-foreground hover:text-foreground hover:bg-muted'
1721-
}`}
1722-
title={isFileTreeOpen ? 'Hide file tree' : 'Show file tree'}
1723-
>
1724-
<svg className="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}>
1725-
<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" />
1726-
</svg>
1727-
</button>
1715+
<>
1716+
<button
1717+
onClick={() => setIsFileTreeOpen(prev => !prev)}
1718+
className={`p-1 rounded-md transition-all focus-visible:outline-none ${
1719+
isFileTreeOpen
1720+
? 'text-primary'
1721+
: 'text-muted-foreground hover:text-foreground hover:bg-muted'
1722+
}`}
1723+
title={isFileTreeOpen ? 'Hide file tree' : 'Show file tree'}
1724+
>
1725+
<svg className="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}>
1726+
<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" />
1727+
</svg>
1728+
</button>
1729+
<div className="w-px h-5 bg-border/50 mx-1 hidden md:block" />
1730+
</>
17281731
)}
17291732
{prMetadata ? (
17301733
<div className="min-w-0 flex items-center gap-2 md:gap-3">
1731-
{(gitContext || agentCwd) && (
1732-
<button
1733-
onClick={() => setShowWorktreeDialog(true)}
1734-
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"
1735-
>
1736-
worktree
1737-
</button>
1738-
)}
17391734
<span
17401735
className="text-xs text-muted-foreground/60 inline-flex items-center gap-1 whitespace-nowrap"
17411736
>
@@ -1983,6 +1978,18 @@ const ReviewApp: React.FC = () => {
19831978

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

1981+
<ReviewHeaderMenu
1982+
onOpenSettings={() => setOpenSettingsMenu(true)}
1983+
onOpenExport={() => setShowExportModal(true)}
1984+
onToggleFileTree={() => setIsFileTreeOpen(prev => !prev)}
1985+
onToggleSidebar={() => reviewSidebar.isOpen ? reviewSidebar.close() : reviewSidebar.open()}
1986+
isFileTreeOpen={isFileTreeOpen}
1987+
isSidebarOpen={reviewSidebar.isOpen}
1988+
appVersion={appVersion}
1989+
/>
1990+
1991+
<div className="w-px h-5 bg-border/50 mx-1 hidden md:block" />
1992+
19861993
{/* Sidebar tab toggles */}
19871994
<button
19881995
onClick={() => reviewSidebar.toggleTab('annotations')}
@@ -2034,18 +2041,6 @@ const ReviewApp: React.FC = () => {
20342041
)}
20352042
</button>
20362043
)}
2037-
2038-
<div className="w-px h-5 bg-border/50 mx-1 hidden md:block" />
2039-
2040-
<ReviewHeaderMenu
2041-
onOpenSettings={() => setOpenSettingsMenu(true)}
2042-
onOpenExport={() => setShowExportModal(true)}
2043-
onToggleFileTree={() => setIsFileTreeOpen(prev => !prev)}
2044-
onToggleSidebar={() => reviewSidebar.isOpen ? reviewSidebar.close() : reviewSidebar.open()}
2045-
isFileTreeOpen={isFileTreeOpen}
2046-
isSidebarOpen={reviewSidebar.isOpen}
2047-
appVersion={appVersion}
2048-
/>
20492044
</div>
20502045
</header>
20512046

‎packages/review-editor/components/AllFilesDiffView.tsx‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,24 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
154154
});
155155
}, [activeFilePath]);
156156

157+
const collapseFile = useCallback((filePath: string) => {
158+
if (!collapsedFiles.has(filePath)) {
159+
collapseHistory.current.push(filePath);
160+
setCollapsedFiles(prev => {
161+
const next = new Set(prev);
162+
next.add(filePath);
163+
return next;
164+
});
165+
}
166+
if (activeFilePath === filePath) setActiveFilePath(null);
167+
}, [activeFilePath, collapsedFiles]);
168+
169+
const toggleViewedAndCollapse = useCallback((filePath: string) => {
170+
const isCurrentlyViewed = viewedFiles.has(filePath);
171+
onToggleViewed?.(filePath);
172+
if (!isCurrentlyViewed) collapseFile(filePath);
173+
}, [collapseFile, onToggleViewed, viewedFiles]);
174+
157175
const setHeaderRef = useCallback((path: string, el: HTMLDivElement | null) => {
158176
if (el) headerRefs.current.set(path, el);
159177
else headerRefs.current.delete(path);
@@ -243,17 +261,7 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
243261

244262
if (e.key === 'v' && currentPath) {
245263
e.preventDefault();
246-
const isCurrentlyViewed = viewedFiles.has(currentPath);
247-
onToggleViewed?.(currentPath);
248-
if (!isCurrentlyViewed) {
249-
collapseHistory.current.push(currentPath);
250-
setCollapsedFiles(prev => {
251-
const next = new Set(prev);
252-
next.add(currentPath);
253-
return next;
254-
});
255-
setActiveFilePath(null);
256-
}
264+
toggleViewedAndCollapse(currentPath);
257265
return;
258266
}
259267

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

292300
// Click-and-drag line selection in diff content
293301
useEffect(() => {
@@ -391,7 +399,7 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
391399
filePath={file.path}
392400
patch={file.patch}
393401
isViewed={viewedFiles.has(file.path)}
394-
onToggleViewed={onToggleViewed ? () => onToggleViewed(file.path) : undefined}
402+
onToggleViewed={onToggleViewed ? () => toggleViewedAndCollapse(file.path) : undefined}
395403
isStaged={stagedFiles?.has(file.path)}
396404
isStaging={stagingFile === file.path}
397405
onStage={onStage ? () => onStage(file.path) : undefined}
@@ -437,7 +445,15 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
437445
disableBackground,
438446
hunkSeparators: 'line-info',
439447
enableLineSelection: true,
440-
enableHoverUtility: true,
448+
enableGutterUtility: true,
449+
onGutterUtilityClick: (range: SelectedLineRange) => {
450+
if (activeFilePath === file.path) {
451+
toolbarHostRef.current?.handleLineSelectionEnd(range);
452+
} else {
453+
pendingToolbarRange.current = range;
454+
setActiveFilePath(file.path);
455+
}
456+
},
441457
onLineSelectionEnd: (range: SelectedLineRange | null) => {
442458
if (range) {
443459
if (activeFilePath === file.path) {
@@ -464,25 +480,6 @@ export const AllFilesDiffView: React.FC<AllFilesDiffViewProps> = ({
464480
/>
465481
);
466482
}}
467-
renderHoverUtility={(getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => (
468-
<button
469-
className="hover-add-comment"
470-
onClick={(e) => {
471-
e.stopPropagation();
472-
const line = getHoveredLine();
473-
if (!line) return;
474-
const range = { start: line.lineNumber, end: line.lineNumber, side: line.side };
475-
if (activeFilePath === file.path) {
476-
toolbarHostRef.current?.handleLineSelectionEnd(range);
477-
} else {
478-
pendingToolbarRange.current = range;
479-
setActiveFilePath(file.path);
480-
}
481-
}}
482-
>
483-
+
484-
</button>
485-
)}
486483
/>
487484
)}
488485
</div>

‎packages/review-editor/components/DiffViewer.tsx‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ interface PierreDiffContentProps {
3737
mergedAnnotations: DiffLineAnnotation<DiffAnnotationMetadata>[];
3838
pendingSelection: SelectedLineRange | null;
3939
onLineSelectionEnd: (range: SelectedLineRange | null) => void;
40+
onGutterUtilityClick: (range: SelectedLineRange) => void;
4041
renderAnnotation: (annotation: { side: string; lineNumber: number; metadata?: DiffAnnotationMetadata }) => React.ReactNode;
41-
renderHoverUtility: (getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => React.ReactNode;
4242
onTokenClick?: (props: DiffTokenEventBaseProps, event: MouseEvent) => void;
4343
onTokenEnter?: (props: DiffTokenEventBaseProps, event: PointerEvent) => void;
4444
onTokenLeave?: (props: DiffTokenEventBaseProps, event: PointerEvent) => void;
@@ -57,8 +57,8 @@ const PierreDiffContent = React.memo(({
5757
mergedAnnotations,
5858
pendingSelection,
5959
onLineSelectionEnd,
60+
onGutterUtilityClick,
6061
renderAnnotation,
61-
renderHoverUtility,
6262
onTokenClick,
6363
onTokenEnter,
6464
onTokenLeave,
@@ -79,7 +79,8 @@ const PierreDiffContent = React.memo(({
7979
disableBackground,
8080
hunkSeparators: 'line-info',
8181
enableLineSelection: true,
82-
enableHoverUtility: true,
82+
enableGutterUtility: true,
83+
onGutterUtilityClick,
8384
onLineSelectionEnd,
8485
onTokenClick,
8586
onTokenEnter,
@@ -88,7 +89,6 @@ const PierreDiffContent = React.memo(({
8889
lineAnnotations={mergedAnnotations}
8990
selectedLines={pendingSelection || undefined}
9091
renderAnnotation={renderAnnotation}
91-
renderHoverUtility={renderHoverUtility}
9292
/>
9393
);
9494
}, (prev, next) => (
@@ -107,8 +107,8 @@ const PierreDiffContent = React.memo(({
107107
prev.mergedAnnotations === next.mergedAnnotations &&
108108
prev.pendingSelection === next.pendingSelection &&
109109
prev.onLineSelectionEnd === next.onLineSelectionEnd &&
110+
prev.onGutterUtilityClick === next.onGutterUtilityClick &&
110111
prev.renderAnnotation === next.renderAnnotation &&
111-
prev.renderHoverUtility === next.renderHoverUtility &&
112112
prev.onTokenClick === next.onTokenClick &&
113113
prev.onTokenEnter === next.onTokenEnter &&
114114
prev.onTokenLeave === next.onTokenLeave
@@ -494,29 +494,8 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
494494
);
495495
}, [filePath, onSelectAnnotation, handleEdit, onDeleteAnnotation, onClickAIMarker]);
496496

497-
// Render hover utility (+ button).
498-
// Pierre manages visibility imperatively — it inserts/removes the slot
499-
// container on line hover. We must always return a button; calling
500-
// getHoveredLine() to gate rendering would return stale data because
501-
// Pierre's hover state changes don't trigger React re-renders.
502-
const renderHoverUtility = useCallback((getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => {
503-
return (
504-
<button
505-
className="hover-add-comment"
506-
onClick={(e) => {
507-
e.stopPropagation();
508-
const line = getHoveredLine();
509-
if (!line) return;
510-
toolbarHostRef.current?.handleLineSelectionEnd({
511-
start: line.lineNumber,
512-
end: line.lineNumber,
513-
side: line.side,
514-
});
515-
}}
516-
>
517-
+
518-
</button>
519-
);
497+
const handleGutterUtilityClick = useCallback((range: SelectedLineRange) => {
498+
toolbarHostRef.current?.handleLineSelectionEnd(range);
520499
}, []);
521500

522501
useEffect(() => {
@@ -613,8 +592,8 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
613592
mergedAnnotations={mergedAnnotations}
614593
pendingSelection={pendingSelection}
615594
onLineSelectionEnd={handlePierreLineSelectionEnd}
595+
onGutterUtilityClick={handleGutterUtilityClick}
616596
renderAnnotation={renderAnnotation}
617-
renderHoverUtility={renderHoverUtility}
618597
onTokenClick={handleTokenClick}
619598
onTokenEnter={handleTokenEnter}
620599
onTokenLeave={handleTokenLeave}

‎packages/review-editor/components/LazyFileDiff.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ interface LazyFileDiffProps {
1616
annotations: DiffLineAnnotation<DiffAnnotationMetadata>[];
1717
selectedLines: SelectedLineRange | undefined;
1818
renderAnnotation: (annotation: DiffLineAnnotation<DiffAnnotationMetadata>) => React.ReactNode;
19-
renderHoverUtility: (getHoveredLine: () => { lineNumber: number; side: 'deletions' | 'additions' } | undefined) => React.ReactNode;
2019
}
2120

2221
function estimateHeight(fileDiff: FileDiffMetadata, diffStyle: 'split' | 'unified'): number {
@@ -35,7 +34,6 @@ export const LazyFileDiff: React.FC<LazyFileDiffProps> = ({
3534
annotations,
3635
selectedLines,
3736
renderAnnotation,
38-
renderHoverUtility,
3937
}) => {
4038
const [mounted, setMounted] = useState(forceMount);
4139
const sentinelRef = useRef<HTMLDivElement>(null);
@@ -114,7 +112,6 @@ export const LazyFileDiff: React.FC<LazyFileDiffProps> = ({
114112
lineAnnotations={annotations}
115113
selectedLines={selectedLines}
116114
renderAnnotation={renderAnnotation}
117-
renderHoverUtility={renderHoverUtility}
118115
/>
119116
</div>
120117
);

‎packages/review-editor/hooks/usePierreTheme.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin
6868
--diffs-dark-bg: ${bg}; --diffs-light-bg: ${bg}; --diffs-dark: ${fg}; --diffs-light: ${fg};
6969
}
7070
pre, code { background-color: ${bg} !important; }
71+
:host { --diffs-bg-separator-override: color-mix(in srgb, ${fg} 8%, ${bg}); }
72+
[data-separator='line-info'], [data-separator='line-info-basic'] { height: 24px !important; }
73+
[data-separator='line-info'] { margin-block: 4px !important; }
7174
`};
7275
});
7376

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

@@ -120,6 +125,50 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin
120125
text-underline-offset: 2px;
121126
cursor: pointer;
122127
}
128+
129+
/* Separator bars — slimmer, semi-transparent, integrated with theme */
130+
:host {
131+
--diffs-bg-separator-override: color-mix(in srgb, ${border || fg} 25%, ${bg});
132+
}
133+
[data-separator='line-info'],
134+
[data-separator='line-info-basic'] {
135+
height: 24px !important;
136+
}
137+
[data-separator='line-info'] {
138+
margin-block: 4px !important;
139+
}
140+
[data-separator-content] {
141+
font-size: 11px !important;
142+
color: ${mutedFg || fg} !important;
143+
opacity: 0.7;
144+
}
145+
[data-separator-content]:hover {
146+
opacity: 1;
147+
}
148+
[data-expand-button] {
149+
min-width: 24px !important;
150+
color: ${mutedFg || fg} !important;
151+
opacity: 0.5;
152+
}
153+
[data-expand-button]:hover {
154+
color: ${fg} !important;
155+
opacity: 1;
156+
}
157+
[data-expand-index] [data-separator-wrapper] {
158+
grid-template-columns: 24px auto !important;
159+
}
160+
[data-expand-index] [data-separator-wrapper][data-separator-multi-button] {
161+
grid-template-columns: 24px 24px auto !important;
162+
}
163+
@media (pointer: fine) {
164+
[data-separator='line-info'] [data-separator-wrapper] {
165+
grid-template-columns: 26px auto !important;
166+
}
167+
[data-separator='line-info'] [data-separator-wrapper][data-separator-multi-button] {
168+
grid-template-columns: 26px 26px auto !important;
169+
}
170+
}
171+
123172
${fontCSS}
124173
`,
125174
});

0 commit comments

Comments
 (0)