-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat/chatui quote text #4387
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
feat/chatui quote text #4387
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.
Hey - 我发现了 3 个问题,并给出了一些总体反馈:
- 在
handleTextSelection中,window.getSelection().getRangeAt(0)在未检查selection和rangeCount的情况下被调用,当没有活动选区时可能会抛出异常;建议在访问 range 之前先通过 early-return 做保护。 - 在
useMessages.ts中还有一处console.log('ReplyTo in sendMessage:', replyTo);,看起来是调试代码,建议移除或放到条件日志中。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `handleTextSelection`, `window.getSelection().getRangeAt(0)` is used without checking `selection` and `rangeCount`, which may throw when there is no active selection; consider early-return guards before accessing the range.
- There is a `console.log('ReplyTo in sendMessage:', replyTo);` left in `useMessages.ts` which looks like debug code and should likely be removed or behind a conditional logger.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/chat/MessageList.vue:372-381` </location>
<code_context>
+ handleTextSelection() {
</code_context>
<issue_to_address>
**issue (bug_risk):** 为 `window.getSelection()` 和 `getRangeAt(0)` 的使用增加 null / range-count 检查保护。
在某些浏览器或快速交互中,`window.getSelection()` 可能为 `null`,或者 `rangeCount === 0`,这会导致调用 `getRangeAt(0)` 时抛出异常。请在调用 `toString()` / `getRangeAt` 之前,当 `!selection` 或 `selection.rangeCount === 0` 时直接返回;同时在 `handleQuoteSelected` 中调用 `removeAllRanges()` 之前也做相同的检查。
</issue_to_address>
### Comment 2
<location> `dashboard/src/components/chat/MessageList.vue:279-282` </location>
<code_context>
</div>
+
+ <!-- 浮动引用按钮 -->
+ <div v-if="selectedText.content && selectedText.messageIndex !== null" class="selection-quote-button" :style="{
+ top: selectedText.position.top + 'px',
+ left: selectedText.position.left + 'px',
+ position: 'fixed'
+ }">
+ <v-btn size="large" rounded="xl" @click="handleQuoteSelected" class="quote-btn"
</code_context>
<issue_to_address>
**nitpick:** 避免在类的 CSS 和内联样式中重复设置 `position: fixed`。
由于 `.selection-quote-button` 已在 CSS 中设置了 `position: fixed`,可以移除内联样式里的 `position: 'fixed'`,以便将定位控制集中在一个地方。
</issue_to_address>
### Comment 3
<location> `dashboard/src/components/chat/Chat.vue:299-308` </location>
<code_context>
+ }
+
+ // 使用选中的文本作为引用内容,而不是整个消息
+ let displayText = selectedText.trim();
+
+ // 截断过长的内容
+ if (displayText.length > 100) {
+ displayText = displayText.substring(0, 100) + '...';
+ }
+
+ replyTo.value = {
+ messageId,
+ selectedText: selectedText // 保存原始的选中文本
+ };
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 回复预览的截断逻辑已计算但未被真正使用。
`displayText` 会被裁剪并截断到 100 个字符,但在设置 `replyTo.value` 时并未使用。如果输入框中的预览需要被缩短,请在 `selectedText` 中使用 `displayText`(或引入单独的预览字段)。如果不需要,则移除 `displayText` 以避免死代码。
Suggested implementation:
```
// 使用选中的文本作为引用内容,而不是整个消息
let displayText = (selectedText ?? '').trim();
// 截断过长的内容用于输入框预览
if (displayText.length > 100) {
displayText = displayText.substring(0, 100) + '...';
}
replyTo.value = {
messageId,
selectedText: displayText
};
}
```
1. 确保 `const replyTo = ref<ReplyInfo | null>(null);` 只在 `Chat.vue` 的 `<script setup>`(或 setup 函数)中合适的作用域里声明一次,而不是在 `handleReplyWithText` 内部声明。
2. 如果 `ReplyInfo` 目前还没有 `selectedText` 字段,请更新其类型 / 接口定义,以便允许 `selectedText: string`。
3. 如果在其他地方需要单独的“完整选中文本”,可以新增一个属性(例如 `fullSelectedText`),将未截断的 `selectedText` 存到该字段,同时使用 `displayText` 作为预览。
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的 Review。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
handleTextSelection,window.getSelection().getRangeAt(0)is used without checkingselectionandrangeCount, which may throw when there is no active selection; consider early-return guards before accessing the range. - There is a
console.log('ReplyTo in sendMessage:', replyTo);left inuseMessages.tswhich looks like debug code and should likely be removed or behind a conditional logger.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `handleTextSelection`, `window.getSelection().getRangeAt(0)` is used without checking `selection` and `rangeCount`, which may throw when there is no active selection; consider early-return guards before accessing the range.
- There is a `console.log('ReplyTo in sendMessage:', replyTo);` left in `useMessages.ts` which looks like debug code and should likely be removed or behind a conditional logger.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/chat/MessageList.vue:372-381` </location>
<code_context>
+ handleTextSelection() {
</code_context>
<issue_to_address>
**issue (bug_risk):** Add null/range-count guards around `window.getSelection()` and `getRangeAt(0)` usage.
In some browsers or quick interactions, `window.getSelection()` may be `null` or have `rangeCount === 0`, causing `getRangeAt(0)` to throw. Please early-return when `!selection` or `selection.rangeCount === 0` before calling `toString()`/`getRangeAt`, and do the same in `handleQuoteSelected` before `removeAllRanges()`.
</issue_to_address>
### Comment 2
<location> `dashboard/src/components/chat/MessageList.vue:279-282` </location>
<code_context>
</div>
+
+ <!-- 浮动引用按钮 -->
+ <div v-if="selectedText.content && selectedText.messageIndex !== null" class="selection-quote-button" :style="{
+ top: selectedText.position.top + 'px',
+ left: selectedText.position.left + 'px',
+ position: 'fixed'
+ }">
+ <v-btn size="large" rounded="xl" @click="handleQuoteSelected" class="quote-btn"
</code_context>
<issue_to_address>
**nitpick:** Avoid duplicating `position: fixed` in both class CSS and inline style.
Since `.selection-quote-button` already sets `position: fixed` in CSS, drop the inline `position: 'fixed'` so positioning is controlled in one place.
</issue_to_address>
### Comment 3
<location> `dashboard/src/components/chat/Chat.vue:299-308` </location>
<code_context>
+ }
+
+ // 使用选中的文本作为引用内容,而不是整个消息
+ let displayText = selectedText.trim();
+
+ // 截断过长的内容
+ if (displayText.length > 100) {
+ displayText = displayText.substring(0, 100) + '...';
+ }
+
+ replyTo.value = {
+ messageId,
+ selectedText: selectedText // 保存原始的选中文本
+ };
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Truncation logic for reply preview is computed but not used.
`displayText` is trimmed and truncated to 100 chars but not used when setting `replyTo.value`. If the preview in the input should be shortened, use `displayText` for `selectedText` (or introduce a dedicated preview field). If not, remove `displayText` to avoid dead code.
Suggested implementation:
```
// 使用选中的文本作为引用内容,而不是整个消息
let displayText = (selectedText ?? '').trim();
// 截断过长的内容用于输入框预览
if (displayText.length > 100) {
displayText = displayText.substring(0, 100) + '...';
}
replyTo.value = {
messageId,
selectedText: displayText
};
}
```
1. Ensure that `const replyTo = ref<ReplyInfo | null>(null);` is declared once at the appropriate scope in the `<script setup>` (or setup function) of `Chat.vue`, not inside `handleReplyWithText`.
2. If `ReplyInfo` does not yet have a `selectedText` field, update its type/interface definition accordingly so that `selectedText: string` is allowed.
3. If a separate "full selected text" field is needed elsewhere, introduce an additional property (e.g. `fullSelectedText`) and store the untruncated `selectedText` there while keeping `displayText` for the preview.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <div v-if="selectedText.content && selectedText.messageIndex !== null" class="selection-quote-button" :style="{ | ||
| top: selectedText.position.top + 'px', | ||
| left: selectedText.position.left + 'px', | ||
| position: 'fixed' |
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.
nitpick: 避免在类的 CSS 和内联样式中重复设置 position: fixed。
由于 .selection-quote-button 已在 CSS 中设置了 position: fixed,可以移除内联样式里的 position: 'fixed',以便将定位控制集中在一个地方。
Original comment in English
nitpick: Avoid duplicating position: fixed in both class CSS and inline style.
Since .selection-quote-button already sets position: fixed in CSS, drop the inline position: 'fixed' so positioning is controlled in one place.
Modifications / 改动点
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
通过允许用户引用选中文本,并优化回复预览 UI 和消息渲染样式,增强聊天回复交互体验。
新功能:
增强内容:
Original summary in English
Summary by Sourcery
Enhance chat reply interactions by allowing users to quote selected text and refining the reply preview UI and message rendering styles.
New Features:
Enhancements: