diff --git a/src/features/plugins/__tests__/plugins.test.tsx b/src/features/plugins/__tests__/plugins.test.tsx index b51f5f60d..cab548ace 100644 --- a/src/features/plugins/__tests__/plugins.test.tsx +++ b/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"); diff --git a/src/features/plugins/points.tsx b/src/features/plugins/points.tsx index 60007fb97..192a15026 100644 --- a/src/features/plugins/points.tsx +++ b/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 }) => { diff --git a/src/platform/actions.test.ts b/src/platform/actions.test.ts index 614393ce1..88f0b899d 100644 --- a/src/platform/actions.test.ts +++ b/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((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((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({ diff --git a/src/platform/actions.ts b/src/platform/actions.ts index 74362d1fa..7e763cbe5 100644 --- a/src/platform/actions.ts +++ b/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; } @@ -61,6 +67,19 @@ interface ActionStore { export const useActions = create(() => ({ actions: new Map() })); +/** Ids of `indicatesRunning` actions with a run in flight, with how many. */ +export const useRunningActions = create<{ running: ReadonlyMap }>(() => ({ 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(); @@ -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; 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; } diff --git a/src/ui/Button.module.css b/src/ui/Button.module.css index 5864ea3fb..bb9a465b2 100644 --- a/src/ui/Button.module.css +++ b/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); diff --git a/src/ui/Button.test.tsx b/src/ui/Button.test.tsx new file mode 100644 index 000000000..d6bb50ef4 --- /dev/null +++ b/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((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(); + 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(); + 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(); + fireEvent.click(screen.getByRole("button", { name: "Plain" })); + expect(screen.queryByRole("status")).toBeNull(); + dispose(); + }); +}); diff --git a/src/ui/Button.tsx b/src/ui/Button.tsx index ab58b1055..b0b787173 100644 --- a/src/ui/Button.tsx +++ b/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(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 ( ); }); diff --git a/src/ui/Feedback.module.css b/src/ui/Feedback.module.css index cc46ab10e..409edef9a 100644 --- a/src/ui/Feedback.module.css +++ b/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; - } } diff --git a/src/ui/Feedback.tsx b/src/ui/Feedback.tsx index 9f776214c..cfb381eef 100644 --- a/src/ui/Feedback.tsx +++ b/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 ( - - {variant === "spokes" ? ( - Array.from({ length: 8 }, (_, index) => ( - - )) - ) : ( - <> - - - - )} - - ); -} +// Lives in its own module so `Button` can show it without importing this file back. +export { Spinner } from "./Spinner"; export interface BadgeProps { children: ReactNode; diff --git a/src/ui/Spinner.module.css b/src/ui/Spinner.module.css new file mode 100644 index 000000000..839ff3e69 --- /dev/null +++ b/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; + } +} diff --git a/src/ui/Spinner.tsx b/src/ui/Spinner.tsx new file mode 100644 index 000000000..0c0ba3958 --- /dev/null +++ b/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 ( + + {variant === "spokes" ? ( + Array.from({ length: 8 }, (_, index) => ( + + )) + ) : ( + <> + + + + )} + + ); +}