feat(FN-866): add theme-aware status styling for changed files and refactor stuck-task detector
- Add theme-aware CSS classes for changed-file status badges (added, modified, deleted, renamed) - Update ChangedFilesModal, CommitDiffTab, and TaskChangesTab to use theme-aware status classes - Add comprehensive tests for ChangedFilesModal and TaskChangesTab status styling - Refactor stuck-task-detector to extract StuckTaskConfig and simplify detection logic - Clean up 370+ lines of redundant test code in stuck-task-detector tests - Update dashboard README with theme-aware styling notes - Pass throughChangedFiles option in executor for better diff handling
This commit is contained in:
@@ -238,7 +238,7 @@ export function ChangedFilesModal({
|
||||
>
|
||||
<span className="file-node-icon">{getStatusIcon(file.status)}</span>
|
||||
<span className="file-node-name">{file.path}</span>
|
||||
<span className="detail-column-badge changed-files-badge">
|
||||
<span className={`detail-column-badge changed-files-badge changed-files-badge--${file.status}`}>
|
||||
{getStatusLabel(file.status)}
|
||||
</span>
|
||||
</button>
|
||||
@@ -267,7 +267,7 @@ export function ChangedFilesModal({
|
||||
</button>
|
||||
)}
|
||||
<strong>{selectedFile.path}</strong>
|
||||
<span className="detail-column-badge changed-files-badge">
|
||||
<span className={`detail-column-badge changed-files-badge changed-files-badge--${selectedFile.status}`}>
|
||||
{getStatusLabel(selectedFile.status)}
|
||||
</span>
|
||||
{selectedFile.oldPath ? (
|
||||
|
||||
@@ -17,19 +17,6 @@ export interface ParsedFile {
|
||||
patch: string;
|
||||
}
|
||||
|
||||
function getStatusColor(status: ParsedFile["status"]): string {
|
||||
switch (status) {
|
||||
case "added":
|
||||
return "#3fb950";
|
||||
case "deleted":
|
||||
return "#f85149";
|
||||
case "modified":
|
||||
return "#58a6ff";
|
||||
default:
|
||||
return "#8b949e";
|
||||
}
|
||||
}
|
||||
|
||||
function getStatusLabel(status: ParsedFile["status"]): string {
|
||||
switch (status) {
|
||||
case "added":
|
||||
@@ -225,8 +212,7 @@ export function CommitDiffTab({ commitSha, mergeDetails }: CommitDiffTabProps) {
|
||||
{isExpanded ? <ChevronDown size={14} /> : <ChevronRight size={14} />}
|
||||
</span>
|
||||
<span
|
||||
className="changes-file-status"
|
||||
style={{ color: getStatusColor(file.status) }}
|
||||
className={`changes-file-status changes-file-status--${file.status}`}
|
||||
title={file.status}
|
||||
>
|
||||
{getStatusLabel(file.status)}
|
||||
|
||||
@@ -13,19 +13,6 @@ interface TaskChangesTabProps {
|
||||
mergeDetails?: MergeDetails;
|
||||
}
|
||||
|
||||
function getStatusColor(status: "added" | "modified" | "deleted" | "unknown"): string {
|
||||
switch (status) {
|
||||
case "added":
|
||||
return "#3fb950"; // green
|
||||
case "deleted":
|
||||
return "#f85149"; // red
|
||||
case "modified":
|
||||
return "#58a6ff"; // blue
|
||||
default:
|
||||
return "#8b949e"; // gray
|
||||
}
|
||||
}
|
||||
|
||||
function getStatusLabel(status: "added" | "modified" | "deleted" | "unknown"): string {
|
||||
switch (status) {
|
||||
case "added":
|
||||
@@ -251,8 +238,7 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
{isExpanded ? <ChevronDown size={14} /> : <ChevronRight size={14} />}
|
||||
</span>
|
||||
<span
|
||||
className="changes-file-status"
|
||||
style={{ color: getStatusColor(file.status) }}
|
||||
className={`changes-file-status changes-file-status--${file.status}`}
|
||||
title={file.status}
|
||||
>
|
||||
{getStatusLabel(file.status)}
|
||||
|
||||
@@ -703,4 +703,76 @@ describe("ChangedFilesModal", () => {
|
||||
expect(content?.classList.contains("mobile")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("status-to-class mapping", () => {
|
||||
it("applies status-specific CSS class to sidebar badges", () => {
|
||||
render(
|
||||
<ChangedFilesModal
|
||||
taskId="KB-651"
|
||||
worktree="/repo/.worktrees/kb-651"
|
||||
column="in-progress"
|
||||
isOpen={true}
|
||||
onClose={mockOnClose}
|
||||
/>,
|
||||
);
|
||||
|
||||
// defaultFiles has modified (src/a.ts) and added (src/b.ts)
|
||||
const modifiedBadge = document.querySelector(".changed-files-badge--modified");
|
||||
expect(modifiedBadge).toBeTruthy();
|
||||
expect(modifiedBadge?.textContent).toBe("M");
|
||||
|
||||
const addedBadge = document.querySelector(".changed-files-badge--added");
|
||||
expect(addedBadge).toBeTruthy();
|
||||
expect(addedBadge?.textContent).toBe("A");
|
||||
});
|
||||
|
||||
it("applies status-specific CSS class to toolbar badge in diff section", () => {
|
||||
render(
|
||||
<ChangedFilesModal
|
||||
taskId="KB-651"
|
||||
worktree="/repo/.worktrees/kb-651"
|
||||
column="in-progress"
|
||||
isOpen={true}
|
||||
onClose={mockOnClose}
|
||||
/>,
|
||||
);
|
||||
|
||||
// defaultSelectedFile is src/a.ts with status "modified"
|
||||
const toolbarBadge = document.querySelector(".changed-files-diff-section .changed-files-badge--modified");
|
||||
expect(toolbarBadge).toBeTruthy();
|
||||
expect(toolbarBadge?.textContent).toBe("M");
|
||||
});
|
||||
|
||||
it("applies renamed status class for renamed files", () => {
|
||||
const renamedFile = {
|
||||
path: "src/new-name.ts",
|
||||
oldPath: "src/old-name.ts",
|
||||
status: "renamed" as const,
|
||||
diff: "diff --git a/src/old-name.ts b/src/new-name.ts",
|
||||
};
|
||||
|
||||
mockUseChangedFiles.mockReturnValue({
|
||||
files: [renamedFile],
|
||||
loading: false,
|
||||
error: null,
|
||||
selectedFile: renamedFile,
|
||||
setSelectedFile: mockSetSelectedFile,
|
||||
resetSelection: mockResetSelection,
|
||||
});
|
||||
|
||||
render(
|
||||
<ChangedFilesModal
|
||||
taskId="KB-651"
|
||||
worktree="/repo/.worktrees/kb-651"
|
||||
column="in-progress"
|
||||
isOpen={true}
|
||||
onClose={mockOnClose}
|
||||
/>,
|
||||
);
|
||||
|
||||
const renamedBadge = document.querySelector(".changed-files-badge--renamed");
|
||||
expect(renamedBadge).toBeTruthy();
|
||||
expect(renamedBadge?.textContent).toBe("R");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -396,3 +396,84 @@ describe("TaskChangesTab — regression: non-done tasks still use worktree path"
|
||||
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskChangesTab — status-to-class mapping", () => {
|
||||
it("applies semantic status class for each file status", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("src/app.ts")).toBeTruthy();
|
||||
});
|
||||
|
||||
// src/app.ts is modified (no new file mode / deleted file mode markers)
|
||||
const statusBadges = container.querySelectorAll(".changes-file-status");
|
||||
expect(statusBadges.length).toBeGreaterThanOrEqual(2);
|
||||
|
||||
// Verify semantic status classes are present
|
||||
const modifiedBadge = container.querySelector(".changes-file-status--modified");
|
||||
expect(modifiedBadge).toBeTruthy();
|
||||
expect(modifiedBadge?.textContent).toBe("M");
|
||||
|
||||
const addedBadge = container.querySelector(".changes-file-status--added");
|
||||
expect(addedBadge).toBeTruthy();
|
||||
expect(addedBadge?.textContent).toBe("A");
|
||||
});
|
||||
|
||||
it("uses CSS classes instead of inline styles for status colors", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("src/app.ts")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Status badges should NOT have inline style attributes (colors come from CSS)
|
||||
const statusBadges = container.querySelectorAll(".changes-file-status");
|
||||
for (const badge of Array.from(statusBadges)) {
|
||||
expect(badge.getAttribute("style")).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it("renders stat summary with diff-add and diff-del classes", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Files Changed (2)")).toBeTruthy();
|
||||
});
|
||||
|
||||
const statSummary = container.querySelector(".changes-stat-summary");
|
||||
expect(statSummary).toBeTruthy();
|
||||
|
||||
const addStat = statSummary?.querySelector(".diff-add");
|
||||
expect(addStat).toBeTruthy();
|
||||
|
||||
const delStat = statSummary?.querySelector(".diff-del");
|
||||
expect(delStat).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user