-
-
Notifications
You must be signed in to change notification settings - Fork 921
Sender 组件插槽模式tag标签添加可关闭功能 #1410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Sender 组件插槽模式tag标签添加可关闭功能 #1410
Conversation
📝 WalkthroughWalkthrough为SlotTextArea组件中的tag类型插槽增强了关闭功能支持。通过扩展SlotConfigTagType接口添加allowClose属性,在renderSlot中实现关闭按钮逻辑,并在demo中演示了该功能的使用方式。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @kuangyaqin, 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! 此拉取请求旨在增强 Highlights
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this 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组件中的tag类型插槽增加了可关闭的功能。代码实现很直接,但在onClick事件处理器中存在一些实践上的问题。
主要的反馈点如下:
- 在
SlotTextArea.tsx中,关闭标签的onClick事件处理器直接操作了DOM并修改了ref。这是一种脆弱的实现方式,可能会导致UI状态不一致。建议采用更符合React模式的方法,通过父组件的状态更新来驱动DOM变化。 - 代码中残留了一个用于调试的
console.log语句,建议在合并前移除。
详细的建议请见具体的代码审查评论。总的来说,功能是好的,但实现方式可以更健壮、更易于维护。
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| // 从配置中移除 | ||
| slotConfigRef.current = slotConfigRef.current.filter( | ||
| (item) => item.key !== node.key, | ||
| ); | ||
| // 从DOM中移除 | ||
| const slotDom = getSlotDom(node.key as string); | ||
| slotDom?.parentNode?.removeChild(slotDom); | ||
| // 更新状态 | ||
| const newValue = getEditorValue(); | ||
| onChange?.(newValue.value, undefined, newValue.config); | ||
| }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个 onClick 处理器直接修改了DOM (removeChild) 和一个React ref (slotConfigRef.current)。这种做法比较脆弱,并且违反了React的单向数据流原则,可能导致难以预料的bug。
- 直接DOM操作:
slotDom?.parentNode?.removeChild(slotDom)会导致React的虚拟DOM和真实DOM之间出现不一致。如果父组件没有正确地通过onChange回调更新slotConfigprop来触发重渲染,UI就会处于一个损坏的状态。 - Ref突变: 对
slotConfigRef.current的直接修改是多余的,因为getEditorValue()会从DOM中重新生成配置,并且这个ref会在slotConfigprop变化时被重置。这使得代码的意图变得不清晰。
建议重构此逻辑,避免在事件处理器中直接操作DOM和ref。一个更安全的方式是,在临时修改DOM以计算新值后,立即恢复DOM,然后通过onChange将新的配置和值传递给父组件,由父组件来驱动UI的更新。
| const renderSlot = (node: SlotConfigType, slotSpan: HTMLSpanElement) => { | ||
| if (!node.key) return null; | ||
| const value = getSlotValues()[node.key]; | ||
| console.log(node, 'nodeqqqqq'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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
🧹 Nitpick comments (2)
packages/x/components/sender/SlotTextArea.tsx (1)
199-223: tag 的 allowClose 关闭逻辑整体正确,但建议同步清理相关状态当前关闭逻辑能正确:
- 从
slotConfigRef.current里移除配置;- 从 DOM 中移除对应的
slotSpan;- 调用
onChange通知外部。但同时还保留了与该 tag 相关的内部状态(如
slotDomMap、slotPlaceholders,以及通过getSlotValues()维护的值),可能导致:
- 残留的 portal 继续存在于
slotPlaceholders中,白占内存;- 内部状态与实际 DOM 不完全一致。
建议在关闭时一并清理这些映射,类似:
case 'tag': return ( <div className={`${prefixCls}-slot-tag`}> {node.props?.label || ''} {node.allowClose && ( <span className={`${prefixCls}-slot-tag-close`} onClick={(e) => { e.stopPropagation(); - // 从配置中移除 - slotConfigRef.current = slotConfigRef.current.filter( - (item) => item.key !== node.key, - ); - // 从DOM中移除 - const slotDom = getSlotDom(node.key as string); - slotDom?.parentNode?.removeChild(slotDom); - // 更新状态 + const slotKey = node.key as string; + // 从配置中移除 + slotConfigRef.current = slotConfigRef.current.filter( + (item) => item.key !== slotKey, + ); + // 从 DOM 中移除并清理映射 + const slotDom = getSlotDom(slotKey); + slotDom?.parentNode?.removeChild(slotDom); + slotDomMap.current.delete(slotKey); + setSlotPlaceholders((prev) => { + const newMap = new Map(prev); + newMap.delete(slotKey); + return newMap; + }); + // 同步清理该 slot 的值 + setSlotValues((prev) => { + const next = { ...prev }; + delete next[slotKey]; + return next; + }); + // 更新状态 const newValue = getEditorValue(); onChange?.(newValue.value, undefined, newValue.config); }} > × </span> )} </div> );另外,若后续有精力,可以考虑将关闭图标改为具有
role="button"或使用真正的按钮组件,并支持键盘触发,以提升可访问性。packages/x/components/sender/demo/agent.tsx (1)
63-71: ai_code 场景的可关闭 tag 示例清晰,可考虑同步强化 Files 动态 tag 的行为一致性这一项
{ type: 'tag', key: 'tag_type', allowClose: true, ... }很好地演示了新加的allowClose能力,示例本身没有问题。为了让示例行为更加统一,也可以考虑让下方
fileItemClick中动态插入的文件 tag 同样支持关闭,例如:const fileItemClick: MenuProps['onClick'] = (item) => { const { icon, label } = FileInfo[item.key]; senderRef.current?.insert?.([ { type: 'tag', key: `${item.key}_${Date.now()}`, + allowClose: true, props: { label: ( <Flex gap="small"> {icon} {label} </Flex> ), value: item.key, }, }, ]); };这样 demo 中所有 tag 插槽的交互能力会更加一致,也更直观展示
allowClose的用途。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/x/components/sender/SlotTextArea.tsx(2 hunks)packages/x/components/sender/demo/agent.tsx(2 hunks)packages/x/components/sender/interface.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
🔇 Additional comments (3)
packages/x/components/sender/interface.ts (2)
63-71: 为 tag 插槽增加 allowClose 字段的类型设计是合理的
allowClose?: boolean放在SlotConfigTagType上,与后续在SlotTextArea中通过node.allowClose控制是否展示关闭按钮的实现保持一致,向下兼容已有配置,类型上没有明显问题。
101-104: SenderProps 的 Pick 拆行只影响格式,不影响行为这里对
SenderProps的extends Pick<TextareaProps, ...>仅做了排版调整,没有增加或删除字段,类型行为保持不变,可以接受。packages/x/components/sender/demo/agent.tsx (1)
39-46: deep_search 场景新增只读 tag 配置是合理的这里新增的
{ type: 'tag', key: 'tag_type', props: { value: 'tag1', label: 'tag 1' } }与SlotConfigTagType定义完全匹配,未开启allowClose,可以很好地作为纯展示型 tag 示例。
| const renderSlot = (node: SlotConfigType, slotSpan: HTMLSpanElement) => { | ||
| if (!node.key) return null; | ||
| const value = getSlotValues()[node.key]; | ||
| console.log(node, 'nodeqqqqq'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
请移除调试用的 console.log
console.log(node, 'nodeqqqqq'); 属于调试残留,保留在组件库中会污染使用方控制台输出,建议删除或改为受控的调试开关。
🤖 Prompt for AI Agents
In packages/x/components/sender/SlotTextArea.tsx around line 143, there is a
leftover debugging statement `console.log(node, 'nodeqqqqq');` that should be
removed from the production component; delete this line (or replace it with a
controlled debug/logging mechanism—e.g., use the library's logger or a
conditional debug flag driven by env or props) so the component no longer
pollutes consumers' consoles.
|
特性功能需要提交到feature |

中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.