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

feat: CP-9360 wallet selector rework #68

Merged
merged 11 commits into from
Nov 4, 2024
Merged

feat: CP-9360 wallet selector rework #68

merged 11 commits into from
Nov 4, 2024

Conversation

vvava
Copy link
Contributor

@vvava vvava commented Oct 14, 2024

Description

Our old method how we stored the different Wallet Extension has been deprecated we could not detect a lot of them and made the whole thing messy.

Changes

Now we are using EICP-6963 to detect them and add them to the providers list. We have an updated UI as well.

Testing

Install multiple extensions -> go to the playground and try to connect

Screenshots:

Screen.Recording.2024-10-17.at.20.02.45.mov

Checklist for the author

Tick each of them when done or if not applicable.

  • I've covered new/modified business logic with Jest test cases.
  • I've tested the changes myself before sending it to code review and QA.

@vvava vvava changed the title Feat/wallet selector feat: CP-9360 wallet selector rework Oct 17, 2024
Copy link
Member

@meeh0w meeh0w left a comment

Choose a reason for hiding this comment

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

I like where this is going, please let me know when it's ready for review (lots of commented code / console.logs at the moment)👏

Copy link
Member

@meeh0w meeh0w left a comment

Choose a reason for hiding this comment

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

This is awesome! I'm not sure about the coin-flip animation on hover, but other than that I really like it 🎉

@@ -4,4 +4,7 @@ export enum WalletExtensionType {
UNKNOWN = 'UNKNOWN',
RABBY = 'RABBY',
COINBASE = 'COINBASE',
PHANTOM = 'PHANTOM',
Copy link
Member

Choose a reason for hiding this comment

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

I can't really make Phantom appear, I guess they don't announce themselves? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm checking that when I get home later today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
It seems it can work :D

Copy link
Member

@meeh0w meeh0w left a comment

Choose a reason for hiding this comment

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

Actually, let's remove the console.logs 😬

bferenc
bferenc previously approved these changes Nov 4, 2024
Copy link
Contributor

@bferenc bferenc left a comment

Choose a reason for hiding this comment

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

Works great, however (as Michal mentioned) there are some console logs in the changes.

meeh0w
meeh0w previously approved these changes Nov 4, 2024
@bferenc bferenc self-requested a review November 4, 2024 08:48
bferenc
bferenc previously approved these changes Nov 4, 2024
@vvava vvava merged commit 8e61a2a into main Nov 4, 2024
5 of 6 checks passed
@vvava vvava deleted the feat/wallet-selector branch November 4, 2024 17:36
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.

3 participants