-
-
Notifications
You must be signed in to change notification settings - Fork 457
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
feat: support overriding default component behavior for the onBlur event in tags mode #1081
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改对 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/Select.tsx (3 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/Select.tsx (2)
218-221
: 确认新增属性的默认值是否符合预期
onBlurRemoveSpace
和onBlurAddValue
的默认值设置为true
,请确认这是否与预期的默认行为一致,避免对现有功能产生意外影响。
586-588
: 检查onInternalSearch
方法中条件判断的逻辑在
onInternalSearch
方法中,使用了onBlurRemoveSpace
和onBlurAddValue
来控制搜索文本的处理方式。请确保条件判断的逻辑符合预期,特别是在处理空字符串或仅包含空格的情况,避免可能出现的边缘案例。
src/Select.tsx
Outdated
|
||
// >>> tags mode only | ||
onBlurRemoveSpace?: boolean; | ||
onBlurAddValue?: boolean; |
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.
🛠️ Refactor suggestion
建议为新增的属性添加注释或文档说明
为了提高代码的可维护性和可读性,建议为新增的 onBlurRemoveSpace
和 onBlurAddValue
属性添加注释,解释其用途和使用方法,或在组件的文档中进行说明。
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: 0
🧹 Outside diff range and nitpick comments (1)
README.md (1)
136-137
: 新属性添加得当,描述清晰。新添加的
onBlurRemoveSpace
和onBlurAddValue
属性与 PR 的目标一致,增强了组件在标签模式下的灵活性。描述简洁明了,默认值设置合理。为了进一步提高可读性,建议稍微调整描述措辞:
- | onBlurRemoveSpace | Whether to remove space when losing focus, only applies when `mode` is `tags` | boolean | true | + | onBlurRemoveSpace | 失去焦点时是否移除空格,仅在 `mode` 为 `tags` 时适用 | boolean | true | - | onBlurAddValue | Whether to add the input value to the selected item when losing focus, only applies when `mode` is `tags` | boolean | true | + | onBlurAddValue | 失去焦点时是否将输入值添加到已选项中,仅在 `mode` 为 `tags` 时适用 | boolean | true |这样的描述更符合中文文档的习惯,使用户更容易理解这些新属性的作用。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1081 +/- ##
=======================================
Coverage 98.24% 98.24%
=======================================
Files 39 39
Lines 1482 1484 +2
Branches 418 442 +24
=======================================
+ Hits 1456 1458 +2
Misses 26 26 ☔ View full report in Codecov by Sentry. |
@zombieJ 大佬帮忙review一下 |
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
🧹 Outside diff range and nitpick comments (5)
docs/examples/tags.tsx (2)
114-130
: 建议保持事件处理程序的一致性为了与其他示例保持一致,建议添加
onSelect
和onDeselect
事件处理程序。这样可以更全面地展示组件的功能。建议按照如下方式修改:
onChange={(val) => { console.log('change:', val); setValue(val); }} + onSelect={(val, option) => { + console.log('selected', val, option); + }} + onDeselect={(val, option) => { + console.log('deselected', val, option); + }}
131-147
: 建议保持事件处理程序的一致性与上一个示例类似,建议添加
onSelect
和onDeselect
事件处理程序,以保持示例的完整性。建议按照如下方式修改:
onChange={(val) => { console.log('change:', val); setValue(val); }} + onSelect={(val, option) => { + console.log('selected', val, option); + }} + onDeselect={(val, option) => { + console.log('deselected', val, option); + }}tests/Tags.test.tsx (2)
544-552
: 测试用例需要补充验证场景当前测试用例仅验证了 Tab 键触发的失焦场景。建议增加以下验证场景:
- 点击外部区域触发失焦
- 切换窗口触发失焦
- 输入框为空时的失焦行为
建议添加如下测试用例:
it('should not add value when onBlurAddValue is false', () => { const { container } = render(<Select mode="tags" onBlurAddValue={false} />); const input = container.querySelector<HTMLInputElement>('input'); // 测试 Tab 键失焦 toggleOpen(container); fireEvent.change(input, { target: { value: 'test' } }); keyDown(input, KeyCode.TAB); expect(container.querySelectorAll('.rc-select-selection-item')).toHaveLength(0); + + // 测试点击外部失焦 + toggleOpen(container); + fireEvent.change(input, { target: { value: 'test2' } }); + fireEvent.blur(input); + expect(container.querySelectorAll('.rc-select-selection-item')).toHaveLength(0); + + // 测试空值失焦 + toggleOpen(container); + fireEvent.change(input, { target: { value: '' } }); + fireEvent.blur(input); + expect(container.querySelectorAll('.rc-select-selection-item')).toHaveLength(0); });
554-560
: 测试用例需要验证更多边界情况当前测试用例仅验证了回车键添加标签的场景。建议增加以下验证:
- 多个连续空格的处理
- 仅包含空格的输入值
- 失焦时的空格保留行为
建议扩展测试用例如下:
it('should not add value when onBlurRemoveSpaces is false', () => { const { container } = render(<Select mode="tags" onBlurRemoveSpaces={false} />); // 测试回车添加 toggleOpen(container); fireEvent.change(container.querySelector('input'), { target: { value: ' test ' } }); keyDown(container.querySelector('input'), KeyCode.ENTER); expect(findSelection(container).textContent).toEqual(' test '); + + // 测试连续空格 + toggleOpen(container); + fireEvent.change(container.querySelector('input'), { target: { value: ' test ' } }); + keyDown(container.querySelector('input'), KeyCode.ENTER); + expect(findSelection(container).textContent).toEqual(' test '); + + // 测试仅空格输入 + toggleOpen(container); + fireEvent.change(container.querySelector('input'), { target: { value: ' ' } }); + keyDown(container.querySelector('input'), KeyCode.ENTER); + expect(findSelection(container).textContent).toEqual(' '); + + // 测试失焦保留空格 + toggleOpen(container); + fireEvent.change(container.querySelector('input'), { target: { value: ' test ' } }); + fireEvent.blur(container.querySelector('input')); + expect(findSelection(container).textContent).toEqual(' test '); });src/Select.tsx (1)
165-168
: 需要为新属性添加 TypeScript 文档注释建议为新增的标签模式属性添加详细的 TypeScript 文档注释,以便用户理解其用途:
// >>> tags mode only + /** + * 在失焦时是否移除空格 + * @default true + */ onBlurRemoveSpaces?: boolean; + /** + * 在失焦时是否添加值 + * @default true + */ onBlurAddValue?: boolean;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
README.md
(1 hunks)docs/examples/tags.tsx
(1 hunks)src/Select.tsx
(3 hunks)tests/Tags.test.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🔇 Additional comments (3)
docs/examples/tags.tsx (1)
114-147
: 新功能示例实现正确且清晰
新增的两个示例很好地展示了 onBlurRemoveSpaces
和 onBlurAddValue
的用法,代码结构清晰,与现有示例保持一致的风格。
src/Select.tsx (2)
218-221
: 属性初始化实现正确
默认值设置和分组注释的实现都很合理。
586-595
: 建议验证新属性的测试覆盖情况
请确保为新增的 onBlurRemoveSpaces
和 onBlurAddValue
属性添加充分的测试用例,包括:
- 默认行为测试
- 属性设为 false 时的行为测试
- 边界条件测试(空字符串、纯空格等)
✅ Verification successful
测试覆盖已经完备,无需额外验证
tests/Tags.test.tsx
文件中已包含了对 onBlurRemoveSpaces
和 onBlurAddValue
属性的完整测试用例:
- 验证了
onBlurAddValue={false}
时不会添加新标签 - 验证了
onBlurRemoveSpaces={false}
时保留空格的行为 - 测试用例覆盖了关键场景和边界条件
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查测试文件中是否包含新属性的测试用例
rg -l "onBlurRemoveSpaces|onBlurAddValue" "*test*"
Length of output: 99
Script:
#!/bin/bash
# 搜索测试文件中的新属性
rg -l "onBlurRemoveSpaces|onBlurAddValue" --type test
# 查找所有测试文件
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts
# 查找Select组件的测试文件
fd "Select.*test" -e tsx -e ts
Length of output: 630
Script:
#!/bin/bash
# 检查Select相关测试文件中的新属性使用情况
rg "onBlurRemoveSpaces|onBlurAddValue" -A 5 -B 5 tests/Select.test.tsx tests/BaseSelect.test.tsx tests/Tags.test.tsx
# 检查Select组件中这些属性的定义
rg "onBlurRemoveSpaces|onBlurAddValue" -A 2 -B 2 src/Select.tsx
Length of output: 2209
const formatted = onBlurRemoveSpaces ? (searchText || '').trim() : searchText || ''; | ||
// prevent empty tags from appearing when you click the Enter button | ||
if (formatted) { | ||
if (formatted.trim() && onBlurAddValue) { | ||
const newRawValues = Array.from(new Set<RawValueType>([...rawValues, formatted])); | ||
triggerChange(newRawValues); | ||
triggerSelect(formatted, true); | ||
setSearchValue(''); | ||
} | ||
|
||
setSearchValue(''); | ||
|
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.
🛠️ Refactor suggestion
建议优化空格处理逻辑
当前实现存在以下可以改进的地方:
- 当
onBlurRemoveSpaces
为 true 时存在重复的 trim 操作 - 条件判断的顺序可能导致混淆
建议重构如下:
- const formatted = onBlurRemoveSpaces ? (searchText || '').trim() : searchText || '';
- // prevent empty tags from appearing when you click the Enter button
- if (formatted.trim() && onBlurAddValue) {
+ const formatted = searchText || '';
+ const valueToAdd = onBlurRemoveSpaces ? formatted.trim() : formatted;
+ // prevent empty tags from appearing when you click the Enter button
+ if (valueToAdd && onBlurAddValue) {
const newRawValues = Array.from(new Set<RawValueType>([...rawValues, formatted]));
triggerChange(newRawValues);
triggerSelect(formatted, true);
}
这样可以:
- 避免重复的 trim 操作
- 使空格处理逻辑更清晰
- 保持与属性名称的语义一致性
Committable suggestion skipped: line range outside the PR's diff.
这个命名看起来过于业务了,如果我业务里要求 blur 的时候保留后面的空格不保留前面的空格?或者 blur 的时候如果输入内容符合某种规则才添加,不符合就不添加?感觉应该是类似某种 post 事件提供一个 source 告知是来自哪里的,然后用户可以选择如何去处理。 |
🤔 Background and solution
在 Select 的 tags 模式下,当用户在输入框中输入内容并失焦时,组件会自动将输入内容作为新标签添加,同时会自动去除输入内容的首尾空格。但在某些场景下,用户可能需要:
因此添加了两个新的 API:
onBlurAddValue
: 控制失焦时是否将输入内容添加为新标签onBlurRemoveSpaces
: 控制是否自动去除输入内容的首尾空格📝 Changelog
onBlurAddValue
&onBlurRemoveSpaces
props to Select component in tags mode for more flexible input handling on bluronBlurAddValue
和onBlurRemoveSpaces
属性,提供更灵活的失焦输入处理⚡ API Changes
🔍 Implementation & Test Cases
onBlurAddValue={false}
时不会添加新标签onBlurRemoveSpaces={false}
时保留空格📋 Related Issue
Summary by CodeRabbit
Summary by CodeRabbit
onBlurRemoveSpace
和onBlurAddValue
,用户可自定义失焦时是否添加value,或是否移除value前后的空格。