-
-
Notifications
You must be signed in to change notification settings - Fork 199
feat: Merge 'afterClose' to 'closable' #500
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?
feat: Merge 'afterClose' to 'closable' #500
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次变更将 Dialog 组件的关闭相关回调 afterClose 从顶层属性迁移到 closable 对象中,形成统一的 API 设计。涉及类型定义、文档、实现和测试的同步调整,保留了旧 afterClose 属性但标注为废弃,并确保新旧回调均被调用。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dialog
participant DialogWrap
User->>Dialog: 关闭 Dialog
Dialog->>DialogWrap: 触发关闭流程
alt closable.afterClose 存在
DialogWrap->>DialogWrap: 调用 closable.afterClose()
end
alt 顶层 afterClose 存在
DialogWrap->>DialogWrap: 调用 afterClose()
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 分钟 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
src/DialogWrap.tsxOops! Something went wrong! :( ESLint: 7.32.0 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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. 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 (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #500 +/- ##
==========================================
+ Coverage 98.43% 98.46% +0.02%
==========================================
Files 8 8
Lines 192 195 +3
Branches 66 68 +2
==========================================
+ Hits 189 192 +3
Misses 3 3 ☔ 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/Dialog/index.tsx (1)
170-170
: 建议优化 useEffect 依赖项以提升性能将
doClose
添加到依赖数组中虽然符合 React Hook 规则,但会导致不必要的重新渲染,因为doClose
函数在每次组件渲染时都会重新创建。建议使用
useCallback
包装doClose
函数来优化性能:- function doClose() { + const doClose = React.useCallback(() => { // Clean up scroll bar & focus back setAnimatedVisible(false); if (mask && lastOutSideActiveElementRef.current && focusTriggerAfterClose) { try { lastOutSideActiveElementRef.current.focus({ preventScroll: true }); } catch (e) { // Do nothing } lastOutSideActiveElementRef.current = null; } // Trigger afterClose only when change visible from true to false if (animatedVisible) { afterClose?.(); } - } + }, [mask, focusTriggerAfterClose, animatedVisible, afterClose]);这样可以避免 useEffect 因为
doClose
函数的重新创建而频繁执行。
将afterClose属性合并到closable中,并添加测试与修改文档
afterClose -> closable.afterClose
关联issue ant-design/ant-design#54394
Summary by CodeRabbit
文档
closable
属性的说明,支持在对象类型中新增可选的afterClose
回调,并将独立的afterClose
属性标记为已废弃。新特性
closable
属性对象传递afterClose
回调函数,实现更灵活的关闭后处理。测试
afterClose
回调时的行为测试,确保兼容性。