Skip to content

feat: mcp-add-form component and docs#215

Merged
gene9831 merged 9 commits intoopentiny:developfrom
SonyLeo:component/mcp-add-form
Sep 9, 2025
Merged

feat: mcp-add-form component and docs#215
gene9831 merged 9 commits intoopentiny:developfrom
SonyLeo:component/mcp-add-form

Conversation

@SonyLeo
Copy link
Copy Markdown
Collaborator

@SonyLeo SonyLeo commented Sep 8, 2025

Summary by CodeRabbit

  • New Features

    • Added McpAddForm: two-mode add UI (form or code) with confirm/cancel and thumbnail support; globally installable.
  • Refactor

    • Plugin modal now uses McpAddForm, consolidating add workflow and smaller editor pieces.
  • Style

    • Updated modal, picker, and editor layouts for improved responsiveness and reduced max widths.
  • Documentation

    • New demo and comprehensive docs for McpAddForm covering usage, API, events, types, and theme variables.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 8, 2025

Walkthrough

Adds a new McpAddForm component (with FormEditor and CodeEditor subcomponents), registers and exports it from the components library, replaces inline editors in PluginModal with McpAddForm, adjusts modal/picker sizing and responsive styles, and adds docs and a demo describing props, emits, types, and CSS variables.

Changes

