feat(lanes): stage/undo one hunk at a time, sticky review file headers

Review and the single-file view gain per-hunk Stage and Undo (stage-hunk / discard-hunk: the hunk is named by its @@ line and re-matched against a fresh diff, then git apply --cached / -R; undo needs confirm: true and two clicks). Each review file header stays pinned while its diff scrolls and carries the file's own stage/unstage/discard.
2026-09-24 14:38:48 +07:00
parent 0737786faf
commit cfe832383c
17 changed files with 546 additions and 97 deletions
@@ -41,6 +41,7 @@ import {
  RefreshCw,
  Rows3,
  Trash2,
+  Undo2,
  UnfoldVertical,
 } from "lucide-react";
 import { api } from "../../lib/api";
@@ -51,6 +52,7 @@ import type {
  LaneChangeRange,
  LaneChanges,
  LaneCommit,
+  LaneDiffHunk,
  LaneTreeEntry,
 } from "../../lib/types";

@@ -192,6 +194,62 @@ function IconButton({
  );
 }

+type FileAct = (action: "stage" | "unstage" | "discard", path: string, confirm?: boolean) => void;
+
+/** A work-range file's stage / unstage / discard buttons — the overview row's
+ *  and the review header's. */
+function FileActions({
+  f,
+  busy,
+  armed,
+  onAct,
+  onArm,
+}: {
+  f: LaneChangeFile;
+  busy: boolean;
+  armed: boolean;
+  onAct: FileAct;
+  onArm: (path: string) => void;
+}) {
+  const { t } = useTranslation(["lanes"]);
+  return (
+    <span className="flex shrink-0 items-center gap-0.5">
+      {f.staged ? (
+        <IconButton
+          label={t("changes.unstage")}
+          disabled={busy}
+          onClick={() => onAct("unstage", f.path)}
+        >
+          <Minus className="h-3 w-3" />
+        </IconButton>
+      ) : null}
+      {f.unstaged || f.untracked ? (
+        <IconButton
+          label={t("changes.stage")}
+          disabled={busy}
+          onClick={() => onAct("stage", f.path)}
+        >
+          <Plus className="h-3 w-3" />
+        </IconButton>
+      ) : null}
+      <IconButton
+        label={armed ? t("changes.discardConfirm") : t("changes.discard")}
+        disabled={busy}
+        danger
+        onClick={() => {
+          // Two clicks, always: the first arms the row, the second is the
+          // only one that reaches the server (which itself refuses the
+          // call without `confirm: true`).
+          if (armed) onAct("discard", f.path, true);
+          else onArm(f.path);
+        }}
+      >
+        <Trash2 className={`h-3 w-3 ${armed ? "text-status-danger" : ""}`} />
+      </IconButton>
+    </span>
+  );
+}
+
 /** One overview row. At module scope on purpose: declared inside the panel it
 *  was a new component type every render, so each poll and each keystroke in
 *  the commit box remounted every row (focus dropped to <body>). */
