-
Notifications
You must be signed in to change notification settings - Fork 117
feat: Change duration to number|false type #379
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
base: master
Are you sure you want to change the base?
Conversation
|
@wanpan11 is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough将通知的 duration 类型扩展为 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API as Notification API
participant Notice as Notice Component
participant Timer
User->>API: open(config)
API->>Notice: render with config.duration
Note right of Notice #DDEBF7: Notice 内部计算 mergedDuration\n(非数值归一为 0)
alt mergedDuration > 0
Note right of Notice #E6F4EA: 启动自动关闭与进度显示
Notice->>Timer: setTimeout(mergedDuration * 1000)
Timer-->>Notice: timeout -> trigger close
Notice->>API: emit close
else mergedDuration === 0
Note right of Notice #FFF4E6: 不启动超时,进度隐藏
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @wanpan11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant functional improvement by extending the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
这个 PR 正确地将 duration prop 的类型更改为 number | false,以控制通知的自动关闭行为。该实现在类型定义、组件逻辑和测试中都是一致的,现在可以正确处理 false 或 0 来阻止自动关闭。我唯一的建议是改进 README.md 中的文档,以更清晰地反映这一新行为。
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(2 hunks)src/Notice.tsx(2 hunks)src/hooks/useNotification.tsx(1 hunks)src/interface.ts(2 hunks)tests/index.test.tsx(4 hunks)tests/stack.test.tsx(2 hunks)
🔇 Additional comments (7)
README.md (1)
100-100: 列宽调整合理将 type 列宽从 50px 增加到 200px 是合理的,因为新的类型
number | false需要更多空间来显示。tests/stack.test.tsx (1)
20-20: 测试更新正确对齐新的 API测试正确地将
duration: null更改为duration: false,这与新的类型定义number | false一致。使用false来禁用自动关闭功能是清晰且符合语义的。Also applies to: 59-59
tests/index.test.tsx (1)
165-165: 测试用例统一更新为新的 API所有相关测试用例都正确地从
duration: null迁移到duration: false。这些更改:
- 与新的类型定义一致
- 保持了禁用自动关闭的原有功能
- 使用
false比null语义更清晰Also applies to: 195-195, 207-207, 493-493
src/hooks/useNotification.tsx (1)
22-22: 类型定义更新正确
NotificationConfig接口中的duration类型从number扩展到number | false,这与src/interface.ts中的类型定义保持一致,允许用户通过传入false来禁用自动关闭功能。src/Notice.tsx (2)
41-41: 进度条显示逻辑更新正确将条件从
duration > 0 && showProgress改为duration && showProgress是正确的。新逻辑:
duration = false或duration = 0→ 不显示进度条(正确)duration > 0→ 显示进度条(如果 showProgress 为 true)这与新的 API 语义一致,其中
false和0都表示禁用自动关闭。
56-56: 自动关闭逻辑更新正确将条件从
duration > 0改为duration的真值检查是正确的。新行为:
duration = false→ 不会自动关闭(新功能)duration = 0→ 不会自动关闭(保持原有行为)duration > 0→ 按指定时间自动关闭注意:此更改确保了
duration = 0和duration = false行为一致,都表示禁用自动关闭。这比之前的duration > 0检查更符合 JavaScript 的真值语义。src/interface.ts (1)
9-9: 核心类型定义更新合理将
duration类型从number | null更改为number | false是一个很好的改进:优点:
false比null更明确地表达"禁用"的语义- 在 TypeScript 中,
false作为布尔值比null更容易进行类型检查- 与其他配置选项的常见模式一致(如
closable: boolean | object)一致性:
- 与
src/hooks/useNotification.tsx中的NotificationConfig类型一致- 与
src/Notice.tsx中的真值检查逻辑一致Also applies to: 35-35
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #379 +/- ##
=======================================
Coverage 99.74% 99.74%
=======================================
Files 7 7
Lines 775 776 +1
Branches 126 127 +1
=======================================
+ Hits 773 774 +1
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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)
src/Notice.tsx (1)
41-41:mergedDuration归一化方法合理将非数字类型的
duration值统一转换为0是清晰的实现方式,后续的> 0检查可以统一处理所有禁用自动关闭的情况(false、null、0)。可选建议: 考虑增加对
Infinity和负数的显式防护,确保只有有效的正数才会触发自动关闭:-const mergedDuration: number = typeof duration === 'number' ? duration : 0; +const mergedDuration: number = typeof duration === 'number' && isFinite(duration) && duration > 0 ? duration : 0;这样可以避免边缘情况(虽然极少见):
Infinity会导致setTimeout永不触发且进度条卡在 0%- 负数的语义不明确(虽然当前实现安全地将其视为"不自动关闭")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Notice.tsx(5 hunks)src/hooks/useNotification.tsx(1 hunks)src/interface.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useNotification.tsx
🔇 Additional comments (3)
src/interface.ts (1)
9-9: 类型扩展正确,保持了向后兼容性
duration类型从number | null扩展到number | false | null是合理的改动:
- 添加
false选项允许显式禁用自动关闭功能- 保留
null确保现有代码升级时不会出现兼容性问题两个接口(
NoticeConfig和OpenConfig)的类型定义保持一致。Also applies to: 35-35
src/Notice.tsx (2)
42-42: 自动关闭逻辑正确更新相关改动正确地使用了
mergedDuration:
- Line 42:
mergedShowProgress基于mergedDuration > 0判断,只有有效正数才显示进度条- Line 57: 自动关闭的触发条件改为
mergedDuration > 0,符合预期语义- Line 63: 超时计算使用
mergedDuration * 1000- Line 74: 依赖数组正确更新为
[mergedDuration, ...],因为 effect 内部使用的是归一化后的值这些改动确保了只有有效的正数 duration 才会触发自动关闭,
0、false、null等值都会被正确处理为"不自动关闭"。Also applies to: 57-74
85-85: 进度条计算逻辑正确更新进度条相关的改动与超时逻辑保持一致:
- Line 85: 进度计算使用
mergedDuration * 1000作为总时长- Line 102: 依赖数组正确包含
mergedDuration改动确保了进度条的行为与自动关闭逻辑同步,只有在
mergedDuration > 0时才会显示和更新进度。Also applies to: 102-102
ref: ant-design/ant-design#55465 (comment)
duration = 0 | false 时 不自动关闭
Summary by CodeRabbit
新功能
false来显式禁用自动关闭,相关行为(如进度显示与超时判断)被统一为对“假值”的一致处理。文档更新
number | false,默认时长示例更新为 4.5 秒,表格中“type”列宽扩大以提升可读性,并补充 0/false 可禁用自动关闭的说明。重构