Skip to content

Commit 1891f13

Browse files
authored
Revert diff panel file view changes (#204)
- Restore file labels and navigation wiring - Remove locale-dependent timestamps from the diff selector - Simplify patch rendering layout and inline raw patch fallback
1 parent a78d3b1 commit 1891f13

1 file changed

Lines changed: 42 additions & 49 deletions

File tree

apps/web/src/components/DiffPanel.tsx

Lines changed: 42 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@ import { useNavigate, useParams, useSearch } from "@tanstack/react-router";
55
import { ThreadId, type TurnId } from "@okcode/contracts";
66
import { CheckIcon, ChevronDownIcon, Columns2Icon, Rows3Icon, TextWrapIcon } from "lucide-react";
77
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
8-
import { RawPatchViewer } from "~/components/pr-review/RawPatchViewer";
98
import { gitBranchesQueryOptions } from "~/lib/gitReactQuery";
109
import { checkpointDiffQueryOptions } from "~/lib/providerReactQuery";
1110
import { cn } from "~/lib/utils";
12-
import { useFileViewNavigation } from "~/hooks/useFileViewNavigation";
11+
import { useCodeViewerStore } from "../codeViewerStore";
1312
import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch";
1413
import { useTheme } from "../hooks/useTheme";
1514
import { buildPatchCacheKey } from "../lib/diffRendering";
@@ -23,7 +22,6 @@ import {
2322
} from "../lib/diffFileReviewState";
2423
import { resolveDiffThemeName } from "../lib/diffRendering";
2524
import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries";
26-
import { useI18n } from "../i18n/useI18n";
2725
import { useStore } from "../store";
2826
import { useAppSettings } from "../appSettings";
2927
import { formatShortTimestamp } from "../timestampFormat";
@@ -208,7 +206,7 @@ function DiffFileSection(props: {
208206
<Button
209207
size="icon-xs"
210208
variant="ghost"
211-
aria-label={collapsed ? "Expand file diff" : "Collapse file diff"}
209+
aria-label={collapsed ? `Expand ${filePath}` : `Collapse ${filePath}`}
212210
aria-expanded={!collapsed}
213211
onClick={() => onToggleCollapsed(filePath)}
214212
className="text-muted-foreground/80"
@@ -221,8 +219,9 @@ function DiffFileSection(props: {
221219
type="button"
222220
className="min-w-0 flex-1 truncate text-left font-mono text-[11px] text-foreground/90 underline-offset-2 hover:underline"
223221
onClick={() => onOpenInEditor(filePath)}
222+
title={`Open ${filePath}`}
224223
>
225-
{filePath.split("/").pop() ?? filePath}
224+
{filePath}
226225
</button>
227226
{hasNonZeroStat(stats) && (
228227
<span className="hidden shrink-0 font-mono text-[10px] tabular-nums text-muted-foreground/80 sm:inline">
@@ -270,7 +269,6 @@ export { DiffWorkerPoolProvider } from "./DiffWorkerPoolProvider";
270269
export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
271270
const navigate = useNavigate();
272271
const { resolvedTheme } = useTheme();
273-
const { resolvedLocale } = useI18n();
274272
const { settings } = useAppSettings();
275273
const [diffRenderMode, setDiffRenderMode] = useState<DiffRenderMode>("stacked");
276274
const [diffWordWrap, setDiffWordWrap] = useState(settings.diffWordWrap);
@@ -489,7 +487,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
489487
target?.scrollIntoView({ block: "nearest" });
490488
}, [selectedFilePath, renderableFiles]);
491489

492-
const openFileInCodeViewer = useFileViewNavigation();
490+
const openFileInCodeViewer = useCodeViewerStore((state) => state.openFile);
493491
const openDiffFileInCodeViewer = useCallback(
494492
(filePath: string) => {
495493
if (!activeCwd) return;
@@ -527,23 +525,21 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
527525
},
528526
[updateActiveReviewState],
529527
);
528+
530529
const latestSelectedTurnId = orderedTurnDiffSummaries[0]?.turnId ?? null;
531530

532-
const selectTurn = useCallback(
533-
(turnId: TurnId) => {
534-
if (!activeThread) return;
535-
void navigate({
536-
to: "/$threadId",
537-
params: { threadId: activeThread.id },
538-
search: (previous) => {
539-
const rest = stripDiffSearchParams(previous);
540-
return { ...rest, diff: "1", diffTurnId: turnId };
541-
},
542-
});
543-
},
544-
[activeThread, navigate],
545-
);
546-
const selectWholeConversation = useCallback(() => {
531+
const selectTurn = (turnId: TurnId) => {
532+
if (!activeThread) return;
533+
void navigate({
534+
to: "/$threadId",
535+
params: { threadId: activeThread.id },
536+
search: (previous) => {
537+
const rest = stripDiffSearchParams(previous);
538+
return { ...rest, diff: "1", diffTurnId: turnId };
539+
},
540+
});
541+
};
542+
const selectWholeConversation = () => {
547543
if (!activeThread) return;
548544
void navigate({
549545
to: "/$threadId",
@@ -553,7 +549,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
553549
return { ...rest, diff: "1" };
554550
},
555551
});
556-
}, [activeThread, navigate]);
552+
};
557553
const turnSelectValue = selectedTurnId ?? "all";
558554
const handleTurnSelectChange = useCallback(
559555
(value: string | null) => {
@@ -574,26 +570,14 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
574570
{selectedTurnId === null
575571
? "All changes"
576572
: selectedTurn?.turnId === latestSelectedTurnId
577-
? `Latest • ${formatShortTimestamp(
578-
selectedTurn.completedAt,
579-
settings.timestampFormat,
580-
resolvedLocale,
581-
)}`
573+
? `Latest • ${formatShortTimestamp(selectedTurn.completedAt, settings.timestampFormat)}`
582574
: `Change ${
583575
selectedTurn?.checkpointTurnCount ??
584576
(selectedTurn
585577
? inferredCheckpointTurnCountByTurnId[selectedTurn.turnId]
586578
: null) ??
587579
"?"
588-
}${
589-
selectedTurn
590-
? formatShortTimestamp(
591-
selectedTurn.completedAt,
592-
settings.timestampFormat,
593-
resolvedLocale,
594-
)
595-
: ""
596-
}`}
580+
}${selectedTurn ? formatShortTimestamp(selectedTurn.completedAt, settings.timestampFormat) : ""}`}
597581
</SelectButton>
598582
<SelectPopup>
599583
<SelectItem value="all">All changes</SelectItem>
@@ -610,11 +594,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
610594
}`}
611595
</span>
612596
<span className="text-muted-foreground text-xs">
613-
{formatShortTimestamp(
614-
summary.completedAt,
615-
settings.timestampFormat,
616-
resolvedLocale,
617-
)}
597+
{formatShortTimestamp(summary.completedAt, settings.timestampFormat)}
618598
</span>
619599
</span>
620600
</SelectItem>
@@ -686,7 +666,10 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
686666
</div>
687667
) : (
688668
<>
689-
<div ref={patchViewportRef} className="min-h-0 min-w-0 flex-1 overflow-hidden">
669+
<div
670+
ref={patchViewportRef}
671+
className="diff-panel-viewport min-h-0 min-w-0 flex-1 overflow-hidden"
672+
>
690673
{checkpointDiffError && !renderablePatch && (
691674
<div className="px-3">
692675
<p className="mb-2 text-[11px] text-red-500/80">{checkpointDiffError}</p>
@@ -706,11 +689,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
706689
)
707690
) : renderablePatch.kind === "files" ? (
708691
<Virtualizer
709-
// `@pierre/diffs` virtualizes file bodies against its own scroll root.
710-
// Nesting that root inside another scrolling viewport leaves the diff
711-
// rows stuck in placeholder mode, which matches the blank panels here.
712-
className="diff-panel-viewport diff-render-surface h-full min-h-0 min-w-0 overflow-x-hidden overflow-y-auto"
713-
contentClassName="px-2 pb-2"
692+
className="diff-render-surface h-full min-h-0 overflow-auto px-2 pb-2"
714693
config={{
715694
overscrollSize: 600,
716695
intersectionObserverMargin: 1200,
@@ -743,7 +722,21 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
743722
})}
744723
</Virtualizer>
745724
) : (
746-
<RawPatchViewer text={renderablePatch.text} reason={renderablePatch.reason} />
725+
<div className="h-full overflow-auto p-2">
726+
<div className="space-y-2">
727+
<p className="text-[11px] text-muted-foreground/75">{renderablePatch.reason}</p>
728+
<pre
729+
className={cn(
730+
"max-h-[72vh] rounded-md border border-border/70 bg-background/70 p-3 font-mono text-[11px] leading-relaxed text-muted-foreground/90",
731+
diffWordWrap
732+
? "overflow-auto whitespace-pre-wrap wrap-break-word"
733+
: "overflow-auto",
734+
)}
735+
>
736+
{renderablePatch.text}
737+
</pre>
738+
</div>
739+
</div>
747740
)}
748741
</div>
749742
</>

0 commit comments

Comments
 (0)