fix: address #2131 review follow-ups - #2133
Merged
Merged
Conversation
Render the label attr as the default-slot fallback again so text consumers (CodeArtifact, McpJsonViewer, McpServers, TraceDialog) keep their visible caption, and drop the component-level icon key so the success icon swap no longer recreates the button and steals keyboard focus.
Contributor
📝 WalkthroughWalkthroughThe PR adds fallback label rendering to ChangesComposer and toolbar updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ActiveSession
participant composerDraftPersistence
participant useComposerSubmit
participant InputHandle
ActiveSession->>composerDraftPersistence: loadComposerDraftFromStorage
composerDraftPersistence-->>useComposerSubmit: persisted draft
useComposerSubmit->>useComposerSubmit: initialize draft tracking
useComposerSubmit->>InputHandle: restore message, files, skills, and document
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/features/chat-page/composables/useComposerSubmit.ts`:
- Around line 215-222: Keep revisions monotonic in nextRevision by using the
current session revision from draftRevisions as the lower bound for
nextDraftRevision before incrementing. Update the draft submission flow around
nextRevision and add a regression test that restores revision 2, edits the
draft, and verifies the persisted revision becomes 3.
In `@src/renderer/src/features/chat-page/model/composerDraftPersistence.ts`:
- Around line 100-101: Update toPersistableFile so it does not persist arbitrary
file.metadata; either omit metadata or construct it from an explicit schema of
bounded fields with appropriate size limits. Tighten isMessageFile to validate
the same bounded metadata shape, and add a regression test proving content-like
or large nested metadata is not persisted.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 682dacaf-6f14-4b5f-a717-2e6313b35fe1
📒 Files selected for processing (8)
src/dc-ui/components/button/DcCopyButton.vuesrc/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/features/chat-page/composables/useComposerSubmit.tssrc/renderer/src/features/chat-page/model/composerDraftPersistence.tstest/renderer/components/DcCopyButton.test.tstest/renderer/components/message/MessageToolbar.trace.test.tstest/renderer/features/chat-page/composables/useComposerSubmit.test.tstest/renderer/features/chat-page/model/composerDraftPersistence.test.ts
Load the persisted draft while useComposerSubmit is constructed: the ChatPage session watcher fires before the composable exists and each session switch remounts ChatPage, so switchComposerSession alone never restored a stored draft and dispose could flush empty state over it. Strip base64 content, thumbnails and resolved representations from persisted files so localStorage holds bounded descriptors instead of raw attachment payloads.
zhangmo8
force-pushed
the
fix/copy-button-draft-followups
branch
from
August 10, 2026 10:10
2c1e1e5 to
3da3364
Compare
6 tasks
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
2 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.
概述
#2131 合并时仍有 4 条未处理的 review 评论(另有 1 条已 resolve 但修复代码未随之落地),本 PR 作为 follow-up 逐条处理。
[P1] 构建时恢复持久化草稿(useComposerSubmit)
loadComposerDraftFromStorage()此前只能经switchComposerSession()触达,而ChatPage.vue的 immediate session watcher 在 composable 构建前运行(拿到的是初始 no-op),ChatTabView又按 session ID 重建ChatPage,导致重开会话时草稿从不恢复,且下一次dispose()可能用空的 live state 覆盖并删除已存草稿。现在在 composable 构建期间直接从 storage 初始化 message / files / fingerprints;input handle 尚未就绪时经
pendingHandleRestoreSessionId延迟恢复 document 与 skills(captureDraftForPersistence/captureLiveDraft在该窗口内也改从恢复的sessionDrafts取值,不会把空状态 flush 回去)。测试按真实 remount 路径覆盖(不手动调switchComposerSession),并断言dispose()后 storage 草稿仍在。[P1] localStorage 只存有界的附件描述符(composerDraftPersistence)
ComposerSessionDraft.files含完整MessageFile(图片的 base64content+thumbnail、文本附件全文),同步写入 localStorage 可能超配额或卡渲染进程,catch 会静默丢弃整个草稿。现持久化前经toPersistableFile()剥离content/thumbnail/resolvedRepresentation,只保留 name / path / mimeType 等描述符(isMessageFile校验仍通过)。回归测试断言序列化结果不含 base64 payload。[P2] DcCopyButton 恢复 label 可见文字 fallback
重构成 render 函数时丢了原模板的
<slot>{{ label }}</slot>兜底,CodeArtifact/McpJsonViewer/McpServers/TraceDialog这类只传label的消费方从「图标 + 文字」退化成纯图标。现以fallbackSlots恢复兜底:无默认插槽时渲染label文字。[P2] 移除组件级 icon key,保持键盘焦点
key: icon.value使copied切换时整个 DcButton 卸载重建,键盘触发复制后<button>节点被移除、焦点掉回<body>,tooltip 聚焦态同时丢失。DcButton 内部Icon自带元素级:key="icon",zoom 动画不依赖组件级 key;移除后按钮 DOM 全程稳定。测试补了document.activeElement断言(attachTo: document.body,覆盖成功态与 1200ms 回落后)。[style] MessageToolbar 补齐
icon-size="3"对应已 resolve 的「Preserve the toolbar's 12 px icon contract」评论,修复当时未随 #2131 落地,一并带上并在 trace 测试中断言。
验证
format:check/oxlint/typecheck/i18n全部通过NODE_OPTIONS=--no-experimental-webstorage才能让 jsdom 提供localStorage(engines 要求 node >=24.18 <25,CI 不受影响)Relates to #2131.
Summary by CodeRabbit