feat(FN-900): replace ChangedFilesModal with files tab in TaskDetailModal

- Add initialTab prop to TaskDetailModal to open directly on a specific tab
- Replace onOpenFilesForTask with onOpenDetailWithTab across App, Board, Column, WorktreeGroup
- Update TaskCard click handlers to open detail modal with files tab instead of separate modal
- Delete ChangedFilesModal component and useChangedFiles hook (296 + 78 lines removed)
- Remove associated tests (ChangedFilesModal.test.tsx, useChangedFiles.test.ts)
- Add changeset for patch release
This commit is contained in:
gsxdsm
2026-04-04 17:26:51 -07:00
parent b2a8d0c6cc
commit bc900a1a8c
11 changed files with 54 additions and 1486 deletions

View File

@@ -35,7 +35,7 @@ interface BoardProps {
* Called when the user clicks the "Subtask" button in the inline create card.
*/
onSubtaskBreakdown?: (description: string) => void;
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
onOpenDetailWithTab?: (task: TaskDetail, initialTab: "changes") => void;
favoriteProviders?: string[];
favoriteModels?: string[];
onToggleFavorite?: (provider: string) => void;
@@ -60,7 +60,7 @@ function areTaskArraysEqual(previous: Task[], next: Task[]): boolean {
return previous.every((task, index) => task === next[index]);
}
export function Board({ tasks, projectId, maxConcurrent, onMoveTask, onOpenDetail, addToast, onQuickCreate, onNewTask, autoMerge, onToggleAutoMerge, globalPaused, onUpdateTask, onArchiveTask, onUnarchiveTask, onArchiveAllDone, searchQuery = "", availableModels, onPlanningMode, onSubtaskBreakdown, onOpenFilesForTask, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, taskStuckTimeoutMs }: BoardProps) {
export function Board({ tasks, projectId, maxConcurrent, onMoveTask, onOpenDetail, addToast, onQuickCreate, onNewTask, autoMerge, onToggleAutoMerge, globalPaused, onUpdateTask, onArchiveTask, onUnarchiveTask, onArchiveAllDone, searchQuery = "", availableModels, onPlanningMode, onSubtaskBreakdown, onOpenDetailWithTab, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, taskStuckTimeoutMs }: BoardProps) {
const [archivedCollapsed, setArchivedCollapsed] = useState(true);
const { fetchBatch } = useBatchBadgeFetch(projectId);
const debounceTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
@@ -171,7 +171,7 @@ export function Board({ tasks, projectId, maxConcurrent, onMoveTask, onOpenDetai
onUnarchiveTask={onUnarchiveTask}
allTasks={filteredTasks}
availableModels={availableModels}
onOpenFilesForTask={onOpenFilesForTask}
onOpenDetailWithTab={onOpenDetailWithTab}
favoriteProviders={favoriteProviders}
favoriteModels={favoriteModels}
onToggleFavorite={onToggleFavorite}

View File

@@ -1,296 +0,0 @@
import { useEffect, useMemo, useRef, useState, useCallback } from "react";
import {
FileEdit,
FileMinus,
FilePlus,
FileSymlink,
FolderGit2,
ArrowLeft,
X,
} from "lucide-react";
import { useChangedFiles } from "../hooks/useChangedFiles";
import { highlightDiff } from "../utils/highlightDiff";
import { truncateMiddle } from "../utils/truncatePath";
import type { TaskFileDiff } from "../api";
const MOBILE_BREAKPOINT = 768;
interface ChangedFilesModalProps {
taskId: string;
worktree: string | undefined;
column: string;
projectId?: string;
isOpen: boolean;
onClose: () => void;
}
function getStatusLabel(status: TaskFileDiff["status"]): string {
switch (status) {
case "added":
return "A";
case "deleted":
return "D";
case "renamed":
return "R";
default:
return "M";
}
}
function getStatusIcon(status: TaskFileDiff["status"]) {
switch (status) {
case "added":
return <FilePlus size={16} />;
case "deleted":
return <FileMinus size={16} />;
case "renamed":
return <FileSymlink size={16} />;
default:
return <FileEdit size={16} />;
}
}
function getDiffStat(diff: string): string {
const lines = diff.split("\n");
const statLines = lines.filter(
(line) =>
line.startsWith("diff --git") ||
line.startsWith("index ") ||
line.startsWith("--- ") ||
line.startsWith("+++ "),
);
return statLines.join("\n").trim();
}
export function ChangedFilesModal({
taskId,
worktree,
column,
projectId,
isOpen,
onClose,
}: ChangedFilesModalProps) {
const { files, loading, error, selectedFile, setSelectedFile, resetSelection } = useChangedFiles(
taskId,
worktree,
column,
projectId,
);
const [isMobile, setIsMobile] = useState(false);
const [mobileView, setMobileView] = useState<"list" | "diff">("list");
// Track whether the user has manually navigated to the diff view on mobile.
// This prevents the resize-to-mobile effect from stealing navigation intent.
const mobileDiffIntentional = useRef(false);
// Detect mobile viewport
useEffect(() => {
if (!isOpen) return;
const checkMobile = () => {
setIsMobile(window.innerWidth <= MOBILE_BREAKPOINT);
};
checkMobile();
window.addEventListener("resize", checkMobile);
return () => window.removeEventListener("resize", checkMobile);
}, [isOpen]);
// When resizing from desktop to mobile with a file selected, show diff pane
// only if the user hasn't just opened the modal (which starts at the list).
// When resizing from mobile to desktop, no special action needed (both panes visible)
useEffect(() => {
if (!isOpen || !isMobile) return;
// Only auto-switch to diff on resize if the user is actively viewing a diff
// (not the initial open where resetSelection has just cleared selectedFile)
if (selectedFile && mobileDiffIntentional.current) {
setMobileView("diff");
}
}, [isOpen, isMobile, selectedFile]);
// Auto-select first file on desktop when files load
useEffect(() => {
if (!isOpen || isMobile) return;
if (!loading && files.length > 0 && !selectedFile) {
setSelectedFile(files[0]);
}
}, [isOpen, isMobile, loading, files, selectedFile, setSelectedFile]);
// Reset mobile view and selection when modal opens.
// Always start on the file list so mobile users see changed files first.
useEffect(() => {
if (isOpen) {
setMobileView("list");
mobileDiffIntentional.current = false;
resetSelection();
}
}, [isOpen, resetSelection]);
// Escape key handler
useEffect(() => {
if (!isOpen) return;
const handleKeyDown = (event: KeyboardEvent) => {
if (event.key === "Escape") {
// On mobile diff view, Escape goes back to list first
if (isMobile && mobileView === "diff") {
setMobileView("list");
} else {
onClose();
}
}
};
document.addEventListener("keydown", handleKeyDown);
return () => document.removeEventListener("keydown", handleKeyDown);
}, [isOpen, onClose, isMobile, mobileView]);
const handleSelectFile = useCallback(
(file: TaskFileDiff) => {
setSelectedFile(file);
if (isMobile) {
mobileDiffIntentional.current = true;
setMobileView("diff");
}
},
[isMobile, setSelectedFile],
);
const handleBackToList = useCallback(() => {
setMobileView("list");
}, []);
const selectedStat = useMemo(
() => (selectedFile ? getDiffStat(selectedFile.diff) : ""),
[selectedFile],
);
if (!isOpen) return null;
const sidebarClasses = [
"file-browser-sidebar",
"changed-files-sidebar",
isMobile ? "mobile" : "",
isMobile && mobileView === "list" ? "active" : "",
]
.filter(Boolean)
.join(" ");
const contentClasses = [
"file-browser-content",
"changed-files-content",
isMobile ? "mobile" : "",
isMobile && mobileView === "diff" ? "active" : "",
]
.filter(Boolean)
.join(" ");
const showBackButton = isMobile && mobileView === "diff";
return (
<div className="modal-overlay open" onClick={onClose}>
<div
className="modal file-browser-modal changed-files-modal"
onClick={(event) => event.stopPropagation()}
>
<div className="modal-header file-browser-modal-header">
<div className="file-browser-header-title">
<FolderGit2 size={18} />
<span>Changed Files {taskId}</span>
{showBackButton && selectedFile ? (
<span className="file-browser-header-path">{selectedFile.path}</span>
) : null}
</div>
<button className="modal-close" onClick={onClose} aria-label="Close changed files viewer">
<X size={20} />
</button>
</div>
<div className="file-browser-body changed-files-layout">
<aside className={sidebarClasses} aria-label="Changed files sidebar">
{loading ? (
<div className="gm-diff-loading changed-files-loading" role="status">
<span className="changed-files-loading-spinner" aria-hidden="true" />
<span>Loading changed files</span>
</div>
) : error ? (
<div className="gm-diff-error changed-files-error" role="alert">
<span className="changed-files-error-icon" aria-hidden="true"></span>
<span>{error}</span>
</div>
) : files.length === 0 ? (
<div className="file-browser-empty changed-files-empty">
<span className="changed-files-empty-icon" aria-hidden="true">📁</span>
<span>No files changed</span>
</div>
) : (
<div className="file-browser-list" role="list" aria-label="Changed files list">
{files.map((file, index) => {
const active =
selectedFile?.path === file.path && selectedFile?.oldPath === file.oldPath;
return (
<button
key={`${file.oldPath ?? ""}:${file.path}`}
type="button"
role="listitem"
aria-label={file.path}
aria-current={active ? "true" : undefined}
className={`file-node file-node--file changed-files-entry ${active ? "active" : ""}`}
onClick={() => handleSelectFile(file)}
>
<span className="file-node-icon">{getStatusIcon(file.status)}</span>
<span className="file-node-name" title={file.path}>{truncateMiddle(file.path)}</span>
<span className={`detail-column-badge changed-files-badge changed-files-badge--${file.status}`}>
{getStatusLabel(file.status)}
</span>
</button>
);
})}
</div>
)}
</aside>
<section className={contentClasses}>
{selectedFile ? (
<div
className="gm-diff-section changed-files-diff-section"
aria-label={`Diff for ${selectedFile.path}`}
>
<div className="file-browser-toolbar">
<div className="file-browser-file-info">
{showBackButton && (
<button
className="changed-files-back-button"
onClick={handleBackToList}
aria-label="Back to file list"
>
<ArrowLeft size={16} />
<span>Back</span>
</button>
)}
<strong>{selectedFile.path}</strong>
<span className={`detail-column-badge changed-files-badge changed-files-badge--${selectedFile.status}`}>
{getStatusLabel(selectedFile.status)}
</span>
{selectedFile.oldPath ? (
<span className="changed-files-renamed">Renamed from {selectedFile.oldPath}</span>
) : null}
</div>
</div>
<div className="gm-diff-viewer">
{selectedStat ? <pre className="gm-diff-stat">{selectedStat}</pre> : null}
<pre className="gm-diff-patch">
<code>{highlightDiff(selectedFile.diff || "No diff available")}</code>
</pre>
</div>
</div>
) : !loading && !error && files.length > 0 ? (
<div className="file-browser-empty changed-files-empty">
Select a file to view changes
</div>
) : null}
</section>
</div>
</div>
</div>
);
}

View File

@@ -46,7 +46,7 @@ interface ColumnProps {
* Called when the user clicks the "Subtask" button in the inline create card.
*/
onSubtaskBreakdown?: (description: string) => void;
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
onOpenDetailWithTab?: (task: TaskDetail, initialTab: "changes") => void;
favoriteProviders?: string[];
favoriteModels?: string[];
onToggleFavorite?: (provider: string) => void;
@@ -57,7 +57,7 @@ interface ColumnProps {
taskStuckTimeoutMs?: number;
}
function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask, onOpenDetail, addToast, onQuickCreate, onNewTask, autoMerge, onToggleAutoMerge, globalPaused, onUpdateTask, onArchiveTask, onUnarchiveTask, onArchiveAllDone, collapsed, onToggleCollapse, allTasks, availableModels, onPlanningMode, onSubtaskBreakdown, onOpenFilesForTask, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, isSearchActive, taskStuckTimeoutMs }: ColumnProps) {
function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask, onOpenDetail, addToast, onQuickCreate, onNewTask, autoMerge, onToggleAutoMerge, globalPaused, onUpdateTask, onArchiveTask, onUnarchiveTask, onArchiveAllDone, collapsed, onToggleCollapse, allTasks, availableModels, onPlanningMode, onSubtaskBreakdown, onOpenDetailWithTab, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, isSearchActive, taskStuckTimeoutMs }: ColumnProps) {
const [dragOver, setDragOver] = useState(false);
const [visibleTaskCount, setVisibleTaskCount] = useState(VISIBLE_TASKS_INITIAL);
const countFlashing = useFlashOnIncrease(tasks.length);
@@ -226,7 +226,7 @@ function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask,
addToast={addToast}
globalPaused={globalPaused}
onUpdateTask={onUpdateTask}
onOpenFilesForTask={onOpenFilesForTask}
onOpenDetailWithTab={onOpenDetailWithTab}
taskStuckTimeoutMs={taskStuckTimeoutMs}
/>
))
@@ -246,7 +246,7 @@ function ColumnComponent({ column, tasks, projectId, maxConcurrent, onMoveTask,
onUpdateTask={onUpdateTask}
onArchiveTask={onArchiveTask}
onUnarchiveTask={onUnarchiveTask}
onOpenFilesForTask={onOpenFilesForTask}
onOpenDetailWithTab={onOpenDetailWithTab}
taskStuckTimeoutMs={taskStuckTimeoutMs}
/>
))}

View File

@@ -45,7 +45,7 @@ interface TaskCardProps {
) => Promise<Task>;
onArchiveTask?: (id: string) => Promise<Task>;
onUnarchiveTask?: (id: string) => Promise<Task>;
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
onOpenDetailWithTab?: (task: TaskDetail, initialTab: "changes") => void;
/** Project-level stuck task timeout in milliseconds (undefined = disabled) */
taskStuckTimeoutMs?: number;
}
@@ -91,7 +91,7 @@ function areTaskCardPropsEqual(previous: TaskCardProps, next: TaskCardProps): bo
previous.onUpdateTask === next.onUpdateTask &&
previous.onArchiveTask === next.onArchiveTask &&
previous.onUnarchiveTask === next.onUnarchiveTask &&
previous.onOpenFilesForTask === next.onOpenFilesForTask &&
previous.onOpenDetailWithTab === next.onOpenDetailWithTab &&
previousTask.id === nextTask.id &&
previousTask.title === nextTask.title &&
previousTask.description === nextTask.description &&
@@ -133,7 +133,7 @@ function TaskCardComponent({
onUpdateTask,
onArchiveTask,
onUnarchiveTask,
onOpenFilesForTask,
onOpenDetailWithTab,
taskStuckTimeoutMs,
}: TaskCardProps) {
const [dragging, setDragging] = useState(false);
@@ -499,6 +499,16 @@ function TaskCardComponent({
});
}, [addToast, onUnarchiveTask, task.id]);
const handleOpenFiles = useCallback(async (e: React.MouseEvent) => {
e.stopPropagation();
try {
const detail = await fetchTaskDetail(task.id, projectId);
onOpenDetailWithTab?.(detail, "changes");
} catch {
addToast("Failed to load task details", "error");
}
}, [task.id, projectId, onOpenDetailWithTab, addToast]);
const handleToggleSteps = useCallback((e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
setShowSteps((current) => !current);
@@ -687,11 +697,8 @@ function TaskCardComponent({
<button
type="button"
className="card-session-files"
onClick={(e) => {
e.stopPropagation();
onOpenFilesForTask?.(task.id, task.worktree, task.column);
}}
disabled={!onOpenFilesForTask}
onClick={handleOpenFiles}
disabled={!onOpenDetailWithTab}
>
<Folder size={12} />
<span>
@@ -703,11 +710,8 @@ function TaskCardComponent({
<button
type="button"
className="card-session-files"
onClick={(e) => {
e.stopPropagation();
onOpenFilesForTask?.(task.id, task.worktree, task.column);
}}
disabled={!onOpenFilesForTask}
onClick={handleOpenFiles}
disabled={!onOpenDetailWithTab}
>
<Folder size={12} />
<span>{task.mergeDetails.filesChanged} files changed</span>

View File

@@ -158,6 +158,8 @@ function formatBytes(bytes: number): string {
return `${(bytes / (1024 * 1024)).toFixed(1)} MB`;
}
type TabId = "definition" | "logs" | "changes" | "commits" | "comments" | "model" | "workflow";
interface TaskDetailModalProps {
task: TaskDetail;
projectId?: string;
@@ -172,6 +174,8 @@ interface TaskDetailModalProps {
onTaskUpdated?: (task: Task) => void;
addToast: (message: string, type?: ToastType) => void;
githubTokenConfigured?: boolean;
/** Open the modal with this tab active instead of "definition" */
initialTab?: TabId;
}
function truncate(s: string, max: number): string {
@@ -194,8 +198,15 @@ export function TaskDetailModal({
onTaskUpdated,
addToast,
githubTokenConfigured,
initialTab = "definition",
}: TaskDetailModalProps) {
const [activeTab, setActiveTab] = useState<"definition" | "logs" | "changes" | "commits" | "comments" | "model" | "workflow">("definition");
const [activeTab, setActiveTab] = useState<TabId>(initialTab);
// Sync activeTab when the caller changes initialTab (e.g. opening a different tab)
useEffect(() => {
setActiveTab(initialTab);
}, [initialTab]);
const [logSubview, setLogSubview] = useState<"activity" | "agent-log">("activity");
const [attachments, setAttachments] = useState<TaskAttachment[]>(task.attachments || []);
const [uploading, setUploading] = useState(false);

View File

@@ -16,7 +16,7 @@ interface WorktreeGroupProps {
id: string,
updates: { title?: string; description?: string; dependencies?: string[] }
) => Promise<Task>;
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
onOpenDetailWithTab?: (task: TaskDetail, initialTab: "changes") => void;
/** Project-level stuck task timeout in milliseconds (undefined = disabled) */
taskStuckTimeoutMs?: number;
}
@@ -30,7 +30,7 @@ function WorktreeGroupComponent({
addToast,
globalPaused,
onUpdateTask,
onOpenFilesForTask,
onOpenDetailWithTab,
taskStuckTimeoutMs,
}: WorktreeGroupProps) {
return (
@@ -42,7 +42,7 @@ function WorktreeGroupComponent({
<span className="worktree-label">{label}</span>
</div>
{activeTasks.map((task) => (
<TaskCard key={task.id} task={task} projectId={projectId} onOpenDetail={onOpenDetail} addToast={addToast} globalPaused={globalPaused} onUpdateTask={onUpdateTask} onOpenFilesForTask={onOpenFilesForTask} taskStuckTimeoutMs={taskStuckTimeoutMs} />
<TaskCard key={task.id} task={task} projectId={projectId} onOpenDetail={onOpenDetail} addToast={addToast} globalPaused={globalPaused} onUpdateTask={onUpdateTask} onOpenDetailWithTab={onOpenDetailWithTab} taskStuckTimeoutMs={taskStuckTimeoutMs} />
))}
{queuedTasks.map((task) => (
<TaskCard
@@ -54,7 +54,7 @@ function WorktreeGroupComponent({
addToast={addToast}
globalPaused={globalPaused}
onUpdateTask={onUpdateTask}
onOpenFilesForTask={onOpenFilesForTask}
onOpenDetailWithTab={onOpenDetailWithTab}
taskStuckTimeoutMs={taskStuckTimeoutMs}
/>
))}

View File

@@ -1,933 +0,0 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { render, screen, fireEvent, act } from "@testing-library/react";
import { ChangedFilesModal } from "../ChangedFilesModal";
import * as changedFilesHook from "../../hooks/useChangedFiles";
vi.mock("../../hooks/useChangedFiles");
const mockUseChangedFiles = vi.mocked(changedFilesHook.useChangedFiles);
describe("ChangedFilesModal", () => {
const mockOnClose = vi.fn();
const mockSetSelectedFile = vi.fn();
const mockResetSelection = vi.fn();
const defaultFiles = [
{ path: "src/a.ts", status: "modified" as const, diff: "diff --git a/src/a.ts b/src/a.ts\n--- a/src/a.ts\n+++ b/src/a.ts\n+hello" },
{ path: "src/b.ts", status: "added" as const, diff: "diff --git a/src/b.ts b/src/b.ts" },
];
const defaultSelectedFile = defaultFiles[0];
beforeEach(() => {
vi.resetAllMocks();
vi.spyOn(window, "innerWidth", "get").mockReturnValue(1024);
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
});
afterEach(() => {
vi.restoreAllMocks();
});
it("renders changed files and selected diff", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
expect(screen.getByText("Changed Files — KB-651")).toBeInTheDocument();
expect(screen.getByRole("listitem", { name: "src/a.ts" })).toBeInTheDocument();
expect(screen.getByLabelText("Diff for src/a.ts")).toBeInTheDocument();
expect(screen.getByText(/\+hello/)).toBeInTheDocument();
});
it("allows selecting another file from the sidebar", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
fireEvent.click(screen.getByRole("listitem", { name: /src\/b.ts/i }));
expect(mockSetSelectedFile).toHaveBeenCalledWith(defaultFiles[1]);
});
it("shows an empty state when there are no changed files", () => {
mockUseChangedFiles.mockReturnValue({
files: [],
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
expect(screen.getByText("No files changed")).toBeInTheDocument();
});
it("closes on Escape on desktop", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
fireEvent.keyDown(document, { key: "Escape" });
expect(mockOnClose).toHaveBeenCalledTimes(1);
});
it("shows select prompt when no file is selected and files exist", () => {
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
expect(screen.getByText("Select a file to view changes")).toBeInTheDocument();
});
it("shows error state from hook", () => {
mockUseChangedFiles.mockReturnValue({
files: [],
loading: false,
error: "Failed to load",
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
expect(screen.getByText("Failed to load")).toBeInTheDocument();
});
it("shows loading state", () => {
mockUseChangedFiles.mockReturnValue({
files: [],
loading: true,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
expect(screen.getByText("Loading changed files…")).toBeInTheDocument();
});
it("resets selection when modal opens", () => {
const { rerender } = render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={false}
onClose={mockOnClose}
/>,
);
expect(mockResetSelection).not.toHaveBeenCalled();
rerender(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
expect(mockResetSelection).toHaveBeenCalledTimes(1);
});
it("marks the active file with aria-current", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const activeItem = screen.getByRole("listitem", { name: "src/a.ts" });
expect(activeItem).toHaveAttribute("aria-current", "true");
const inactiveItem = screen.getByRole("listitem", { name: /src\/b.ts/i });
expect(inactiveItem).not.toHaveAttribute("aria-current");
});
describe("mobile navigation", () => {
beforeEach(() => {
vi.spyOn(window, "innerWidth", "get").mockReturnValue(600);
});
it("shows file list pane on mobile when mobileView is list", () => {
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// Sidebar should have mobile active class
const sidebar = document.querySelector(".changed-files-sidebar");
expect(sidebar?.classList.contains("mobile")).toBe(true);
expect(sidebar?.classList.contains("active")).toBe(true);
// Content should have mobile class but NOT active
const content = document.querySelector(".changed-files-content");
expect(content?.classList.contains("mobile")).toBe(true);
expect(content?.classList.contains("active")).toBe(false);
});
it("switches to diff view when a file is selected on mobile", () => {
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// Click on a file to select it
fireEvent.click(screen.getByRole("listitem", { name: /src\/b.ts/i }));
expect(mockSetSelectedFile).toHaveBeenCalledWith(defaultFiles[1]);
});
it("shows back button on mobile when user selects a file", () => {
// Start with no selected file so user sees the list
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
const { rerender } = render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// No back button when on file list
expect(screen.queryByLabelText("Back to file list")).not.toBeInTheDocument();
// Simulate user selecting a file - the hook will update selectedFile
// and the component will set mobileView to "diff"
fireEvent.click(screen.getByRole("listitem", { name: /src\/b.ts/i }));
expect(mockSetSelectedFile).toHaveBeenCalledWith(defaultFiles[1]);
// Now simulate the hook providing the selected file (rerender with updated hook state)
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultFiles[1],
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
rerender(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// After user selects, back button should appear since mobileDiffIntentional was set
const backButton = screen.queryByLabelText("Back to file list");
expect(backButton).toBeInTheDocument();
});
it("does NOT show back button when hook provides selectedFile without user action", () => {
// Simulate cached data where the hook already has a selected file
// The modal should NOT auto-switch to diff on mobile without user intent
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// Without explicit user action, the back button should NOT appear
// because the modal should show the file list first on mobile
expect(screen.queryByLabelText("Back to file list")).not.toBeInTheDocument();
});
it("does not show back button on desktop", () => {
vi.spyOn(window, "innerWidth", "get").mockReturnValue(1024);
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
expect(screen.queryByLabelText("Back to file list")).not.toBeInTheDocument();
});
it("shows selected file path in header on mobile diff view after user selects file", () => {
// Start with no selection so user sees the list
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
const { rerender } = render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// User selects a file
fireEvent.click(screen.getByRole("listitem", { name: /src\/a.ts/i }));
// Simulate hook returning selected file
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
rerender(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// The header should show the selected file path when in mobile diff view
const headerPath = document.querySelector(".file-browser-header-path");
expect(headerPath).toBeInTheDocument();
expect(headerPath?.textContent).toBe("src/a.ts");
});
it("shows renamed file info on mobile diff view", () => {
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}
/>,
);
expect(screen.getByText("Renamed from src/old-name.ts")).toBeInTheDocument();
});
it("renders diff viewer with theme-safe CSS classes on mobile", () => {
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// Diff viewer should use theme-aware classes
const diffViewer = document.querySelector(".gm-diff-viewer");
expect(diffViewer).toBeInTheDocument();
const diffStat = document.querySelector(".gm-diff-stat");
expect(diffStat).toBeInTheDocument();
const diffPatch = document.querySelector(".gm-diff-patch");
expect(diffPatch).toBeInTheDocument();
});
it("Escape on mobile diff view goes back to list instead of closing", () => {
// Start with no selection so user sees the list
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
const { rerender } = render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// User selects a file, triggering mobileView to "diff"
fireEvent.click(screen.getByRole("listitem", { name: /src\/a.ts/i }));
// Simulate hook returning selected file (triggers diff view)
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
rerender(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// Now we should be in diff view with back button visible
expect(screen.getByLabelText("Back to file list")).toBeInTheDocument();
// On mobile with diff view, Escape should go back to list, not close
fireEvent.keyDown(document, { key: "Escape" });
expect(mockOnClose).not.toHaveBeenCalled();
// Now pressing Escape again (on list view) should close the modal
fireEvent.keyDown(document, { key: "Escape" });
expect(mockOnClose).toHaveBeenCalledTimes(1);
});
it("starts on list view when modal opens on mobile", () => {
// Simulate hook returning a selected file (e.g., cached from previous open)
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// On first render after open, the resetSelection effect fires.
// However, since the hook provides selectedFile, the mobile view
// may auto-switch to diff. The important thing is resetSelection was called.
expect(mockResetSelection).toHaveBeenCalled();
});
it("loading state has role=status for accessibility", () => {
mockUseChangedFiles.mockReturnValue({
files: [],
loading: true,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const loadingEl = screen.getByRole("status");
expect(loadingEl).toHaveTextContent("Loading changed files…");
});
it("error state has role=alert for accessibility", () => {
mockUseChangedFiles.mockReturnValue({
files: [],
loading: false,
error: "Network error",
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const alertEl = screen.getByRole("alert");
expect(alertEl).toHaveTextContent("Network error");
});
it("sidebar has aria-label for accessibility", () => {
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const sidebar = document.querySelector(".changed-files-sidebar");
expect(sidebar).toHaveAttribute("aria-label", "Changed files sidebar");
});
});
describe("desktop layout", () => {
beforeEach(() => {
vi.spyOn(window, "innerWidth", "get").mockReturnValue(1024);
});
it("auto-selects first file on desktop when files are loaded", () => {
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: null,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// Desktop should auto-select the first file via useEffect
expect(mockSetSelectedFile).toHaveBeenCalledWith(defaultFiles[0]);
});
it("does not add mobile class to panes on desktop", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const sidebar = document.querySelector(".changed-files-sidebar");
expect(sidebar?.classList.contains("mobile")).toBe(false);
const content = document.querySelector(".changed-files-content");
expect(content?.classList.contains("mobile")).toBe(false);
});
});
describe("list structure and styling classes", () => {
it("renders sidebar entries with dedicated changed-files-entry class", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const entries = document.querySelectorAll(".changed-files-entry");
expect(entries.length).toBe(2);
// Each entry should have the file-node base class AND the changed-files-entry class
entries.forEach((entry) => {
expect(entry.classList.contains("file-node")).toBe(true);
expect(entry.classList.contains("file-node--file")).toBe(true);
expect(entry.classList.contains("changed-files-entry")).toBe(true);
});
});
it("renders entry buttons with proper type and role attributes", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const entries = document.querySelectorAll("button.changed-files-entry");
entries.forEach((entry) => {
expect(entry.getAttribute("type")).toBe("button");
expect(entry.getAttribute("role")).toBe("listitem");
});
});
it("applies active class to the selected file entry on desktop", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// defaultSelectedFile is src/a.ts
const activeEntry = document.querySelector(".changed-files-entry.active");
expect(activeEntry).toBeTruthy();
expect(activeEntry?.getAttribute("aria-label")).toBe("src/a.ts");
expect(activeEntry?.getAttribute("aria-current")).toBe("true");
// Non-selected entry should NOT have active class
const allEntries = document.querySelectorAll(".changed-files-entry");
const inactiveEntry = Array.from(allEntries).find(
(el) => el.getAttribute("aria-label") === "src/b.ts",
);
expect(inactiveEntry).toBeTruthy();
expect(inactiveEntry?.classList.contains("active")).toBe(false);
});
it("renders icon, name, and badge spans in each entry", () => {
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const entries = document.querySelectorAll(".changed-files-entry");
entries.forEach((entry) => {
// Each entry should contain an icon span, a name span, and a badge span
const icon = entry.querySelector(".file-node-icon");
const name = entry.querySelector(".file-node-name");
const badge = entry.querySelector(".changed-files-badge");
expect(icon).toBeTruthy();
expect(name).toBeTruthy();
expect(badge).toBeTruthy();
});
});
it("applies active class alongside aria-current on mobile list view", () => {
vi.spyOn(window, "innerWidth", "get").mockReturnValue(600);
// On mobile, the selected file comes from hook but back button is NOT shown
// because mobileDiffIntentional hasn't been set (no user action).
// The active entry in the list should still have both classes.
mockUseChangedFiles.mockReturnValue({
files: defaultFiles,
loading: false,
error: null,
selectedFile: defaultSelectedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
// The selected file entry should have both active class and aria-current
const activeEntry = document.querySelector(".changed-files-entry.active");
expect(activeEntry).toBeTruthy();
expect(activeEntry?.getAttribute("aria-current")).toBe("true");
vi.restoreAllMocks();
});
it("renders deleted status badge class correctly", () => {
const deletedFile = {
path: "src/deleted.ts",
status: "deleted" as const,
diff: "diff --git a/src/deleted.ts b/src/deleted.ts",
};
mockUseChangedFiles.mockReturnValue({
files: [deletedFile],
loading: false,
error: null,
selectedFile: deletedFile,
setSelectedFile: mockSetSelectedFile,
resetSelection: mockResetSelection,
});
render(
<ChangedFilesModal
taskId="KB-651"
worktree="/repo/.worktrees/kb-651"
column="in-progress"
isOpen={true}
onClose={mockOnClose}
/>,
);
const deletedBadge = document.querySelector(".changed-files-badge--deleted");
expect(deletedBadge).toBeTruthy();
expect(deletedBadge?.textContent).toBe("D");
});
});
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");
});
});
});