-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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/support use modal hooks #6739
feat/support use modal hooks #6739
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6739 +/- ##
==========================================
+ Coverage 92.68% 92.76% +0.07%
==========================================
Files 334 337 +3
Lines 7191 7294 +103
Branches 1795 1775 -20
==========================================
+ Hits 6665 6766 +101
- Misses 517 519 +2
Partials 9 9 ☔ View full report in Codecov by Sentry. |
PR preview has been successfully built and deployed to https://antd-mobile-preview-pr-6739.surge.sh |
ConfigProvider, | ||
Modal, | ||
Popup, | ||
PortalProvider, |
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.
I'm not sure PortalProvider is a good concept. Maybe consider the antd PC style of App component:
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.
In early version, antd PC provide useModal with holder & caller. But most user dont need that. So get App component for unique useHook register (like the PortalProvider but wish App can do more instead of portal inject only)
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.
okay, does it mean that if I refer to app. the style will make more sense with ant design
, right?
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.
Ye. We can currently only provide App a dom without style but provide unique useXX ability. It maybe more easy to release.
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.
got it
Deploy PR preview failed. |
8 similar comments
Deploy PR preview failed. |
Deploy PR preview failed. |
Deploy PR preview failed. |
Deploy PR preview failed. |
Deploy PR preview failed. |
Deploy PR preview failed. |
Deploy PR preview failed. |
Deploy PR preview failed. |
add
PortalProvider
to supportconst { renderModalInPortal } = usePortal
render-imperatively
and all props are 100% the same as beforesupport
const { show, confirm, alert, clear } = useModal
which could render element imperatively and inherit context from<ConfigProvider />
smoothlyModal.show
/Modal.confirm
/Modal.alert
/Modal.clear
if this PR approved, will support
usePopup
in the future