Skip to content

Commit 89c5712

Browse files
committed
feat: group review files by type
Generated-By: PostHog Code Task-Id: 633af2d8-9f13-4a43-ac8e-673fc1d367d8
1 parent 0b69cb9 commit 89c5712

5 files changed

Lines changed: 424 additions & 68 deletions

File tree

packages/ui/src/features/task-detail/components/ChangesPanel.tsx

Lines changed: 100 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import {
55
FilePlus,
66
MinusIcon,
77
PlusIcon,
8+
StackIcon,
9+
TreeStructure,
810
} from "@phosphor-icons/react";
911
import { getFileExtension } from "@posthog/shared";
1012
import {
@@ -19,6 +21,7 @@ import {
1921
DropdownMenu,
2022
Flex,
2123
IconButton,
24+
SegmentedControl,
2225
Spinner,
2326
Text,
2427
} from "@radix-ui/themes";
@@ -47,12 +50,17 @@ import { useCloudChangedFiles } from "../hooks/useCloudChangedFiles";
4750
import { useDiscardFile } from "../hooks/useDiscardFile";
4851
import { useStageToggle } from "../hooks/useStageToggle";
4952
import { ChangesTreeView } from "./ChangesTreeView";
53+
import type { ChangesGrouping } from "./changesTree";
5054

5155
interface ChangesPanelProps {
5256
taskId: string;
5357
task: Task;
5458
}
5559

60+
interface GroupedChangesPanelProps extends ChangesPanelProps {
61+
grouping: ChangesGrouping;
62+
}
63+
5664
interface ChangedFileItemProps {
5765
file: ChangedFile;
5866
taskId: string;
@@ -63,6 +71,7 @@ interface ChangedFileItemProps {
6371
onStageToggle?: (file: ChangedFile) => void;
6472
onDiscard?: (file: ChangedFile, fileName: string) => void;
6573
depth?: number;
74+
showFullPath?: boolean;
6675
}
6776

6877
function CompactIconButton({
@@ -99,6 +108,7 @@ function ChangedFileItem({
99108
onStageToggle,
100109
onDiscard,
101110
depth = 0,
111+
showFullPath = false,
102112
}: ChangedFileItemProps) {
103113
const requestScrollToFile = useReviewNavigationStore(
104114
(state) => state.requestScrollToFile,
@@ -283,6 +293,7 @@ function ChangedFileItem({
283293
<Tooltip content={tooltipContent} side="top" delayDuration={500}>
284294
<TreeFileRow
285295
fileName={fileName}
296+
displayName={showFullPath ? file.path : fileName}
286297
depth={depth}
287298
isActive={isActive}
288299
onClick={handleClick}
@@ -296,7 +307,11 @@ function ChangedFileItem({
296307
);
297308
}
298309

299-
function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {
310+
function CloudChangesPanel({
311+
taskId,
312+
task,
313+
grouping,
314+
}: GroupedChangesPanelProps) {
300315
const {
301316
prUrl,
302317
effectiveBranch,
@@ -313,14 +328,15 @@ function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {
313328
const effectiveFiles = changedFiles;
314329

315330
const renderFile = useCallback(
316-
(file: ChangedFile, depth: number) => (
331+
(file: ChangedFile, depth: number, showFullPath: boolean) => (
317332
<ChangedFileItem
318333
key={file.path}
319334
file={file}
320335
taskId={taskId}
321336
fileKey={file.path}
322337
isActive={activeFilePath === file.path}
323338
depth={depth}
339+
showFullPath={showFullPath}
324340
/>
325341
),
326342
[taskId, activeFilePath],
@@ -379,7 +395,11 @@ function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {
379395
return (
380396
<Box height="100%" overflowY="auto" py="2" id="changes-panel-cloud">
381397
<Flex direction="column">
382-
<ChangesTreeView files={effectiveFiles} renderFile={renderFile} />
398+
<ChangesTreeView
399+
files={effectiveFiles}
400+
grouping={grouping}
401+
renderFile={renderFile}
402+
/>
383403
{isRunActive && (
384404
<Flex align="center" gap="2" px="3" py="2">
385405
<Spinner size="1" />
@@ -395,15 +415,55 @@ function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {
395415

396416
export function ChangesPanel({ taskId, task }: ChangesPanelProps) {
397417
const isCloud = useIsCloudTask(taskId);
418+
const isExpanded = useReviewNavigationStore(
419+
(state) => state.reviewModes[taskId] === "expanded",
420+
);
421+
const [grouping, setGrouping] = useState<ChangesGrouping>("directory");
398422

399-
if (isCloud) {
400-
return <CloudChangesPanel taskId={taskId} task={task} />;
423+
const content = isCloud ? (
424+
<CloudChangesPanel taskId={taskId} task={task} grouping={grouping} />
425+
) : (
426+
<LocalChangesPanel taskId={taskId} task={task} grouping={grouping} />
427+
);
428+
429+
if (!isExpanded) {
430+
return content;
401431
}
402432

403-
return <LocalChangesPanel taskId={taskId} task={task} />;
433+
return (
434+
<Flex direction="column" className="h-full min-h-0">
435+
<Flex className="shrink-0 border-(--gray-5) border-b px-2 py-1.5">
436+
<SegmentedControl.Root
437+
value={grouping}
438+
size="1"
439+
onValueChange={(value) => setGrouping(value as ChangesGrouping)}
440+
aria-label="Changed files grouping"
441+
className="w-full"
442+
>
443+
<SegmentedControl.Item value="directory">
444+
<span className="inline-flex items-center gap-1.5">
445+
<TreeStructure size={12} />
446+
Folders
447+
</span>
448+
</SegmentedControl.Item>
449+
<SegmentedControl.Item value="file-type">
450+
<span className="inline-flex items-center gap-1.5">
451+
<StackIcon size={12} />
452+
File type
453+
</span>
454+
</SegmentedControl.Item>
455+
</SegmentedControl.Root>
456+
</Flex>
457+
<Box className="min-h-0 flex-1">{content}</Box>
458+
</Flex>
459+
);
404460
}
405461

406-
function LocalChangesPanel({ taskId, task }: ChangesPanelProps) {
462+
function LocalChangesPanel({
463+
taskId,
464+
task,
465+
grouping,
466+
}: GroupedChangesPanelProps) {
407467
const { effectiveSource, prUrl, linkedBranch } =
408468
useEffectiveDiffSource(taskId);
409469
const repoPath = useCwd(taskId);
@@ -414,21 +474,29 @@ function LocalChangesPanel({ taskId, task }: ChangesPanelProps) {
414474
taskId={taskId}
415475
repoPath={repoPath}
416476
branch={linkedBranch}
477+
grouping={grouping}
417478
/>
418479
);
419480
}
420481

421482
if (effectiveSource === "pr") {
422-
return <PrChangesPanel taskId={taskId} prUrl={prUrl} />;
483+
return <PrChangesPanel taskId={taskId} prUrl={prUrl} grouping={grouping} />;
423484
}
424485

425-
return <LocalWorkingTreeChangesPanel taskId={taskId} task={task} />;
486+
return (
487+
<LocalWorkingTreeChangesPanel
488+
taskId={taskId}
489+
task={task}
490+
grouping={grouping}
491+
/>
492+
);
426493
}
427494

428495
function LocalWorkingTreeChangesPanel({
429496
taskId,
430497
task: _task,
431-
}: ChangesPanelProps) {
498+
grouping,
499+
}: GroupedChangesPanelProps) {
432500
const workspace = useWorkspace(taskId);
433501
const repoPath = useCwd(taskId);
434502
const activeFilePath = useReviewNavigationStore(
@@ -446,7 +514,7 @@ function LocalWorkingTreeChangesPanel({
446514
const hasStagedFiles = stagedFiles.length > 0;
447515

448516
const renderLocalFile = useCallback(
449-
(file: ChangedFile, depth: number) => {
517+
(file: ChangedFile, depth: number, showFullPath: boolean) => {
450518
const key = makeFileKey(file.staged, file.path);
451519
return (
452520
<ChangedFileItem
@@ -460,6 +528,7 @@ function LocalWorkingTreeChangesPanel({
460528
onStageToggle={handleStageToggle}
461529
onDiscard={handleDiscard}
462530
depth={depth}
531+
showFullPath={showFullPath}
463532
/>
464533
);
465534
},
@@ -512,7 +581,11 @@ function LocalWorkingTreeChangesPanel({
512581
</Text>
513582
</Flex>
514583
)}
515-
<ChangesTreeView files={files} renderFile={renderLocalFile} />
584+
<ChangesTreeView
585+
files={files}
586+
grouping={grouping}
587+
renderFile={renderLocalFile}
588+
/>
516589
</Fragment>
517590
))}
518591
</Flex>
@@ -526,6 +599,7 @@ interface RemoteChangesListProps {
526599
isLoading: boolean;
527600
emptyMessage: string;
528601
panelId: string;
602+
grouping: ChangesGrouping;
529603
}
530604

531605
function RemoteChangesList({
@@ -534,20 +608,22 @@ function RemoteChangesList({
534608
isLoading,
535609
emptyMessage,
536610
panelId,
611+
grouping,
537612
}: RemoteChangesListProps) {
538613
const activeFilePath = useReviewNavigationStore(
539614
(s) => s.activeFilePaths[taskId] ?? null,
540615
);
541616

542617
const renderFile = useCallback(
543-
(file: ChangedFile, depth: number) => (
618+
(file: ChangedFile, depth: number, showFullPath: boolean) => (
544619
<ChangedFileItem
545620
key={file.path}
546621
file={file}
547622
taskId={taskId}
548623
fileKey={file.path}
549624
isActive={activeFilePath === file.path}
550625
depth={depth}
626+
showFullPath={showFullPath}
551627
/>
552628
),
553629
[taskId, activeFilePath],
@@ -564,7 +640,11 @@ function RemoteChangesList({
564640
return (
565641
<Box height="100%" overflowY="auto" py="2" id={panelId}>
566642
<Flex direction="column">
567-
<ChangesTreeView files={files} renderFile={renderFile} />
643+
<ChangesTreeView
644+
files={files}
645+
grouping={grouping}
646+
renderFile={renderFile}
647+
/>
568648
</Flex>
569649
</Box>
570650
);
@@ -574,10 +654,12 @@ function BranchChangesPanel({
574654
taskId,
575655
repoPath,
576656
branch,
657+
grouping,
577658
}: {
578659
taskId: string;
579660
repoPath: string | undefined;
580661
branch: string | null;
662+
grouping: ChangesGrouping;
581663
}) {
582664
const { data: files = [], isLoading } = useLocalBranchChangedFiles(
583665
repoPath ?? null,
@@ -595,16 +677,19 @@ function BranchChangesPanel({
595677
isLoading={isLoading}
596678
emptyMessage="No file changes in branch"
597679
panelId="changes-panel-branch"
680+
grouping={grouping}
598681
/>
599682
);
600683
}
601684

602685
function PrChangesPanel({
603686
taskId,
604687
prUrl,
688+
grouping,
605689
}: {
606690
taskId: string;
607691
prUrl: string | null;
692+
grouping: ChangesGrouping;
608693
}) {
609694
const { data: files = [], isLoading } = usePrChangedFiles(prUrl);
610695

@@ -619,6 +704,7 @@ function PrChangesPanel({
619704
isLoading={isLoading}
620705
emptyMessage="No file changes in pull request"
621706
panelId="changes-panel-pr"
707+
grouping={grouping}
622708
/>
623709
);
624710
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import type { ChangedFile } from "@posthog/shared/domain-types";
2+
import { describe, expect, it } from "vitest";
3+
import {
4+
classifyChangedFile,
5+
type FileTypeCategory,
6+
groupChangesByFileType,
7+
} from "./changesTree";
8+
9+
const changedFile = (path: string): ChangedFile => ({
10+
path,
11+
status: "modified",
12+
});
13+
14+
describe("classifyChangedFile", () => {
15+
it.each<[string, FileTypeCategory]>([
16+
["packages/core/src/service.ts", "Implementation"],
17+
["packages/core/src/service.test.ts", "Tests"],
18+
["tests/e2e/review.spec.ts", "Tests"],
19+
["packages/core/service_test.go", "Tests"],
20+
["packages/core/test_service.py", "Tests"],
21+
["packages/api/src/__generated__/schema.ts", "Generated"],
22+
["packages/api/src/schema.generated.ts", "Generated"],
23+
["packages/api/src/messages.pb.go", "Generated"],
24+
["pnpm-lock.yaml", "Generated"],
25+
["docs/code-review.md", "Documentation"],
26+
["README.md", "Documentation"],
27+
[".github/workflows/ci.yml", "Configuration"],
28+
["package.json", "Configuration"],
29+
["requirements.txt", "Configuration"],
30+
["packages/ui/src/assets/review.png", "Assets"],
31+
["Makefile", "Other"],
32+
])("classifies %s as %s", (path, expected) => {
33+
expect(classifyChangedFile(path)).toBe(expected);
34+
});
35+
});
36+
37+
describe("groupChangesByFileType", () => {
38+
it("orders categories and files consistently", () => {
39+
const groups = groupChangesByFileType([
40+
changedFile("docs/z-last.md"),
41+
changedFile("src/z-last.ts"),
42+
changedFile("src/a-first.ts"),
43+
changedFile("src/service.test.ts"),
44+
changedFile("src/schema.generated.ts"),
45+
changedFile("package.json"),
46+
]);
47+
48+
expect(groups.map((group) => group.category)).toEqual([
49+
"Implementation",
50+
"Tests",
51+
"Generated",
52+
"Documentation",
53+
"Configuration",
54+
]);
55+
expect(groups[0]?.files.map((file) => file.path)).toEqual([
56+
"src/a-first.ts",
57+
"src/z-last.ts",
58+
]);
59+
});
60+
});

0 commit comments

Comments
 (0)