-
Notifications
You must be signed in to change notification settings - Fork 304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add copy to clipboard full response functionality in AssistantMessage… #6035
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current implementation uses dangerouslySetInnerHTML to render an SVG icon from a string.
- dangerouslySetInnerHTML is generally discouraged due to security risks (XSS vulnerabilities)
- You can convert this to a proper React component instead
/> | ||
)} | ||
</div> | ||
{showFeedbackButtons && feedbackButtonsOnSubmit && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try this
{showFeedbackButtons && feedbackButtonsOnSubmit && ( | |
{showFeedbackButtons && feedbackButtonsOnSubmit && ( | |
<div className="tw-flex tw-items-center"> | |
<FeedbackButtons | |
feedbackButtonsOnSubmit={feedbackButtonsOnSubmit} | |
className="tw-pr-4" | |
/> | |
<div className="tw-pl-4"> | |
<button | |
type="button" | |
className="tw-flex tw-items-center tw-gap-2 tw-text-sm tw-text-muted-foreground hover:tw-text-foreground" | |
onClick={() => { | |
navigator.clipboard.writeText( | |
message.text?.toString() || '' | |
) | |
copyButtonOnSubmit?.(message.text?.toString() || '') | |
}} | |
> | |
<CopyIcon /> | |
</button> | |
</div> | |
</div> |
and this:
const CopyIcon: React.FC<{ className?: string }> = ({ className }) => (
<svg
role="img"
height={16}
width={16}
viewBox="0 0 16 16"
fill="currentColor"
className={className}
aria-label="Copy icon"
>
<path
fillRule="evenodd"
clipRule="evenodd"
d="M4 4l1-1h5.414L14 6.586V14l-1 1H5l-1-1V4zm9 3l-3-3H5v10h8V7z"
/>
<path fillRule="evenodd" clipRule="evenodd" d="M3 1L2 2v10l1 1V2h6.414l-1-1H3z" />
</svg>
)
For a quick solution as baseline
- Replace dangerouslySetInnerHTML with proper React SVG component - Add accessibility attributes to copy button - Fix duplicate FeedbackButtons rendering - Maintain existing styling and telemetry
</> | ||
)} | ||
</div> | ||
{showFeedbackButtons && feedbackButtonsOnSubmit && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CopyButton is only shown for the very last human message.
Leave it as it is if this behavior is intented.
If you want to show the copy button on every human message in the chat, refactor to that:
{showFeedbackButtons && feedbackButtonsOnSubmit && ( | |
{showFeedbackButtons && feedbackButtonsOnSubmit && ( | |
<div className="tw-flex tw-items-center"> | |
<FeedbackButtons | |
feedbackButtonsOnSubmit={feedbackButtonsOnSubmit} | |
className="tw-pr-4" | |
/> | |
</div> | |
)} | |
<div className="tw-pl-4"> | |
<button | |
type="button" | |
className="tw-flex tw-items-center tw-gap-2 tw-text-sm tw-text-muted-foreground hover:tw-text-foreground" | |
onClick={() => { | |
navigator.clipboard.writeText(message.text?.toString() || '') | |
copyButtonOnSubmit?.(message.text?.toString() || '') | |
}} | |
title="Copy message to clipboard" | |
> | |
<CopyIcon /> | |
</button> | |
</div> | |
{!isLoading && (!message.error || isAborted) && ( | |
<ContextFocusActions | |
humanMessage={humanMessage} | |
longResponseTime={hasLongerResponseTime} | |
className="tw-pl-5" | |
/> | |
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed feedback and guidance! Trying my best to become a contributor to my favorite and most used tool.
- Moved copy button outside of loading/error conditions - Improved UI layout with consistent button placement - Added wrapper div for better structure - Simplified conditional rendering logic - Maintained existing feedback and context focus functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds a copy button to chat messages, matching the existing code block copy functionality in Cody. The button uses the same icon and styling as code blocks for visual consistency across the interface.
Changes
Test Plan
Manual Testing
Screenshots