-
Notifications
You must be signed in to change notification settings - Fork 203
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
Add top-up modal escape [Fixes #383] #495
Conversation
Just commenting on the preview page, it looks great! 👍 Defer to @samajammin on the TypeScript magic. |
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.
Nice @wackerow!
This may have already been an issue (i.e. unrelated from your PR) but I noticed when I'm on a different network other than Mainnet (e.g. Arbitrum), it creates some funky behaviour when trying to connect (nothing happens when I click MetaMask). If I'm on Mainnet, it correctly tells me to switch to Goerli.
Mind making a change here to check if user is on ANY network other than the correct network? If so, we should display that modal to switch:
If you'd like, feel free to create a separate issue for this... ideal logic here might be to auto-switch the network for MetaMask users.
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! left a couple of small suggestions, nothing blocking.
@samajammin Yeah this is another existing issue it looks like. Opened an issue for it, but gonna hold on building it into this PR. Can revisit separately. |
Description
X
to modal pop-up on/top-up
pageRelated issue
[Fixes #383]