Shell DnD extensions: Messages source, Library source, TextEditor target#242
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (47)
📝 WalkthroughWalkthroughThis PR implements drag-and-drop functionality across the application. It adds drag sources to library knowledge items and chat messages, enables a drop target in the text editor that accepts both payload types, and converts dropped content into appropriately formatted Markdown. The editor now exposes an imperative Changes
Sequence DiagramsequenceDiagram
actor User
participant LibraryApp
participant MessageHoverActions
participant DnDBus as DnD Bus
participant TextEditorApp
participant CodeMirror
User->>LibraryApp: Drag knowledge item
LibraryApp->>DnDBus: useDragSource({ kind: "knowledge", id, title, url })
DnDBus->>DnDBus: Set htmlMirror (text/plain, text/uri-list)
DnDBus->>DnDBus: startDrag() event
User->>MessageHoverActions: Drag message via handle
MessageHoverActions->>DnDBus: useDragSource({ kind: "message", channel_id, excerpt, url })
DnDBus->>DnDBus: Set htmlMirror (blockquote + link)
DnDBus->>DnDBus: startDrag() event
User->>TextEditorApp: Drop over editor
TextEditorApp->>DnDBus: useDropTarget listener
DnDBus->>TextEditorApp: Emit payload (kind, data)
TextEditorApp->>TextEditorApp: Convert to Markdown
TextEditorApp->>CodeMirror: editorRef.insertAtCursor(markdown)
CodeMirror->>User: Text inserted at cursor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Reviewed by seed-2-0-pro-260328 · 235,685 tokens |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
desktop/src/apps/TextEditorApp.tsx (1)
232-245: Consider adding type guards for payload properties.The
onDrophandler accesses payload properties (channel_id,message_id,author_id,excerpt,title,url) without type narrowing beyondpayload.kind. While theacceptarray filters valid payloads, adding explicit type definitions would improve maintainability and catch integration issues early.💡 Optional: Define payload types
interface MessagePayload { kind: "message"; channel_id: string; message_id: string; author_id: string; excerpt: string; } interface KnowledgePayload { kind: "knowledge"; id: string; title: string; url?: string; } type DropPayload = MessagePayload | KnowledgePayload; // Then in onDrop: onDrop(payload: DropPayload) { // TypeScript will now narrow based on kind check }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop/src/apps/TextEditorApp.tsx` around lines 232 - 245, Declare explicit discriminated union types for the drop payload (e.g., MessagePayload with kind: "message" and fields channel_id, message_id, author_id, excerpt; KnowledgePayload with kind: "knowledge" and fields id, title, url?) and a DropPayload = MessagePayload | KnowledgePayload, then annotate the onDrop parameter in the useDropTarget call as (payload: DropPayload) so TypeScript narrows by payload.kind inside the onDrop handler; update references in onDrop (the branch for "message" should use MessagePayload properties and the "knowledge" branch should use KnowledgePayload properties) and keep the final editorRef.current?.insertAtCursor(text) call unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@desktop/src/apps/TextEditorApp.tsx`:
- Around line 232-245: Declare explicit discriminated union types for the drop
payload (e.g., MessagePayload with kind: "message" and fields channel_id,
message_id, author_id, excerpt; KnowledgePayload with kind: "knowledge" and
fields id, title, url?) and a DropPayload = MessagePayload | KnowledgePayload,
then annotate the onDrop parameter in the useDropTarget call as (payload:
DropPayload) so TypeScript narrows by payload.kind inside the onDrop handler;
update references in onDrop (the branch for "message" should use MessagePayload
properties and the "knowledge" branch should use KnowledgePayload properties)
and keep the final editorRef.current?.insertAtCursor(text) call unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d03cc5e8-1b62-4e10-af3e-3f4a11a95625
📒 Files selected for processing (49)
desktop/src/apps/LibraryApp.tsxdesktop/src/apps/MessagesApp.tsxdesktop/src/apps/TextEditorApp.tsxdesktop/src/apps/chat/MessageHoverActions.tsxstatic/desktop/assets/FilesApp-DpnvIlme.jsstatic/desktop/assets/FilesApp-hHoWf0qk.jsstatic/desktop/assets/LibraryApp-B3Qs_vI3.jsstatic/desktop/assets/LibraryApp-BD1Wtjm7.jsstatic/desktop/assets/MCPApp-X5Od8JcZ.jsstatic/desktop/assets/MessagesApp-CkhTvSE9.jsstatic/desktop/assets/MessagesApp-DQEnD9za.jsstatic/desktop/assets/ProvidersApp-DXGqum2f.jsstatic/desktop/assets/SettingsApp-gwQ4lLOZ.jsstatic/desktop/assets/TextEditorApp-B1gGb1kQ.jsstatic/desktop/assets/chat-CwQLh7dR.jsstatic/desktop/assets/dnd-bus-CVZcw0P6.jsstatic/desktop/assets/index-B-i367VX.jsstatic/desktop/assets/index-B9J0eMlg.jsstatic/desktop/assets/index-BCptoF-3.jsstatic/desktop/assets/index-BEv-dcGk.jsstatic/desktop/assets/index-BMX3OI1B.jsstatic/desktop/assets/index-BNNBySNU.jsstatic/desktop/assets/index-BbkpXBNy.jsstatic/desktop/assets/index-BjF_tFpD.jsstatic/desktop/assets/index-Bm3UY28S.jsstatic/desktop/assets/index-Bs5JT6mG.jsstatic/desktop/assets/index-By3Mzjc4.jsstatic/desktop/assets/index-CRWv8a-3.jsstatic/desktop/assets/index-Crp4FTaY.jsstatic/desktop/assets/index-DMFyin8A.jsstatic/desktop/assets/index-DTZBLne8.jsstatic/desktop/assets/index-DUWvMC11.jsstatic/desktop/assets/index-DYziI-B-.jsstatic/desktop/assets/index-DuDKTnwy.jsstatic/desktop/assets/index-Dw_yo5I1.jsstatic/desktop/assets/index-EGXegujk.jsstatic/desktop/assets/index-HCqzTdtf.jsstatic/desktop/assets/index-OlIDYpbO.jsstatic/desktop/assets/index-TacK6wxg.jsstatic/desktop/assets/index-o7Ll8rKG.jsstatic/desktop/assets/main-DL8nigqD.jsstatic/desktop/assets/main-FOmUFQfu.jsstatic/desktop/assets/tokens-D7VjNhnz.jsstatic/desktop/assets/tokens-DzPH3AJU.cssstatic/desktop/assets/use-drag-source-t6Ma6rgI.jsstatic/desktop/assets/use-drop-target-DMliBCfi.jsstatic/desktop/assets/vendor-codemirror-DhKtWjws.jsstatic/desktop/chat.htmlstatic/desktop/index.html
💤 Files with no reviewable changes (7)
- static/desktop/assets/index-Crp4FTaY.js
- static/desktop/assets/index-BEv-dcGk.js
- static/desktop/assets/LibraryApp-BD1Wtjm7.js
- static/desktop/assets/FilesApp-DpnvIlme.js
- static/desktop/assets/index-DUWvMC11.js
- static/desktop/assets/index-DTZBLne8.js
- static/desktop/assets/MessagesApp-CkhTvSE9.js
…arget
- MessageHoverActions gains optional dragHandle slot
- MessagesApp adds a ⋮⋮ drag handle in the hover toolbar per message;
publishes {kind:"message", channel_id, message_id, author_id, excerpt}
+ text/plain + text/uri-list mirrors
- LibraryApp knowledge items become drag sources via KnowledgeItemDragWrapper
({kind:"knowledge", id, title, url} + text/plain/uri-list)
- TextEditorApp (CodeMirror) accepts message + knowledge drops;
MarkdownEditor exposes insertAtCursor via forwardRef so the parent drop
target can write markdown at the caret
jaylfc
force-pushed
the
feat/shell-dnd-extensions
branch
from
bf27dbe to
3b1bb71
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
1 participant
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.
Summary
Extends the shell cross-app drag-drop primitive (#239) to cover three more wirings, closing the second half of the DnD rollout.
New drag sources
⋮⋮drag handle in the hover toolbar. Dragging publishes `{kind:"message", channel_id, message_id, author_id, excerpt}` + HTML5 `text/plain` ("@agent: excerpt") + `text/uri-list` (deep-link to the message).New drop target
Implementation notes
Test plan
Follow-ups (not in this PR)
Summary by CodeRabbit
Release Notes
New Features
Improvements