Cohort / File(s) Summary
Docs: Demo & Pages
docs/demos/mcp-add-form/basic.vue, docs/src/components/mcp-add-form.md, docs/src/components/mcp-server-picker.md
New demo and documentation pages for McpAddForm; documents usage, props, emits, types, CSS variables, and embeds the mcp-add-form docs into the server-picker doc.
McpAddForm SFC & types
packages/components/src/mcp-add-form/index.vue, packages/components/src/mcp-add-form/index.type.ts, packages/components/src/mcp-add-form/index.ts
New McpAddForm Vue SFC with typed props/emits, addType switching (form/code), confirm/cancel/update emits, install helper, and exported types.
McpAddForm subcomponents
packages/components/src/mcp-add-form/components/*
packages/components/src/mcp-add-form/components/index.ts
Added FormEditor.vue and CodeEditor.vue, plus a components barrel that re-exports them. FormEditor model type updated to McpAddFormData; CodeEditor provides a v-model for codeData.
Component library public API
packages/components/src/index.ts
Registers and re-exports McpAddForm, re-exports its types, and exposes alias TrMcpAddForm in the public API.
Server picker integration & cleanup
packages/components/src/mcp-server-picker/components/PluginModal.vue, packages/components/src/mcp-server-picker/components/index.ts, packages/components/src/mcp-server-picker/index.vue
Replaced inline editors with <McpAddForm />, updated handleConfirm(addType, data) signature, removed local exports for CodeEditor/FormEditor, adjusted modal width/padding and picker max-width to be fluid and responsive.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant PM as PluginModal
  participant A as McpAddForm
  participant F as FormEditor
  participant C as CodeEditor

  PM->>A: render McpAddForm
  U->>A: select addType (form / code)
  A-->>PM: emit update:addType(value)

  alt addType == form
    A->>F: bind v-model:formData
    U->>F: fill fields / upload thumbnail
  else addType == code
    A->>C: bind v-model:codeData
    U->>C: paste JSON
  end

  U->>A: click Confirm
  alt form
    A-->>PM: emit confirm('form', formData)
  else code
    A-->>PM: emit confirm('code', codeData)
  end
  PM->>PM: handleConfirm(addType, data) → emit confirm(...) and close modal
  U->>A: click Cancel
  A-->>PM: emit cancel()
  PM->>PM: close modal
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • hexqi

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description is currently empty or missing, so it does not convey any information about the changeset and leaves reviewers without context. Please add a concise description summarizing the purpose, scope, and key changes of the new McpAddForm component and its documentation to give reviewers clear context.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “feat: mcp-add-form component and docs” succinctly and accurately highlights the main change—adding the new McpAddForm component along with its documentation—providing clear context for reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

I nibble props and toggle modes,
From form to code my whisker glows.
Radios hum, the confirm sings,
Docs and demos sprout new things.
Hop, patch, deploy — the rabbit brings! 🥕🐇

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d9bcd5e and 7d77acf.

📒 Files selected for processing (2)
  • docs/src/components/mcp-add-form.md (1 hunks)
  • docs/src/components/mcp-server-picker.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • docs/src/components/mcp-add-form.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/src/components/mcp-server-picker.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: e2e-tests
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (13)
packages/components/src/mcp-add-form/components/CodeEditor.vue (2)

9-9: Improve accessibility and editor UX for textarea.

Add label semantics and disable browser auto-corrections for code input.

-        <textarea v-model="codeData" class="code-editor__textarea" placeholder="请输入 JSON 配置..."></textarea>
+        <textarea
+          v-model="codeData"
+          class="code-editor__textarea"
+          placeholder="请输入 JSON 配置..."
+          aria-label="JSON 配置"
+          spellcheck="false"
+          autocapitalize="off"
+          autocorrect="off"
+          autocomplete="off"
+        ></textarea>

43-55: Use a monospaced font for code and align with theming tokens.

This is a code editor; default UI fonts hinder readability. Also consider a CSS var fallback.

   &__textarea {
     width: 100%;
     height: 360px;
     border: none;
     outline: none;
     resize: none;
     padding: 12px;
-    font-size: 13px;
+    font-size: 13px;
+    font-family: var(--tr-font-mono, ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace);
     line-height: 1.4;
-    color: #191919;
-    background-color: #f5f5f5;
+    color: var(--tr-codeeditor-fg, #191919);
+    background-color: var(--tr-codeeditor-bg, #f5f5f5);
     box-sizing: border-box;
packages/components/src/mcp-add-form/index.type.ts (2)

8-11: Consider typing headers as a map, not a string.

If headers are key-value pairs, prefer Record<string, string> for safer downstream usage; accept JSON string only if you must preserve raw text.

-  headers: string
+  headers: Record<string, string>

24-29: Tighten confirm payload typing to avoid mode/data mismatches.

A discriminated union reduces runtime checks for consumers.

+export type McpAddFormConfirmPayload =
+  | { type: 'form'; data: McpAddFormData }
+  | { type: 'code'; data: string }
+
 export interface McpAddFormEmits {
   (e: 'update:addType', value: AddType): void
-  (e: 'confirm', type: AddType, data: McpAddFormData | string): void
+  (e: 'confirm', payload: McpAddFormConfirmPayload): void
   (e: 'cancel'): void
 }
docs/demos/mcp-add-form/basic.vue (1)

1-7: Showcase events in the basic demo to improve discoverability.

Hook up confirm/cancel so users see emitted payloads.

 <template>
-  <TrMcpAddForm />
+  <TrMcpAddForm @confirm="onConfirm" @cancel="onCancel" />
 </template>

 <script setup lang="ts">
 import { TrMcpAddForm } from '@opentiny/tiny-robot'
+
+function onConfirm(type: 'form' | 'code', data: unknown) {
+  console.log('confirm:', type, data)
+}
+function onCancel() {
+  console.log('cancel')
+}
 </script>
packages/components/src/mcp-add-form/components/FormEditor.vue (4)

33-58: Silent validation failures; surface feedback or emit an error

Invalid type/oversize files exit early without UX feedback. Consider emitting a validation event so parents can notify users.

Apply this diff:

+const emit = defineEmits<{
+  (e: 'validationError', message: string): void
+}>()
 ...
   // 验证文件类型
   if (!file.type.startsWith('image/')) {
-    return
+    emit('validationError', '仅支持图片文件')
+    return
   }
 ...
   if (file.size > maxSize) {
-    return
+    emit('validationError', '图片大小不能超过 5MB')
+    return
   }

109-116: Make upload area keyboard-accessible

Use button semantics so users can tab/enter to trigger upload.

-  <div class="form-editor__file-upload" @click="handleOpenFileDialog">
+  <div
+    class="form-editor__file-upload"
+    role="button"
+    tabindex="0"
+    aria-label="上传缩略图"
+    @click="handleOpenFileDialog"
+    @keydown.enter.prevent="handleOpenFileDialog"
+  >

283-293: Remove stray selector referencing non-existent .plugin-editor__footer

This appears to be a leftover and won’t match anything here.

-    .plugin-editor__footer > .button {
-      flex: 1;
-    }

9-10: Consider bundling the placeholder image locally

A remote defaultImageUrl adds an external dependency and can introduce latency or CSP issues. Prefer a local asset import.

Example:

-const defaultImageUrl = 'https://res.hc-cdn.com/tinyui-design/1.1.0.20250526191525/home/images/tiny-ng.svg'
+import defaultLogo from '../assets/svgs/default-plugin.svg'
+const defaultImageUrl = defaultLogo
packages/components/src/mcp-add-form/index.vue (4)

25-28: Verify TinyRadioGroup options shape to ensure v-model works

Same concern as FormEditor: using { label, text } may not bind as expected if the component expects { value, text } (or similar).

If required, switch to { value, text }:

-const addTypeOptions = [
-  { label: 'form', text: '表单添加' },
-  { label: 'code', text: '代码添加' },
-]
+const addTypeOptions = [
+  { value: 'form', text: '表单添加' },
+  { value: 'code', text: '代码添加' },
+]

And pass a value-field prop if TinyRadioGroup supports custom keys.

Also applies to: 54-58


70-77: Use real buttons for accessibility and semantics

Divs with click handlers aren’t focusable by default and miss keyboard/ARIA behavior.

-    <div class="mcp-add-form__footer">
-      <div class="button cancel" @click="handleCancel">
-        <span>取消</span>
-      </div>
-      <div class="button confirm" @click="handleConfirm">
-        <span>确定</span>
-      </div>
-    </div>
+    <div class="mcp-add-form__footer">
+      <button type="button" class="button cancel" @click="handleCancel">
+        <span>取消</span>
+      </button>
+      <button type="button" class="button confirm" @click="handleConfirm">
+        <span>确定</span>
+      </button>
+    </div>

22-24: Optional: simplify addType sync with defineModel

If you want two-way binding with parents, defineModel('addType') reduces boilerplate and keeps props/emits consistent.

-const addType = ref<AddType>(props.addType || 'form')
+const addType = defineModel<AddType>('addType', { default: 'form' })
 ...
-const handleUpdateAddType = (type: AddType) => {
-  emit('update:addType', type)
-}
+const handleUpdateAddType = (type: AddType) => emit('update:addType', type)

Also applies to: 42-44


34-40: Emit immutable payloads to avoid accidental downstream mutation

Clone objects before emitting confirm, so consumers can’t mutate your internal refs by reference.

 if (addType.value === 'form') {
-  emit('confirm', 'form', formData.value)
+  emit('confirm', 'form', structuredClone(formData.value))
 } else {
   emit('confirm', 'code', codeData.value)
 }

If structuredClone isn’t available in your targets, use a shallow clone: { ...formData.value }.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a3145c and 05ef770.

📒 Files selected for processing (10)
  • docs/.vitepress/config.mts (1 hunks)
  • docs/demos/mcp-add-form/basic.vue (1 hunks)
  • docs/src/components/mcp-add-form.md (1 hunks)
  • packages/components/src/index.ts (4 hunks)
  • packages/components/src/mcp-add-form/components/CodeEditor.vue (1 hunks)
  • packages/components/src/mcp-add-form/components/FormEditor.vue (1 hunks)
  • packages/components/src/mcp-add-form/components/index.ts (1 hunks)
  • packages/components/src/mcp-add-form/index.ts (1 hunks)
  • packages/components/src/mcp-add-form/index.type.ts (1 hunks)
  • packages/components/src/mcp-add-form/index.vue (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/components/src/mcp-add-form/index.ts (1)
packages/components/src/index.ts (3)
  • McpAddForm (119-119)
  • McpAddForm (120-120)
  • install (68-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: e2e-tests
🔇 Additional comments (7)
docs/.vitepress/config.mts (1)

72-73: LGTM: sidebar entry correctly added under components.

Link target matches section base; no issues spotted.

packages/components/src/mcp-add-form/components/index.ts (1)

1-2: LGTM: concise barrel exports.

Re-exports look correct and match filenames.

packages/components/src/index.ts (1)

21-22: LGTM: component exposure and install integration are consistent.

  • Added import/export/type re-exports for McpAddForm.
  • Included in components array for global installer.
  • Named exports provide both base and Tr alias.

Also applies to: 38-39, 64-65, 119-121

packages/components/src/mcp-add-form/index.ts (1)

4-12: LGTM with a minor note on naming.

Explicitly setting name = 'TrMcpAddForm' aligns with the root installer’s prefixing strategy; safe even if used standalone via app.use(McpAddForm).

packages/components/src/mcp-add-form/components/FormEditor.vue (2)

25-31: Good lifecycle hygiene on preview URLs

Reclaiming object URLs on replacement and unmount prevents leaks. Nice.

Also applies to: 65-67


15-18: Confirm and align option key mapping
TinyRadioGroup expects option objects with a value key (or id/key) for the bound value and a display key (label by default, or text if you set text-attribute="text"). Your current options use label as the value and text as the display, which mismatches the default mapping and will break v-model binding. Either:

• Rename each object to { value: 'sse', label: '服务器发送事件(SSE)' } (and likewise for 'streamableHttp'),
• Or configure <tiny-radio-group> with value-attribute="label" and text-attribute="text" to preserve your existing keys.

packages/components/src/mcp-add-form/index.vue (1)

11-21: Reasonable defaults and local state initialization

Good use of defaults for formData and codeData; keeps the component usable without strict prop requirements.

Comment thread docs/src/components/mcp-add-form.md
Comment thread docs/src/components/mcp-add-form.md
Comment thread packages/components/src/mcp-add-form/components/CodeEditor.vue
Comment thread packages/components/src/mcp-add-form/components/CodeEditor.vue
Comment thread packages/components/src/mcp-add-form/components/FormEditor.vue
Comment thread packages/components/src/mcp-add-form/index.vue Outdated
@SonyLeo SonyLeo marked this pull request as draft September 8, 2025 03:40
Comment thread docs/.vitepress/config.mts Outdated
Comment thread packages/components/src/mcp-add-form/index.vue Outdated
@SonyLeo SonyLeo marked this pull request as ready for review September 9, 2025 02:12
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/components/src/mcp-add-form/index.vue (1)

11-12: defineEmits usage is now correct

The previous invalid import of defineEmits has been resolved. Good fix.

🧹 Nitpick comments (7)
packages/components/src/mcp-server-picker/components/PluginModal.vue (2)

23-31: Tighten confirm payload typing and propagate union upstream

Use a discriminated union for the emitted payload to match the parent’s expected PluginCreationData and gain exhaustiveness checking.

Apply:

-import type { PluginModalEmits, PluginFormData } from '../index.type'
+import type { PluginModalEmits, PluginFormData } from '../index.type'

-const handleConfirm = (addType: 'form' | 'code', formData: PluginFormData | string) => {
+type CreationPayload =
+  | { type: 'form'; data: PluginFormData }
+  | { type: 'code'; data: string }
+
+const handleConfirm = (addType: 'form' | 'code', formData: PluginFormData | string) => {
   if (addType === 'form') {
-    emit('confirm', 'form', formData)
+    emit('confirm', 'form', formData as PluginFormData)
   } else {
-    emit('confirm', 'code', formData)
+    emit('confirm', 'code', formData as string)
   }

Please confirm that mcp-server-picker/index.vue’s handler signature expects the same discriminated shape (type + data) to avoid TS widening of the payload.


43-44: Consider exposing/controlling addType via v-model for consistency

If the modal needs to preset or track addType, forward it with v-model and sync events.

-<McpAddForm @confirm="handleConfirm" @cancel="handleClose" />
+<McpAddForm
+  v-model:addType="/* local ref if needed */"
+  @confirm="handleConfirm"
+  @cancel="handleClose"
/>

