Skip to content

feat(chat): add rich message copy#48

Open
Mingholy wants to merge 1 commit into
mainfrom
feat/ft-2-rich-copy
Open

feat(chat): add rich message copy#48
Mingholy wants to merge 1 commit into
mainfrom
feat/ft-2-rich-copy

Conversation

@Mingholy

@Mingholy Mingholy commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a reusable message copy button for chat messages.
  • Prefer rich clipboard payloads where supported while preserving plain text fallback behavior.
  • Improve copy handling for rendered markdown/user/assistant content.

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.

after

Test plan

  • cd web && bunx tsc -b
  • bun run build
  • git diff --check
  • Focused clipboard/copy helper coverage was run during implementation.
  • Lint not run: root, web, and server package manifests do not define a lint script.

@Mingholy Mingholy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts provides low-level primitives with a solid fallback chain: ClipboardItem API (rich) -> navigator.clipboard.writeText (plain) -> document.execCommand("copy") (legacy/insecure contexts).
  • useClipboard.ts extracts the plain-text copy hook previously inlined in MarkdownBlock.tsx, now shared by CopyButton and CodeHeader.
  • MessageCopyButton is purpose-built for message-level rich copy with its own idle/copied/failed state machine, correctly separate from the simpler useClipboard hook since it calls writeRichClipboard directly.

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):

  1. setTimeout cleanup on unmount: Both MessageCopyButton and useClipboard set state via setTimeout without cleanup on unmount. If the component unmounts before the timer fires, setState is called on an unmounted component. In React 18+ this is a silent no-op, but it is still technically a minor leak. A useRef flag or useEffect cleanup would be more correct, though not worth blocking the PR.

  2. Dual-naming in useClipboard: The hook exports both copied/isCopied (identical) and copy/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.

  3. buildClipboardHtml clones the entire content DOM on each click via cloneNode(true). For very long messages this could be expensive, but as an on-click operation it is acceptable.

  4. getPlainText in AssistantMessage: The useCallback(() => textContent, [textContent]) is fine, but since textContent is a string that changes on every stream chunk, the callback identity updates frequently during streaming. This is harmless because the button is hidden while isRunning is true, but worth being aware of if the visibility condition ever changes.

Security

  • No XSS risk: buildClipboardHtml writes innerHTML to the clipboard (via Blob), not back into the DOM.
  • execCommandCopy fallback uses a standard off-screen textarea pattern with proper cleanup in finally.
  • No user-supplied strings are injected unsanitized into the DOM.
  • The data-copy-ignore stripping 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant