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

Add displayInInput option to allow externally controlled search #548

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ajenkins-mparticle
Copy link

@ajenkins-mparticle ajenkins-mparticle commented Nov 28, 2024

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组件添加了新的测试用例,验证外部控制搜索功能的正确性。

Copy link

vercel bot commented Nov 28, 2024

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

Name Status Preview Comments Updated (UTC)
cascader ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 7:10pm

Copy link

coderabbitai bot commented Nov 28, 2024

Walkthrough

本次变更引入了一个新的Markdown文件external-search.md,用于文档化与外部搜索功能相关的演示。与此同时,新增了一个React组件Demo,实现了基于Cascader组件的搜索功能,并通过状态管理来跟踪输入值。此外,Cascader组件的接口ShowSearchType中新增了一个可选属性displayInInput,以增强搜索输入的显示控制。测试文件中也增加了对外部控制搜索功能的测试用例。

Changes

文件路径 变更摘要
docs/demo/external-search.md 新增Markdown文件,包含标题、导航和外部TypeScript文件引用。
examples/external-search.tsx 新增React组件Demo,实现基于Cascader的搜索功能。
src/Cascader.tsx ShowSearchType接口中新增可选属性displayInInput
src/hooks/useSearchConfig.ts 修改useSearchConfig函数,返回值基于displayInInput属性。
tests/search.spec.tsx 新增测试用例以验证外部控制搜索功能的正确性。

Possibly related PRs

  • feat: optimize search #530: 本PR引入了一个新的文档文件,用于演示与多重搜索功能相关的内容,与主PR中的外部搜索功能概念相似。
  • feat: test #531: 本PR增强了对Cascader.Search组件的测试用例,与主PR中的搜索功能相关。
  • fix: search no scroll #532: 本PR优化了在存在搜索值时的滚动行为,直接与主PR中讨论的搜索功能相关。
  • fix: antd-issue-51248 #541: 本PR修改了useSearchConfig函数,直接与主PR中提到的搜索功能相关。

🐇 在文档中新增一篇,
外部搜索功能真精彩!
组件Demo闪亮登场,
搜索轻松不再难!
Cascader助力更出色,
测试验证功能全!


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

@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: 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: 建议添加类型定义并优化性能

当前实现可以通过以下方式改进:

  1. 添加 TypeScript 类型定义
  2. 使用 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 函数结构相似,建议考虑以下改进:

  1. 将通用的模拟事件逻辑抽取为共享函数
  2. 添加类型注释提高代码可维护性
-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: 建议增加更多测试场景

当前测试用例已经覆盖了基本的搜索和选择功能,建议添加以下测试场景:

  1. 验证 displayInInput 为 false 时输入框是否正确显示
  2. 测试搜索值为空时的行为
  3. 测试边界情况(如特殊字符搜索)
// 建议添加的测试用例
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

📥 Commits

Reviewing files that changed from the base of the PR and between 751451f and b69ab40.

📒 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

需要实现属性的具体逻辑并添加文档注释

该属性的添加符合需求,但还需要以下改进:

  1. 需要在接口中添加属性的文档注释,说明其用途和预期行为
  2. 需要在组件中实现该属性的具体逻辑,目前只添加了类型定义

建议添加如下文档注释:

 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

Comment on lines +80 to +84
<input
value={searchValue}
onChange={e => setSearchValue(e.target.value)}
placeholder="External Search"
/>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

改进输入框的可访问性和用户体验

搜索输入框缺少一些重要的可访问性和用户体验特性:

  1. 缺少 aria-label 属性
  2. 未指定输入框类型
  3. 建议添加搜索图标以提升可识别性

建议修改如下:

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

Comment on lines +81 to +83
value={searchValue}
onChange={e => setSearchValue(e.target.value)}
placeholder="External Search"
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议添加输入验证和错误处理

当前实现缺少输入验证和错误处理机制:

  1. 没有对搜索值进行长度限制
  2. 没有处理特殊字符
  3. 缺少防抖处理

建议添加以下功能:

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

Comment on lines +89 to +97
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}} />,
</>
);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

需要修复测试组件中的语法和类型问题

ExternallyControlledSearch 组件存在以下问题:

  1. JSX中存在多余的逗号
  2. 代码格式不统一
  3. 缺少类型注释
-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.

Suggested change
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 }}
/>
</>
);
}

Copy link

@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 (5)
examples/external-search.tsx (3)

1-3: 建议优化导入语句的组织结构

建议将导入语句按照以下方式分组:

  1. React 相关导入
  2. 本地组件导入
  3. 样式导入
 import React, { useState } from 'react';
-import '../assets/index.less';
 import Cascader from '../src';
+import '../assets/index.less';

5-66: 建议统一地址选项的标签格式

当前数据中存在不一致的标签格式:

  • 有的使用连字符(如:"马尾-mw")
  • 有的使用纯中文(如:"余杭")
  • 有的带后缀(如:"泉州-qz")

建议统一标签格式以提供一致的用户体验。


72-91: Cascader 配置实现符合需求

通过以下方式很好地实现了外部搜索控制:

  1. showSearch={{ displayInInput: false }} 确保搜索值不在输入框中显示
  2. searchValuesetSearchValue 实现了正确的状态管理
  3. dropdownRender 合理地集成了外部搜索输入框

建议添加注释说明这种实现方式的用途和原理。

 const Demo = () => {
   const [searchValue, setSearchValue] = useState('');
+  // 使用外部搜索输入框控制 Cascader 的搜索行为
+  // displayInInput: false 确保搜索值不在 Cascader 输入框中显示
   return (
tests/search.spec.tsx (2)

76-117: 测试用例格式需要优化

测试用例中存在以下格式问题:

  1. 76行:it('externally controlled search',() => { 中括号后缺少空格
  2. 测试用例的结构可以更清晰,建议在不同测试场景之间添加空行
  3. 测试用例的命名应该更具描述性,建议改为 it('should handle externally controlled search correctly'

建议按照以下方式修改:

-  it('externally controlled search',() => {
+  it('should handle externally controlled search correctly', () => {

76-117: 建议增加更多测试场景

当前的测试用例只覆盖了基本功能,建议添加以下测试场景:

  1. 空搜索值的处理
  2. 特殊字符的处理
  3. 极长搜索值的处理
  4. 搜索值改变时的性能测试
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b69ab40 and d40559e.

📒 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: 需要修复测试组件中的语法和类型问题

这些问题在之前的代码审查中已经提到过,包括:

  1. JSX中存在多余的逗号
  2. 代码格式不统一
  3. 缺少类型注释

请参考之前的审查意见进行修复。

Copy link

codecov bot commented Nov 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.83%. Comparing base (751451f) to head (d40559e).

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

// Change
wrapper.clickOption(0, 0);
expect(onChange).toHaveBeenCalledWith(['bamboo', 'little', 'fish'], expect.anything());
})

Check notice

Code scanning / CodeQL

Semicolon insertion Note test

Avoid automated semicolon insertion (94% of all statements in
the enclosing function
have an explicit semicolon).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant