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.

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: connect-coinbase-wallet #1137

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

karczuRF
Copy link
Contributor

@karczuRF karczuRF commented Jun 24, 2022

Summary

Fixes #1133

If MetaMask and Coinbase wallet are both installed and enabled an error occurs during Coinbase connecting. It is not possible to use this one.

Important note

This fix is just to give an user possibility to connect coinbase wallet. So should be possible to select "Coinbase" from the list and use this wallet if both Coinbase and MetaMask are installed or just Coinbase.
This PR doesn't fix the issue with two popups appearing the same time, switching between wallets or disconnecting them. For that problems another PR is dedicated: #817 and will be applied soon.

image

To Test

Open Swapr app:

  • Check if MetaMask can be connected
  • Check if Coinbase can be connected
  • Both wallets can be used as expected (swith network, approve transaction etc)

@netlify
Copy link

netlify bot commented Jun 24, 2022

Deploy Preview for swapr ready!

Name Link
🔨 Latest commit 34c0a56
🔍 Latest deploy log https://app.netlify.com/sites/swapr/deploys/62b5c273883c7700083b5d39
😎 Deploy Preview https://deploy-preview-1137--swapr.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@MilanVojnovic95
Copy link
Collaborator

Tested and It still doesn't work properly.

  1. When I try to connect on Coinbase
  2. I get Coinbase wallet and approve connection
  3. Then it ask me for password on my Metamask
  4. I type password
  5. Then when I change wallets I never get different balances depending on the wallet (because for example on Metamask I have 0.097 ETH and on Coinbase 0 ETH)

I guess it doesn’t actually view it as two different wallets but still views it as one 🤔

@niemam29 @karczuRF
Is there anything in the Chrome settings that I should do regarding the wallets to test this PR?

@karczuRF
Copy link
Contributor Author

Tested and It still doesn't work properly.

  1. When I try to connect on Coinbase
  2. I get Coinbase wallet and approve connection
  3. Then it ask me for password on my Metamask
  4. I type password
  5. Then when I change wallets I never get different balances depending on the wallet (because for example on Metamask I have 0.097 ETH and on Coinbase 0 ETH)

I guess it doesn’t actually view it as two different wallets but still views it as one thinking

Thanks for testing!
I need to add some clarifications. This fix is just to give an user possibility to connect coinbase wallet. So should be possible to select "Coinbase" from the list and use this wallet if both Coinbase and MetaMask are installed or just Coinbase.
This PR doesn't fix the issue with two popups appearing the same time, switching between wallets or disconnecting them. For that problems another PR is dedicated: #817 and will be applied soon.

@niemam29 @karczuRF Is there anything in the Chrome settings that I should do regarding the wallets to test this PR?

No, all should be set by default.

@MilanVojnovic95
Copy link
Collaborator

Tested and It still doesn't work properly.

  1. When I try to connect on Coinbase
  2. I get Coinbase wallet and approve connection
  3. Then it ask me for password on my Metamask
  4. I type password
  5. Then when I change wallets I never get different balances depending on the wallet (because for example on Metamask I have 0.097 ETH and on Coinbase 0 ETH)

I guess it doesn’t actually view it as two different wallets but still views it as one thinking

Thanks for testing! I need to add some clarifications. This fix is just to give an user possibility to connect coinbase wallet. So should be possible to select "Coinbase" from the list and use this wallet if both Coinbase and MetaMask are installed or just Coinbase. This PR doesn't fix the issue with two popups appearing the same time, switching between wallets or disconnecting them. For that problems another PR is dedicated: #817 and will be applied soon.

@niemam29 @karczuRF Is there anything in the Chrome settings that I should do regarding the wallets to test this PR?

No, all should be set by default.

Ok, then as far for this PR and testing is concerned everything is ok.
It is possible to connect Coinbase wallet when you have just Coinbase or Coinbase and Metamask installed 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problems when using both coinbase and metamask
4 participants