feat(lanes): full-width Changes, Stage all, one-read expand-all
- The Changes panel's width control gains a full-width step (narrow → 50/50 → 70/30 → full → closed); the console beside it is hidden with CSS, never unmounted. - "Expand all folders" is one GET /api/lanes/:id/git/tree-all (one `git ls-files`, every level shaped like /git/tree) instead of one request and one git process per folder. - Overview: a Stage all button and the staged-only commit box sit above the file list and stay pinned while it scrolls; the Files finder is pinned too.
This commit is contained in:
@@ -60,8 +60,6 @@ const OVERVIEW_REFRESH_MS = 15_000;
|
||||
const CONTEXT_STEPS: readonly [number, ...number[]] = [3, 12, 40, 5000];
|
||||
|
||||
const LAST_CONTEXT = CONTEXT_STEPS[CONTEXT_STEPS.length - 1]!;
|
||||
/** Tree reads "expand all" keeps in flight at once. */
|
||||
const TREE_CONCURRENCY = 4;
|
||||
|
||||
type Mode = "overview" | "review" | "file" | "history" | "browse";
|
||||
|
||||
@@ -425,47 +423,18 @@ export function LaneChangesPanel({
|
||||
};
|
||||
|
||||
const [treeBusy, setTreeBusy] = useState(false);
|
||||
// Which lane `treeChildren` was filled for; the reset effect moves it.
|
||||
const treeLane = useRef(laneId);
|
||||
|
||||
// Fetches whatever level isn't cached yet, depth-first, so "expand all"
|
||||
// works even on a tree nothing has been opened in.
|
||||
// "Expand all" is ONE read of the whole tree: a request per folder meant a
|
||||
// `git ls-files` per folder, seconds of waiting on a large repo.
|
||||
const expandAllDirs = async () => {
|
||||
// A walk is many reads long; a lane switch mid-walk must not land lane A's
|
||||
// tree in lane B, nor start from a cache the switch has not cleared yet.
|
||||
const lane = laneId;
|
||||
setTreeBusy(true);
|
||||
const cache = treeLane.current === lane ? { ...treeChildren } : {};
|
||||
const toExpand = new Set(expandedDirs);
|
||||
// Level by level, at most TREE_CONCURRENCY reads in flight: one request
|
||||
// per folder all at once is thousands on a monorepo.
|
||||
const visit = async (path: string, next: string[]) => {
|
||||
let entries = cache[path];
|
||||
if (!entries) {
|
||||
entries = (await api.lanes.gitTree(lane, path)).entries;
|
||||
cache[path] = entries;
|
||||
}
|
||||
for (const e of entries) {
|
||||
if (!e.dir) continue;
|
||||
toExpand.add(e.path);
|
||||
next.push(e.path);
|
||||
}
|
||||
};
|
||||
try {
|
||||
for (let level = [""]; level.length && laneNow.current === lane; ) {
|
||||
const next: string[] = [];
|
||||
// Workers share one iterator, so each path is taken exactly once.
|
||||
const it = level.values();
|
||||
await Promise.all(
|
||||
Array.from({ length: TREE_CONCURRENCY }, async () => {
|
||||
for (const path of it) await visit(path, next);
|
||||
})
|
||||
);
|
||||
level = next;
|
||||
}
|
||||
const { levels } = await api.lanes.gitTreeAll(lane);
|
||||
// A lane switch mid-read must not land lane A's tree in lane B.
|
||||
if (laneNow.current !== lane) return;
|
||||
setTreeChildren(cache);
|
||||
setExpandedDirs(toExpand);
|
||||
setTreeChildren(levels);
|
||||
setExpandedDirs(new Set(Object.keys(levels).filter(Boolean)));
|
||||
} catch (err) {
|
||||
if (laneNow.current === lane) setError(err instanceof Error ? err.message : String(err));
|
||||
} finally {
|
||||
@@ -516,7 +485,6 @@ export function LaneChangesPanel({
|
||||
setSha(null);
|
||||
setRange("work");
|
||||
setTreeChildren({});
|
||||
treeLane.current = laneId;
|
||||
setExpandedDirs(new Set());
|
||||
setTreeBusy(false);
|
||||
setBusy(false);
|
||||
@@ -561,12 +529,17 @@ export function LaneChangesPanel({
|
||||
// An action that resolves after a lane switch belongs to the lane left: its
|
||||
// `load` is the pre-switch one and would take a fresh ticket and paint the
|
||||
// old lane's files here, so everything after the await is dropped.
|
||||
const act = async (action: "stage" | "unstage" | "discard", path: string, confirm?: boolean) => {
|
||||
const act = async (
|
||||
action: "stage" | "unstage" | "discard",
|
||||
path: string | string[],
|
||||
confirm?: boolean
|
||||
) => {
|
||||
const lane = laneId;
|
||||
setBusy(true);
|
||||
setError(null);
|
||||
try {
|
||||
await api.lanes.gitAction(lane, action, { paths: [path], ...(confirm ? { confirm } : {}) });
|
||||
const paths = Array.isArray(path) ? path : [path];
|
||||
await api.lanes.gitAction(lane, action, { paths, ...(confirm ? { confirm } : {}) });
|
||||
if (laneNow.current !== lane) return;
|
||||
setConfirmDiscard(null);
|
||||
refreshGit();
|
||||
@@ -617,6 +590,8 @@ export function LaneChangesPanel({
|
||||
const files = data?.available ? data.files : [];
|
||||
const allCollapsed = files.length > 0 && files.every((f) => collapsed.has(f.path));
|
||||
const stagedCount = range === "work" ? files.filter((f) => f.staged).length : 0;
|
||||
// What "Stage all" sends: every listed file with a change the index lacks.
|
||||
const toStage = range === "work" ? files.filter((f) => f.untracked || f.unstaged) : [];
|
||||
const current = mode === "file" ? files.find((f) => f.path === openFile) : undefined;
|
||||
|
||||
// The armed path left the list (committed, reverted, lane switched): disarm,
|
||||
@@ -836,7 +811,9 @@ export function LaneChangesPanel({
|
||||
</ul>
|
||||
) : mode === "browse" ? (
|
||||
<div data-testid="lane-changes-browse">
|
||||
<div className="mb-2 flex items-center gap-1.5">
|
||||
{/* Pinned: the finder is what you reach for after scrolling a
|
||||
long tree, not only from its top. */}
|
||||
<div className="sticky top-0 z-10 flex items-center gap-1.5 bg-surface-3 pb-2">
|
||||
<input
|
||||
value={query}
|
||||
onChange={(e) => setQuery(e.target.value)}
|
||||
@@ -908,6 +885,54 @@ export function LaneChangesPanel({
|
||||
<p className="text-[11px] text-fg-muted">{t(`changes.empty.${range}`)}</p>
|
||||
) : mode === "overview" ? (
|
||||
<>
|
||||
{/* Commit and Stage all sit ABOVE the list and stay pinned while it
|
||||
scrolls: on a 60-file change-set the box at the bottom was a
|
||||
scroll away from every file you had just staged. */}
|
||||
{(stagedCount > 0 || toStage.length > 0) && (
|
||||
<div
|
||||
data-testid="lane-changes-actions"
|
||||
className="sticky top-0 z-10 mb-1.5 space-y-1.5 border-b border-border-light bg-surface-3 pb-2"
|
||||
>
|
||||
{stagedCount > 0 && (
|
||||
<div data-testid="lane-changes-commit" className="flex items-center gap-1.5">
|
||||
<input
|
||||
value={message}
|
||||
onChange={(e) => setMessage(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" && message.trim() && !busy) void commitStaged();
|
||||
}}
|
||||
placeholder={t("changes.commitPlaceholder")}
|
||||
aria-label={t("changes.commitPlaceholder")}
|
||||
className="min-w-0 flex-1 rounded border border-border-light bg-surface-0 px-2 py-0.5 text-[11px] text-fg-primary placeholder:text-fg-muted focus:border-accent focus:outline-none"
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
disabled={busy || !message.trim()}
|
||||
onClick={() => void commitStaged()}
|
||||
className="shrink-0 rounded border border-border-light px-1.5 py-0.5 text-[11px] text-fg-secondary hover:bg-surface-2 hover:text-fg-primary disabled:cursor-not-allowed disabled:opacity-40"
|
||||
>
|
||||
{t("changes.commitStaged", { count: stagedCount })}
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
{toStage.length > 0 && (
|
||||
<button
|
||||
type="button"
|
||||
disabled={busy}
|
||||
onClick={() =>
|
||||
void act(
|
||||
"stage",
|
||||
toStage.map((f) => f.path)
|
||||
)
|
||||
}
|
||||
className="flex items-center gap-1 rounded border border-border-light px-1.5 py-0.5 text-[11px] text-fg-secondary hover:bg-surface-2 hover:text-fg-primary disabled:cursor-not-allowed disabled:opacity-40"
|
||||
>
|
||||
<Plus className="h-3 w-3" />
|
||||
{t("changes.stageAll", { count: toStage.length })}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
<ul data-testid="lane-changes-overview" className="space-y-0.5">
|
||||
{files.map((f) => (
|
||||
<ChangeRow
|
||||
@@ -922,31 +947,6 @@ export function LaneChangesPanel({
|
||||
/>
|
||||
))}
|
||||
</ul>
|
||||
{stagedCount > 0 && (
|
||||
<div
|
||||
data-testid="lane-changes-commit"
|
||||
className="mt-2 flex items-center gap-1.5 border-t border-border-light pt-2"
|
||||
>
|
||||
<input
|
||||
value={message}
|
||||
onChange={(e) => setMessage(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" && message.trim() && !busy) void commitStaged();
|
||||
}}
|
||||
placeholder={t("changes.commitPlaceholder")}
|
||||
aria-label={t("changes.commitPlaceholder")}
|
||||
className="min-w-0 flex-1 rounded border border-border-light bg-surface-0 px-2 py-0.5 text-[11px] text-fg-primary placeholder:text-fg-muted focus:border-accent focus:outline-none"
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
disabled={busy || !message.trim()}
|
||||
onClick={() => void commitStaged()}
|
||||
className="shrink-0 rounded border border-border-light px-1.5 py-0.5 text-[11px] text-fg-secondary hover:bg-surface-2 hover:text-fg-primary disabled:cursor-not-allowed disabled:opacity-40"
|
||||
>
|
||||
{t("changes.commitStaged", { count: stagedCount })}
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
) : mode === "file" ? (
|
||||
current || (whole && text) ? (
|
||||
|
||||
@@ -28,6 +28,7 @@ vi.mock("../../../lib/api", () => ({
|
||||
gitFile: vi.fn(),
|
||||
gitLs: vi.fn(),
|
||||
gitTree: vi.fn(),
|
||||
gitTreeAll: vi.fn(),
|
||||
},
|
||||
},
|
||||
}));
|
||||
@@ -283,6 +284,22 @@ describe("LaneChangesPanel", () => {
|
||||
await waitFor(() => expect(api.lanes.gitLog).toHaveBeenCalledTimes(1));
|
||||
});
|
||||
|
||||
it("stages every file the index lacks in one call, from above the list", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<LaneChangesPanel laneId={7} />);
|
||||
const list = await screen.findByTestId("lane-changes-overview");
|
||||
const actions = screen.getByTestId("lane-changes-actions");
|
||||
// Commit box and Stage all come BEFORE the file list.
|
||||
expect(actions.compareDocumentPosition(list) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
|
||||
expect(screen.getByTestId("lane-changes-commit")).toBeInTheDocument();
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Stage all (2)" }));
|
||||
// The already-staged docs/API.md is not sent again.
|
||||
expect(api.lanes.gitAction).toHaveBeenCalledWith(7, "stage", {
|
||||
paths: ["server/lib/git-ops.js", "notes.txt"],
|
||||
});
|
||||
});
|
||||
|
||||
it("needs two clicks to discard, and only the second one reaches the server", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<LaneChangesPanel laneId={7} />);
|
||||
@@ -417,8 +434,16 @@ describe("LaneChangesPanel", () => {
|
||||
expect(screen.getByText("src")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("expands every folder at once, fetching whatever level wasn't cached yet", async () => {
|
||||
it("expands every folder at once from one whole-tree read", async () => {
|
||||
const user = userEvent.setup();
|
||||
vi.mocked(api.lanes.gitTreeAll).mockResolvedValue({
|
||||
available: true,
|
||||
truncated: false,
|
||||
levels: {
|
||||
"": [{ name: "src", path: "src", dir: true }],
|
||||
src: [{ name: "main.ts", path: "src/main.ts", dir: false }],
|
||||
},
|
||||
});
|
||||
vi.mocked(api.lanes.gitTree).mockImplementation(async (_id, path) => {
|
||||
if (path === "src")
|
||||
return {
|
||||
@@ -788,37 +813,29 @@ describe("LaneChangesPanel", () => {
|
||||
expect(screen.getByText("server/lib/git-ops.js")).toBe(before);
|
||||
});
|
||||
|
||||
it("expands all folders with at most four tree reads in flight", async () => {
|
||||
it("expands all folders with ONE request, not one per folder", async () => {
|
||||
const user = userEvent.setup();
|
||||
let inFlight = 0;
|
||||
let peak = 0;
|
||||
vi.mocked(api.lanes.gitTree).mockClear();
|
||||
vi.mocked(api.lanes.gitTree).mockImplementation(async (_id, path = "") => {
|
||||
inFlight += 1;
|
||||
peak = Math.max(peak, inFlight);
|
||||
await new Promise((r) => setTimeout(r, 5));
|
||||
inFlight -= 1;
|
||||
const depth = path ? path.split("/").length : 0;
|
||||
const entries =
|
||||
depth >= 2
|
||||
? [{ name: "leaf.ts", path: `${path}/leaf.ts`, dir: false }]
|
||||
: Array.from({ length: 5 }, (_, i) => ({
|
||||
name: `d${i}`,
|
||||
path: path ? `${path}/d${i}` : `d${i}`,
|
||||
dir: true,
|
||||
}));
|
||||
return { available: true, path, entries, truncated: false };
|
||||
});
|
||||
vi.mocked(api.lanes.gitTreeAll)
|
||||
.mockClear()
|
||||
.mockResolvedValue({
|
||||
available: true,
|
||||
truncated: false,
|
||||
levels: {
|
||||
"": [{ name: "src", path: "src", dir: true }],
|
||||
src: [{ name: "main.ts", path: "src/main.ts", dir: false }],
|
||||
},
|
||||
});
|
||||
render(<LaneChangesPanel laneId={7} />);
|
||||
await screen.findByTestId("lane-changes-overview");
|
||||
await user.click(screen.getByRole("button", { name: "Files" }));
|
||||
await screen.findByText("d0");
|
||||
await screen.findByText("src");
|
||||
await user.click(screen.getByRole("button", { name: "Expand all folders" }));
|
||||
|
||||
await waitFor(() => expect(screen.getAllByText("leaf.ts")).toHaveLength(25));
|
||||
// Root + 5 + 25 folders, each fetched exactly once.
|
||||
expect(api.lanes.gitTree).toHaveBeenCalledTimes(31);
|
||||
expect(peak).toBeLessThanOrEqual(4);
|
||||
expect(await screen.findByText("main.ts")).toBeInTheDocument();
|
||||
expect(api.lanes.gitTreeAll).toHaveBeenCalledTimes(1);
|
||||
// Only the root read the Files tab made on entry — no per-folder walk.
|
||||
expect(api.lanes.gitTree).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not repaint the previous lane after its git action resolves post-switch", async () => {
|
||||
@@ -861,21 +878,25 @@ describe("LaneChangesPanel", () => {
|
||||
expect(screen.queryByText("server/lib/git-ops.js")).toBeNull();
|
||||
});
|
||||
|
||||
it("does not paint the previous lane's tree when an expand-all walk outlives a lane switch", async () => {
|
||||
it("does not paint the previous lane's tree when an expand-all read outlives a lane switch", async () => {
|
||||
const user = userEvent.setup();
|
||||
let resolveSrc: () => void = () => {};
|
||||
vi.mocked(api.lanes.gitTree).mockImplementation((id, path) => {
|
||||
if (id === 7 && path === "src")
|
||||
return new Promise(
|
||||
vi.mocked(api.lanes.gitTreeAll).mockImplementation(
|
||||
() =>
|
||||
new Promise(
|
||||
(r) =>
|
||||
(resolveSrc = () =>
|
||||
r({
|
||||
available: true,
|
||||
path: "src",
|
||||
entries: [{ name: "main.ts", path: "src/main.ts", dir: false }],
|
||||
truncated: false,
|
||||
levels: {
|
||||
"": [{ name: "src", path: "src", dir: true }],
|
||||
src: [{ name: "main.ts", path: "src/main.ts", dir: false }],
|
||||
},
|
||||
}))
|
||||
);
|
||||
)
|
||||
);
|
||||
vi.mocked(api.lanes.gitTree).mockImplementation((id) => {
|
||||
return Promise.resolve({
|
||||
available: true,
|
||||
path: "",
|
||||
|
||||
@@ -110,6 +110,8 @@
|
||||
"changes.size.compact": "Narrow",
|
||||
"changes.size.half": "50 / 50",
|
||||
"changes.size.wide": "70 / 30",
|
||||
"changes.size.full": "Full width",
|
||||
"changes.stageAll": "Stage all ({{count}})",
|
||||
"changes.stage": "Stage this file",
|
||||
"changes.title": "Changes",
|
||||
"changes.truncated": "Some changes were left out to keep this panel responsive.",
|
||||
|
||||
@@ -110,6 +110,8 @@
|
||||
"changes.size.compact": "Hẹp",
|
||||
"changes.size.half": "50 / 50",
|
||||
"changes.size.wide": "70 / 30",
|
||||
"changes.size.full": "Toàn màn hình",
|
||||
"changes.stageAll": "Stage tất cả ({{count}})",
|
||||
"changes.stage": "Đưa file này vào index",
|
||||
"changes.title": "Thay đổi",
|
||||
"changes.truncated": "Một phần thay đổi đã bị lược bớt để bảng này còn mượt.",
|
||||
|
||||
@@ -2462,6 +2462,16 @@ export const api = {
|
||||
* @param id The lane id.
|
||||
* @param path Repo-relative directory; "" for the repo root.
|
||||
*/
|
||||
/**
|
||||
* GET /api/lanes/:id/git/tree-all — every level of the tracked tree in one
|
||||
* read, keyed by directory ("" is the root), each shaped like `gitTree`'s.
|
||||
*/
|
||||
gitTreeAll: (id: number) =>
|
||||
request<{
|
||||
available: boolean;
|
||||
levels: Record<string, LaneTreeEntry[]>;
|
||||
truncated: boolean;
|
||||
}>(`/lanes/${id}/git/tree-all`),
|
||||
gitTree: (id: number, path = "") =>
|
||||
request<{
|
||||
available: boolean;
|
||||
|
||||
@@ -53,6 +53,7 @@ import {
|
||||
PanelLeftClose,
|
||||
ChevronsLeftRight,
|
||||
PanelRightClose,
|
||||
Maximize2,
|
||||
} from "lucide-react";
|
||||
import { api } from "../lib/api";
|
||||
import type { CwdSuggestion, RunListResponse } from "../lib/api";
|
||||
@@ -88,21 +89,25 @@ const CHANGES_SIZE_KEY = "agenthub.workspace.changesSize";
|
||||
/** The three widths the Changes panel cycles through, then closed. Reading a
|
||||
* diff and watching a console want opposite amounts of room, and which one is
|
||||
* "work" changes several times an hour — hence ONE control that steps
|
||||
* narrow → 50/50 → 70/30 → closed, rather than a width button beside a
|
||||
* narrow → 50/50 → 70/30 → full → closed, rather than a width button beside a
|
||||
* collapse button that both live in the panel's tightest corner. */
|
||||
const CHANGES_SIZES = ["compact", "half", "wide"] as const;
|
||||
const CHANGES_SIZES = ["compact", "half", "wide", "full"] as const;
|
||||
type ChangesSize = (typeof CHANGES_SIZES)[number];
|
||||
const CHANGES_WIDTH: Record<ChangesSize, string> = {
|
||||
compact: "w-[28rem] xl:w-[34rem]",
|
||||
half: "w-1/2",
|
||||
wide: "w-[70%]",
|
||||
// The console beside it is HIDDEN, not unmounted: a live run's terminal
|
||||
// and a half-typed prompt must survive a round trip through full width.
|
||||
full: "w-full",
|
||||
};
|
||||
/** The icon the one resize control shows AT each size: what the next click
|
||||
* gives you — an even split, a wider panel, then closed. */
|
||||
const CHANGES_NEXT_ICON: Record<ChangesSize, typeof Columns2> = {
|
||||
compact: Columns2,
|
||||
half: ChevronsLeftRight,
|
||||
wide: PanelRightClose,
|
||||
wide: Maximize2,
|
||||
full: PanelRightClose,
|
||||
};
|
||||
|
||||
// ── Page ──────────────────────────────────────────────────────────────
|
||||
@@ -733,7 +738,9 @@ export function Workspace() {
|
||||
<div className="flex min-h-0 flex-1 gap-4">
|
||||
<section
|
||||
data-testid="lane-detail"
|
||||
className={`flex min-h-0 min-w-0 flex-1 flex-col ${currentLane ? "card p-4" : ""}`}
|
||||
className={`flex min-h-0 min-w-0 flex-1 flex-col ${currentLane ? "card p-4" : ""}${
|
||||
currentLane && changesOpen && changesSize === "full" ? " hidden" : ""
|
||||
}`}
|
||||
>
|
||||
{/* One header row, not two. The pipeline picker, the stage badges,
|
||||
the feature picker and the router select used to sit on a second
|
||||
|
||||
@@ -142,7 +142,7 @@ describe("Workspace — the Changes panel's width", () => {
|
||||
}
|
||||
}
|
||||
|
||||
it("steps narrow → 50/50 → 70/30 → closed on one control", async () => {
|
||||
it("steps narrow → 50/50 → 70/30 → full → closed on one control", async () => {
|
||||
await openPanel();
|
||||
const card = () => screen.getByTestId("lane-changes-card");
|
||||
expect(card().className).toContain("w-[28rem]");
|
||||
@@ -153,6 +153,11 @@ describe("Workspace — the Changes panel's width", () => {
|
||||
fireEvent.click(screen.getByTestId("lane-changes-resize"));
|
||||
expect(card().className).toContain("w-[70%]");
|
||||
|
||||
// Full width HIDES the console beside it — hidden, still mounted.
|
||||
fireEvent.click(screen.getByTestId("lane-changes-resize"));
|
||||
expect(card().className).toContain("w-full");
|
||||
expect(screen.getByTestId("lane-detail").className).toContain("hidden");
|
||||
|
||||
// Past the widest step the panel closes — and reopens narrow, not back at
|
||||
// 70/30, which is why closing resets the width.
|
||||
fireEvent.click(screen.getByTestId("lane-changes-resize"));
|
||||
|
||||
Reference in New Issue
Block a user