Also verify clicks on dropdowns/popovers inside McpAddForm don’t trigger onClickOutside closing the modal. If they do, add ignore targets to onClickOutside.

packages/components/src/mcp-add-form/index.vue (5)

13-26: Avoid stale local copies; make formData/codeData controlled or sync from props

Local refs are initialized from props but never updated if props change later. Either treat them explicitly as internal state (documented) or add two-way bindings/watchers.

Option A (recommended): expose two-way bindings to parent for controlled usage.

-const formData = ref<McpAddFormData>(props.formData || { ... })
-const codeData = ref<string>(props.codeData || '')
+import { computed } from 'vue'
+const formData = computed<McpAddFormData>({
+  get: () => props.formData || { name: '', description: '', type: 'sse', url: '', headers: '', thumbnail: null },
+  set: (v) => emit('update:formData', v),
+})
+const codeData = computed<string>({
+  get: () => props.codeData || '',
+  set: (v) => emit('update:codeData', v),
+})

Option B: if intended as “initial value only,” add a brief prop JSDoc and a watch to resync on explicit parent updates.

Please confirm how you want these to behave (controlled vs. uncontrolled) so we can align the API/docs.


25-26: Use computed get/set for addType instead of duplicating state + update handler

Simplifies syncing and avoids drift between prop and local state.

