coinbase / onchainkit

React components and TypeScript utilities to help you build top-tier onchain apps.
https://onchainkit.xyz
MIT License
534 stars 123 forks source link

feat: ConnectWallet component #720

Closed kyhyco closed 3 months ago

kyhyco commented 3 months ago

What changed? Why?

Screenshot 2024-06-25 at 12 57 22 PM
No ENS Avatar and Name No ENS Avatar with ENS Name ENS Avatar + ENS Name
Screenshot 2024-06-25 at 12 59 41 PM Screenshot 2024-06-25 at 12 51 36 PM Screenshot 2024-06-25 at 12 52 39 PM

Component API

<Wallet>
  <ConnectWallet>
    <Avatar className="h-6 w-6" />
    <Name />
  </ConnectWallet>
  <WalletDropdown>
    <IdentityLayout>
      <Avatar />
      <Name>
        <Badge />
      </Name>
      <Address className={color.foregroundMuted} />
    </IdentityLayout>
    <WalletDisconnect />
    <GoToWalletDashboard />
  </WalletDropdown>
</Wallet>

Notes to reviewers

How has it been tested?

mindapivessa commented 3 months ago

UI:

kyhyco commented 3 months ago

@mindapivessa

Screenshot 2024-06-25 at 2 07 01 PM
mindapivessa commented 3 months ago

@kyhyco can we remove top padding and only add the bottom. I think you just added the top

Zizzamia commented 3 months ago

@kyhyco I like this direction a lot, and I like how you made it super easy to include IdentityProvider.

<Wallet>
  <ConnectWallet>
    <Avatar className="h-6 w-6" />
    <Name />
  </ConnectWallet>
  <WalletDropdown>
    <IdentityLayout>
      <Avatar />
      <Name>
        <Badge />
      </Name>
      <Address className={color.foregroundMuted} />
    </IdentityLayout>
    <WalletDisconnect />
    <GoToWalletDashboard />
  </WalletDropdown>
</Wallet>

So there are only 3 points I would like to reflect as follow up PR.

  1. Should we ship <IdentityLayout />, I feel like it removes the magic. I feel like that's a special case of a WalletDropdownItem, so should this instead being named <WalletDropodownIdentity /> ?
  2. <WalletDisconnect /> let's probably name it <WalletDropdownDisconnect /> so it's clear is part of the dropdown
  3. we need a component that a developer can safely extend the dropdown, with new Links. Maybe <WalletDropdownLink />
  4. if we want to keep <GoToWalletDashboard /> to make it super easy for this particular case, than let's try to have it with a name normalized for this case so <WalletDropdownDashboardLink />