feat(tag-input): tag-input component #3951
Conversation
WalkthroughNew TagInput component introduced across the monorepo, including renderless logic with drag-and-drop tag management, Vue component implementation with template, comprehensive type definitions, LESS styling with theme variables, menu and module registry updates, and demo components with bilingual documentation. Changes
Sequence DiagramsequenceDiagram
participant User
participant Template
participant Renderless
participant State
participant Vue
rect rgb(200, 220, 255)
Note over User,Vue: Adding a Tag
User->>Template: enters text & presses enter
Template->>Renderless: addTag()
Renderless->>State: validate & update tagList
State->>Vue: reactive state change
Vue->>Template: re-render with new tags
Template->>User: display updated tags
end
rect rgb(220, 200, 255)
Note over User,Vue: Drag & Drop Reordering
User->>Template: drag tag from index A
Template->>Renderless: handleDragStart(A)
Renderless->>State: set draggingIndex = A
User->>Template: drop tag on index B
Template->>Renderless: handleDrop(B)
Renderless->>State: reorder tagList[A] ↔ tagList[B]
State->>Vue: emit modelValue update
Vue->>Template: re-render reordered list
Template->>User: display new order
end
rect rgb(200, 255, 220)
Note over User,Vue: Removing Tag
User->>Template: click close icon on tag[i]
Template->>Renderless: removeTag(i)
Renderless->>State: splice tagList at index i
State->>Vue: emit updated modelValue
Vue->>Template: re-render without tag
Template->>User: display updated list
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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.
Actionable comments posted: 14
🧹 Nitpick comments (9)
examples/sites/demos/pc/app/tag-input/webdoc/tag-input.cn.md (1)
1-7: Consider expanding the documentation.The documentation is quite minimal. While this is acceptable as initial documentation, consider adding:
- Usage examples
- Component features
- API reference or link to API docs
- Common use cases
packages/theme/src/tag-input/index.less (2)
9-9: Replace hard-coded border color with a CSS variable.The border color
#cccis hard-coded. For consistency with the theming system and to support theme customization, use a CSS variable instead.🔎 Suggested refactor
-.@{tag-input-prefix-cls} { - .inject-TagInput-vars(); - border: 1px solid #ccc; +.@{tag-input-prefix-cls} { + .inject-TagInput-vars(); + border: 1px solid var(--tv-TagInput-border-color, #ccc);Then add the corresponding variable in
vars.less:--tv-TagInput-border-color: var(--tv-color-border, #ccc);
48-53: Replace hard-coded values with CSS variables.Several hard-coded values (
height: 32px,line-height: 32px,font-size: 14px) should be replaced with CSS variables to maintain consistency with the theming system and allow easier customization.🔎 Suggested refactor
.@{tag-input-prefix-cls}-inner { border: none; outline: none; - height: 32px; - line-height: 32px; + height: var(--tv-TagInput-inner-height, 32px); + line-height: var(--tv-TagInput-inner-line-height, 32px); padding: 0 var(--tv-TagInput-padding); - font-size: 14px; + font-size: var(--tv-TagInput-inner-font-size, 14px); width: auto; }Then add the corresponding variables in
vars.less:--tv-TagInput-inner-height: 32px; --tv-TagInput-inner-line-height: 32px; --tv-TagInput-inner-font-size: var(--tv-font-size-base, 14px);packages/theme/src/tag-input/vars.less (1)
35-35: Maintain consistent comment formatting.The comment is missing a space after
//. For consistency with other comments in the file, add a space.🔎 Suggested fix
- //标签输入框圆角 + // 标签输入框圆角 --tv-TagInput-border-radius: var(--tv-border-radius-sm, 4px);examples/sites/demos/pc/app/tag-input/basic-usage-composition-api.vue (1)
11-13: Consider consolidating duplicate array definitions.The three reactive arrays have identical initial values. While this works correctly for demonstrating different tag types, consider either:
- Using a shared initial array and spreading it
- Adding a comment explaining why separate arrays are needed
🔎 Possible refactor
+const initialTags = ['tag1', 'tag2', 'tag3', 'tag4', 'tag5', 'tag6'] -const data = reactive(['tag1', 'tag2', 'tag3', 'tag4', 'tag5', 'tag6']) -const data1 = reactive(['tag1', 'tag2', 'tag3', 'tag4', 'tag5', 'tag6']) -const data2 = reactive(['tag1', 'tag2', 'tag3', 'tag4', 'tag5', 'tag6']) +const data = reactive([...initialTags]) +const data1 = reactive([...initialTags]) +const data2 = reactive([...initialTags]) const placeholder = 'please enter a tag'packages/vue/src/tag-input/package.json (1)
24-24: Remove or uncomment the postversion script.The postversion script uses an unusual comment syntax (
"//postversion":). Either:
- Remove this line if the script is not needed
- Change to
"postversion":if it should be activeThis non-standard syntax may cause confusion and doesn't follow JSON commenting conventions.
🔎 Suggested fix
If the script should be active:
- "//postversion": "pnpm build" + "postversion": "pnpm build"If not needed:
- "build": "pnpm -w build:ui $npm_package_name", - "//postversion": "pnpm build" + "build": "pnpm -w build:ui $npm_package_name"examples/sites/demos/pc/app/tag-input/basic-usage.vue (1)
21-21: Remove unused variable.The
maxTagCountvariable is declared but never used in the template or script.🔎 Proposed fix
- maxTagCount: 3, placeholder: 'please enter a tag'packages/renderless/src/tag-input/index.ts (2)
45-49: Unused parameter:eventis not used.The
eventparameter is declared but never used in the function body.🔎 Proposed fix
export const handleMouseOver = ({ state }: Pick<ITagRenderlessParams, 'state'>) => - (event: MouseEvent) => { + () => { state.isHovering = true }
85-91: Unused parameter:emitis not used.The
emitparameter is declared in the Pick type on line 86 but never used in the function body.🔎 Proposed fix
export const handleDragEnter = - ({ state, emit }: Pick<ITagRenderlessParams, 'state' | 'emit'>) => + ({ state }: Pick<ITagRenderlessParams, 'state'>) => (index: number, event: DragEvent) => { event.preventDefault() if (index === state.draggingIndex) return state.dragTargetIndex = index }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
examples/sites/demos/apis/tag-input.jsexamples/sites/demos/pc/app/tag-input/basic-usage-composition-api.vueexamples/sites/demos/pc/app/tag-input/basic-usage.vueexamples/sites/demos/pc/app/tag-input/webdoc/tag-input.cn.mdexamples/sites/demos/pc/app/tag-input/webdoc/tag-input.en.mdexamples/sites/demos/pc/app/tag-input/webdoc/tag-input.jsexamples/sites/demos/pc/menus.jspackages/modules.jsonpackages/renderless/src/tag-input/index.tspackages/renderless/src/tag-input/vue.tspackages/theme/src/tag-input/index.lesspackages/theme/src/tag-input/vars.lesspackages/vue/index.tspackages/vue/package.jsonpackages/vue/src/tag-input/index.tspackages/vue/src/tag-input/package.jsonpackages/vue/src/tag-input/src/index.tspackages/vue/src/tag-input/src/mobile-first.vuepackages/vue/src/tag-input/src/pc.vue
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-04T09:35:13.159Z
Learnt from: zzcr
Repo: opentiny/tiny-vue PR: 2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中，应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值，以保持一致性。
Applied to files:
packages/theme/src/tag-input/vars.less
🧬 Code graph analysis (1)
packages/renderless/src/tag-input/vue.ts (2)
packages/vue-common/src/index.ts (1)
props(55-73)packages/renderless/src/tag-input/index.ts (12)
addTag(1-18)removeTag(20-26)handleBackspace(28-36)handleClear(38-43)handleMouseLeave(51-55)handleMouseOver(45-49)handleInputBlur(63-67)handleInputFocus(57-61)handleDragStart(69-78)handleDragOver(80-83)handleDragEnter(85-91)handleDrop(93-104)
🔇 Additional comments (7)
packages/vue/package.json (1)
200-200: LGTM!The workspace dependency is correctly added in alphabetical order and follows the existing pattern.
packages/vue/src/tag-input/package.json (1)
3-3: Verify the version alignment.The tag-input package version is
3.27.0, while the main Vue package is at3.28.0. Please confirm whether this version difference is intentional or if it should be updated to match.examples/sites/demos/pc/menus.js (1)
178-178: LGTM!The TagInput menu entry is properly placed within the Form components group and follows the existing format consistently.
packages/vue/index.ts (1)
166-166: LGTM!The TagInput component integration follows the established pattern correctly: import, components array registration, and dual export (named + Tiny-prefixed alias).
Also applies to: 368-368, 754-755, 1155-1156
examples/sites/demos/pc/app/tag-input/webdoc/tag-input.js (1)
1-20: LGTM!The webdoc configuration is properly structured with appropriate metadata and demo entries.
examples/sites/demos/apis/tag-input.js (1)
1-182: LGTM!The API documentation descriptor is comprehensive and properly structured, covering all component props and slots.
packages/modules.json (1)
2197-2215: LGTM!The module manifest entries for TagInput, TagInputPc, and TagInputMobileFirst follow the established pattern and are correctly structured.
| @dragstart="handleDragStart(index, $event)" | ||
| @dragover="handleDragOver(index, $event)" | ||
| @dragenter="handleDragEnter(index, $event)" | ||
| @dragend="handleDrop(index, $event)" |
There was a problem hiding this comment.
Use @drop instead of @dragend for drop target.
Line 22 uses @dragend which fires on the dragged element when dragging ends, not when the element is dropped on a target. For drag-and-drop reordering, you need @drop to fire on the drop target.
🔎 Proposed fix
- @dragend="handleDrop(index, $event)"
+ @drop="handleDrop(index, $event)"📝 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.
| @dragend="handleDrop(index, $event)" | |
| @drop="handleDrop(index, $event)" |
🤖 Prompt for AI Agents
In packages/vue/src/tag-input/src/mobile-first.vue around line 22, replace the
@dragend binding on the drop target with @drop so the handler runs when an item
is dropped on that element; keep the handler call as handleDrop(index, $event)
and ensure you also have a dragover listener that prevents default (e.g.,
@dragover.prevent or event.preventDefault()) so the drop event will be
delivered.
| <tiny-tooltip v-if="state.collpasedTagList.length" placement="top"> | ||
| <tiny-tag :size="size" :type="tagType" :effect="tagEffect">+{{ state.collpasedTagList.length }}</tiny-tag> | ||
| <template #content> | ||
| <div class="tiny-tag-input-collapsed-tags"> | ||
| <tiny-tag | ||
| v-for="(tag, index) in state.collpasedTagList" | ||
| :key="index" | ||
| :size="size" | ||
| :type="tagType" | ||
| :effect="tagEffect" | ||
| :closable="state.closeable" | ||
| @close="removeTag(state.showTagList.length + index)" | ||
| >{{ tag }}</tiny-tag | ||
| > | ||
| </div> | ||
| </template> | ||
| </tiny-tooltip> |
There was a problem hiding this comment.
Fix typo: collpasedTagList should be collapsedTagList.
The property name is misspelled throughout this section. This typo originates from the renderless vue.ts file and must be fixed in both locations.
🔎 Proposed fix
- <tiny-tooltip v-if="state.collpasedTagList.length" placement="top">
+ <tiny-tooltip v-if="state.collapsedTagList.length" placement="top">
- <tiny-tag :size="size" :type="tagType" :effect="tagEffect">+{{ state.collpasedTagList.length }}</tiny-tag>
+ <tiny-tag :size="size" :type="tagType" :effect="tagEffect">+{{ state.collapsedTagList.length }}</tiny-tag>
<template #content>
<div class="tiny-tag-input-collapsed-tags">
<tiny-tag
- v-for="(tag, index) in state.collpasedTagList"
+ v-for="(tag, index) in state.collapsedTagList"
:key="index"
:size="size"
:type="tagType"📝 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.
| <tiny-tooltip v-if="state.collpasedTagList.length" placement="top"> | |
| <tiny-tag :size="size" :type="tagType" :effect="tagEffect">+{{ state.collpasedTagList.length }}</tiny-tag> | |
| <template #content> | |
| <div class="tiny-tag-input-collapsed-tags"> | |
| <tiny-tag | |
| v-for="(tag, index) in state.collpasedTagList" | |
| :key="index" | |
| :size="size" | |
| :type="tagType" | |
| :effect="tagEffect" | |
| :closable="state.closeable" | |
| @close="removeTag(state.showTagList.length + index)" | |
| >{{ tag }}</tiny-tag | |
| > | |
| </div> | |
| </template> | |
| </tiny-tooltip> | |
| <tiny-tooltip v-if="state.collapsedTagList.length" placement="top"> | |
| <tiny-tag :size="size" :type="tagType" :effect="tagEffect">+{{ state.collapsedTagList.length }}</tiny-tag> | |
| <template #content> | |
| <div class="tiny-tag-input-collapsed-tags"> | |
| <tiny-tag | |
| v-for="(tag, index) in state.collapsedTagList" | |
| :key="index" | |
| :size="size" | |
| :type="tagType" | |
| :effect="tagEffect" | |
| :closable="state.closeable" | |
| @close="removeTag(state.showTagList.length + index)" | |
| >{{ tag }}</tiny-tag | |
| > | |
| </div> | |
| </template> | |
| </tiny-tooltip> |
🤖 Prompt for AI Agents
In packages/vue/src/tag-input/src/mobile-first.vue around lines 25 to 41 the
property name is misspelled as "collpasedTagList"; update all occurrences in
this file to "collapsedTagList" (v-if, v-for source, and any other
bindings/handlers) and then fix the corresponding export/definition in the
renderless vue.ts so both locations use the exact same "collapsedTagList"
identifier; ensure keys, event handlers (e.g., removeTag index calculation) and
any tests or imports that reference the old name are updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/vue/src/tag-input/src/pc.vue (2)
42-52: Consider adding a ref for focus management.The input element lacks a
refattribute. If programmatic focus management is needed (e.g., re-focusing after tag removal or clearing), a ref would be beneficial.🔎 Suggested addition
<input + ref="inputRef" v-if="!readonly" class="tiny-tag-input-inner" v-model="state.currentValue"Then expose the ref in the renderless state or API if external focus control is required.
78-92: Consider using typed prop definitions for better type safety.The props are defined as a string array, which is valid but provides no TypeScript type checking or IDE autocomplete. If type safety is a priority, consider using Vue's
PropTypeto define each prop with its expected type, default value, and required status.Example for a few props:
props: { ...props, size: { type: String as PropType<'small' | 'medium' | 'large'>, default: 'medium' }, modelValue: { type: Array as PropType<string[]>, default: () => [] }, max: { type: Number, default: Infinity }, // ... other props }Note: Only apply this if it aligns with the project's conventions.
packages/renderless/src/tag-input/vue.ts (1)
36-36: Remove unused parameter.The
parentparameter is destructured from the utils object but never used in the function body. Consider removing it to keep the signature clean.🔎 Proposed fix
{ reactive, computed, ref }: ISharedRenderlessParamHooks, - { emit, parent }: ISharedRenderlessParamUtils<never> + { emit }: ISharedRenderlessParamUtils<never> ): ITagApi => {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/renderless/src/tag-input/index.tspackages/renderless/src/tag-input/vue.tspackages/theme/src/tag-input/index.lesspackages/vue/src/tag-input/src/index.tspackages/vue/src/tag-input/src/mobile-first.vuepackages/vue/src/tag-input/src/pc.vue
✅ Files skipped from review due to trivial changes (1)
- packages/theme/src/tag-input/index.less
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/renderless/src/tag-input/index.ts
- packages/vue/src/tag-input/src/mobile-first.vue
- packages/vue/src/tag-input/src/index.ts
🔇 Additional comments (3)
packages/vue/src/tag-input/src/pc.vue (1)
25-41: Typo fix confirmed.The spelling of
collapsedTagListis now correct throughout this section (lines 25, 26, 30). This addresses the past review comment about thecollpasedTagListtypo.packages/renderless/src/tag-input/vue.ts (2)
55-58: Typo fix confirmed.The
collapsedTagListcomputed property now uses the correct spelling. This addresses the past review comment about thecollpasedTagListtypo.
78-78: No issue found.handleDragOver()correctly takes no parameters because its function signature is() => (index: number, event: DragEvent) => { ... }. It's a factory function that returns a handler, and the returned handler does callevent.preventDefault(). This is the same curried pattern used by all other handlers in the object.Likely an incorrect or invalid review comment.
| @@ -0,0 +1,84 @@ | |||
| import type { ISharedRenderlessParamHooks } from '@/types' | |||
There was a problem hiding this comment.
Add missing type imports.
Lines 36-38 reference ISharedRenderlessParamUtils, ITagApi, and ITagState, but these types are not imported. This will cause TypeScript compilation errors.
🔎 Proposed fix
-import type { ISharedRenderlessParamHooks } from '@/types'
+import type { ISharedRenderlessParamHooks, ISharedRenderlessParamUtils, ITagApi, ITagState } from '@/types'Alternatively, if these types are defined elsewhere, import them from the appropriate module.
📝 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.
| import type { ISharedRenderlessParamHooks } from '@/types' | |
| import type { ISharedRenderlessParamHooks, ISharedRenderlessParamUtils, ITagApi, ITagState } from '@/types' |
🤖 Prompt for AI Agents
In packages/renderless/src/tag-input/vue.ts at around lines 1 and 36-38, the
types ISharedRenderlessParamUtils, ITagApi, and ITagState are referenced but not
imported; add missing type imports (e.g. import type {
ISharedRenderlessParamUtils, ITagApi, ITagState } from '@/types' or from the
correct module where those interfaces are defined) so TypeScript can resolve
them, and ensure imports use "import type" to avoid runtime bundling.
| :draggable="draggable" | ||
| @close="removeTag(index)" | ||
| @dragstart="handleDragStart(index, $event)" | ||
| @dragover="handleDragOver(index, $event)" | ||
| @dragenter="handleDragEnter(index, $event)" | ||
| @dragend="handleDrop(index, $event)" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the handleDrop function in the pc.vue file
rg -A 10 "handleDrop" packages/vue/src/tag-input/src/pc.vueRepository: opentiny/tiny-vue
Length of output: 550
🏁 Script executed:
#!/bin/bash
# Also check the full script section to see all handler implementations
sed -n '/<script/,/<\/script>/p' packages/vue/src/tag-input/src/pc.vue | head -200Repository: opentiny/tiny-vue
Length of output: 896
🏁 Script executed:
#!/bin/bash
# Search for the renderless tag-input implementation
find . -path "*/renderless/tag-input*" -type f | head -20Repository: opentiny/tiny-vue
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for handleDrop function across the codebase
rg -l "handleDrop" --type ts --type jsRepository: opentiny/tiny-vue
Length of output: 1092
🏁 Script executed:
#!/bin/bash
# Search for handleDrop in the renderless tag-input implementation
rg -A 15 "handleDrop" packages/renderless/src/tag-input/vue.tsRepository: opentiny/tiny-vue
Length of output: 874
🏁 Script executed:
#!/bin/bash
# Also look for handleDragStart and handleDragOver to understand the full drag-drop context
rg -B 2 -A 10 "handleDrag(Start|Over|Enter)" packages/renderless/src/tag-input/vue.tsRepository: opentiny/tiny-vue
Length of output: 988
🏁 Script executed:
#!/bin/bash
# Find the handleDrop implementation in the index file
rg -B 5 -A 20 "export.*handleDrop\|const handleDrop" packages/renderless/src/tag-input/index.tsRepository: opentiny/tiny-vue
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Let's read the index.ts file to find the handleDrop implementation
cat packages/renderless/src/tag-input/index.tsRepository: opentiny/tiny-vue
Length of output: 3168
Bind handleDrop to @drop instead of @dragend.
The dragend event fires on the dragged element when a drag operation ends, regardless of whether a drop occurred, and cannot be prevented. The handleDrop function calls event.preventDefault() and performs reordering based on drop target state—both operations appropriate for the drop event, which fires on valid drop targets. Using @dragend causes the drop logic to execute unconditionally on drag end rather than only on actual drops, and may execute with stale dragTargetIndex state. Bind this handler to @drop and ensure the handler is on the drop target element.
🤖 Prompt for AI Agents
In packages/vue/src/tag-input/src/pc.vue around lines 17 to 22, the tag reorder
handler is incorrectly bound to @dragend; change the binding to @drop on the
drop target element (replace @dragend="handleDrop(...)" with
@drop="handleDrop(...)"), ensure the drop target also has dragover handling that
calls event.preventDefault() so drops are allowed, keep any dragend listener
only for cleanup (not for performing the drop/reorder), and verify handleDrop
expects the drop event and uses the current drag target index when called from
the drop handler.
| }, | ||
| data() { | ||
| return { | ||
| data: ['tag1', 'tag2', 'tag3', 'tag4', 'tag5', 'tag6'], |
There was a problem hiding this comment.
问题： tagInput的数据格式，是否需要支持对象，比如 ： {label: 'tag1', value: 't1" }
原因： select中的多选是使用到tagInput组件的， 它需要上面的格式。
以往在其它系统中，看到给数据行打tag, tag的数据体通常是一处复杂对象，类似于：
{id , text, color, createTime, ......} 这种
所以tagINput的data 不能是简单的 string[]
There was a problem hiding this comment.
select多选组件应该无法使用这个组件。
这个是一个用户通过输出添加标签的功能，只能新增或者删除操作，操作的只能是文本，除了text外，用户并不能操作其他属性，添加的也没有其他属性。
string[]类型应该是绝大部分场景了，其他类型得考虑下使用场景
| defaultValue: "'light'", | ||
| desc: { | ||
| 'zh-CN': '主题', | ||
| 'en-US': 'Subject' |
| mfDemo: '' | ||
| }, | ||
| { | ||
| name: 'max', |
There was a problem hiding this comment.
AUI6曾使用 maxTagCount 来表示要展示的最大数。---- 建议只使用这个
max如果是限制 data的数组长度，完全没有必要。
minCollapsedNum： 这个属性名字也不好理解哪个值最小， 显示的最小还是隐藏起来的tag数最小。
max, minCollapsedNum 建议都去掉
There was a problem hiding this comment.
我认为还是有必要的
max：限制用户输入添加的标签数量的
minCollapsedNum：表示标签数量超过minCollapsedNum，就折叠展示后面的标签
| return setup({ props, context, renderless, api }) | ||
| } | ||
| }) | ||
| </script> |
There was a problem hiding this comment.
我看 mobile-first 模板并未实现，可以先去掉这个文件以及其他 mobile-first 相关代码
|
@zhaoxiaofeng876 已检视，主要有以下问题：
|
@zhaoxiaofeng876 has reviewed and the main problems are as follows:
|
examples/sites/demos/pc/menus.js
Outdated
| { 'nameCn': '输入框', 'name': 'Input', 'key': 'input' }, | ||
| { 'nameCn': ' IP地址输入框', 'name': 'IpAddress', 'key': 'ip-address' }, | ||
| { 'nameCn': '数字输入框', 'name': 'Numeric', 'key': 'numeric' }, | ||
| { 'nameCn': '标签输入框', 'name': 'TagInput', 'key': 'tag-input' }, |
There was a problem hiding this comment.
添加组件 meta 信息。
| { 'nameCn': '标签输入框', 'name': 'TagInput', 'key': 'tag-input' }, | |
| { | |
| 'nameCn': '标签输入框', 'name': 'TagInput', 'key': 'tag-input', | |
| 'meta': { | |
| 'experimental': '3.29.0' | |
| } | |
| }, |
| @@ -0,0 +1,20 @@ | |||
| export default { | |||
| column: '2', | |||
| owner: '', | |||
There was a problem hiding this comment.
| owner: '', | |
| owner: '', | |
| meta: { | |
| experimental: '3.29.0' | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/renderless/src/tag-input/index.ts (2)
86-89: Major: Unsafe non-null assertion on dataTransfer.Line 88 uses a non-null assertion (
!) onevent.dataTransfer, which can be null in some browsers or drag scenarios. This will cause a runtime error ifdataTransferis null.🔎 Proposed fix
export const handleDragOver = () => (index: number, event: DragEvent) => { event.preventDefault() - event.dataTransfer!.dropEffect = 'move' + if (event.dataTransfer) { + event.dataTransfer.dropEffect = 'move' + } }
76-84: Critical: Incorrect dataTransfer usage and unsafe null assertion.Three critical issues:
Line 81:
setDatareceivesevent.target(a DOM element) instead of a string. The DataTransfer API expects a string identifier, but passing a DOM element will serialize it to"[object HTMLElement]", breaking drag-and-drop functionality.Line 82: Non-null assertion (
!) after optional chaining on line 81. IfdataTransferis null, line 82 will throw a runtime error.Line 80: Checking
event.targetwhen the guard should be onevent.dataTransfer.🔎 Proposed fix
export const handleDragStart = ({ state }: Pick<ITagInputRenderlessParams, 'state'>) => (index: number, event: DragEvent) => { state.draggingIndex = index - if (event.target) { - event.dataTransfer?.setData('text/plain', event.target) - event.dataTransfer!.effectAllowed = 'move' + if (event.dataTransfer) { + event.dataTransfer.setData('text/plain', String(index)) + event.dataTransfer.effectAllowed = 'move' } }
🧹 Nitpick comments (3)
examples/sites/demos/apis/tag-input.js (1)
155-156: Consider documenting emitted events.The
eventsandmethodsarrays are empty, but the TagInput component emitsupdate:modelValuefor v-model binding. Consider documenting this event for API completeness.🔎 Suggested addition
- events: [], + events: [ + { + name: 'update:modelValue', + type: '(value: string[]) => void', + defaultValue: '', + desc: { + 'zh-CN': '标签列表变化时触发', + 'en-US': 'Triggered when tag list changes' + }, + mode: ['pc'], + pcDemo: 'basic-usage' + } + ], methods: [],packages/renderless/src/tag-input/index.ts (2)
28-33: Consider using immutable array update pattern.The function directly mutates
state.tagListwithsplice(). While this works since it's reactive state, creating a new array is better practice for predictable reactivity and easier debugging.🔎 Proposed refactor
export const removeTag = ({ emit, props, state }: Pick<ITagInputRenderlessParams, 'emit' | 'props' | 'state'>) => (index: number) => { - state.tagList.splice(index, 1) - emit('update:modelValue', state.tagList) + const newTagList = [...state.tagList] + newTagList.splice(index, 1) + state.tagList = newTagList + emit('update:modelValue', newTagList) }
35-42: Consider using immutable array update pattern.Similar to
removeTag, this function directly mutatesstate.tagListwithpop(). Using an immutable pattern is clearer and more maintainable.🔎 Proposed refactor
export const handleBackspace = ({ emit, props, state }: Pick<ITagInputRenderlessParams, 'emit' | 'props' | 'state'>) => () => { if (state.currentValue === '') { - state.tagList.pop() - emit('update:modelValue', state.tagList) + const newTagList = state.tagList.slice(0, -1) + state.tagList = newTagList + emit('update:modelValue', newTagList) } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
examples/sites/demos/apis/tag-input.jsexamples/sites/demos/pc/app/tag-input/basic-usage-composition-api.vueexamples/sites/demos/pc/app/tag-input/webdoc/tag-input.cn.mdexamples/sites/demos/pc/app/tag-input/webdoc/tag-input.en.mdexamples/sites/demos/pc/app/tag-input/webdoc/tag-input.jsexamples/sites/demos/pc/menus.jspackages/renderless/src/tag-input/index.tspackages/renderless/src/tag-input/vue.tspackages/renderless/types/index.tspackages/renderless/types/tag-input.type.tspackages/theme/src/index.lesspackages/theme/src/tag-input/index.lesspackages/theme/src/tag-input/vars.lesspackages/vue/src/tag-input/index.tspackages/vue/src/tag-input/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- examples/sites/demos/pc/app/tag-input/webdoc/tag-input.en.md
- examples/sites/demos/pc/app/tag-input/basic-usage-composition-api.vue
- examples/sites/demos/pc/app/tag-input/webdoc/tag-input.js
- packages/theme/src/tag-input/index.less
- examples/sites/demos/pc/app/tag-input/webdoc/tag-input.cn.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-04T09:35:13.159Z
Learnt from: zzcr
Repo: opentiny/tiny-vue PR: 2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中，应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值，以保持一致性。
Applied to files:
packages/theme/src/tag-input/vars.less
🧬 Code graph analysis (5)
packages/renderless/types/tag-input.type.ts (3)
packages/renderless/src/tag-input/index.ts (4)
addTag(3-26)removeTag(28-33)handleBackspace(35-42)handleClear(44-50)packages/vue/src/tag-input/src/index.ts (1)
tagInputProps(15-68)packages/vue-hooks/types/shared.type.ts (1)
ISharedRenderlessFunctionParams(1-15)
packages/renderless/src/tag-input/index.ts (2)
packages/renderless/types/tag-input.type.ts (1)
ITagInputRenderlessParams(51-55)packages/vue-common/src/index.ts (1)
props(55-73)
packages/renderless/src/tag-input/vue.ts (2)
packages/renderless/types/tag-input.type.ts (2)
ITagInputApi(33-47)ITagInputState(19-31)packages/renderless/src/tag-input/index.ts (12)
addTag(3-26)removeTag(28-33)handleBackspace(35-42)handleClear(44-50)handleMouseLeave(58-62)handleMouseOver(52-56)handleInputBlur(70-74)handleInputFocus(64-68)handleDragStart(76-84)handleDragOver(86-89)handleDragEnter(91-97)handleDrop(99-111)
packages/vue/src/tag-input/index.ts (1)
packages/vue/index.ts (3)
TagInput(754-754)TagInput(755-755)version(421-421)
packages/vue/src/tag-input/src/index.ts (1)
packages/vue-common/src/index.ts (4)
$props(45-53)$prefix(43-43)props(55-73)$setup(199-233)
🔇 Additional comments (9)
packages/theme/src/index.less (1)
137-137: LGTM!The tag-input style import is correctly placed in alphabetical order within the theme index file.
packages/theme/src/tag-input/vars.less (1)
13-41: LGTM!The theming variables mixin follows the established pattern with comprehensive coverage for layout, spacing, disabled states, and color properties. All CSS custom properties have appropriate fallback values.
packages/vue/src/tag-input/index.ts (1)
17-19: LGTM!The install function correctly registers
TagInputusing its own name. The previous issue with incorrect component registration has been addressed.packages/renderless/types/index.ts (1)
183-183: LGTM!The tag-input type export is correctly placed in alphabetical order within the types barrel file.
examples/sites/demos/pc/menus.js (1)
178-178: LGTM!The TagInput menu entry is correctly added with the required
meta.experimentalinformation marking it as experimental in version 3.29.0. The placement in the Form section is appropriate for an input component.packages/vue/src/tag-input/src/index.ts (1)
70-76: LGTM!The component definition correctly uses the
$setuppattern and follows the established conventions. The previousdrageabletypo has been removed, leaving only the correctdraggableprop.packages/renderless/types/tag-input.type.ts (2)
19-31: LGTM!The
ITagInputStateinterface comprehensively covers all necessary state fields including input value, tag list, UI flags, and drag-drop indices.
33-47: LGTM!The
ITagInputApiinterface correctly usesReturnType<typeof handler>pattern to infer types from the handler implementations, ensuring type consistency between the API surface and the actual handler functions.examples/sites/demos/apis/tag-input.js (1)
1-3: Remove mobile-first mode references per reviewer feedback.The reviewer specifically requested that mobile-first template code be removed and only the PC template be implemented. Update the
modearray to only include'pc'.🔎 Proposed fix
export default { - mode: ['pc', 'mobile-first'], + mode: ['pc'], apis: [Likely an incorrect or invalid review comment.
| export const handleDrop = | ||
| ({ emit, props, state }: Pick<ITagInputRenderlessParams, 'emit' | 'props' | 'state'>) => | ||
| (index: number, event: DragEvent) => { | ||
| event.preventDefault() | ||
| const newTags = [...(state.tagList ?? [])] | ||
| const draggingTag = newTags[state.draggingIndex] | ||
| newTags.splice(state.draggingIndex, 1) | ||
| newTags.splice(state.dragTargetIndex, 0, draggingTag) | ||
| state.draggingIndex = null | ||
| state.dragTargetIndex = null | ||
| state.tagList = newTags | ||
| emit('update:modelValue', newTags) | ||
| } |
There was a problem hiding this comment.
Major: Missing null and bounds checks for drag indices.
Lines 104-106 use state.draggingIndex and state.dragTargetIndex without verifying they are valid:
- If either index is
null(the initial state per vue.ts line 61-62),newTags[null]returnsundefinedandsplice(null, ...)behaves unexpectedly. - No bounds checking ensures indices are within the array range.
This can corrupt the tag list if a drop event fires without a proper drag sequence or due to a race condition.
🔎 Proposed fix
export const handleDrop =
({ emit, props, state }: Pick<ITagInputRenderlessParams, 'emit' | 'props' | 'state'>) =>
(index: number, event: DragEvent) => {
event.preventDefault()
+
+ // Guard against invalid drag state
+ if (state.draggingIndex === null || state.dragTargetIndex === null) {
+ return
+ }
+
const newTags = [...(state.tagList ?? [])]
+
+ // Guard against out-of-bounds indices
+ if (state.draggingIndex < 0 || state.draggingIndex >= newTags.length ||
+ state.dragTargetIndex < 0 || state.dragTargetIndex >= newTags.length) {
+ state.draggingIndex = null
+ state.dragTargetIndex = null
+ return
+ }
+
const draggingTag = newTags[state.draggingIndex]
newTags.splice(state.draggingIndex, 1)
newTags.splice(state.dragTargetIndex, 0, draggingTag)
state.draggingIndex = null
state.dragTargetIndex = null
state.tagList = newTags
emit('update:modelValue', newTags)
}🤖 Prompt for AI Agents
In packages/renderless/src/tag-input/index.ts around lines 99 to 111, the drop
handler uses state.draggingIndex and state.dragTargetIndex directly which can be
null or out-of-bounds; add defensive checks: after event.preventDefault(),
validate that both indices are non-null, are finite integers, and lie within
0..(newTags.length-1); if any check fails, reset state.draggingIndex and
state.dragTargetIndex to null and return early without modifying tagList or
emitting; only when indices are valid perform the splice reordering, update
state.tagList, reset the indices, and emit('update:modelValue', newTags).
| collapsedTagList: computed(() => { | ||
| return props.minCollapsedNum < props.max ? (state.tagList || []).slice(props.minCollapsedNum) : [] | ||
| }), |
There was a problem hiding this comment.
Critical: Incorrect collapsedTagList logic causes tags to be lost.
The collapsedTagList computation is inconsistent with showTagList. When minCollapsedNum >= max, this returns an empty array even though there are tags beyond the display limit that should be collapsed.
Example scenario:
minCollapsedNum = 5,max = 3,tagList = ['a', 'b', 'c', 'd', 'e']showTagListcorrectly shows['a', 'b', 'c'](first 3 tags)collapsedTagListincorrectly returns[]- Expected:
['d', 'e'](tags 3-4 should be collapsed)
The fix is to use the same limit calculation as showTagList and slice from that position.
🔎 Proposed fix
collapsedTagList: computed(() => {
- return props.minCollapsedNum < props.max ? (state.tagList || []).slice(props.minCollapsedNum) : []
+ const limit = props.minCollapsedNum < props.max ? props.minCollapsedNum : props.max
+ return (state.tagList || []).slice(limit)
}),📝 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.
| collapsedTagList: computed(() => { | |
| return props.minCollapsedNum < props.max ? (state.tagList || []).slice(props.minCollapsedNum) : [] | |
| }), | |
| collapsedTagList: computed(() => { | |
| const limit = props.minCollapsedNum < props.max ? props.minCollapsedNum : props.max | |
| return (state.tagList || []).slice(limit) | |
| }), |
🤖 Prompt for AI Agents
In packages/renderless/src/tag-input/vue.ts around lines 56 to 58, the computed
collapsedTagList uses props.minCollapsedNum < props.max to decide whether to
return a slice, which drops tags when minCollapsedNum >= max; change
collapsedTagList to use the same limit calculation as showTagList (compute limit
= Math.min(props.max, (state.tagList || []).length) or the existing showTagList
logic) and return (state.tagList || []).slice(limit) so collapsedTagList
contains all tags after the displayed limit.
| tagType: { | ||
| type: String, | ||
| default: '', | ||
| validator: (value: string) => ['info', 'success', 'warning', 'danger', 'primary'].includes(value) | ||
| }, |
There was a problem hiding this comment.
Fix validator to include default value.
The tagType prop has a default value of '' (empty string), but the validator only allows ['info', 'success', 'warning', 'danger', 'primary']. This will cause Vue validation warnings when the component is used without explicitly setting tagType.
🔎 Proposed fix
tagType: {
type: String,
default: '',
- validator: (value: string) => ['info', 'success', 'warning', 'danger', 'primary'].includes(value)
+ validator: (value: string) => ['', 'info', 'success', 'warning', 'danger', 'primary'].includes(value)
},📝 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.
| tagType: { | |
| type: String, | |
| default: '', | |
| validator: (value: string) => ['info', 'success', 'warning', 'danger', 'primary'].includes(value) | |
| }, | |
| tagType: { | |
| type: String, | |
| default: '', | |
| validator: (value: string) => ['', 'info', 'success', 'warning', 'danger', 'primary'].includes(value) | |
| }, |
🤖 Prompt for AI Agents
In packages/vue/src/tag-input/src/index.ts around lines 26 to 30, the tagType
prop uses default:'' but the validator only allows
['info','success','warning','danger','primary'], causing Vue warnings; modify
the prop so the validator accepts the empty string (add '' to the allowed list)
or change the default to one of the allowed values (e.g., 'info') — pick one
approach and update the validator/default so they are consistent.
4 participants
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.