From 298f6bd4b1d6bd125dc458f3047319bb9d2f9bb7 Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Thu, 16 Jul 2026 11:49:20 +0000 Subject: [PATCH 1/3] fix(dashboard): clear the CopyButton reset timer The revert-to-copy-icon setTimeout was fired on every click with no stored id and no cleanup, causing two bugs: - Rapid re-click: clicking again before the 2s window elapsed left the stale timer from the first click running, so it could flip the checkmark back to the copy icon early, contradicting the just-shown feedback from the second click. - Unmount mid-window: if the component unmounted (e.g. sessions table re-paginating/filtering) before the timer fired, setCopied ran on an unmounted component, logging a React warning. Store the timer id in a ref, clearTimeout it before re-arming on each copy, and clear it in a useEffect cleanup on unmount. Also add the missing type="button" the issue called out, so the button can't act as an implicit form submit. Closes #523 Co-Authored-By: Claude Sonnet 5 --- __tests__/components/copy-button.test.tsx | 61 +++++++++++++++++++++++ app/components/copy-button.tsx | 28 ++++++++--- 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/__tests__/components/copy-button.test.tsx b/__tests__/components/copy-button.test.tsx index 09ffd94a..91918ee8 100644 --- a/__tests__/components/copy-button.test.tsx +++ b/__tests__/components/copy-button.test.tsx @@ -65,6 +65,67 @@ describe("CopyButton", () => { vi.useRealTimers(); }); + it("keeps the checkmark for the full 2s window after the last of two rapid clicks", async () => { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); + vi.useFakeTimers({ shouldAdvanceTime: true }); + + render(); + const button = screen.getByTitle("Copy to clipboard"); + + // Click at t=0 (timer A would revert at t=2.0s) + await user.click(button); + expect(screen.getByTestId("check-icon")).toBeInTheDocument(); + + // Click again at t=1.5s — should reset the window (arm timer B, clear timer A) + act(() => { + vi.advanceTimersByTime(1500); + }); + await user.click(button); + expect(screen.getByTestId("check-icon")).toBeInTheDocument(); + + // t=2.0s: stale timer A must NOT flip the icon back early + act(() => { + vi.advanceTimersByTime(500); + }); + expect(screen.getByTestId("check-icon")).toBeInTheDocument(); + expect(screen.queryByTestId("copy-icon")).not.toBeInTheDocument(); + + // t=3.5s (2.0s after the LAST click at t=1.5s): timer B reverts it + act(() => { + vi.advanceTimersByTime(1500); + }); + expect(screen.getByTestId("copy-icon")).toBeInTheDocument(); + expect(screen.queryByTestId("check-icon")).not.toBeInTheDocument(); + + vi.useRealTimers(); + }); + + it("does not update state or warn after unmounting mid-window", async () => { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); + vi.useFakeTimers({ shouldAdvanceTime: true }); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const { unmount } = render(); + await user.click(screen.getByTitle("Copy to clipboard")); + expect(screen.getByTestId("check-icon")).toBeInTheDocument(); + + // Unmount before the 2s revert timer fires + act(() => { + vi.advanceTimersByTime(500); + }); + unmount(); + + // Advance well past the original 2s window — no "state update on + // unmounted component" warning should be logged + act(() => { + vi.advanceTimersByTime(2000); + }); + + expect(errorSpy).not.toHaveBeenCalled(); + + vi.useRealTimers(); + }); + it("applies custom className", () => { render(); const button = screen.getByTitle("Copy to clipboard"); diff --git a/app/components/copy-button.tsx b/app/components/copy-button.tsx index 7640ff4b..70c1a8d3 100644 --- a/app/components/copy-button.tsx +++ b/app/components/copy-button.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useCallback } from "react"; +import { useState, useCallback, useEffect, useRef } from "react"; import { Copy, Check } from "lucide-react"; import { cn } from "@/lib/utils"; @@ -25,6 +25,23 @@ interface CopyButtonProps { export function CopyButton({ text, className }: CopyButtonProps) { const [copied, setCopied] = useState(false); + const revertTimerRef = useRef>(undefined); + + // Clear any pending revert timer on unmount so `setCopied` never fires + // after the component is gone. + useEffect(() => { + return () => { + if (revertTimerRef.current) clearTimeout(revertTimerRef.current); + }; + }, []); + + const armRevertTimer = useCallback(() => { + // A re-click resets the 2s window rather than being cut short by the + // previous (stale) timer. + if (revertTimerRef.current) clearTimeout(revertTimerRef.current); + setCopied(true); + revertTimerRef.current = setTimeout(() => setCopied(false), 2000); + }, []); const handleCopy = useCallback(async () => { try { @@ -33,24 +50,23 @@ export function CopyButton({ text, className }: CopyButtonProps) { } else if (!fallbackCopyText(text)) { throw new Error("Both clipboard methods failed"); } - setCopied(true); - setTimeout(() => setCopied(false), 2000); + armRevertTimer(); } catch (err) { console.error("Failed to copy to clipboard:", err); // Try fallback if the modern API threw try { if (fallbackCopyText(text)) { - setCopied(true); - setTimeout(() => setCopied(false), 2000); + armRevertTimer(); } } catch { // Both methods failed — do nothing } } - }, [text]); + }, [text, armRevertTimer]); return (