Add error handling and fallback for clipboard operations#8
Add error handling and fallback for clipboard operations#8
Conversation
Co-authored-by: tomiwa-a <[email protected]>
Co-authored-by: tomiwa-a <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR improves clipboard copy UX in the website by handling Clipboard API failures and adding an execCommand('copy') fallback, and it upgrades @modelcontextprotocol/sdk to pick up security patches.
Changes:
- Added
copyToClipboard()with a legacyexecCommandfallback and wired it into both copy button handlers. - Added explicit UI feedback for copy failures ("Failed!").
- Updated
@modelcontextprotocol/sdkdependency (and lockfile) to a patched 1.x version.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| package.json | Fixes scripts JSON formatting by adding the missing comma after website:build. |
| package-lock.json | Locks updated transitive dependency graph due to SDK upgrade and other minor bumps. |
| apps/website/src/components/preview.ts | Introduces clipboard error handling + fallback copy logic and updates copy button handlers. |
| apps/mcp/package.json | Bumps @modelcontextprotocol/sdk to 1.x range for security fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| copyBtn?.addEventListener("click", async () => { | ||
| const activePanel = document.querySelector(".preview-code.active"); | ||
| if (!activePanel) return; | ||
|
|
||
| const text = activePanel.textContent || ""; | ||
| const success = await copyToClipboard(text); | ||
|
|
||
| if (success) { | ||
| copyBtn.textContent = "Copied!"; | ||
| copyBtn.classList.add("copied"); | ||
| setTimeout(() => { | ||
| copyBtn.textContent = "Copy"; | ||
| copyBtn.classList.remove("copied"); | ||
| }, 2000); | ||
| } else { | ||
| copyBtn.textContent = "Failed!"; | ||
| copyBtn.classList.add("error"); | ||
| setTimeout(() => { | ||
| copyBtn.textContent = "Copy"; | ||
| copyBtn.classList.remove("error"); | ||
| }, 2000); | ||
| } | ||
| }); |
There was a problem hiding this comment.
copyBtn?.addEventListener(...) does not narrow the copyBtn type inside the async callback under strictNullChecks, so copyBtn.textContent / copyBtn.classList... will be flagged as possibly null. Wrap the listener registration in an if (copyBtn) block (or assign to a non-null local) so the callback closes over a non-null button reference.
| copyBtn?.addEventListener("click", async () => { | |
| const activePanel = document.querySelector(".preview-code.active"); | |
| if (!activePanel) return; | |
| const text = activePanel.textContent || ""; | |
| const success = await copyToClipboard(text); | |
| if (success) { | |
| copyBtn.textContent = "Copied!"; | |
| copyBtn.classList.add("copied"); | |
| setTimeout(() => { | |
| copyBtn.textContent = "Copy"; | |
| copyBtn.classList.remove("copied"); | |
| }, 2000); | |
| } else { | |
| copyBtn.textContent = "Failed!"; | |
| copyBtn.classList.add("error"); | |
| setTimeout(() => { | |
| copyBtn.textContent = "Copy"; | |
| copyBtn.classList.remove("error"); | |
| }, 2000); | |
| } | |
| }); | |
| if (copyBtn) { | |
| const nonNullCopyBtn = copyBtn; | |
| nonNullCopyBtn.addEventListener("click", async () => { | |
| const activePanel = document.querySelector(".preview-code.active"); | |
| if (!activePanel) return; | |
| const text = activePanel.textContent || ""; | |
| const success = await copyToClipboard(text); | |
| if (success) { | |
| nonNullCopyBtn.textContent = "Copied!"; | |
| nonNullCopyBtn.classList.add("copied"); | |
| setTimeout(() => { | |
| nonNullCopyBtn.textContent = "Copy"; | |
| nonNullCopyBtn.classList.remove("copied"); | |
| }, 2000); | |
| } else { | |
| nonNullCopyBtn.textContent = "Failed!"; | |
| nonNullCopyBtn.classList.add("error"); | |
| setTimeout(() => { | |
| nonNullCopyBtn.textContent = "Copy"; | |
| nonNullCopyBtn.classList.remove("error"); | |
| }, 2000); | |
| } | |
| }); | |
| } |
| textArea.focus(); | ||
| textArea.select(); | ||
|
|
||
| try { | ||
| const successful = document.execCommand("copy"); | ||
| document.body.removeChild(textArea); | ||
| return successful; | ||
| } catch (err) { | ||
| document.body.removeChild(textArea); | ||
| return false; |
There was a problem hiding this comment.
fallbackCopyText removes the temporary textarea in both the try and catch blocks, but if an exception occurs before entering the try (e.g., during focus() / select()), the element won’t be cleaned up. Consider moving the focus/select into the try and using a finally block to guarantee removal.
| textArea.focus(); | |
| textArea.select(); | |
| try { | |
| const successful = document.execCommand("copy"); | |
| document.body.removeChild(textArea); | |
| return successful; | |
| } catch (err) { | |
| document.body.removeChild(textArea); | |
| return false; | |
| try { | |
| textArea.focus(); | |
| textArea.select(); | |
| const successful = document.execCommand("copy"); | |
| return successful; | |
| } catch (err) { | |
| return false; | |
| } finally { | |
| if (textArea.parentNode === document.body) { | |
| document.body.removeChild(textArea); | |
| } |
| copyBtn.textContent = "Failed!"; | ||
| copyBtn.classList.add("error"); | ||
| setTimeout(() => { | ||
| copyBtn.textContent = "Copy"; | ||
| copyBtn.classList.remove("error"); | ||
| }, 2000); | ||
| } | ||
| }); | ||
|
|
||
| // Quick start copy button | ||
| document.querySelectorAll(".qs-copy-btn").forEach((btn) => { | ||
| btn.addEventListener("click", () => { | ||
| btn.addEventListener("click", async () => { | ||
| const codeBlock = | ||
| btn.closest(".qs-code-window")?.querySelector("pre") ?? | ||
| btn.parentElement?.querySelector("code"); | ||
| if (!codeBlock) return; | ||
|
|
||
| const text = codeBlock.textContent || ""; | ||
| navigator.clipboard.writeText(text).then(() => { | ||
| const success = await copyToClipboard(text); | ||
|
|
||
| if (success) { | ||
| (btn as HTMLElement).textContent = "Copied!"; | ||
| btn.classList.add("copied"); | ||
| setTimeout(() => { | ||
| (btn as HTMLElement).textContent = "Copy"; | ||
| btn.classList.remove("copied"); | ||
| }, 2000); | ||
| }); | ||
| } else { | ||
| (btn as HTMLElement).textContent = "Failed!"; | ||
| btn.classList.add("error"); | ||
| setTimeout(() => { | ||
| (btn as HTMLElement).textContent = "Copy"; | ||
| btn.classList.remove("error"); | ||
| }, 2000); |
There was a problem hiding this comment.
The new failure path adds an error CSS class, but there doesn’t appear to be any .error styling in the website CSS. Either add styling for the error state (e.g., red text/border) or remove the class to avoid dead/unused UI states.
| "@modelcontextprotocol/sdk": "^0.6.0", | ||
| "@modelcontextprotocol/sdk": "^1.25.2", | ||
| "dotenv": "^16.3.1", | ||
| "zod": "^3.22.4" |
There was a problem hiding this comment.
@modelcontextprotocol/sdk@^1.25 declares a peer dependency on zod of ^3.25 || ^4.0 (and also depends on the same range). This package.json currently allows installing zod versions <3.25 via ^3.22.4, which can lead to peer-dep warnings or runtime incompatibilities if a lockfile changes. Update the declared zod range to at least ^3.25.0 (or move to ^4).
| "zod": "^3.22.4" | |
| "zod": "^3.25.0" |
Addresses unhandled promise rejections in clipboard write operations that fail in non-secure contexts, unsupported browsers, or when permissions are denied.
Changes
navigator.clipboard.writeText()in try-catch with proper rejection handlingdocument.execCommand('copy')fallback for environments where Clipboard API is unavailable or fails@modelcontextprotocol/sdkfrom 0.6.0 to 1.26.0 to address:Implementation
Both copy button handlers now use the unified
copyToClipboard()function and provide visual feedback for failures.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.