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

Added open magic wallet functionality #10019

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

Israellund
Copy link
Collaborator

@Israellund Israellund commented Nov 25, 2024

Link to Issue

Closes: #5309

Description of Changes

  • Added open magic wallet functionality for accounts made through email and Google

NOTE: I made these changes based off of the Figma designs. I tried desperately to embed the widget in the account creation modal as was shown in the original designs, but unfortunately widget's don't work that way. Sabina and I decided to simply replace it with an image and change the copy telling the user where they can access their magic wallet

Test Plan

  • Sign in with email
  • confirm you see the arrow-square-out icon, and when you hover over it a tooltip reads "Open wallet"
  • click the icon and confirm you see the Magic wallet widget
  • go to a community with Stake and click "Buy Stake"
  • confirm you see the "Add funds" button
  • click it and confirm you see the Magic widget
  • click the profile dropdown and confirm you see "Open wallet" with the arrow-square-out icon
  • click either the text or the icon and confirm that you see the Magic widget
  • in the Profile dropdown click "My community stake"
  • on My Community Stake page confirm you see the Open wallet button
  • click the button and confirm you see the Magic widget.
  • Sign in with Google and go through all of the above steps
  • Sign in with any other wallet/method and confirm that you do not see any of the above, as they are set to only show when a user has a Magic wallet.
Screenshot 2024-11-25 at 10 12 43 AM Screenshot 2024-11-25 at 10 13 00 AM Screenshot 2024-11-25 at 10 13 10 AM Screenshot 2024-11-25 at 10 13 20 AM Screenshot 2024-11-25 at 10 14 14 AM

Comment on lines +60 to +67
const openMagicWallet = async () => {
try {
await magic.wallet.showUI();
} catch (error) {
console.trace(error);
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

can we move this to useAuthentication?

@@ -25,6 +25,8 @@ const UpdateCustomDomainTask = () => {
buttonType: 'destructive',
buttonHeight: 'sm',
onClick: () => {
console.log('communityId', communityId);
console.log('customDomain', customDomain);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lol nope

Comment on lines +259 to +266
const openMagicWallet = async () => {
try {
await magic.wallet.showUI();
} catch (error) {
console.trace(error);
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

lets move this to useAuthentication

Comment on lines +136 to +137
const userData = useUserStore();
const hasMagic = userData.addresses?.[0]?.walletId === WalletId.Magic;
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

Comment on lines +71 to +76
case WelcomeOnboardModalSteps.MagicWallet: {
return {
index: 4,
title: 'Magic Wallet Creation',
component: (
<MagicWalletCreationStep
Copy link
Contributor

Choose a reason for hiding this comment

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

magic wallets are only made for sms + other sso methods right? If yes shouldn't this be displayed conditionally when a signup with one of those method happens?

@@ -83,6 +83,8 @@ export enum WalletSsoSource {
Twitter = 'twitter',
Apple = 'apple',
Email = 'email',
Farcaster = 'farcaster',
Copy link
Contributor

Choose a reason for hiding this comment

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

is farcaster logic a part of this pr?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mzparacha this branch was made off of a branch Kurtis made, at the request of Dillon. So I believe that is a change he made.

Comment on lines +168 to +175
const openMagicWallet = async () => {
try {
await magic.wallet.showUI();
} catch (error) {
console.trace(error);
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

move to useAuthentication

Copy link
Contributor

@mzparacha mzparacha left a comment

Choose a reason for hiding this comment

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

SMS login works, didn't test other sso methods. Codewise lgtm

Copy link
Contributor

@ianrowan ianrowan left a comment

Choose a reason for hiding this comment

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

tested, LGTM from my POV other than Malik's comments + CI failure

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.

Include Magic Embedded Wallet into UI if Address Source is from Magic
4 participants