search

leather-io / extension

Leather browser extension
https://leather.io
MIT License
292 stars 140 forks source link

Design System: Label #4894

Open mica000 opened 7 months ago

mica000 commented 7 months ago
mica000 commented 7 months ago

@kyranjamie Also curious on your take around this component.

kyranjamie commented 7 months ago

Looks fine to build, but a few thoughts:

mica000 commented 7 months ago

Is CopyLabel used for anything else other than copying things? What should the default state say, if so "Copy"? It says caption right now. Would it change in certain cases or always be the same?

@kyranjamie Here is an example of how both being used for the current UI. The idea is the have the address replace "copy" and allow the user to copy from this screen.

Image

fbwoolf commented 7 months ago

I am still unclear here about why these need to be shared components? The checkmark next to a piece of text seems pretty minor. Typically we just add a CheckmarkIcon next to the span text. With the radix Select, the checkmark icon is placed automatically for us, this really can't be shared bc it is specific to the radix primitive.

The CopyLabel might be if it is interactive, but I'm unclear abt it atm. So, when a user hovers over the address in your example, the text changes to copy with the CopyIcon beside it?

mica000 commented 7 months ago

Typically we just add a CheckmarkIcon next to the span text. With the radix Select, the checkmark icon is placed automatically for us, this really can't be shared bc it is specific to the radix primitive.

Ah ok! this I didnt knew. We added it this component because it was needed for Radix. Then Radix primitives are not going to be shared with the rest of the design system? In this case, would make sense to move the Label to the ElevatedList page in Figma, right?

The CopyLabel might be if it is interactive, but I'm unclear abt it atm. So, when a user hovers over the address in your example, the text changes to copy with the CopyIcon beside it?

Yes, that was the intended behavior for this one. The rational behind was to allow users to copy their address quickly from the accounts modal. However, I'm thinking that since this area will be redesigned in the Wallet UX, pherhaps we can discard this functionality from this step for now. cc @markmhendrickson

fbwoolf commented 6 months ago

Looking at Radix, they do have a Label primitive, but it is important to keep in mind that the word Label is referring to an accessible label associated with controls. I think our use of the term in designs/Figma for just text anywhere is slightly confusing.

https://www.radix-ui.com/primitives/docs/components/label

fbwoolf commented 4 months ago

@kyranjamie I don't think this component is nec, is it?