feat(chat): provide buttons to copy messages or snippets#1205
Open
fbricon wants to merge 1 commit intokortex-hub:mainfrom
Open
feat(chat): provide buttons to copy messages or snippets#1205fbricon wants to merge 1 commit intokortex-hub:mainfrom
fbricon wants to merge 1 commit intokortex-hub:mainfrom
Conversation
fbricon
requested review from
benoitf and
gastoner
and removed request for
a team
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements copy and edit functionality for chat messages and code snippets. It introduces a MessageActions component providing per-message action buttons, refactors icon rendering to use svelte-fa, integrates copy buttons into markdown code blocks, and adds comprehensive test coverage for the new functionality. Changes
Sequence DiagramssequenceDiagram
participant User
participant MessageActions as MessageActions<br/>Component
participant EditState as EditState<br/>Context
participant Clipboard as window.clipboard<br/>API
User->>MessageActions: Click "Copy message"
activate MessageActions
MessageActions->>MessageActions: Filter text parts<br/>from message
MessageActions->>Clipboard: clipboardWriteText(text)
Clipboard-->>MessageActions: Promise resolves
MessageActions->>MessageActions: Set copied=true<br/>Update button UI
MessageActions->>MessageActions: Schedule 2s reset
deactivate MessageActions
MessageActions-->>User: Show "Copied!" feedback
User->>MessageActions: Click "Edit message"
activate MessageActions
MessageActions->>EditState: startEditing(message)
EditState-->>MessageActions: Edit mode activated
deactivate MessageActions
sequenceDiagram
participant User
participant Markdown as Markdown<br/>Component
participant DOM as Code Block<br/>Copy Button
participant Clipboard as window.clipboard<br/>API
Markdown->>DOM: afterUpdate: Inject copy button<br/>into pre elements
activate DOM
User->>DOM: Click copy button
DOM->>DOM: Extract code text<br/>from code element
DOM->>Clipboard: clipboardWriteText(code)
Clipboard-->>DOM: Promise resolves
DOM->>DOM: Swap icon to checkmark<br/>Update tooltip to "Copied!"
DOM->>DOM: Schedule 2s revert
deactivate DOM
DOM-->>User: Show success feedback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/lib/chat/components/messages/message-actions.svelte`:
- Around line 32-39: Wrap navigator.clipboard.writeText in a try/catch inside
copyToClipboard so clipboard failures are caught and handled (e.g., log error
and do not set copied to true on failure); add a module-scoped timer variable
(e.g., let copyTimeout: number | null = null) and clear any existing timeout
(clearTimeout(copyTimeout)) before creating a new one so rapid clicks don't
create overlapping timers; store the timeout id in copyTimeout and on component
teardown (use Svelte's onDestroy) clearTimeout(copyTimeout) to avoid leaks; keep
references to the copied boolean and the getMessageText() call as-is but only
set copied = true after a successful writeText.
In `@packages/renderer/src/lib/chat/components/messages/preview-message.svelte`:
- Around line 112-114: The current conditional prevents the entire
MessageActions bar from rendering when readonly is true, hiding copy even though
readonly should only disable editing; change the render condition to remove the
top-level readonly gate so the bar still shows when hasText and (message.role
=== 'user' || !isGeneratingResponse), i.e. replace "{`#if` !readonly && hasText &&
(message.role === 'user' || !isGeneratingResponse)}" with "{`#if` hasText &&
(message.role === 'user' || !isGeneratingResponse)}", and ensure MessageActions
receives the readonly prop (MessageActions {message} {readonly}) and that the
MessageActions component itself disables or hides edit-specific controls based
on readonly while still exposing copy/share actions.
In `@packages/renderer/src/lib/markdown/Markdown.svelte`:
- Around line 74-102: The copy button and its tooltip are only exposed on :hover
which breaks keyboard accessibility; update the CSS selectors that target
.markdown > :global(pre) > :global(.code-copy-btn) and the tooltip
pseudo-element so they also respond to keyboard focus (add :focus-within on the
pre container and :focus / :focus-visible on the button) to make the button
visible and its ::after tooltip opacity transition when focused as well as
hovered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a74b0c7-5412-4188-8a31-ede05d2a681e
📒 Files selected for processing (10)
packages/renderer/src/lib/chat/components/icons/check-circle-fill.sveltepackages/renderer/src/lib/chat/components/icons/copy.sveltepackages/renderer/src/lib/chat/components/icons/paths.tspackages/renderer/src/lib/chat/components/messages/message-actions-test.sveltepackages/renderer/src/lib/chat/components/messages/message-actions.spec.tspackages/renderer/src/lib/chat/components/messages/message-actions.sveltepackages/renderer/src/lib/chat/components/messages/preview-message.sveltepackages/renderer/src/lib/chat/components/multimodal-input.sveltepackages/renderer/src/lib/markdown/Markdown.spec.tspackages/renderer/src/lib/markdown/Markdown.svelte
packages/renderer/src/lib/chat/components/messages/message-actions.svelte
Show resolved
Hide resolved
packages/renderer/src/lib/chat/components/messages/preview-message.svelte
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
fbricon
force-pushed
the
worktree-GH-1185
branch
from
9688529 to
004d803
Compare
gastoner
reviewed
Mar 31, 2026
Contributor
gastoner
left a comment
gastoner
left a comment
There was a problem hiding this comment.
Left one comment about the icons
Also I've noticed that when you click on the copy button, I believe you should see the 'Copied!' tooltip ? I can see it only when re-hovering the button
Screen.Recording.2026-03-31.at.9.23.31.mov
| 'M2 2.5C2 1.11929 3.11929 0 4.5 0H9.5C9.89782 0 10.2794 0.158035 10.5607 0.43934L13.5607 3.43934C13.842 3.72064 14 4.10218 14 4.5V10.5C14 11.8807 12.8807 13 11.5 13H4.5C3.11929 13 2 11.8807 2 10.5V2.5ZM4.5 1.5C3.94772 1.5 3.5 1.94772 3.5 2.5V10.5C3.5 11.0523 3.94772 11.5 4.5 11.5H11.5C12.0523 11.5 12.5 11.0523 12.5 10.5V5H10.5H9.75V4.25V2.25V1.5H4.5ZM11.3107 3.5L11.25 3.43934V1.93934L12.5607 3.25V3.31066L11.3107 3.5ZM0 5.5C0 4.39543 0.895431 3.5 2 3.5V5H1.5V13.5H9V13H10.5V13.5C10.5 14.6046 9.60457 15.5 8.5 15.5H2C0.895431 15.5 0 14.6046 0 13.5V5.5Z'; | ||
|
|
||
| export const checkCircleFillIconPath = | ||
| 'M16 8C16 12.4183 12.4183 16 8 16C3.58172 16 0 12.4183 0 8C0 3.58172 3.58172 0 8 0C12.4183 0 16 3.58172 16 8ZM11.5303 6.53033L12.0607 6L11 4.93934L10.4697 5.46967L6.5 9.43934L5.53033 8.46967L5 7.93934L3.93934 9L4.46967 9.53033L5.96967 11.0303C6.26256 11.3232 6.73744 11.3232 7.03033 11.0303L11.5303 6.53033Z'; |
Contributor
There was a problem hiding this comment.
I would avoid moving the svg paths to single file.
We tend to have one icon === one file
Also the copy and check-circle exists in font awesome so not sure why we are using the custom check-circle here cc @benoitf @jeffmaury ? But I'm fine with having the new custom copy button here to match the check-circle-fill custom icon
Contributor
Author
There was a problem hiding this comment.
It's now using fa icons for the files I touched
benoitf
requested changes
Mar 31, 2026
Contributor
benoitf
left a comment
benoitf
left a comment
There was a problem hiding this comment.
we do have a <CopyToClipboard> svelte component so IMHO we should reuse it
using clipboard API sometimes is not working. Also I see some timeout or timer while we have an electron API (used by the svelte component) to do the copy/paste stuff
Contributor
|
IMHO you should move out the 'tiny fixes" from big PR as it prevents to merge quickly these tiny fixes as we're discussing the big part of this PR Regarding one of the tiny fix, I believe there is an existing issue as we should not use any of the tailwind color directly, but always use constants from color-registry so the values can be tuned by a theme |
fbricon
force-pushed
the
worktree-GH-1185
branch
from
004d803 to
1da505a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/renderer/src/lib/chat/components/messages/message-actions.spec.ts (1)
113-131: Consider adding copied-state timer assertions.Optional: add fake-timer checks for “Copied!” state transition and 2s reset to guard against future regressions in the timeout logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/chat/components/messages/message-actions.spec.ts` around lines 113 - 131, Update the test "should copy message text to clipboard on click" to use fake timers and assert the transient "Copied!" UI state: before clicking assert initial label (e.g., getByLabelText('Copy message')), call fireEvent.click(getByLabelText('Copy message')), advance timers (e.g., vi.advanceTimersByTime(0)) to let the immediate state change occur and assert the button now shows "Copied!" (or equivalent), then advance timers by 2000ms and assert the label has reverted back to "Copy message"; use vi.useFakeTimers()/vi.restoreAllMocks() (or appropriate fake-timer helpers) and reference MessageActionsTest, navigator.clipboard.writeText, and the 'Copy message' label in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/lib/chat/components/messages/message-actions.svelte`:
- Around line 56-64: The actions container only becomes visible on hover, which
hides controls from keyboard users; update the class expression in the div that
builds the container (the cn call around '-mt-3 flex items-center...') to also
reveal on keyboard focus by adding the appropriate focus variant (e.g., include
'group-focus-within/message:opacity-100' or 'group-focus/message:opacity-100'
alongside 'group-hover/message:opacity-100'), and ensure interactive child
controls remain focusable (no aria-hidden or pointer-events blocking) so tabbing
will make the actions visible for keyboard users.
---
Nitpick comments:
In `@packages/renderer/src/lib/chat/components/messages/message-actions.spec.ts`:
- Around line 113-131: Update the test "should copy message text to clipboard on
click" to use fake timers and assert the transient "Copied!" UI state: before
clicking assert initial label (e.g., getByLabelText('Copy message')), call
fireEvent.click(getByLabelText('Copy message')), advance timers (e.g.,
vi.advanceTimersByTime(0)) to let the immediate state change occur and assert
the button now shows "Copied!" (or equivalent), then advance timers by 2000ms
and assert the label has reverted back to "Copy message"; use
vi.useFakeTimers()/vi.restoreAllMocks() (or appropriate fake-timer helpers) and
reference MessageActionsTest, navigator.clipboard.writeText, and the 'Copy
message' label in your changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7bb7bac9-7625-4f8b-981a-4836c86109b6
📒 Files selected for processing (9)
packages/renderer/src/lib/chat/components/icons/check-circle-fill.sveltepackages/renderer/src/lib/chat/components/icons/copy.sveltepackages/renderer/src/lib/chat/components/icons/paths.tspackages/renderer/src/lib/chat/components/messages/message-actions-test.sveltepackages/renderer/src/lib/chat/components/messages/message-actions.spec.tspackages/renderer/src/lib/chat/components/messages/message-actions.sveltepackages/renderer/src/lib/chat/components/messages/preview-message.sveltepackages/renderer/src/lib/markdown/Markdown.spec.tspackages/renderer/src/lib/markdown/Markdown.svelte
✅ Files skipped from review due to trivial changes (4)
- packages/renderer/src/lib/chat/components/icons/check-circle-fill.svelte
- packages/renderer/src/lib/chat/components/messages/message-actions-test.svelte
- packages/renderer/src/lib/chat/components/icons/paths.ts
- packages/renderer/src/lib/chat/components/icons/copy.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/renderer/src/lib/markdown/Markdown.spec.ts
- packages/renderer/src/lib/markdown/Markdown.svelte
packages/renderer/src/lib/chat/components/messages/message-actions.svelte
Show resolved
Hide resolved
fbricon
force-pushed
the
worktree-GH-1185
branch
2 times, most recently
from
98b7483 to
51e483f
Compare
Contributor
Author
|
Thanks for the review! I've switched to using Regarding reusing
The important part from -- message written by Claude |
fbricon
force-pushed
the
worktree-GH-1185
branch
from
51e483f to
7bb9441
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/renderer/src/lib/chat/components/messages/message-actions.svelte (1)
36-45:⚠️ Potential issue | 🟡 MinorMissing error handling for clipboard write.
The
window.clipboardWriteTextcall can reject (e.g., IPC failure), causing an unhandled promise rejection. While the timer overlap issue was addressed, the try/catch for error handling appears to be missing.🛡️ Proposed fix
async function copyToClipboard(): Promise<void> { const text = getMessageText(); if (copiedResetTimer) clearTimeout(copiedResetTimer); - await window.clipboardWriteText(text); - copied = true; - copiedResetTimer = setTimeout(() => { - copied = false; - copiedResetTimer = undefined; - }, 2000); + try { + await window.clipboardWriteText(text); + copied = true; + copiedResetTimer = setTimeout(() => { + copied = false; + copiedResetTimer = undefined; + }, 2000); + } catch { + // Clipboard write failed silently + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/chat/components/messages/message-actions.svelte` around lines 36 - 45, The copyToClipboard function lacks error handling around the async window.clipboardWriteText call; wrap that call in a try/catch inside copyToClipboard, only set copied = true and start copiedResetTimer if the write succeeds, and in the catch branch clear any existing copiedResetTimer, set copied = false (or leave unchanged), and surface the error (e.g., console.error or send to your app's notification/logger). Ensure you still clear the previous copiedResetTimer at the start of copyToClipboard and avoid leaking timers by always resetting copiedResetTimer to undefined when the timer clears or on error.
🧹 Nitpick comments (1)
packages/renderer/src/lib/markdown/Markdown.svelte (1)
237-248: Move imports to the top of the script block.The FontAwesome imports on lines 237-238 are placed after the
onMountfunction. While JavaScript hoists imports, placing them mid-script reduces readability. Conventional practice is to group all imports at the top of the<script>block.📦 Suggested refactor
Move lines 237-238 to immediately after line 128:
import { afterUpdate, onDestroy, onMount } from 'svelte'; +import { faCircleCheck } from '@fortawesome/free-regular-svg-icons'; +import { faCopy } from '@fortawesome/free-solid-svg-icons'; import { button } from './micromark-button-directive';Then keep
faIconToSvg,sectionRef,copyIconHtml, andcheckIconHtmldeclarations together after the reactive statement (line 161) or beforeonMount.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/markdown/Markdown.svelte` around lines 237 - 248, Move the FontAwesome imports (faCircleCheck, faCopy) to the top of the <script> block so imports are grouped before any runtime code; then place the faIconToSvg function and the related declarations (sectionRef, copyIconHtml, checkIconHtml) together either immediately after the reactive section or just before the onMount function to keep icon utilities and state grouped. Ensure you update any references to faCopy/faCircleCheck if their relative positions change, but do not alter their names or usages in faIconToSvg, copyIconHtml, checkIconHtml, or onMount.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/renderer/src/lib/chat/components/messages/message-actions.svelte`:
- Around line 36-45: The copyToClipboard function lacks error handling around
the async window.clipboardWriteText call; wrap that call in a try/catch inside
copyToClipboard, only set copied = true and start copiedResetTimer if the write
succeeds, and in the catch branch clear any existing copiedResetTimer, set
copied = false (or leave unchanged), and surface the error (e.g., console.error
or send to your app's notification/logger). Ensure you still clear the previous
copiedResetTimer at the start of copyToClipboard and avoid leaking timers by
always resetting copiedResetTimer to undefined when the timer clears or on
error.
---
Nitpick comments:
In `@packages/renderer/src/lib/markdown/Markdown.svelte`:
- Around line 237-248: Move the FontAwesome imports (faCircleCheck, faCopy) to
the top of the <script> block so imports are grouped before any runtime code;
then place the faIconToSvg function and the related declarations (sectionRef,
copyIconHtml, checkIconHtml) together either immediately after the reactive
section or just before the onMount function to keep icon utilities and state
grouped. Ensure you update any references to faCopy/faCircleCheck if their
relative positions change, but do not alter their names or usages in
faIconToSvg, copyIconHtml, checkIconHtml, or onMount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15a35916-2853-4462-97b4-9962a656f14b
📒 Files selected for processing (8)
packages/renderer/src/lib/chat/components/icons/check-circle-fill.sveltepackages/renderer/src/lib/chat/components/icons/copy.sveltepackages/renderer/src/lib/chat/components/messages/message-actions-test.sveltepackages/renderer/src/lib/chat/components/messages/message-actions.spec.tspackages/renderer/src/lib/chat/components/messages/message-actions.sveltepackages/renderer/src/lib/chat/components/messages/preview-message.sveltepackages/renderer/src/lib/markdown/Markdown.spec.tspackages/renderer/src/lib/markdown/Markdown.svelte
✅ Files skipped from review due to trivial changes (1)
- packages/renderer/src/lib/markdown/Markdown.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/renderer/src/lib/chat/components/icons/check-circle-fill.svelte
- packages/renderer/src/lib/chat/components/messages/message-actions-test.svelte
- packages/renderer/src/lib/chat/components/messages/message-actions.spec.ts
- packages/renderer/src/lib/chat/components/icons/copy.svelte
Contributor
Author
Should be fixed |
fbricon
force-pushed
the
worktree-GH-1185
branch
from
7bb9441 to
e005c6d
Compare
gastoner
approved these changes
Apr 1, 2026
Contributor
gastoner
left a comment
gastoner
left a comment
There was a problem hiding this comment.
LGTM on Mac thanks for the changes
packages/renderer/src/lib/chat/components/icons/check-circle-fill.svelte
Show resolved
Hide resolved
bmahabirbu
added a commit
to bmahabirbu/kortex
that referenced
this pull request
Apr 2, 2026
Add UI development guidelines covering color-registry usage, Icon component from @podman-desktop/ui-svelte, and component reuse priorities. Adds a new .agents/skills/ui-components/ skill for detailed reference. Addresses feedback from PR kortex-hub#1205 review (benoitf, gastoner). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
3 tasks
fbricon
force-pushed
the
worktree-GH-1185
branch
2 times, most recently
from
62aa8d5 to
c1693e4
Compare
Contributor
Author
|
@benoitf I've extracted the copy button code out of the Markdown component and into a Svelte action (use:codeCopyButtons). The button wasn't properly aligned initially, so I added a light background to the snippets, to help me figure it out. I eventually kept it because it looks better. 
|
fbricon
force-pushed
the
worktree-GH-1185
branch
2 times, most recently
from
0e939d8 to
5380864
Compare
fbricon
force-pushed
the
worktree-GH-1185
branch
from
5380864 to
1116f50
Compare
benoitf
approved these changes
Apr 3, 2026
…1185) Add a small actions area below chat messages with copy-to-clipboard and edit buttons, similar to Claude Desktop. Edit button appears for user messages only, copy button for all messages. Actions are left-aligned for assistant messages and right-aligned for user messages. Add a copy-to-clipboard button on code blocks via a Svelte action (codeCopyButtons) that injects buttons into pre elements. This keeps the Markdown component clean and reusable. The tooltip is rendered as a fixed-position element on document.body to escape the multiple nested overflow:hidden containers in the chat UI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Fred Bricon <fbricon@gmail.com>
fbricon
force-pushed
the
worktree-GH-1185
branch
from
1116f50 to
f44aff4
Compare
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
3 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.
Add a small actions area below chat messages with copy-to-clipboard and
edit buttons, similar to Claude Desktop. Edit button appears for user
messages only, copy button for all messages. Actions are left-aligned
for assistant messages and right-aligned for user messages.
Add a copy-to-clipboard button on code blocks via a Svelte action
(codeCopyButtons) that injects buttons into pre elements. This keeps
the Markdown component clean and reusable.
copy-buttons.mp4
This PR also includes 2 tiny changes to fix the message box LnF, as identified by @serbangeorge-mfix: improve attach button hover visibility in dark modefix: vertically center input button bar with equal paddingalready merged in fix(chat): fix message buttons bar LnF #1207
Fixes #1185