Skip to content

Conversation

@kimteayon
Copy link
Collaborator

@kimteayon kimteayon commented Dec 8, 2025

中文版模板 / Chinese template

🤔 This is a ...

  • 🆕 New feature
  • 🐞 Bug fix
  • 📝 Site / documentation improvement
  • 📽️ Demo improvement
  • 💄 Component style improvement
  • 🤖 TypeScript definition improvement
  • 📦 Bundle size optimization
  • ⚡️ Performance optimization
  • ⭐️ Feature enhancement
  • 🌐 Internationalization
  • 🛠 Refactoring
  • 🎨 Code style optimization
  • ✅ Test Case
  • 🔀 Branch merge
  • ⏩ Workflow
  • ⌨️ Accessibility improvement
  • ❓ Other (about what?)

🔗 Related Issues

  • Describe the source of related requirements, such as links to relevant issue discussions.
  • For example: close #xxxx, fix #xxxx

💡 Background and Solution

  • The specific problem to be addressed.
  • List the final API implementation and usage if needed.
  • If there are UI/interaction changes, consider providing screenshots or GIFs.

📝 Change Log

Language Changelog
🇺🇸 English
🇨🇳 Chinese Sender 修复 onchange 缺少 skill 参数问题 ,优化技能时 placeholder 展示逻辑

Summary by CodeRabbit

  • 改进
    • onChange 签名扩展以携带当前 skill,编辑器在插入/移除/合并/清空技能及内容变更时即时触发更新,技能状态与编辑值同步更可靠。
    • 技能显示结构、占位与垂直对齐优化,插入/移除后视觉与行为更一致。
  • 新功能
    • Sender 支持 placeholder 属性。
    • Suggestion 向渲染子组件暴露 open 状态,并支持 className/rootClassName 配置。
  • 示例
    • 示例中统一采用共享技能配置并集中管理槽位配置以改善演示交互。
  • 测试
    • 多项测试改为异步断言并统一渲染上下文以提高稳定性。

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 8, 2025

Warning

Rate limit exceeded

@kimteayon has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 7fb781c and b36a821.

📒 Files selected for processing (1)
  • packages/x/components/actions/__tests__/copy.test.tsx (1 hunks)
📝 Walkthrough

Walkthrough

将 SlotTextArea 的 skill 管理改为显式 refs(skillDomRef/skillRef),并通过扩展后的 onChange(nextValue, event, slotConfig, skill) 在插入/删除/编辑等路径统一上报完整编辑器值与 skill 信息。(不超过50字)

Changes

