deprecate / metal-clay-components

10 stars 14 forks source link

Clay Link icon #180

Closed carloslancha closed 6 years ago

carloslancha commented 6 years ago

Do we really need an specific parameter icon in this component?

If we want ClayLink to render an icon we can just pass it in the label.

What do you think?

matuzalemsteles commented 6 years ago

Hey @carloslancha, With the icon parameter I see that we make it easy for the developer to use when using soy for example, but when used in JSP it becomes a bit more complex to have to add HasMap. But it can ignore icon and go through the label as an alternative.

From another point of view, if someone simply wants to use a icon quickly using the icon parameter is more viable and simple than having to call clay-icon 'and aligning it inside the label, I see as an alternative to a quick and simple use case.

I see that we are not limiting its use but rather leaving options for its use cases.

jbalsas commented 6 years ago

I agree that we should keep icon as is, in part also to make very explicit how it is meant to be used and what output one might expect rather than letting people randomly set weird things. It is also easier to consume from other languages (jsp).

Agreed with @carloslancha to close this :)