Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aojunhao123
Copy link
Contributor

@aojunhao123 aojunhao123 commented Oct 20, 2024

🤔 Background and solution

在 Select 的 tags 模式下,当用户在输入框中输入内容并失焦时,组件会自动将输入内容作为新标签添加,同时会自动去除输入内容的首尾空格。但在某些场景下,用户可能需要:

  1. 失焦时不自动添加标签
  2. 保留输入内容中首尾的空格

因此添加了两个新的 API:

  • onBlurAddValue: 控制失焦时是否将输入内容添加为新标签
  • onBlurRemoveSpaces: 控制是否自动去除输入内容的首尾空格

📝 Changelog

Language Changelog
🇺🇸 English Add onBlurAddValue & onBlurRemoveSpaces props to Select component in tags mode for more flexible input handling on blur
🇨🇳 Chinese 为 Select 组件的 tags 模式新增 onBlurAddValueonBlurRemoveSpaces 属性,提供更灵活的失焦输入处理

⚡ API Changes

interface SelectProps {
/** Whether to add input value as tag when blur in tags mode 
  * @default true 
  */
onBlurAddValue?: boolean;
/** Whether to remove spaces of input value when blur in tags mode 
  * @default true 
  */
onBlurRemoveSpaces?: boolean;
}

🔍 Implementation & Test Cases

  1. 添加了相关属性的类型定义
  2. 实现了失焦时的条件处理逻辑
  3. 添加了测试用例验证新功能:
    • 测试 onBlurAddValue={false} 时不会添加新标签
    • 测试 onBlurRemoveSpaces={false} 时保留空格

📋 Related Issue

Summary by CodeRabbit

Summary by CodeRabbit

  • 新功能
    • 新增属性 onBlurRemoveSpaceonBlurAddValue,用户可自定义失焦时是否添加value,或是否移除value前后的空格。

Copy link

vercel bot commented Oct 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
select ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 2, 2024 4:10pm

Copy link
Contributor

coderabbitai bot commented Oct 20, 2024

Walkthrough

此次更改对src/Select.tsx中的Select组件进行了重大修改,主要集中在API变更、功能更新和可访问性改进。新增了listHeightlistItemHeightcomponent等属性,同时移除了多个已弃用的属性。combobox模式的行为也进行了调整,限制了某些功能并增强了可访问性。

Changes

文件路径 更改摘要
src/Select.tsx - 新增属性:listHeightlistItemHeightcomponent
- 移除属性:multipletagscomboboxfirstActiveValuedropdownMenuStyleopenClassName
- 更新backfill属性和combobox模式的功能。
- 修改onChange事件返回类型为OptionData
- 增强可访问性,新增aria-live="polite"属性。
- 更新onInternalSearch函数以处理搜索文本。
README.md - 新增属性:onBlurRemoveSpaces(布尔类型,默认值:true)。
- 新增属性:onBlurAddValue(布尔类型,默认值:true)。
docs/examples/tags.tsx - 新增两个示例,分别展示onBlurRemoveSpacesonBlurAddValue属性的使用。
tests/Tags.test.tsx - 新增两个测试用例,验证onBlurAddValueonBlurRemoveSpaces属性的行为。

Assessment against linked issues

