Resolves #364, Append macro content into the editors instead of replacing the entire content - #366
Resolves #364, Append macro content into the editors instead of replacing the entire content#366vanboom wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughMacro content handlers in two conversation UI components now preserve existing editor content. The new conversation macro and reply macro watchers append their injected content to the current editor state instead of replacing it entirely, allowing users to combine macro text with manually entered content. ChangesMacro Content Appending in Editors
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/apps/main/src/features/conversation/CreateConversation.vue (1)
503-513:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd guard to check
message_contentexists before appending.The watcher appends
message_contentwithout first verifying it exists. Sincemessage_contentis optional in the macro schema (context snippet fromformSchema.js:11-34), this could appendundefinedto the content. This is inconsistent withReplyBox.vueline 479, which includes a guard:if (conversationStore.getMacro('reply').message_content).🛡️ Proposed fix to add guard and improve consistency
watch( () => conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).id, () => { + const macro = conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION) + if (!macro.message_content) return + const currentValue = form.values?.content || '' - form.setFieldValue( - 'content', - currentValue + conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).message_content - ) + form.setFieldValue('content', currentValue + macro.message_content) }, { deep: true } )🤖 Prompt for 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. In `@frontend/apps/main/src/features/conversation/CreateConversation.vue` around lines 503 - 513, The watcher for conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).id appends message_content unguarded, which can add "undefined" because message_content is optional; update the watcher in CreateConversation.vue to first read the macro via conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION), check that macro.message_content is truthy (or !== undefined) before calling form.setFieldValue('content', ...), and only append when message_content exists—mirroring the guard used in ReplyBox.vue.
🧹 Nitpick comments (2)
frontend/apps/main/src/features/conversation/CreateConversation.vue (1)
506-510: ⚡ Quick winConsider adding spacing between existing content and appended macro.
When macro content is appended directly to existing editor content, text may run together without proper separation. Consider adding a space, newline, or other separator to ensure readability when composing multiple text snippets.
💡 Example with spacing
const currentValue = form.values?.content || '' form.setFieldValue( 'content', - currentValue + conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).message_content + currentValue + ' ' + conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).message_content )Or with a line break if that better fits the use case:
const currentValue = form.values?.content || '' form.setFieldValue( 'content', - currentValue + conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).message_content + currentValue + '<br>' + conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).message_content )🤖 Prompt for 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. In `@frontend/apps/main/src/features/conversation/CreateConversation.vue` around lines 506 - 510, Append a separator when concatenating macro text to existing editor content to avoid words running together: in the block using currentValue, form.setFieldValue, and conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).message_content, detect if currentValue is non-empty and insert a single space or newline (depending on UX preference) between currentValue and the macro message_content before calling form.setFieldValue so the appended macro is always separated from existing text.frontend/apps/main/src/features/conversation/ReplyBox.vue (1)
479-481: ⚡ Quick winConsider adding spacing between existing content and appended macro.
Similar to
CreateConversation.vue, when macro content is appended directly to existing editor content, text may run together. Consider adding a separator to improve readability when composing multiple text snippets.💡 Example with spacing
if (conversationStore.getMacro('reply').message_content) { - htmlContent.value = htmlContent.value + conversationStore.getMacro('reply').message_content + htmlContent.value = htmlContent.value + ' ' + conversationStore.getMacro('reply').message_content }Or with a line break:
if (conversationStore.getMacro('reply').message_content) { - htmlContent.value = htmlContent.value + conversationStore.getMacro('reply').message_content + htmlContent.value = htmlContent.value + '<br>' + conversationStore.getMacro('reply').message_content }🤖 Prompt for 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. In `@frontend/apps/main/src/features/conversation/ReplyBox.vue` around lines 479 - 481, The macro append currently concatenates directly: when conversationStore.getMacro('reply').message_content exists it does htmlContent.value = htmlContent.value + ... which can run text together; update the logic around htmlContent.value and conversationStore.getMacro('reply').message_content to insert a separator (e.g., a space or "\n\n" / "<br/>" depending on HTML vs plain text context) only when htmlContent.value is non-empty so appended macro content is clearly separated from existing content.
🤖 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.
Outside diff comments:
In `@frontend/apps/main/src/features/conversation/CreateConversation.vue`:
- Around line 503-513: The watcher for
conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).id appends
message_content unguarded, which can add "undefined" because message_content is
optional; update the watcher in CreateConversation.vue to first read the macro
via conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION), check that
macro.message_content is truthy (or !== undefined) before calling
form.setFieldValue('content', ...), and only append when message_content
exists—mirroring the guard used in ReplyBox.vue.
---
Nitpick comments:
In `@frontend/apps/main/src/features/conversation/CreateConversation.vue`:
- Around line 506-510: Append a separator when concatenating macro text to
existing editor content to avoid words running together: in the block using
currentValue, form.setFieldValue, and
conversationStore.getMacro(MACRO_CONTEXT.NEW_CONVERSATION).message_content,
detect if currentValue is non-empty and insert a single space or newline
(depending on UX preference) between currentValue and the macro message_content
before calling form.setFieldValue so the appended macro is always separated from
existing text.
In `@frontend/apps/main/src/features/conversation/ReplyBox.vue`:
- Around line 479-481: The macro append currently concatenates directly: when
conversationStore.getMacro('reply').message_content exists it does
htmlContent.value = htmlContent.value + ... which can run text together; update
the logic around htmlContent.value and
conversationStore.getMacro('reply').message_content to insert a separator (e.g.,
a space or "\n\n" / "<br/>" depending on HTML vs plain text context) only when
htmlContent.value is non-empty so appended macro content is clearly separated
from existing content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c2423247-2b50-42b3-9b7f-e4c529cb1c47
📒 Files selected for processing (2)
frontend/apps/main/src/features/conversation/CreateConversation.vuefrontend/apps/main/src/features/conversation/ReplyBox.vue
vanboom
changed the title
|
We should insert macro content at the cursor position - that's even better. The current string concat also sometimes leaves a stray on an empty editor inserting through the editor avoids that too. |
2 participants
Appending macros allows them to be used as composable text snippets while editing messages. This works very well in companion with the CTRL+M feature that allows quick access to the macros from the text editor.
Summary by CodeRabbit