-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tagUpdate #174
tagUpdate #174
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple small comments.
@@ -26,33 +26,33 @@ cbp-tag { | |||
cursor: default; | |||
padding: 0 var(--cbp-space-3x); | |||
text-align: center; | |||
font-weight: var(--cbp-font-weight-medium); | |||
font-weight: var(--cbp-font-size-subhead); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is incorrect. This variable is a font size, which needs to be specified properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old font-weight is still needed though.
@@ -44,7 +44,7 @@ const Template = ({ label, color, withIcon, context, sx }) => { | |||
${context && context != 'light-inverts' ? `context=${context}` : ''} | |||
${sx ? `sx=${JSON.stringify(sx)}` : ''} | |||
> | |||
${Icon ? `<cbp-icon name="${Icon}" sx='{"margin-right":"var(--cbp-space-2x)"}'></cbp-icon>` : ''} | |||
${Icon ? `<cbp-icon name="${Icon}" size="0.75rem"sx='{"margin-right":"var(--cbp-space-1x)"}'></cbp-icon>` : ''} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the icon is the same size as the font size, you shouldn't need this. The default icon size is 1em, which is relative to the surrounding context. Fix the font size (above) and this fixes itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now. Confirmed that the icon is the same size as the text.
updates to tag from design review