fix: embedded modal not closing after connecting #5367
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: a53ce1f The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
10 Skipped Deployments
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
tomiir
reviewed
Nov 27, 2025
Comment on lines
+38
to
+45
| "@reown/appkit": "1.8.14-embedded-mode-fix.0", | ||
| "@reown/appkit-adapter-bitcoin": "1.8.14-embedded-mode-fix.0", | ||
| "@reown/appkit-adapter-ethers": "1.8.14-embedded-mode-fix.0", | ||
| "@reown/appkit-adapter-solana": "1.8.14-embedded-mode-fix.0", | ||
| "@reown/appkit-common": "1.8.14-embedded-mode-fix.0", | ||
| "@reown/appkit-controllers": "1.8.14-embedded-mode-fix.0", | ||
| "@reown/appkit-scaffold-ui": "1.8.14-embedded-mode-fix.0", | ||
| "@reown/appkit-ui": "1.8.14-embedded-mode-fix.0", |
Collaborator
There was a problem hiding this comment.
Is this a canary from the fix currently being implemented?
Contributor
Visual Regression Test Results ✅ Passed✨ No visual changes detected Chromatic Build: https://www.chromatic.com/build?appId=6493191bf4b10fed8ca7353f&number=467 |
tomiir
reviewed
Nov 27, 2025
Comment on lines
247
to
258
| const isSwitchingNamespace = ChainController.state.isSwitchingNamespace | ||
| const isInProfileView = RouterController.state.view === 'ProfileWallets' | ||
|
|
||
| const shouldClose = !caipAddress && !isSwitchingNamespace && !isInProfileView | ||
| if (shouldClose) { | ||
| const shouldCloseModal = !caipAddress && !isSwitchingNamespace && !isInProfileView | ||
|
|
||
| const hasPreviouslyDisconnected = !CoreHelperUtil.getPlainAddress(this.caipAddress) | ||
| const shouldCloseEmbeddedModal = | ||
| this.enableEmbedded && Boolean(caipAddress) && hasPreviouslyDisconnected | ||
|
|
||
| if (shouldCloseModal || shouldCloseEmbeddedModal) { | ||
| ModalController.close() | ||
| } |
Collaborator
There was a problem hiding this comment.
private async onNewAddress(newCaipAddress?: CaipAddress) {
const oldAddress = CoreHelperUtil.getPlainAddress(this.caipAddress)
const newAddress = CoreHelperUtil.getPlainAddress(newCaipAddress)
const isSwitchingNamespace = ChainController.state.isSwitchingNamespace
const isInProfileView = RouterController.state.view === 'ProfileWallets'
const isDisconnectedAndIdle = !newAddress && !isSwitchingNamespace && !isInProfileView
const isReconnect = this.enableEmbedded && !oldAddress && !!newAddress
if (isDisconnectedAndIdle || isReconnect) {
ModalController.close()
}
}
can we apply something like this? think we should have more descriptive boolean names if we are introducing new conditions
Collaborator
There was a problem hiding this comment.
maybe isreconnect is not the proper one but I think you get my point
shouldClose tells me nothing about why it should close
tomiir
approved these changes
Nov 27, 2025
Collaborator
tomiir
left a comment
tomiir
left a comment
There was a problem hiding this comment.
Approved with one nit comment
Contributor
📦 Bundle Size Check✅ All bundles are within size limits 📊 View detailed bundle sizes> @reown/appkit-monorepo@1.7.1 size /home/runner/work/appkit/appkit > size-limit |
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixed an issue where the embedded modal did not close properly after connecting
Type of change
Showcase (Optional)
If there is a UI change include the screenshots with before and after state.
If new feature is being introduced, include the link to demo recording.
Checklist
Note
Fixes embedded modal not closing after connect and updates demo to patched packages; adds TON namespace in constants.
onNewAddressinpackages/scaffold-ui/src/modal/w3m-modal/index.tsto close when embedded and a newcaipAddressappears after a prior disconnect; renameshouldClose->shouldCloseModaland importCoreHelperUtil.tontoNAMESPACE_NETWORK_IDS_MAPinapps/demo/lib/constants.ts.@reown/*dependencies to1.8.14-embedded-mode-fix.0inapps/demo/package.json(and testing package) to consume the fix.@reown/*packages noting the embedded modal close fix.Written by Cursor Bugbot for commit a53ce1f. This will update automatically on new commits. Configure here.