feat(FN-906): add file navigation to changes tab and reduce diff margins
- Add file navigation dropdown/keyboard support to the TaskChangesTab component - Reduce horizontal margins in diff display for better readability - Add comprehensive tests for file navigation behavior - Include changeset for patch release
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { useState, useEffect, useCallback } from "react";
|
||||
import { FileCode, ChevronDown, ChevronRight, AlertCircle, GitCommit } from "lucide-react";
|
||||
import { FileCode, ChevronDown, ChevronRight, ChevronLeft, AlertCircle, GitCommit } from "lucide-react";
|
||||
import type { MergeDetails, Column } from "@fusion/core";
|
||||
import { fetchTaskDiff, type TaskDiff } from "../api";
|
||||
import { highlightDiff } from "../utils/highlightDiff";
|
||||
@@ -49,6 +49,7 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [expandedFiles, setExpandedFiles] = useState<Set<string>>(new Set());
|
||||
const [currentFileIndex, setCurrentFileIndex] = useState<number | null>(null);
|
||||
|
||||
const canLoad = column === "in-progress" || column === "in-review" || column === "done";
|
||||
|
||||
@@ -73,6 +74,7 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
setStats(data.stats);
|
||||
if (normalized.length > 0) {
|
||||
setExpandedFiles(new Set([normalized[0].path]));
|
||||
setCurrentFileIndex(0);
|
||||
}
|
||||
} catch (err: any) {
|
||||
setError(err.message || "Failed to load diff");
|
||||
@@ -92,11 +94,27 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
next.delete(filePath);
|
||||
} else {
|
||||
next.add(filePath);
|
||||
// Update currentFileIndex to the newly expanded file
|
||||
const idx = files.findIndex((f) => f.path === filePath);
|
||||
if (idx !== -1) {
|
||||
setCurrentFileIndex(idx);
|
||||
}
|
||||
}
|
||||
return next;
|
||||
});
|
||||
};
|
||||
|
||||
const navigateToFile = (index: number) => {
|
||||
if (index < 0 || index >= files.length) return;
|
||||
const targetPath = files[index].path;
|
||||
// Collapse all files and expand only the target
|
||||
setExpandedFiles(new Set([targetPath]));
|
||||
setCurrentFileIndex(index);
|
||||
};
|
||||
|
||||
const canGoPrev = currentFileIndex !== null && currentFileIndex > 0;
|
||||
const canGoNext = currentFileIndex !== null && currentFileIndex < files.length - 1;
|
||||
|
||||
if (loading) {
|
||||
return (
|
||||
<div className="detail-section">
|
||||
@@ -183,13 +201,40 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
<span className="diff-del">-{stats.deletions}</span>
|
||||
</span>
|
||||
</h4>
|
||||
<button
|
||||
className="btn btn-sm"
|
||||
onClick={loadDiff}
|
||||
disabled={loading}
|
||||
>
|
||||
Refresh
|
||||
</button>
|
||||
<div className="changes-header-actions">
|
||||
{files.length > 0 && (
|
||||
<div className="changes-nav">
|
||||
<button
|
||||
className="btn btn-sm btn-icon"
|
||||
onClick={() => canGoPrev && navigateToFile(currentFileIndex! - 1)}
|
||||
disabled={!canGoPrev}
|
||||
title="Previous file"
|
||||
aria-label="Previous file"
|
||||
>
|
||||
<ChevronLeft size={14} />
|
||||
</button>
|
||||
<span className="changes-nav-indicator" aria-live="polite">
|
||||
{currentFileIndex !== null ? `${currentFileIndex + 1}/${files.length}` : `—/${files.length}`}
|
||||
</span>
|
||||
<button
|
||||
className="btn btn-sm btn-icon"
|
||||
onClick={() => canGoNext && navigateToFile(currentFileIndex! + 1)}
|
||||
disabled={!canGoNext}
|
||||
title="Next file"
|
||||
aria-label="Next file"
|
||||
>
|
||||
<ChevronRight size={14} />
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
<button
|
||||
className="btn btn-sm"
|
||||
onClick={loadDiff}
|
||||
disabled={loading}
|
||||
>
|
||||
Refresh
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div className="changes-file-list">
|
||||
|
||||
@@ -13,6 +13,7 @@ vi.mock("lucide-react", () => ({
|
||||
FileCode: () => null,
|
||||
ChevronDown: ({ size }: any) => <span data-testid="chevron-down" />,
|
||||
ChevronRight: ({ size }: any) => <span data-testid="chevron-right" />,
|
||||
ChevronLeft: ({ size }: any) => <span data-testid="chevron-left" />,
|
||||
AlertCircle: () => null,
|
||||
GitCommit: () => null,
|
||||
}));
|
||||
@@ -444,3 +445,176 @@ describe("TaskChangesTab — status-to-class mapping", () => {
|
||||
expect(delStat).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskChangesTab — file navigation", () => {
|
||||
it("renders Previous and Next navigation buttons", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Files Changed (2)")).toBeTruthy();
|
||||
});
|
||||
|
||||
expect(screen.getByLabelText("Previous file")).toBeTruthy();
|
||||
expect(screen.getByLabelText("Next file")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows file position indicator in current/total format", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("disables Previous button on first file", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
expect(screen.getByLabelText("Previous file")).toBeDisabled();
|
||||
});
|
||||
|
||||
it("enables Next button on first file", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
expect(screen.getByLabelText("Next file")).not.toBeDisabled();
|
||||
});
|
||||
|
||||
it("navigates to next file when Next is clicked", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Click Next
|
||||
fireEvent.click(screen.getByLabelText("Next file"));
|
||||
|
||||
// Indicator should update to 2/2
|
||||
expect(screen.getByText("2/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("disables Next button on last file", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Navigate to last file
|
||||
fireEvent.click(screen.getByLabelText("Next file"));
|
||||
|
||||
expect(screen.getByText("2/2")).toBeTruthy();
|
||||
expect(screen.getByLabelText("Next file")).toBeDisabled();
|
||||
expect(screen.getByLabelText("Previous file")).not.toBeDisabled();
|
||||
});
|
||||
|
||||
it("navigates back to previous file when Previous is clicked", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Go to next, then back
|
||||
fireEvent.click(screen.getByLabelText("Next file"));
|
||||
expect(screen.getByText("2/2")).toBeTruthy();
|
||||
|
||||
fireEvent.click(screen.getByLabelText("Previous file"));
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("expands only the navigated file", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("1/2")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Initially only first file expanded
|
||||
expect(container.querySelectorAll(".changes-file-content")).toHaveLength(1);
|
||||
|
||||
// Navigate to second file
|
||||
fireEvent.click(screen.getByLabelText("Next file"));
|
||||
|
||||
// Still only one file expanded (the second one)
|
||||
expect(container.querySelectorAll(".changes-file-content")).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3458,6 +3458,26 @@ body {
|
||||
margin: 0;
|
||||
}
|
||||
|
||||
.changes-header-actions {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
gap: var(--space-sm);
|
||||
}
|
||||
|
||||
.changes-nav {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
gap: var(--space-xs);
|
||||
}
|
||||
|
||||
.changes-nav-indicator {
|
||||
font-size: 12px;
|
||||
color: var(--text-muted);
|
||||
font-family: var(--font-mono);
|
||||
min-width: 32px;
|
||||
text-align: center;
|
||||
}
|
||||
|
||||
.changes-file-list {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
@@ -3578,7 +3598,7 @@ body {
|
||||
|
||||
.changes-diff-patch {
|
||||
margin: 0;
|
||||
padding: var(--space-sm) var(--space-md);
|
||||
padding: var(--space-sm) 0;
|
||||
background: rgba(0, 0, 0, 0.14);
|
||||
font-family: var(--font-mono);
|
||||
font-size: 11px;
|
||||
@@ -15116,7 +15136,7 @@ html .column.drag-over * {
|
||||
|
||||
.changes-diff-patch {
|
||||
margin: 0;
|
||||
padding: var(--space-md);
|
||||
padding: var(--space-sm) 0;
|
||||
font-size: 12px;
|
||||
line-height: 1.5;
|
||||
overflow-x: auto;
|
||||
|
||||
Reference in New Issue
Block a user