-
Notifications
You must be signed in to change notification settings - Fork 1.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
fix: siwe modal not opening for some wallets #3012
Conversation
🦋 Changeset detectedLatest commit: 205e8b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 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 Git ↗︎
|
♻️ Vite-Size ♻️Size Difference
Current Size
Base Size
|
view: 'ConnectingSiwe' | ||
}) | ||
} | ||
ModalController.open({ |
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.
One thing is ModalController.open has await ApiController.state.prefetchPromise
logic. This might cause us to refetch many things unnecessarily.
0a7f89b
to
205e8b0
Compare
} else if (!this.isSiweEnabled) { | ||
ModalController.close() | ||
} |
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.
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.
this.caipAddress = caipAddress | ||
|
||
if (nextConnected && !isSameAddress && this.isSiweEnabled) { |
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.
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.
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.
It this the one fixing the main issue?
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.
Yeah
: undefined | ||
const nextConnected = caipAddress ? CoreHelperUtil.getPlainAddress(caipAddress) : undefined | ||
const isSameAddress = prevConnected === nextConnected | ||
|
||
this.caipAddress = caipAddress |
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.
Are we sure we want to set this before doing some logics?
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.
Might it be better to do it at the end?
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.
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.
Description
There was an issue where if you used SIWE and connected with your mobile wallet, the SIWE modal never showed up.
Type of change
Associated Issues
For Linear issues: Closes APKT-1254
Checklist