Objective Addressed Explanation
在tags模式中,失焦时不改变用户输入的值(#51149, #1075
在tags模式中,失焦时不添加tag(#952
移除onBlur的默认行为,避免不必要的选项添加(#952

Possibly related PRs

Suggested reviewers

  • zombieJ
  • afc163

兔子欢跳在草地上,
选择组件新变化,
高度和项高都更新,
可访问性更是加分,
让每个用户都能欢,
兔子乐在其中欢! 🐇✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between ca86130 and 14e0a33.

📒 Files selected for processing (1)
  • src/Select.tsx (3 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/Select.tsx (2)

218-221: 确认新增属性的默认值是否符合预期

onBlurRemoveSpaceonBlurAddValue 的默认值设置为 true,请确认这是否与预期的默认行为一致,避免对现有功能产生意外影响。


586-588: 检查 onInternalSearch 方法中条件判断的逻辑

onInternalSearch 方法中,使用了 onBlurRemoveSpaceonBlurAddValue 来控制搜索文本的处理方式。请确保条件判断的逻辑符合预期,特别是在处理空字符串或仅包含空格的情况,避免可能出现的边缘案例。

src/Select.tsx Outdated
Comment on lines 165 to 168

// >>> tags mode only
onBlurRemoveSpace?: boolean;
onBlurAddValue?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议为新增的属性添加注释或文档说明

为了提高代码的可维护性和可读性,建议为新增的 onBlurRemoveSpaceonBlurAddValue 属性添加注释,解释其用途和使用方法,或在组件的文档中进行说明。

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

🧹 Outside diff range and nitpick comments (1)
README.md (1)

136-137: 新属性添加得当,描述清晰。

新添加的 onBlurRemoveSpaceonBlurAddValue 属性与 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 |

这样的描述更符合中文文档的习惯,使用户更容易理解这些新属性的作用。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 14e0a33 and 8dee5af.

📒 Files selected for processing (1)
  • README.md (1 hunks)
🧰 Additional context used

Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.24%. Comparing base (ca86130) to head (5afe84f).

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.
📢 Have feedback on the report? Share it here.

@aojunhao123
Copy link
Contributor Author

@zombieJ 大佬帮忙review一下

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

🧹 Outside diff range and nitpick comments (5)
docs/examples/tags.tsx (2)

114-130: 建议保持事件处理程序的一致性

为了与其他示例保持一致,建议添加 onSelectonDeselect 事件处理程序。这样可以更全面地展示组件的功能。

建议按照如下方式修改:

         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: 建议保持事件处理程序的一致性

与上一个示例类似,建议添加 onSelectonDeselect 事件处理程序,以保持示例的完整性。

建议按照如下方式修改:

         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 键触发的失焦场景。建议增加以下验证场景:

  1. 点击外部区域触发失焦
  2. 切换窗口触发失焦
  3. 输入框为空时的失焦行为

建议添加如下测试用例:

 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: 测试用例需要验证更多边界情况

当前测试用例仅验证了回车键添加标签的场景。建议增加以下验证:

  1. 多个连续空格的处理
  2. 仅包含空格的输入值
  3. 失焦时的空格保留行为

建议扩展测试用例如下:

 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

📥 Commits

Reviewing files that changed from the base of the PR and between 8dee5af and 5afe84f.

📒 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: 新功能示例实现正确且清晰

新增的两个示例很好地展示了 onBlurRemoveSpacesonBlurAddValue 的用法,代码结构清晰,与现有示例保持一致的风格。

src/Select.tsx (2)

218-221: 属性初始化实现正确

默认值设置和分组注释的实现都很合理。


586-595: 建议验证新属性的测试覆盖情况

请确保为新增的 onBlurRemoveSpacesonBlurAddValue 属性添加充分的测试用例,包括:

  • 默认行为测试
  • 属性设为 false 时的行为测试
  • 边界条件测试(空字符串、纯空格等)
✅ Verification successful

测试覆盖已经完备,无需额外验证

tests/Tags.test.tsx 文件中已包含了对 onBlurRemoveSpacesonBlurAddValue 属性的完整测试用例:

  • 验证了 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

Comment on lines +586 to +595
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('');

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议优化空格处理逻辑

当前实现存在以下可以改进的地方:

  1. onBlurRemoveSpaces 为 true 时存在重复的 trim 操作
  2. 条件判断的顺序可能导致混淆

建议重构如下:

- 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.

@aojunhao123
Copy link
Contributor Author

@afc163 @zombieJ 大佬们有空的时候瞅瞅

@zombieJ
Copy link
Member

zombieJ commented Nov 4, 2024

这个命名看起来过于业务了,如果我业务里要求 blur 的时候保留后面的空格不保留前面的空格?或者 blur 的时候如果输入内容符合某种规则才添加,不符合就不添加?感觉应该是类似某种 post 事件提供一个 source 告知是来自哪里的,然后用户可以选择如何去处理。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants