-
-
Notifications
You must be signed in to change notification settings - Fork 478
fix: Select dropdown on search not scroll to the correct options #1161
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: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough在 Select 组件链路中新增可选属性 optionFilterProp,并将其通过 SelectContext 传递到 OptionList。OptionList 更新搜索匹配逻辑:按 optionFilterProp 或 value 进行不区分大小写的包含匹配,调整单选自动滚动的匹配路径,并更新相关 effect 的依赖。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant S as Select
participant C as SelectContext
participant L as OptionList
U->>S: 输入搜索词 (onSearch)
S->>C: 更新上下文 { searchValue, optionFilterProp }
Note over C: Context 提供过滤配置
C-->>L: 提供 searchValue / optionFilterProp
rect rgba(230,245,255,0.6)
L->>L: 计算 matchValue: data[optionFilterProp] 或 data.value
L->>L: includes(matchValue, searchValue)(不区分大小写)
end
alt 单选模式
L->>L: 根据匹配结果确定自动滚动目标
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ 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. Comment |
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.
Summary of Changes
Hello @cactuser-Lu, 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!
This pull request resolves a critical bug in the select dropdown component where the scroll position would not correctly align with the filtered options during a search. By unifying the filtering and scrolling logic, the change ensures a consistent and intuitive user experience, making search functionality more reliable and predictable.
Highlights
- Standardized Search and Scroll Logic: Resolved an issue where the select dropdown failed to scroll to the correct option during search by unifying the filtering and scrolling mechanisms.
- Introduced Case-Insensitive Matching: Added a new
includes
utility function insrc/OptionList.tsx
to perform case-insensitive string matching, aligning the scroll logic with the existing filtering behavior. - Respected
optionFilterProp
: Ensured that theoptionFilterProp
is now considered when determining the active item for scrolling, providing consistency with how options are filtered.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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
感谢您的贡献。此更改通过统一滚动定位逻辑和过滤逻辑,解决了在搜索时下拉列表无法滚动到正确选项的问题。现在两者都使用不区分大小写的 includes
检查并支持 optionFilterProp
,这很好。
我只在代码中发现了一个可以改进的地方,即一个重复的辅助函数。详细信息请参阅我的具体评论。
function includes(test: React.ReactNode, search: string) { | ||
return toArray(test).join('').toUpperCase().includes(search.toUpperCase()); | ||
} |
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: 2
🧹 Nitpick comments (4)
src/SelectContext.ts (1)
40-41
: 在上下文中暴露 optionFilterProp 是对的,但请补充类型注释与文档同步新增字段与 SearchConfig.optionFilterProp 一致,便于下游消费。建议补充 JSDoc,说明该值与 fieldNames 的关系与优先级。
src/OptionList.tsx (3)
18-19
: 引入 toArray 的用途需谨慎:ReactNode 文本提取可能不完整toArray + join 在遇到 React 元素(如高亮包装)时会拼出 “[object Object]”,可能导致 includes 匹配失败。见下方对 includes 的改进建议。
189-191
: 副作用依赖应包含可能影响匹配/定位结果的输入当前依赖为 [open, searchValue, optionFilterProp]。若 fieldNames(或其 value 映射)变化,或可见数据变化但 searchValue 未变,可能导致闭包持有旧值。建议最小化地加入 memoFlattenOptions.length 与 fieldNames(或其序列化):
- }, [open, searchValue, optionFilterProp]); + }, [open, searchValue, optionFilterProp, memoFlattenOptions.length, JSON.stringify(fieldNames)]);
156-167
: 小优化:单选场景 rawValues.size 判等可提前返回,避免不必要计算非功能性建议,避免在不满足条件时进入 findIndex。
- if (!multiple && open && rawValues.size === 1) { + if (!(open && !multiple && rawValues.size === 1)) { + // 强制滚动条可见逻辑保持不变 + if (open) listRef.current?.scrollTo(undefined); + return () => clearTimeout(timeoutId); + } + { const value: RawValueType = Array.from(rawValues)[0]; // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/OptionList.tsx
(5 hunks)src/Select.tsx
(2 hunks)src/SelectContext.ts
(1 hunks)
🔇 Additional comments (4)
src/Select.tsx (2)
680-681
: 将 optionFilterProp 透传到 SelectContext:一致性 OK这使 OptionList 能与 useFilterOptions 的匹配字段保持一致。
701-704
: 依赖数组新增 optionFilterProp:必要且正确这样当 optionFilterProp 变更时能触发重建上下文,避免使用陈旧配置。
src/OptionList.tsx (2)
68-69
: 从上下文取 optionFilterProp:方向正确与 Select 侧的 SearchConfig 保持一致,便于统一过滤与定位逻辑。
324-337
: 与虚拟列表的无障碍冗余渲染:保持不变,OKa11y 预渲染邻近项以提升读屏体验,变更未涉及此处。
function includes(test: React.ReactNode, search: string) { | ||
return toArray(test).join('').toUpperCase().includes(search.toUpperCase()); | ||
} | ||
|
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
includes 对 ReactNode 的文本提取不可靠,建议递归提取纯文本以对齐 useFilterOptions 行为
当前 join 可能丢失元素内的文本(如 America),从而错过匹配,导致未能滚到期望位置。可改为递归提取 children 文本:
-function includes(test: React.ReactNode, search: string) {
- return toArray(test).join('').toUpperCase().includes(search.toUpperCase());
-}
+function nodeText(n: React.ReactNode): string {
+ if (n == null || n === false) return '';
+ if (typeof n === 'string' || typeof n === 'number') return String(n);
+ if (Array.isArray(n)) return n.map(nodeText).join('');
+ if (React.isValidElement(n)) return nodeText((n as any).props?.children);
+ return '';
+}
+function includes(test: React.ReactNode, search: string) {
+ return nodeText(test).toUpperCase().includes(search.toUpperCase());
+}
📝 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.
function includes(test: React.ReactNode, search: string) { | |
return toArray(test).join('').toUpperCase().includes(search.toUpperCase()); | |
} | |
function nodeText(n: React.ReactNode): string { | |
if (n == null || n === false) return ''; | |
if (typeof n === 'string' || typeof n === 'number') return String(n); | |
if (Array.isArray(n)) return n.map(nodeText).join(''); | |
if (React.isValidElement(n)) return nodeText((n as any).props?.children); | |
return ''; | |
} | |
function includes(test: React.ReactNode, search: string) { | |
return nodeText(test).toUpperCase().includes(search.toUpperCase()); | |
} |
🤖 Prompt for AI Agents
In src/OptionList.tsx around lines 33 to 36, the current includes function
flattens a ReactNode via toArray().join('') which can drop nested element text
(e.g., <mark>Am</mark>erica) and miss matches; replace it with a recursive
text-extraction routine that walks the ReactNode tree: if the node is
null/boolean return empty string, if string/number convert to string, if an
array map and concat, and if a React element access its props.children and
recursively extract text; then uppercase and perform the includes check so
behavior matches useFilterOptions and nested/marked text is reliably matched.
const index = memoFlattenOptions.findIndex(({ data }) => { | ||
if (searchValue) { | ||
const matchValue = optionFilterProp ? data[optionFilterProp] : data.value; | ||
return includes(matchValue, searchValue); | ||
} | ||
return data.value === value; | ||
}); | ||
|
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
定位所选项/匹配项时不应使用 data.value;应使用扁平项的 item.value,且当 optionFilterProp 未命中时需兜底 fieldNames.value
当自定义 fieldNames(例如 value 映射为 id)时,data.value 可能为 undefined,导致无法在未搜索时滚动到已选项;另外,搜索时未提供 optionFilterProp 时,也应优先按 fieldNames.value 查找,否则会偏离 useFilterOptions 的字段选择。
- const index = memoFlattenOptions.findIndex(({ data }) => {
- if (searchValue) {
- const matchValue = optionFilterProp ? data[optionFilterProp] : data.value;
- return includes(matchValue, searchValue);
- }
- return data.value === value;
- });
+ const index = memoFlattenOptions.findIndex(({ data, value: itemValue, group }) => {
+ if (group) return false;
+ if (searchValue) {
+ const field = optionFilterProp || fieldNames?.value || 'value';
+ const matchValue = data?.[field];
+ return includes(matchValue, searchValue);
+ }
+ // 未搜索时按已选值精确命中,应使用扁平后的 itemValue
+ return itemValue === value;
+ });
+ // 兜底:如未找到匹配项但处于搜索态,则滚到第一个可用选项(即“置顶”期望)
+ const finalIndex = index !== -1 ? index : (searchValue ? getEnabledActiveIndex(0) : -1);
并同步使用 finalIndex:
- if (index !== -1) {
- setActive(index);
+ if (finalIndex !== -1) {
+ setActive(finalIndex);
timeoutId = setTimeout(() => {
- scrollIntoView(index);
+ scrollIntoView(finalIndex);
});
}
🤖 Prompt for AI Agents
In src/OptionList.tsx around lines 168 to 175, the current findIndex uses
data.value which can be undefined when callers pass custom fieldNames (e.g.
value mapped to id) and also doesn't fallback to fieldNames.value when
optionFilterProp is not provided; update the logic to read the flattened item
(use item.value) and when matching use optionFilterProp ? data[optionFilterProp]
: data[fieldNames.value] (or item.value as final fallback) for search matches,
and when not searching compare item.value to the selected value; finally ensure
the computed index is stored/used via finalIndex consistently wherever the
previous index variable was used.
fix: ant-design/ant-design#54884
related issiue: ant-design/ant-design#53752
related PR: #1146
问题分析:
OptionList 中:使用 startsWith + 硬编码 data.value
useFilterOptions 中:使用 includes + 大小写不敏感
过滤逻辑 (useFilterOptions) 和 定位逻辑 (OptionList)应该一致
Summary by CodeRabbit