feat(viewer): artifact copy button in toolbar - #13
Merged
Merged
Conversation
- Toolbar Copy uses getArtifactBody (same raw text as Download) with feedback states - Extract copyTextToClipboard for link creator and shell (clipboard API + fallback) - Document shell actions; add Playwright coverage for copy Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughA clipboard-copy utility was added and used by ViewerShell and Link Creator to enable copying artifact bodies; the viewer toolbar UI and Playwright tests were updated. Documentation files were revised to document copy, download, and markdown print-to-PDF capabilities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ViewerShell
participant ClipboardAPI
participant FallbackDOM
User->>ViewerShell: Click "Copy" button
ViewerShell->>ViewerShell: read artifact body (getArtifactBody)
ViewerShell->>ClipboardAPI: call navigator.clipboard.writeText(body)
alt Clipboard API available & succeeds
ClipboardAPI-->>ViewerShell: resolves
ViewerShell-->>User: show "Copied" state (auto-reset after 2s)
else Clipboard API unavailable or rejects
ViewerShell->>FallbackDOM: create hidden textarea, select text, execCommand("copy")
FallbackDOM-->>ViewerShell: execCommand result (success/fail)
ViewerShell-->>User: show "Copied" or "Copy failed" state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying agent-render with
|
| Latest commit: |
5c519b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0fafabaa.agent-render.pages.dev |
| Branch Preview URL: | https://cursor-artifacts-copy-button.agent-render.pages.dev |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThe PR adds clipboard copy functionality to the artifact viewer. The implementation is well-structured:
Changes Reviewed
Files Reviewed (7 files)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af14386875
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+5
to
+8
| export async function copyTextToClipboard(value: string): Promise<void> { | ||
| if (typeof navigator !== "undefined" && navigator.clipboard?.writeText) { | ||
| await navigator.clipboard.writeText(value); | ||
| return; |
There was a problem hiding this comment.
navigator.clipboard.writeText rejects
In browsers/webviews that expose navigator.clipboard but reject writeText at runtime (for example because clipboard-write is blocked or the page is not in an allowed context), this returns immediately and never tries the execCommand("copy") fallback below. That means the new artifact-copy button can show Copy failed even though the legacy fallback would have worked. Catching writeText failures here before falling back would make the new viewer action behave reliably across those environments.
Useful? React with 👍 / 👎.
Comment on lines
+359
to
+361
| try { | ||
| await copyTextToClipboard(getArtifactBody(activeArtifact)); | ||
| setArtifactCopyState("copied"); |
There was a problem hiding this comment.
Because clipboard writes are async, a user can click Copy on artifact A and switch to artifact B before the promise resolves. When A's copy eventually succeeds, this unconditionally sets the shared toolbar state to copied, so B now advertises Copied even though its contents were never copied. The success/failure update needs to be tied to the artifact that initiated the request (or discarded once activeArtifact changes).
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/e2e/viewer.spec.ts (1)
195-211: Pre-clear the probe key to prevent stale-storage false positives.Consider removing
copied-artifact-bodybefore stubbing/clicking so the poll can only pass from the current action.Suggested tweak
await page.evaluate(() => { + window.localStorage.removeItem("copied-artifact-body"); Object.defineProperty(navigator, "clipboard", { configurable: true, value: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/viewer.spec.ts` around lines 195 - 211, Pre-clear the localStorage probe key before stubbing the clipboard and performing the click: call page.evaluate(() => window.localStorage.removeItem("copied-artifact-body")) immediately before the Object.defineProperty(navigator, "clipboard", ...) block (or before the click) so the subsequent poll that reads window.localStorage.getItem("copied-artifact-body") cannot return a stale value; keep references to the existing symbols (navigator.clipboard stub, "copied-artifact-body", page.evaluate, and the poll/assertion) and only add the single removeItem call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/viewer-shell.tsx`:
- Around line 354-365: The async copy handler handleArtifactCopy can update
artifactCopyState after activeArtifact changes; guard updates by capturing the
current artifact id (e.g., activeArtifact.id) or a request token before awaiting
copyTextToClipboard(getArtifactBody(activeArtifact)) and verify it still matches
the current activeArtifact (or token) before calling
setArtifactCopyState("copied") or setArtifactCopyState("failed"); implement this
check both in the try and catch paths so stale completions cannot overwrite
state for a newly selected artifact.
In `@src/lib/copy-text.ts`:
- Around line 6-9: The current early return when navigator.clipboard.writeText
exists causes runtime errors to skip the sync fallback; update the clipboard
branch in the copy function so you call navigator.clipboard.writeText(value)
inside a try/catch (catching any error like NotAllowedError) and only return on
success—if it throws, swallow/handle the error and allow the function to
continue to the existing document.execCommand("copy") fallback; reference
navigator.clipboard.writeText and document.execCommand("copy") in the function
(e.g., the copyText handler) so the async failure falls through to the
synchronous fallback.
---
Nitpick comments:
In `@tests/e2e/viewer.spec.ts`:
- Around line 195-211: Pre-clear the localStorage probe key before stubbing the
clipboard and performing the click: call page.evaluate(() =>
window.localStorage.removeItem("copied-artifact-body")) immediately before the
Object.defineProperty(navigator, "clipboard", ...) block (or before the click)
so the subsequent poll that reads
window.localStorage.getItem("copied-artifact-body") cannot return a stale value;
keep references to the existing symbols (navigator.clipboard stub,
"copied-artifact-body", page.evaluate, and the poll/assertion) and only add the
single removeItem call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fcbacec8-a3d4-4c3e-938d-b0c72256ead5
📒 Files selected for processing (9)
AGENTS.mdREADME.mddocs/architecture.mddocs/payload-format.mddocs/testing.mdsrc/components/home/link-creator.tsxsrc/components/viewer-shell.tsxsrc/lib/copy-text.tstests/e2e/viewer.spec.ts
…obe reset - Ignore copy completion when activeArtifact.id changed during await - Try navigator.clipboard.writeText then fall back to execCommand on failure - Clear copied-artifact-body in clipboard stub evaluate before assert Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/viewer-shell.tsx`:
- Around line 245-247: The artifact copy status can be overwritten by stale
async completions; modify the copy flow to use a per-request token and guard
updates by checking both token and artifact id before calling
setArtifactCopyState: generate a new unique token when a copy starts, store it
alongside activeArtifactIdRef (e.g., activeArtifactIdRef.current and
activeCopyTokenRef), attach the token to the async copy promise, and on
resolution/rejection verify that activeArtifactIdRef matches the artifact id and
the token matches the stored token before setting artifactCopyState to "copied"
or "failed" (also apply same guard in any other copy handlers referenced around
lines 356-376).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29dec23d-a078-449c-808c-f45d6724aeb2
📒 Files selected for processing (3)
src/components/viewer-shell.tsxsrc/lib/copy-text.tstests/e2e/viewer.spec.ts
✅ Files skipped from review due to trivial changes (1)
- tests/e2e/viewer.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/copy-text.ts
- Place Copy before Download; keep Download as primary - Auto-clear copied and failed states after 2s - handleArtifactCopy: use activeArtifactRef + useCallback([]) for honest memoization - E2E: assert Copy failed when clipboard rejects and execCommand copy returns false Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Prevents overlapping Copy clicks on the same artifact from applying stale success/failure to the latest UI state. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Baseline PNGs drifted after Copy was added to the artifact stage header; regenerated via playwright test visual.spec.ts --update-snapshots. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
origExec expects string | undefined; coerce null from the wrapper to undefined. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/viewer-shell.tsx (1)
508-515: Consider addingaria-livefor accessibility.The copy button correctly reflects all three states. For improved screen reader support, consider adding
aria-live="polite"to announce state transitions ("Copied" / "Copy failed") without requiring focus.♿ Optional accessibility enhancement
<button type="button" className={cn("artifact-action", artifactCopyState === "copied" && "is-primary")} onClick={handleArtifactCopy} + aria-live="polite" > {artifactCopyState === "copied" ? <Check className="h-3.5 w-3.5" /> : <Copy className="h-3.5 w-3.5" />} {artifactCopyState === "copied" ? "Copied" : artifactCopyState === "failed" ? "Copy failed" : "Copy"} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/viewer-shell.tsx` around lines 508 - 515, Add an accessible live region to the copy button in the ViewerShell component so screen readers announce the artifactCopyState changes; specifically update the button element (the one using artifactCopyState and handleArtifactCopy) to include aria-live="polite" (or wrap the text node in a visually-hidden element with aria-live="polite") so transitions "Copied" / "Copy failed" are announced without requiring focus.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/viewer-shell.tsx`:
- Around line 508-515: Add an accessible live region to the copy button in the
ViewerShell component so screen readers announce the artifactCopyState changes;
specifically update the button element (the one using artifactCopyState and
handleArtifactCopy) to include aria-live="polite" (or wrap the text node in a
visually-hidden element with aria-live="polite") so transitions "Copied" / "Copy
failed" are announced without requiring focus.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f57ad9b9-ae74-4b5b-8e51-d48f0a12f18e
⛔ Files ignored due to path filters (8)
tests/e2e/visual.spec.ts-snapshots/bundle-switcher-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/code-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/csv-compact-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/diff-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/empty-state-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/json-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/markdown-dark-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/markdown-light-chromium.pngis excluded by!**/*.png
📒 Files selected for processing (2)
src/components/viewer-shell.tsxtests/e2e/viewer.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/e2e/viewer.spec.ts
This was referenced Mar 27, 2026
This was referenced Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getArtifactBody()(markdown/code/csv/jsoncontent; diff usespatchoroldContent/newContentjoin).src/lib/copy-text.ts(copyTextToClipboard) — Async Clipboard API withexecCommand('copy')fallback — and reuses it from the link creator to avoid duplication.navigator.clipboard.writeTextfailures fall through to the sync fallback.Test plan
npm run lintnpm run testnpx playwright test tests/e2e/viewer.spec.ts -g "copy action"Summary by CodeRabbit
New Features
Documentation
Tests