fix(sender): handle slot backspace after dynamic insert - #1967
fix(sender): handle slot backspace after dynamic insert#1967cc-hearts wants to merge 10 commits into
Conversation
dosubot
Bot
added
bug
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough调整 ChangesBackspace 删除 slot/skill 逻辑
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant SlotTextArea
participant DOMSelection
participant SlotStore
User->>SlotTextArea: 按下 Backspace
SlotTextArea->>DOMSelection: 读取光标位置
SlotTextArea->>SlotTextArea: 清理零宽字符并回溯节点
alt 找到 slot
SlotTextArea->>SlotStore: removeSlot(slotKey, event)
else 找到 skill
SlotTextArea->>SlotStore: removeSkill()
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the backspace deletion behavior for slots and skills in the SlotTextArea component by skipping empty text nodes when searching for the previous element to delete, and adds corresponding unit tests. The review feedback suggests enhancing the isEmptyTextNode helper to also treat zero-width spaces (such as \u200B and \uFEFF) as empty, which prevents backspace deletion from getting stuck on invisible characters commonly inserted by browsers in contenteditable fields.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/x/components/sender/components/SlotTextArea.tsx (1)
514-567: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBackspace 查找逻辑正确，可选：合并两个重复的遍历分支。
两个分支（
anchorNode === editableRef.current的索引回溯与非根节点的previousSibling回溯）共享完全相同的「跳过空文本节点 / 遇到 slot/skill 命中 / 遇到普通节点停止」规则。由于childNodes[i-1] === childNodes[i].previousSibling，可统一为单一的previousSibling遍历，避免后续两处逻辑各自演进而产生分歧。♻️ 合并遍历逻辑
const isEmptyTextNode = (node: Node) => node.nodeType === Node.TEXT_NODE && !node.textContent; let previousNode: HTMLElement | null = null; - if (anchorNode === editableRef.current) { - for (let i = focusOffset - 1; i >= 0; i -= 1) { - const childNode = editableRef.current.childNodes[i]; - if (childNode && isSlotOrSkill(childNode)) { - previousNode = childNode; - break; - } - if (childNode && !isEmptyTextNode(childNode)) { - break; - } - } - } else { - let currentNode: Node | null = anchorNode.previousSibling; - - while (currentNode) { - if (isSlotOrSkill(currentNode)) { - previousNode = currentNode; - break; - } - if (!isEmptyTextNode(currentNode)) { - break; - } - currentNode = currentNode.previousSibling; - } - } + let currentNode: Node | null = + anchorNode === editableRef.current + ? editableRef.current.childNodes[focusOffset - 1] ?? null + : anchorNode.previousSibling; + + while (currentNode) { + if (isSlotOrSkill(currentNode)) { + previousNode = currentNode; + break; + } + if (!isEmptyTextNode(currentNode)) { + break; + } + currentNode = currentNode.previousSibling; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/x/components/sender/components/SlotTextArea.tsx` around lines 514 - 567, The backspace lookup in SlotTextArea duplicates the same slot/skill traversal logic in two branches: one using editableRef.current childNodes indexing and one using previousSibling walking. Refactor this into a single shared traversal path, using the existing isSlotOrSkill and isEmptyTextNode checks, so both anchorNode cases resolve previousNode with the same stop/continue rules and won’t drift apart later.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/x/components/sender/components/SlotTextArea.tsx`:
- Around line 514-567: The backspace lookup in SlotTextArea duplicates the same
slot/skill traversal logic in two branches: one using editableRef.current
childNodes indexing and one using previousSibling walking. Refactor this into a
single shared traversal path, using the existing isSlotOrSkill and
isEmptyTextNode checks, so both anchorNode cases resolve previousNode with the
same stop/continue rules and won’t drift apart later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 83776de4-a2d9-4ef9-b01f-5573ed943670
📒 Files selected for processing (2)
packages/x/components/sender/__tests__/slot.test.tsxpackages/x/components/sender/components/SlotTextArea.tsx
|
复现步骤如： |
Bundle ReportChanges will increase total bundle size by 8 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: antdx-array-pushAssets Changed:
|
5742fce to
02f8eea
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1967 +/- ##
==========================================
+ Coverage 97.06% 97.11% +0.04%
==========================================
Files 159 159
Lines 5766 5788 +22
Branches 1712 1733 +21
==========================================
+ Hits 5597 5621 +24
+ Misses 167 165 -2
Partials 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
02f8eea to
0a03fe0
Compare
There was a problem hiding this comment.
发现 2 个需要修改的问题：
1、非折叠选区可能误删 slot" body="这里将 anchorNode 与 focusOffset 配对使用，但非折叠 Selection 中 focusOffset 属于 focusNode。例如 DOM 为 [slot, Text(\"ab\")]，选区从根节点 offset 1 到文本 offset 1 时，Backspace 会取到 childNodes[0] 并删除 slot，而不是删除选中的 a。回溯前应要求 range.collapsed，并使用同一端点的 node/offset。" file="/private/tmp/x-pr1967-review/packages/x/components/sender/components/SlotTextArea.tsx" start=514 end=517 priority=1}
2、零宽节点仍会污染 Sender 值" body="这些节点被判定为空并跳过，但没有从 DOM 或序列化结果中清理。删除 slot/skill 后，triggerValueChange() 会通过 getEditorValue() 原样返回 \\u200B\\uFEFF，同时把它们作为 text slot；因此视觉上清空的 Sender 仍可能有非空 value。新增测试只检查 slot 列表，遗漏了 onChange 第一参数和 getValue()。应在删除前移除已跳过节点，或统一在值序列化时过滤。" file="/private/tmp/x-pr1967-review/packages/x/components/sender/components/SlotTextArea.tsx" start=524 end=525 priority=2}
…idth characters from text nodes
|
我尝试修复了一下，请review下 |
2 participants
🤔 这个变动的性质是？
🔗 相关 Issue
复现地址：https://codesandbox.io/p/sandbox/zhi-neng-ti-shu-ru-antd-6-1-1-forked-9jn7t7
💡 需求背景和解决方案
当 Sender 使用 slotConfig 并通过 insert 动态插入词槽后，输入区域末尾可能出现空文本节点。此时光标位于 Sender 根 contenteditable 节点末尾，按 Backspace 无法命中前一个词槽节点，导致动态插入后的词槽无法正常删除。
复现步骤：
@，触发动态插入词槽。本次修复在处理 Backspace 时支持从 Sender 根节点的 childNodes 中向前查找前一个 slot/skill 节点，并且只跳过空文本节点，避免跨过普通文本误删更早的词槽。
📝 更新日志
✅ 验证
npm test --workspace packages/x -- sender/__tests__/slot.test.tsx --runInBandnpm exec biome check components/sender/components/SlotTextArea.tsx components/sender/__tests__/slot.test.tsxSummary by CodeRabbit
\u200B/\uFEFF污染值字符串。