fix(chatui): add copy rollback path and error message.#5352
fix(chatui): add copy rollback path and error message.#5352Soulter merged 3 commits intoAstrBotDevs:masterfrom
Conversation
Summary of ChangesHello @hanbings, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求解决了 ChatUI 中复制消息或代码块因浏览器权限或用户手势限制而失败时,用户无法收到通知的问题。本次更改引入了带有回退机制的健壮剪贴板处理、成功和失败复制操作的视觉反馈(包括 Toast 提示和动态按钮图标/标题),并改进了这些消息的国际化。这通过清晰地传达复制操作的结果,显著提升了用户体验。 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些整体层面的反馈:
- 在
tryExecCommandCopy中,如果在 textarea 已经被追加到文档后document.execCommand('copy')抛出异常,那么该元素将不会被移除;可以考虑把removeChild调用移到finally块中,这样无论成功还是失败都能清理 DOM 节点。 - 复制成功/失败的处理目前分散在
copyTextToClipboard、copyCodeToClipboard和copyBotMessage中(包括 toast、状态标志、按钮标题);建议集中到一个单一的辅助函数中,由它返回一个结构化的结果,并在一个地方统一更新 UI/toast,以避免消息复制与代码复制之间的逻辑重复和行为偏差。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `tryExecCommandCopy`, if `document.execCommand('copy')` throws after the textarea is appended, the element is never removed; consider moving the `removeChild` call into a `finally` block so it always cleans up DOM nodes on both success and failure.
- The copy success/failure handling is now split between `copyTextToClipboard`, `copyCodeToClipboard`, and `copyBotMessage` (toasts, state flags, button titles); consider centralizing this into a single helper that returns a structured result and updates UI/toasts in one place to avoid duplicated logic and drifting behavior between message and code copying.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/chat/MessageList.vue:503-510` </location>
<code_context>
- }).catch(err => {
- console.error('复制失败:', err);
- // 如果现代API失败,使用传统方法
+ tryExecCommandCopy(text) {
+ try {
const textArea = document.createElement('textarea');
- textArea.value = code;
+ textArea.value = text;
document.body.appendChild(textArea);
+ textArea.focus();
textArea.select();
+ const ok = document.execCommand('copy');
+ document.body.removeChild(textArea);
+ return ok;
</code_context>
<issue_to_address>
**issue (bug_risk):** The fallback copy implementation can leak the temporary textarea node if `execCommand` throws.
In `tryExecCommandCopy`, the textarea is created and appended inside the `try`, but if `document.execCommand('copy')` throws, control jumps to `catch` and the node is never removed, so failed attempts will accumulate hidden textareas. Move the cleanup to a `finally` block (or equivalent) so the textarea is always removed, regardless of success or failure.
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
tryExecCommandCopy, ifdocument.execCommand('copy')throws after the textarea is appended, the element is never removed; consider moving theremoveChildcall into afinallyblock so it always cleans up DOM nodes on both success and failure. - The copy success/failure handling is now split between
copyTextToClipboard,copyCodeToClipboard, andcopyBotMessage(toasts, state flags, button titles); consider centralizing this into a single helper that returns a structured result and updates UI/toasts in one place to avoid duplicated logic and drifting behavior between message and code copying.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `tryExecCommandCopy`, if `document.execCommand('copy')` throws after the textarea is appended, the element is never removed; consider moving the `removeChild` call into a `finally` block so it always cleans up DOM nodes on both success and failure.
- The copy success/failure handling is now split between `copyTextToClipboard`, `copyCodeToClipboard`, and `copyBotMessage` (toasts, state flags, button titles); consider centralizing this into a single helper that returns a structured result and updates UI/toasts in one place to avoid duplicated logic and drifting behavior between message and code copying.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/chat/MessageList.vue:503-510` </location>
<code_context>
- }).catch(err => {
- console.error('复制失败:', err);
- // 如果现代API失败,使用传统方法
+ tryExecCommandCopy(text) {
+ try {
const textArea = document.createElement('textarea');
- textArea.value = code;
+ textArea.value = text;
document.body.appendChild(textArea);
+ textArea.focus();
textArea.select();
+ const ok = document.execCommand('copy');
+ document.body.removeChild(textArea);
+ return ok;
</code_context>
<issue_to_address>
**issue (bug_risk):** The fallback copy implementation can leak the temporary textarea node if `execCommand` throws.
In `tryExecCommandCopy`, the textarea is created and appended inside the `try`, but if `document.execCommand('copy')` throws, control jumps to `catch` and the node is never removed, so failed attempts will accumulate hidden textareas. Move the cleanup to a `finally` block (or equivalent) so the textarea is always removed, regardless of success or failure.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
bbcd77c to
2cd60b9
Compare
|
Generated docs update PR (pending manual review): AI change summary:
Experimental bot notice:
|
fixes: #5351
ChatUI 里“复制消息/代码块”在部分浏览器/HTTP 场景下会因剪贴板权限或用户手势限制导致复制失败,但之前失败时缺少用户可见提示(仅有 console 输出),用户无法感知结果。本改动为复制失败补齐 toast 提示与按钮失败状态反馈。
Modifications / 改动点
在
dashboard/src/components/chat/MessageList.vue中调整了复制功能的执行路径,优先使用同步方法避免过早消耗用户手势上下文;并添加了错误 toast 和错误消息 i18n。Screenshots or Test Results / 运行截图或测试结果
复制成功:
复制失败:
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
改进 ChatUI 中对消息和代码块的复制到剪贴板行为,提供更健壮的回退方案,并向用户清晰展示复制成功/失败的反馈。
Bug 修复:
增强功能:
Original summary in English
Summary by Sourcery
Improve ChatUI copy-to-clipboard behavior for messages and code blocks with robust fallbacks and user-visible success/failure feedback.
Bug Fixes:
Enhancements: