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: rename dropdown to popup #1103

Merged
merged 7 commits into from
Jan 13, 2025

Conversation

aojunhao123
Copy link
Contributor

@aojunhao123 aojunhao123 commented Jan 9, 2025

Summary by CodeRabbit

  • 重大变更

    • 将下拉菜单(dropdown)相关属性和方法重命名为弹出层(popup)相关属性和方法
    • 移除了一些过时的属性,如 multipletagscombobox
    • 更新了 Select 组件的 API,提高了一致性和清晰度
  • 属性变更

    • dropdownMatchSelectWidthpopupMatchSelectWidth
    • dropdownStylepopupStyle
    • dropdownClassNamepopupClassName
    • dropdownRenderpopupRender
    • onDropdownVisibleChangeonPopupVisibleChange
  • 兼容性提示

    • 建议开发者更新代码中的相关属性名称
    • 不再支持某些旧版本属性

Copy link

vercel bot commented Jan 9, 2025

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

Name Status Preview Comments Updated (UTC)
select ❌ Failed (Inspect) Jan 13, 2025 2:39am

Copy link
Contributor

coderabbitai bot commented Jan 9, 2025

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

docs/examples/auto-adjust-dropdown.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Cannot read config file: /.eslintrc.js
Error: Cannot find module '@umijs/fabric/dist/eslint'
Require stack:

  • /.eslintrc.js
  • /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/cli-engine/cli-engine.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/eslint.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/index.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/cli.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/bin/eslint.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Module._load (node:internal/modules/cjs/loader:1074:27)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
    at Module.require (node:internal/modules/cjs/loader:1339:12)
    at require (node:internal/modules/helpers:135:16)
    at Object. (/.eslintrc.js:1:14)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
    at Module.load (node:internal/modules/cjs/loader:1317:32)

概述

演练

这个拉取请求主要是对 rc-select 组件的 API 进行了重大重构。核心变化是将与下拉菜单相关的属性和方法从 dropdown 前缀更改为 popup 前缀。这包括重命名诸如 dropdownStyledropdownClassNamedropdownMatchSelectWidth 等属性为对应的 popupStylepopupClassNamepopupMatchSelectWidth。同时,事件处理方法 onDropdownVisibleChange 也被更改为 onPopupVisibleChange

变更

文件 变更摘要
src/BaseSelect/index.tsx 重命名下拉菜单相关属性,从 dropdown 前缀改为 popup 前缀
src/Select.tsx 移除多个废弃属性,更新上下文和属性处理逻辑
src/SelectTrigger.tsx 更新下拉菜单相关属性和方法的命名
文档和测试文件 同步更新属性名称和方法名称

建议相关的 PR

建议的审阅者

  • zombieJ

诗歌

🐰 下拉菜单变身啦!
Popup 取代 Dropdown
代码重构如春风
属性名更加通用
兔子跳跃,API 飞扬 🌈


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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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
Contributor

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

🔭 Outside diff range comments (1)
src/Select.tsx (1)

Line range hint 110-110: popupPrefixCls 的定义和使用可能导致类名不一致

在第 110 行,popupPrefixCls 被定义为 ${prefixCls}-dropdown,但我们已经将属性从“dropdown”重命名为“popup”。建议将 popupPrefixCls 修改为 ${prefixCls}-popup,以保持命名的一致性。这将确保生成的类名(例如 ${popupPrefixCls}-empty)与新的命名约定一致,避免可能的样式混淆。

应用以下代码修改以更新 popupPrefixCls 的定义:

- const popupPrefixCls = `${prefixCls}-dropdown`;
+ const popupPrefixCls = `${prefixCls}-popup`;

Also applies to: 123-123, 159-159, 167-168

🧹 Nitpick comments (2)
src/BaseSelect/index.tsx (2)

158-160: 接口重命名符合最佳实践

接口重命名遵循了以下最佳实践:

  • 使用 @deprecated 标记废弃的属性
  • 保持新旧属性类型一致
  • 提供明确的迁移建议

建议在后续版本中:

  • 考虑添加版本号到废弃注释中
  • 考虑添加废弃时间表

Also applies to: 192-211


422-426: 事件处理器调用顺序合理

同时调用新旧可见性变更处理器的实现:

  • 保证了向后兼容性
  • 依赖数组正确包含了所有依赖项
  • 处理器调用顺序合适

建议添加以下改进:

  • 考虑添加性能优化,避免不必要的重复调用
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a56cfe and e20892c.

📒 Files selected for processing (4)
  • src/BaseSelect/index.tsx (7 hunks)
  • src/Select.tsx (4 hunks)
  • src/SelectTrigger.tsx (5 hunks)
  • src/utils/warningPropsUtil.ts (1 hunks)
🔇 Additional comments (4)
src/Select.tsx (1)

184-184: 确保所有dropdownMatchSelectWidth的引用都已替换为popupMatchSelectWidth

我们注意到在代码中引入了新的属性popupMatchSelectWidth,并使用了合并变量mergedPopupMatchSelectWidth。为确保一致性,请检查代码库中是否还有未替换的dropdownMatchSelectWidth引用,以避免潜在的混淆或错误。

请运行以下脚本来查找代码库中所有对dropdownMatchSelectWidth的引用:

Also applies to: 612-613, 644-644, 681-681

✅ Verification successful

dropdownMatchSelectWidth 的使用符合预期的废弃流程

该属性正在进行标准的废弃流程:

  • 通过 warningPropsUtil.ts 提供废弃警告
  • 保留旧属性以保持向后兼容性
  • 新属性 popupMatchSelectWidth 在合并时具有优先权
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:查找代码库中所有对 `dropdownMatchSelectWidth` 的引用
rg 'dropdownMatchSelectWidth'

Length of output: 2226

src/utils/warningPropsUtil.ts (1)

154-160: 成功添加了对已弃用属性的警告提示

代码中新增了针对已弃用属性的警告,有助于提醒开发者属性的更新,确保他们使用最新的属性名称。这有助于维护代码库的一致性,并为未来的属性移除做好准备。

Also applies to: 168-173

src/BaseSelect/index.tsx (2)

335-340: 合并逻辑实现优雅

使用空值合并运算符 ?? 来合并新旧属性值的实现方式非常优雅:

  • 确保了向后兼容性
  • 代码简洁易读
  • 正确处理了 undefinednull 的情况

800-805: 属性传递实现规范

SelectTrigger 组件中的属性传递实现:

  • 正确使用了合并后的属性值
  • 保持了命名一致性
  • 遵循了新的命名约定

src/SelectTrigger.tsx Show resolved Hide resolved
Copy link
Contributor

@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

🧹 Nitpick comments (4)
src/Select.tsx (1)

184-184: 重命名 dropdown 相关属性为 popup

代码实现了平滑过渡,通过以下方式保持向后兼容:

  1. 使用 fallback 机制: popupMatchSelectWidth = dropdownMatchSelectWidth
  2. 更新了所有相关的依赖和引用

建议:

  • 在后续版本中考虑完全移除 dropdown 相关的属性
  • 添加 deprecation 警告,提示用户使用新的 popup 属性

Also applies to: 613-613, 642-642, 679-679

src/BaseSelect/index.tsx (3)

158-160: 更新事件处理器命名

onDropdownVisibleChange 已标记为废弃,请使用 onPopupVisibleChange 替代。这个改动与整体重命名策略保持一致。


188-211: 系统性重命名 dropdown 属性

以下属性已被标记为废弃,建议使用对应的新属性:

  • dropdownStylepopupStyle
  • dropdownClassNamepopupClassName
  • dropdownMatchSelectWidthpopupMatchSelectWidth
  • dropdownRenderpopupRender
  • dropdownAlignpopupAlign

这种统一的命名方案提高了 API 的一致性。


422-426: 事件处理器的向后兼容

同时调用新旧事件处理器确保了平滑过渡:

onDropdownVisibleChange?.(nextOpen);
onPopupVisibleChange?.(nextOpen);

建议在未来版本中移除对 onDropdownVisibleChange 的支持。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e20892c and 236853d.

📒 Files selected for processing (2)
  • src/BaseSelect/index.tsx (7 hunks)
  • src/Select.tsx (4 hunks)
🔇 Additional comments (2)
src/BaseSelect/index.tsx (2)

335-340: 优秀的兼容性处理

通过 nullish coalescing 运算符实现了优雅的属性合并:

const mergedPopupStyle = popupStyle ?? dropdownStyle;
const mergedPopupClassName = popupClassName ?? dropdownClassName;
const mergedPopupRender = popupRender ?? dropdownRender;
const mergedPopupAlign = popupAlign ?? dropdownAlign;
const mergedPopupMatchSelectWidth = popupMatchSelectWidth ?? dropdownMatchSelectWidth;

这确保了在过渡期间新旧属性都能正常工作。


800-805: 更新组件属性传递

