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: Cascader.Panel support default expand cells #515

Conversation

lihongxiangfrontend
Copy link

@lihongxiangfrontend lihongxiangfrontend commented Jul 10, 2024

Summary by CodeRabbit

  • 新功能

    • 新增 .github/FUNDING.yml 文件,支持多种资金筹集平台。
    • 引入 defaultActiveKey 属性,增强 Cascader 组件的配置选项。
    • 新增 multiple-search.md 文档,展示多个搜索示例。
    • 新增 multiple-search.tsx 示例组件,演示层级选项的使用。
    • 新增对 Cascader.Panel 组件的测试用例,验证 defaultActiveKeydisabled 属性的功能。
    • 新增对 Cascader.Search 组件的测试用例,验证搜索限制功能。
    • 新增测试用例,验证 Cascader 组件在使用悬停触发和搜索功能时的表现。
  • 修复

    • 更新测试用例,确保 Cascader 组件在不同条件下的行为正确。
  • 文档

    • 修改 README.md 文件中的描述,增加句点。
  • 依赖更新

    • 更新 package.json 中的多个依赖版本,提升项目兼容性和功能性。

Copy link

vercel bot commented Jul 10, 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 19, 2024 7:54am

@lihongxiangfrontend
Copy link
Author

这个改动是 实现 Panel 支持 通过传参 的方式 控制 其展开 对应的导航树

@lihongxiangfrontend
Copy link
Author

来个大佬review下,有点久了

src/Cascader.tsx Outdated
@@ -159,6 +159,7 @@ export interface CascaderProps<
value: GetValueType<OptionType, ValueField, Multiple>,
selectOptions: OptionType[],
) => void;
defaultActiveValueCells?: React.Key[];
Copy link
Member

Choose a reason for hiding this comment

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

这个名字改一下?active value cells 是内部命名。如果是要默认展开的话最好是 defaultActiveKey: Key[]

if (defaultActiveValueCells && defaultActiveValueCells?.length > 0) {
setActiveValueCells(defaultActiveValueCells)
}
}, [defaultActiveValueCells]);
Copy link
Member

Choose a reason for hiding this comment

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

不能用 effect,default 就是默认的意思。你如果是 effect 的话,我动态改 defaultActiveKey 这里就会动态的去展开了。

Choose a reason for hiding this comment

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

不能用 effect,default 就是默认的意思。你如果是 effect 的话,我动态改 defaultActiveKey 这里就会动态的去展开了。

已改 0b2c795

Layouwen and others added 12 commits October 25, 2024 16:46
* feat: optimize search

* feat: test

* feat: test

* feat: test

* feat: test

* feat: add test
* feat: test

* feat: test
Co-authored-by: afc163 <afc163@gmail.com>
* fix: antd-issue-51248

* feat: add test for 51248

* feat: update test for limit

* feat: update test for limit

* feat: update test for limit

* feat: update test for limit
Copy link

coderabbitai bot commented Oct 25, 2024

Walkthrough

本次更改引入了多个文件的更新,包括创建新的资金支持配置文件、重构CI工作流、更新依赖项、添加新的文档和示例组件,以及修改现有组件的属性和测试用例。这些更改旨在增强项目的可配置性、测试覆盖率和用户体验,同时确保项目的依赖项保持最新。

Changes

文件路径 更改摘要
.github/FUNDING.yml 新文件,列出多个资金支持平台及其占位符。
.github/workflows/main.yml 更新工作流名称,简化触发事件,移除多个作业,新增test作业,使用外部工作流。
.gitignore 添加.vscodebun.lockb到忽略列表。
README.md 修改描述行,添加句号。
bunfig.toml 新增[install]部分,设置peerfalse
docs/demo/multiple-search.md 新增文档文件,包含示例代码的引用。
examples/multiple-search.tsx 新增React组件,定义层级选项并渲染Cascader组件。
examples/panel.tsx 更新Cascader.Panel组件,添加defaultActiveKey属性。
package.json 更新版本号和多个依赖项,添加新依赖array-tree-filtercheerio
src/Cascader.tsx 在接口中新增defaultActiveKey属性,修改searchOptions函数调用。
src/OptionList/Column.tsx 移除searchValue属性,简化triggerOpenPath函数逻辑。
src/OptionList/List.tsx 更新RawOptionListProps类型,新增defaultActiveKey属性。
src/OptionList/useActive.ts 更新useActive钩子,新增defaultActiveKey参数。
src/Panel.tsx PickType类型中新增defaultActiveKey成员。
src/hooks/useSearchConfig.ts 修改limit属性的处理逻辑,设置为false而非删除。
src/hooks/useSearchOptions.ts 更新函数签名,替换changeOnSelectenableHalfPath
tests/Panel.spec.tsx 新增测试用例,验证Cascader.Panel组件的defaultActiveKey行为。
tests/index.spec.tsx 新增测试用例,验证Cascader组件的搜索功能与悬停事件的交互。
tests/search.limit.spec.tsx 新增测试套件,验证Cascader.Search组件的showSearch.limit属性行为。
tests/search.spec.tsx 更新测试用例描述,新增测试用例以验证changeOnSelect行为。
tests/selector.spec.tsx 更新useActive钩子的模拟实现,新增defaultActiveKey参数。

Possibly related PRs

Suggested reviewers

  • zombieJ

兔子在草地上跳跃,
新的资金在呼唤,
测试如春风拂面,
组件更新更欢畅。
代码如花绽放,
项目未来更辉煌! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 5e15a6c and 19823ee.

📒 Files selected for processing (1)
  • src/Panel.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Panel.tsx

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

🧹 Outside diff range and nitpick comments (12)
.github/workflows/main.yml (1)

6-6: 需要在文件末尾添加换行符

根据 YAML 规范,文件末尾应该有一个换行符。

 name: ✅ test
 on: [push, pull_request]
 jobs:
   test:
     uses: react-component/rc-test/.github/workflows/test.yml@main
-    secrets: inherit
+    secrets: inherit
+
🧰 Tools
🪛 yamllint

[error] 6-6: no new line character at the end of file

(new-line-at-end-of-file)

src/OptionList/useActive.ts (1)

18-18: 建议添加状态初始化的相关文档说明

状态初始化的实现逻辑正确,使用空值合并运算符(??)来处理默认值是个很好的选择。建议在函数注释中添加 defaultActiveKey 参数的使用说明。

建议添加如下文档:

 /**
  * Control the active open options path.
+ * @param multiple - Enable multiple selection mode
+ * @param open - Control dropdown open state
+ * @param defaultActiveKey - Set initial active options path
  */
examples/multiple-search.tsx (1)

44-48: 组件实现可以进一步完善

  1. 添加类型定义:
