-
Notifications
You must be signed in to change notification settings - Fork 19
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(frontend): move wallet connect btn out of menu into header #3103
Conversation
…alletConnect part out of it.
…ct-btn' into feat(frontend)/move-wallet-connect-btn # Conflicts: # src/frontend/src/lib/components/hero/WalletConnectMenu.svelte
{#if walletOptions} | ||
<MenuWallet on:icMenuClick={hidePopover} /> | ||
{#if yourAddressesOption} | ||
<MenuYourAddresses on:icMenuClick={hidePopover} /> |
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.
Out of curiosity, is there a good reason to not show this option when a user is in the settings route?
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.
that is a good question... I remember there was some talk about this at some point, but the PR is #1900
Maybe worth asking design team?
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.
Perfect, thank you for linking this issue - It seems as the YourAdresses functionality also does not work on the /explore route, so I have to exclude it there as well.
{#if walletOptions} | ||
<MenuWallet on:icMenuClick={hidePopover} /> | ||
{#if yourAddressesOption} | ||
<MenuYourAddresses on:icMenuClick={hidePopover} /> |
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.
that is a good question... I remember there was some talk about this at some point, but the PR is #1900
Maybe worth asking design team?
disabled={$walletConnectPaired} | ||
> | ||
<IconWalletConnect size="22" slot="icon" /> | ||
<svelte:fragment>{$i18n.wallet_connect.text.name}</svelte:fragment> |
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.
no need for svelte:fragment
here
…disconnect button depending
</script> | ||
|
||
{#if nonNullish(listener)} | ||
<WalletConnectButton on:click={disconnect} | ||
>{$i18n.wallet_connect.text.disconnect}</WalletConnectButton | ||
> | ||
{:else} |
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.
@AntonioVentilii-DFINITY How do you test the wallet connect functionality locally?
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 generally connect with https://app.uniswap.org/
src/frontend/src/eth/components/wallet-connect/WalletConnectButton.svelte
Show resolved
Hide resolved
</script> | ||
|
||
{#if nonNullish(listener)} | ||
<WalletConnectButton on:click={disconnect} | ||
>{$i18n.wallet_connect.text.disconnect}</WalletConnectButton | ||
> | ||
{:else} | ||
<WalletConnectButton ariaLabel={$i18n.wallet_connect.text.name} on:click={openWalletConnectAuth} | ||
></WalletConnectButton> |
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.
/>
instead of ></WalletConnectButton>
?
src/frontend/src/eth/components/wallet-connect/WalletConnectButton.svelte
Show resolved
Hide resolved
…ct-btn' into feat(frontend)/move-wallet-connect-btn
Motivation
Instead of displaying the wallet connect button as a menu item in the popover, we want to display it in the header at any time.
Changes
Tests
before:
after: