feat(FN-776): rework changed-files modal into mobile-friendly navigation flow
- Redesign ChangedFilesModal as a two-view navigation flow (file list → file detail) with back navigation - Fix theming and loading/reading ergonomics for the diff viewer - Enhance useChangedFiles hook with improved loading and error states - Add comprehensive tests for modal navigation, diff rendering, and edge cases - Update styles with mobile-friendly layout and diff viewer improvements - Update dashboard README with mobile navigation description
This commit is contained in:
@@ -47,7 +47,7 @@ AI-guided interactive planning for creating well-specified tasks from high-level
|
||||
- **Layered Model Dropdowns**: Shared model combobox menus render in a top-level portal attached to `document.body`, so they stay above board columns and scrollable modal content instead of being clipped behind surrounding dashboard surfaces.
|
||||
- **Bulk Model Editing**: Update AI model configuration for multiple tasks at once in the list view. Select tasks via checkboxes (archived tasks excluded), then use the "Bulk Edit Models" toolbar to apply executor and/or validator model changes to all selected tasks. Selection persists in localStorage across page reloads.
|
||||
- **Task Details**: View full task specifications, agent logs, and attachments. The Agent Log tab header shows the effective executor and validator model names resolved from task-level overrides or project/global settings fallbacks, matching the same resolution order the engine uses at runtime.
|
||||
- **Changed Files Viewer**: Click a task card's "files changed" button to open a dedicated diff viewer showing only files changed in that task worktree, with per-file statuses and sidebar navigation
|
||||
- **Changed Files Viewer**: Click a task card's "files changed" button to open a dedicated diff viewer showing only files changed in that task worktree, with per-file statuses and sidebar navigation. On mobile (≤768px), the viewer switches to a single-pane flow: the file list and diff are shown one at a time with a back button for navigation between them
|
||||
- **GitHub Import**: Import issues directly from GitHub repositories
|
||||
- **PR Management**: Create, monitor, and merge pull requests for in-review tasks
|
||||
|
||||
|
||||
@@ -1,9 +1,19 @@
|
||||
import { useEffect, useMemo } from "react";
|
||||
import { FileEdit, FileMinus, FilePlus, FileSymlink, FolderGit2, X } from "lucide-react";
|
||||
import { useEffect, useMemo, 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 type { TaskFileDiff } from "../api";
|
||||
|
||||
const MOBILE_BREAKPOINT = 768;
|
||||
|
||||
interface ChangedFilesModalProps {
|
||||
taskId: string;
|
||||
worktree: string | undefined;
|
||||
@@ -41,13 +51,74 @@ function getStatusIcon(status: TaskFileDiff["status"]) {
|
||||
|
||||
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("+++ "));
|
||||
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 } = useChangedFiles(taskId, worktree, column, projectId);
|
||||
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");
|
||||
|
||||
// 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
|
||||
// When resizing from mobile to desktop, no special action needed (both panes visible)
|
||||
useEffect(() => {
|
||||
if (!isOpen || !isMobile) return;
|
||||
// If we just became mobile and have a selected file, show diff
|
||||
if (selectedFile) {
|
||||
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
|
||||
useEffect(() => {
|
||||
if (isOpen) {
|
||||
setMobileView("list");
|
||||
resetSelection();
|
||||
}
|
||||
}, [isOpen, resetSelection]);
|
||||
|
||||
// Escape key handler
|
||||
useEffect(() => {
|
||||
if (!isOpen) return;
|
||||
const handleKeyDown = (event: KeyboardEvent) => {
|
||||
@@ -59,17 +130,60 @@ export function ChangedFilesModal({ taskId, worktree, column, projectId, isOpen,
|
||||
return () => document.removeEventListener("keydown", handleKeyDown);
|
||||
}, [isOpen, onClose]);
|
||||
|
||||
const selectedStat = useMemo(() => (selectedFile ? getDiffStat(selectedFile.diff) : ""), [selectedFile]);
|
||||
const handleSelectFile = useCallback(
|
||||
(file: TaskFileDiff) => {
|
||||
setSelectedFile(file);
|
||||
if (isMobile) {
|
||||
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 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} />
|
||||
@@ -77,7 +191,7 @@ export function ChangedFilesModal({ taskId, worktree, column, projectId, isOpen,
|
||||
</div>
|
||||
|
||||
<div className="file-browser-body changed-files-layout">
|
||||
<aside className="file-browser-sidebar changed-files-sidebar">
|
||||
<aside className={sidebarClasses}>
|
||||
{loading ? (
|
||||
<div className="gm-diff-loading">Loading changed files…</div>
|
||||
) : error ? (
|
||||
@@ -87,7 +201,8 @@ export function ChangedFilesModal({ taskId, worktree, column, projectId, isOpen,
|
||||
) : (
|
||||
<div className="file-browser-list" role="list" aria-label="Changed files list">
|
||||
{files.map((file) => {
|
||||
const active = selectedFile?.path === file.path && selectedFile?.oldPath === file.oldPath;
|
||||
const active =
|
||||
selectedFile?.path === file.path && selectedFile?.oldPath === file.oldPath;
|
||||
return (
|
||||
<button
|
||||
key={`${file.oldPath ?? ""}:${file.path}`}
|
||||
@@ -95,11 +210,13 @@ export function ChangedFilesModal({ taskId, worktree, column, projectId, isOpen,
|
||||
role="listitem"
|
||||
aria-label={file.path}
|
||||
className={`file-node file-node--file changed-files-entry ${active ? "active" : ""}`}
|
||||
onClick={() => setSelectedFile(file)}
|
||||
onClick={() => handleSelectFile(file)}
|
||||
>
|
||||
<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">{getStatusLabel(file.status)}</span>
|
||||
<span className="detail-column-badge changed-files-badge">
|
||||
{getStatusLabel(file.status)}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
@@ -107,18 +224,31 @@ export function ChangedFilesModal({ taskId, worktree, column, projectId, isOpen,
|
||||
)}
|
||||
</aside>
|
||||
|
||||
<section className="file-browser-content changed-files-content">
|
||||
{!loading && !error && files.length > 0 && !selectedFile ? (
|
||||
<div className="file-browser-empty">Select a file to view changes</div>
|
||||
) : null}
|
||||
|
||||
<section className={contentClasses}>
|
||||
{selectedFile ? (
|
||||
<div className="gm-diff-section changed-files-diff-section" aria-label={`Diff for ${selectedFile.path}`}>
|
||||
<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">{getStatusLabel(selectedFile.status)}</span>
|
||||
{selectedFile.oldPath ? <span>Renamed from {selectedFile.oldPath}</span> : null}
|
||||
<span className="detail-column-badge changed-files-badge">
|
||||
{getStatusLabel(selectedFile.status)}
|
||||
</span>
|
||||
{selectedFile.oldPath ? (
|
||||
<span>Renamed from {selectedFile.oldPath}</span>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
<div className="gm-diff-viewer">
|
||||
@@ -128,6 +258,8 @@ export function ChangedFilesModal({ taskId, worktree, column, projectId, isOpen,
|
||||
</pre>
|
||||
</div>
|
||||
</div>
|
||||
) : !loading && !error && files.length > 0 ? (
|
||||
<div className="file-browser-empty">Select a file to view changes</div>
|
||||
) : null}
|
||||
</section>
|
||||
</div>
|
||||
|
||||
@@ -10,18 +10,25 @@ 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: [
|
||||
{ path: "src/a.ts", status: "modified", 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", diff: "diff --git a/src/b.ts b/src/b.ts" },
|
||||
],
|
||||
files: defaultFiles,
|
||||
loading: false,
|
||||
error: null,
|
||||
selectedFile: { path: "src/a.ts", status: "modified", diff: "diff --git a/src/a.ts b/src/a.ts\n--- a/src/a.ts\n+++ b/src/a.ts\n+hello" },
|
||||
selectedFile: defaultSelectedFile,
|
||||
setSelectedFile: mockSetSelectedFile,
|
||||
resetSelection: mockResetSelection,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -59,7 +66,7 @@ describe("ChangedFilesModal", () => {
|
||||
|
||||
fireEvent.click(screen.getByRole("listitem", { name: /src\/b.ts/i }));
|
||||
|
||||
expect(mockSetSelectedFile).toHaveBeenCalledWith({ path: "src/b.ts", status: "added", diff: "diff --git a/src/b.ts b/src/b.ts" });
|
||||
expect(mockSetSelectedFile).toHaveBeenCalledWith(defaultFiles[1]);
|
||||
});
|
||||
|
||||
it("shows an empty state when there are no changed files", () => {
|
||||
@@ -69,6 +76,7 @@ describe("ChangedFilesModal", () => {
|
||||
error: null,
|
||||
selectedFile: null,
|
||||
setSelectedFile: mockSetSelectedFile,
|
||||
resetSelection: mockResetSelection,
|
||||
});
|
||||
|
||||
render(
|
||||
@@ -99,4 +107,349 @@ describe("ChangedFilesModal", () => {
|
||||
|
||||
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);
|
||||
});
|
||||
|
||||
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 viewing diff", () => {
|
||||
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}
|
||||
/>,
|
||||
);
|
||||
|
||||
// When selectedFile is set, the mobile view should switch to diff
|
||||
// and show the back button. Since the hook returns selectedFile,
|
||||
// the component's isMobile+selectedFile effect will set mobileView to "diff"
|
||||
const backButton = screen.queryByLabelText("Back to file list");
|
||||
expect(backButton).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", () => {
|
||||
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 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();
|
||||
});
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -18,7 +18,7 @@ describe("useChangedFiles", () => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("fetches changed files for active tasks with a worktree and auto-selects first file", async () => {
|
||||
it("fetches changed files for active tasks with a worktree", async () => {
|
||||
mockFetchTaskFileDiffs.mockResolvedValueOnce([
|
||||
{ path: "src/a.ts", status: "modified", diff: "diff --git a/src/a.ts b/src/a.ts" },
|
||||
{ path: "src/b.ts", status: "added", diff: "diff --git a/src/b.ts b/src/b.ts" },
|
||||
@@ -30,7 +30,8 @@ describe("useChangedFiles", () => {
|
||||
|
||||
expect(result.current.error).toBeNull();
|
||||
expect(result.current.files).toHaveLength(2);
|
||||
expect(result.current.selectedFile?.path).toBe("src/a.ts");
|
||||
// Hook no longer auto-selects; component handles selection
|
||||
expect(result.current.selectedFile).toBeNull();
|
||||
expect(mockFetchTaskFileDiffs).toHaveBeenCalledWith("KB-651", undefined);
|
||||
});
|
||||
|
||||
@@ -76,4 +77,55 @@ describe("useChangedFiles", () => {
|
||||
|
||||
expect(result.current.selectedFile?.path).toBe("src/b.ts");
|
||||
});
|
||||
|
||||
it("preserves selection when refetching finds a matching file", async () => {
|
||||
const fileA = { path: "src/a.ts", status: "modified" as const, diff: "first" };
|
||||
const fileB = { path: "src/b.ts", status: "added" as const, diff: "second" };
|
||||
|
||||
mockFetchTaskFileDiffs.mockResolvedValueOnce([fileA, fileB]);
|
||||
|
||||
const { result } = renderHook(() => useChangedFiles("KB-651", "/repo/.worktrees/kb-651", "in-progress"));
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
|
||||
// Manually select file B
|
||||
act(() => {
|
||||
result.current.setSelectedFile(fileB);
|
||||
});
|
||||
|
||||
expect(result.current.selectedFile?.path).toBe("src/b.ts");
|
||||
});
|
||||
|
||||
it("provides resetSelection that clears selectedFile", async () => {
|
||||
mockFetchTaskFileDiffs.mockResolvedValueOnce([
|
||||
{ path: "src/a.ts", status: "modified", diff: "first" },
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useChangedFiles("KB-651", "/repo/.worktrees/kb-651", "in-progress"));
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
|
||||
// Select a file
|
||||
act(() => {
|
||||
result.current.setSelectedFile(result.current.files[0]!);
|
||||
});
|
||||
|
||||
expect(result.current.selectedFile?.path).toBe("src/a.ts");
|
||||
|
||||
// Reset selection
|
||||
act(() => {
|
||||
result.current.resetSelection();
|
||||
});
|
||||
|
||||
expect(result.current.selectedFile).toBeNull();
|
||||
});
|
||||
|
||||
it("returns empty files and null selection when column is inactive", () => {
|
||||
const { result } = renderHook(() => useChangedFiles("KB-651", "/repo/.worktrees/kb-651", "done"));
|
||||
|
||||
expect(result.current.files).toEqual([]);
|
||||
expect(result.current.loading).toBe(false);
|
||||
expect(result.current.selectedFile).toBeNull();
|
||||
expect(result.current.error).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useState } from "react";
|
||||
import { useEffect, useState, useCallback } from "react";
|
||||
import { fetchTaskFileDiffs, type TaskFileDiff } from "../api";
|
||||
|
||||
const ACTIVE_COLUMNS = new Set(["in-progress", "in-review"]);
|
||||
@@ -9,9 +9,15 @@ interface UseChangedFilesResult {
|
||||
error: string | null;
|
||||
selectedFile: TaskFileDiff | null;
|
||||
setSelectedFile: (file: TaskFileDiff) => void;
|
||||
resetSelection: () => void;
|
||||
}
|
||||
|
||||
export function useChangedFiles(taskId: string, worktree: string | undefined, column: string, projectId?: string): UseChangedFilesResult {
|
||||
export function useChangedFiles(
|
||||
taskId: string,
|
||||
worktree: string | undefined,
|
||||
column: string,
|
||||
projectId?: string,
|
||||
): UseChangedFilesResult {
|
||||
const [files, setFiles] = useState<TaskFileDiff[]>([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
@@ -38,10 +44,12 @@ export function useChangedFiles(taskId: string, worktree: string | undefined, co
|
||||
setSelectedFile((current) => {
|
||||
if (result.length === 0) return null;
|
||||
if (current) {
|
||||
const match = result.find((file) => file.path === current.path && file.oldPath === current.oldPath);
|
||||
const match = result.find(
|
||||
(file) => file.path === current.path && file.oldPath === current.oldPath,
|
||||
);
|
||||
if (match) return match;
|
||||
}
|
||||
return result[0] ?? null;
|
||||
return null;
|
||||
});
|
||||
} catch (err) {
|
||||
if (cancelled) return;
|
||||
@@ -62,5 +70,9 @@ export function useChangedFiles(taskId: string, worktree: string | undefined, co
|
||||
};
|
||||
}, [taskId, worktree, column, projectId]);
|
||||
|
||||
return { files, loading, error, selectedFile, setSelectedFile };
|
||||
const resetSelection = useCallback(() => {
|
||||
setSelectedFile(null);
|
||||
}, []);
|
||||
|
||||
return { files, loading, error, selectedFile, setSelectedFile, resetSelection };
|
||||
}
|
||||
|
||||
@@ -3410,34 +3410,81 @@ body {
|
||||
margin-top: 0;
|
||||
}
|
||||
|
||||
/* Mobile responsive for spec tab */
|
||||
/* Back button for changed-files modal - hidden on desktop */
|
||||
.changed-files-back-button {
|
||||
display: none;
|
||||
}
|
||||
|
||||
/* Mobile responsive for changed-files modal */
|
||||
@media (max-width: 768px) {
|
||||
.changed-files-modal .changed-files-layout {
|
||||
grid-template-columns: 1fr;
|
||||
grid-template-rows: 1fr;
|
||||
}
|
||||
|
||||
.changed-files-modal .changed-files-sidebar {
|
||||
/* Mobile two-state view: show only list or diff at a time */
|
||||
.changed-files-sidebar.mobile {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.changed-files-sidebar.mobile.active {
|
||||
display: flex;
|
||||
flex: 1;
|
||||
border-right: none;
|
||||
border-bottom: 1px solid var(--border);
|
||||
max-height: 35vh;
|
||||
border-bottom: none;
|
||||
max-height: none;
|
||||
overflow-y: auto;
|
||||
padding: 0;
|
||||
}
|
||||
|
||||
.changed-files-modal .changed-files-content {
|
||||
.changed-files-sidebar.mobile.active .file-browser-list {
|
||||
flex: 1;
|
||||
overflow-y: auto;
|
||||
}
|
||||
|
||||
.changed-files-content.mobile {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.changed-files-content.mobile.active {
|
||||
display: flex;
|
||||
flex: 1;
|
||||
padding: var(--space-sm);
|
||||
}
|
||||
|
||||
.changed-files-layout .changes-header {
|
||||
flex-direction: column;
|
||||
align-items: flex-start;
|
||||
}
|
||||
|
||||
.changed-files-layout .changes-file-header {
|
||||
.changed-files-modal .file-browser-file-info {
|
||||
flex-wrap: wrap;
|
||||
gap: var(--space-sm);
|
||||
}
|
||||
|
||||
.changed-files-layout .changes-file-stat {
|
||||
width: 100%;
|
||||
padding-left: 30px;
|
||||
.changed-files-modal .file-browser-file-info strong {
|
||||
font-size: 12px;
|
||||
word-break: break-all;
|
||||
}
|
||||
|
||||
/* Back button styles for mobile */
|
||||
.changed-files-back-button {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
gap: var(--space-sm);
|
||||
padding: 6px 12px;
|
||||
background: transparent;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-md);
|
||||
color: var(--text);
|
||||
font-size: 13px;
|
||||
cursor: pointer;
|
||||
transition: background var(--transition-fast);
|
||||
margin-right: 8px;
|
||||
}
|
||||
|
||||
.changed-files-back-button:hover {
|
||||
background: var(--card-hover);
|
||||
}
|
||||
|
||||
.changed-files-back-button:focus {
|
||||
outline: none;
|
||||
box-shadow: var(--focus-ring);
|
||||
}
|
||||
|
||||
.detail-section--spec {
|
||||
@@ -12526,7 +12573,7 @@ html .column.drag-over * {
|
||||
|
||||
.gm-diff-stat {
|
||||
padding: var(--space-sm) var(--space-md);
|
||||
background: rgba(0, 0, 0, 0.1);
|
||||
background: var(--surface);
|
||||
font-family: var(--font-mono);
|
||||
font-size: 11px;
|
||||
color: var(--text-muted);
|
||||
@@ -12552,16 +12599,23 @@ html .column.drag-over * {
|
||||
.gm-diff-loading {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
gap: var(--space-sm);
|
||||
padding: var(--space-lg);
|
||||
color: var(--text-muted);
|
||||
font-size: 13px;
|
||||
background: var(--surface);
|
||||
border-radius: var(--radius-md);
|
||||
border: 1px solid var(--border);
|
||||
}
|
||||
|
||||
.gm-diff-error {
|
||||
padding: var(--space-md);
|
||||
color: var(--color-error);
|
||||
font-size: 13px;
|
||||
background: var(--surface);
|
||||
border-radius: var(--radius-md);
|
||||
border: 1px solid var(--border);
|
||||
}
|
||||
|
||||
.changed-files-layout {
|
||||
@@ -12578,6 +12632,9 @@ html .column.drag-over * {
|
||||
.changed-files-content {
|
||||
padding: var(--space-md);
|
||||
min-width: 0;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
overflow-y: auto;
|
||||
}
|
||||
|
||||
.changed-files-entry {
|
||||
|
||||
Reference in New Issue
Block a user