Cohort / File(s) 变更摘要
SlotTextArea:技能引用与上报
packages/x/components/sender/SlotTextArea.tsx
skillDomRef / skillRef 替代原有 slotInnerRef/skillDom 查找,新增 data-placeholder 支持;插入/移除/输入等路径统一调用 triggerValueChange 上报完整编辑器值及 skill,清理 refs 时触发变更;重构 getNodeTextValuegetEditorValue 以同时识别 slot/skill 节点并处理 nbsp 与格式化。
Sender 核心触发逻辑与接口
packages/x/components/sender/index.tsx, packages/x/components/sender/interface.ts
triggerValueChange 增加 skill 参数并始终更新 innerValue;统一调用 `onChange(nextValue, event, slotConfig
Skill 组件结构与样式适配
packages/x/components/sender/components/Skill.tsx, packages/x/components/sender/style/slot-textarea.ts
调整组件类名与 DOM 结构,引入 wrapper / tag / holder 容器并移动关闭控件;样式更新以支持 .skill-empty 占位渲染、占位符伪元素与 holder 布局,微调对齐与间距。
Demo 与示例调整
packages/x/components/sender/demo/slot-filling.tsx, packages/x/components/sender/demo/agent.tsx, packages/x/components/sender/demo/slot-with-suggestion.tsx
slot-filling:提取并复用 skillConfig 初始化 skill,Sender 增加 placeholder,onChange 接收第四个参数并在 skill 变 falsy 时清理本地 state;agent:引入并深拷贝 slotConfig 以切换 agent 配置;slot-with-suggestion:移除附件相关 UI/state,插入项类型由 input 改为 content,调整 Suggestion 与 onKeyDown 的交互路径。
Suggestion API / 用法拓展
packages/x/components/suggestion/index.tsx, packages/x/components/suggestion/demo/basic.tsx
RenderChildrenProps<T> 添加 open: boolean 并将 mergedOpen 传给 children;SuggestionProps 新增 className?rootClassName?,示例中暴露并打印 open 状态,rootClassName 组合包含 className
测试与工具调整
packages/x/components/actions/__tests__/*, packages/x/components/sender/__tests__/slot.test.tsx, packages/x/components/sender/__tests__/index.test.tsx, packages/x/components/sender/__tests__/skill.test.tsx
测试渲染上下文改为使用 XProvider / tests/utils,slot 测试加入 document.createRange polyfill、改为异步 waitFor 与容器查询;更新断言以匹配占位符渲染与扩展后的 onChange 调用参数;新增 Skill 组件单元测试覆盖多种交互。
依赖与包配置
package.json, packages/x/package.json, packages/x-markdown/package.json, packages/x/.dumi/theme/builtins/Sandpack/index.tsx
升级若干依赖:antd^6.0.1^6.1.0@ant-design/tools 小幅升级,@rc-component/trigger devDependency 升级等;Sandpack 中 Ant Design 版本同步更新。
导出/接口声明
packages/x/components/sender/interface.ts
onChange 接口签名由 (nextValue, event, slotConfig) 扩展为 (nextValue, event, slotConfig, skill),其他导出无变更。

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SlotTextArea
    participant SenderIndex as Sender/index
    participant DemoHandler as Demo onChange

    User->>SlotTextArea: 插入/删除/编辑(含 skill 操作)
    SlotTextArea->>SlotTextArea: 更新 skillDomRef/skillRef 与 DOM dataset
    SlotTextArea->>SenderIndex: 调用 triggerValueChange(nextValue, event, slotConfig, skill)
    SenderIndex->>SenderIndex: 更新 innerValue,slotConfig 默认化为 [](若为 falsy)
    SenderIndex->>DemoHandler: 触发 onChange(nextValue, event, slotConfig, skill)
    DemoHandler->>DemoHandler: 可根据 skill 更新示例状态(如 setSkill(undefined))
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 分钟

  • 建议重点检查:
    • packages/x/components/sender/SlotTextArea.tsx 中 skillDomRef/skillRef 的生命周期、并发与插入/删除时机(插入后立即上报、删除/清空时清理)。
    • packages/x/components/sender/index.tsx 中 triggerValueChange 对 innerValue 的更新与 slotConfig 默认化逻辑,以及对外 onChange 签名兼容性。
    • Demo 示例对新增第四个 onChange 参数的处理及在 skill 变 falsy 时的状态清理路径。
    • Suggestion 的 open 回调与 rootClassName 组合对现有使用的影响。
    • 测试中 document.createRange polyfill 与 async 重写在 CI 环境的稳定性。

诗歌

🐰 我把技能放进小槽里藏,
轻抱着 ref 在光阴旁,
回调多了好友不迷茫,
插入删去节奏忙,
胡萝卜庆祝蹦了两脚 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 拉取请求的标题准确反映了主要变更内容:修复 Sender 组件的 onChange 回调缺少 skill 参数的问题,并优化了技能插槽占位符的展示逻辑,标题简洁明了。

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.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

Preview failed

@dosubot dosubot bot added the bug Something isn't working label Dec 8, 2025
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @kimteayon, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a bug where the skill parameter was missing from the Sender component's onChange callback. It also significantly enhances the control over the send button's disabled state by introducing a new submitDisabled prop and refactoring the submission logic. These changes provide greater flexibility for developers to implement custom validation and manage the sending process more effectively, ensuring a more robust and predictable user experience. New demos have been added to showcase these capabilities.

Highlights

  • Bug Fix: Missing Skill Parameter in onChange: The onChange callback for the Sender component now correctly includes the skill parameter, resolving a previous omission.
  • Enhanced Send Button Control: Introduced a new submitDisabled prop and refactored the submission logic to allow for more granular control over when the send button is enabled or disabled, particularly useful for custom validation or asynchronous operations.
  • Refactored Submission Handling: The internal onSubmit prop has been replaced with a triggerSend function and submitDisabled prop within SlotTextArea and TextArea components, centralizing the submission trigger and its disabled state management.
  • New Demos for Send Control: Added new demo examples (disable-ctrl.tsx and disable-ctrl-slot.tsx) to illustrate how to customize the disable sending logic, both with and without slot configurations.
  • Documentation Updates: The documentation for Sender component's onSubmit and onChange props has been updated to reflect the inclusion of the skill parameter and to clarify the slotConfig parameter as non-optional. Minor corrections were also made to property names and descriptions.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 8, 2025

Deploying ant-design-x with  Cloudflare Pages  Cloudflare Pages

Latest commit: b36a821
Status: ✅  Deploy successful!
Preview URL: https://f7eac835.ant-design-x.pages.dev
Branch Preview URL: https://sender-disable.ant-design-x.pages.dev

View logs

@codecov
Copy link

codecov bot commented Dec 8, 2025

Bundle Report

Changes will decrease total bundle size by 1.51MB (-44.7%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
x-markdown-array-push 106.91kB -1.51MB (-93.38%) ⬇️

Affected Assets, Files, and Routes:

view changes for bundle: x-markdown-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
x-markdown.min.js (New) 104.16kB 104.16kB 100.0% 🚀
x-markdown.min.css (New) 2.75kB 2.75kB 100.0% 🚀
latex.js (Deleted) -513.04kB 0 bytes -100.0% 🗑️
static/KaTeX_AMS-Regular.*.ttf (Deleted) -63.63kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Regular.*.ttf (Deleted) -53.58kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Bold.*.ttf (Deleted) -51.34kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Italic.*.ttf (Deleted) -33.58kB 0 bytes -100.0% 🗑️
static/KaTeX_AMS-Regular.*.woff (Deleted) -33.52kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-BoldItalic.*.ttf (Deleted) -32.97kB 0 bytes -100.0% 🗑️
static/KaTeX_Math-Italic.*.ttf (Deleted) -31.31kB 0 bytes -100.0% 🗑️
static/KaTeX_Math-BoldItalic.*.ttf (Deleted) -31.2kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Regular.*.woff (Deleted) -30.77kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Bold.*.woff (Deleted) -29.91kB 0 bytes -100.0% 🗑️
static/KaTeX_AMS-Regular.*.woff2 (Deleted) -28.08kB 0 bytes -100.0% 🗑️
static/KaTeX_Typewriter-Regular.*.ttf (Deleted) -27.56kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Regular.*.woff2 (Deleted) -26.27kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Bold.*.woff2 (Deleted) -25.32kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Bold.*.ttf (Deleted) -24.5kB 0 bytes -100.0% 🗑️
latex.css (Deleted) -24.32kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Italic.*.ttf (Deleted) -22.36kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Italic.*.woff (Deleted) -19.68kB 0 bytes -100.0% 🗑️
static/KaTeX_Fraktur-Bold.*.ttf (Deleted) -19.58kB 0 bytes -100.0% 🗑️
static/KaTeX_Fraktur-Regular.*.ttf (Deleted) -19.57kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Regular.*.ttf (Deleted) -19.44kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-BoldItalic.*.woff (Deleted) -19.41kB 0 bytes -100.0% 🗑️
static/KaTeX_Math-Italic.*.woff (Deleted) -18.75kB 0 bytes -100.0% 🗑️
static/KaTeX_Math-BoldItalic.*.woff (Deleted) -18.67kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-Italic.*.woff2 (Deleted) -16.99kB 0 bytes -100.0% 🗑️
static/KaTeX_Main-BoldItalic.*.woff2 (Deleted) -16.78kB 0 bytes -100.0% 🗑️
static/KaTeX_Script-Regular.*.ttf (Deleted) -16.65kB 0 bytes -100.0% 🗑️
static/KaTeX_Math-Italic.*.woff2 (Deleted) -16.44kB 0 bytes -100.0% 🗑️
static/KaTeX_Math-BoldItalic.*.woff2 (Deleted) -16.4kB 0 bytes -100.0% 🗑️
static/KaTeX_Typewriter-Regular.*.woff (Deleted) -16.03kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Bold.*.woff (Deleted) -14.41kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Italic.*.woff (Deleted) -14.11kB 0 bytes -100.0% 🗑️
static/KaTeX_Typewriter-Regular.*.woff2 (Deleted) -13.57kB 0 bytes -100.0% 🗑️
static/KaTeX_Fraktur-Bold.*.woff (Deleted) -13.3kB 0 bytes -100.0% 🗑️
static/KaTeX_Fraktur-Regular.*.woff (Deleted) -13.21kB 0 bytes -100.0% 🗑️
static/KaTeX_Caligraphic-Bold.*.ttf (Deleted) -12.37kB 0 bytes -100.0% 🗑️
static/KaTeX_Caligraphic-Regular.*.ttf (Deleted) -12.34kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Regular.*.woff (Deleted) -12.32kB 0 bytes -100.0% 🗑️
static/KaTeX_Size1-Regular.*.ttf (Deleted) -12.23kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Bold.*.woff2 (Deleted) -12.22kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Italic.*.woff2 (Deleted) -12.03kB 0 bytes -100.0% 🗑️
static/KaTeX_Size2-Regular.*.ttf (Deleted) -11.51kB 0 bytes -100.0% 🗑️
static/KaTeX_Fraktur-Bold.*.woff2 (Deleted) -11.35kB 0 bytes -100.0% 🗑️
static/KaTeX_Fraktur-Regular.*.woff2 (Deleted) -11.32kB 0 bytes -100.0% 🗑️
static/KaTeX_Script-Regular.*.woff (Deleted) -10.59kB 0 bytes -100.0% 🗑️
static/KaTeX_Size4-Regular.*.ttf (Deleted) -10.36kB 0 bytes -100.0% 🗑️
static/KaTeX_SansSerif-Regular.*.woff2 (Deleted) -10.34kB 0 bytes -100.0% 🗑️
static/KaTeX_Script-Regular.*.woff2 (Deleted) -9.64kB 0 bytes -100.0% 🗑️
static/KaTeX_Caligraphic-Bold.*.woff (Deleted) -7.72kB 0 bytes -100.0% 🗑️
static/KaTeX_Caligraphic-Regular.*.woff (Deleted) -7.66kB 0 bytes -100.0% 🗑️
static/KaTeX_Size3-Regular.*.ttf (Deleted) -7.59kB 0 bytes -100.0% 🗑️
static/KaTeX_Caligraphic-Bold.*.woff2 (Deleted) -6.91kB 0 bytes -100.0% 🗑️
static/KaTeX_Caligraphic-Regular.*.woff2 (Deleted) -6.91kB 0 bytes -100.0% 🗑️
static/KaTeX_Size1-Regular.*.woff (Deleted) -6.5kB 0 bytes -100.0% 🗑️
static/KaTeX_Size2-Regular.*.woff (Deleted) -6.19kB 0 bytes -100.0% 🗑️
static/KaTeX_Size4-Regular.*.woff (Deleted) -5.98kB 0 bytes -100.0% 🗑️
static/KaTeX_Size1-Regular.*.woff2 (Deleted) -5.47kB 0 bytes -100.0% 🗑️
static/KaTeX_Size2-Regular.*.woff2 (Deleted) -5.21kB 0 bytes -100.0% 🗑️
static/KaTeX_Size4-Regular.*.woff2 (Deleted) -4.93kB 0 bytes -100.0% 🗑️
static/KaTeX_Size3-Regular.*.woff (Deleted) -4.42kB 0 bytes -100.0% 🗑️
static/KaTeX_Size3-Regular.*.woff2 (Deleted) -3.62kB 0 bytes -100.0% 🗑️

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

本次 PR 修复了 Sender 组件 onChange 回调缺少 skill 参数的问题,并对提交逻辑进行了重构,使其更加灵活和清晰。主要变更包括:在 SlotTextArea 中,onChange 回调现在会正确传递 skill 参数;onSubmit 事件现在会传递完整的结构化数据(包括 slotConfigskill);对 onKeyDown 事件处理进行了优化;对组件的 CSS 结构进行了改进,引入了 ant-sender-main 类,使样式更模块化;并更新了相关测试和文档。整体来看,这些改动质量很高,不仅修复了 bug,还提升了代码的可维护性。我只发现了一个可以简化代码的小地方,请查看具体评论。

I am having trouble creating individual review comments. Click here to see my feedback.

packages/x/components/sender/TextArea.tsx (123-136)

medium

此处的 if (key === 'Enter') 判断是多余的,因为在第 118 行的 if 条件中已经判断了 key !== 'Enter' 的情况。如果代码执行到这里,key 的值必然是 'Enter'。移除这个多余的判断可以使代码更简洁。

    const isModifierPressed = ctrlKey || altKey || metaKey;
    const shouldSubmit =
      (submitType === 'enter' && !shiftKey && !isModifierPressed) ||
      (submitType === 'shiftEnter' && shiftKey && !isModifierPressed);

    if (shouldSubmit) {
      e.preventDefault();
      if (!submitDisabled) {
        triggerSend?.();
      }
      return;
    }

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

size-limit report 📦

Path Size
packages/x/dist/antdx.min.js 439.46 KB (-140 B 🔽)
packages/x-sdk/dist/x-sdk.min.js 7.37 KB
packages/x-markdown/dist/x-markdown.min.js 28.85 KB
packages/x-markdown/dist/plugins/latex.min.js 61.95 KB

Copy link
Contributor

@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: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/x/components/sender/SlotTextArea.tsx (1)

460-479: skillRef 赋值顺序可能导致状态不一致。

第 462 行设置 skillRef.current = skill,但紧接着第 463 行调用 removeSkill() 会将 skillRef.current 重置为 null(见第 486 行)。建议将 skillRef.current 的赋值移到 removeSkill() 调用之后,以正确反映当前 skill 状态。

  const insertSkill = () => {
    if (slotInnerRef.current && skill) {
-      skillRef.current = skill;
      removeSkill();
+      skillRef.current = skill;
      const skillSpan = buildSkillSpan(skill.value);
      // ... rest of the code
🧹 Nitpick comments (1)
packages/x/components/sender/demo/agent.tsx (1)

196-196: 变量命名可能引起混淆。

状态变量 slotConfig 实际存储的是整个 AgentInfo 对象(包含 skillzh_skillslotConfigzh_slotConfig 等),与内部属性 slotConfig 同名容易产生歧义。建议使用更明确的命名如 activeAgentConfig

-  const [slotConfig, setSlotConfig] = useState(AgentInfo[activeAgentKey]);
+  const [activeAgentConfig, setActiveAgentConfig] = useState(AgentInfo[activeAgentKey]);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dba20d0 and 6dab394.

📒 Files selected for processing (5)
  • packages/x/components/sender/SlotTextArea.tsx (8 hunks)
  • packages/x/components/sender/components/Skill.tsx (1 hunks)
  • packages/x/components/sender/demo/agent.tsx (5 hunks)
  • packages/x/components/sender/demo/slot-filling.tsx (2 hunks)
  • packages/x/components/sender/index.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/x/components/sender/components/Skill.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/x/components/sender/demo/slot-filling.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/x/components/sender/index.tsx (1)
packages/x/components/sender/interface.ts (1)
  • SenderProps (124-158)
packages/x/components/sender/SlotTextArea.tsx (1)
packages/x/components/sender/interface.ts (1)
  • SkillType (54-65)
⏰ 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). (5)
  • GitHub Check: build preview
  • GitHub Check: size
  • GitHub Check: test
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (8)
packages/x/components/sender/index.tsx (1)

146-149: LGTM!

triggerValueChange 签名正确扩展以包含 skill 参数,与 SenderProps['onChange'] 接口定义一致。slotConfig ?? [] 提供了合理的空数组兜底。

packages/x/components/sender/demo/agent.tsx (2)

238-243: JSON 深拷贝在此场景可行,但需注意局限性。

JSON.parse(JSON.stringify()) 无法处理 undefinedfunctionSymbol 和循环引用。当前 AgentInfo 中的 skillslotConfig 数据是可序列化的,此方法可行。但若未来数据结构包含不可序列化类型,建议改用 structuredClone() 或引入如 lodash.cloneDeep 的工具。


288-288: 配置访问方式统一化。

通过状态管理集中访问 agent 配置,使得切换 agent 时能正确更新 skillslotConfig。逻辑正确。

Also applies to: 350-350, 356-356, 419-419

packages/x/components/sender/SlotTextArea.tsx (5)

85-86: LGTM!

引入 skillDomRefskillRef 显式跟踪 skill 的 DOM 节点和数据状态,替代之前的隐式 DOM 操作方式,使状态管理更加清晰可控。


481-489: 移除技能时正确清理状态并通知父组件。

添加了 skillDomRef.current 的空检查作为守卫,避免对 null 调用 remove()。清理 ref 并调用 onChange 确保父组件同步接收到 skill 被移除的状态。


848-852: skill 变为空值时正确同步状态。

当外部 skill prop 变为 falsy 时,通过 onChange 通知父组件当前编辑器的完整状态,确保状态同步。


648-657: 核心修复:onInternalInput 现在正确传递 skill 参数。

这是 PR 的核心修复点,确保在用户输入时 onChange 回调能正确接收到 skill 参数。


123-130: 为 skill span 添加 data-placeholder 属性。

支持通过 CSS 伪元素显示占位符文本,提升用户体验。

Copy link
Contributor

@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 (2)
packages/x/components/sender/SlotTextArea.tsx (2)

881-884: useEffect 依赖数组中使用 slotInnerRef.current 依然不会按预期触发

这里的 effect:

useEffect(() => {
  if (slotInnerRef.current && skillRef.current !== skill) {
    insertSkill();
  }
}, [skill, slotInnerRef.current]);

slotInnerRef.current 是 ref 的可变属性,改变时不会触发组件重渲染,因此即便放入依赖数组也不会让 effect 在其变化时重新执行,反而增加了阅读成本。

同时,insertSkill 内部本身已经检查了 slotInnerRef.currentskill,可以只依赖 skill 即可:

-  useEffect(() => {
-    if (slotInnerRef.current && skillRef.current !== skill) {
-      insertSkill();
-    }
-  }, [skill, slotInnerRef.current]);
+  useEffect(() => {
+    if (slotInnerRef.current && skillRef.current !== skill) {
+      insertSkill();
+    }
+  }, [skill]);

逻辑行为不变,但依赖关系更准确,也避免给人「监听 ref.current 变化」的误解。


616-624: Backspace 删除 skill 时未使用 skill.closable,且保留了无效表达式

当前分支逻辑为:

const skillKey = (selection.anchorNode?.previousSibling as Element)?.getAttribute?.(
  'data-skill-key',
);
if (skillKey) {
  e.preventDefault();
  removeSkill();
  skill?.closable;
  return;
}

问题有两点:

  1. skill?.closable; 只是计算表达式但不使用,属于死代码;
  2. 如果 skill.closable === false 设计上是「不可关闭」,这里仍会在 Backspace 时直接调用 removeSkill(),与 UI 上是否显示关闭按钮可能不一致。

建议要么完全移除 closable 相关逻辑(如果不需要该能力),要么在删除前显式判断,例如:

-        if (skillKey) {
-          e.preventDefault();
-          removeSkill();
-          skill?.closable;
-          return;
-        }
+        if (skillKey) {
+          // 如果存在 closable 语义,可以在这里做保护
+          if (skill && skill.hasOwnProperty('closable') && !skill.closable) {
+            return;
+          }
+          e.preventDefault();
+          removeSkill();
+          return;
+        }

这样既去掉了无效表达式,也避免键盘操作绕过 closable 约束。

🧹 Nitpick comments (3)
packages/x/components/sender/style/slot-textarea.ts (3)

187-207: 技能占位符样式依赖 skill-empty 类的正确维护

这里通过 &${skillCls}-empty::after 渲染占位符,而不是像上面 slotContent 一样配合 :empty 伪类。如果运行时不严格保证只有在真正“空内容”时才加上 skill-empty 类,就会出现实际内容和占位符同时渲染的情况,建议确认对应组件逻辑中 class 的切换是否完全可靠;如有疑虑,可以改为 &${skillCls}-empty:empty::after 之类的更保守写法。

另外,这里使用的是 token.colorTextPlaceholder,而其他 slot 相关占位符多用 colorTextSlotPlaceholder;可以评估一下是否需要统一成 slot 系列的占位颜色以保持视觉一致性。


208-212: skill-wrapper 使用 height: 100% 需确认父级高度来源

&${skillCls}-wrapper 设为 display: inline-flexheight: '100%',这要求其父级有明确高度,否则在某些场景(如多行换行、不同字体行高)可能出现对不齐或高度塌陷的问题。建议在实际使用场景下(含多行文本、缩放等)确认该 wrapper 与 skill 本身高度、外围 slot 行高的一致性是否满足预期。


243-246: skill-holder 尺寸配置看起来合理,但用途可在注释中说明

&${skillCls}-holder 仅用 width: token.marginXSheight: '100%' 做一个窄占位,看起来是用于对齐或光标占位。实现本身没有明显问题,如后续团队成员对其用途不太直观,可以考虑补一行注释说明该占位元素的作用,方便维护。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6dab394 and 35f1596.

📒 Files selected for processing (3)
  • packages/x/components/sender/SlotTextArea.tsx (14 hunks)
  • packages/x/components/sender/components/Skill.tsx (3 hunks)
  • packages/x/components/sender/style/slot-textarea.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/x/components/sender/components/Skill.tsx
⏰ 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). (5)
  • GitHub Check: build preview
  • GitHub Check: test
  • GitHub Check: size
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
packages/x/components/sender/SlotTextArea.tsx (2)

85-87: 集中封装 skill 状态与触发 onChange 的整体设计是正确的

这几处改动把 skill 相关的 DOM/状态管理都集中到 skillDomRef/skillRef + triggerValueChange 上,并统一在:

  • updateSlotremoveSlot
  • insertSkill / removeSkill
  • onInternalInput
  • clearslotConfig 变更时(无 skill 场景)

通过 getEditorValue() 计算一次值,然后由 triggerValueChange 统一调用 onChange(value, event, slotConfig, skill),同时根据内容是否为空为 skillDomRef 切换 *-skill-empty class,这样既满足「始终带上 skill」的 PR 目标,又避免多处重复拼装参数,整体实现是合理的,没发现明显逻辑问题。

Also applies to: 113-124, 135-141, 494-521, 680-683, 856-862, 875-878


431-437: 在 slot/skill 内插入时,将插入点统一移动到节点之后的方案是合理的

getInsertPosition 中新增:

if (
  endContainer?.className?.includes(`${prefixCls}-skill`) ||
  endContainer?.className?.includes(`${prefixCls}-slot`)
) {
  return { type: 'slot', range };
}

配合 insert 里:

if (type === 'slot') {
  range = selection?.getRangeAt?.(0);
  if (selection.focusNode) {
    range.setStartAfter(selection.focusNode);
  }
}

在光标位于带有 *-skill / *-slot class 的元素上时,会把插入位置统一移动到该节点之后,再批量 insertNode 新的 slot 列表。结合前面对 slot/skill 的识别逻辑,这样能避免把新内容插到组件内部(尤其是 portal 渲染出来的内容里),行为符合预期,没有看到明显的边界风险。

Also applies to: 745-816

Copy link
Contributor

@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: 0

🧹 Nitpick comments (3)
packages/x/components/sender/demo/slot-with-suggestion.tsx (1)

261-267: 考虑使用新的 skill 参数展示修复效果。

根据 PR 目标,onChange 现在应该包含 skill 参数。当前实现仅使用 value 参数:

onChange={(value) => {

根据相关文件的上下文,新的签名应为:

onChange={(value, event, slotConfig, skill) => {

虽然在 JavaScript/TypeScript 中省略未使用的参数是合法的,但作为演示文件,建议至少在注释中说明新参数的可用性,或者添加简单的日志来展示 skill 参数的传递,以更好地体现此次修复的效果。

可以考虑更新为:

-onChange={(value) => {
+onChange={(value, event, slotConfig, skill) => {
+  // skill 参数现在可用,可用于追踪当前激活的技能
  if (value?.endsWith('@')) {
    onTrigger();
  } else {
    onTrigger(false);
  }
}}

或者至少添加注释说明:

-onChange={(value) => {
+// onChange 现在提供 (value, event, slotConfig, skill) 参数
+onChange={(value) => {
  if (value?.endsWith('@')) {
    onTrigger();
  } else {
    onTrigger(false);
  }
}}
packages/x/components/suggestion/index.tsx (1)

174-184: className/rootClassName 合成逻辑略显混乱,语义可以再梳理一下

当前实现中:

  • className 从 props 解构后,仅参与 CascaderrootClassName 合成;
  • rootClassName 既参与 Cascader.rootClassName,又参与内部 content <div>className
  • 而传入的 className 并不会作用在 content <div> 上,也不会以 className 形式透传给 Cascader 本身。

这会导致:

  • 对使用者来说,classNamerootClassName 的作用范围不太直观;
  • 如果调用方期望用 className 定位「触发区域/内容容器」,现在只能通过 rootClassName 间接完成。

可以考虑更清晰地区分语义,例如(思路示例):

-      rootClassName={classnames(
-        rootClassName,
-        className,
-        classNames.root,
-        prefixCls,
-        hashId,
-        cssVarCls,
-        {
-          [`${prefixCls}-block`]: block,
-        },
-      )}
+      rootClassName={classnames(
+        rootClassName,
+        classNames.root,
+        prefixCls,
+        hashId,
+        cssVarCls,
+        {
+          [`${prefixCls}-block`]: block,
+        },
+      )}

并在 content 容器上额外挂载 className(示意):

<div
  className={classnames(
    prefixCls,
    rootClassName,
    className, // 让外部 className 直接作用在内容容器上
    contextConfig.classNames.content,
    classNames.content,
    `${prefixCls}-content`,
    hashId,
    cssVarCls,
  )}
>

上述只是一个语义更清晰的拆分方向,是否调整取决于你们对 Suggestion 的对外 API 约定,但现在的形态确实有一定混淆空间。

Also applies to: 200-209

packages/x/components/suggestion/demo/basic.tsx (1)

40-41: 示例中的 console.log 建议去掉,避免文档示例产生多余日志

在 demo 中解构 open 并使用完全没问题,不过持续保留:

console.log(open, 'suggestion open');

会让使用者在查看官方示例时控制台一直输出日志,略显打扰。建议直接移除该行,或在需要调试时临时添加即可:示例代码更干净,也更贴近真实使用场景。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b1a32bf and 30f208c.

📒 Files selected for processing (3)
  • packages/x/components/sender/demo/slot-with-suggestion.tsx (4 hunks)
  • packages/x/components/suggestion/demo/basic.tsx (1 hunks)
  • packages/x/components/suggestion/index.tsx (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/x/components/sender/demo/slot-with-suggestion.tsx (1)
packages/x/components/x-provider/demo/direction.tsx (1)
  • value (23-149)
⏰ 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). (5)
  • GitHub Check: test
  • GitHub Check: size
  • GitHub Check: build preview
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
packages/x/components/sender/demo/slot-with-suggestion.tsx (4)

206-206: 正确集成 Suggestion 的 onKeyDown 处理。

添加 onKeyDown 到解构参数中符合 Suggestion 组件的集成模式,这将支持键盘导航和选择功能。


268-268: 正确传递 onKeyDown 处理器。

onKeyDown 从 Suggestion 传递到 Sender 组件是正确的,这使得 Suggestion 可以拦截和处理键盘事件(例如导航或选择)。


13-13: No issues found. The removal of Attachments and AttachmentsProps is complete with no residual references in the file.


196-196: Change from 'input' to 'content' slot type is valid.

The 'content' type is a supported slot type in SlotConfigType union (defined in interface.ts). It follows the same API pattern as 'input' type—both require a key property and optional props object. The skill parameter is passed through callbacks (onSubmit, onChange), not through slot config, so this change maintains full API compatibility.

packages/x/components/suggestion/index.tsx (1)

21-25: open field addition to RenderChildrenProps is fully backward compatible

Adding open: boolean to RenderChildrenProps (line 24) and passing mergedOpen to children (line 146) is safe. Search of the codebase confirms that RenderChildrenProps has no external usages outside this file—it's only referenced internally at line 52 (children prop type declaration) and line 107 (onTrigger property access).

Since the interface is not consumed by external code or narrower type annotations, adding a new field introduces no TypeScript compilation errors or breaking changes. The implementation is correct as-is.

@kimteayon kimteayon changed the title fix(Sender): Sender onchange fix(Sender): Sender onchange & slot skill placeholder Dec 9, 2025
Copy link
Contributor

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
packages/x/components/sender/SlotTextArea.tsx (4)

494-521: insertSkill/removeSkill 中 onChange 触发时机需要梳理,避免中间“skill 被清空”的瞬间

当前流程:

  • insertSkill 每次插入前都会调用一次 removeSkill()removeSkill 内部会:
    • 删除现有 skill DOM;
    • skillDomRefskillRef 置空;
    • 立即 triggerValueChange(),对外表现为 “skill 被清空” 的一次 onChange。
  • 然后 insertSkill 创建新 span,设置 skillDomRef,再 triggerValueChange() 一次,把新 skill 上报出去。

结果是“切换技能”会产生两次 onChange:先发一条 skill = undefined,再发一条 skill = 新值
如果上层逻辑只是简单用 setSkill(skill) 同步状态,这个中间状态多半没问题,但:

  • onChange 调用次数翻倍,可能影响性能或埋点统计;
  • 如果上层有基于 skill 是否存在的瞬时逻辑(如立即关闭某些面板),中间那次 “skill 为空” 的回调可能导致意外行为。

建议:

  • 为“替换 skill”单独走一条路径:在有旧 DOM 时仅做 DOM 替换,不对外触发一次“清空 skill”的 onChange。
  • 如上条评论所述,可以拆分出内部 removeSkillDom() 与对外 removeSkill(),前者供 insertSkill 使用。

这样可以保持对外 onChange 语义更清晰:

  • 用户手动删除 skill:触发一次 skill = undefined
  • 用户切换 skill:只触发一次 skill = 新值

585-592: onInternalKeyDown 会重复调用 onKeyDown 且存在类型不匹配

当前实现:

const eventRes = onKeyDown?.(e);
// ...
if (keyLockRef.current || isCompositionRef.current || eventRes === false) {
  onKeyDown?.(e as unknown as React.KeyboardEvent<HTMLTextAreaElement>);
  return;
}

问题:

  1. 若满足条件(键盘锁定 / 组合输入 / 外部返回 false),onKeyDown 会被调用两次,可能导致上层逻辑执行两遍。
  2. onKeyDown 在 context 中大概率类型为 React.KeyboardEvent<HTMLTextAreaElement>,这里第一次直接用 eHTMLDivElement)调用,在 TS 下会有类型不匹配。

建议改成只调用一次,并统一 cast:

- const eventRes = onKeyDown?.(e);
+ const eventRes = onKeyDown?.(
+   e as unknown as React.KeyboardEvent<HTMLTextAreaElement>,
+ );
...
- if (keyLockRef.current || isCompositionRef.current || eventRes === false) {
-   onKeyDown?.(e as unknown as React.KeyboardEvent<HTMLTextAreaElement>);
-   return;
- }
+ if (keyLockRef.current || isCompositionRef.current || eventRes === false) {
+   return;
+ }

这样既保留 eventRes === false 的“阻止内部处理”语义,又避免了重复调用和 TS 报错。


858-868: clear 重置 skillRef/skillDomRef 后又调用 insertSkill,可能触发两次 onChange

clear 现在流程是:

  1. 清空 editableDom.innerHTML
  2. skillRef.currentskillDomRef.current 置为 null
  3. 调用 insertSkill()(内部会触发一次 triggerValueChange());
  4. 重置 slot state;
  5. 最后调用 onInternalInput(null),再次触发一次 triggerValueChange()

如果存在 skill,这个顺序意味着:

  • 清空后插入 skill → 触发一次 onChange;
  • 紧接着又通过 onInternalInput 再触发一次 onChange,payload 基本相同。

建议:

  • 要么在 clear 里只保留一次 triggerValueChange(比如依赖最后的 onInternalInput),把 insertSkill 中的回调关闭;
  • 要么在 clear 中不再调用 onInternalInput,直接依赖 insertSkill 的回调即可。

统一一个出口可以减少 onChange 调用次数,避免上层监听到两次“清空并重新挂 skill”事件。


871-880: slotConfig/skill 两个 effect 之间的协作有潜在状态不同步风险

当前两个 effect:

useEffect(() => {
  initClear();
  if (slotConfig && slotConfig.length > 0 && editableRef.current) {
    appendNodeList(...);
  }
  slotInnerRef.current = true;
  if (!skill) {
    triggerValueChange();
  }
}, [slotConfig]);

useEffect(() => {
  if (skillRef.current !== skill) {
    insertSkill();
  }
}, [skill]);

潜在问题:

  • 如果运行时仅 slotConfig 变化而 skill 保持不变(例如更新槽位配置但沿用当前 skill),[slotConfig] effect 会:
    • 调用 initClear() 把编辑区和旧 skill DOM 一起清空;
    • 重新 append slot;
    • 由于 skill 仍然存在,不会调用 triggerValueChange()
  • 同一轮 render 中 [skill] effect 不会触发(依赖未变),于是 skill 不会重新插入,父组件也收不到新的 onChange,最终表现是:
    • 视觉上 skill 消失;
    • 上层状态里 skill 还是旧值。

建议其中一种调整:

  1. [slotConfig] effect 末尾,如果 skill 存在,显式调用一次 insertSkill()(此时 slotInnerRef.current 已经被设为 true);或
  2. 改造 [skill] effect 的依赖,将 slotInnerRef 的变更纳入触发条件(但不要把 .current 直接放进依赖数组,可通过额外 state 标记 slot 初始化完成)。

简单方案示例:

useEffect(() => {
  initClear();
  ...
  slotInnerRef.current = true;
- if (!skill) {
-   triggerValueChange();
- }
+ if (skill) {
+   insertSkill();
+ } else {
+   triggerValueChange();
+ }
}, [slotConfig]);

这样在 slotConfig 变化时,无论是否有 skill,都能保证 DOM 与对外状态保持同步。

Also applies to: 883-886

🧹 Nitpick comments (7)
packages/x/components/sender/SlotTextArea.tsx (7)

113-124: triggerValueChange 统一出口不错,可考虑更严格的“空内容”判断

集中在 triggerValueChange 里计算当前 value/slotConfig/skill 并统一触发 onChange 很好,同时顺便维护 -skill-empty 类名也合理。

小建议:

  • 当前空判断为 !newValue?.value && newValue.slotConfig.length === 0,如果用户只输入若干空格,value 仍为 truthy,会被视为“有内容”,占位符不再展示。
    如希望“全是空白字符也视为无内容”,可以改为 !newValue.value.trim() 再配合 slotConfig 长度判断。

这一点属于 UX 微调,可按产品预期决定是否调整。


135-141: skill 占位文案写入 dataset 时类型需注意

buildSkillSpan 中:

span.setAttribute('contenteditable', 'true');
span.dataset.skillKey = key;
span.dataset.placeholder = placeholder;

Sender 的 placeholder 类型通常是 ReactNode,这里直接赋给 dataset 会在 placeholder 为元素或数字等情况时得到类似 [object Object] 的字符串。

建议:

if (typeof placeholder === 'string') {
  span.dataset.placeholder = placeholder;
} else if (placeholder != null) {
  span.dataset.placeholder = String(placeholder);
}

或者约束 placeholder 类型为 string,二选一即可。


351-393: getEditorValue 中 skill 判定依赖外部 skill,与 skillRef 不完全一致

getEditorValue 里 skill 的判定逻辑是:

const skillKey = el.getAttribute('data-skill-key');
if (skillKey && skill) {
  currentSkillConfig = skill;
}
...
if (!currentSkillConfig) {
  skillDomRef.current = null;
}

几点观察:

  • 这里完全依赖 context 传入的 skill,而不是 skillRef.current;而上面又有一套独立的 skillRef 维护逻辑(插入/删除时修改)。两者可能出现不一致,从而导致:
    • DOM 里仍有 skill span,但因为 skill 已被父组件清空,这里拿不到 currentSkillConfig,进而把 skillDomRef 置成 null
  • 这会让后续如 removeSkill 或基于 skillDomRef 的 class 切换出现“DOM 还在,但 ref 已丢”的情况。

建议统一数据来源,要么:

  • 完全依赖 skill 作为单一真源(那就不需要 skillRef);要么
  • 统一走 skillRef.current(在插入/删除时维护它),并在 effect 中以它为准。

当前实现功能上大体可用,但存在潜在状态不同步风险,建议后续整理 skill 的单一数据源。


431-437: 光标位于 skill/slot 内时直接视作 slot 插入位置,基本合理

这里将光标所在节点 className 包含 -skill-slot 时统一视作 type: 'slot',避免把 slot/skill 内部当成普通文本插入点,逻辑上是合理的。

不过要注意:

  • 对于更深层级(如 skill 内部 wrapper 里的子元素),focusNode 可能是子节点而不是带 -skill 类名的 span,这种情况仍然会走后面的 box/end 分支。
    若未来希望“在 skill 内部点击时都算在 skill 后插入”,可以考虑用 closest(\.${prefixCls}-skill, .${prefixCls}-slot`)而不是直接看focusNode.className`。

该段当前可以接受,是否增强为更鲁棒的 closest 逻辑可再斟酌。


617-624: Backspace 删除 skill 的逻辑建议考虑 closable 语义与事件透传

当前 Backspace 处理:

const skillKey = (selection.anchorNode?.previousSibling as Element)?.getAttribute?.(
  'data-skill-key',
);
if (skillKey) {
  e.preventDefault();
  removeSkill();
  return;
}

几点建议:

  • SkillType 里已有 closable / closable.disabled 定义,这里无条件允许 Backspace 删除 skill,和 UI 上的关闭能力可能不一致。
    若希望“不可关闭的 skill 也不可通过键盘删除”,最好在这里做一次 skill?.closable 判断。
  • 与 slot 删除不同,这里调用 removeSkill() 没有把键盘事件传下去,而 removeSkill 内部也不接受 event;如果上层希望基于事件信息做埋点或其它处理,就拿不到 Backspace 的上下文。

可选优化方案:

  • removeSkill 增加可选 event?: EventType 参数,并在 Backspace 分支按需传入;
  • 或者保持现在的签名,但在 triggerValueChange 的第二个参数上统一传递最后一次 KeyboardEvent(需要稍微重构签名)。

如果目前上层对事件本身不敏感,可以暂时维持现状,但建议至少补一条 closable 判断。


680-683: onInternalInput 改为走 triggerValueChange,行为统一

onInternalInput 现在统一通过 triggerValueChange 上报 value/slotConfig/skill,很好地保证了普通输入与 slot/skill 变更走相同管道。

唯一需要注意的是,这里将 React.FormEvent<HTMLDivElement> 强转为 EventType(textarea 事件),如果上层对事件类型有严格依赖,可能需要在 interface 里把 EventType 放宽为更通用的类型。

如果当前上层仅把第二个参数当“原始事件”透传或做日志,这样的 cast 是可以接受的。


745-818: insert 中 slot 类型分支 range 调整逻辑需留意 focusNode 场景

type === 'slot' 场景下:

range = selection?.getRangeAt?.(0);
if (selection.focusNode) {
  range.setStartAfter(selection.focusNode);
}
...
selection.deleteFromDocument();
slotNode.forEach((node) => {
  range.insertNode(node);
  range.setStartAfter(node);
  range = range.cloneRange();
});

这意味着:

  • 插槽插入位置依据 focusNode 而非包含 slot/skill 的 span,本质上是“在当前 focus 节点之后插入”。
  • 当 focusNode 是 slot/skill 的子节点(如内部的 input 或 wrapper 子元素)时,插入位置可能略偏(插进 span 内或 wrapper 后)。

如果希望语义更明确为“在当前 slot/skill 节点之后插入”,可以考虑基于:

const focusElement =
  (selection.focusNode as Node).nodeType === Node.ELEMENT_NODE
    ? (selection.focusNode as Element)
    : selection.focusNode?.parentElement;

const host = focusElement?.closest(
  `.${prefixCls}-slot, .${prefixCls}-skill`,
);

if (host) {
  range.setStartAfter(host);
}

当前实现功能上大概率可用,但语义略依赖具体 DOM 结构,后续如果插槽内部结构调整,建议一并加强这里的鲁棒性。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30f208c and 67ed850.

📒 Files selected for processing (2)
  • packages/x/components/sender/SlotTextArea.tsx (16 hunks)
  • packages/x/components/sender/style/slot-textarea.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/x/components/sender/style/slot-textarea.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/x/components/sender/SlotTextArea.tsx (1)
packages/x/components/sender/interface.ts (2)
  • SkillType (54-65)
  • EventType (121-123)
⏰ 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). (5)
  • GitHub Check: build preview
  • GitHub Check: size
  • GitHub Check: test
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
packages/x/components/sender/SlotTextArea.tsx (2)

304-348: getNodeTextValue 对 slot/skill 的解析整体合理,可关注 skill 文本截取边界

这段重写基本满足需求:

  • slot 节点通过 dataset.slotKey + slotConfigMap 还原配置,并用 ?? + 显式 formatResult 调用避免 falsy 值被吞。
  • nbsp slot 返回 ' ',保证空占位在拼接字符串时保留空格。
  • skill 节点通过查找 .${prefixCls}-skill-wrapper,用 fullText.replace(skillText, '').replace(/\n/g, '') 提取 skill 外的用户文本。

可能的边界点:

  • replace(skillText, '') 只去掉第一次出现的 wrapper 文本;若日后 skill 结构变化、wrapper 不一定在最前面时,需要重新评估这段逻辑。
  • 如果未来允许在 skill chip 前面也输入文字,fullText 就不再是“wrapper + 用户输入”的简单结构,届时这里也需要一起调整。

当前实现对现有 DOM 结构是可接受的,但后续如 Skill 组件结构调整,记得同步检查这里。

请在后续如调整 Skill 组件 DOM 结构时,同步确认这段逻辑仍然能正确剥离 wrapper 文本。


467-492: removeSlot 改为走 triggerValueChange 一致性良好

removeSlot 在清理 DOM、refs 和本地 state 后,统一通过 triggerValueChange(e) 上报当前 value/slotConfig/skill,这样可以保证 onChange 的第四个 skill 参数始终由相同路径计算,避免遗漏。

这一块改动没有明显问题。

Copy link
Contributor

@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: 2

🧹 Nitpick comments (3)
packages/x/components/actions/__tests__/copy.test.tsx (1)

1-1: 为 ActionsCopy 补齐 XProvider 上下文是合理的修复

在所有用例中用 XProvider 包裹 ActionsCopy,能够更贴近真实使用场景,避免组件在缺失上下文时出现运行时异常或行为差异,且断言本身未变,说明只是补充依赖环境,整体改动是合理的。

如果后续还有更多依赖 XProvider 的组件测试,可以考虑在 ../../../tests/utils 中增加一个 renderWithXProvider 之类的封装,减少在各个用例里重复写 <XProvider>...</XProvider> 的样板代码。

Also applies to: 8-12, 16-20, 24-28, 34-36, 43-47, 53-57

packages/x/package.json (1)

93-149: @ant-design/x 的 antd / trigger 版本基线提升整体合理,但需确认兼容与发布影响

这里有三处关键升级:

  • devDependencies:@rc-component/trigger 升级到 ^3.7.1
  • devDependencies:antd 升级到 ^6.1.0
  • peerDependencies:antd 的要求从 ^6.0.1 提升为 ^6.1.0

整体上看:

  • 将 devDependencies 和 peerDependencies 的 antd 版本对齐到 ^6.1.0 是正确的,可以避免开发环境与下游应用的 antd 行为不一致。
  • @rc-component/trigger 新大版本往往会对弹层行为(对齐、动画、交互回调等)有一些调整,这里虽然只在 devDependencies,但如果文档站 / Demo / 可视化测试依赖它,建议重点跑一下相关用例。

建议进一步确认 / 补充:

  • 从下游视角看,peer antd 最低版本提升到 ^6.1.0 意味着使用 antd 6.0.x 的项目会收到 peer 警告甚至潜在不兼容,需要确认这是有意的“支持基线升级”,并在 Changelog / Release Note 中清晰说明。
  • 针对依赖 @rc-component/trigger 的 Sender / 弹出层相关 Demo 和 tests,最好在本次 PR 或后续 CI 中确保它们在 ^3.7.1 下全部通过且交互符合预期。

如果需要,我可以帮你草拟一段发布说明,专门说明“升级要求 antd >= 6.1.0 且内部 Trigger 依赖同步到 3.7.1”。

Also applies to: 245-249

packages/x/components/sender/__tests__/slot.test.tsx (1)

417-499: 测试断言较弱,建议验证实际 Skill 内容。

当前所有 Skill 相关测试都仅检查 #ant-sender-slot-placeholders 容器是否存在,但没有验证实际的 Skill 组件是否正确渲染(例如 title、closable 按钮等)。这可能导致即使 Skill 渲染逻辑有问题,测试仍然通过。

建议增强断言,例如:

  await waitFor(() => {
    const placeholders = container.querySelector('#ant-sender-slot-placeholders');
    expect(placeholders).toBeInTheDocument();
+   // 验证 Skill 的实际内容
+   expect(placeholders?.textContent).toContain('skill_title');
  });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67ed850 and 49bb4fb.

📒 Files selected for processing (9)
  • package.json (1 hunks)
  • packages/x-markdown/package.json (1 hunks)
  • packages/x/.dumi/theme/builtins/Sandpack/index.tsx (1 hunks)
  • packages/x/components/actions/__tests__/copy.test.tsx (1 hunks)
  • packages/x/components/actions/__tests__/feedback.test.tsx (1 hunks)
  • packages/x/components/actions/__tests__/index.test.tsx (1 hunks)
  • packages/x/components/sender/SlotTextArea.tsx (16 hunks)
  • packages/x/components/sender/__tests__/slot.test.tsx (6 hunks)
  • packages/x/package.json (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-04T03:46:13.221Z
Learnt from: YumoImer
Repo: ant-design/x PR: 293
File: components/x-request/demo/custom-transformer.tsx:75-79
Timestamp: 2024-12-04T03:46:13.221Z
Learning: When reviewing code examples in `components/x-request/demo/custom-transformer.tsx` that use `mockFetch`, it's unnecessary to suggest enhancements to the `TransformStream` implementation, such as adding error handling and data validation, since the code is intended as a simple demonstration.

Applied to files:

  • packages/x/components/actions/__tests__/feedback.test.tsx
🧬 Code graph analysis (1)
packages/x/components/sender/SlotTextArea.tsx (1)
packages/x/components/sender/interface.ts (2)
  • SkillType (54-65)
  • EventType (121-123)
⏰ 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). (6)
  • GitHub Check: Socket Security: Pull Request Alerts
  • GitHub Check: build preview
  • GitHub Check: test
  • GitHub Check: size
  • GitHub Check: Cloudflare Pages
  • GitHub Check: test
🔇 Additional comments (10)
packages/x/components/actions/__tests__/index.test.tsx (1)

3-3: 统一使用本地测试工具导入是合理调整

render/fireEvent 改为从 ../../../tests/utils 导入,有利于在测试中复用统一封装的 Provider 和配置,保持测试环境一致性,当前改动看起来是安全的。

packages/x/components/actions/__tests__/feedback.test.tsx (1)

2-2: 使用统一的测试工具封装可以简化用例维护

render/fireEvent 切换到从 ../../../tests/utils 导入,有利于集中管理测试环境与公共 wrapper,保持与其他用例一致,改动本身没有发现问题。

package.json (1)

31-38: 根 devDependencies 版本对齐是合理的,但请确认工具链与 UI 库兼容性

这里把 @ant-design/tools 升级到 ^19.1.0,同时将根层 antd dev 依赖统一到 ^6.1.0,和子包保持一致是好事,有利于避免多版本冲突。

建议额外确认两点:

  • 各类基于 antd-tools 的脚本(如 api-collectiondeps-lint 等)在新版本下跑一遍,确保没有 CLI 行为变更或参数弃用。
  • 结合 React 19 的使用情况,确认 antd@^6.1.0 官方已声明兼容,且本仓库 CI / 视觉回归用例都在该版本下通过。
packages/x/.dumi/theme/builtins/Sandpack/index.tsx (1)

58-67: Sandpack Demo 依赖对齐到 antd 6.1.0 合理,注意同步验证示例兼容性

将 Sandpack 的 customSetup.dependencies.antd 调整为 ^6.1.0,与根项目和各包的 antd 版本保持一致,这样在线 Demo 行为就能更贴近真实使用场景,这个方向是对的。

建议补充验证:

  • 打开主要 Sender / Suggestion 相关示例,确认在 Sandpack 中渲染、交互(尤其是 Trigger / 弹层类交互)在 antd 6.1.0 下无样式或行为回归。
  • 若后续还会频繁升级 antd,可以考虑(仅建议)集中管理这些 Demo 依赖版本,避免 hard-code 多处时容易遗漏。
packages/x-markdown/package.json (1)

56-67: x-markdown 中对 antd 的开发依赖升级与仓库主线保持一致

这里将 @ant-design/x-markdown 的 devDependency antd 升级到 ^6.1.0,与根项目和 @ant-design/x 的基线保持一致,避免本包在本地开发 / 测试时拉起旧版 antd,是合理的。

建议:

  • 在本包内跑一遍 test / test:dekko / prestart,确认渲染 Markdown 相关的 Demo、样式检查在 antd 6.1.0 下没有行为差异。
  • 如果后续计划在该包内直接使用 antd 组件对外暴露(而不仅是 Demo),再评估是否需要将 antd 视为 peerDependency,目前只在 devDependencies 里问题不大。
packages/x/components/sender/SlotTextArea.tsx (5)

112-123: LGTM!

新增的 triggerValueChange 辅助函数很好地统一了 onChange 触发逻辑,确保每次变更都包含完整的 skill 参数,修复了 PR 描述中提到的问题。同时根据内容是否为空动态管理 skill-empty 类,优化了 placeholder 显示逻辑。


134-141: 请验证 Skill 区域可编辑的行为是否符合预期。

第 136 行将 Skill span 的 contenteditable'false' 改为 'true',这意味着用户现在可以直接在 Skill 区域内输入文本。此外,第 138 行使用了外层组件的 placeholder 而非 Skill 特定的占位符。

请确认:

  1. Skill 区域允许用户直接编辑是否为预期行为?
  2. 是否应该为 Skill 提供独立的 placeholder 属性?

303-348: LGTM!

getNodeTextValue 的重构很好地统一了 slot 和 skill 节点的文本提取逻辑。特别值得称赞的是:

  1. 使用 ?? 代替 ||,正确处理了 0false 等合法的 falsy 值
  2. 第 331 行使用显式的三元运算符而非 || 链式调用 formatResult,避免了 formatResult 返回空字符串时被错误回退

这些改进解决了之前评审中提出的问题。

Based on past review comments about formatResult handling.


492-510: Good!解决了之前的重复 onChange 问题。

通过引入 isChange 参数,insertSkill 现在调用 removeSkill(false) 进行 DOM 清理而不触发 onChange,然后在插入完成后统一调用 triggerValueChange(),避免了之前提到的两次 onChange 触发问题。

Based on past review comments about duplicate onChange emissions.


617-624: LGTM!

新增的 Skill 退格删除逻辑正确处理了用户在 Skill 节点前按 Backspace 的场景,移除了之前评审中提到的无效 skill?.closable; 语句。

Based on past review comments about the no-op expression.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
packages/x/components/sender/__tests__/index.test.tsx (1)

131-136: onChange 新签名断言合理,可选地收紧第二次调用校验

这里对 onChange 追加 slotConfigskill 两个入参,并在无 slot/skill 情况下分别断言为 []undefined,与新的 onChange(nextValue, event, slotConfig, skill) 设计是对齐的,没有明显问题。

如果希望测试更精确地区分“输入变更”和“清空”两次调用,可以把第二个断言改成检查第 2 次调用(或最后一次调用),避免将来内部多一次调用时产生误报:

-    expect(onChange).toHaveBeenCalledWith('bamboo', {}, [], undefined);
+    expect(onChange).toHaveBeenCalledWith('bamboo', {}, [], undefined);
@@
-    expect(onChange).toHaveBeenCalledWith('', undefined, [], undefined);
+    expect(onChange).toHaveBeenNthCalledWith(2, '', undefined, [], undefined);
+    // 或者:
+    // expect(onChange).toHaveBeenLastCalledWith('', undefined, [], undefined);

当前实现可以工作,上述只是提高断言精度的可选优化。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 49bb4fb and d4ab7c4.

📒 Files selected for processing (1)
  • packages/x/components/sender/__tests__/index.test.tsx (1 hunks)
⏰ 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). (5)
  • GitHub Check: build preview
  • GitHub Check: test
  • GitHub Check: size
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages

@kimteayon kimteayon linked an issue Dec 9, 2025 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

❌ Patch coverage is 78.57143% with 15 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@0e81224). Learn more about missing BASE report.

Files with missing lines Patch % Lines
packages/x/components/sender/SlotTextArea.tsx 77.27% 15 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1477   +/-   ##
=======================================
  Coverage        ?   94.66%           
=======================================
  Files           ?      137           
  Lines           ?     3920           
  Branches        ?     1108           
=======================================
  Hits            ?     3711           
  Misses          ?      206           
  Partials        ?        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (3)
packages/x/components/sender/__tests__/skill.test.tsx (3)

30-35: 与 tooltip 相关的用例断言偏弱,可更精确验证行为

当前三个与 tooltip 相关的用例基本只检查 .ant-sender-skill-tag-text 是否存在或有文本,这与“有/无 tooltip”“复杂 tooltip 配置”的描述不完全对应:即便组件完全忽略 toolTip 属性,这几条用例也仍然会通过。

可以考虑:

  • 在有 tooltip 的场景中,断言 tooltip wrapper / 触发器是否存在,或者模拟 mouseOver 后检查 tooltip 内容是否渲染到 DOM。
  • 在“without tooltip” 场景中,显式断言 tooltip wrapper/元素不存在,而不仅仅是文本存在。
  • 或者根据实际实现,调整用例描述,使其更贴合当前真正验证的行为。

这类增强不会影响现有逻辑,但能显著提高回归测试的信噪比。

Also applies to: 123-127, 129-142


144-156: 键盘事件用例目前只验证静态属性,fireEvent.keyDown 有些多余

这里的注释已经说明组件暂不处理键盘事件,但仍然调用了 fireEvent.keyDown(closeButton!, { key: 'Enter' });,后续断言其实只依赖 tabIndexrole 属性,与触发 keyDown 没有直接关系。

可以考虑二选一:

  • 要么移除 fireEvent.keyDown,仅保留结构/属性断言,使用例语义更纯粹;
  • 要么补充断言:例如确认 mockRemoveSkill 未被调用,显式保证当前实现不会意外响应键盘事件,为将来支持键盘操作留出安全回归空间。

整体属于小优化,不影响现有功能。


101-115: 阻止冒泡用例可以稍作增强提高可读性

当前通过 clickEvent.stopPropagation = mockStopPropagation 并断言其被调用,能基本验证组件调用了 event.stopPropagation()。为了让测试意图更直观,可以考虑:

  • 使用 jest.spyOn(clickEvent, 'stopPropagation')Object.defineProperty 定义只读属性,避免读代码时误以为是简单字段覆盖;
  • 或额外在父节点挂一个 onClick 并断言父级未被触发,从用户角度验证“点击关闭不触发外层点击”。

这是可选增强,现有写法可以工作,只是稍显“魔法”。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4ab7c4 and 4df1e47.

⛔ Files ignored due to path filters (6)
  • packages/x/components/actions/__tests__/__snapshots__/demo-extend.test.ts.snap is excluded by !**/*.snap
  • packages/x/components/actions/__tests__/__snapshots__/demo.test.ts.snap is excluded by !**/*.snap
  • packages/x/components/bubble/__tests__/__snapshots__/demo.test.ts.snap is excluded by !**/*.snap
  • packages/x/components/sender/__tests__/__snapshots__/demo-extend.test.ts.snap is excluded by !**/*.snap
  • packages/x/components/sender/__tests__/__snapshots__/demo.test.ts.snap is excluded by !**/*.snap
  • packages/x/components/sources/__tests__/__snapshots__/demo.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (1)
  • packages/x/components/sender/__tests__/skill.test.tsx (1 hunks)
⏰ 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). (5)
  • GitHub Check: build preview
  • GitHub Check: test
  • GitHub Check: size
  • GitHub Check: test
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
packages/x/components/sender/__tests__/skill.test.tsx (1)

5-12: 整体 Skill 单测结构与覆盖面不错

  • 覆盖了标题/值渲染、closable 多种形态、禁用样式、removeSkill/onClose 回调以及基础可访问性属性,整体用例设计合理。
  • 对新 DOM 结构(*-skill-wrapper / *-skill-tag / *-skill-holder / *-skill-close* 等类名)的断言也有到位,有助于防止回归。

Also applies to: 17-22, 36-48, 83-99, 158-170

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

实现链接的功能,add open to suggestion's children

3 participants