Conversation
refactor: Sender component supports template field
…unctionality in SuggestionPills component (#133)
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sender
participant TemplateEditor
participant UndoRedo
User->>Sender: Provide templateData (UserItem[])
Sender->>TemplateEditor: Pass templateData as model-value
TemplateEditor->>UndoRedo: Commit state on input
TemplateEditor->>Sender: Emit update:model-value (UserItem[])
Sender->>User: Emit update:templateData (UserItem[])
User->>Sender: Select new template
Sender->>TemplateEditor: Update templateData, activate first field
TemplateEditor->>UndoRedo: Clear history on template change
sequenceDiagram
participant App
participant DropdownMenu
App->>DropdownMenu: Render with appendTo prop
DropdownMenu->>BasePopper: Pass appendTo as teleport target
BasePopper->>DOM: Mount menu to specified container
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
docs/demos/dropdown-menu/basic.vueOops! Something went wrong! :( ESLint: 9.30.1 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. docs/demos/sender/Template.vueOops! Something went wrong! :( ESLint: 9.30.1 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. packages/components/src/base-popper/index.vueOops! Something went wrong! :( ESLint: 9.30.1 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it.
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
packages/components/src/sender/index.vue (1)
238-256: Use the dedicated focus method for consistency.The
clearInputmethod usessenderRef.value?.focus()directly, but there's a dedicatedfocusInputmethod that handles focus logic more comprehensively, including template editor focus.const clearInput = () => { originalClearInput() // 如果当前是模板编辑模式，退出模板编辑模式（已包含聚焦逻辑） if (showTemplateEditor.value) { exitTemplateMode() } else { // 普通模式下直接聚焦 - senderRef.value?.focus() + focusInput() } // 确保DOM更新后再次检查 nextTick(() => { if (inputValue.value === '') { currentMode.value = props.mode || 'single' } }) closeSuggestionsPopup() }
🧹 Nitpick comments (3)
docs/src/components/sender.md (1)
136-136: Use proper markdown heading instead of emphasis.Replace the bold emphasis with a proper heading for better document structure and accessibility.
-**模板示例** +#### 模板示例packages/components/src/sender/components/Block.vue (1)
12-12: Consider translating the Chinese comment to English for consistency.-<!-- 设置 contenteditable="false" 是为了不会将文本插入到非text元素。但是可能导致光标不显示 --> +<!-- Setting contenteditable="false" prevents text insertion into non-text elements. However, it may cause the cursor to not display -->packages/components/src/sender/components/TemplateEditor.vue (1)
288-708: Complex but well-implemented input handling.The input handling logic is comprehensive and covers numerous edge cases including composition events, various input types, and cursor positioning. While the complexity is high, it appears necessary for proper rich-text editing functionality. The code is well-structured with clear separation of concerns.
Consider adding more inline documentation for the most complex sections to aid future maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
docs/demos/dropdown-menu/basic.vue(1 hunks)docs/demos/sender/Template.vue(3 hunks)docs/src/components/dropdown-menu.md(2 hunks)docs/src/components/sender.md(2 hunks)docs/src/releases/update-log.md(1 hunks)packages/components/package.json(1 hunks)packages/components/src/base-popper/index.vue(2 hunks)packages/components/src/dropdown-menu/index.type.ts(1 hunks)packages/components/src/dropdown-menu/index.vue(3 hunks)packages/components/src/sender/components/Block.vue(1 hunks)packages/components/src/sender/components/TemplateEditor.vue(1 hunks)packages/components/src/sender/composables/useKeyboardHandler.ts(4 hunks)packages/components/src/sender/composables/useTemplateHandler.ts(0 hunks)packages/components/src/sender/composables/useUndoRedo.ts(1 hunks)packages/components/src/sender/index.type.ts(4 hunks)packages/components/src/sender/index.vue(9 hunks)packages/components/src/sender/types/editor.type.ts(1 hunks)packages/components/src/sender/utils/zeroWidthUtils.ts(0 hunks)packages/components/src/suggestion-pills/index.vue(1 hunks)packages/kit/package.json(1 hunks)packages/svgs/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- packages/components/src/sender/utils/zeroWidthUtils.ts
- packages/components/src/sender/composables/useTemplateHandler.ts
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
packages/components/src/dropdown-menu/index.type.ts (1)
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
docs/demos/dropdown-menu/basic.vue (2)
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:0-0
Timestamp: 2025-05-27T03:45:56.392Z
Learning: In Vue components using VueUse's onClickOutside, calling stopPropagation() in the onClickOutside handler can prevent subsequent click event handlers on the same element from being triggered. This means that clicking a trigger element while a popover is open can successfully close the popover without immediately reopening it, even when the onClickOutside only listens to the popover element and not the trigger element.
packages/components/src/dropdown-menu/index.vue (3)
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:0-0
Timestamp: 2025-05-27T03:45:56.392Z
Learning: In Vue components using VueUse's onClickOutside, calling stopPropagation() in the onClickOutside handler can prevent subsequent click event handlers on the same element from being triggered. This means that clicking a trigger element while a popover is open can successfully close the popover without immediately reopening it, even when the onClickOutside only listens to the popover element and not the trigger element.
Learnt from: SonyLeo
PR: opentiny/tiny-robot#119
File: packages/components/src/attachments/index.less:213-213
Timestamp: 2025-06-18T09:29:47.974Z
Learning: 在 packages/components/src/attachments/index.less 中，.tr-file-card__close 的背景色使用了硬编码的 rgb(194, 194, 194)，但这个UI元素（关闭按钮）将会被直接替换为图标，所以不需要抽取为CSS变量。
packages/components/src/suggestion-pills/index.vue (2)
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:0-0
Timestamp: 2025-05-27T03:45:56.392Z
Learning: In Vue components using VueUse's onClickOutside, calling stopPropagation() in the onClickOutside handler can prevent subsequent click event handlers on the same element from being triggered. This means that clicking a trigger element while a popover is open can successfully close the popover without immediately reopening it, even when the onClickOutside only listens to the popover element and not the trigger element.
packages/components/src/base-popper/index.vue (2)
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:0-0
Timestamp: 2025-05-27T03:45:56.392Z
Learning: In Vue components using VueUse's onClickOutside, calling stopPropagation() in the onClickOutside handler can prevent subsequent click event handlers on the same element from being triggered. This means that clicking a trigger element while a popover is open can successfully close the popover without immediately reopening it, even when the onClickOutside only listens to the popover element and not the trigger element.
docs/src/components/sender.md (1)
Learnt from: gene9831
PR: opentiny/tiny-robot#123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.
packages/components/src/sender/components/Block.vue (1)
Learnt from: gene9831
PR: opentiny/tiny-robot#123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.
docs/demos/sender/Template.vue (1)
Learnt from: gene9831
PR: opentiny/tiny-robot#123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.
packages/components/src/sender/index.type.ts (1)
Learnt from: gene9831
PR: opentiny/tiny-robot#123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.
packages/components/src/sender/index.vue (2)
Learnt from: gene9831
PR: opentiny/tiny-robot#123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
packages/components/src/sender/composables/useKeyboardHandler.ts (1)
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.
packages/components/src/sender/components/TemplateEditor.vue (1)
Learnt from: gene9831
PR: opentiny/tiny-robot#123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.
🧬 Code Graph Analysis (1)
packages/components/src/sender/index.type.ts (1)
packages/components/src/sender/types/editor.type.ts (2)
TextItem(7-9)TemplateItem(11-15)
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/sender.md
136-136: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (23)
packages/svgs/package.json (1)
3-3: Version bump looks good.The version update to 0.2.13 aligns with the coordinated release across all packages.
packages/kit/package.json (1)
3-3: Version bump is consistent with the release.The version update to 0.2.13 maintains consistency across all packages in the monorepo.
packages/components/src/suggestion-pills/index.vue (1)
152-152: Good enhancement to broaden auto-scroll behavior.Removing the
overflowMode === 'scroll'check allows the auto-scroll-on-hover feature to work with all overflow modes, making the component more flexible.packages/components/src/sender/composables/useUndoRedo.ts (1)
1-55: Well-implemented undo/redo composable.The implementation follows standard undo/redo patterns with proper state management. The generic design makes it reusable across different data types, and the optional cleanup callback provides good flexibility for memory management.
packages/components/package.json (1)
3-3: Version bump completes the coordinated release.The version update to 0.2.13 is consistent across all packages and reflects the new features and refactoring included in this release.
packages/components/src/dropdown-menu/index.type.ts (1)
9-9: LGTM! Type definition is correct and follows Vue patterns.The
appendToprop type correctly matches Vue's Teleport component API, supporting both string selectors and HTMLElement references.docs/demos/dropdown-menu/basic.vue (1)
21-26: LGTM! Demo correctly showcases the new appendTo prop.The demo properly demonstrates the usage of
append-to="#app"with a valid CSS selector.packages/components/src/dropdown-menu/index.vue (2)
78-78: LGTM! Prop is correctly passed through to base popper.The
appendToprop is properly passed to the underlying TrBasePopper component, enabling the teleport functionality.
109-109: LGTM! CSS variable addition follows existing patterns.The new
--tr-dropdown-menu-item-font-weightCSS variable provides good customization control and is properly applied to the menu items.Also applies to: 152-152
packages/components/src/base-popper/index.vue (2)
26-26: LGTM! Prop definition matches the type interface.The
appendToprop is correctly typed to match Vue's Teleport component API.
183-183: LGTM! Teleport implementation with proper fallback.The
props.appendTo || teleportTargetpattern correctly provides the new functionality while maintaining backward compatibility.docs/src/releases/update-log.md (2)
4-7: LGTM! Clear documentation of the new appendTo feature.The changelog entry accurately describes the new DropdownMenu
appendToprop functionality.
8-16: LGTM! Well-structured changelog with appropriate breaking change notice.The version 0.2.12 entry properly highlights the breaking change and includes relevant feature additions.
docs/src/components/sender.md (1)
132-143: Documentation updates align well with the code refactoring.The transition from
setTemplatemethod totemplateDataprop is clearly documented, including the recommendedv-model:templateDatasyntax and the structure ofUserItemtypes.Also applies to: 259-259
packages/components/src/sender/components/Block.vue (1)
1-59: Well-structured component for rendering nested template data.The component properly handles recursive rendering of different data types with appropriate styling. The use of
data-*attributes for styling and theasChildprop for flexible rendering are good design choices.docs/src/components/dropdown-menu.md (1)
21-66: Comprehensive documentation updates for DropdownMenu.The addition of the
appendToprop and CSS variables section provides clear guidance for component usage and customization. The formatting improvements enhance readability.packages/components/src/sender/composables/useKeyboardHandler.ts (1)
22-24: Clean integration of template mode handling.The optional parameters maintain backward compatibility while properly handling template mode exit before submission. The implementation aligns well with the overall template refactoring.
Also applies to: 40-41, 74-77
packages/components/src/sender/index.type.ts (2)
2-2: Well-structured type refactoring for template data model.The transition from string-based templates with initial values to a structured
UserItem[]model provides better type safety and clearer data flow. The simplified user-facing types (UserTextItem,UserTemplateItem) appropriately hide internal implementation details.Also applies to: 49-49, 73-73, 124-128
126-126: Intentional exclusion of prefix/suffix from UserTemplateItemThe
UserTemplateItemtype deliberately only exposestypeandcontent. InTemplateEditor.vue,prefixandsuffixare automatically injected using the internalPREFIX/SUFFIXconstants, so they aren’t part of the public API. No changes are required.docs/demos/sender/Template.vue (3)
4-11: LGTM! Correct usage of the new template data binding.The component correctly uses the new
v-model:template-databinding pattern for structured template data, which aligns with the refactored sender component API.
51-121: LGTM! Well-structured template data format.The template data structure is correctly implemented using the
UserItemtype with clear separation between 'text' and 'template' segments. This provides better type safety and structure compared to the previous string-based approach.
124-128: LGTM! Correct usage of the new API method.The
selectTemplatemethod correctly updates the reactive state and uses the newactivateTemplateFirstField()method, which aligns with the refactored sender component API.packages/components/src/sender/components/TemplateEditor.vue (1)
95-131: Add tests for Safari-specific TemplateEditor logicWe couldn’t find any existing tests covering the
isSafariBrowserbranches in TemplateEditor.vue. To prevent regressions, please:• Create or update unit tests for
packages/components/src/sender/components/TemplateEditor.vuethat
– Mocknavigator.userAgentto return Safari vs non-Safari values
– Assert the correct DOM structure is returned in each case (the nestedcontentarrays differ)
• Cover thegetComposedRangescall with both{ shadowRoots: […] }and rawshadowRootparameters
• Add integration or E2E tests (or manual test steps) verifying behavior in:
– Safari on macOS (latest + previous major)
– Safari on iOS/iPadOS
– A non-Safari WebKit browser as a controlFile to update:
• packages/components/src/sender/components/TemplateEditor.vue (lines ~95–131)fix_required
| </span> | ||
| <template v-else> | ||
| <template v-if="props.asChild"> | ||
| <Block v-for="item in props.content as DataItem[]" :key="`${item.id}-${item.type}`" v-bind="item" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type guard before casting content to DataItem[].
The type casting assumes props.content is always an array when type is 'block', but this isn't enforced by TypeScript.
Consider adding a type guard or runtime check:
-<Block v-for="item in props.content as DataItem[]" :key="`${item.id}-${item.type}`" v-bind="item" />
+<Block v-for="item in (Array.isArray(props.content) ? props.content : []) as DataItem[]" :key="`${item.id}-${item.type}`" v-bind="item" />Also applies to: 22-22
🤖 Prompt for AI Agents
In packages/components/src/sender/components/Block.vue at lines 19 and 22, the
code casts props.content to DataItem[] without verifying its type, which can
cause runtime errors. Add a type guard or runtime check to confirm props.content
is an array and its items conform to DataItem before casting. This ensures safe
access and prevents invalid assumptions about the data structure.
| export interface StructuredDataItem { | ||
| id: string | ||
| type: 'block' | 'text' | 'template' | 'prefix' | 'suffix' | ||
| content: string | StructuredDataItem[] | ||
| asChild?: boolean | ||
| readonly?: boolean | ||
| } | ||
|
|
||
| export interface EditorRange extends StaticRange { | ||
| readonly endId?: string | ||
| readonly endType?: string | ||
| readonly startId?: string | ||
| readonly startType?: string | ||
| } | ||
|
|
||
| export interface SelectedItem { | ||
| id: string | ||
| type: ExtendedTextItem['type'] | ||
| startOffset: number | ||
| endOffset: number | ||
| } | ||
|
|
||
| export interface CreateItem { | ||
| tag: 'new' | ||
| afterId?: string | ||
| type: 'text' | ||
| content: string | ||
| } | ||
|
|
||
| export interface DataItem { | ||
| id: string | ||
| type: 'block' | 'text' | 'template' | 'prefix' | 'suffix' | ||
| content: string | DataItem[] | ||
| readonly?: boolean | ||
| asChild?: boolean | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating duplicate interfaces.
The StructuredDataItem and DataItem interfaces are nearly identical, differing only in the order of properties. This duplication could lead to confusion and maintenance issues. Consider consolidating them into a single interface or clearly documenting the intended use case for each.
-export interface StructuredDataItem {
- id: string
- type: 'block' | 'text' | 'template' | 'prefix' | 'suffix'
- content: string | StructuredDataItem[]
- asChild?: boolean
- readonly?: boolean
-}
-
export interface EditorRange extends StaticRange {
readonly endId?: string
readonly endType?: string
readonly startId?: string
readonly startType?: string
}
export interface SelectedItem {
id: string
type: ExtendedTextItem['type']
startOffset: number
endOffset: number
}
export interface CreateItem {
tag: 'new'
afterId?: string
type: 'text'
content: string
}
+// Use a single interface for structured data items
export interface DataItem {
id: string
type: 'block' | 'text' | 'template' | 'prefix' | 'suffix'
content: string | DataItem[]
readonly?: boolean
asChild?: boolean
}
+
+// Type alias for backward compatibility if needed
+export type StructuredDataItem = DataItem📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface StructuredDataItem { | |
| id: string | |
| type: 'block' | 'text' | 'template' | 'prefix' | 'suffix' | |
| content: string | StructuredDataItem[] | |
| asChild?: boolean | |
| readonly?: boolean | |
| } | |
| export interface EditorRange extends StaticRange { | |
| readonly endId?: string | |
| readonly endType?: string | |
| readonly startId?: string | |
| readonly startType?: string | |
| } | |
| export interface SelectedItem { | |
| id: string | |
| type: ExtendedTextItem['type'] | |
| startOffset: number | |
| endOffset: number | |
| } | |
| export interface CreateItem { | |
| tag: 'new' | |
| afterId?: string | |
| type: 'text' | |
| content: string | |
| } | |
| export interface DataItem { | |
| id: string | |
| type: 'block' | 'text' | 'template' | 'prefix' | 'suffix' | |
| content: string | DataItem[] | |
| readonly?: boolean | |
| asChild?: boolean | |
| } | |
| export interface EditorRange extends StaticRange { | |
| readonly endId?: string | |
| readonly endType?: string | |
| readonly startId?: string | |
| readonly startType?: string | |
| } | |
| export interface SelectedItem { | |
| id: string | |
| type: ExtendedTextItem['type'] | |
| startOffset: number | |
| endOffset: number | |
| } | |
| export interface CreateItem { | |
| tag: 'new' | |
| afterId?: string | |
| type: 'text' | |
| content: string | |
| } | |
| // Use a single interface for structured data items | |
| export interface DataItem { | |
| id: string | |
| type: 'block' | 'text' | 'template' | 'prefix' | 'suffix' | |
| content: string | DataItem[] | |
| readonly?: boolean | |
| asChild?: boolean | |
| } | |
| // Type alias for backward compatibility if needed | |
| export type StructuredDataItem = DataItem |
🤖 Prompt for AI Agents
In packages/components/src/sender/types/editor.type.ts between lines 21 and 56,
the interfaces StructuredDataItem and DataItem are nearly identical except for
property order, causing duplication and potential confusion. To fix this,
consolidate these two interfaces into a single interface by merging their
properties and using that unified interface throughout the codebase, or
alternatively add clear documentation explaining the distinct use cases for each
if they must remain separate.
| watch( | ||
| () => props.templateData, | ||
| () => { | ||
| inputValue.value = props.templateData.map((item) => item.content).join('') | ||
| }, | ||
| { deep: true }, | ||
| ) |
There was a problem hiding this comment.
Potential formatting loss when concatenating template data.
The watcher concatenates all content without preserving the structure or adding separators between segments. This could result in loss of formatting or readability, especially when template fields are empty.
Consider preserving the structure or adding appropriate separators:
watch(
() => props.templateData,
() => {
- inputValue.value = props.templateData.map((item) => item.content).join('')
+ // Preserve visual separation between template fields
+ inputValue.value = props.templateData
+ .map((item) => item.type === 'template' && !item.content ? '[...]' : item.content)
+ .join('')
},
{ deep: true },
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| watch( | |
| () => props.templateData, | |
| () => { | |
| inputValue.value = props.templateData.map((item) => item.content).join('') | |
| }, | |
| { deep: true }, | |
| ) | |
| watch( | |
| () => props.templateData, | |
| () => { | |
| // Preserve visual separation between template fields | |
| inputValue.value = props.templateData | |
| .map((item) => | |
| item.type === 'template' && !item.content ? '[...]' : item.content | |
| ) | |
| .join('') | |
| }, | |
| { deep: true }, | |
| ) |
🤖 Prompt for AI Agents
In packages/components/src/sender/index.vue around lines 262 to 268, the watcher
concatenates all templateData content into a single string without separators,
risking loss of formatting or readability. Modify the concatenation to include
appropriate separators (such as spaces or newlines) between items or preserve
the original structure to maintain clarity and formatting when template fields
are empty or varied.
| const PREFIX = '\u200B' | ||
| const SUFFIX = '\u200B' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document the use of zero-width space characters.
The use of U+200B (zero-width space) for PREFIX and SUFFIX markers should be documented to explain why this approach was chosen and any potential implications for text processing or clipboard operations.
+// Zero-width space (U+200B) is used as invisible prefix/suffix markers for template fields
+// This allows visual separation in the editor while remaining invisible to users
+// Note: These characters may affect text processing and should be filtered during input/paste operations
const PREFIX = '\u200B'
const SUFFIX = '\u200B'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const PREFIX = '\u200B' | |
| const SUFFIX = '\u200B' | |
| // Zero-width space (U+200B) is used as invisible prefix/suffix markers for template fields | |
| // This allows visual separation in the editor while remaining invisible to users | |
| // Note: These characters may affect text processing and should be filtered during input/paste operations | |
| const PREFIX = '\u200B' | |
| const SUFFIX = '\u200B' |
🤖 Prompt for AI Agents
In packages/components/src/sender/components/TemplateEditor.vue around lines 38
to 40, add comments explaining that the zero-width space character (U+200B) is
used as PREFIX and SUFFIX markers to avoid visible characters interfering with
text display, and note any effects this may have on text processing or clipboard
operations to clarify the rationale and potential side effects for future
maintainers.
| // 由于 composition 事件导致 dom 结构变化，Vue 无法控制，需要强制重新渲染 | ||
| forceRerender.value++ | ||
| } else { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Investigate alternatives to force rerender.
Using a counter to force re-render suggests potential Vue reactivity issues. While this works, it's a workaround that might have performance implications.
Consider investigating:
- Whether the DOM mutations from composition events can be better integrated with Vue's reactivity
- Using
nextTickor other Vue lifecycle methods - Whether the issue persists in Vue 3's latest versions
🌐 Web query:
Vue 3 best practices for handling DOM mutations from composition events
💡 Result:
The best practices for handling DOM mutations resulting from composition events in Vue 3 are centered on leveraging Vue’s reactivity system and component architecture, minimizing direct DOM manipulation, and using the component event system effectively. Here’s a synthesis of the current guidance:
1. Rely on Vue’s Reactive Data Flow
- Always use reactive state (with
reforreactive) to drive DOM updates. When you mutate reactive state, Vue automatically schedules and batches DOM updates, applying them on the next tick to optimize performance[4][7]. - If you need to perform operations immediately after the DOM has updated due to a state change, use
await nextTick():
import { ref, nextTick } from 'vue'
const count = ref(0)
async function increment() {
count.value++
await nextTick()
// Safe to access the updated DOM here
}[4]
2. Use Component Event Emission for DOM-Driven Communication
- If a child component needs to signal a parent about a composition event (e.g., text input, selection), use the custom event system with
emit:
// In child component
const handleInput = (value) => {
emit('update:modelValue', value)
}[2][5][8]
- In the parent, handle the emitted event to mutate state, letting Vue update the DOM based on this reactivity.
- This maintains one-way data flow and predictable updates—do not mutate props directly inside child components[6].
3. Avoid Direct Manual DOM Manipulation
- Let Vue manage all DOM mutations resulting from state changes and event handling.
- Direct DOM manipulation (using
$refs,document.querySelector, etc.) should be used only as an escape hatch—for example, when integrating with third-party libraries that require direct access. Even then, such access should occur in lifecycle hooks likeonMountedto avoid conflicts with the virtual DOM and to ensure refs are populated[10]. - Manual DOM manipulation can introduce bugs, inconsistencies, and breaks Vue’s reactivity optimizations[10].
4. Use Watchers for Side Effects, Not for DOM Sync
- Use
watchorwatchEffectonly for legitimate side effects, not for synchronizing DOM state with data. The typical pattern is:
watch(someReactiveVar, (newValue) => {
// call APIs, log analytics, etc.
})- Avoid manipulating the DOM inside watchers; rely on Vue’s rendering to do this[9].
5. Reactivity and Updates: Trust the Virtual DOM
- Vue’s Virtual DOM and tree flattening ensure that only dynamic, relevant parts of the DOM are touched on updates, making mutation performance significantly better than direct DOM handling[1][7].
- By keeping all mutations within the reactivity system and responding to composition events only by changing state, you ensure efficient rendering and maintainability.
Summary Table: Best Practices
| Practice | Recommendation |
|---|---|
| State-driven DOM changes | Preferred (use ref, reactive) |
| Emitting events from components | Preferred for parent-child communication |
| Direct DOM manipulation | Avoid unless absolutely necessary |
| Use of watchers | For side effects only, not DOM sync |
| Prop mutation in child | Avoid; use events to request changes |
In short:
Always drive DOM changes through reactive state, respond to composition events using component events and state mutation, and avoid direct DOM manipulation except in rare, controlled cases. Leverage Vue 3’s built-in update mechanisms for best performance and maintainability.[4][5][10]
Citations:
- 1: https://vuejs.org/guide/extras/rendering-mechanism
- 2: https://learnvue.co/articles/vue-event-handling-guide
- 3: https://vuex.vuejs.org/guide/mutations
- 4: https://vuejs.org/guide/essentials/reactivity-fundamentals.html
- 5: https://vuejs.org/guide/components/events.html
- 6: https://vuejs.org/guide/components/props
- 7: https://vuejs.org/guide/extras/reactivity-in-depth
- 8: https://www.freecodecamp.org/news/how-event-handling-works-in-vue-3-guide-for-devs/
- 9: https://vuejs.org/guide/essentials/watchers.html
- 10: https://www.danvega.dev/blog/vue-tips-avoid-direct-dom-manipulation
Replace manual forceRerender hack with Vue’s reactive updates and nextTick
The counter‐increment workaround bypasses Vue’s reactivity and can lead to performance and maintenance issues. Instead, drive all DOM changes through reactive state and lifecycle hooks:
• In TemplateEditor.vue (around lines 701–703):
- Remove
forceRerender.value++and the accompanying comment. - Introduce a
reforreactiveproperty to represent the composition event’s output. - In your composition event handler, update that reactive state.
- If you need to act on the DOM after it updates, call:
import { nextTick } from 'vue' // after updating your state: await nextTick() // safe to interact with updated DOM here
• Use component events (emit) to bubble changes up, rather than manual DOM reads/writes.
• Reserve direct DOM access (e.g., via $refs) for integration with third-party libraries, and perform it inside onMounted or after nextTick().
By trusting Vue’s virtual DOM and reactivity system, you’ll ensure efficient, predictable updates and avoid side-effect workarounds.
🤖 Prompt for AI Agents
In packages/components/src/sender/components/TemplateEditor.vue around lines 701
to 703, replace the manual forceRerender.value++ hack with Vue's reactive state
management. Remove the forceRerender increment and its comment, create a
reactive ref or reactive property to hold the composition event output, and
update this reactive state inside the composition event handler. If you need to
perform actions after the DOM updates, use Vue's nextTick() to wait for the DOM
update before interacting with it. Also, use component emits to propagate
changes instead of manual DOM manipulation, and limit direct DOM access to
lifecycle hooks or after nextTick().
3 participants
Summary by CodeRabbit
New Features
appendToproperty to control where the menu is mounted in the DOM.Blockcomponent for structured content rendering.Bug Fixes
Documentation
Refactor
Chores