@@ -210,7 +268,7 @@ function ChangeRow({
  /** This row's discard has been clicked once and awaits the second click. */
  armed: boolean;
  onOpen: (path: string) => void;
-  onAct: (action: "stage" | "unstage" | "discard", path: string, confirm?: boolean) => void;
+  onAct: FileAct;
  onArm: (path: string) => void;
 }) {
  const { t } = useTranslation(["lanes"]);
@@ -241,40 +299,7 @@ function ChangeRow({
        <Delta added={f.added} deleted={f.deleted} binary={f.binary} />
      </button>
      {range === "work" && (
-        <span className="flex shrink-0 items-center gap-0.5">
-          {f.staged ? (
-            <IconButton
-              label={t("changes.unstage")}
-              disabled={busy}
-              onClick={() => onAct("unstage", f.path)}
-            >
-              <Minus className="h-3 w-3" />
-            </IconButton>
-          ) : null}
-          {f.unstaged || f.untracked ? (
-            <IconButton
-              label={t("changes.stage")}
-              disabled={busy}
-              onClick={() => onAct("stage", f.path)}
-            >
-              <Plus className="h-3 w-3" />
-            </IconButton>
-          ) : null}
-          <IconButton
-            label={armed ? t("changes.discardConfirm") : t("changes.discard")}
-            disabled={busy}
-            danger
-            onClick={() => {
-              // Two clicks, always: the first arms the row, the second is the
-              // only one that reaches the server (which itself refuses the
-              // call without `confirm: true`).
-              if (armed) onAct("discard", f.path, true);
-              else onArm(f.path);
-            }}
-          >
-            <Trash2 className={`h-3 w-3 ${armed ? "text-status-danger" : ""}`} />
-          </IconButton>
-        </span>
+        <FileActions f={f} busy={busy} armed={armed} onAct={onAct} onArm={onArm} />
      )}
    </li>
  );
@@ -316,6 +341,8 @@ export function LaneChangesPanel({
  const [loading, setLoading] = useState(false);
  const [busy, setBusy] = useState(false);
  const [confirmDiscard, setConfirmDiscard] = useState<string | null>(null);
+  // The hunk whose Undo was clicked once: `${path}\n${header}`.
+  const [armedHunk, setArmedHunk] = useState<string | null>(null);
  // Files folded shut in review mode. A change-set is read file by file, and
  // a file already read is in the way of the next one — so the set is what is
  // COLLAPSED, and a file arriving later is open by default.
@@ -551,6 +578,62 @@ export function LaneChangesPanel({
    }
  };

+  // One hunk, matched by its `@@` line against a fresh diff at the context
+  // this diff was read with — so the server never applies a stale patch.
+  const actHunk = async (action: "stage-hunk" | "discard-hunk", path: string, hunk: string) => {
+    const lane = laneId;
+    setBusy(true);
+    setError(null);
+    try {
+      await api.lanes.gitAction(lane, action, {
+        path,
+        hunk,
+        context: data?.available ? data.context : undefined,
+        ...(action === "discard-hunk" ? { confirm: true } : {}),
+      });
+      if (laneNow.current !== lane) return;
+      setArmedHunk(null);
+      refreshGit();
+      await load();
+    } catch (err) {
+      if (laneNow.current === lane) setError(err instanceof Error ? err.message : String(err));
+    } finally {
+      if (laneNow.current === lane) setBusy(false);
+    }
+  };
+
+  // Stage / Undo on each hunk of a file with unstaged changes. The diff is
+  // HEAD → working tree, so a staged-only file gets none: undoing its tree
+  // would leave the index holding the change.
+  const hunkActions = (f: LaneChangeFile) =>
+    range === "work" && f.unstaged && !f.untracked && !f.binary
+      ? (hunk: LaneDiffHunk) => {
+          const key = `${f.path}\n${hunk.header}`;
+          const armed = armedHunk === key;
+          return (
+            <>
+              <IconButton
+                label={t("changes.stageHunk")}
+                disabled={busy}
+                onClick={() => void actHunk("stage-hunk", f.path, hunk.header)}
+              >
+                <Plus className="h-3 w-3" />
+              </IconButton>
+              <IconButton
+                label={armed ? t("changes.undoHunkConfirm") : t("changes.undoHunk")}
+                disabled={busy}
+                danger
+                onClick={() =>
+                  armed ? void actHunk("discard-hunk", f.path, hunk.header) : setArmedHunk(key)
+                }
+              >
+                <Undo2 className={`h-3 w-3 ${armed ? "text-status-danger" : ""}`} />
+              </IconButton>
+            </>
+          );
+        }
+      : undefined;
+
  const commitStaged = async () => {
    const lane = laneId;
    setBusy(true);
@@ -972,7 +1055,12 @@ export function LaneChangesPanel({
                  <p className="text-[11px] text-fg-muted">{t("changes.loading")}</p>
                )
              ) : current ? (
-                <LaneDiffView file={current} layout={layout} fill />
+                <LaneDiffView
+                  file={current}
+                  layout={layout}
+                  fill
+                  hunkActions={whole ? undefined : hunkActions(current)}
+                />
              ) : null}
            </div>
          ) : (
@@ -1006,7 +1094,7 @@ export function LaneChangesPanel({
                        ? t("changes.expandFile", { path: f.path })
                        : t("changes.collapseFile", { path: f.path })
                    }
-                    className={`flex cursor-pointer items-baseline gap-1.5 bg-surface-1 px-2 py-1 hover:bg-surface-2 ${
+                    className={`sticky top-0 z-10 flex cursor-pointer items-baseline gap-1.5 rounded-t bg-surface-1 px-2 py-1 hover:bg-surface-2 ${
                      shut ? "" : "border-b border-border-light"
                    }`}
                  >
@@ -1032,8 +1120,20 @@ export function LaneChangesPanel({
                      {f.path}
                    </button>
                    <Delta added={f.added} deleted={f.deleted} binary={f.binary} />
+                    {range === "work" && (
+                      // The header folds on click; its buttons must not.
+                      <span className="self-center" onClick={(e) => e.stopPropagation()}>
+                        <FileActions
+                          f={f}
+                          busy={busy}
+                          armed={confirmDiscard === f.path}
+                          onAct={(...a) => void act(...a)}
+                          onArm={setConfirmDiscard}
+                        />
+                      </span>
+                    )}
                  </div>
-                  {!shut && <LaneDiffView file={f} layout={layout} />}
+                  {!shut && <LaneDiffView file={f} layout={layout} hunkActions={hunkActions(f)} />}
                </div>
              );
            })}
@@ -21,7 +21,15 @@
 * @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
 */

-import { useMemo, useRef, useState, type CSSProperties } from "react";
+import {
+  createContext,
+  useContext,
+  useMemo,
+  useRef,
+  useState,
+  type CSSProperties,
+  type ReactNode,
+} from "react";
 import { useTranslation } from "react-i18next";
 import { canonicalLang, highlight, tokenClass } from "../../lib/highlight";
 import type { LaneChangeFile, LaneDiffHunk, LaneDiffLine } from "../../lib/types";
@@ -276,19 +284,38 @@ function markerClass(type: LaneDiffLine["type"]) {
  return "text-fg-muted";
 }

-function HunkHeader({ hunk, fixed }: { hunk: LaneDiffHunk; fixed?: boolean }) {
+/** Per-hunk controls (stage / undo) beside each `@@` line. A context rather
+ *  than a prop because the header renders from four places. */
+const HunkActions = createContext<((hunk: LaneDiffHunk) => ReactNode) | null>(null);
+
+function HunkHeader({
+  hunk,
+  fixed,
+  noActions,
+}: {
+  hunk: LaneDiffHunk;
+  fixed?: boolean;
+  /** The split view's left column: one set of controls per hunk, not two. */
+  noActions?: boolean;
+}) {
+  const actions = useContext(HunkActions);
  const at = hunk.header.split("@@")[1];
  // Windowed, the header is one ROW_PX row like the lines around it.
  return (
    <div
-      className={`bg-surface-2 px-2 text-[10px] text-fg-muted ${
+      className={`flex items-center gap-2 bg-surface-2 px-2 text-[10px] text-fg-muted ${
        fixed ? "overflow-hidden whitespace-nowrap leading-4" : "py-0.5"
      }`}
      style={fixed ? FIXED : undefined}
      data-diff-item={fixed ? "" : undefined}
    >
-      {at ? `@@${at}@@` : hunk.header}
-      {hunk.context ? ` ${hunk.context}` : ""}
+      <span className="min-w-0 flex-1 truncate">
+        {at ? `@@${at}@@` : hunk.header}
+        {hunk.context ? ` ${hunk.context}` : ""}
+      </span>
+      {actions && !noActions && (
+        <span className="sticky right-2 flex shrink-0 items-center gap-0.5">{actions(hunk)}</span>
+      )}
    </div>
  );
 }
@@ -394,7 +421,7 @@ function SplitColumn({
    <>
      {hunks.map((hunk, h) => (
        <div key={`${hunk.header}:${h}`}>
-          <HunkHeader hunk={hunk} />
+          <HunkHeader hunk={hunk} noActions={side === "left"} />
          {analyseHunk(hunk).rows.map((row, i) => (
            <SplitLine key={i} row={row} lang={lang} side={side} />
          ))}
@@ -422,7 +449,7 @@ function SplitSlice({
        .slice(range.start, range.end)
        .map(({ hunk, i }, k) =>
          i < 0 ? (
-            <HunkHeader key={range.start + k} hunk={hunk} fixed />
+            <HunkHeader key={range.start + k} hunk={hunk} fixed noActions={side === "left"} />
          ) : (
            <SplitLine
              key={range.start + k}
@@ -520,11 +547,14 @@ export function LaneDiffView({
  file,
  layout,
  fill = false,
+  hunkActions,
 }: {
  file: LaneChangeFile;
  layout: DiffLayout;
  /** Take the parent's height (single-file view) instead of the 60vh cap. */
  fill?: boolean;
+  /** Controls drawn on each hunk's header row. */
+  hunkActions?: (hunk: LaneDiffHunk) => ReactNode;
 }) {
  const { t } = useTranslation(["lanes"]);
  const lang = langFor(file.path);
@@ -544,43 +574,45 @@ export function LaneDiffView({
    return <p className="px-2 py-1 text-[10px] text-fg-muted">{t("changes.noHunks")}</p>;

  return (
-    <div
-      data-testid="lane-diff-view"
-      data-layout={layout}
-      onScroll={(e) => win.onScroll(e.currentTarget)}
-      className={`flex flex-col font-mono text-[11px] leading-[1.45] ${
-        layout === "split" ? boxHeight(fill) : `overflow-auto ${boxHeight(fill)}`
-      }`}
-    >
-      {layout === "split" ? (
-        <SplitView hunks={file.hunks} lang={lang} fill={fill} />
-      ) : win.on ? (
-        <Windowed count={items.length} range={win.range} minWidth={minWidth}>
-          {items
-            .slice(win.range.start, win.range.end)
-            .map(({ hunk, i }, k) =>
-              i < 0 ? (
-                <HunkHeader key={win.range.start + k} hunk={hunk} fixed />
-              ) : (
-                <UnifiedLine
-                  key={win.range.start + k}
-                  line={hunk.lines[i]!}
-                  lang={lang}
-                  emph={analyseHunk(hunk).emph.get(i)}
-                  style={FIXED}
-                />
-              )
-            )}
-        </Windowed>
-      ) : (
-        file.hunks.map((hunk, i) => (
-          <UnifiedHunk key={`${hunk.header}:${i}`} hunk={hunk} lang={lang} />
-        ))
-      )}
-      {file.truncated && (
-        <p className="px-2 py-1 text-[10px] text-status-warning">{t("changes.fileTruncated")}</p>
-      )}
-    </div>
+    <HunkActions.Provider value={hunkActions ?? null}>
+      <div
+        data-testid="lane-diff-view"
+        data-layout={layout}
+        onScroll={(e) => win.onScroll(e.currentTarget)}
+        className={`flex flex-col font-mono text-[11px] leading-[1.45] ${
+          layout === "split" ? boxHeight(fill) : `overflow-auto ${boxHeight(fill)}`
+        }`}
+      >
+        {layout === "split" ? (
+          <SplitView hunks={file.hunks} lang={lang} fill={fill} />
+        ) : win.on ? (
+          <Windowed count={items.length} range={win.range} minWidth={minWidth}>
+            {items
+              .slice(win.range.start, win.range.end)
+              .map(({ hunk, i }, k) =>
+                i < 0 ? (
+                  <HunkHeader key={win.range.start + k} hunk={hunk} fixed />
+                ) : (
+                  <UnifiedLine
+                    key={win.range.start + k}
+                    line={hunk.lines[i]!}
+                    lang={lang}
+                    emph={analyseHunk(hunk).emph.get(i)}
+                    style={FIXED}
+                  />
+                )
+              )}
+          </Windowed>
+        ) : (
+          file.hunks.map((hunk, i) => (
+            <UnifiedHunk key={`${hunk.header}:${i}`} hunk={hunk} lang={lang} />
+          ))
+        )}
+        {file.truncated && (
+          <p className="px-2 py-1 text-[10px] text-status-warning">{t("changes.fileTruncated")}</p>
+        )}
+      </div>
+    </HunkActions.Provider>
  );
 }

@@ -668,6 +668,97 @@ describe("LaneChangesPanel", () => {
    expect(screen.getAllByTestId("lane-diff-view")).toHaveLength(2);
  });

+  describe("review actions", () => {
+    const reviewed = () =>
+      changes({
+        files: [
+          {
+            path: "server/lib/git-ops.js",
+            status: " M",
+            untracked: false,
+            unstaged: true,
+            added: 1,
+            deleted: 1,
+            binary: false,
+            hunks: [HUNK, { ...HUNK, header: "@@ -9,2 +10,3 @@" }],
+          },
+          {
+            path: "notes.txt",
+            status: "??",
+            untracked: true,
+            added: 1,
+            deleted: 0,
+            binary: false,
+            hunks: [HUNK],
+          },
+        ],
+      });
+    const openReview = async (user: ReturnType<typeof userEvent.setup>) => {
+      render(<LaneChangesPanel laneId={7} />);
+      await screen.findByTestId("lane-changes-overview");
+      vi.mocked(api.lanes.changes).mockResolvedValue(reviewed());
+      await user.click(screen.getByRole("button", { name: "Review" }));
+      await screen.findByTestId("lane-changes-review");
+    };
+
+    it("stages one hunk by its @@ line, at the context the diff was read with", async () => {
+      const user = userEvent.setup();
+      await openReview(user);
+      // Two hunks on the tracked file; the untracked one has none to act on.
+      const stage = screen.getAllByRole("button", { name: "Stage this hunk" });
+      expect(stage).toHaveLength(2);
+      await user.click(stage[1]!);
+      expect(api.lanes.gitAction).toHaveBeenCalledWith(7, "stage-hunk", {
+        path: "server/lib/git-ops.js",
+        hunk: "@@ -9,2 +10,3 @@",
+        context: 3,
+      });
+    });
+
+    it("needs two clicks to undo a hunk, and arms only that hunk", async () => {
+      const user = userEvent.setup();
+      await openReview(user);
+      const [first] = screen.getAllByRole("button", { name: "Undo this hunk" });
+      await user.click(first!);
+      expect(api.lanes.gitAction).not.toHaveBeenCalled();
+      // The other hunk is still at its first click.
+      expect(screen.getAllByRole("button", { name: "Undo this hunk" })).toHaveLength(1);
+      await user.click(
+        screen.getByRole("button", {
+          name: "Click again to undo this hunk — this cannot be undone",
+        })
+      );
+      expect(api.lanes.gitAction).toHaveBeenCalledWith(7, "discard-hunk", {
+        path: "server/lib/git-ops.js",
+        hunk: "@@ -1,2 +1,3 @@",
+        context: 3,
+        confirm: true,
+      });
+    });
+
+    it("stages a whole file from its review header without folding it", async () => {
+      const user = userEvent.setup();
+      await openReview(user);
+      const [stageFile] = screen.getAllByRole("button", { name: "Stage this file" });
+      await user.click(stageFile!);
+      expect(api.lanes.gitAction).toHaveBeenCalledWith(7, "stage", {
+        paths: ["server/lib/git-ops.js"],
+      });
+      expect(
+        screen.getByRole("button", { name: "Collapse server/lib/git-ops.js" })
+      ).toBeInTheDocument();
+    });
+
+    it("keeps each file's header pinned while its diff scrolls", async () => {
+      const user = userEvent.setup();
+      await openReview(user);
+      expect(screen.getByRole("button", { name: "Collapse server/lib/git-ops.js" })).toHaveClass(
+        "sticky",
+        "top-0"
+      );
+    });
+  });
+
  it("opens a file only from its path, and folds it from anywhere else", async () => {
    const user = userEvent.setup();
    render(<LaneChangesPanel laneId={7} />);