Skip to content
This repository has been archived by the owner on Jan 16, 2018. It is now read-only.

Update markups and fix #234

Closed

Conversation

matuzalemsteles
Copy link
Collaborator

@matuzalemsteles matuzalemsteles commented Dec 26, 2017

hey @carloslancha, ClayUserCard customize the label of ClaySticker mean that we are duplicating the markup, but it is obliged to use the sticker-overlay class in the label to be positioned correctly, we can add as default in ClaySticker but I do not know what effects it can cause it being used on other components. What do you think about this?

This pr was tested with #233.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 83.144% when pulling ce0e803 on matuzalemsteles:update-markup into 7e67869 on metal:master.

@carloslancha
Copy link
Collaborator

Just started reviewing :)

:octocat: Sent from GH.

Copy link
Collaborator

@carloslancha carloslancha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since class .sticker has position: relative adding a span inside with .sticker-overlay should not be a problem. Go for it, add it as default wrapper on all stickers, remove it from ClayUserCard and update the ussage by passing imageSrc to it. Nice catch @matuzalemsteles !

Btw... stop working and enjoy your holidays!! 😂

@@ -72,7 +72,21 @@
{@param? imageAlt: string}
{@param? imageSrc: string}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact this two params should be mandatory for this .image template, so we can get rid of let and just place the variables in the image, or at least remove the conditions.

@carloslancha
Copy link
Collaborator

Resent #237

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants