Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce size of Label--inline #1908

Merged
merged 3 commits into from
Feb 1, 2022
Merged

Reduce size of Label--inline #1908

merged 3 commits into from
Feb 1, 2022

Conversation

simurai
Copy link
Contributor

@simurai simurai commented Feb 1, 2022

What are you trying to accomplish?

This closes https://github.com/github/primer/issues/664 by reducing the size of Label--inline.

Before After
Screen Shot 2022-02-01 at 12 16 45 Screen Shot 2022-02-01 at 12 17 13

Markup used for the screenshots above:

<p class="col-4">
  Lorem Ipsum is simply <span class="Label Label--inline">dummy text</span> of the <span class="Label Label--inline">dummy text</span> industry.
  <span class="IssueLabel Label--inline color-bg-accent-emphasis color-fg-on-emphasis mr-1">Lorem Ipsum</span> has been <span class="Label Label--inline">dummy text</span> the industry's standard dummy text.
</p>

What approach did you choose and why?

Reducing the font-size and padding makes sure the labels don't touch each other when wrapped onto multiple lines. The font-size: 85%; also matches the <code>'s font size.

-  padding: 0.1667em 0.5em;
-  font-size: 0.9em;
+  padding: 0.12em 0.5em;
+  font-size: 85%;

What should reviewers focus on?

Since Label--inline isn't really meant to be used with IssueLabel, I was contemplating adding a separate IssueLabel--inline variation. But there was some talk to replace IssueLabel with a variation like Label Label--filled. So for now using IssueLabel Label--inline might still be fine.

Are additional changes needed?

  • No, this PR should be ok to ship as is. 馃殺

@simurai simurai requested a review from a team as a code owner February 1, 2022 03:32
@changeset-bot
Copy link

changeset-bot bot commented Feb 1, 2022

馃 Changeset detected

Latest commit: cb9a7b7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/css Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jonrohan jonrohan enabled auto-merge (squash) February 1, 2022 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants