Closed winkerVSbecks closed 3 years ago
I'm suggesting padding: 4px 10px && line-height: 20px to yield a 32px tall tag element.
padding: 4px 10px
line-height: 20px
On Thu, Dec 3, 2020 at 6:53 PM Varun Vachhar notifications@github.com wrote:
@winkerVSbecks commented on this pull request. In src/components/tag/TagItem.tsx https://github.com/storybookjs/design-system/pull/235#discussion_r535732258 : @@ -21,7 +21,7 @@ export const TagItem = styled.div.attrs(({ isLoading, children }) display: inline-block; background: ${background.app}; border-radius: ${spacing.borderRadius.small}px; padding: 5px 10px; padding: 4px 10px 2px 10px; Do you mean padding: 5px 10px cause 4px makes the height 30px. Also, I would recommend padding: 6px 10px 4px 10px; for optical correction. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/storybookjs/design-system/pull/235#discussion_r535732258, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACAJWOW3GKUB43PSM3DAHTSTAQF5ANCNFSM4UMPEZZQ .
In src/components/tag/TagItem.tsx https://github.com/storybookjs/design-system/pull/235#discussion_r535732258 :
@@ -21,7 +21,7 @@ export const TagItem = styled.div.attrs(({ isLoading, children }) display: inline-block; background: ${background.app}; border-radius: ${spacing.borderRadius.small}px; padding: 5px 10px; padding: 4px 10px 2px 10px;
@@ -21,7 +21,7 @@ export const TagItem = styled.div.attrs(({ isLoading, children }) display: inline-block; background: ${background.app}; border-radius: ${spacing.borderRadius.small}px;
Do you mean padding: 5px 10px cause 4px makes the height 30px. Also, I would recommend padding: 6px 10px 4px 10px; for optical correction.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/storybookjs/design-system/pull/235#discussion_r535732258, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACAJWOW3GKUB43PSM3DAHTSTAQF5ANCNFSM4UMPEZZQ .
@domyen so, padding: 4px 10px && line-height: 20px yields 30px height. That's why I recommended those other options.
:rocket: PR was released in v5.4.3 :rocket:
v5.4.3
I'm suggesting
padding: 4px 10px
&&line-height: 20px
to yield a 32px tall tag element.On Thu, Dec 3, 2020 at 6:53 PM Varun Vachhar notifications@github.com wrote: