-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add vertical padding to "Tag" component for large style #4099
Add vertical padding to "Tag" component for large style #4099
Conversation
Thanks for your interest in palantir/blueprint, @Eusbolh! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
@@ -70,7 +71,7 @@ $tag-round-adjustment: 2px !default; | |||
line-height: $tag-line-height-large; | |||
min-height: $tag-height-large; | |||
min-width: $tag-height-large; | |||
padding: 0 $tag-padding-large; | |||
padding: $tag-padding-large-top $tag-padding-large; |
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.
can you just use ($tag-padding-large / 2)
here instead? I'd prefer that over adding a new Sass variable
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.
see above
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.
thanks for the PR @Eusbolh!
hope you don't mind if i push through these changes... I'd like to cut a release soon
Fixes #4037
Checklist
Changes proposed in this pull request:
Add vertical padding to "Tag" component for large style
Reviewers should focus on:
Ratio between horizontal and vertical padding value should be decided. For default style, the ratio between horizontal and vertical padding is 3 to 2. However, I followed 2 to 1 ratio for large style which is the current ratio for single line large tags.
Screenshot
There is no visual change for single line content.
For multiline content, the old version is:
The updated version for multiline content is: