feat(chat): add rich message copy#48
Conversation
Mingholy
left a comment
There was a problem hiding this comment.
Code Review: PR #48
Summary
Adds a rich message copy feature to chat messages (both user and assistant). When clicking the copy button on a message, the clipboard receives both text/html (rendered DOM with UI chrome stripped) and text/plain (raw markdown) payloads. The clipboard logic is extracted into shared modules (clipboard.ts, useClipboard.ts), and a data-copy-ignore attribute convention is used throughout to mark elements (tool calls, code headers, thinking triggers, footers) that should be stripped from the copied HTML.
Implementation Quality
Architecture is clean and well-layered:
clipboard.tsprovides low-level primitives with a solid fallback chain:ClipboardItemAPI (rich) ->navigator.clipboard.writeText(plain) ->document.execCommand("copy")(legacy/insecure contexts).useClipboard.tsextracts the plain-text copy hook previously inlined inMarkdownBlock.tsx, now shared byCopyButtonandCodeHeader.MessageCopyButtonis purpose-built for message-level rich copy with its ownidle/copied/failedstate machine, correctly separate from the simpleruseClipboardhook since it callswriteRichClipboarddirectly.
The data-copy-ignore attribute pattern is well-designed -- it is declarative, extensible, and applied comprehensively: JSON block headers, thinking triggers, tool call wrappers, code headers, footer chrome, and the copy button itself are all marked. The companion select-none utility is a good UX detail.
CSS selector [data-role="assistant"]:not(:has(~ [data-role="assistant"])) .aui-msg-copy is clever for keeping the latest assistant message's copy button always visible. Note that :has() with the general sibling combinator has broad support in modern browsers but will silently fail in Firefox < 121 (Dec 2023). Given this app's target audience, this is probably fine.
Code header data-copy-ignore scope change: Previously only the inner button container was ignored; now the entire header (including the language label) is ignored. This is correct -- you don't want "typescript Copy Wrap" in pasted content.
Code Quality
Minor concerns (non-blocking):
-
setTimeoutcleanup on unmount: BothMessageCopyButtonanduseClipboardset state viasetTimeoutwithout cleanup on unmount. If the component unmounts before the timer fires,setStateis called on an unmounted component. In React 18+ this is a silent no-op, but it is still technically a minor leak. AuseRefflag oruseEffectcleanup would be more correct, though not worth blocking the PR. -
Dual-naming in
useClipboard: The hook exports bothcopied/isCopied(identical) andcopy/copyToClipboard(identical) for backward compatibility with two call sites. This is pragmatic but slightly messy -- consider picking one name pair and updating call sites, or adding a brief comment explaining why both exist. -
buildClipboardHtmlclones the entire content DOM on each click viacloneNode(true). For very long messages this could be expensive, but as an on-click operation it is acceptable. -
getPlainTextinAssistantMessage: TheuseCallback(() => textContent, [textContent])is fine, but sincetextContentis a string that changes on every stream chunk, the callback identity updates frequently during streaming. This is harmless because the button is hidden whileisRunningis true, but worth being aware of if the visibility condition ever changes.
Security
- No XSS risk:
buildClipboardHtmlwritesinnerHTMLto the clipboard (viaBlob), not back into the DOM. execCommandCopyfallback uses a standard off-screen textarea pattern with proper cleanup infinally.- No user-supplied strings are injected unsanitized into the DOM.
- The
data-copy-ignorestripping operates on a clone, so the live DOM is never mutated.
No security concerns.
Verdict
LGTM. This is a well-structured feature with good separation of concerns, thorough fallback handling, and an elegant declarative pattern for controlling what gets copied. The minor concerns listed above (setTimeout cleanup, dual-naming) are non-blocking quality nits that can be addressed in a follow-up if desired.
Summary
Screenshot
A per-message copy button now renders in the footer of every chat message (highlighted in red below; also visible under the following user message). Clicking copies a rich clipboard payload where supported, falling back to plain text.
Test plan
cd web && bunx tsc -bbun run buildgit diff --checkweb, andserverpackage manifests do not define alintscript.