feat(plugins): show a running state on a plugin command's toolbar button (#2371)

A plugin command that takes seconds (an LLM call that fills the commit message) gave no
sign at the button that the click was taken, and a second click started a second run.

Actions may now set `indicatesRunning`: the action runner counts their in-flight runs
and an IconButton bound to such an action shows a spinner, sets aria-busy and ignores
clicks until the run settles. Plugin commands set it, so every toolbar slot that draws
them (editor.title, mainToolbar, commit.message) is covered without a plugin API change
and without features importing each other. The button stays focusable (aria-disabled,
not disabled). Spinner moves to its own module so Button can use it without a cycle
with Feedback, which re-exports it.

Refs #2254

Co-authored-by: NguyenBang <[email protected]>

src/features/plugins/__tests__/plugins.test.tsx

@@ -155,7 +155,7 @@ describe("plugin actions", () => {
       [cmd("a", "go"), cmd("a", "when", { when: "editorFocus", category: "Cat" }), cmd("gone", "x")],
       [plugin({ id: "a", name: "Alpha" })],
     );
-    expect(getAction("plugin.a.go")).toMatchObject({ title: "T go", category: "Alpha" });
+    expect(getAction("plugin.a.go")).toMatchObject({ title: "T go", category: "Alpha", indicatesRunning: true });
     expect(getAction("plugin.a.when")).toMatchObject({ when: "editorFocus", category: "Cat" });
     expect(getAction("plugin.gone.x")).toBeUndefined();
     expect(pluginActionId("p", "c.d")).toBe("plugin.p.c.d");

src/features/plugins/points.tsx

@@ -208,6 +208,8 @@ export function defineBuiltinUiPoints(): () => void {
                 : "package",
           when: decl.when ?? undefined,
           verbatim: true,
+          // A command can call a model or a server; its toolbar button must show the click was taken.
+          indicatesRunning: true,
           keywords: ["plugin", plugin.name, pluginId],
           description: tx`From plugin ${plugin.name}`,
           run: async ({ arg }: { arg?: unknown }) => {

src/platform/actions.test.ts

@@ -1,5 +1,59 @@
 import { expect, it, vi } from "vitest";
-import { executeAction, executeActionOrThrow, registerAction } from "./actions";
+import { executeAction, executeActionOrThrow, registerAction, useRunningActions } from "./actions";
+
+const running = (id: string) => useRunningActions.getState().running.get(id) ?? 0;
+
+it("counts a run of an `indicatesRunning` action until it settles, on success and on failure", async () => {
+  const finishers: (() => void)[] = [];
+  const dispose = registerAction({
+    id: "test.slow",
+    title: "Slow",
+    indicatesRunning: true,
+    run: () => new Promise<void>((resolve) => finishers.push(resolve)),
+  });
+  const failing = registerAction({
+    id: "test.failing",
+    title: "Failing",
+    indicatesRunning: true,
+    run: async () => {
+      throw new Error("nope");
+    },
+  });
+  const log = vi.spyOn(console, "error").mockImplementation(() => {});
+  try {
+    const first = executeAction("test.slow");
+    const second = executeAction("test.slow");
+    expect(running("test.slow")).toBe(2);
+    finishers[0]!();
+    await first;
+    expect(running("test.slow")).toBe(1);
+    finishers[1]!();
+    await second;
+    expect(running("test.slow")).toBe(0);
+    expect(useRunningActions.getState().running.has("test.slow")).toBe(false);
+
+    await executeAction("test.failing");
+    expect(running("test.failing")).toBe(0);
+  } finally {
+    dispose();
+    failing();
+    log.mockRestore();
+  }
+});
+
+it("does not track actions that did not ask for it", async () => {
+  let finish!: () => void;
+  const dispose = registerAction({
+    id: "test.plain",
+    title: "Plain",
+    run: () => new Promise<void>((resolve) => (finish = resolve)),
+  });
+  const run = executeAction("test.plain");
+  expect(running("test.plain")).toBe(0);
+  finish();
+  await run;
+  dispose();
+});
 
 it("propagates host launch errors while retaining ordinary action error handling", async () => {
   const dispose = registerAction({

src/platform/actions.ts

@@ -52,6 +52,12 @@ export interface Action {
    * Search Everywhere shows an ON/OFF switch and keeps the popup open.
    */
   toggled?: () => boolean;
+  /**
+   * The run takes long enough to need feedback where it was clicked: toolbar buttons of this
+   * action show a spinner and ignore clicks until `run` settles. Only for actions that finish
+   * by themselves, not ones that wait on the user (a dialog).
+   */
+  indicatesRunning?: boolean;
   run: (c: ActionContext) => void | Promise<void>;
 }
 
@@ -61,6 +67,19 @@ interface ActionStore {
 
 export const useActions = create<ActionStore>(() => ({ actions: new Map() }));
 
+/** Ids of `indicatesRunning` actions with a run in flight, with how many. */
+export const useRunningActions = create<{ running: ReadonlyMap<string, number> }>(() => ({ running: new Map() }));
+
+function trackRun(id: string, delta: 1 | -1) {
+  useRunningActions.setState(({ running }) => {
+    const next = new Map(running);
+    const count = (next.get(id) ?? 0) + delta;
+    if (count > 0) next.set(id, count);
+    else next.delete(id);
+    return { running: next };
+  });
+}
+
 /** Registered actions as written (English), by id; the store holds their localised copies. */
 const sources = new Map<string, Action>();
 
@@ -191,6 +210,7 @@ async function runAction(id: string, arg: unknown, propagateError: boolean): Pro
       } catch (e) {
         console.error("action listener failed", e);
       }
+  if (action.indicatesRunning) trackRun(id, 1);
   try {
     let pending: void | Promise<void>;
     nesting++;
@@ -203,6 +223,8 @@ async function runAction(id: string, arg: unknown, propagateError: boolean): Pro
   } catch (e) {
     if (propagateError) throw e;
     console.error(`action ${id} failed`, e);
+  } finally {
+    if (action.indicatesRunning) trackRun(id, -1);
   }
   return true;
 }

src/ui/Button.module.css

@@ -98,6 +98,9 @@
   opacity: 0.4;
   cursor: default;
 }
+.iconButton[aria-busy="true"] {
+  cursor: progress;
+}
 .iconButton[aria-pressed="true"] {
   background: var(--bg-toolbar-selected, var(--bg-tree-selected));
   color: var(--fg-strong);

src/ui/Button.test.tsx

@@ -0,0 +1,60 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { act, cleanup, fireEvent, render, screen } from "@testing-library/react";
+import { registerAction } from "@/platform/actions";
+import { IconButton } from "./Button";
+
+afterEach(cleanup);
+
+describe("IconButton bound to an action that indicates running", () => {
+  const setup = () => {
+    let finish!: () => void;
+    const run = vi.fn(() => new Promise<void>((resolve) => (finish = resolve)));
+    const dispose = registerAction({ id: "test.generate", title: "Generate", indicatesRunning: true, run });
+    return { run, finish: () => finish(), dispose };
+  };
+
+  it("shows a spinner and ignores clicks until the run settles, then goes back to its icon", async () => {
+    const { run, finish, dispose } = setup();
+    render(<IconButton icon="plugin" action="test.generate" />);
+    const button = screen.getByRole("button", { name: "Generate" });
+    expect(button).not.toHaveAttribute("aria-busy");
+    expect(button.querySelector('[role="status"]')).toBeNull();
+
+    fireEvent.click(button);
+    expect(await screen.findByRole("status")).toBeInTheDocument();
+    expect(button).toHaveAttribute("aria-busy", "true");
+    expect(button).toHaveAttribute("aria-disabled", "true");
+    expect(button).not.toBeDisabled();
+
+    fireEvent.click(button);
+    fireEvent.click(button);
+    expect(run).toHaveBeenCalledTimes(1);
+
+    await act(async () => finish());
+    expect(button).not.toHaveAttribute("aria-busy");
+    expect(screen.queryByRole("status")).toBeNull();
+    fireEvent.click(button);
+    expect(run).toHaveBeenCalledTimes(2);
+    await act(async () => finish());
+    dispose();
+  });
+
+  it("also shows the run started from elsewhere, such as a shortcut", async () => {
+    const { finish, dispose } = setup();
+    render(<IconButton icon="plugin" action="test.generate" />);
+    const { executeAction } = await import("@/platform/actions");
+    void executeAction("test.generate");
+    expect(await screen.findByRole("status")).toBeInTheDocument();
+    await act(async () => finish());
+    expect(screen.queryByRole("status")).toBeNull();
+    dispose();
+  });
+
+  it("leaves other buttons alone", () => {
+    const dispose = registerAction({ id: "test.plainAction", title: "Plain", run: () => {} });
+    render(<IconButton icon="plugin" action="test.plainAction" />);
+    fireEvent.click(screen.getByRole("button", { name: "Plain" }));
+    expect(screen.queryByRole("status")).toBeNull();
+    dispose();
+  });
+});

src/ui/Button.tsx

@@ -1,8 +1,9 @@
 import { forwardRef, type ButtonHTMLAttributes, type ReactNode } from "react";
 import clsx from "clsx";
-import { getAction, executeAction } from "@/platform/actions";
+import { getAction, executeAction, useRunningActions } from "@/platform/actions";
 import { shortcutLabel } from "@/platform/keymap";
 import { Icon, type IconName } from "./icons";
+import { Spinner } from "./Spinner";
 import { tooltipAttrs } from "./Tooltip";
 import styles from "./Button.module.css";
 import { useGivenTx } from "@/platform/i18n";
@@ -73,6 +74,8 @@ export const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>(functio
   const a = action ? getAction(action) : undefined;
   const label = tx(title) ?? a?.title ?? action ?? icon;
   const keys = shortcut ?? (action ? shortcutLabel(action) : undefined);
+  const busy = useRunningActions((s) => (action ? s.running.has(action) : false));
+  const glyphSize = iconSize ?? (size === "small" ? 14 : 16);
   return (
     <button
       ref={ref}
@@ -80,16 +83,20 @@ export const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>(functio
       aria-label={tx(ariaLabel) ?? label}
       aria-pressed={pressed}
       aria-keyshortcuts={keys}
+      aria-busy={busy || undefined}
+      // Not `disabled`: a disabled button drops keyboard focus while the action runs.
+      aria-disabled={busy || undefined}
       disabled={disabled}
       className={clsx(styles.iconButton, styles[`is-${size}`], tone !== "default" && styles[`tone-${tone}`], className)}
       onClick={(e) => {
+        if (busy) return;
         onClick?.(e);
         if (!e.defaultPrevented && action) void executeAction(action, actionArg);
       }}
       {...tooltipAttrs(label, keys, true)}
       {...rest}
     >
-      <Icon name={icon} size={iconSize ?? (size === "small" ? 14 : 16)} />
+      {busy ? <Spinner size={glyphSize} /> : <Icon name={icon} size={glyphSize} />}
     </button>
   );
 });

src/ui/Feedback.module.css

@@ -28,20 +28,6 @@
   }
 }
 
-.spinner {
-  flex: none;
-  color: var(--fg-muted);
-  animation: spin 0.9s linear infinite;
-}
-.spokes {
-  animation: spin 1s steps(8, end) infinite;
-}
-@keyframes spin {
-  to {
-    transform: rotate(360deg);
-  }
-}
-
 .badge {
   display: inline-flex;
   align-items: center;
@@ -207,10 +193,4 @@
     left: 0;
     opacity: 0.6;
   }
-  .spinner {
-    animation-duration: 2.4s;
-  }
-  .spokes {
-    animation: none;
-  }
 }

src/ui/Feedback.tsx

@@ -34,49 +34,8 @@ export function ProgressBar({ value, className, ...aria }: ProgressBarProps) {
   );
 }
 
-export function Spinner({
-  size = 14,
-  className,
-  label = "Loading",
-  variant = "ring",
-}: {
-  size?: number;
-  className?: string;
-  label?: string;
-  variant?: "ring" | "spokes";
-}) {
-  return (
-    <svg
-      className={clsx(styles.spinner, variant === "spokes" && styles.spokes, className)}
-      width={size}
-      height={size}
-      viewBox={variant === "spokes" ? "0 0 16 16" : "0 0 24 24"}
-      role="status"
-      aria-label={label}
-    >
-      {variant === "spokes" ? (
-        Array.from({ length: 8 }, (_, index) => (
-          <rect
-            key={index}
-            x="7"
-            y="1"
-            width="2"
-            height="4"
-            rx="1"
-            fill="currentColor"
-            opacity={1 - index * 0.1}
-            transform={`rotate(${-index * 45} 8 8)`}
-          />
-        ))
-      ) : (
-        <>
-          <circle cx="12" cy="12" r="9" fill="none" stroke="currentColor" strokeOpacity="0.2" strokeWidth="2.4" />
-          <path d="M21 12a9 9 0 0 0-9-9" fill="none" stroke="currentColor" strokeWidth="2.4" strokeLinecap="round" />
-        </>
-      )}
-    </svg>
-  );
-}
+// Lives in its own module so `Button` can show it without importing this file back.
+export { Spinner } from "./Spinner";
 
 export interface BadgeProps {
   children: ReactNode;

src/ui/Spinner.module.css

@@ -0,0 +1,22 @@
+.spinner {
+  flex: none;
+  color: var(--fg-muted);
+  animation: spin 0.9s linear infinite;
+}
+.spokes {
+  animation: spin 1s steps(8, end) infinite;
+}
+@keyframes spin {
+  to {
+    transform: rotate(360deg);
+  }
+}
+
+@media (prefers-reduced-motion: reduce) {
+  .spinner {
+    animation-duration: 2.4s;
+  }
+  .spokes {
+    animation: none;
+  }
+}

src/ui/Spinner.tsx

@@ -0,0 +1,46 @@
+import clsx from "clsx";
+import styles from "./Spinner.module.css";
+
+export function Spinner({
+  size = 14,
+  className,
+  label = "Loading",
+  variant = "ring",
+}: {
+  size?: number;
+  className?: string;
+  label?: string;
+  variant?: "ring" | "spokes";
+}) {
+  return (
+    <svg
+      className={clsx(styles.spinner, variant === "spokes" && styles.spokes, className)}
+      width={size}
+      height={size}
+      viewBox={variant === "spokes" ? "0 0 16 16" : "0 0 24 24"}
+      role="status"
+      aria-label={label}
+    >
+      {variant === "spokes" ? (
+        Array.from({ length: 8 }, (_, index) => (
+          <rect
+            key={index}
+            x="7"
+            y="1"
+            width="2"
+            height="4"
+            rx="1"
+            fill="currentColor"
+            opacity={1 - index * 0.1}
+            transform={`rotate(${-index * 45} 8 8)`}
+          />
+        ))
+      ) : (
+        <>
+          <circle cx="12" cy="12" r="9" fill="none" stroke="currentColor" strokeOpacity="0.2" strokeWidth="2.4" />
+          <path d="M21 12a9 9 0 0 0-9-9" fill="none" stroke="currentColor" strokeWidth="2.4" strokeLinecap="round" />
+        </>
+      )}
+    </svg>
+  );
+}