feat: mcp server picker component#125
Conversation
WalkthroughThis change set introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant McpServerPicker
participant PluginCard
participant PluginModal
participant FormEditor
participant CodeEditor
User->>McpServerPicker: Open plugin picker (show popup)
McpServerPicker->>User: Display installed/market plugins
User->>McpServerPicker: Search/filter/toggle plugin/tool
McpServerPicker->>PluginCard: Render plugin cards
PluginCard-->>McpServerPicker: Emit toggle/add/delete/expand events
User->>McpServerPicker: Click "Add Plugin"
McpServerPicker->>PluginModal: Open add plugin modal
PluginModal->>User: Show FormEditor or CodeEditor
User->>FormEditor: Fill form and confirm
FormEditor-->>PluginModal: Provide form data
PluginModal-->>McpServerPicker: Emit confirm event with data
User->>CodeEditor: Input plugin JSON and confirm
CodeEditor-->>PluginModal: Provide code data
PluginModal-->>McpServerPicker: Emit confirm event with data
McpServerPicker->>User: Update plugin list and UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes 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/mcp-server-picker/basic-usage.vueOops! Something went wrong! :( ESLint: 9.31.0 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. packages/components/src/mcp-server-picker/components/FormEditor.vueOops! Something went wrong! :( ESLint: 9.31.0 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. packages/components/src/mcp-server-picker/components/PluginModal.vueOops! Something went wrong! :( ESLint: 9.31.0 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it.
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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: 9
🧹 Nitpick comments (10)
packages/components/package.json (1)
29-33: Align dependency versions to avoid multiple installsNew UI dependencies are locked at
^3.21.0, while existing ones (@opentiny/vue-icon,@opentiny/vue-tooltip) are on^3.22.0.
Mixing ranges forces the package manager to hoist the highest minor (3.22.x), but it can still duplicate sub-trees if any peer range mismatch occurs.Prefer pinning the newly-added deps to the same
^3.22.0range (or keep everything on^3.21.0) to guarantee a single copy is resolved and cut bundle size.- "@opentiny/vue-tabs": "^3.21.0", - "@opentiny/vue-tab-item": "^3.21.0", - "@opentiny/vue-switch": "^3.21.0", - "@opentiny/vue-select": "^3.21.0", - "@opentiny/vue-option": "^3.21.0", + "@opentiny/vue-tabs": "^3.22.0", + "@opentiny/vue-tab-item": "^3.22.0", + "@opentiny/vue-switch": "^3.22.0", + "@opentiny/vue-select": "^3.22.0", + "@opentiny/vue-option": "^3.22.0",docs/.vitepress/config.mts (1)
69-70: Nit: keep sidebar items alphabetically sorted
McpServerPickernow sits betweenSuggestionPills(S) and the end of the list, breaking the current alphabetical ordering policy used above (Bubble → Welcome).
Placing it afterSuggestionPopoverpreserves scan-ability.packages/components/src/mcp-server-picker/index.ts (1)
6-10: Minor: exposeinstallviadefinePropertyto make it non-enumerableVue’s SFC compile output sometimes uses
Object.assignwhich will copy enumerable props; keepinginstallnon-enumerable prevents it from leaking into component props.-const install = function <T>(app: App<T>) { - app.component(MCPServerPicker.name!, MCPServerPicker) -} - -MCPServerPicker.install = install +const install = function <T>(app: App<T>) { + app.component(McpServerPicker.name!, McpServerPicker) +} +Object.defineProperty(McpServerPicker, 'install', { + value: install, + writable: false, +})packages/components/src/index.ts (1)
18-19: Path import succeeds but consider explicit file for clarity
import McpServerPicker from './mcp-server-picker'relies on directoryindex.tsresolution.
An explicit./mcp-server-picker/index.tsimport is less ambiguous to tools that don’t follow Node/TS resolution rules.packages/components/src/mcp-server-picker/components/PluginCodeDialog.vue (1)
81-95: Consider internationalization for UI textThe UI contains hardcoded Chinese text that might not be suitable for all users.
Consider using an i18n system for better internationalization support:
-<span class="plugin-code-dialog__editor-title">ai_plugin（填写json）</span> +<span class="plugin-code-dialog__editor-title">{{ $t('plugin.code.aiPluginTitle') }}</span> -<span class="plugin-code-dialog__editor-title">openapi（填写yaml）</span> +<span class="plugin-code-dialog__editor-title">{{ $t('plugin.code.openapiTitle') }}</span>packages/components/src/mcp-server-picker/components/PluginCard.vue (1)
261-266: Add tooltip for truncated descriptionsPlugin descriptions are truncated with ellipsis but users have no way to see the full text.
Add a title attribute or tooltip to show full description on hover:
&__desc { font-size: 14px; font-weight: 400; line-height: 24px; text-align: justify; color: rgb(89, 89, 89); white-space: nowrap; overflow: hidden; text-overflow: ellipsis; + cursor: help; }And in the template:
-<div class="plugin-card__desc">{{ plugin.description }}</div> +<div class="plugin-card__desc" :title="plugin.description">{{ plugin.description }}</div>packages/components/src/mcp-server-picker/index.type.ts (1)
68-116: Consider breaking down the large props interface for better maintainability.The
McpServerPickerPropsinterface contains 26 properties, making it difficult to understand at a glance. Consider grouping related properties into sub-interfaces for better organization and reusability.Example refactoring approach:
+export interface DataSourceProps { + installedPlugins?: PluginInfo[] + marketPlugins?: PluginInfo[] +} + +export interface SearchProps { + searchPlaceholder?: string + enableSearch?: boolean + marketCategoryOptions?: MarketCategoryOption[] + marketCategoryPlaceholder?: string + enableMarketCategoryFilter?: boolean +} + +export interface PanelControlProps { + defaultActiveTab?: 'installed' | 'market' + showInstalledTab?: boolean + showMarketTab?: boolean + visible?: boolean + popupConfig?: PopupConfig + activeCount?: number + installedTabTitle?: string + marketTabTitle?: string +} + +export interface HeaderProps { + title?: string + showCustomAddButton?: boolean + customAddButtonText?: string +} + +export interface BehaviorControlProps { + allowPluginToggle?: boolean + allowToolToggle?: boolean + allowPluginDelete?: boolean + allowPluginAdd?: boolean + enableParentChildSync?: boolean +} + +export interface LoadingProps { + loading?: boolean + marketLoading?: boolean +} + -export interface McpServerPickerProps { - // ... all 26 properties -} +export interface McpServerPickerProps extends + DataSourceProps, + SearchProps, + PanelControlProps, + HeaderProps, + BehaviorControlProps, + LoadingProps {}docs/src/components/mcp-server-picker.md (1)
24-29: Add blank lines around tables for proper markdown formatting.The static analysis tool indicates that tables should be surrounded by blank lines for proper markdown formatting compliance.
Apply this pattern to all tables in the document:
#### 数据源配置 + | 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| | `installedPlugins` | `PluginInfo[]` | `[]` | 已安装插件列表 | | `marketPlugins` | `PluginInfo[]` | `[]` | 市场插件列表 | + #### 搜索与筛选Also applies to: 30-38, 39-50, 52-58, 59-67, 68-73, 76-81, 82-88, 89-96, 97-101, 102-108, 109-113
packages/components/src/mcp-server-picker/index.vue (2)
388-388: Replace magic numbers with named constants or CSS variables.Using hard-coded pixel values reduces maintainability and makes it harder to ensure consistent spacing throughout the application.
- <div v-if="props.enableMarketCategoryFilter" style="width: 168px"> + <div v-if="props.enableMarketCategoryFilter" class="market-category-select"> <TinySelect v-model="marketCategory" :placeholder="props.marketCategoryPlaceholder"> - <div v-if="props.enableSearch" style="width: 264px; flex-shrink: 0"> + <div v-if="props.enableSearch" class="market-search-input"> <TinyInput v-model="marketSearch" :placeholder="currentSearchPlaceholder">Then define the widths in your styles section:
.market-category-select { width: 168px; } .market-search-input { width: 264px; flex-shrink: 0; }Also applies to: 400-400
1-615: Consider splitting this large component into smaller, focused sub-components.At 615 lines, this component handles multiple responsibilities including dialog management, event handling, style computation, and UI rendering. This makes it harder to maintain and test.
Consider extracting:
- Style computation logic into a composable (e.g.,
usePickerStyles)- Dialog management into a separate composable (e.g.,
usePluginDialogs)- Plugin list sections into dedicated components (e.g.,
InstalledPluginsList,MarketPluginsList)This would improve code organization, reusability, and testability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/components/src/assets/svgs/edit.svgis excluded by!**/*.svgpackages/svgs/src/assets/arrow-right.svgis excluded by!**/*.svgpackages/svgs/src/assets/plugin.svgis excluded by!**/*.svgpackages/svgs/src/assets/plus.svgis excluded by!**/*.svgpackages/svgs/src/assets/selected.svgis excluded by!**/*.svgpackages/svgs/src/assets/shell.svgis excluded by!**/*.svgpackages/svgs/src/assets/unselected.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
docs/.vitepress/config.mts(1 hunks)docs/demos/mcp-server-picker/basic-usage.vue(1 hunks)docs/demos/mcp-server-picker/popup-config.vue(1 hunks)docs/src/components/mcp-server-picker.md(1 hunks)packages/components/package.json(1 hunks)packages/components/src/index.ts(4 hunks)packages/components/src/mcp-server-picker/components/PluginCard.vue(1 hunks)packages/components/src/mcp-server-picker/components/PluginCodeDialog.vue(1 hunks)packages/components/src/mcp-server-picker/components/PluginFormDialog.vue(1 hunks)packages/components/src/mcp-server-picker/components/index.ts(1 hunks)packages/components/src/mcp-server-picker/index.ts(1 hunks)packages/components/src/mcp-server-picker/index.type.ts(1 hunks)packages/components/src/mcp-server-picker/index.vue(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/components/src/mcp-server-picker/index.ts (1)
packages/components/src/index.ts (1)
install(57-62)
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
69-69: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
77-77: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
83-83: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
90-90: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
98-98: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
103-103: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
110-110: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
🔇 Additional comments (6)
packages/components/src/mcp-server-picker/components/index.ts (1)
1-3: LGTM – clean barrel exportStraightforward barrel keeps import paths short.
No issues spotted.packages/components/src/index.ts (2)
53-54: Duplicate install when both plugin array & self-install coexist
McpServerPickeralready exposes its owninstall; pushing it intocomponentscauses the top-level installer to register it again.
While harmless, it wastes cycles and can warn in Vue 3.3 (app.componentduplicate).
Either:
- Remove it from the
componentsarray, relying onMcpServerPicker.install, or- Keep it in the array and remove the local
.installproperty.
100-102: Alias still inconsistent after renamingOnce the component
nameis fixed toMcpServerPicker, export alias should stay as-is (TrMcpServerPicker).
Verify bothexportandglobal component nameare identical (TrMcpServerPicker) to avoid confusion.packages/components/src/mcp-server-picker/components/PluginFormDialog.vue (1)
461-461: Verify the existence of the hardcoded asset pathThe edit icon uses a hardcoded relative path that might not exist in the build output.
#!/bin/bash # Description: Check if the edit.svg file exists at the expected location # Look for the edit.svg file fd -t f "edit.svg" packages/components/src/ # Check if there's an assets directory structure fd -t d "assets" packages/components/src/mcp-server-picker/packages/components/src/mcp-server-picker/components/PluginCard.vue (1)
74-87: ```shell
#!/bin/bashDisplay the first 200 lines to inspect pluginState and watcher implementation
sed -n '1,200p' packages/components/src/mcp-server-picker/components/PluginCard.vue
</details> <details> <summary>docs/demos/mcp-server-picker/popup-config.vue (1)</summary> `166-188`: **Well-implemented single-instance modal pattern!** The use of computed properties to manage modal visibility ensures only one modal is shown at a time, providing excellent user experience. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/components/src/mcp-server-picker/index.vue (1)
163-177: Still violating Vue's one-way data flow principle with direct prop mutations.The component continues to directly mutate prop objects in
handleAddPlugin(line 167:plugin.added = added) andhandlePluginExpand(line 174:plugin.expanded = expanded). This violates Vue's one-way data flow principle.Remove these direct mutations and rely solely on event emission:
const handleAddPlugin = (plugin: PluginInfo, added: boolean) => { if (!props.allowPluginAdd) return - // 直接更新插件对象的added状态 - plugin.added = added - emit('plugin-add', plugin, added) } const handlePluginExpand = (plugin: PluginInfo, expanded: boolean) => { - // 直接更新插件数据 - plugin.expanded = expanded - emit('plugin-expand', plugin, expanded) }The parent component should handle these state updates and pass the updated data back through props.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/components/src/mcp-server-picker/components/PluginFormDialog.vue(1 hunks)packages/components/src/mcp-server-picker/index.ts(1 hunks)packages/components/src/mcp-server-picker/index.type.ts(1 hunks)packages/components/src/mcp-server-picker/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/components/src/mcp-server-picker/index.ts
- packages/components/src/mcp-server-picker/components/PluginFormDialog.vue
- packages/components/src/mcp-server-picker/index.type.ts
🔇 Additional comments (2)
packages/components/src/mcp-server-picker/index.vue (2)
1-62: Well-structured component setup with comprehensive TypeScript integration.The imports, props definition, and initial state setup follow Vue 3 best practices with proper TypeScript support and sensible defaults.
437-603: Comprehensive and well-structured styling with proper mode support.The LESS styles are well-organized with clear separation between popup modes, smooth animations, and appropriate deep selectors for component customization.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/components/src/mcp-server-picker/index.vue (1)
166-169: Fix direct prop mutation - violates Vue's one-way data flow principle.The code directly mutates the
plugin.addedproperty, which violates Vue's one-way data flow principle and can lead to unexpected behavior.const handleAddPlugin = (plugin: PluginInfo, added: boolean) => { if (!props.allowPluginAdd) return - // 直接更新插件对象的added状态 - plugin.added = added - emit('plugin-add', plugin, added) }The parent component should handle updating the plugin state and pass the updated data back through props.
🧹 Nitpick comments (3)
packages/components/src/mcp-server-picker/index.vue (3)
78-79: Consider memoization for computed properties that reference props.These computed properties directly return props values, which may cause unnecessary re-renders. Consider whether they're needed or if direct prop access would be more appropriate.
-const installedPluginsList = computed(() => props.installedPlugins) -const marketPluginsList = computed(() => props.marketPlugins) +// Use props directly in template or add filtering logic if needed +// const installedPluginsList = computed(() => +// props.installedPlugins.filter(/* your filter logic */) +// )If these computed properties don't add transformation logic, consider using props directly in the template.
253-273: Simplify position styles function with better type safety.The current implementation uses
Record<string, string>but could be more type-safe and concise.-const getFixedPositionStyles = (position: PopupConfig['position'] = {}) => { - const styles: Record<string, string> = {} - - // 处理位置属性 - Object.entries(position).forEach(([key, value]) => { - if (value !== undefined) { - styles[key] = typeof value === 'number' ? `${value}px` : value - } - }) - - // 如果没有设置任何位置，使用默认居中 - if (Object.keys(styles).length === 0) { - return { - top: '50%', - left: '50%', - transform: 'translate(-50%, -50%)', - } - } - - return styles -} +const getFixedPositionStyles = (position: PopupConfig['position'] = {}) => { + const hasPosition = Object.keys(position).length > 0 + + if (!hasPosition) { + return { + top: '50%', + left: '50%', + transform: 'translate(-50%, -50%)', + } + } + + return Object.fromEntries( + Object.entries(position) + .filter(([, value]) => value !== undefined) + .map(([key, value]) => [key, typeof value === 'number' ? `${value}px` : value]) + ) +}
549-583: Consider extracting animation styles to CSS custom properties for maintainability.The animation durations and timing functions are hardcoded. Consider using CSS custom properties for easier theming and maintenance.
+:root { + --mcp-picker-transition-duration: 0.3s; + --mcp-picker-transition-timing: ease; +} + .drawer-left, .drawer-right { &-enter-active, &-leave-active { - transition: transform 0.3s ease; + transition: transform var(--mcp-picker-transition-duration) var(--mcp-picker-transition-timing); } } .fade { &-enter-active, &-leave-active { - transition: opacity 0.3s ease; + transition: opacity var(--mcp-picker-transition-duration) var(--mcp-picker-transition-timing); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/demos/mcp-server-picker/basic-usage.vue(1 hunks)packages/components/src/mcp-server-picker/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/demos/mcp-server-picker/basic-usage.vue
🔇 Additional comments (2)
packages/components/src/mcp-server-picker/index.vue (2)
358-362: LGTM! Event-based prop communication is correctly implemented.The component properly uses
:expandedbinding with@update:expandedevent handler instead ofv-model, which avoids direct prop mutations and maintains proper one-way data flow.
115-139: LGTM! Plugin toggle logic correctly uses event emission.The refactored event handling properly emits events instead of directly mutating plugin properties, which maintains Vue's one-way data flow principle.
SonyLeo
force-pushed
the
component/mcp-server-picker
branch
from
d70ffbf to
48d99e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/components/src/mcp-server-picker/index.type.ts (1)
41-184: Comprehensive type definitions with good structure.The type system provides:
- Clear interfaces for all component interactions
- Proper event typing with specific parameters
- Good separation between props and emits
- Flexible configuration options
However, there's still the language consistency issue mentioned in previous reviews.
🧹 Nitpick comments (4)
packages/components/src/mcp-server-picker/components/PluginModal.vue (1)
52-87: Consider adding accessibility attributes for the modal.The modal implementation is functional but missing some accessibility features that would improve screen reader support.
- <div v-if="visible" class="plugin-editor" ref="dialogRef"> + <div v-if="visible" class="plugin-editor" ref="dialogRef" role="dialog" aria-modal="true" aria-labelledby="modal-title"> <div class="plugin-editor__header"> - <h3 class="plugin-editor__title">添加插件</h3> + <h3 id="modal-title" class="plugin-editor__title">添加插件</h3> <IconClose class="plugin-editor__close" @click="handleClose" /> </div>packages/components/src/mcp-server-picker/components/FormEditor.vue (1)
9-9: Consider using a configurable default image URL.The hardcoded CDN URL could become a maintenance issue if the CDN changes or becomes unavailable.
Consider making this configurable via props:
-const defaultImageUrl = 'https://res.hc-cdn.com/tinyui-design/1.1.0.20250526191525/home/images/tiny-ng.svg' +const props = withDefaults(defineProps<FormEditorProps>(), { + defaultImageUrl: 'https://res.hc-cdn.com/tinyui-design/1.1.0.20250526191525/home/images/tiny-ng.svg' +})packages/components/src/mcp-server-picker/components/PluginCard.vue (1)
74-87: Optimize watcher performance with better dependency tracking.The current watcher may trigger unnecessarily on unrelated changes due to deep watching the entire tools array.
Consider a more targeted approach:
-watch( - () => plugin.value.tools?.map((tool) => tool.enabled), - () => { +watch( + () => plugin.value.tools?.map((tool) => tool.enabled).join(','), + (newEnabledStates) => { if (enableParentChildSync.value && mode.value === 'installed' && plugin.value.tools?.length) { const newPluginState = pluginState.value if (plugin.value.enabled !== newPluginState.checked) { plugin.value.enabled = newPluginState.checked emit('toggle-plugin', newPluginState.checked) } } }, - { deep: true }, )This reduces the frequency of watcher executions by only watching the enabled states.
docs/src/components/mcp-server-picker.md (1)
24-72: Fix table formatting issues flagged by markdownlint.The documentation has multiple tables that lack proper blank line spacing, which violates markdown formatting standards.
Add blank lines before and after each table as indicated by the static analysis:
#### 数据源配置 + | 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| | `installedPlugins` | `PluginInfo[]` | `[]` | 已安装插件列表 | | `marketPlugins` | `PluginInfo[]` | `[]` | 市场插件列表 | + #### 搜索与筛选 + | 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| | `enableSearch` | `boolean` | `true` | 是否启用搜索功能 | | `searchPlaceholder` | `string` | `'搜索插件'` | 搜索框占位符 | | `enableMarketCategoryFilter` | `boolean` | `true` | 是否启用市场分类筛选功能 | | `marketCategoryOptions` | `MarketCategoryOption[]` | `[]` | 市场分类选项列表 | | `marketCategoryPlaceholder` | `string` | `'按照分类筛选'` | 分类筛选下拉框占位符 | +Apply the same pattern to all remaining tables (lines 40, 53, 60, 69, 77, 83, 90, 98, 103, 110).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/components/src/assets/svgs/edit.svgis excluded by!**/*.svgpackages/svgs/src/assets/arrow-right.svgis excluded by!**/*.svgpackages/svgs/src/assets/plugin.svgis excluded by!**/*.svgpackages/svgs/src/assets/plus.svgis excluded by!**/*.svgpackages/svgs/src/assets/selected.svgis excluded by!**/*.svgpackages/svgs/src/assets/shell.svgis excluded by!**/*.svgpackages/svgs/src/assets/unselected.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
docs/.vitepress/config.mts(1 hunks)docs/demos/mcp-server-picker/basic-usage.vue(1 hunks)docs/demos/mcp-server-picker/popup-config.vue(1 hunks)docs/src/components/mcp-server-picker.md(1 hunks)packages/components/package.json(1 hunks)packages/components/src/index.ts(4 hunks)packages/components/src/mcp-server-picker/components/CodeEditor.vue(1 hunks)packages/components/src/mcp-server-picker/components/FormEditor.vue(1 hunks)packages/components/src/mcp-server-picker/components/PluginCard.vue(1 hunks)packages/components/src/mcp-server-picker/components/PluginModal.vue(1 hunks)packages/components/src/mcp-server-picker/components/index.ts(1 hunks)packages/components/src/mcp-server-picker/index.ts(1 hunks)packages/components/src/mcp-server-picker/index.type.ts(1 hunks)packages/components/src/mcp-server-picker/index.vue(1 hunks)
🧠 Learnings (4)
packages/components/src/mcp-server-picker/components/PluginModal.vue (1)
Learnt from: gene9831
PR: #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/mcp-server-picker/components/PluginCard.vue (1)
Learnt from: SonyLeo
PR: #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/mcp-server-picker/index.vue (3)
Learnt from: gene9831
PR: #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: #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: #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/mcp-server-picker/components/FormEditor.vue (1)
Learnt from: gene9831
PR: #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.
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
69-69: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
77-77: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
83-83: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
90-90: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
98-98: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
103-103: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
110-110: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
✅ Files skipped from review due to trivial changes (2)
- packages/components/src/mcp-server-picker/components/CodeEditor.vue
- packages/components/src/mcp-server-picker/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/components/src/index.ts
- packages/components/package.json
- docs/demos/mcp-server-picker/popup-config.vue
- docs/.vitepress/config.mts
- docs/demos/mcp-server-picker/basic-usage.vue
🧰 Additional context used
🧠 Learnings (4)
packages/components/src/mcp-server-picker/components/PluginModal.vue (1)
Learnt from: gene9831
PR: #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/mcp-server-picker/components/PluginCard.vue (1)
Learnt from: SonyLeo
PR: #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/mcp-server-picker/index.vue (3)
Learnt from: gene9831
PR: #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: #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: #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/mcp-server-picker/components/FormEditor.vue (1)
Learnt from: gene9831
PR: #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.
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
69-69: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
77-77: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
83-83: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
90-90: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
98-98: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
103-103: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
110-110: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (16)
packages/components/src/mcp-server-picker/components/index.ts (1)
1-4: LGTM! Clean barrel export pattern.The export structure is well-organized and follows standard conventions for component libraries.
packages/components/src/mcp-server-picker/components/PluginModal.vue (2)
1-50: Well-structured modal component with proper event handling.The component correctly implements:
- Two-way binding with
defineModelfor visibility- Proper TypeScript typing for emits
- Click-outside functionality for UX
- Clean separation of form and code data handling
89-222: Well-designed styles with smooth transitions.The CSS implementation provides:
- Proper z-index layering for backdrop and modal
- Smooth enter/leave transitions
- Responsive button styling with hover states
- Good visual hierarchy
packages/components/src/mcp-server-picker/components/FormEditor.vue (2)
73-127: Well-structured form template with proper accessibility.The form implementation provides:
- Proper label associations
- Appropriate input types (text, url, textarea)
- Good placeholder text
- Accessible file upload with visual feedback
240-240: SVG asset path validatedThe
edit.svgfile exists atpackages/components/src/assets/svgs/edit.svg, which correctly matches the relative reference in
packages/components/src/mcp-server-picker/components/FormEditor.vue(line 240):background-image: url('../../assets/svgs/edit.svg');No changes required.
packages/components/src/mcp-server-picker/components/PluginCard.vue (3)
37-53: Well-implemented tri-state logic for plugin status.The computed property correctly handles the three states:
- All tools disabled (unchecked)
- All tools enabled (checked)
- Mixed state (indeterminate)
This provides good UX for managing parent-child relationships.
185-395: Comprehensive and well-organized styles.The CSS implementation provides:
- Smooth animations for expand/collapse
- Proper layout with flexbox
- Good visual hierarchy and spacing
- Responsive design considerations
- Consistent color scheme
102-183: Well-structured template with good slot usage.The template provides:
- Flexible slot system for customization
- Proper conditional rendering based on mode
- Clean separation of plugin and tool UI
- Smooth transitions for expandable content
docs/src/components/mcp-server-picker.md (1)
116-204: Comprehensive and well-structured type definitions.The TypeScript interface definitions are thorough and properly documented, covering all necessary types for plugin management, popup configuration, and data structures.
packages/components/src/mcp-server-picker/index.vue (7)
1-11: Well-structured imports and type definitions.The imports are properly organized with external dependencies, internal components, and TypeScript types clearly separated.
12-40: Comprehensive props definition with sensible defaults.The props are well-defined with proper TypeScript typing and reasonable default values that provide good out-of-the-box functionality.
44-87: Proper reactive state management and event emission.The component correctly uses Vue 3's composition API with appropriate watchers for emitting events when state changes. The computed properties efficiently derive state from props.
89-149: Event handlers correctly implement one-way data flow.The event handling functions properly emit events to the parent component rather than directly mutating props, following Vue's recommended patterns for component communication.
172-258: Robust popup positioning and animation system.The computed properties for popup positioning and animations handle both fixed and drawer modes effectively, with proper fallbacks and transition management.
261-363: Well-structured template with proper Vue 3 patterns.The template correctly uses Vue 3 composition API patterns, conditional rendering, and component integration. Event handling and data binding are implemented properly.
365-531: Comprehensive and well-organized component styling.The LESS styles are properly scoped with good visual design, appropriate animations for popup transitions, and effective use of deep selectors for child component styling.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
docs/demos/mcp-server-picker/basic-usage.vue (2)
165-187: Plugin removal logic still uses name-based matching.This issue was flagged in previous reviews but remains unaddressed. The function adds plugins with unique IDs but removes them by name, which could remove the wrong plugin if multiple plugins share the same name.
Apply this fix to use proper ID-based matching:
const handlePluginAdd = (plugin: PluginInfo, added: boolean) => { console.log('插件添加状态变化:', plugin, added) const targetPlugin = marketPlugins.value.find((p) => p.id === plugin.id) targetPlugin.added = added if (added) { // 如果是添加操作，创建新的插件副本并添加到已安装列表 const newPlugin: PluginInfo = { ...plugin, id: `${plugin.id}-installed-${Date.now()}`, // 生成新的ID避免冲突 enabled: false, // 新添加的插件默认不启用 added: true, + marketPluginId: plugin.id, // Keep reference to market plugin } installedPlugins.value.push(newPlugin) } else { // 如果是取消添加操作，从已安装列表中移除 - const index = installedPlugins.value.findIndex((p) => p.name === plugin.name) + const index = installedPlugins.value.findIndex((p) => p.marketPluginId === plugin.id) if (index > -1) { installedPlugins.value.splice(index, 1) } } }
205-217: Thumbnail file processing still missing.The previous review flagged that the thumbnail file from form data is ignored when creating plugins. This remains unaddressed.
Process the thumbnail file to create a preview URL:
const createPluginByForm = (data: FormData) => { console.log('表单方式添加插件:', data) + + // Create thumbnail URL if file exists + let thumbnailUrl = '' + if (data.thumbnail) { + thumbnailUrl = URL.createObjectURL(data.thumbnail) + } + // 可以在这里处理表单数据，例如发送到服务器 const newPlugin: PluginInfo = { id: `custom-${Date.now()}`, name: data.name, - icon: '', // 如果有缩略图可以处理 data.thumbnail + icon: thumbnailUrl || 'https://via.placeholder.com/40', // Use thumbnail or placeholder description: data.description, enabled: false, tools: [], + // Store reference for cleanup + _thumbnailObjectUrl: thumbnailUrl, } installedPlugins.value.push(newPlugin) }
🧹 Nitpick comments (6)
docs/demos/mcp-server-picker/basic-usage.vue (1)
224-230: Add type annotation for data parameter.The
dataparameter type should be explicitly typed as the union typeDatafor better type safety.-const handlePluginCreate = (type: 'form' | 'code', data: Data) => { +const handlePluginCreate = (type: 'form' | 'code', data: Data) => { if (type === 'form') { - createPluginByForm(data) + createPluginByForm(data as FormData) } else { - createPluginByCode(data) + createPluginByCode(data as string) } }packages/components/src/mcp-server-picker/components/PluginCard.vue (2)
146-172: Optimize transition performance.The slide transition uses
max-height: 1000pxwhich could cause performance issues and doesn't provide accurate animations. Consider using a more precise approach.Replace the fixed max-height with dynamic calculation:
<transition name="plugin-card-slide" v-if="expandable"> - <div v-show="isExpanded" class="plugin-card__tools"> + <div v-show="isExpanded" class="plugin-card__tools" ref="toolsContainer">And update the CSS:
.plugin-card-slide { &-enter-active, &-leave-active { transition: all 0.3s ease; overflow: hidden; } &-enter-from, &-leave-to { - max-height: 0; + height: 0; opacity: 0; transform: translateY(-10px); } &-enter-to, &-leave-from { - max-height: 1000px; + height: auto; opacity: 1; transform: translateY(0); } }
383-385: CSS custom property override may cause specificity issues.The TinyButton style override using CSS custom properties might not work reliably across different contexts and could interfere with other components.
Use a more specific selector or move to a global stylesheet:
-.tiny-button { +.plugin-card .tiny-button { --tv-Button-height-small: 16px; }Or consider using a CSS module approach for better encapsulation.
docs/src/components/mcp-server-picker.md (3)
24-29: Fix markdown table formatting.The table lacks blank lines before and after it, violating markdown standards.
#### 数据源配置 + | 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| | `installedPlugins` | `PluginInfo[]` | `[]` | 已安装插件列表 | | `marketPlugins` | `PluginInfo[]` | `[]` | 市场插件列表 | +
30-38: Fix markdown table formatting.Add blank lines around the table to comply with markdown standards.
#### 搜索与筛选 + | 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| | `enableSearch` | `boolean` | `true` | 是否启用搜索功能 | | `searchPlaceholder` | `string` | `'搜索插件'` | 搜索框占位符 | | `enableMarketCategoryFilter` | `boolean` | `true` | 是否启用市场分类筛选功能 | | `marketCategoryOptions` | `MarketCategoryOption[]` | `[]` | 市场分类选项列表 | | `marketCategoryPlaceholder` | `string` | `'按照分类筛选'` | 分类筛选下拉框占位符 | +
177-191: Clarify PopupConfig type definition.The interface definition is clear, but it would be helpful to specify which properties are required vs optional and provide examples.
```typescript interface PopupConfig { type: 'fixed' | 'drawer' // fixed模式配置 position?: { top?: string | number left?: string | number right?: string | number bottom?: string | number } // drawer模式配置 drawer?: { direction: 'left' | 'right' } }
+示例：
+
+```typescript
+// Fixed 模式
+const fixedConfig: PopupConfig = {
- type: 'fixed',
- position: { top: 100, right: 20 }
+}+// Drawer 模式
+const drawerConfig: PopupConfig = {
- type: 'drawer',
- drawer: { direction: 'right' }
+}
+```</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 48d99e25ebebd458c7d770251ea44f4b502466a2 and e8173972d5d31cdeb1e351840ebd7ba4f1ccfe76. </details> <details> <summary>📒 Files selected for processing (8)</summary> * `docs/demos/mcp-server-picker/basic-usage.vue` (1 hunks) * `docs/demos/mcp-server-picker/popup-config.vue` (1 hunks) * `docs/src/components/mcp-server-picker.md` (1 hunks) * `packages/components/src/mcp-server-picker/components/FormEditor.vue` (1 hunks) * `packages/components/src/mcp-server-picker/components/PluginCard.vue` (1 hunks) * `packages/components/src/mcp-server-picker/components/PluginModal.vue` (1 hunks) * `packages/components/src/mcp-server-picker/index.type.ts` (1 hunks) * `packages/components/src/mcp-server-picker/index.vue` (1 hunks) </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/src/components/mcp-server-picker.md</summary> 25-25: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 31-31: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 40-40: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 53-53: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 60-60: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 68-68: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 76-76: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 82-82: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 89-89: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 97-97: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 102-102: Tables should be surrounded by blank lines (MD058, blanks-around-tables) </details> </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (5)</summary> * packages/components/src/mcp-server-picker/components/PluginModal.vue * packages/components/src/mcp-server-picker/components/FormEditor.vue * docs/demos/mcp-server-picker/popup-config.vue * packages/components/src/mcp-server-picker/index.vue * packages/components/src/mcp-server-picker/index.type.ts </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/src/components/mcp-server-picker.md</summary> 25-25: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 31-31: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 40-40: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 53-53: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 60-60: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 68-68: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 76-76: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 82-82: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 89-89: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 97-97: Tables should be surrounded by blank lines (MD058, blanks-around-tables) --- 102-102: Tables should be surrounded by blank lines (MD058, blanks-around-tables) </details> </details> </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>packages/components/src/mcp-server-picker/components/PluginCard.vue (3)</summary> `1-12`: **LGTM! Clean component setup and imports.** The component setup follows Vue 3 composition API best practices with proper TypeScript integration and well-organized imports. --- `32-47`: **Efficient tri-state checkbox implementation.** The computed property correctly implements tri-state logic for parent plugins based on their tools' enabled states. The logic handles all three states appropriately: unchecked (no tools enabled), checked (all tools enabled), and indeterminate (partial tools enabled). --- `49-61`: **Handle plugin toggle efficiently.** The plugin toggle logic correctly synchronizes all child tools when the parent plugin is toggled, and emits appropriate events. The implementation avoids unnecessary updates by checking if the tool's state actually changes. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/src/components/mcp-server-picker.md (1)
113-120:toolsshould be optional – unresolved from last review.Previous review already flagged this; demos show plugins without a
toolsarray.
Update the interface accordingly:- tools: PluginTool[] // 工具列表 + tools?: PluginTool[] // 工具列表（可选）
🧹 Nitpick comments (1)
docs/src/components/mcp-server-picker.md (1)
24-28: Add blank lines before each Markdown table.
markdownlint(rule MD058) complains because every table starts immediately after the preceding heading.
Insert a single blank line before the|header row (the closing blank lines are already present).Example fix for the first two tables (replicate for the rest):
-#### 数据源配置 -| 属性 | 类型 | 默认值 | 说明 | +#### 数据源配置 + +| 属性 | 类型 | 默认值 | 说明 | @@ -#### 搜索与筛选 -| 属性 | 类型 | 默认值 | 说明 | +#### 搜索与筛选 + +| 属性 | 类型 | 默认值 | 说明 |This satisfies MD058, improves readability, and keeps CI quiet.
Also applies to: 30-37, 39-49, 53-57, 59-66, 68-72, 75-80, 81-87, 88-94, 96-100, 101-104
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/demos/mcp-server-picker/basic-usage.vue(1 hunks)docs/src/components/mcp-server-picker.md(1 hunks)packages/components/src/mcp-server-picker/components/PluginCard.vue(1 hunks)packages/components/src/mcp-server-picker/index.type.ts(1 hunks)
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
76-76: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
82-82: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
89-89: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
97-97: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
102-102: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/components/src/mcp-server-picker/components/PluginCard.vue
- docs/demos/mcp-server-picker/basic-usage.vue
- packages/components/src/mcp-server-picker/index.type.ts
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
76-76: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
82-82: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
89-89: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
97-97: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
102-102: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/components/src/mcp-server-picker/index.vue (1)
269-273: Add missing accessibility attributes for interactive elements.The custom add button and close icon still lack proper accessibility attributes for keyboard navigation and screen reader support.
Apply the accessibility improvements suggested in previous reviews:
- <div v-if="props.showCustomAddButton" class="mcp-server-picker__header-right-item" @click="handleCustomAdd"> + <div + v-if="props.showCustomAddButton" + class="mcp-server-picker__header-right-item" + role="button" + tabindex="0" + :aria-label="props.customAddButtonText" + @click="handleCustomAdd" + @keydown.enter="handleCustomAdd" + @keydown.space.prevent="handleCustomAdd" + > <IconPlus style="font-size: 16px; cursor: pointer" /> <span>{{ props.customAddButtonText }}</span> </div> - <IconClose class="mcp-server-picker__header-right-close" @click="handleClose" /> + <IconClose + class="mcp-server-picker__header-right-close" + role="button" + tabindex="0" + aria-label="关闭" + @click="handleClose" + @keydown.enter="handleClose" + @keydown.space.prevent="handleClose" + />docs/src/components/mcp-server-picker.md (1)
112-123: Fix TypeScript interface documentation for optional properties.The
toolsandenabledproperties should be marked as optional based on the component's usage patterns and past review feedback.interface PluginInfo { id: string // 插件唯一标识 name: string // 插件名称 icon: string // 插件图标URL description: string // 插件描述 - enabled: boolean // 是否启用 - tools: PluginTool[] // 工具列表 + enabled?: boolean // 是否启用（可选） + tools?: PluginTool[] // 工具列表（可选） added?: boolean // 市场插件添加状态(可选) category?: string // 插件分类(可选，用于市场分类筛选) }
🧹 Nitpick comments (1)
docs/src/components/mcp-server-picker.md (1)
24-105: Comprehensive API documentation with minor formatting improvements needed.The props and events documentation is thorough and well-categorized. However, markdown tables should be surrounded by blank lines for better formatting compliance.
Add blank lines before and after each table section to improve markdown formatting:
#### 数据源配置 + | 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| | `installedPlugins` | `PluginInfo[]` | `[]` | 已安装插件列表 | | `marketPlugins` | `PluginInfo[]` | `[]` | 市场插件列表 | +Apply similar spacing to all table sections throughout the document.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/demos/mcp-server-picker/basic-usage.vue(1 hunks)docs/src/components/mcp-server-picker.md(1 hunks)packages/components/src/mcp-server-picker/components/FormEditor.vue(1 hunks)packages/components/src/mcp-server-picker/index.type.ts(1 hunks)packages/components/src/mcp-server-picker/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/components/src/mcp-server-picker/components/FormEditor.vue
- docs/demos/mcp-server-picker/basic-usage.vue
- packages/components/src/mcp-server-picker/index.type.ts
🧰 Additional context used
🧠 Learnings (1)
packages/components/src/mcp-server-picker/index.vue (3)
Learnt from: gene9831
PR: #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: #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: #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变量。
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
76-76: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
82-82: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
89-89: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
97-97: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
102-102: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (9)
packages/components/src/mcp-server-picker/index.vue (7)
1-17: LGTM! Well-organized imports following Vue 3 best practices.The imports are properly structured with clear separation between external libraries, local components, and TypeScript types.
18-46: Excellent TypeScript props definition with comprehensive defaults.The use of
withDefaults(defineProps<McpServerPickerProps>(), {...})follows Vue 3 + TypeScript best practices, providing type safety while maintaining sensible default values for all configuration options.
47-73: Proper Vue 3 reactivity implementation with type-safe event handling.The reactive state management using
ref(),computed(), andwatch()follows composition API best practices. The immediate execution of the activePluginCount watcher ensures proper initial state synchronization.
94-147: Excellent event handling that maintains Vue's one-way data flow.The event handlers properly avoid direct prop mutations and use event emission for all state changes. The parent-child synchronization logic between plugins and tools is well-implemented while maintaining proper data flow patterns.
169-256: Well-architected popup configuration system with flexible styling.The dynamic style computation effectively supports both fixed and drawer popup modes with proper fallbacks. The separation of concerns between position calculation and animation classes demonstrates good component architecture.
274-357: Clean template structure with proper component composition.The tabbed interface is well-implemented using TinyTabs, and the PluginCard components are used consistently with appropriate event binding patterns. Loading states are handled gracefully with conditional rendering.
359-525: Comprehensive and well-organized LESS styles with proper scoping.The styles effectively support both popup modes with appropriate animations. The use of scoped styles prevents CSS leakage, and the deep selectors for third-party components are properly implemented.
docs/src/components/mcp-server-picker.md (2)
1-19: Clear and well-structured documentation introduction.The component description accurately reflects the MCP Server Picker's functionality, and the demo references follow the established documentation patterns.
149-191: Accurate and comprehensive TypeScript interface documentation.The remaining interface definitions correctly reflect the component's implementation, with proper documentation of optional properties and union types.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/components/src/mcp-server-picker/components/PluginCard.vue (2)
81-170: Well-structured template with good use of slots and transitions.The template is well-organized with proper conditional rendering and customization slots. The delete confirmation popup addresses the previous feedback about preventing accidental operations.
Consider adding accessibility improvements:
- <img :src="plugin.icon" class="plugin-card__icon" /> + <img :src="plugin.icon" :alt="`${plugin.name} icon`" class="plugin-card__icon" />And consider adding ARIA labels for interactive elements like the expand button and switches for better screen reader support.
172-382: Well-structured styles with modern CSS practices.The LESS styles are well-organized with consistent naming conventions and good use of transitions. The component styling follows modern UI patterns with appropriate shadows, spacing, and typography.
Consider making the component more responsive:
&__main { - width: 440px; + width: 100%; + max-width: 440px;This would allow the component to adapt better to different container sizes while maintaining the intended maximum width.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/package.json(1 hunks)packages/components/src/mcp-server-picker/components/PluginCard.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/package.json
🔇 Additional comments (4)
packages/components/src/mcp-server-picker/components/PluginCard.vue (4)
1-6: LGTM! Well-structured imports.The imports are properly organized with clear separation between external dependencies, Vue composition functions, icons, and TypeScript types.
8-13: LGTM! Proper Vue 3 TypeScript setup.The props definition with defaults and typed emits follow Vue 3 best practices and provide good developer experience.
15-48: LGTM! Well-implemented tri-state logic.The computed properties effectively manage the plugin's state with proper tri-state checkbox logic. The implementation correctly handles all edge cases for plugin and tool enabled states.
50-78: Event handlers look correct after infinite loop fix.The event handlers properly manage plugin and tool state synchronization. The previous infinite loop issue appears to have been resolved, and the current implementation correctly:
- Synchronizes all tool states when toggling the parent plugin
- Emits appropriate events for state changes
- Handles market mode plugin addition state
SonyLeo
force-pushed
the
component/mcp-server-picker
branch
from
64f46e6 to
6973896
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/components/src/mcp-server-picker/index.type.ts (1)
34-39: Language consistency issue persists in comments.The Chinese comments mixed with English code identifiers create inconsistency for international development teams. This was previously flagged but the issue remains unresolved.
Also applies to: 40-54, 56-66, 68-72, 74-120
docs/src/components/mcp-server-picker.md (1)
112-123: Fix PluginInfo interface documentation consistency.The documentation shows
enabled: booleanandtools: PluginTool[]as required, but based on usage patterns and the type definitions, these should be marked as optional.interface PluginInfo { id: string // 插件唯一标识 name: string // 插件名称 icon: string // 插件图标URL description: string // 插件描述 - enabled: boolean // 是否启用 - tools: PluginTool[] // 工具列表 + enabled?: boolean // 是否启用（可选） + tools?: PluginTool[] // 工具列表（可选） added?: boolean // 市场插件添加状态(可选) category?: string // 插件分类(可选，用于市场分类筛选) }packages/components/src/mcp-server-picker/index.vue (2)
269-272: Add accessibility attributes for custom add button.The custom add button lacks proper accessibility support for keyboard navigation and screen readers.
- <div v-if="props.showCustomAddButton" class="mcp-server-picker__header-right-item" @click="handleCustomAdd"> + <div + v-if="props.showCustomAddButton" + class="mcp-server-picker__header-right-item" + role="button" + tabindex="0" + :aria-label="props.customAddButtonText" + @click="handleCustomAdd" + @keydown.enter="handleCustomAdd" + @keydown.space.prevent="handleCustomAdd" + >
273-273: Add accessibility attributes for close button.The close button needs proper accessibility support.
- <IconClose class="mcp-server-picker__header-right-close" @click="handleClose" /> + <IconClose + class="mcp-server-picker__header-right-close" + role="button" + tabindex="0" + aria-label="关闭" + @click="handleClose" + @keydown.enter="handleClose" + @keydown.space.prevent="handleClose" + />
🧹 Nitpick comments (3)
packages/components/src/mcp-server-picker/index.type.ts (1)
13-17: Consider makingenabledandtoolsproperties optional in PluginInfo.Based on the usage patterns in the documentation and implementation, both
enabledandtoolsproperties might not always be required. Theenabledproperty could default tofalseandtoolscould be an empty array if not provided.export interface PluginInfo { id: string name: string icon: string description: string - enabled: boolean + enabled?: boolean - tools: PluginTool[] + tools?: PluginTool[] added?: boolean category?: string }docs/src/components/mcp-server-picker.md (1)
25-25: Fix markdown table formatting issues.Multiple tables lack proper blank line spacing as flagged by the markdown linter.
Add blank lines before and after each table to improve markdown compliance and readability. For example:
#### 数据源配置 + | 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| | `installedPlugins` | `PluginInfo[]` | `[]` | 已安装插件列表 | | `marketPlugins` | `PluginInfo[]` | `[]` | 市场插件列表 | +Apply this pattern to all tables in the document.
Also applies to: 31-31, 40-40, 53-53, 60-60, 68-68, 76-76, 82-82, 89-89, 97-97, 102-102
packages/components/src/mcp-server-picker/index.vue (1)
161-163: Consider making visible model optional for better component flexibility.The
visiblemodel is marked as required, but for better component reusability, it should be optional with a sensible default.const visible = defineModel<boolean>('visible', { - required: true, + default: false, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/components/src/assets/svgs/edit.svgis excluded by!**/*.svgpackages/svgs/src/assets/plugin.svgis excluded by!**/*.svgpackages/svgs/src/assets/plus.svgis excluded by!**/*.svgpackages/svgs/src/assets/selected.svgis excluded by!**/*.svgpackages/svgs/src/assets/shell.svgis excluded by!**/*.svgpackages/svgs/src/assets/unselected.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
docs/.vitepress/config.mts(1 hunks)docs/demos/mcp-server-picker/basic-usage.vue(1 hunks)docs/demos/mcp-server-picker/popup-config.vue(1 hunks)docs/src/components/mcp-server-picker.md(1 hunks)packages/components/package.json(1 hunks)packages/components/src/index.ts(4 hunks)packages/components/src/mcp-server-picker/components/CodeEditor.vue(1 hunks)packages/components/src/mcp-server-picker/components/FormEditor.vue(1 hunks)packages/components/src/mcp-server-picker/components/PluginCard.vue(1 hunks)packages/components/src/mcp-server-picker/components/PluginModal.vue(1 hunks)packages/components/src/mcp-server-picker/components/index.ts(1 hunks)packages/components/src/mcp-server-picker/index.ts(1 hunks)packages/components/src/mcp-server-picker/index.type.ts(1 hunks)packages/components/src/mcp-server-picker/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/.vitepress/config.mts
- packages/components/src/mcp-server-picker/components/index.ts
- packages/components/src/index.ts
- packages/components/src/mcp-server-picker/index.ts
- packages/components/package.json
- packages/components/src/mcp-server-picker/components/FormEditor.vue
- packages/components/src/mcp-server-picker/components/CodeEditor.vue
- packages/components/src/mcp-server-picker/components/PluginModal.vue
- packages/components/src/mcp-server-picker/components/PluginCard.vue
- docs/demos/mcp-server-picker/basic-usage.vue
- docs/demos/mcp-server-picker/popup-config.vue
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
76-76: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
82-82: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
89-89: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
97-97: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
102-102: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (2)
packages/components/src/mcp-server-picker/index.vue (2)
95-119: Excellent parent-child synchronization logic for plugin and tool states.The implementation correctly handles the complex relationship between plugin and tool activation states with proper cascading behavior. The logic ensures that:
- Disabling a parent plugin disables all child tools
- Enabling a parent plugin activates all tools if none are currently enabled
- The state changes are properly communicated through events
121-136: Well-implemented bidirectional synchronization for tool-to-plugin state updates.The tool toggle handler correctly implements the reverse synchronization logic, ensuring that when tools are toggled, the parent plugin state is updated appropriately. The logic properly calculates whether the plugin should remain enabled based on the status of other tools.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/src/components/mcp-server-picker.md (1)
118-119:enabledandtoolsshould be optional as previously requestedThe earlier review already pointed out that both properties are optional in real usage (see demo files). Please change:
- enabled: boolean // 是否启用 - tools: PluginTool[] // 工具列表 + enabled?: boolean // 是否启用（可选） + tools?: PluginTool[] // 工具列表（可选）
🧹 Nitpick comments (1)
docs/src/components/mcp-server-picker.md (1)
24-30: Add blank lines around all tables to satisfy MD058markdownlint flags every table because there’s no blank line before and after.
Example fix for the first table:-#### 数据源配置 -| 属性 | 类型 | 默认值 | 说明 | +#### 数据源配置 + +| 属性 | 类型 | 默认值 | 说明 | |------|------|--------|------| ... +Apply the same pattern to every table section.
Also applies to: 31-37, 39-49, 52-57, 59-66, 67-72, 75-80, 81-87, 88-95, 96-100, 101-105
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/demos/mcp-server-picker/basic-usage.vue(1 hunks)docs/src/components/mcp-server-picker.md(1 hunks)packages/components/src/mcp-server-picker/components/FormEditor.vue(1 hunks)packages/components/src/mcp-server-picker/components/PluginModal.vue(1 hunks)packages/components/src/mcp-server-picker/index.type.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/components/src/mcp-server-picker/components/FormEditor.vue
- packages/components/src/mcp-server-picker/components/PluginModal.vue
- docs/demos/mcp-server-picker/basic-usage.vue
- packages/components/src/mcp-server-picker/index.type.ts
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/components/mcp-server-picker.md
25-25: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
60-60: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
76-76: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
82-82: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
89-89: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
97-97: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
102-102: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
2 participants
Summary by CodeRabbit
New Features
Chores