-
Notifications
You must be signed in to change notification settings - Fork 992
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
[#20150] feat: wallet connect pairing via deep links #21050
Conversation
00f3dc8
to
5dc907b
Compare
Jenkins BuildsClick to see older builds (20)
|
572ace4
to
e6783e8
Compare
I would appreciate any suggestion about how we can test app with installed dapps, in this website https://lab.web3modal.com/library/wagmi-siwe/ there is an option to add custom wallet but in example native app there is no such option. |
We should submit it to the WalletConnect cloud so it's available in the explorer (https://walletconnect.com/explorer-guidelines). @jakubgs can we get access to the project to submit the app? Also, did we do this before with v1? |
@clauxx I can see old status app here https://explorer.walletconnect.com/status, the deeplink schema is the same( |
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.
Have a few "stylistic" comments, but otherwise looks good!
(when platform/android? | ||
(rf/dispatch [:wallet-connect/redirect-to-dapp])) |
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.
did you try it on iOS as well? I wonder what happens when we try to open the dapp.
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.
I first tested it with url
and it was working fine in both platform but with native
as i couldn't test it with installed app still don't know.
I may create a simple dapp to test logic if it doesn't take time
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.
I may create a simple dapp to test logic if it doesn't take time
This may be too time consuming I guess
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 i thought too, another option is to build app in release mode and test it with https://explorer.walletconnect.com/status, will let you know if it works fine
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.
I removed the platform condition to have chance to test it with both platform, if in iOS it cause any issue i will disable it, in wallet connect doc it didn't mention any issue about iOS, lets see
src/status_im/contexts/wallet/wallet_connect/responding_events.cljs
Outdated
Show resolved
Hide resolved
@clauxx yes, we have a WalletConnect project created over 2 years ago: |
Do you know what's with the draft tag below it? |
e6783e8
to
0f85f61
Compare
Hey @mohsen-ghafouri! Thanks for the PR. Could you please resolve the conflicts and rebase the branch? Thank you. |
b66712c
to
9aa96a0
Compare
Hey @pavloburykh, thanks for checking this, done. |
71% of end-end tests have passed
Failed tests (2)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Passed tests (5)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
|
@mohsen-ghafouri thanks for the PR. No issues from my side. Please, let us know once wallet connect team approves our new deep link configuration. After that we will re-check this feature using real dapps. Meanwhile I think we can merge this PR. |
Thanks @pavloburykh for you time, i will merge it once test pipeline being fixed. |
9aa96a0
to
a4cae86
Compare
fixes #20150
Summary
As a user, I want to pair with the wallet via deeplinks, so that I can interact with the dApps I have on my phone.
Areas that maybe impacted
Steps to test
Result
Simulator.Screen.Recording.-.iPhone.13.-.2024-08-19.at.19.22.18.mp4
status: ready