SelectTrigger 组件现在使用合并后的 popup 属性,保持了一致性:

popupStyle={mergedPopupStyle}
popupClassName={mergedPopupClassName}
popupMatchSelectWidth={mergedPopupMatchSelectWidth}
popupRender={mergedPopupRender}
popupAlign={mergedPopupAlign}

@aojunhao123
Copy link
Contributor Author

CC @li-jia-nan 大佬,我在做Cascader deprecated api的时候发现得先rename dropdown --> popup,就先把这个工作给做了

@li-jia-nan
Copy link
Member

CC @li-jia-nan 大佬,我在做Cascader deprecated api的时候发现得先rename dropdown --> popup,就先把这个工作给做了

👌

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.25%. Comparing base (4779fb8) to head (fead9e5).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1103   +/-   ##
=======================================
  Coverage   98.25%   98.25%           
=======================================
  Files          39       39           
  Lines        1486     1486           
  Branches      448      421   -27     
=======================================
  Hits         1460     1460           
  Misses         26       26           

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

Copy link
Contributor

@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

🧹 Nitpick comments (1)
tests/Select.test.tsx (1)

2436-2458: 向后兼容性测试设计合理!

测试同时使用新旧 API 时的行为,验证:

  1. 新 API 的值优先生效
  2. 仍然会显示废弃警告

建议添加更多边缘情况的测试:

 it('should work with both old and new APIs', () => {
   const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});

   const { container } = render(
     <Select dropdownMatchSelectWidth={100} popupMatchSelectWidth={200}>
       <Option value="1">1</Option>
     </Select>,
   );

   // Should use new API value
   toggleOpen(container);
   expect((container.querySelector('.rc-select-dropdown') as HTMLElement).style.width).toBe(
     '200px',
   );

   // Should still show warning
   expect(errorSpy).toHaveBeenCalledWith(
     `Warning: \`dropdownMatchSelectWidth\` is deprecated. Please use \`popupMatchSelectWidth\` instead.`,
   );

   errorSpy.mockRestore();
 });
+
+it('should handle undefined values correctly', () => {
+  const { container } = render(
+    <Select dropdownMatchSelectWidth={100} popupMatchSelectWidth={undefined}>
+      <Option value="1">1</Option>
+    </Select>,
+  );
+
+  toggleOpen(container);
+  expect((container.querySelector('.rc-select-dropdown') as HTMLElement).style.width).toBe(
+    '100px',
+  );
+});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 236853d and dc4a7d4.

📒 Files selected for processing (2)
  • src/utils/warningPropsUtil.ts (1 hunks)
  • tests/Select.test.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/utils/warningPropsUtil.ts
🔇 Additional comments (4)
tests/Select.test.tsx (4)

2371-2384: 测试套件设置良好!

测试套件的设置包含了必要的定时器模拟和警告重置,这对于测试废弃 API 的警告消息很重要。


2385-2392: 废弃 API 映射清晰完整!

使用对象映射来管理废弃的 API 和新 API 的对应关系,使代码更易维护。


2394-2414: 废弃 API 测试实现完善!

使用 Object.entries 遍历所有废弃的 API,确保每个废弃的 API 都会触发正确的警告消息。测试用例设计合理。


2416-2434: 新 API 测试覆盖完整!

验证使用新 API 时不会触发警告,测试用例包含了所有新的属性。

Copy link
Contributor

@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

🧹 Nitpick comments (1)
src/BaseSelect/index.tsx (1)

Line range hint 159-196: 接口定义更新完整!

所有 dropdown 相关的属性已统一更新为 popup 前缀:

  • onDropdownVisibleChangeonPopupVisibleChange
  • 样式相关属性已更新
  • 保持了原有的类型定义

建议添加 JSDoc 注释说明这些属性的用途。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f252168 and 72e4d5c.

📒 Files selected for processing (13)
  • docs/examples/auto-adjust-dropdown.tsx (3 hunks)
  • docs/examples/controlled.tsx (2 hunks)
  • docs/examples/dropdownRender.tsx (1 hunks)
  • docs/examples/getPopupContainer.tsx (1 hunks)
  • docs/examples/single-animation.tsx (1 hunks)
  • docs/examples/single.tsx (1 hunks)
  • src/BaseSelect/index.tsx (6 hunks)
  • src/Select.tsx (4 hunks)
  • src/SelectTrigger.tsx (5 hunks)
  • tests/Combobox.test.tsx (1 hunks)
  • tests/Custom.test.tsx (1 hunks)
  • tests/Popup.test.tsx (1 hunks)
  • tests/Select.test.tsx (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/SelectTrigger.tsx
🔇 Additional comments (20)
tests/Custom.test.tsx (1)

18-18: 重命名回调函数符合预期!

onDropdownVisibleChange 更改为 onPopupVisibleChange 的修改保持了测试的功能完整性,同时与整体重命名策略保持一致。

Also applies to: 22-22, 27-27

tests/Popup.test.tsx (1)

16-16: 测试用例更新正确!

成功将 onDropdownVisibleChange 更新为 onPopupVisibleChange,保持了测试逻辑的一致性。测试用例仍然正确验证了点击弹出框时不应触发关闭的行为。

Also applies to: 21-21, 27-27

docs/examples/single-animation.tsx (1)

24-26: 样式属性重命名符合规范!

dropdownStyle 更改为 popupStyle 的修改保持了样式配置的功能不变,同时符合新的命名规范。

docs/examples/getPopupContainer.tsx (1)

78-80: 宽度匹配属性更新完整!

成功将所有 dropdownMatchSelectWidth 实例更新为 popupMatchSelectWidth,包括布尔值和数值类型的使用场景。修改保持了组件的所有功能特性。

docs/examples/dropdownRender.tsx (1)

53-53: 属性名称已更新,但文件名需要同步更改

属性从 dropdownRender 更改为 popupRender 的修改是正确的,与整体重命名策略保持一致。

建议将文件名从 dropdownRender.tsx 更改为 popupRender.tsx 以保持一致性。

docs/examples/controlled.tsx (1)

46-46: 方法名称更新正确

onDropdownVisibleChange 更改为 onPopupVisibleChange 的修改已正确应用于方法定义和使用处。

Also applies to: 71-71

docs/examples/auto-adjust-dropdown.tsx (1)

37-37: 属性名称统一更新,建议更新文件名

所有 dropdownMatchSelectWidth 属性已正确更新为 popupMatchSelectWidth

建议将文件名从 auto-adjust-dropdown.tsx 更改为 auto-adjust-popup.tsx 以保持命名一致性。

Also applies to: 44-44, 53-53, 67-67, 74-74

docs/examples/single.tsx (2)

133-134: RTL Select 组件属性更新完整

以下属性已正确更新:

  • dropdownMatchSelectWidthpopupMatchSelectWidth
  • dropdownStylepopupStyle

Line range hint 1-1: 验证整个代码库中的命名一致性

为确保重命名的完整性,需要检查是否还有遗漏的 "dropdown" 用例。

tests/Combobox.test.tsx (1)

458-467: 测试用例已正确更新为使用新的 API!

测试用例已从 onDropdownVisibleChange 更新为 onPopupVisibleChange,并且保持了相同的功能验证。

src/Select.tsx (3)

183-183: 新属性 popupMatchSelectWidth 添加正确!

默认值保持为 true,与原有的 dropdownMatchSelectWidth 行为一致。


612-612: 虚拟列表逻辑更新正确!

条件判断中使用新的 popupMatchSelectWidth 替换原有的 dropdownMatchSelectWidth,保持了相同的功能逻辑。


678-678: 属性传递更新正确!

新的 popupMatchSelectWidth 属性已正确传递给 BaseSelect 组件。

src/BaseSelect/index.tsx (1)

772-777: SelectTrigger 组件属性更新正确!

完整更新了传递给 SelectTrigger 的所有属性,保持了一致的命名规范。

tests/Select.test.tsx (6)

Line range hint 318-324: 测试用例更新正确!

dropdownRender 更新为 popupRender,测试功能保持不变。


895-901: 受控组件测试更新正确!

更新使用 onPopupVisibleChange 替代 onDropdownVisibleChange,验证受控行为保持不变。


Line range hint 1326-1336: 自定义渲染测试更新正确!

使用 popupRender 替代原有的 dropdownRender,测试自定义渲染功能保持不变。


Line range hint 1348-1358: 自定义节点点击测试更新正确!

使用 popupRender 替代 dropdownRender,点击事件处理测试保持不变。


1489-1492: 宽度匹配测试更新正确!

使用 popupMatchSelectWidth 替代 dropdownMatchSelectWidth,宽度匹配行为测试保持不变。


1520-1523: 宽度调整测试更新正确!

使用 popupMatchSelectWidth 替代 dropdownMatchSelectWidth,宽度调整行为测试保持不变。

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.

3 participants