-
-
Notifications
You must be signed in to change notification settings - Fork 194
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
refactor: rename visibility change props for clarity #494
base: master
Are you sure you want to change the base?
refactor: rename visibility change props for clarity #494
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此PR对项目的两个部分进行了更新。首先,在 Changes
Sequence Diagram(s)sequenceDiagram
participant 用户 as 用户
participant Tooltip as Tooltip组件
participant Trigger as Trigger组件
用户->>Tooltip: 触发显示/隐藏事件
Tooltip->>Trigger: 调用 onOpenChange / afterOpenChange 回调
Suggested reviewers
Poem
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
docs/examples/formError.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
docs/examples/onVisibleChange.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
docs/examples/simple.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🔇 Additional comments (9)
✨ Finishing Touches
🪧 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: 0
🔭 Outside diff range comments (1)
src/Tooltip.tsx (1)
30-135
: 💡 Verification agent🧩 Analysis chain
验证属性重命名的向后兼容性
虽然重命名属性提高了清晰度,但需要确保这个更改不会破坏现有的使用该组件的应用程序。建议考虑添加临时的向后兼容代码,或者在发布说明中明确标记这是一个破坏性变更。
🏁 Script executed:
#!/bin/bash # 搜索代码库中可能使用旧属性名的地方 echo "检查对 onVisibleChange 的使用:" rg "onVisibleChange" echo "检查对 afterVisibleChange 的使用:" rg "afterVisibleChange" echo "检查其他 rc-component 库中对这些回调的命名约定:" rg -g "*.tsx" -g "*.ts" "on.*Change|after.*Change" | grep -E "onOpenChange|onVisibleChange|afterOpenChange|afterVisibleChange"Length of output: 1961
请确认旧属性名称的兼容处理方案
目前代码已将
onVisibleChange
和afterVisibleChange
重命名为onOpenChange
和afterOpenChange
。但通过检索发现,文档和示例代码(如docs/examples/simple.tsx
、docs/examples/onVisibleChange.tsx
、README.md
以及CHANGELOG.md
)中依然存在对旧属性的引用。这表明升级可能会对依赖旧属性的现有代码产生破坏性影响。
- 建议一: 提供临时的向后兼容代码,支持同时检测旧属性(
onVisibleChange
、afterVisibleChange
)并自动映射到新属性(onOpenChange
、afterOpenChange
),确保过渡期间不会影响现有用户。- 建议二: 如果不打算修改代码,则必须在发布说明中明确标注这一破坏性变更,并建议用户尽快更新相关引用和文档。
请评估并做出相应调整,以防止升级后出现不可预期的问题。
🧹 Nitpick comments (1)
src/Tooltip.tsx (1)
30-135
: 更新相关文档请确保任何相关的 API 文档、教程或示例也进行了相应的更新,以反映这些属性名称的变化。
是否需要我帮助检查或生成相关文档的更新?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.gitignore
(1 hunks)src/Tooltip.tsx
(3 hunks)
🔇 Additional comments (4)
.gitignore (1)
44-45
: 添加适当的文件到忽略列表,改进版本控制。添加
.vscode/
目录和恢复bun.lockb
到忽略列表是一个良好的做法,这可以防止团队成员的个人编辑器配置和特定包管理器的锁文件被提交到仓库中。src/Tooltip.tsx (3)
30-31
: 属性重命名提高了 API 的一致性和清晰度将
onVisibleChange
和afterVisibleChange
重命名为onOpenChange
和afterOpenChange
提高了 API 的清晰度,使属性名称更好地反映了它们的用途。这种命名方式与现代 React 组件库中常见的开关状态回调命名约定更加一致。
71-72
: 解构参数重命名保持一致在组件内部解构参数时正确更新了属性名称,与接口定义保持一致。
134-135
: 传递给 Trigger 组件的属性名称更新更新了传递给 Trigger 组件的属性名称,确保了组件内部逻辑与新命名保持一致。
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
🧹 Nitpick comments (1)
tests/index.test.tsx (1)
221-241
: 测试名称与属性不一致测试名称仍然使用的是
visible
,但现在组件属性已经更新为open
。建议同时更新测试名称以保持一致性。-it('visible', () => { +it('open', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Tooltip.tsx
(4 hunks)tests/index.test.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Tooltip.tsx
🔇 Additional comments (2)
tests/index.test.tsx (2)
225-225
: 属性名称变更正确将
visible
属性更改为open
与PR的重命名目标一致,提高了API的清晰度。
266-266
: 属性名称变更正确将
visible
属性更改为open
符合组件API的一致性要求。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #494 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 33 30 -3
Branches 12 11 -1
=========================================
- Hits 33 30 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Chores
Refactor