Skip to content

Commit b866107

Browse files
bohutangbohutang
committed
Improve diff hunk styling and hover controls
1 parent 1d93b9d commit b866107

File tree

2 files changed

+109
-50
lines changed

2 files changed

+109
-50
lines changed

‎packages/ui/src/components/panels/diff/ZedDiffViewer.test.tsx‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, vi } from 'vitest';
2-
import { fireEvent, render, screen } from '@testing-library/react';
2+
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
33
import { ZedDiffViewer } from './ZedDiffViewer';
44
import { API } from '../../../utils/api';
55

@@ -59,9 +59,13 @@ describe('ZedDiffViewer', () => {
5959
);
6060

6161
const stage = screen.getAllByTestId('diff-hunk-stage')[0] as HTMLButtonElement;
62-
fireEvent.click(stage);
62+
await act(async () => {
63+
fireEvent.click(stage);
64+
});
6365

64-
expect(API.sessions.stageHunk).toHaveBeenCalledWith('s1', expect.objectContaining({ isStaging: true }));
66+
await waitFor(() => {
67+
expect(API.sessions.stageHunk).toHaveBeenCalledWith('s1', expect.objectContaining({ isStaging: true }));
68+
});
6569
});
6670

6771
it('unstages a hunk when scope is staged', async () => {
@@ -76,9 +80,13 @@ describe('ZedDiffViewer', () => {
7680
);
7781

7882
const stage = screen.getAllByTestId('diff-hunk-stage')[0] as HTMLButtonElement;
79-
fireEvent.click(stage);
83+
await act(async () => {
84+
fireEvent.click(stage);
85+
});
8086

81-
expect(API.sessions.stageHunk).toHaveBeenCalledWith('s1', expect.objectContaining({ isStaging: false }));
87+
await waitFor(() => {
88+
expect(API.sessions.stageHunk).toHaveBeenCalledWith('s1', expect.objectContaining({ isStaging: false }));
89+
});
8290
});
8391

8492
it('restores a hunk using the current scope', async () => {
@@ -93,9 +101,13 @@ describe('ZedDiffViewer', () => {
93101
);
94102

95103
const restore = screen.getAllByTestId('diff-hunk-restore')[0] as HTMLButtonElement;
96-
fireEvent.click(restore);
104+
await act(async () => {
105+
fireEvent.click(restore);
106+
});
97107

98-
expect(API.sessions.restoreHunk).toHaveBeenCalledWith('s1', expect.objectContaining({ scope: 'unstaged' }));
108+
await waitFor(() => {
109+
expect(API.sessions.restoreHunk).toHaveBeenCalledWith('s1', expect.objectContaining({ scope: 'unstaged' }));
110+
});
99111
});
100112

101113
it('scrolls to a file header when scrollToFilePath changes', () => {

‎packages/ui/src/components/panels/diff/ZedDiffViewer.tsx‎

Lines changed: 90 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useCallback, useEffect, useMemo, useRef } from 'react';
1+
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
22
import { Diff, Hunk, getChangeKey, parseDiff, textLinesToHunk, type ChangeData, type DiffType, type HunkData } from 'react-diff-view';
33
import 'react-diff-view/style/index.css';
44
import { API } from '../../../utils/api';
@@ -16,6 +16,8 @@ type FileModel = {
1616
>;
1717
};
1818

19+
type HunkKind = 'added' | 'deleted' | 'modified';
20+
1921
function toFilePath(raw: { newPath: string; oldPath: string }) {
2022
return (raw.newPath || raw.oldPath || '(unknown)').trim() || '(unknown)';
2123
}
@@ -40,6 +42,19 @@ function hunkSignature(hunk: HunkData): string {
4042
return parts.join('\n');
4143
}
4244

45+
function hunkKind(hunk: HunkData): HunkKind | null {
46+
const changes = hunk.changes as ChangeData[];
47+
let hasInsert = false;
48+
let hasDelete = false;
49+
for (const change of changes) {
50+
if ((change as any).isInsert) hasInsert = true;
51+
else if ((change as any).isDelete) hasDelete = true;
52+
}
53+
if (!hasInsert && !hasDelete) return null;
54+
if (hasInsert && hasDelete) return 'modified';
55+
return hasInsert ? 'added' : 'deleted';
56+
}
57+
4358
function normalizeHunks(hunks: HunkData[]): HunkData[] {
4459
return hunks.map((h) => {
4560
const parsed = parseHunkHeader(h.content);
@@ -99,6 +114,16 @@ export const ZedDiffViewer: React.FC<{
99114
onChanged?: () => void;
100115
}> = ({ diff, className, sessionId, currentScope, stagedDiff, unstagedDiff, fileSources, scrollToFilePath, fileOrder, onChanged }) => {
101116
const fileHeaderRefs = useRef<Map<string, HTMLElement>>(new Map());
117+
const [pendingHunkKeys, setPendingHunkKeys] = useState<Set<string>>(() => new Set());
118+
119+
const setPending = useCallback((key: string, next: boolean) => {
120+
setPendingHunkKeys((prev) => {
121+
const copy = new Set(prev);
122+
if (next) copy.add(key);
123+
else copy.delete(key);
124+
return copy;
125+
});
126+
}, []);
102127

103128
const stagedHunkHeaderBySig = useMemo(() => {
104129
if (!stagedDiff || stagedDiff.trim() === '') return new Map<string, Map<string, string>>();
@@ -192,29 +217,42 @@ export const ZedDiffViewer: React.FC<{
192217
}, [scrollToFilePath, scrollToFile]);
193218

194219
const stageOrUnstageHunk = useCallback(
195-
async (filePath: string, isStaging: boolean, hunkHeader: string) => {
220+
async (filePath: string, isStaging: boolean, hunkHeader: string, hunkKey: string) => {
196221
if (!sessionId) return;
197222
try {
223+
setPending(hunkKey, true);
198224
await API.sessions.stageHunk(sessionId, { filePath, isStaging, hunkHeader });
199225
onChanged?.();
200226
} catch (err) {
201227
console.error(`[Diff] Failed to ${isStaging ? 'stage' : 'unstage'} hunk`, { filePath, hunkHeader, err });
228+
} finally {
229+
setPending(hunkKey, false);
230+
// Prevent focus from sticking to the old hunk after staging (avoids "hover controls" lingering).
231+
if (document.activeElement instanceof HTMLElement) {
232+
document.activeElement.blur();
233+
}
202234
}
203235
},
204-
[sessionId, onChanged]
236+
[sessionId, onChanged, setPending]
205237
);
206238

207239
const restoreHunk = useCallback(
208-
async (filePath: string, scope: 'staged' | 'unstaged', hunkHeader: string) => {
240+
async (filePath: string, scope: 'staged' | 'unstaged', hunkHeader: string, hunkKey: string) => {
209241
if (!sessionId) return;
210242
try {
243+
setPending(hunkKey, true);
211244
await API.sessions.restoreHunk(sessionId, { filePath, scope, hunkHeader });
212245
onChanged?.();
213246
} catch (err) {
214247
console.error('[Diff] Failed to restore hunk', { filePath, hunkHeader, err });
248+
} finally {
249+
setPending(hunkKey, false);
250+
if (document.activeElement instanceof HTMLElement) {
251+
document.activeElement.blur();
252+
}
215253
}
216254
},
217-
[sessionId, onChanged]
255+
[sessionId, onChanged, setPending]
218256
);
219257

220258
if (!diff || diff.trim() === '' || files.length === 0) {
@@ -304,30 +342,38 @@ export const ZedDiffViewer: React.FC<{
304342
? 'st-hunk-status--unstaged'
305343
: 'st-hunk-status--unknown';
306344

345+
const kind = hunkKind(hunk);
346+
const kindClass =
347+
kind === 'added' ? 'st-hunk-kind--added' : kind === 'deleted' ? 'st-hunk-kind--deleted' : 'st-hunk-kind--modified';
348+
const hunkKey = (hunk as any).__st_hunkKey as string;
349+
const isPending = pendingHunkKeys.has(hunkKey);
350+
307351
const changeKey = getChangeKey(first);
308352

309353
const element = (
310-
<div data-testid="diff-hunk-controls" className={`st-diff-hunk-actions-anchor ${statusClass}`}>
354+
<div data-testid="diff-hunk-controls" className={`st-diff-hunk-actions-anchor ${statusClass} ${kindClass}`}>
311355
<div className="st-diff-hunk-actions">
312356
<button
313357
type="button"
314358
data-testid="diff-hunk-stage"
315-
disabled={!canStageOrUnstage}
316-
onClick={() => stageOrUnstageHunk(file.path, stageLabel === 'Stage', stageHeader)}
359+
disabled={!canStageOrUnstage || isPending}
360+
onMouseDown={(e) => e.preventDefault()}
361+
onClick={() => stageOrUnstageHunk(file.path, stageLabel === 'Stage', stageHeader, hunkKey)}
317362
className="st-diff-hunk-btn"
318363
title={canStageOrUnstage ? `${stageLabel} hunk` : 'Unavailable'}
319364
>
320-
{stageLabel}
365+
{isPending ? '…' : stageLabel}
321366
</button>
322367
<button
323368
type="button"
324369
data-testid="diff-hunk-restore"
325-
disabled={!canRestore}
326-
onClick={() => restoreHunk(file.path, restoreScope, stageHeader)}
370+
disabled={!canRestore || isPending}
371+
onMouseDown={(e) => e.preventDefault()}
372+
onClick={() => restoreHunk(file.path, restoreScope, stageHeader, hunkKey)}
327373
className="st-diff-hunk-btn"
328374
title={canRestore ? 'Restore hunk' : 'Unavailable'}
329375
>
330-
Restore
376+
{isPending ? '…' : 'Restore'}
331377
</button>
332378
</div>
333379
</div>
@@ -350,6 +396,8 @@ export const ZedDiffViewer: React.FC<{
350396
--st-diff-line-height: 20px;
351397
/* Zed-like hunk padding (blank line feel). */
352398
--st-diff-hunk-pad-y: 30px;
399+
/* Zed-like hunk rounding. */
400+
--st-diff-hunk-radius: 16px;
353401
}
354402
355403
.st-diff-table.diff { table-layout: fixed; width: 100%; }
@@ -362,19 +410,25 @@ export const ZedDiffViewer: React.FC<{
362410
/* Zed-like: only "edit hunks" are treated as blocks. */
363411
.st-diff-table .diff-hunk { position: relative; }
364412
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) {
365-
--st-hunk-marker-color: var(--st-diff-modified-marker);
366-
--st-hunk-bg: var(--st-diff-hunk-bg);
367-
--st-hunk-frame-color: var(--st-accent);
413+
--st-hunk-color: var(--st-diff-modified-marker);
414+
--st-hunk-marker-color: var(--st-hunk-color);
415+
--st-hunk-solid-bg: color-mix(in srgb, var(--st-hunk-color) 12%, transparent);
416+
--st-hunk-hollow-bg: color-mix(in srgb, var(--st-hunk-color) 7%, transparent);
417+
--st-hunk-bg: var(--st-hunk-solid-bg);
418+
--st-hunk-frame-color: var(--st-hunk-color);
368419
}
369-
.st-diff-table .diff-hunk:has(.st-hunk-status--staged) {
370-
--st-hunk-marker-color: var(--st-diff-added-marker);
371-
--st-hunk-bg: color-mix(in srgb, var(--st-success) 12%, transparent);
372-
--st-hunk-frame-color: var(--st-diff-added-marker);
420+
.st-diff-table .diff-hunk:has(.st-hunk-kind--added):has(.diff-code-insert, .diff-code-delete) {
421+
--st-hunk-color: var(--st-diff-added-marker);
373422
}
374-
.st-diff-table .diff-hunk:has(.st-hunk-status--unstaged) {
375-
--st-hunk-marker-color: var(--st-diff-modified-marker);
376-
--st-hunk-bg: var(--st-diff-hunk-bg);
377-
--st-hunk-frame-color: var(--st-accent);
423+
.st-diff-table .diff-hunk:has(.st-hunk-kind--deleted):has(.diff-code-insert, .diff-code-delete) {
424+
--st-hunk-color: var(--st-diff-deleted-marker);
425+
}
426+
.st-diff-table .diff-hunk:has(.st-hunk-kind--modified):has(.diff-code-insert, .diff-code-delete) {
427+
--st-hunk-color: var(--st-diff-modified-marker);
428+
}
429+
/* Zed-like hunk_style: staged is hollow by default (bordered, faded). */
430+
.st-diff-table .diff-hunk:has(.st-hunk-status--staged):has(.diff-code-insert, .diff-code-delete) {
431+
--st-hunk-bg: var(--st-hunk-hollow-bg);
378432
}
379433
.st-diff-table .diff-hunk:has(.st-hunk-status--unknown) {
380434
--st-hunk-marker-color: var(--st-text-faint);
@@ -406,8 +460,8 @@ export const ZedDiffViewer: React.FC<{
406460
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete)::after {
407461
content: '';
408462
position: absolute;
409-
inset: 0;
410-
border-radius: 10px;
463+
inset: 2px;
464+
border-radius: var(--st-diff-hunk-radius);
411465
pointer-events: none;
412466
opacity: 0;
413467
z-index: 1;
@@ -420,8 +474,7 @@ export const ZedDiffViewer: React.FC<{
420474
.st-diff-table .diff-hunk:has(.st-hunk-status--staged):has(.diff-code-insert, .diff-code-delete)::after {
421475
opacity: 0.55;
422476
}
423-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover::after,
424-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):focus-within::after {
477+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover::after {
425478
opacity: 1;
426479
}
427480
@@ -437,26 +490,21 @@ export const ZedDiffViewer: React.FC<{
437490
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:last-child td { border-bottom: 1px solid transparent; }
438491
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr td:first-child { border-left: 1px solid transparent; }
439492
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr td:last-child { border-right: 1px solid transparent; }
440-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:first-child td:first-child { border-top-left-radius: 10px; }
441-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:first-child td:last-child { border-top-right-radius: 10px; }
442-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:last-child td:first-child { border-bottom-left-radius: 10px; }
443-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:last-child td:last-child { border-bottom-right-radius: 10px; }
493+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:first-child td:first-child { border-top-left-radius: var(--st-diff-hunk-radius); }
494+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:first-child td:last-child { border-top-right-radius: var(--st-diff-hunk-radius); }
495+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:last-child td:first-child { border-bottom-left-radius: var(--st-diff-hunk-radius); }
496+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete) tr:last-child td:last-child { border-bottom-right-radius: var(--st-diff-hunk-radius); }
444497
445-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr td,
446-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):focus-within tr td {
498+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr td {
447499
background-image: linear-gradient(
448500
color-mix(in srgb, var(--st-hunk-bg) 90%, transparent),
449501
color-mix(in srgb, var(--st-hunk-bg) 90%, transparent)
450502
);
451503
}
452-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr:first-child td,
453-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):focus-within tr:first-child td { border-top-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
454-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr:last-child td,
455-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):focus-within tr:last-child td { border-bottom-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
456-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr td:first-child,
457-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):focus-within tr td:first-child { border-left-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
458-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr td:last-child,
459-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):focus-within tr td:last-child { border-right-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
504+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr:first-child td { border-top-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
505+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr:last-child td { border-bottom-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
506+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr td:first-child { border-left-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
507+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover tr td:last-child { border-right-color: color-mix(in srgb, var(--st-hunk-frame-color) 45%, transparent); }
460508
461509
/* Extra contrast on hover (more Zed-like). */
462510
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover {
@@ -489,8 +537,7 @@ export const ZedDiffViewer: React.FC<{
489537
transition: opacity 120ms ease;
490538
z-index: 5;
491539
}
492-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover .st-diff-hunk-actions,
493-
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):focus-within .st-diff-hunk-actions {
540+
.st-diff-table .diff-hunk:has(.diff-code-insert, .diff-code-delete):hover .st-diff-hunk-actions {
494541
opacity: 1;
495542
pointer-events: auto;
496543
}

0 commit comments

Comments
 (0)