feat: add activeOptionFilter prop and update combobox examples #1156
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次更改为 Select 组件引入了新的可选属性 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Select
participant OptionList
User->>Select: 输入搜索内容
Select->>OptionList: 传递 activeOptionFilter、searchValue 和 options
OptionList->>OptionList: 使用 activeOptionFilter 判断激活项
OptionList-->>Select: 返回激活项索引
Select-->>User: 高亮/滚动到对应选项
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/Select.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/Select.tsx (1)
217-218: 建议：提醒使用方包裹activeOptionFilter以保持引用稳定虽然这里没有问题，但如果调用者在渲染期间不断创建新函数，会导致
selectContext频繁变动，进而触发OptionList重新渲染。可以在文档中提示用户使用useCallback。
[建议，可选优化]docs/examples/combobox.tsx (2)
155-163: 示例函数建议用useCallback保持稳定引用示例中
activeOptionFilter直接内联定义，每次渲染都会创建新函数，不利于性能演示。虽然示例影响有限，但推荐写法为：const middleMatch = React.useCallback( (searchValue, option) => String(option.value).includes(searchValue), [], ); ... activeOptionFilter={middleMatch}
173-180: 同上，禁用高亮示例也可提取回调保持代码一致性和最佳实践。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)docs/examples/combobox.tsx(1 hunks)src/OptionList.tsx(2 hunks)src/Select.tsx(4 hunks)src/SelectContext.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/SelectContext.ts (1)
src/Select.tsx (1)
SelectProps(113-167)
🔇 Additional comments (3)
src/SelectContext.ts (1)
40-41:activeOptionFilter字段加入上下文 👍上下文定义同步新增了
activeOptionFilter，类型引用SelectProps['activeOptionFilter']，保持了单一来源，非常可读。src/Select.tsx (1)
154-155: Props 透传完整，依赖数组同步更新已到位
activeOptionFilter已正确暴露到SelectProps并随selectContext一并 memo 化，符合预期。README.md (1)
135-136: 文档更新同步，可读性良好
activeOptionFilter参数已补充到 API 表格，没有遗漏默认值说明，清晰易懂。
| if (!multiple && open && rawValues.size === 1) { | ||
| const value: RawValueType = Array.from(rawValues)[0]; | ||
| // Scroll to the option closest to the searchValue if searching. | ||
| const index = memoFlattenOptions.findIndex(({ data }) => | ||
| searchValue ? String(data.value).startsWith(searchValue) : data.value === value, | ||
| ); | ||
| const index = memoFlattenOptions.findIndex(({ data }) => { | ||
| if (activeOptionFilter) { | ||
| return activeOptionFilter(searchValue, data); | ||
| } | ||
| return searchValue ? String(data.value).startsWith(searchValue) : data.value === value; | ||
| }); | ||
|
|
There was a problem hiding this comment.
主动过滤逻辑对分组项存在空指针风险
findIndex 的回调中直接使用 data.value，然而当当前项为分组节点时 data 可能为 undefined，会导致运行时抛出 TypeError: Cannot read properties of undefined (reading 'value')。同时把 activeOptionFilter 应用到分组节点也没有意义。
建议在回调里先排除分组节点或缺少 data 的情况：
- const index = memoFlattenOptions.findIndex(({ data }) => {
- if (activeOptionFilter) {
- return activeOptionFilter(searchValue, data);
- }
- return searchValue ? String(data.value).startsWith(searchValue) : data.value === value;
- });
+ const index = memoFlattenOptions.findIndex(({ group, data }) => {
+ // 忽略分组标题或异常节点
+ if (group || !data) {
+ return false;
+ }
+
+ if (activeOptionFilter) {
+ return activeOptionFilter(searchValue, data);
+ }
+ return searchValue
+ ? String(data.value).startsWith(searchValue)
+ : data.value === value;
+ });这样可以避免空指针，同时保持逻辑的一致性。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!multiple && open && rawValues.size === 1) { | |
| const value: RawValueType = Array.from(rawValues)[0]; | |
| // Scroll to the option closest to the searchValue if searching. | |
| const index = memoFlattenOptions.findIndex(({ data }) => | |
| searchValue ? String(data.value).startsWith(searchValue) : data.value === value, | |
| ); | |
| const index = memoFlattenOptions.findIndex(({ data }) => { | |
| if (activeOptionFilter) { | |
| return activeOptionFilter(searchValue, data); | |
| } | |
| return searchValue ? String(data.value).startsWith(searchValue) : data.value === value; | |
| }); | |
| if (!multiple && open && rawValues.size === 1) { | |
| const value: RawValueType = Array.from(rawValues)[0]; | |
| // Scroll to the option closest to the searchValue if searching. | |
| - const index = memoFlattenOptions.findIndex(({ data }) => { | |
| - if (activeOptionFilter) { | |
| - return activeOptionFilter(searchValue, data); | |
| - } | |
| - return searchValue ? String(data.value).startsWith(searchValue) : data.value === value; | |
| - }); | |
| + const index = memoFlattenOptions.findIndex(({ group, data }) => { | |
| + // 忽略分组标题或异常节点 | |
| + if (group || !data) { | |
| + return false; | |
| + } | |
| + | |
| + if (activeOptionFilter) { | |
| + return activeOptionFilter(searchValue, data); | |
| + } | |
| + return searchValue | |
| + ? String(data.value).startsWith(searchValue) | |
| + : data.value === value; | |
| + }); |
🤖 Prompt for AI Agents
In src/OptionList.tsx around lines 160 to 169, the findIndex callback accesses
data.value without checking if data exists, causing a TypeError when the item is
a group node with undefined data. To fix this, add a guard clause in the
callback to skip items where data is undefined or the item is a group node
before accessing data.value or applying activeOptionFilter, ensuring no null
pointer errors and preserving the filtering logic.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1156 +/- ##
=======================================
Coverage 98.23% 98.23%
=======================================
Files 38 38
Lines 1471 1473 +2
Branches 446 440 -6
=======================================
+ Hits 1445 1447 +2
Misses 26 26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/Combobox.test.tsx (2)
135-149: 断言过于依赖固定选项顺序，建议改为断言“首个满足条件的选项”当前测试假设匹配字符串
'an'时激活的一定是'banana'，而这取决于渲染顺序与内部遍历实现；
如若以后更改为“按字母排序后再匹配”或“返回所有匹配中的首条可见项”，测试就会误失败。
可考虑改为：
- 先拿到所有
.rc-select-item-option-active元素，断言其数量为 1。- 断言其
textContent与input当前值存在包含关系（或调用同一activeOptionFilter判断为真）。这样能保证测试关注“过滤逻辑正确”而非“具体哪条被激活”。
151-165: 缺少对「无匹配时亦不激活」场景的负面验证第二个用例只在输入
'an'时断言无 active 选项；若activeOptionFilter总返回false，理应在任意输入下都不激活。
可追加一次输入'ora'的断言，增强覆盖率：fireEvent.change(input, { target: { value: 'an' } }); await delay(); expect(container.querySelector('.rc-select-item-option-active')).toBeNull(); + +fireEvent.change(input, { target: { value: 'ora' } }); +await delay(); +expect(container.querySelector('.rc-select-item-option-active')).toBeNull();这样能防止实现不一致导致 “第一次正常、第二次错误” 却漏检。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Combobox.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/Combobox.test.tsx (1)
docs/examples/combobox.tsx (1)
render(68-190)
| it('activeOptionFilter should work', async () => { | ||
| const { container } = render( | ||
| <Select | ||
| mode="combobox" | ||
| showSearch | ||
| activeOptionFilter={(searchValue, option) => { | ||
| return String(option.value).includes(searchValue); | ||
| }} | ||
| > | ||
| <Option value="apple">apple</Option> | ||
| <Option value="banana">banana</Option> | ||
| <Option value="orange">orange</Option> | ||
| </Select>, | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议使用 waitFor 或 findBy... 替换自定义 delay 以消除非必要的 setTimeout
delay() 只是为了等下一个事件循环，但这会引入真实 setTimeout，在 CI 上容易出现偶发现象。
采用 Testing-Library 自带的 waitFor（或直接使用 findByRole / findByText）能在内部自动处理异步刷新，更符合该库的最佳实践，也无需显式 await delay()。
- fireEvent.change(input, { target: { value: 'an' } });
- await delay();
+ fireEvent.change(input, { target: { value: 'an' } });
+ await waitFor(() =>
+ expect(
+ container.querySelector('.rc-select-item-option-active .rc-select-item-option-content'),
+ ).toHaveTextContent('banana'),
+ );同理可删除自定义 delay 函数。
这样做能减少对实现细节的耦合，并提升测试稳定性。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('activeOptionFilter should work', async () => { | |
| const { container } = render( | |
| <Select | |
| mode="combobox" | |
| showSearch | |
| activeOptionFilter={(searchValue, option) => { | |
| return String(option.value).includes(searchValue); | |
| }} | |
| > | |
| <Option value="apple">apple</Option> | |
| <Option value="banana">banana</Option> | |
| <Option value="orange">orange</Option> | |
| </Select>, | |
| ); | |
| it('activeOptionFilter should work', async () => { | |
| const { container } = render( | |
| <Select | |
| mode="combobox" | |
| showSearch | |
| activeOptionFilter={(searchValue, option) => { | |
| return String(option.value).includes(searchValue); | |
| }} | |
| > | |
| <Option value="apple">apple</Option> | |
| <Option value="banana">banana</Option> | |
| <Option value="orange">orange</Option> | |
| </Select>, | |
| ); | |
| const input = container.querySelector('input')!; | |
| fireEvent.change(input, { target: { value: 'an' } }); | |
| await waitFor(() => | |
| expect( | |
| container.querySelector('.rc-select-item-option-active .rc-select-item-option-content'), | |
| ).toHaveTextContent('banana'), | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In tests/Combobox.test.tsx around lines 120 to 134, replace any custom delay or
setTimeout calls used to wait for asynchronous updates with Testing Library's
built-in waitFor or findBy... queries. Remove the custom delay function
entirely. This change will make the tests more stable and aligned with best
practices by relying on Testing Library's internal async handling instead of
manual timing.
| searchValue ? String(data.value).startsWith(searchValue) : data.value === value, | ||
| ); | ||
| const index = memoFlattenOptions.findIndex(({ data }) => { | ||
| if (activeOptionFilter) { |
There was a problem hiding this comment.
感觉有点怪异，如果是 value 被选中了，展开不应该是用当前这个逻辑。否则会导致选中的是一个，展开激活的却是另一个。它应该是 default active 的逻辑
按照原有的逻辑，会默认根据 input 字符串 startsWith 规则自动将匹配的第一项设置为 activate ，且这种行为写死不可配置。
我认为以上行为不对，匹配规则应当能可选地由用户自己配置。
我的修改使得用户可以自行定制匹配规则，详见代码里的两个例子
任意部分匹配（而不仅开头）
强制不匹配（很多时候用户希望按下 Enter 键会直接发起搜索，而不是自动选中第一项）
像B站搜索框就是不会默认 activate 第一个匹配项的
Summary by CodeRabbit
Summary by CodeRabbit
activeOptionFilter，允许自定义搜索时激活选项的过滤逻辑。activeOptionFilter的用法演示，包括自定义匹配和禁用高亮的场景。