-import { ref } from 'vue'
+import { ref, computed } from 'vue'
...
-const addType = ref<AddType>(props.addType)
+const addType = computed<AddType>({
+  get: () => props.addType,
+  set: (v) => emit('update:addType', v),
+})
...
-const handleUpdateAddType = (type: AddType) => {
-  emit('update:addType', type)
-}
+// handler no longer needed
...
-<tiny-radio-group
-  v-model="addType"
-  type="button"
-  :options="addTypeOptions"
-  @update:model-value="handleUpdateAddType"
-/>
+<tiny-radio-group v-model="addType" type="button" :options="addTypeOptions" />

Also applies to: 44-46, 55-60


36-42: Add minimal validation and button states

Prevent empty submissions and give quick UX feedback (e.g., disable “确定” when invalid).

-const handleConfirm = () => {
+const handleConfirm = () => {
   if (addType.value === 'form') {
+    if (!formData.value.name?.trim() || !formData.value.url?.trim()) return
     emit('confirm', 'form', formData.value)
   } else {
+    if (!codeData.value?.trim()) return
     emit('confirm', 'code', codeData.value)
   }
 }

And:

-<div class="button confirm" @click="handleConfirm">
+<div class="button confirm" :aria-disabled="(addType==='form' && (!formData.name || !formData.url)) || (addType==='code' && !codeData)"
+     :style="{ opacity: ((addType==='form' && (!formData.name || !formData.url)) || (addType==='code' && !codeData)) ? 0.5 : 1 }"
+     @click="handleConfirm">

Also applies to: 72-79


49-61: Minor: prefer PascalCase in template for imported components

Using makes the local registration explicit and avoids occasional lint false-positives.

-<tiny-radio-group ... />
+<TinyRadioGroup ... />

83-147: Avoid global :root CSS var injection; scope defaults to the component

Writing defaults into :root can cause unexpected overrides app-wide. Define defaults under the component scope and keep them overridable at higher scopes if needed.

-:root {
-  each(@vars, {
-    --@{prefix}-@{key}: @{value};
-  });
-
-  each(@mobile-vars, {
-    --@{prefix}-@{key}: @{value};
-  });
-}
+.mcp-add-form {
+  each(@vars, {
+    --@{prefix}-@{key}: @{value};
+  });
+}
+
+@media (max-width: 768px) {
+  .mcp-add-form {
+    each(@mobile-vars, {
+      --@{prefix}-@{key}: @{value};
+    });
+  }
+}

If you need global theming, document optional overrides at :root while keeping component-scoped defaults by default.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05ef770 and 8cd716c.

📒 Files selected for processing (6)
  • docs/demos/mcp-add-form/basic.vue (1 hunks)
  • docs/src/components/mcp-server-picker.md (3 hunks)
  • packages/components/src/mcp-add-form/index.vue (1 hunks)
  • packages/components/src/mcp-server-picker/components/PluginModal.vue (6 hunks)
  • packages/components/src/mcp-server-picker/components/index.ts (0 hunks)
  • packages/components/src/mcp-server-picker/index.vue (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/components/src/mcp-server-picker/components/index.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/src/components/mcp-server-picker.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/demos/mcp-add-form/basic.vue
🔇 Additional comments (7)
packages/components/src/mcp-server-picker/index.vue (1)

397-399: Fluid width refactor looks good

Switching to max-width: 482px with width: 100% improves responsiveness and aligns with the drawer animation based on element width.

packages/components/src/mcp-server-picker/components/PluginModal.vue (4)

6-6: Good encapsulation by delegating to McpAddForm

Replacing inline editors with reduces duplication and centralizes add-flow UI/logic.


62-64: Responsive modal width change — good

max-width: 570px; width: 100% improves adaptability without breaking layout.


94-95: Header padding tweak is sensible

Reducing bottom padding aligns better with embedded form spacing.


121-125: Mobile header padding — good responsiveness

Nice touch for small screens; keeps visual balance.

packages/components/src/mcp-add-form/index.vue (2)

149-188: CSS variable mapping layer is clean

Good separation between token vars and component vars; names are consistent.


260-281: Mobile layout tweaks look good

Column layout for add-type and adjusted paddings improve small-screen UX.

Comment thread packages/components/src/mcp-add-form/index.vue
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
packages/components/src/mcp-add-form/index.vue (1)

27-30: TinyRadioGroup options should use value (not label) for v-model binding

Rename to { value, text } to match the component’s options contract.

-const addTypeOptions = [
-  { label: 'form', text: '表单添加' },
-  { label: 'code', text: '代码添加' },
-]
+const addTypeOptions: Array<{ value: AddType; text: string }> = [
+  { value: 'form', text: '表单添加' },
+  { value: 'code', text: '代码添加' },
+]
What is the expected item shape for OpenTiny TinyRadioGroup options? Does v-model bind to item.value?
🧹 Nitpick comments (1)
packages/components/src/mcp-add-form/index.vue (1)

222-257: Add focus-visible styles for keyboard users

Provide a visible focus indicator on action buttons.

     & > .button {
@@
       box-sizing: border-box;
+      &:focus-visible {
+        outline: 2px solid #5e9ed6;
+        outline-offset: 2px;
+      }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8cd716c and 1259e9d.

📒 Files selected for processing (2)
  • packages/components/src/mcp-add-form/components/FormEditor.vue (6 hunks)
  • packages/components/src/mcp-add-form/index.vue (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/components/src/mcp-add-form/components/FormEditor.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: e2e-tests
🔇 Additional comments (2)
packages/components/src/mcp-add-form/index.vue (2)

63-70: Confirm desired behavior: v-if unmounts editors (state loss) vs v-show (state preserved)

If you want to retain editor instance/scroll/undo across switching, prefer v-show.


83-147: Nice theming architecture

Good two-layer variable mapping with mobile overrides; aligns with prior feedback to avoid hardcoding container bg/width/radius.

Also applies to: 149-188

Comment thread packages/components/src/mcp-add-form/index.vue
Comment thread packages/components/src/mcp-add-form/index.vue
Comment thread packages/components/src/mcp-add-form/index.vue
Comment thread docs/src/components/mcp-add-form.md
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
packages/components/src/mcp-add-form/components/FormEditor.vue (1)

247-254: Broken icon path in background-image

Path likely off by one directory from this file. Use project alias to avoid fragility.
Apply either fix:

-    background-image: url('../../assets/svgs/edit.svg');
+    /* Option A: correct relative path */
+    background-image: url('../assets/svgs/edit.svg');
-    background-image: url('../../assets/svgs/edit.svg');
+    /* Option B: use alias for stability */
+    background-image: url('~@/packages/components/src/assets/svgs/edit.svg');

Adjust alias to match your build config.

packages/components/src/mcp-add-form/index.vue (4)

25-26: Proxy addType via computed v-model; remove manual handler to prevent desync

Local ref won’t update if parent changes prop. Use a computed getter/setter and drop the event listener.

-import { ref } from 'vue'
+import { ref, computed } from 'vue'
@@
-const addType = ref<AddType>(props.addType)
+const addType = computed<AddType>({
+  get: () => props.addType,
+  set: (v) => emit('update:addType', v),
+})
@@
-const handleUpdateAddType = (type: AddType) => {
-  emit('update:addType', type)
-}
+// (no longer needed)
@@
         <tiny-radio-group
           v-model="addType"
           type="button"
           :options="addTypeOptions"
-          @update:model-value="handleUpdateAddType"
         />

Also applies to: 44-46, 55-61


72-79: Use real buttons for accessibility and keyboard support

Clickable divs miss semantics/focus. Swap to buttons.

-    <div class="mcp-add-form__footer">
-      <div class="button cancel" @click="handleCancel">
-        <span>取消</span>
-      </div>
-      <div class="button confirm" @click="handleConfirm">
-        <span>确定</span>
-      </div>
-    </div>
+    <div class="mcp-add-form__footer">
+      <button type="button" class="button cancel" @click="handleCancel">
+        <span>取消</span>
+      </button>
+      <button type="button" class="button confirm" @click="handleConfirm">
+        <span>确定</span>
+      </button>
+    </div>

CSS selectors already target .button, so styles should carry over.


13-22: Avoid mutating parent props: deep-clone formData for local edits

Current code aliases props.formData. Edits leak even on Cancel.

-const formData = ref<McpAddFormData>(
-  props.formData || {
+const formData = ref<McpAddFormData>(
+  (props.formData ? structuredClone(props.formData) : {
     name: '',
     description: '',
     type: 'streamableHttp',
     url: '',
     headers: '',
     thumbnail: null,
-  },
+  }),
 )

Optionally watch for external prop changes to resync. I can add that if needed.


27-30: Replace label with value in addTypeOptions to match TinyRadioGroup API
TinyRadioGroup options must use a value key (not label); v-model will bind to each option’s value.

 const addTypeOptions = [
-  { label: 'form', text: '表单添加' },
-  { label: 'code', text: '代码添加' },
+  { value: 'form', text: '表单添加' },
+  { value: 'code', text: '代码添加' },
 ]
🧹 Nitpick comments (3)
packages/components/src/mcp-add-form/components/FormEditor.vue (3)

39-47: Surface feedback for invalid file type/size instead of silent return

Early returns give no UX signal. Emit an error event or show a hint/toast.

Example:

-    if (!file.type.startsWith('image/')) {
-      return
-    }
+    if (!file.type.startsWith('image/')) {
+      // emit('file-error', '仅支持图片文件')
+      return
+    }
@@
-    if (file.size > maxSize) {
-      return
-    }
+    if (file.size > maxSize) {
+      // emit('file-error', '图片大小不能超过 5MB')
+      return
+    }

If you want, I can wire up an emits type and minimal inline tip.


288-298: Dead CSS: .plugin-editor__footer rule won’t match within scoped FormEditor

This selector doesn’t exist in this template and is scoped, so it never applies. Remove or move to the owning component.

-  @media (max-width: 768px) {
-    &__item {
-      flex-direction: column;
-      align-items: flex-start;
-      gap: 8px;
-    }
-
-    .plugin-editor__footer > .button {
-      flex: 1;
-    }
-  }
+  @media (max-width: 768px) {
+    &__item {
+      flex-direction: column;
+      align-items: flex-start;
+      gap: 8px;
+    }
+  }

256-279: Unused selectors (&__file-input, &__file-label)

Not used in the template; consider removing to reduce CSS footprint.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1259e9d and d9bcd5e.

📒 Files selected for processing (2)
  • packages/components/src/mcp-add-form/components/FormEditor.vue (6 hunks)
  • packages/components/src/mcp-add-form/index.vue (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-27T03:35:11.008Z
Learnt from: gene9831
PR: opentiny/tiny-robot#59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the `show` computed property has a custom setter that prevents state mutations when `props.trigger === 'manual'`. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.

Applied to files:

  • packages/components/src/mcp-add-form/index.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: e2e-tests
🔇 Additional comments (2)
packages/components/src/mcp-add-form/components/FormEditor.vue (1)

64-67: Good: object URL cleanup on unmount

Prevents leaks when switching routes/components.

packages/components/src/mcp-add-form/index.vue (1)

36-42: Confirm/Cancel emit shape looks good

Emits align with the documented API: ('form', McpAddFormData) | ('code', string).

Comment thread packages/components/src/mcp-add-form/components/FormEditor.vue
@gene9831 gene9831 merged commit 7509a79 into opentiny:develop Sep 9, 2025
1 check passed
@SonyLeo SonyLeo deleted the component/mcp-add-form branch December 23, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants