Skip to content
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.

Sign up for GitHub

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

fix: siwe modal not opening for some wallets #3012

Merged
merged 1 commit into from
Oct 7, 2024
Merged

fix: siwe modal not opening for some wallets #3012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/giant-moons-guess.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
'@reown/appkit-scaffold-ui': patch
'@apps/laboratory': patch
'@apps/demo': patch
'@apps/gallery': patch
'@reown/appkit-adapter-ethers': patch
'@reown/appkit-adapter-ethers5': patch
'@reown/appkit-adapter-polkadot': patch
'@reown/appkit-adapter-solana': patch
'@reown/appkit-adapter-wagmi': patch
'@reown/appkit': patch
'@reown/appkit-utils': patch
'@reown/appkit-cdn': patch
'@reown/appkit-common': patch
'@reown/appkit-core': patch
'@reown/appkit-ethers': patch
'@reown/appkit-ethers5': patch
'@reown/appkit-polyfills': patch
'@reown/appkit-siwe': patch
'@reown/appkit-solana': patch
'@reown/appkit-ui': patch
'@reown/appkit-wagmi': patch
'@reown/appkit-wallet': patch
---

Fixed an issue where SIWE modal wasn't showing up for some mobile wallets.
28 changes: 17 additions & 11 deletions packages/scaffold-ui/src/modal/w3m-modal/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,31 +189,37 @@ export class W3mModal extends LitElement {
}

private async onNewAddress(caipAddress?: CaipAddress) {
const prevConnected = this.caipAddress
? CoreHelperUtil.getPlainAddress(this.caipAddress)
const prevCaipAddress = this.caipAddress
const prevConnected = prevCaipAddress
? CoreHelperUtil.getPlainAddress(prevCaipAddress)
: undefined
const nextConnected = caipAddress ? CoreHelperUtil.getPlainAddress(caipAddress) : undefined
const isSameAddress = prevConnected === nextConnected

this.caipAddress = caipAddress
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure we want to set this before doing some logics?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might it be better to do it at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's better to do at the end, but seems like there is some async tasks that's blocking it from having address being assigned earlier. Otherwise in here it's going to close the modal.


if (nextConnected && !isSameAddress && this.isSiweEnabled) {
Comment on lines +199 to 201
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were some asynchronous tasks happening before setting caipAddress. In here i just made sure to assign caipAddress before asynchronous tasks start to kick in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It this the one fixing the main issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah

const { SIWEController } = await import('@reown/appkit-siwe')
const signed = AccountController.state.siweStatus === 'success'
try {
const { SIWEController } = await import('@reown/appkit-siwe')
const signed = AccountController.state.siweStatus === 'success'

if (!prevConnected && nextConnected) {
this.onSiweNavigation()
} else if (signed && prevConnected && nextConnected && prevConnected !== nextConnected) {
if (SIWEController.state._client?.options.signOutOnAccountChange) {
await SIWEController.signOut()
if (!prevConnected && nextConnected) {
this.onSiweNavigation()
} else if (signed && prevConnected && nextConnected && prevConnected !== nextConnected) {
if (SIWEController.state._client?.options.signOutOnAccountChange) {
await SIWEController.signOut()
this.onSiweNavigation()
}
}
} catch (err) {
this.caipAddress = prevCaipAddress
throw err
}
}

if (!nextConnected) {
ModalController.close()
}

this.caipAddress = caipAddress
}

private async onNewNetwork(nextCaipNetwork: CaipNetwork | undefined) {
Expand Down
4 changes: 3 additions & 1 deletion packages/scaffold-ui/src/views/w3m-connecting-wc-view/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export class W3mConnectingWcView extends LitElement {

@state() private platforms: Platform[] = []

@state() private isSiweEnabled = OptionsController.state.isSiweEnabled

public constructor() {
super()
this.determinePlatforms()
Expand Down Expand Up @@ -72,7 +74,7 @@ export class W3mConnectingWcView extends LitElement {
OptionsController.state.hasMultipleAddresses
) {
RouterController.push('SelectAddresses')
} else {
} else if (!this.isSiweEnabled) {
ModalController.close()
}
Comment on lines +77 to 79
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we did RouterController.replace('ConnectingSiwe'), right after the modal would be closed. I did a small check just to make sure if siwe is not enabled then we go ahead and close the modal.

}
Expand Down
Loading