- 
                Notifications
    
You must be signed in to change notification settings  - Fork 117
 
feat: Add onClose to Closable #376
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
Conversation
| 
           The latest updates on your projects. Learn more about Vercel for Git ↗︎ 
  | 
    
          Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@           Coverage Diff           @@
##           master     #376   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files           7        7           
  Lines         772      777    +5     
  Branches      124      126    +2     
=======================================
+ Hits          770      775    +5     
  Misses          2        2           ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
  | 
    
          
Walkthrough本次变更扩展了通知组件中  Changes
 Sequence Diagram(s)sequenceDiagram
    participant User
    participant Notification
    participant Config
    User->>Notification: 关闭通知(如点击关闭按钮或超时)
    Notification->>Config: 检查 closable
    alt closable 是对象且有 onClose
        Notification->>Config: 调用 closable.onClose()
    else
        Notification->>Config: 调用 config.onClose()
    end
    Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 分钟 Suggested reviewers
 Poem
 Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
 🚧 Files skipped from review as they are similar to previous changes (2)
 ✨ Finishing Touches
 🧪 Generate unit tests
 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit: 
 SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)src/Notifications.tsx(1 hunks)src/hooks/useNotification.tsx(1 hunks)src/interface.ts(1 hunks)tests/index.test.tsx(1 hunks)
🔇 Additional comments (4)
src/interface.ts (1)
13-15: 类型定义实现正确
closable属性的类型扩展实现得很好:
- 保持了向后兼容性(支持布尔值)
 - 正确添加了对象形式,包含可选的
 closeIcon和onClose属性- 包含了
 React.AriaAttributes以支持无障碍功能- 类型安全性得到保障
 src/hooks/useNotification.tsx (1)
18-20: 钩子接口类型定义一致性良好
NotificationConfig接口中closable属性的类型定义与interface.ts中的定义保持一致,这确保了:
- 类型安全性
 - 可以在钩子级别配置默认的
 closable.onClose行为- 与核心接口定义的一致性
 src/Notifications.tsx (1)
48-52: 关闭回调优先级逻辑实现正确关闭通知时的回调函数优先级逻辑实现得很好:
- 首先检查
 closable是否为对象类型且不为 null- 优先使用
 closable.onClose回调- 如果
 closable.onClose不存在,则回退到config.onClose- 安全地调用确定的回调函数
 这正确实现了 PR 目标中描述的优先级行为,空值检查也增强了代码的健壮性。
tests/index.test.tsx (1)
781-877: 测试覆盖率全面且实现正确新增的测试套件 "onClose and closable.onClose" 很好地覆盖了新功能的关键场景:
- 基本功能测试:验证
 closable.onClose回调被正确调用- 优先级测试:确认
 closable.onClose优先于顶级onClose被调用- 配置继承测试:验证钩子级别的
 closable.onClose配置正常工作所有测试都正确地:
- 设置了 1 秒持续时间的通知
 - 使用
 vi.runAllTimers()触发过期- 断言正确的回调被调用
 - 通过卸载组件进行清理
 测试实现稳健,全面验证了功能的正确性。
closable添加onClose中,保留原onClose,优先调用closable.onClose
关联issue:ant-design/ant-design#54394
Summary by CodeRabbit
新功能
文档
测试