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

Web3GL Template - Onboard.js Overhaul #348

Merged
merged 3 commits into from
Mar 9, 2023
Merged

Conversation

RyRy79261
Copy link
Contributor

Changelog

  • Web3gl template has been replaced by Onboard.js
  • Template is minified using Vite

@RyRy79261 RyRy79261 self-assigned this Mar 3, 2023
sneakzttv
sneakzttv previously approved these changes Mar 3, 2023
KBryan
KBryan previously approved these changes Mar 3, 2023
Copy link
Contributor

@juans-chainsafe juans-chainsafe left a comment

Choose a reason for hiding this comment

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

The following wallets are not working:

  • Crypto.com
  • Wallet3
  • Exodus
  • Bobablocks
  • Verso
    Example error: Failed to launch 'cryptowallet://wc?uri=wc%3A2babb67a-2f0d-4d04-a285-a04a6c2d5bf8%401%3Fbridge%3Dhttps%253A%252F%252Fx.bridge.walletconnect.org%26key%3D73ecfb1cf0a8ad4796d41257cbf1308d871d14684ca698510e3e897e729779bf' because the scheme does not have a registered handler.

Screenshot 2023-03-03 at 13 57 16

Also, Wallet3 is repeated in page 1 and page 2

@sneakzttv
Copy link
Contributor

gj @juans-chainsafe i didn't even think to check the alternate wallets, i just checked metamask and wc, my fault there.

@RyRy79261
Copy link
Contributor Author

@juans-chainsafe Thanks for the rigorous testing, though I'm unsure if thats within the scope of what I can do in this code base.

Like wallet connect is a standard, so if those wallets dont implement it, then I'm not sure, it says we need a registered handler, but as far as onboard is concerned, wallet connect is wallet connect and I just specify a bridge if I want.

@FSM1 ever seen this?

@RyRy79261 RyRy79261 dismissed stale reviews from KBryan and sneakzttv via 11be701 March 8, 2023 12:44
@RyRy79261
Copy link
Contributor Author

This is the reference set up config

@RyRy79261
Copy link
Contributor Author

@juans-chainsafe

So those links you were trying to open appear to be for opening up apps on mobile. The apps name as the protocol portion should normally deep link open the specific app.

You make reference to Page 1 & 2 where are you seeing pages?

@juans-chainsafe
Copy link
Contributor

@juans-chainsafe

So those links you were trying to open appear to be for opening up apps on mobile. The apps name as the protocol portion should normally deep link open the specific app.

You make reference to Page 1 & 2 where are you seeing pages?

They appear after generating a build on Unity with the WebGL template:
page 1
Screenshot 2023-03-09 at 11 44 08
page 2
Screenshot 2023-03-09 at 11 46 28

(Now Wallet3 is repeated on page 1)

  • I also noticed that the Search input doesn't work

One question, in the modal says "Desktop" so why they appear there if they are meant for mobile?

@RyRy79261
Copy link
Contributor Author

@juans-chainsafe Ahh ok, so thats wallet connects specific pages.

Wallet connect is for connecting wallets that aren't installed on the local machine via a QR code or deep linking.

You're clicking Desktop implying that you have these wallets installed on your desktop machine

Copy link
Contributor

@juans-chainsafe juans-chainsafe left a comment

Choose a reason for hiding this comment

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

due that the above errors are not on our side, I approve this PR

Copy link
Contributor

@sneakzttv sneakzttv left a comment

Choose a reason for hiding this comment

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

lets light this bad boy up

@sneakzttv sneakzttv merged commit 0771e58 into main Mar 9, 2023
@RyRy79261 RyRy79261 deleted the feat/onboard-template branch March 9, 2023 16:41
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.

None yet

4 participants