Design System: Button (and stories)

[mica000]

EDIT: This issue will now be tracking the coordination of all button variants and creating stories for them.

I believe we already build this on the swap but not sure if we need to improve it

This ticket for the button with tokens, used for swaps.
I believe we already build this on the swap but not sure if we need to improve it?

These are the states on designs
Default, Hover, Active, Focused, Disabled

• button with tokens
• avatars (There is a safety border on them now)

[mica000]

@fbwoolf and finally, could you also review this one? I believe it's 90% done?

[fbwoolf]

I don't think the component should be name here with token bc that is too specific, should it be ButtonWithIcon? It could be anything, not just a token icon, correct?

[mica000]

True! Maybe avatar? since icon there are instances of an avatar with an icon inside.

[fbwoolf]

When I look at Radix, I feel like Avatar is always associated to a user, is that not how you think about it?
https://www.radix-ui.com/themes/docs/components/avatar

[pete-watters]

token buttons and avatar buttons seem to be the same as ghost but with different child elements.

I would keep the name Avatar for user avatars only and maybe we can just have 3 button variants:

• solid
• outline
• ghost

I don't really understand what token is for, it looks like a DropDownTrigger ?

In any case, the Button doesn't need to know what's inside it. So it could be a ghost that has an icon child / a dropdown trigger

[mica000]

I don't really understand what token is for, it looks like a DropDownTrigger ?

Yes, used to trigger the swaps and possibly send in the future.

So it could be a ghost that has an icon child / a dropdown trigger

Good point.. we could do this - however the icon and token/avatar have different sizes. Considering the new icon package has 24px, avatar/token have 32px, the size of the "button with tokens" is slightly bigger, 48px. Hence, fine to use the ghost variant as long as the size adapts to it. The padding all around them should be 8px (this value was also updated recently with the introduction of new icon set - before it was 12px)

[fbwoolf]

Yeah, agree with @pete-watters here, I think there is a way to simplify these button variants. I'll rename this issue to handle coordinating all buttons and will propose a solution in a PR where we can make sure to coordinate with Figma if needed for naming, etc. I'll handle making stories for the buttons so we can evaluate them all once done.

[fbwoolf]

@kyranjamie do you have any thoughts here too as I work on this one to coordinate the buttons with stories?

[kyranjamie]

I think I share Pete's thoughts. The token/avatar/icon is just a child of the button, not part of the button. So we don't need a specific button component just for this type.

Let's just make sure, and provide some examples in Storybook, of how buttons look with tokens/avatars/icons as children?

// Composing components like this
<Button>
  <Image />
  Text
</Button>
// is better than creating a new one that does everything imo
<ButtonWithIcon />

@mica000 in atomic design speak, the buttons with images in them are molecules composed of atoms (buttons and image) components

[fbwoolf]

Let's just make sure, and provide some examples in Storybook, of how buttons look with tokens/avatars/icons as children?

Yep, agree, thanks.

[mica000]

cc @fabric-8

[fabric-8]

Just a small addition: We'd love to try 48px as a default button heught and see how that'll look overall :) The height is already reflected in the design system button component

Also agree that token buttons should really just be a particular ghost button configuration

[fbwoolf]

@mica000 @fabric-8 just a heads up that the links to Figma in this issue description do not lead to the most up-to-date version. I have been working on this w/out knowing there are more updated designs. I'm not sure how to know as a dev when the links become outdated? I should just make sure to see the Ready for dev label and question if I don't see it?

[fabric-8]

@fbwoolf whoops thanks for the hint, updated — Mistake on our end and you should generally not have to worry about that. Just happened as the ticket has been created a while ago while we reiterated on the button component later on without updating things here