From 2b08a35b217f878c1214f5b0e8a865f6471795f2 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 20 Mar 2026 06:38:24 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A7=AA=20fix(ShareButtons):=20Handle?= =?UTF-8?q?=20copy=20errors=20and=20add=20missing=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Conditionally show 'Copied!' feedback only on successful copy operations. - Ensure document.execCommand fallback explicitly handles boolean failures. - Throw and log errors via logger when both copy operations fail. - Added comprehensive unit tests for these error scenarios. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com> --- pr_desc.txt | 12 ++++++++ src/components/ShareButtons.test.tsx | 41 ++++++++++++++++++++++++++++ src/components/ShareButtons.tsx | 39 +++++++++++++++++++------- 3 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 pr_desc.txt diff --git a/pr_desc.txt b/pr_desc.txt new file mode 100644 index 0000000..f123515 --- /dev/null +++ b/pr_desc.txt @@ -0,0 +1,12 @@ +๐Ÿงช [testing improvement] Add tests for ShareButtons clipboard error handling + +๐ŸŽฏ **What:** The testing gap addressed +This PR addresses the missing error handling tests in the `ShareButtons` component. Previously, if `navigator.clipboard.writeText` failed, it correctly fell back to `document.execCommand("copy")`. However, if the fallback also failed (it returns `false` on failure instead of throwing), the component incorrectly and unconditionally showed the "Copied!" feedback regardless. + +๐Ÿ“Š **Coverage:** What scenarios are now tested +- Added a robust unit test simulating a total failure of both `navigator.clipboard.writeText` and `document.execCommand`. +- Verified that `logger.error` is properly called with both the original exception and the fallback exception. +- Verified that the "Copied!" UI text remains hidden if copying fundamentally fails. + +โœจ **Result:** The improvement in test coverage +The code was updated to conditionally check the success of `document.execCommand` and wrap the fallback manipulation in a `try...finally` block for consistent cleanup. `logger.error` is properly integrated. This correctly stops the app from displaying an incorrect success state and improves overall code reliability through targeted unit test verification. diff --git a/src/components/ShareButtons.test.tsx b/src/components/ShareButtons.test.tsx index 3067c63..7a1c47c 100644 --- a/src/components/ShareButtons.test.tsx +++ b/src/components/ShareButtons.test.tsx @@ -4,6 +4,7 @@ import { render, screen, fireEvent, waitFor, act } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import ShareButtons from "./ShareButtons"; +import { logger } from "@/lib/logger"; describe("ShareButtons", () => { let originalClipboard: Navigator["clipboard"] | undefined; @@ -11,6 +12,7 @@ describe("ShareButtons", () => { let originalLocation: Location; beforeEach(() => { + vi.spyOn(logger, 'error').mockImplementation(() => {}); originalClipboard = navigator.clipboard; originalExecCommand = document.execCommand; originalLocation = window.location; @@ -152,4 +154,43 @@ describe("ShareButtons", () => { expect(screen.getByText("Copy URL")).toBeDefined(); }); }); + + it("logs an error and does not show 'Copied!' feedback when both copy methods fail", async () => { + // 1. Mock clipboard.writeText to reject + const writeTextMock = vi.fn().mockRejectedValue(new Error("Clipboard API failed")); + Object.assign(navigator, { + clipboard: { + writeText: writeTextMock, + }, + }); + + // 2. Mock execCommand to return false (failure) + const execCommandMock = vi.fn().mockReturnValue(false); + document.execCommand = execCommandMock; + + render(); + + const copyButton = screen.getByRole("button", { name: "Copy profile URL" }); + + fireEvent.click(copyButton); + + await waitFor(() => { + expect(writeTextMock).toHaveBeenCalledWith("http://localhost/johndoe"); + }); + + await waitFor(() => { + expect(execCommandMock).toHaveBeenCalledWith("copy"); + }); + + // Verify logger.error was called + expect(logger.error).toHaveBeenCalledWith( + "Failed to copy", + expect.any(Error), // error from clipboard.writeText + expect.any(Error) // error from execCommand fallback failing + ); + + // Verify button text remains unchanged + expect(screen.getByText("Copy URL")).toBeDefined(); + expect(screen.queryByText("Copied!")).toBeNull(); + }); }); diff --git a/src/components/ShareButtons.tsx b/src/components/ShareButtons.tsx index c326238..69a4b30 100644 --- a/src/components/ShareButtons.tsx +++ b/src/components/ShareButtons.tsx @@ -1,6 +1,7 @@ "use client"; import { useCallback, useEffect, useRef, useState } from "react"; +import { logger } from "@/lib/logger"; type Props = { username: string; @@ -29,17 +30,35 @@ export default function ShareButtons({ username }: Props) { const handleCopy = useCallback(async () => { try { - await navigator.clipboard.writeText(getShareUrl()); - } catch { - // Fallback for older browsers - const textArea = document.createElement("textarea"); - textArea.value = getShareUrl(); - document.body.appendChild(textArea); - textArea.select(); - document.execCommand("copy"); - document.body.removeChild(textArea); + if (navigator.clipboard && navigator.clipboard.writeText) { + await navigator.clipboard.writeText(getShareUrl()); + showCopiedFeedback(); + return; + } + throw new Error("Clipboard API not available"); + } catch (err) { + try { + // Fallback for older browsers + const textArea = document.createElement("textarea"); + textArea.value = getShareUrl(); + document.body.appendChild(textArea); + let successful = false; + try { + textArea.select(); + successful = document.execCommand("copy"); + } finally { + document.body.removeChild(textArea); + } + + if (successful) { + showCopiedFeedback(); + } else { + throw new Error("document.execCommand('copy') failed"); + } + } catch (fallbackErr) { + logger.error("Failed to copy", err, fallbackErr); + } } - showCopiedFeedback(); }, [getShareUrl, showCopiedFeedback]); const handleTwitterShare = useCallback(() => { From 333c23d76729971b076d72c66d5c9efc35aa11cd Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 20 Mar 2026 07:06:24 +0000 Subject: [PATCH 2/2] refactor(ShareButtons): flatten error handling in handleCopy Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com> --- pr_desc.txt | 12 ------- src/components/ShareButtons.tsx | 56 +++++++++++++++++++-------------- 2 files changed, 33 insertions(+), 35 deletions(-) delete mode 100644 pr_desc.txt diff --git a/pr_desc.txt b/pr_desc.txt deleted file mode 100644 index f123515..0000000 --- a/pr_desc.txt +++ /dev/null @@ -1,12 +0,0 @@ -๐Ÿงช [testing improvement] Add tests for ShareButtons clipboard error handling - -๐ŸŽฏ **What:** The testing gap addressed -This PR addresses the missing error handling tests in the `ShareButtons` component. Previously, if `navigator.clipboard.writeText` failed, it correctly fell back to `document.execCommand("copy")`. However, if the fallback also failed (it returns `false` on failure instead of throwing), the component incorrectly and unconditionally showed the "Copied!" feedback regardless. - -๐Ÿ“Š **Coverage:** What scenarios are now tested -- Added a robust unit test simulating a total failure of both `navigator.clipboard.writeText` and `document.execCommand`. -- Verified that `logger.error` is properly called with both the original exception and the fallback exception. -- Verified that the "Copied!" UI text remains hidden if copying fundamentally fails. - -โœจ **Result:** The improvement in test coverage -The code was updated to conditionally check the success of `document.execCommand` and wrap the fallback manipulation in a `try...finally` block for consistent cleanup. `logger.error` is properly integrated. This correctly stops the app from displaying an incorrect success state and improves overall code reliability through targeted unit test verification. diff --git a/src/components/ShareButtons.tsx b/src/components/ShareButtons.tsx index 69a4b30..c9688bf 100644 --- a/src/components/ShareButtons.tsx +++ b/src/components/ShareButtons.tsx @@ -29,35 +29,45 @@ export default function ShareButtons({ username }: Props) { }, [username]); const handleCopy = useCallback(async () => { - try { - if (navigator.clipboard && navigator.clipboard.writeText) { + let clipboardError: unknown = null; + + if (navigator.clipboard && navigator.clipboard.writeText) { + try { await navigator.clipboard.writeText(getShareUrl()); showCopiedFeedback(); return; + } catch (err) { + clipboardError = err; } - throw new Error("Clipboard API not available"); - } catch (err) { - try { - // Fallback for older browsers - const textArea = document.createElement("textarea"); - textArea.value = getShareUrl(); - document.body.appendChild(textArea); - let successful = false; - try { - textArea.select(); - successful = document.execCommand("copy"); - } finally { - document.body.removeChild(textArea); - } + } else { + clipboardError = new Error("Clipboard API not available"); + } - if (successful) { - showCopiedFeedback(); - } else { - throw new Error("document.execCommand('copy') failed"); - } - } catch (fallbackErr) { - logger.error("Failed to copy", err, fallbackErr); + // Fallback for older browsers + const textArea = document.createElement("textarea"); + textArea.value = getShareUrl(); + document.body.appendChild(textArea); + + let successful = false; + let fallbackError: unknown = null; + + try { + textArea.select(); + successful = document.execCommand("copy"); + if (!successful) { + fallbackError = new Error("document.execCommand('copy') failed"); } + } catch (err) { + successful = false; + fallbackError = err; + } finally { + document.body.removeChild(textArea); + } + + if (successful) { + showCopiedFeedback(); + } else { + logger.error("Failed to copy", clipboardError, fallbackError); } }, [getShareUrl, showCopiedFeedback]);