From d9aeda2b157bf4bf582f3ae1db412e83269a8821 Mon Sep 17 00:00:00 2001 From: Nickk'z Date: Sat, 31 Oct 2026 04:23:51 +0530 Subject: [PATCH] Spin the Files refresh icon only on a manual refresh The Files rail reloads every 5 seconds while a task runs, and that poll reused the same path as the refresh button, so the icon spun and the button flickered disabled every 5 seconds on its own. Track the manual refresh separately: load() now spins the icon only when the viewer clicks, and the background poll and org-change reloads refresh quietly. Move the markup into ArtifactsRailView, which takes the rail's state as props, and add render tests for the loading, empty, unavailable and list states plus the rule that the icon spins only on a manual click. --- .../components/chat/artifacts-rail.test.tsx | 157 ++++++++++++++++++ frontend/components/chat/artifacts-rail.tsx | 122 ++++++++------ 2 files changed, 232 insertions(+), 47 deletions(-) create mode 100644 frontend/components/chat/artifacts-rail.test.tsx diff --git a/frontend/components/chat/artifacts-rail.test.tsx b/frontend/components/chat/artifacts-rail.test.tsx new file mode 100644 index 000000000..0bbe29398 --- /dev/null +++ b/frontend/components/chat/artifacts-rail.test.tsx @@ -0,0 +1,157 @@ +// Two halves. First, static-markup tests of ArtifactsRailView, the presentational +// half that takes the rail's state as props: the loading, empty, unavailable and +// list states each render their own copy, and the refresh icon spins only when the +// `refreshing` prop is set. Second, an interaction test that mounts the real +// ArtifactsRail container and proves the spinner rule end to end: the initial load +// and the 5-second poll share one non-manual code path that never spins the icon, +// while a viewer's click does. +// +// The container test drives the rail through local `fetch`/`EventSource` stubs +// restored in a finally, rather than bun's mock.module: mock.module is +// process-wide and would leak a never-resolving backendFetch into every test +// file loaded after this one, hanging unrelated component tests. +import { expect, test } from "bun:test"; +import { GlobalRegistrator } from "@happy-dom/global-registrator"; +import { renderToStaticMarkup } from "react-dom/server"; +import type { ArtifactDescriptor } from "@/components/artifacts/model"; +import { ArtifactsRail, ArtifactsRailView } from "./artifacts-rail"; + +if (typeof window === "undefined") GlobalRegistrator.register(); + +const { act } = await import("react"); +const { createRoot } = await import("react-dom/client"); + +const FILE: ArtifactDescriptor = { + id: "a1", + run_id: "r1", + thread_id: "t1", + name: "report.docx", + source_path: "/root/work/report.docx", + content_type: "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + size_bytes: 2048, + sha256: "abc123", + created_at: "2026-01-01T00:00:00Z", + preview_url: "https://example.com/preview", + download_url: "https://example.com/download", + preview_pdf_url: null, + workpiece: null, +}; + +const noop = () => {}; + +test("the loading state shows while the backend has not answered yet", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("Loading files..."); + expect(html).not.toContain("No files yet"); + expect(html).not.toContain("Files unavailable"); +}); + +test("the empty state shows once the backend answers with no files", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("No files yet"); + expect(html).not.toContain("Loading files..."); +}); + +test("the unavailable state shows when the backend request failed", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("Files unavailable"); + expect(html).toContain("Refresh to reconnect."); +}); + +test("the list state renders a card per file and no placeholder", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("report.docx"); + expect(html).not.toContain("No files yet"); + expect(html).not.toContain("Files unavailable"); +}); + +test("the refresh icon spins and the button disables only when refreshing is set", () => { + const spinning = renderToStaticMarkup( + , + ); + expect(spinning).toContain("animate-spin"); + expect(spinning).toContain("disabled"); + + const still = renderToStaticMarkup( + , + ); + expect(still).not.toContain("animate-spin"); +}); + +test("a background load never spins the icon; a manual click does", async () => { + const originalFetch = globalThis.fetch; + const originalEventSource = (globalThis as { EventSource?: unknown }).EventSource; + // happy-dom has no EventSource; the rail's org-change subscription opens one. + (globalThis as { EventSource?: unknown }).EventSource = class { + close() {} + addEventListener() {} + removeEventListener() {} + }; + // Hand out responses on demand so we can inspect the in-flight state. + const resolvers: Array<(body: unknown) => void> = []; + globalThis.fetch = (() => + new Promise((resolve) => { + resolvers.push((body) => + resolve({ + ok: true, + status: 200, + headers: new Headers(), + json: async () => body, + } as unknown as Response), + ); + })) as unknown as typeof fetch; + + const host = document.createElement("div"); + document.body.appendChild(host); + const root = createRoot(host); + try { + // Mount: the initial load fires on the same non-manual path the 5-second + // poll uses. Its request is in flight and must not spin the control. + await act(async () => { + root.render(); + }); + const button = host.querySelector( + 'button[aria-label="Refresh session files"]', + ); + if (!button) throw new Error("refresh button not found"); + expect(resolvers.length).toBe(1); // the initial (non-manual) load is pending + expect(button.querySelector(".animate-spin")).toBeNull(); + expect(button.disabled).toBe(false); + + // Finish the initial load with an empty list. + await act(async () => { + resolvers.shift()?.({ artifacts: [] }); + }); + expect(button.querySelector(".animate-spin")).toBeNull(); + + // A viewer's click starts a manual load: now the icon spins and the button + // locks until the request settles. + await act(async () => { + button.click(); + }); + expect(resolvers.length).toBe(1); + expect(button.querySelector(".animate-spin")).not.toBeNull(); + expect(button.disabled).toBe(true); + + await act(async () => { + resolvers.shift()?.({ artifacts: [] }); + }); + expect(button.querySelector(".animate-spin")).toBeNull(); + expect(button.disabled).toBe(false); + } finally { + await act(async () => { + root.unmount(); + }); + host.remove(); + globalThis.fetch = originalFetch; + (globalThis as { EventSource?: unknown }).EventSource = originalEventSource; + } +}); diff --git a/frontend/components/chat/artifacts-rail.tsx b/frontend/components/chat/artifacts-rail.tsx index 643d6e757..9e7d36e9d 100644 --- a/frontend/components/chat/artifacts-rail.tsx +++ b/frontend/components/chat/artifacts-rail.tsx @@ -23,54 +23,22 @@ function SpinningRefreshIcon({ return ; } -export function ArtifactsRail({ - threadId, - live, +/** The Files rail, driven entirely by its props: the list, whether the backend + * answered at all, and whether a manual refresh is in flight. `refreshing` is + * the viewer's own click only; the background poll never sets it, so the icon + * spins when they ask for a refresh and stays still while a task streams in + * new files on its own. */ +export function ArtifactsRailView({ + artifacts, + available, + refreshing, + onRefresh, }: { - readonly threadId: string; - readonly live: boolean; + readonly artifacts: readonly ArtifactDescriptor[]; + readonly available: boolean | null; + readonly refreshing: boolean; + readonly onRefresh: () => void; }) { - const [artifacts, setArtifacts] = useState([]); - const [available, setAvailable] = useState(null); - const [refreshing, setRefreshing] = useState(false); - - const load = useCallback( - async (signal?: AbortSignal) => { - setRefreshing(true); - try { - const response = await backendFetch(artifactQueryForThread(threadId), { - cache: "no-store", - signal, - }); - if (!response.ok) throw new Error(`artifact request failed (${response.status})`); - const parsed = extractArtifacts(await response.json()); - if (!parsed) throw new Error("artifact response was invalid"); - setArtifacts(parsed); - setAvailable(true); - } catch { - if (!signal?.aborted) setAvailable(false); - } finally { - if (!signal?.aborted) setRefreshing(false); - } - }, - [threadId], - ); - - useOrgChanges((change) => { - if (change.type === "artifact" && change.threadId === threadId) void load(); - }); - - useEffect(() => { - const controller = new AbortController(); - void load(controller.signal); - if (!live) return () => controller.abort(); - const interval = setInterval(() => void load(controller.signal), LIVE_REFRESH_MS); - return () => { - controller.abort(); - clearInterval(interval); - }; - }, [live, load]); - return (
@@ -83,7 +51,7 @@ export function ArtifactsRail({ void load()} + onClick={onRefresh} disabled={refreshing} aria-label="Refresh session files" title="Refresh" @@ -117,3 +85,63 @@ export function ArtifactsRail({
); } + +export function ArtifactsRail({ + threadId, + live, +}: { + readonly threadId: string; + readonly live: boolean; +}) { + const [artifacts, setArtifacts] = useState([]); + const [available, setAvailable] = useState(null); + const [refreshing, setRefreshing] = useState(false); + + const load = useCallback( + async ({ manual = false, signal }: { manual?: boolean; signal?: AbortSignal } = {}) => { + // Only a viewer's click spins the icon; the 5-second poll and org-change + // reloads refresh quietly so the control is not in constant motion. + if (manual) setRefreshing(true); + try { + const response = await backendFetch(artifactQueryForThread(threadId), { + cache: "no-store", + signal, + }); + if (!response.ok) throw new Error(`artifact request failed (${response.status})`); + const parsed = extractArtifacts(await response.json()); + if (!parsed) throw new Error("artifact response was invalid"); + setArtifacts(parsed); + setAvailable(true); + } catch { + if (!signal?.aborted) setAvailable(false); + } finally { + if (manual) setRefreshing(false); + } + }, + [threadId], + ); + + useOrgChanges((change) => { + if (change.type === "artifact" && change.threadId === threadId) void load(); + }); + + useEffect(() => { + const controller = new AbortController(); + void load({ signal: controller.signal }); + if (!live) return () => controller.abort(); + const interval = setInterval(() => void load({ signal: controller.signal }), LIVE_REFRESH_MS); + return () => { + controller.abort(); + clearInterval(interval); + }; + }, [live, load]); + + return ( + void load({ manual: true })} + /> + ); +}