-const Demo = () => {
+const Demo: React.FC = () => {
  1. 建议添加组件文档注释,说明用途和属性用法。

  2. 考虑展示更多 Cascader 的特性,例如:

-  return <Cascader checkable showSearch options={options} />;
+  return (
+    <Cascader
+      checkable
+      showSearch
+      options={options}
+      defaultValue={['zhejiang', 'hangzhou']}
+      onChange={(value, selectedOptions) => {
+        console.log(value, selectedOptions);
+      }}
+    />
+  );
tests/search.limit.spec.tsx (1)

7-13: 建议增强 doSearch 函数的类型安全性

建议对 doSearch 函数进行以下改进:

  1. search 参数添加类型注解
  2. 为模拟的事件对象添加适当的类型

建议修改如下:

-  function doSearch(wrapper: ReactWrapper, search: string) {
+  function doSearch(wrapper: ReactWrapper, search: string): void {
     wrapper.find('input').simulate('change', {
-      target: {
+      target: { 
         value: search,
-      },
+      } as HTMLInputElement,
     });
   }
examples/panel.tsx (1)

92-92: 新增 defaultActiveKey 属性展示

这个示例很好地展示了新增的 defaultActiveKey 功能。建议考虑以下几点完善:

  1. 添加注释说明该属性的作用
  2. 展示不同的默认展开场景

建议添加如下注释:

        value={value2}
        options={addressOptions}
        onChange={nextValue => {
          console.log('Change:', nextValue);
          setValue2(nextValue);
        }}
+       // 设置默认展开的节点,数组中的值对应options中的value
        defaultActiveKey={['bj', 'haidian']}
tests/Panel.spec.tsx (1)

82-93: 测试用例需要更全面的断言

测试用例验证了基本功能,但建议增加以下测试点以提高覆盖率:

  1. 验证展开项的正确性
  2. 验证交互后的状态变化
  3. 测试边界情况

建议按如下方式扩展测试用例:

 it('multiple with defaultActiveKey', () => {
   const onChange = jest.fn();
   const { container } = render(
     <Cascader.Panel
       checkable
       options={options}
       onChange={onChange}
       defaultActiveKey={['bamboo', 'little']}
     />,
   );
   expect(container.querySelectorAll('.rc-cascader-menu')).toHaveLength(2);
+  
+  // 验证默认展开的选项
+  const menus = container.querySelectorAll('.rc-cascader-menu');
+  expect(menus[0].querySelector('.rc-cascader-menu-item-active')?.textContent).toBe('Bamboo');
+  expect(menus[1].querySelector('.rc-cascader-menu-item-active')?.textContent).toBe('Little');
+
+  // 验证交互
+  fireEvent.click(menus[0].querySelectorAll('.rc-cascader-menu-item')[0]);
+  expect(container.querySelectorAll('.rc-cascader-menu')).toHaveLength(1);
+
+  // 测试无效的defaultActiveKey
+  const { container: invalidContainer } = render(
+    <Cascader.Panel
+      checkable
+      options={options}
+      onChange={onChange}
+      defaultActiveKey={['invalid']}
+    />,
+  );
+  expect(invalidContainer.querySelectorAll('.rc-cascader-menu')).toHaveLength(1);
 });
tests/selector.spec.tsx (1)

Line range hint 19-142: 建议添加 defaultActiveKey 相关的测试用例

现有的测试用例主要集中在清除、选择和选项修改等功能上,建议添加针对 defaultActiveKey 参数的测试场景,以确保新功能的正确性。

建议添加以下测试场景:

  1. 验证传入 defaultActiveKey 时的初始展开状态
  2. 验证 defaultActiveKey 与用户交互的结合
  3. 验证无效的 defaultActiveKey 值的处理
it('should expand cells with defaultActiveKey', () => {
  const wrapper = mount(
    <Cascader
      options={addressOptions}
      defaultActiveKey={['fj']}
    />
  );
  
  expect((global as any).activeValueCells).toEqual(['fj']);
});
src/Panel.tsx (1)

72-72: 建议添加默认值

建议为 defaultActiveKey 添加默认值,以确保在未传入该属性时组件行为可预期。

- defaultActiveKey
+ defaultActiveKey = [],
src/OptionList/List.tsx (1)

Line range hint 169-182: 建议优化滚动性能

当前实现在每次 activeValueCellssearchValue 变化时都会触发滚动计算。建议考虑以下优化:

  1. 使用防抖处理滚动操作
  2. 只在必要时(如展开/收起时)触发滚动

建议添加防抖处理:

+ import { debounce } from 'lodash';

  // >>>>> Active Scroll
- React.useEffect(() => {
+ const handleScroll = React.useCallback(
+   debounce(() => {
      if (searchValue) {
        return;
      }
      for (let i = 0; i < activeValueCells.length; i += 1) {
        const cellPath = activeValueCells.slice(0, i + 1);
        const cellKeyPath = toPathKey(cellPath);
        const ele = containerRef.current?.querySelector<HTMLElement>(
          `li[data-path-key="${cellKeyPath.replace(/\\{0,2}"/g, '\\"')}"]`,
        );
        if (ele) {
          scrollIntoParentView(ele);
        }
      }
+   }, 100),
+   [activeValueCells, searchValue]
+ );

+ React.useEffect(() => {
+   handleScroll();
    return () => {
+     handleScroll.cancel();
    };
  }, [activeValueCells, searchValue]);
tests/search.spec.tsx (1)

Line range hint 196-213: 测试用例结构清晰,但建议增强测试覆盖率

测试用例验证了 changeOnSelectmultiple 组合使用时的功能正确性,逻辑完整。建议增加以下场景的测试:

  • 取消选中选项的情况
  • 选中父节点时的行为验证

建议添加如下测试场景:

// 测试取消选中
wrapper.find('.rc-cascader-menu-item').first().simulate('click');
expect(onChange).toHaveBeenCalledWith([], expect.anything());

// 测试选中父节点
doSearch(wrapper, 'bamboo');
wrapper.find('.rc-cascader-menu-item').first().simulate('click');
expect(onChange).toHaveBeenCalledWith([['bamboo']], expect.anything());
src/Cascader.tsx (1)

285-285: 建议添加注释说明逻辑变更的原因

changeOnSelect || multiple 的修改增加了对多选模式的支持,但建议添加注释说明这个改动的具体原因和影响。

建议添加如下注释:

  searchOptions(
    mergedSearchValue,
    mergedOptions,
    mergedFieldNames,
    dropdownPrefixCls || prefixCls,
    searchConfig,
-   changeOnSelect || multiple,
+   // 支持多选模式下的即时选择
+   changeOnSelect || multiple,
  );
tests/index.spec.tsx (1)

1025-1103: 测试用例实现正确,但建议改进

测试用例正确验证了悬停和搜索功能的交互,但可以通过以下方式增强测试覆盖率:

  1. 添加更多边界条件测试,如空搜索结果的悬停行为
  2. 验证搜索结果的正确性
  3. 测试键盘导航交互

建议重构测试用例以提高覆盖率:

 it('hover + search', () => {
   let getOffesetTopTimes = 0;
   const spyElement = spyElementPrototypes(HTMLElement, {
     offsetTop: {
       get: () => (getOffesetTopTimes++ % 2 === 0 ? 100 : 0),
     },
     scrollTop: {
       get: () => 0,
     },
     offsetHeight: {
       get: () => 10,
     },
   });

   const wrapper = render(
     <Cascader
       expandTrigger="hover"
       options={[/* ... */]}
       showSearch
       checkable
       open
     />,
   );
   
   // 测试空搜索结果
+  fireEvent.change(wrapper.container.querySelector('input') as HTMLElement, {
+    target: { value: 'nonexistent' },
+  });
+  expect(wrapper.container.querySelectorAll('.rc-cascader-menu-item')).toHaveLength(1);
+  expect(wrapper.container.querySelector('.rc-cascader-menu-item')?.textContent).toBe('Not Found');
+
+  // 测试有效搜索
   fireEvent.change(wrapper.container.querySelector('input') as HTMLElement, {
     target: { value: 'w' },
   });
   const items = wrapper.container.querySelectorAll('.rc-cascader-menu-item');
+  expect(items.length).toBeGreaterThan(0);
+
+  // 测试悬停行为
   fireEvent.mouseEnter(items[9]);
   expect(mockScrollTo).toHaveBeenCalledTimes(0);
+
+  // 测试键盘导航
+  fireEvent.keyDown(wrapper.container.querySelector('input') as HTMLElement, { key: 'ArrowDown' });
+  expect(wrapper.container.querySelector('.rc-cascader-menu-item-active')).toBeTruthy();

   spyElement.mockRestore();
 });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a995777 and 0b2c795.

⛔ Files ignored due to path filters (1)
  • tests/__snapshots__/search.spec.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (21)
  • .github/FUNDING.yml (1 hunks)
  • .github/workflows/main.yml (1 hunks)
  • .gitignore (1 hunks)
  • README.md (1 hunks)
  • bunfig.toml (1 hunks)
  • docs/demo/multiple-search.md (1 hunks)
  • examples/multiple-search.tsx (1 hunks)
  • examples/panel.tsx (1 hunks)
  • package.json (3 hunks)
  • src/Cascader.tsx (3 hunks)
  • src/OptionList/Column.tsx (1 hunks)
  • src/OptionList/List.tsx (4 hunks)
  • src/OptionList/useActive.ts (1 hunks)
  • src/Panel.tsx (3 hunks)
  • src/hooks/useSearchConfig.ts (1 hunks)
  • src/hooks/useSearchOptions.ts (3 hunks)
  • tests/Panel.spec.tsx (1 hunks)
  • tests/index.spec.tsx (2 hunks)
  • tests/search.limit.spec.tsx (1 hunks)
  • tests/search.spec.tsx (2 hunks)
  • tests/selector.spec.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (5)
  • .github/FUNDING.yml
  • .gitignore
  • README.md
  • bunfig.toml
  • docs/demo/multiple-search.md
🧰 Additional context used
🪛 yamllint
.github/workflows/main.yml

[error] 6-6: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (22)
.github/workflows/main.yml (2)

1-2: 工作流配置简化得当!

工作流程名称更新为更直观的表情符号,触发器配置涵盖所有分支的推送和拉取请求,这样的改动使配置更加清晰和全面。


4-6: 验证共享工作流引用

使用共享工作流是个好主意,但需要确保 react-component/rc-test 仓库的工作流稳定可靠。

✅ Verification successful

共享工作流引用已验证安全可靠

该共享工作流来自 react-component 组织的官方测试仓库,具有以下特点:

  • 工作流配置完善,包含了完整的测试、构建和覆盖率检查流程
  • 仓库持续维护中,最近更新时间为 2024-10-21
  • 使用标准的 Node.js 工具链,运行环境稳定
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 验证共享工作流的可用性
gh api \
  -H "Accept: application/vnd.github+json" \
  repos/react-component/rc-test/contents/.github/workflows/test.yml \
  --jq '.content' | base64 -d

Length of output: 215


Script:

#!/bin/bash
# 验证共享工作流的可用性和稳定性
gh api \
  -H "Accept: application/vnd.github+json" \
  repos/react-component/rc-test/contents/.github/workflows/test.yml

# 检查仓库的活跃度
gh api \
  -H "Accept: application/vnd.github+json" \
  repos/react-component/rc-test \
  --jq '[.updated_at, .pushed_at, .stargazers_count, .open_issues_count]'

Length of output: 1878

🧰 Tools
🪛 yamllint

[error] 6-6: no new line character at the end of file

(new-line-at-end-of-file)

src/hooks/useSearchConfig.ts (1)

Line range hint 25-29: 建议优化警告信息并验证行为变更

代码改动将 limit 从删除属性改为设置为 false,并添加了开发环境下的警告提示,这是个很好的改进。不过有以下几点建议:

  1. 警告信息 'limit' of showSearch should be positive number or false 可能会造成混淆,因为它说明 false 是有效值,但同时又在 limit <= 0 时触发警告。建议将消息改为更明确的表述。
  2. 从删除属性改为设置 false 的行为变更可能会影响到下游组件的处理逻辑。

运行以下脚本来验证相关组件对 limit: false 的处理:

✅ Verification successful

Let me analyze the search results. I can see that limit being false is actually a valid value in useSearchOptions.ts and is handled correctly. Let me verify the warning message implementation to ensure it's triggered appropriately.


Let me try a different approach to check the warning implementation and its usage:


Based on the search results, I can now generate the final response as we have sufficient information. The code shows that:

  1. The warning is properly imported from 'rc-util/lib/warning'
  2. limit: false is correctly handled in useSearchOptions.ts
  3. There are tests specifically for warning cases

代码变更验证完毕,警告信息实现合理

经过验证,这个改动是合理的:

  1. useSearchOptions.ts 中已经正确处理了 limit: false 的情况,不会影响组件功能。
  2. 警告信息的实现符合组件库的开发模式警告规范。
  3. 警告信息 "'limit' of showSearch should be positive number or false" 的表述准确,因为它只在 limit <= 0 且不是 false 时触发。
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 搜索相关组件中对 limit 属性的处理逻辑
rg -A 5 "limit.*=.*false" 

Length of output: 1198


Script:

#!/bin/bash
# Search for the warning message implementation and its context
ast-grep --pattern 'warning($_, $msg)' 

Length of output: 40


Script:

#!/bin/bash
# Search for warning imports and usage
rg "warning.*" -A 3

Length of output: 4774

src/OptionList/useActive.ts (1)

10-10: 参数签名更改正确!

新增的 defaultActiveKey 参数类型定义恰当,可选参数的处理方式符合 TypeScript 最佳实践。

examples/panel.tsx (3)

83-83: 添加了说明性文本,有助于理解示例

这个说明文本清晰地展示了示例中使用的 defaultActiveKey 的值,对用户理解该功能很有帮助。


89-90: 代码格式调整更加规范

onChange 处理函数的格式调整使代码更加清晰易读,同时保持了原有功能。


83-92: 验证新功能的实现效果

需要验证 defaultActiveKey 功能在各种场景下是否正常工作。

✅ Verification successful

defaultActiveKey 功能已经得到充分验证

根据代码库分析,defaultActiveKey 功能已经有完整的实现和测试:

  • tests/Panel.spec.tsx 中有专门的测试用例 "multiple with defaultActiveKey"
  • 该功能在以下关键文件中都有完整实现:
    • src/Panel.tsx
    • src/OptionList/List.tsx
    • src/OptionList/useActive.ts
  • 测试用例验证了多选模式下的默认展开功能
  • 示例代码中的使用方式正确
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否有相关的测试用例

# 查找与 defaultActiveKey 相关的测试
rg -l "defaultActiveKey.*Panel" "tests/"

# 检查是否有其他使用示例
rg -l "defaultActiveKey.*Panel" "examples/"

Length of output: 84


Script:

#!/bin/bash
# 扩大搜索范围,查找所有与 defaultActiveKey 相关的测试和使用示例
rg -i "defaultActiveKey" --type ts --type tsx -A 5

# 查找 Cascader.Panel 的所有使用示例
rg "Cascader\.Panel" --type ts --type tsx -A 5

# 查找测试文件中的相关内容
fd "cascader.*test.*" --type f

Length of output: 188


Script:

#!/bin/bash
# 不指定文件类型,直接搜索所有文件
rg -i "defaultActiveKey" -A 5

# 搜索 Cascader.Panel 的使用
rg "Cascader\.Panel" -A 5

# 使用 fd 查找所有可能的测试文件
fd -e test.js -e test.ts -e test.tsx -e spec.js -e spec.ts -e spec.tsx

Length of output: 8824

package.json (3)

45-49: 核心依赖包版本更新幅度较大

以下依赖包的版本更新幅度较大,需要特别关注:

  • @babel/runtime: ^7.12.5 → ^7.25.7
  • rc-select: ~14.15.0 → ~14.16.2
  • rc-tree: ~5.8.1 → ~5.10.1
  • rc-util: ^5.37.0 → ^5.43.0

建议:

  1. 确认这些依赖的更新是否与新功能相关
  2. 验证是否有破坏性更新
  3. 进行充分的回归测试
#!/bin/bash
# 检查依赖包的使用情况
echo "检查 @babel/runtime 的使用..."
rg "require\(['\"]@babel/runtime" 

echo "检查 rc-select 的使用..."
rg "from ['\"](rc-select|@rc-component/select)"

echo "检查 rc-tree 的使用..."
rg "from ['\"](rc-tree|@rc-component/tree)"

echo "检查 rc-util 的使用..."
rg "from ['\"](rc-util|@rc-component/util)"

62-63: 新增开发依赖包的用途需要说明

新增了两个开发依赖:

  • array-tree-filter@^3.0.2
  • cheerio@1.0.0-rc.12

建议:

  1. 在提交信息或文档中说明这些依赖的用途
  2. 确认这些依赖仅用于开发/测试环境
#!/bin/bash
# 检查新增依赖的使用情况
echo "检查 array-tree-filter 的使用..."
rg "require\(['\"]array-tree-filter['\"]" 

echo "检查 cheerio 的使用..."
rg "require\(['\"]cheerio['\"]"

3-3: 版本号更新需要符合语义化版本规范

版本从 3.27.0 升级到 3.29.1,这是一个次要版本和补丁版本的组合更新。根据语义化版本规范,这表明添加了向后兼容的新功能并修复了一些问题。

✅ Verification successful

版本更新符合规范且有完整的版本历史记录

从版本历史记录可以看出,版本更新遵循了渐进式的升级路径:
3.27.0 -> 3.27.1 -> 3.28.0 -> 3.28.1 -> 3.28.2 -> 3.29.0 -> 3.29.1

版本号的递增符合语义化版本规范,通过小版本号(3.28.x、3.29.x)和补丁版本号的有序递增,表明这是一个稳健的版本演进过程。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查 CHANGELOG.md 中是否记录了相应的版本更新信息
rg -A 10 "3\.29\.1|3\.28\.0" CHANGELOG.md

Length of output: 336


Script:

#!/bin/bash
# 检查 CHANGELOG 相关文件
fd -g "*CHANGELOG*" -g "!node_modules"

# 检查 git tag 历史中的版本信息
git tag | grep -E "v?3\.(27|28|29)"

# 检查 package.json 的历史版本变更
git log -p package.json | grep -E "\"version\".*3\.(27|28|29)"

Length of output: 554

src/hooks/useSearchOptions.ts (2)

90-90: 依赖数组更新正确

useMemo 的依赖数组已正确更新,包含了新的 enableHalfPath 参数。


92-93: 导出语句改进符合最佳实践

将默认导出改为具名函数导出是一个很好的改进,这样可以在调试和开发工具中看到更有意义的函数名。

tests/selector.spec.tsx (1)

8-11: 更新了 useActive mock 实现以支持 defaultActiveKey

mock 函数已正确更新以支持新的 defaultActiveKey 参数,并且正确地将其传递给了原始的 hook。这与组件的新功能保持一致。

不过建议验证一下新参数的测试覆盖率。

✅ Verification successful

已验证 defaultActiveKey 参数的测试覆盖情况

tests/Panel.spec.tsx 中已经包含了针对 defaultActiveKey 参数的专门测试用例,测试了多选模式下的默认展开功能。mock 实现和测试用例都已经正确覆盖了这个新参数。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否有针对 defaultActiveKey 参数的测试用例
rg -l "defaultActiveKey.*=.*\[.*\].*Cascader" tests/

Length of output: 54


Script:

#!/bin/bash
# 搜索测试文件中包含 defaultActiveKey 的相关测试用例
rg "defaultActiveKey" tests/ -C 3

# 查找所有与 Cascader 相关的测试文件
fd ".*\.spec\." tests/

Length of output: 1807

src/Panel.tsx (2)

38-39: 类型定义正确添加

defaultActiveKey 的类型定义位置合适,与其他UI控制相关的属性放在一起,保持了良好的代码组织。


205-205: 验证组件兼容性

需要确认 RawOptionList 组件是否正确处理 defaultActiveKey 属性。

✅ Verification successful

Based on the shell script results, I can now generate the final response since I have found clear evidence that the RawOptionList component does support the defaultActiveKey prop. The interface definition and usage patterns are clearly visible in the codebase.

defaultActiveKey 属性验证完成,使用安全

RawOptionList 组件完全支持 defaultActiveKey 属性:

  • src/OptionList/List.tsx 中已定义该属性类型为 React.Key[]
  • 在组件内部通过 useActive hook 正确处理了该属性
  • 属性值会被用于初始化活动项状态
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 RawOptionList 组件是否支持 defaultActiveKey 属性

# 测试:搜索 RawOptionList 的属性定义
ast-grep --pattern 'interface $PROPS {
  $$$
  defaultActiveKey$_
  $$$
}'

# 测试:搜索 defaultActiveKey 的使用
rg -A 5 'defaultActiveKey' src/OptionList/

Length of output: 1849

src/OptionList/Column.tsx (2)

Line range hint 118-124: 代码简化提高了可读性!

通过移除冗余的条件检查,使代码更加清晰和直观。这个改动与支持默认展开单元格的目标相符合,同时保持了核心功能的完整性。


Line range hint 1-205: 建议验证搜索功能的完整性

由于移除了 searchValue 属性,建议:

  1. 确认搜索相关功能是否仍然正常工作
  2. 验证这个改动是否会影响现有的实现
  3. 考虑在文档中说明这个变更
src/OptionList/List.tsx (3)

25-25: 类型定义正确且完整!

defaultActiveKey 的类型定义为可选的 React.Key[] 符合 TypeScript 最佳实践。


28-28: 属性解构合理!

defaultActiveKey 的解构位置正确,与其他相关属性放在一起,保持了代码的一致性。


92-92: 请确认 useActive 钩子的行为符合预期

根据之前的评审意见,defaultActiveKey 应该只在初始化时生效,而不是在值变化时触发展开。请确保 useActive 钩子的实现符合这个要求。

✅ Verification successful

useActive 钩子的实现符合预期行为

根据源码分析,useActive 钩子的实现是正确的:

  • defaultActiveKey 仅在初始化时通过 useState 设置默认值:
const [activeValueCells, setActiveValueCells] = React.useState<React.Key[]>(defaultActiveKey ?? []);
  • 后续的值更新是由 setActiveValueCells 触发,与 defaultActiveKey 无关
  • 在非多选模式下,活动项会跟随选中值自动更新,这是预期的行为
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查 useActive 钩子的实现
ast-grep --pattern 'export default function useActive($_) {
  $$$
}'

Length of output: 70


Script:

#!/bin/bash
# 搜索 useActive 的定义和实现
rg -g '*.{ts,tsx}' "useActive" -A 10

# 搜索包含 defaultActiveKey 的相关代码
rg -g '*.{ts,tsx}' "defaultActiveKey" -A 5

Length of output: 7982

src/Cascader.tsx (2)

162-162: defaultActiveKey 属性的类型定义正确且命名恰当

属性类型定义为 React.Key[] 符合 React 的类型系统规范,且命名遵循了之前代码评审的建议。

Also applies to: 179-179


162-162: 验证新增属性的使用场景

新增的 defaultActiveKey 属性和搜索逻辑的变更可能会影响现有功能,建议验证以下场景:

  1. 默认展开项的正确性
  2. 多选模式下搜索功能的表现

Also applies to: 179-179, 285-285

✅ Verification successful

defaultActiveKey 属性已得到充分验证和测试

根据代码分析结果,defaultActiveKey 属性已经:

  • useActive hook 中得到正确实现和处理
  • 在多选模式下有专门的测试用例覆盖
  • 在示例代码中有实际使用场景展示

搜索功能与 defaultActiveKey 属性相互独立,不会产生功能冲突。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查 defaultActiveKey 的使用方式
echo "检查 defaultActiveKey 的使用场景..."
rg "defaultActiveKey.*=.*\[.*\]" -g "!*.{test,spec}.{ts,tsx,js,jsx}"

# 检查多选模式下的搜索相关测试用例
echo "检查多选模式的测试覆盖..."
rg "describe.*multiple.*search" -g "*.{test,spec}.{ts,tsx,js,jsx}" -A 5

Length of output: 503


Script:

#!/bin/bash
# 检查 defaultActiveKey 的使用和相关测试
echo "检查 defaultActiveKey 的实现和测试..."
rg "defaultActiveKey" -g "*.{ts,tsx}" -A 5

# 检查多选模式相关的测试用例
echo "检查多选模式的测试用例..."
rg "multiple.*mode" -g "*.{test,spec}.{ts,tsx}" -A 5

# 检查搜索功能相关的测试
echo "检查搜索功能的测试..."
rg "search.*test" -g "*.{test,spec}.{ts,tsx}" -A 5

Length of output: 5397

Comment on lines +5 to +42
const options = [
{
value: 'zhejiang',
label: 'Zhejiang',
children: [
{
value: 'hangzhou',
label: 'Hangzhou',
children: [
{
value: 'xihu',
label: 'West Lake',
},
{
value: 'xiasha',
label: 'Xia Sha',
},
],
},
],
},
{
value: 'jiangsu',
label: 'Jiangsu',
children: [
{
value: 'nanjing',
label: 'Nanjing',
children: [
{
value: 'zhonghuamen',
label: 'Zhong Hua men',
},
],
},
],
},
];
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. 建议为选项数据添加 TypeScript 接口定义,提高代码的类型安全性:
interface CascaderOption {
  value: string;
  label: string;
  children?: CascaderOption[];
}
  1. 建议将模拟数据移至单独的文件(如 mock/cascaderData.ts),以提高代码的可维护性和复用性。

Comment on lines +1 to +3
import React from 'react';
import '../assets/index.less';
import Cascader from '../src';
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议使用路径别名优化导入语句

当前的相对路径导入方式可能会在项目结构变化时造成维护困难。建议配置 TypeScript 的 path aliases,使用 @ 或类似的别名来简化导入路径。

例如:

-import '../assets/index.less';
-import Cascader from '../src';
+import '@/assets/index.less';
+import Cascader from '@/src';

Committable suggestion was skipped due to low confidence.

Comment on lines +14 to +27
const options = [
{
children: [] as any[],
isParent: true,
label: 'Asia',
value: 'Asia',
},
];
for (let i = 0; i < 100; i++) {
options[0].children.push({
label: 'label' + i,
value: 'value' + i,
});
}
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. 魔法数字 100 应该提取为常量
  2. 可以使用更简洁的数组生成方法
  3. 类型 any[] 应该更具体

建议修改如下:

+  const TOTAL_OPTIONS = 100;
   const options = [
     {
-      children: [] as any[],
+      children: [] as Array<{ label: string; value: string }>,
       isParent: true,
       label: 'Asia',
       value: 'Asia',
     },
   ];
-  for (let i = 0; i < 100; i++) {
-    options[0].children.push({
+  options[0].children = Array.from({ length: TOTAL_OPTIONS }, (_, i) => ({
       label: 'label' + i,
       value: 'value' + i,
-    });
-  }
+  }));
📝 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 options = [
{
children: [] as any[],
isParent: true,
label: 'Asia',
value: 'Asia',
},
];
for (let i = 0; i < 100; i++) {
options[0].children.push({
label: 'label' + i,
value: 'value' + i,
});
}
const TOTAL_OPTIONS = 100;
const options = [
{
children: [] as Array<{ label: string; value: string }>,
isParent: true,
label: 'Asia',
value: 'Asia',
},
];
options[0].children = Array.from({ length: TOTAL_OPTIONS }, (_, i) => ({
label: 'label' + i,
value: 'value' + i,
}));

Comment on lines +29 to +75
it('limit', () => {
const wrapper = mount(
<Cascader
options={options}
open
showSearch={{
limit: false,
}}
/>,
);

doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(100);
});

it('limit', () => {
const wrapper = mount(
<Cascader
options={options}
open
showSearch={{
limit: 0,
}}
/>,
);

doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(100);
});

it('limit', () => {
const wrapper = mount(
<Cascader
options={options}
open
showSearch={{
limit: 20,
}}
/>,
);

doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(20);
});
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. 测试名称不够具体,三个测试都叫 "limit"
  2. 测试设置代码重复
  3. 可以使用 test.each 来简化测试结构

建议重构如下:

-  it('limit', () => {
-    const wrapper = mount(
-      <Cascader
-        options={options}
-        open
-        showSearch={{
-          limit: false,
-        }}
-      />,
-    );
-
-    doSearch(wrapper, 'as');
-    const itemList = wrapper.find('div.rc-cascader-menu-item-content');
-    expect(itemList).toHaveLength(100);
-  });
-
-  it('limit', () => {
-    // ... 重复的测试
-  });
-
-  it('limit', () => {
-    // ... 重复的测试
-  });
+  const createWrapper = (limit: number | boolean) =>
+    mount(
+      <Cascader
+        options={options}
+        open
+        showSearch={{ limit }}
+      />
+    );
+
+  test.each([
+    ['显示所有选项当 limit 为 false', false, 100],
+    ['显示所有选项当 limit 为 0', 0, 100],
+    ['限制显示 20 个选项', 20, 20],
+  ])('%s', (_, limit, expectedLength) => {
+    const wrapper = createWrapper(limit);
+    doSearch(wrapper, 'as');
+    const itemList = wrapper.find('div.rc-cascader-menu-item-content');
+    expect(itemList).toHaveLength(expectedLength);
+  });
📝 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
it('limit', () => {
const wrapper = mount(
<Cascader
options={options}
open
showSearch={{
limit: false,
}}
/>,
);
doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(100);
});
it('limit', () => {
const wrapper = mount(
<Cascader
options={options}
open
showSearch={{
limit: 0,
}}
/>,
);
doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(100);
});
it('limit', () => {
const wrapper = mount(
<Cascader
options={options}
open
showSearch={{
limit: 20,
}}
/>,
);
doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(20);
});
const createWrapper = (limit: number | boolean) =>
mount(
<Cascader
options={options}
open
showSearch={{ limit }}
/>
);
test.each([
['显示所有选项当 limit 为 false', false, 100],
['显示所有选项当 limit 为 0', 0, 100],
['限制显示 20 个选项', 20, 20],
])('%s', (_, limit, expectedLength) => {
const wrapper = createWrapper(limit);
doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(expectedLength);
});

Comment on lines +49 to +50
// If is changeOnSelect or multiple
enableHalfPath
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

条件判断逻辑简化建议

当前条件判断 enableHalfPath 可以与上面的叶子节点判断合并,使代码更简洁。

建议重构为:

-        if (
-          // If is leaf option
-          !children ||
-          children.length === 0 ||
-          // If is changeOnSelect or multiple
-          enableHalfPath
-        ) {
+        if (!children?.length || enableHalfPath) {
📝 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
// If is changeOnSelect or multiple
enableHalfPath
// If is changeOnSelect or multiple
if (!children?.length || enableHalfPath) {

Comment on lines +12 to +18
const useSearchOptions = (
search: string,
options: DefaultOptionType[],
fieldNames: InternalFieldNames,
prefixCls: string,
config: ShowSearchType,
changeOnSelect?: boolean,
enableHalfPath?: boolean,
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

需要更新多个文件以反映参数变更

根据验证结果,changeOnSelect 参数在多个重要文件中被使用:

  • README.md 中的 API 文档需要更新
  • docs/demo/change-on-select.md 示例文档需要修改
  • CHANGELOG.md 需要添加此破坏性变更说明
  • 多个示例文件(如 examples/change-on-select.tsx)需要更新
  • 相关测试文件需要同步修改

建议在合并此更改之前,确保完成以上文档和示例的更新,以保持文档与代码的一致性。

🔗 Analysis chain

参数命名更改需要更新相关文档

参数从 changeOnSelect 更改为 enableHalfPath 反映了功能语义的变化。建议:

  1. 确保更新相关的API文档
  2. 在 CHANGELOG 中记录这个破坏性变更
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否有相关文档需要更新
rg -l "changeOnSelect" docs/

Length of output: 28


Script:

#!/bin/bash
# 检查整个代码库中是否有相关文档或测试文件包含 changeOnSelect
rg -l "changeOnSelect"

# 检查是否有 markdown 或其他文档文件
fd -e md -e mdx

Length of output: 1054


Script:

#!/bin/bash
# 检查文档中 changeOnSelect 的具体使用情况
rg "changeOnSelect" docs/demo/change-on-select.md -A 5
rg "changeOnSelect" README.md -A 5
rg "changeOnSelect" CHANGELOG.md

Length of output: 401

Comment on lines +215 to +229
it('onChange should be triggered when click option with multiple', () => {
const onChange = jest.fn();
const wrapper = mount(<Cascader checkable options={options} onChange={onChange} showSearch />);
doSearch(wrapper, 'toy');
wrapper.find('.rc-cascader-menu-item').first().simulate('click');
wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown');
expect(onChange).toHaveBeenCalledWith([['bamboo', 'little', 'fish']], expect.anything());

doSearch(wrapper, 'light');
wrapper.find('.rc-cascader-menu-item').first().simulate('click');
wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown');
expect(onChange).toHaveBeenCalledWith(
[['bamboo', 'little', 'fish'], ['light']],
expect.anything(),
);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

测试用例存在重复代码,建议重构

该测试用例与前一个测试用例有大量重复代码,仅配置项不同。建议提取共同的测试逻辑到辅助函数中。

建议重构为:

const testMultipleSelection = (props) => {
  const onChange = jest.fn();
  const wrapper = mount(<Cascader {...props} options={options} onChange={onChange} showSearch />);
  
  // 测试选中第一个选项
  doSearch(wrapper, 'toy');
  wrapper.find('.rc-cascader-menu-item').first().simulate('click');
  wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown');
  expect(onChange).toHaveBeenCalledWith([['bamboo', 'little', 'fish']], expect.anything());

  // 测试选中第二个选项
  doSearch(wrapper, 'light');
  wrapper.find('.rc-cascader-menu-item').first().simulate('click');
  wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown');
  expect(onChange).toHaveBeenCalledWith(
    [['bamboo', 'little', 'fish'], ['light']],
    expect.anything(),
  );
};

it('onChange should be triggered when click option with changeOnSelect + multiple', () => {
  testMultipleSelection({ checkable: true, changeOnSelect: true });
});

it('onChange should be triggered when click option with multiple', () => {
  testMultipleSelection({ checkable: true });
});

Copy link

codecov bot commented Oct 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.83%. Comparing base (19a025d) to head (0b2c795).
Report is 18 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #515      +/-   ##
==========================================
+ Coverage   99.67%   99.83%   +0.16%     
==========================================
  Files          23       23              
  Lines         607      611       +4     
  Branches      183      186       +3     
==========================================
+ Hits          605      610       +5     
+ Misses          2        1       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zombieJ
Copy link
Member

zombieJ commented Nov 19, 2024

PR 乱了,解决一下冲突?

@lihongxiangfrontend
Copy link
Author

PR 乱了,解决一下冲突?

我看下

aojunhao123 and others added 5 commits November 19, 2024 15:18
* feat: add disabled api for panel

* feat: add test case

* docs: add demo

* fix: test case fixed

* feat: adjust demo position

* feat: improve demo

* chore: reorganize code for better readability

* test: update snapshot

* fix: lint fix

* chore: adjust code structure

* test: update snapshot

* chore: code optimization

---------

Co-authored-by: 小豪 <aojunhao@cai-inc.com>
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 (4)
examples/panel.tsx (1)

99-100: 代码格式需要调整

console.log 语句的缩进不一致,建议保持统一的缩进风格。

建议修改为:

  onChange={nextValue => {
-        console.log('Change:', nextValue);
-        setValue2(nextValue);
+    console.log('Change:', nextValue);
+    setValue2(nextValue);
  }}
tests/Panel.spec.tsx (1)

82-93: 建议增强测试用例的覆盖范围

测试用例验证了 defaultActiveKey 属性的基本功能,但建议添加以下断言以提高测试覆盖率:

  • 验证激活项的状态(例如检查 aria-checked 属性)
  • 验证菜单项的内容是否正确显示
  • 验证用户交互后的行为

建议添加以下测试代码:

  it('multiple with defaultActiveKey', () => {
    const onChange = jest.fn();
    const { container } = render(
      <Cascader.Panel
        checkable
        options={options}
        onChange={onChange}
        defaultActiveKey={['bamboo', 'little']}
      />,
    );
    expect(container.querySelectorAll('.rc-cascader-menu')).toHaveLength(2);
+   // 验证激活项
+   const activeItems = container.querySelectorAll('.rc-cascader-menu-item-active');
+   expect(activeItems).toHaveLength(2);
+   expect(activeItems[0].textContent).toBe('Bamboo');
+   expect(activeItems[1].textContent).toBe('Little');
+
+   // 验证用户交互
+   fireEvent.click(container.querySelectorAll('.rc-cascader-menu-item')[0]);
+   expect(onChange).toHaveBeenCalled();
  });
src/Panel.tsx (1)

Line range hint 1-213: 建议添加属性文档和类型验证

为了提高代码的可维护性和使用体验,建议:

  1. 为新增的 defaultActiveKeydisabled 属性添加 JSDoc 文档说明
  2. 考虑添加属性类型验证

需要我帮您生成相关的文档注释和类型验证代码吗?

src/OptionList/List.tsx (1)

120-127: 建议优化禁用状态的判断逻辑

当前实现是可行的,但可以通过合并条件判断来优化代码:

-  if (disabled) {
-    return false;
-  }
-
-  const { disabled: optionDisabled } = option;
-  const isMergedLeaf = isLeaf(option, fieldNames);
-
-  return !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
+  const { disabled: optionDisabled } = option;
+  const isMergedLeaf = isLeaf(option, fieldNames);
+  return !disabled && !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 0b2c795 and 7c7f117.

📒 Files selected for processing (6)
  • examples/panel.tsx (2 hunks)
  • package.json (3 hunks)
  • src/OptionList/Column.tsx (7 hunks)
  • src/OptionList/List.tsx (5 hunks)
  • src/Panel.tsx (3 hunks)
  • tests/Panel.spec.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • package.json
  • src/OptionList/Column.tsx
🧰 Additional context used
🪛 Biome
examples/panel.tsx

[error] 103-104: This JSX property is assigned multiple times.

This attribute is assigned again here.

(lint/suspicious/noDuplicateJsxProps)

src/Panel.tsx

[error] 74-74: expected , but instead found disabled

Remove disabled

(parse)


[error] 75-75: Declarations inside of a const declaration may not have duplicates

a second declaration of defaultActiveKey is not allowed

defaultActiveKey is first declared here

(parse)


[error] 208-208: This JSX property is assigned multiple times.

This attribute is assigned again here.

(lint/suspicious/noDuplicateJsxProps)

🪛 eslint
examples/panel.tsx

[error] 104-104: No duplicate props allowed

(react/jsx-no-duplicate-props)

🔇 Additional comments (9)
examples/panel.tsx (3)

64-65: 状态管理的良好实践!

新增的 disabled 状态变量使用了 React 的 useState Hook,实现了组件的可控性,这是一个很好的实践。


76-82: 用户交互控制完善!

新增的按钮提供了良好的交互体验,允许用户动态切换面板的禁用状态。


107-107: RTL 支持的良好实践!

添加 direction="rtl" 支持右到左布局是一个很好的国际化实践。

tests/Panel.spec.tsx (1)

114-125: 测试用例设计合理

该测试用例完整地覆盖了禁用状态的核心功能:

  • 验证了禁用样式的存在
  • 验证了禁用状态下点击事件被阻止
  • 验证了回调函数未被触发
src/Panel.tsx (1)

38-40: 类型定义扩展正确

新增的 defaultActiveKeydisabled 属性扩展了 PickType 类型,这些改动符合组件功能的需求。

src/OptionList/List.tsx (4)

24-32: 类型定义更新合理!

RawOptionListProps 类型的扩展符合组件需求,新增的 disableddefaultActiveKey 属性定义清晰。


35-45: 属性解构合理!

新增属性的解构符合代码风格,顺序合理。


109-109: 很好地采纳了之前的反馈!

defaultActiveKey 直接传递给 useActive hook 而不是使用 effect,这样确保了默认值不会随着属性变化而动态改变,符合 "default" 的语义。


Line range hint 190-203: 滚动逻辑优化合理!

在搜索状态下跳过滚动操作的优化很好,可以避免不必要的 DOM 操作。代码实现清晰,维护了原有功能。

Comment on lines 102 to 104
defaultActiveKey={['bj', 'haidian']}
disabled={disabled}
defaultActiveKey={['bj', 'haidian']}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

发现重复的属性设置

defaultActiveKey 属性在组件中被重复定义了两次,这可能会导致意外的行为。

建议移除重复的属性:

  <Cascader.Panel
    checkable
    value={value2}
    options={addressOptions}
    onChange={nextValue => {
      console.log('Change:', nextValue);
      setValue2(nextValue);
    }}
    defaultActiveKey={['bj', 'haidian']}
    disabled={disabled}
-   defaultActiveKey={['bj', 'haidian']}
  />

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome

[error] 103-104: This JSX property is assigned multiple times.

This attribute is assigned again here.

(lint/suspicious/noDuplicateJsxProps)

🪛 eslint

[error] 104-104: No duplicate props allowed

(react/jsx-no-duplicate-props)

src/Panel.tsx Outdated
Comment on lines 208 to 210
defaultActiveKey={defaultActiveKey}
disabled={disabled}
defaultActiveKey={defaultActiveKey}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

修复 RawOptionList 组件的重复属性

defaultActiveKey 属性在 RawOptionList 组件中被重复传递了。

建议按照以下方式修改:

  <RawOptionList
    prefixCls={prefixCls}
    searchValue=""
    multiple={multiple}
    toggleOpen={noop}
    open
    direction={direction}
    defaultActiveKey={defaultActiveKey}
    disabled={disabled}
-   defaultActiveKey={defaultActiveKey}
  />
📝 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
defaultActiveKey={defaultActiveKey}
disabled={disabled}
defaultActiveKey={defaultActiveKey}
defaultActiveKey={defaultActiveKey}
disabled={disabled}
🧰 Tools
🪛 Biome

[error] 208-208: This JSX property is assigned multiple times.

This attribute is assigned again here.

(lint/suspicious/noDuplicateJsxProps)

src/Panel.tsx Outdated
Comment on lines 73 to 75
defaultActiveKey
disabled,
defaultActiveKey
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

修复属性解构中的重复声明

代码中存在以下问题:

  1. defaultActiveKey 被重复声明了两次
  2. disabled 的声明位置不正确

建议按照以下方式修改:

  const {
    prefixCls = 'rc-cascader',
    style,
    className,
    options,
    checkable,
    defaultValue,
    value,
    fieldNames,
    changeOnSelect,
    onChange,
    showCheckedStrategy,
    loadData,
    expandTrigger,
    expandIcon = '>',
    loadingIcon,
    direction,
    notFoundContent = 'Not Found',
-   defaultActiveKey
-   disabled,
-   defaultActiveKey
+   defaultActiveKey,
+   disabled
  } = props as Pick<InternalCascaderProps, PickType>;
📝 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
defaultActiveKey
disabled,
defaultActiveKey
prefixCls = 'rc-cascader',
style,
className,
options,
checkable,
defaultValue,
value,
fieldNames,
changeOnSelect,
onChange,
showCheckedStrategy,
loadData,
expandTrigger,
expandIcon = '>',
loadingIcon,
direction,
notFoundContent = 'Not Found',
defaultActiveKey,
disabled
🧰 Tools
🪛 Biome

[error] 74-74: expected , but instead found disabled

Remove disabled

(parse)


[error] 75-75: Declarations inside of a const declaration may not have duplicates

a second declaration of defaultActiveKey is not allowed

defaultActiveKey is first declared here

(parse)

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 (2)
examples/panel.tsx (2)

76-82: 用户界面交互设计合理

切换按钮的文本会根据当前状态动态改变,这提供了良好的用户体验。建议考虑添加一些视觉反馈,比如禁用状态下的样式变化。

 <button
   onClick={() => {
     setDisabled(prev => !prev);
   }}
+  style={{
+    opacity: disabled ? 0.5 : 1,
+    cursor: disabled ? 'not-allowed' : 'pointer'
+  }}
 >
   {disabled ? 'enable panel' : 'disable panel'}
 </button>

99-100: 代码格式需要调整

console.logsetValue2 的缩进不一致,影响代码的可读性。

 onChange={nextValue => {
-        console.log('Change:', nextValue);
-        setValue2(nextValue);
+  console.log('Change:', nextValue);
+  setValue2(nextValue);
 }}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 7c7f117 and 5e15a6c.

📒 Files selected for processing (1)
  • examples/panel.tsx (2 hunks)
🔇 Additional comments (3)
examples/panel.tsx (3)

64-65: 状态管理的良好实践

使用 useState 来管理 disabled 状态是一个很好的做法,这样可以统一控制所有面板的禁用状态。


102-103: 验证默认展开功能的实现

需要确保 defaultActiveKeydisabled 属性在组件内部正确处理,建议添加相应的单元测试。

✅ Verification successful

Let me gather more information about the test coverage for the disabled prop.


defaultActiveKeydisabled 属性已有测试覆盖

根据测试文件的分析,这两个属性已经有相应的单元测试覆盖:

  • defaultActiveKeymultiple with defaultActiveKey 测试用例中验证了多选模式下的默认展开功能
  • disabled 属性在测试中验证了禁用状态下的渲染和点击行为

无需添加额外的测试用例。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查是否存在相关的测试用例

# 查找测试文件中的相关测试用例
rg -A 5 "defaultActiveKey|disabled.*Panel" tests/

Length of output: 1284


Script:

#!/bin/bash
# 查找与 Panel 组件的 disabled 属性相关的测试用例
rg -A 10 "disabled.*Panel|Panel.*disabled" tests/

# 查找 Panel 组件的实现,了解 disabled 属性的处理
ast-grep --pattern 'interface PanelProps {
  $$$
  disabled?: $_;
  $$$
}'

Length of output: 679


106-106: RTL支持的完整性检查

在添加 RTL 支持时,建议确保所有子组件和样式都正确支持从右到左的布局。

@lihongxiangfrontend
Copy link
Author

PR 乱了,解决一下冲突?

已改

@thinkasany
Copy link

thinkasany commented Nov 25, 2024

commit 历史有点乱了,携带了主分支其他的合并,你应该使用 rebasegit cherry-pick摘出重要的提交,重新提交一个pr 或者其他方式整理一下 或者 其他任意更好的方式,那些无关修改会影响到别人帮忙看。

image image

@lihongxiangfrontend
Copy link
Author

commit 历史有点乱了,携带了主分支其他的合并,你应该使用 rebasegit cherry-pick摘出重要的提交,重新提交一个pr 或者其他方式整理一下 或者 其他任意更好的方式,那些无关修改会影响到别人帮忙看。

image image

@lihongxiangfrontend
Copy link
Author

commit 历史有点乱了,携带了主分支其他的合并,你应该使用 rebasegit cherry-pick摘出重要的提交,重新提交一个pr 或者其他方式整理一下 或者 其他任意更好的方式,那些无关修改会影响到别人帮忙看。
image image

新的 pull-request : https://github.com/react-component/cascader/pull/547/commits

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.

8 participants