-
Notifications
You must be signed in to change notification settings - Fork 477
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
[SDK] Add onClose callback to details modal #5605
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: e9c23ad The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Your org has enabled the Graphite merge queue for merging into mainAdd the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5605 +/- ##
==========================================
+ Coverage 54.12% 54.53% +0.40%
==========================================
Files 1102 1105 +3
Lines 59206 59332 +126
Branches 4886 4924 +38
==========================================
+ Hits 32044 32354 +310
+ Misses 26438 26258 -180
+ Partials 724 720 -4
*This pull request uses carry forward flags. Click here to find out more.
|
4e1ad8d
to
a1ddcc2
Compare
a1ddcc2
to
d15a998
Compare
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.
LGTM but I'm a little confused how this solves the original issue, also why we need the screen
param to be passed to onClose
(feels like something a little too internal to be exposing). I trust though
f7728bd
to
ee38970
Compare
ee38970
to
c6ef3f2
Compare
c6ef3f2
to
1154410
Compare
CNCT-2444 CNCT-2444 <!-- start pr-codex --> --- ## PR-Codex overview This PR primarily introduces an `onClose` callback for the `ConnectButton` component's details modal, enhancing its functionality. It also improves the rendering of various components and adds tests for new features. ### Detailed summary - Added `onClose` callback to `ConnectButtonProps` for modal closure. - Updated `ConnectButton` to utilize the `onClose` callback. - Improved test coverage for `NetworkSwitcherButton` and `ChainIcon`. - Enhanced `fetchChainIcon` functionality to handle various icon resolvers. - Introduced `ChainActiveDot` for visual representation of active chains. - Updated `NetworkSelector` and `DetailsModal` components for better user experience. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex -->
1154410
to
e9c23ad
Compare
CNCT-2444
CNCT-2444
PR-Codex overview
This PR focuses on enhancing the
ConnectButton
component by adding anonClose
callback for the details modal, improving functionality and user experience. It also includes various updates to theChainIcon
,NetworkSwitcherButton
, and related components.Detailed summary
onClose
callback toConnectButtonProps
for modal closure.ConnectButton
documentation to include usage ofonClose
.NetworkSwitcherButton
to utilize the newonClose
callback.NetworkSwitcherButton
and related components.ChainIcon
andfetchChainIcon
for better icon resolution.