-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
Add displayInInput option to allow externally controlled search #548
base: master
Are you sure you want to change the base?
Add displayInInput option to allow externally controlled search #548
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次变更引入了一个新的Markdown文件 Changes
Possibly related PRs
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: 3
🧹 Outside diff range and nitpick comments (5)
src/hooks/useSearchConfig.ts (1)
15-15
: 新属性实现正确且保持向后兼容性新增的
displayInInput
属性默认值设置为true
,这样可以确保现有代码不会受到影响,同时为新的外部搜索控制功能提供了支持。建议为新属性添加 JSDoc 文档说明其用途:
searchConfig: ShowSearchType = { matchInputWidth: true, limit: 50, + /** 控制搜索值是否显示在输入框中。默认为 true */ displayInInput: true, };
examples/external-search.tsx (2)
15-16
: 标签命名不一致地址选项数据中的标签命名方式不统一:
- 有些标签包含后缀(如 "马尾-mw","泉州-qz")
- 有些标签没有后缀(如 "福州","杭州")
建议统一标签的命名规范,以提高可维护性。
建议修改如下:
- label: '马尾-mw', + label: '马尾', - label: '泉州-qz', + label: '泉州',Also applies to: 21-22
68-69
: 建议添加类型定义并优化性能当前实现可以通过以下方式改进:
- 添加 TypeScript 类型定义
- 使用 React.memo 优化性能
建议修改如下:
-const Demo = () => { +interface DemoProps {} + +const Demo: React.FC<DemoProps> = React.memo(() => { const [searchValue, setSearchValue] = useState('');tests/search.spec.tsx (2)
80-86
: 建议优化测试辅助函数的实现当前的
doExternalSearch
函数实现与现有的doSearch
函数结构相似,建议考虑以下改进:
- 将通用的模拟事件逻辑抽取为共享函数
- 添加类型注释提高代码可维护性
-function doExternalSearch(wrapper: ReactWrapper, search: string) { +type SimulateChangeParams = { + wrapper: ReactWrapper; + search: string; + selector: string; +}; + +function simulateInputChange({ wrapper, search, selector }: SimulateChangeParams) { wrapper.find('input[data-test="external-search"]').simulate('change', { target: { value: search, }, }); } + +function doExternalSearch(wrapper: ReactWrapper, search: string) { + simulateInputChange({ + wrapper, + search, + selector: 'input[data-test="external-search"]' + }); +}
100-116
: 建议增加更多测试场景当前测试用例已经覆盖了基本的搜索和选择功能,建议添加以下测试场景:
- 验证 displayInInput 为 false 时输入框是否正确显示
- 测试搜索值为空时的行为
- 测试边界情况(如特殊字符搜索)
// 建议添加的测试用例 it('should not display search value in input when displayInInput is false', () => { // 实现代码 }); it('should handle empty search value correctly', () => { // 实现代码 }); it('should handle special characters in search', () => { // 实现代码 });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
docs/demo/external-search.md
(1 hunks)examples/external-search.tsx
(1 hunks)src/Cascader.tsx
(1 hunks)src/hooks/useSearchConfig.ts
(2 hunks)tests/search.spec.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/demo/external-search.md
🔇 Additional comments (3)
src/hooks/useSearchConfig.ts (1)
33-33
: 返回值修改逻辑合理
使用 !!searchConfig.displayInInput
来确保返回布尔值是一个好的实践。这个改动与新功能需求完全匹配,并且保持了返回类型的一致性。
tests/search.spec.tsx (1)
4-4
: 导入声明看起来没有问题!
新增的 useState 导入用于管理测试组件中的搜索状态,符合组件需求。
src/Cascader.tsx (1)
60-60
: 🛠️ Refactor suggestion
需要实现属性的具体逻辑并添加文档注释
该属性的添加符合需求,但还需要以下改进:
- 需要在接口中添加属性的文档注释,说明其用途和预期行为
- 需要在组件中实现该属性的具体逻辑,目前只添加了类型定义
建议添加如下文档注释:
export interface ShowSearchType<
OptionType extends DefaultOptionType = DefaultOptionType,
ValueField extends keyof OptionType = keyof OptionType,
> {
+ /**
+ * 是否在输入框中显示搜索值
+ * @default true
+ */
displayInInput?: boolean;
✅ Verification successful
displayInInput
属性已正确实现和使用
根据代码分析,该属性已经:
- 在
useSearchConfig
钩子中设置了默认值true
- 在实际组件中被正确使用,如测试和示例代码所示
- 通过
useSearchConfig
钩子返回值提供给组件使用
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查属性在组件中的使用情况
# 搜索组件中是否使用了 displayInInput 属性
rg "displayInInput" --type ts
Length of output: 465
Script:
#!/bin/bash
# 检查 useSearchConfig 中的实现
ast-grep --pattern 'export function useSearchConfig' -A 30
Length of output: 60
Script:
#!/bin/bash
# 检查 useSearchConfig.ts 的完整实现
cat src/hooks/useSearchConfig.ts
Length of output: 994
Script:
#!/bin/bash
# 检查属性的使用场景
rg "useSearchConfig" -A 5 --type ts
Length of output: 1062
<input | ||
value={searchValue} | ||
onChange={e => setSearchValue(e.target.value)} | ||
placeholder="External Search" | ||
/> |
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
改进输入框的可访问性和用户体验
搜索输入框缺少一些重要的可访问性和用户体验特性:
- 缺少
aria-label
属性 - 未指定输入框类型
- 建议添加搜索图标以提升可识别性
建议修改如下:
<input
value={searchValue}
onChange={e => setSearchValue(e.target.value)}
placeholder="External Search"
+ type="search"
+ aria-label="外部搜索输入框"
+ className="external-search-input"
/>
另外,建议在输入框旁添加搜索图标:
import { SearchOutlined } from '@ant-design/icons';
// 在输入框前添加图标
<span className="search-icon">
<SearchOutlined />
</span>
value={searchValue} | ||
onChange={e => setSearchValue(e.target.value)} | ||
placeholder="External Search" |
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
建议添加输入验证和错误处理
当前实现缺少输入验证和错误处理机制:
- 没有对搜索值进行长度限制
- 没有处理特殊字符
- 缺少防抖处理
建议添加以下功能:
+import { useMemo } from 'react';
+import debounce from 'lodash/debounce';
+
const Demo = () => {
const [searchValue, setSearchValue] = useState('');
+ const [error, setError] = useState('');
+
+ const handleSearch = useMemo(
+ () =>
+ debounce((value: string) => {
+ if (value.length > 50) {
+ setError('搜索内容不能超过50个字符');
+ return;
+ }
+ setError('');
+ setSearchValue(value);
+ }, 300),
+ [],
+ );
+
return (
// ...
<input
value={searchValue}
- onChange={e => setSearchValue(e.target.value)}
+ onChange={e => handleSearch(e.target.value)}
placeholder="External Search"
+ maxLength={50}
/>
+ {error && <div className="error-message">{error}</div>}
Committable suggestion skipped: line range outside the PR's diff.
const ExternallyControlledSearch = () => { | ||
const [searchValue,setSearchValue] = useState('') | ||
return ( | ||
<> | ||
<input data-test="external-search" value={searchValue} onChange={e => setSearchValue(e.target.value)} /> | ||
<Cascader options={options} searchValue={searchValue} onChange={onChange} onSearch={onSearch} open showSearch={{displayInInput:false}} />, | ||
</> | ||
); | ||
} |
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.
需要修复测试组件中的语法和类型问题
ExternallyControlledSearch 组件存在以下问题:
- JSX中存在多余的逗号
- 代码格式不统一
- 缺少类型注释
-const ExternallyControlledSearch = () => {
- const [searchValue,setSearchValue] = useState('')
+interface ExternallyControlledSearchProps {}
+
+const ExternallyControlledSearch: React.FC<ExternallyControlledSearchProps> = () => {
+ const [searchValue, setSearchValue] = useState<string>('')
return (
<>
<input data-test="external-search" value={searchValue} onChange={e => setSearchValue(e.target.value)} />
- <Cascader options={options} searchValue={searchValue} onChange={onChange} onSearch={onSearch} open showSearch={{displayInInput:false}} />,
+ <Cascader
+ options={options}
+ searchValue={searchValue}
+ onChange={onChange}
+ onSearch={onSearch}
+ open
+ showSearch={{ displayInInput: false }}
+ />
</>
);
}
📝 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.
const ExternallyControlledSearch = () => { | |
const [searchValue,setSearchValue] = useState('') | |
return ( | |
<> | |
<input data-test="external-search" value={searchValue} onChange={e => setSearchValue(e.target.value)} /> | |
<Cascader options={options} searchValue={searchValue} onChange={onChange} onSearch={onSearch} open showSearch={{displayInInput:false}} />, | |
</> | |
); | |
} | |
interface ExternallyControlledSearchProps {} | |
const ExternallyControlledSearch: React.FC<ExternallyControlledSearchProps> = () => { | |
const [searchValue, setSearchValue] = useState<string>('') | |
return ( | |
<> | |
<input data-test="external-search" value={searchValue} onChange={e => setSearchValue(e.target.value)} /> | |
<Cascader | |
options={options} | |
searchValue={searchValue} | |
onChange={onChange} | |
onSearch={onSearch} | |
open | |
showSearch={{ displayInInput: false }} | |
/> | |
</> | |
); | |
} |
b69ab40
to
d40559e
Compare
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 (5)
examples/external-search.tsx (3)
1-3
: 建议优化导入语句的组织结构建议将导入语句按照以下方式分组:
- React 相关导入
- 本地组件导入
- 样式导入
import React, { useState } from 'react'; -import '../assets/index.less'; import Cascader from '../src'; +import '../assets/index.less';
5-66
: 建议统一地址选项的标签格式当前数据中存在不一致的标签格式:
- 有的使用连字符(如:"马尾-mw")
- 有的使用纯中文(如:"余杭")
- 有的带后缀(如:"泉州-qz")
建议统一标签格式以提供一致的用户体验。
72-91
: Cascader 配置实现符合需求通过以下方式很好地实现了外部搜索控制:
showSearch={{ displayInInput: false }}
确保搜索值不在输入框中显示searchValue
和setSearchValue
实现了正确的状态管理dropdownRender
合理地集成了外部搜索输入框建议添加注释说明这种实现方式的用途和原理。
const Demo = () => { const [searchValue, setSearchValue] = useState(''); + // 使用外部搜索输入框控制 Cascader 的搜索行为 + // displayInInput: false 确保搜索值不在 Cascader 输入框中显示 return (tests/search.spec.tsx (2)
76-117
: 测试用例格式需要优化测试用例中存在以下格式问题:
- 76行:
it('externally controlled search',() => {
中括号后缺少空格- 测试用例的结构可以更清晰,建议在不同测试场景之间添加空行
- 测试用例的命名应该更具描述性,建议改为
it('should handle externally controlled search correctly'
建议按照以下方式修改:
- it('externally controlled search',() => { + it('should handle externally controlled search correctly', () => {
76-117
: 建议增加更多测试场景当前的测试用例只覆盖了基本功能,建议添加以下测试场景:
- 空搜索值的处理
- 特殊字符的处理
- 极长搜索值的处理
- 搜索值改变时的性能测试
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
docs/demo/external-search.md
(1 hunks)examples/external-search.tsx
(1 hunks)src/Cascader.tsx
(1 hunks)src/hooks/useSearchConfig.ts
(2 hunks)tests/search.spec.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/demo/external-search.md
- src/Cascader.tsx
- src/hooks/useSearchConfig.ts
🔇 Additional comments (4)
examples/external-search.tsx (2)
80-84
: 需要改进输入框的可访问性和用户体验
与之前的审查意见一致,搜索输入框仍需要改进可访问性和用户体验。
81-83
: 建议添加输入验证和防抖处理
与之前的审查意见一致,建议添加输入验证和防抖机制。
tests/search.spec.tsx (2)
4-4
: 导入声明看起来没有问题!
从React中导入useState钩子是正确的,这对于新增的ExternallyControlledSearch组件的状态管理是必要的。
89-97
: 需要修复测试组件中的语法和类型问题
这些问题在之前的代码审查中已经提到过,包括:
- JSX中存在多余的逗号
- 代码格式不统一
- 缺少类型注释
请参考之前的审查意见进行修复。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #548 +/- ##
=======================================
Coverage 99.83% 99.83%
=======================================
Files 23 23
Lines 614 614
Branches 187 187
=======================================
Hits 613 613
Misses 1 1 ☔ View full report in Codecov by Sentry. |
// Change | ||
wrapper.clickOption(0, 0); | ||
expect(onChange).toHaveBeenCalledWith(['bamboo', 'little', 'fish'], expect.anything()); | ||
}) |
Check notice
Code scanning / CodeQL
Semicolon insertion Note test
cascader accepts a
searchValue
prop which means it can be externally controlled.This implies that the search can come from anywhere outside.
I have a usecase where I want an external field - one I render inside the dropdown menu - to control the search value for cascader, but I don't want to display this value in the Cascader's input.
I've included updated code, a demo, and a test.
Summary by CodeRabbit
新功能
external-search.md
,提供外部搜索功能的演示文档。Demo
组件,允许用户通过Cascader
组件进行层级地址数据的搜索。Cascader
组件中增加了displayInInput
属性,以增强搜索输入的显示控制。测试
Cascader.Search
组件添加了新的测试用例,验证外部控制搜索功能的正确性。