Skip to content

fix: keep logo & text alignments consistent#11284

Open
LitoMore wants to merge 7 commits into
badges:masterfrom
LitoMore:align-items
Open

fix: keep logo & text alignments consistent#11284
LitoMore wants to merge 7 commits into
badges:masterfrom
LitoMore:align-items

Conversation

@LitoMore

@LitoMore LitoMore commented Aug 12, 2025

Copy link
Copy Markdown
Member

Some of our current badges have some inconsistent spacing. Especially the logo-only badge. I've fixed/changed them to be consistent.

I've attached the 5x zoom level preview image with measurements. As you can see, some badges don't have consistent left-right padding. But if the numbers differ by only 1, it can be ignored.

I didn't change the style of the social badge since its design is a bit special. I think the padding is fine. Please let me know if you want the social badge to be changed as well.

The measurement tool I use: PixelSnap 2.

img.shields.io

![](https://img.shields.io/badge/M-M-000) ![](https://img.shields.io/badge/-M-000) ![](https://img.shields.io/badge/M-M-000?logo=npm) ![](https://img.shields.io/badge/-M-000?logo=npm) ![](https://img.shields.io/badge/--000?logo=npm)\
![](https://img.shields.io/badge/M-M-000?style=flat-square) ![](https://img.shields.io/badge/-M-000?style=flat-square) ![](https://img.shields.io/badge/M-M-000?logo=npm&style=flat-square) ![](https://img.shields.io/badge/-M-000?logo=npm&style=flat-square) ![](https://img.shields.io/badge/--000?logo=npm&style=flat-square)\
![](https://img.shields.io/badge/M-M-000?style=plastic) ![](https://img.shields.io/badge/-M-000?style=plastic) ![](https://img.shields.io/badge/M-M-000?logo=npm&style=plastic) ![](https://img.shields.io/badge/-M-000?logo=npm&style=plastic) ![](https://img.shields.io/badge/--000?logo=npm&style=plastic)\
![](https://img.shields.io/badge/M-M-000?style=for-the-badge) ![](https://img.shields.io/badge/-M-000?style=for-the-badge) ![](https://img.shields.io/badge/M-M-000?logo=npm&style=for-the-badge) ![](https://img.shields.io/badge/-M-000?logo=npm&style=for-the-badge) ![](https://img.shields.io/badge/--000?logo=npm&style=for-the-badge)\
![](https://img.shields.io/badge/M-M-000?style=social) ![](https://img.shields.io/badge/-M-000?style=social) ![](https://img.shields.io/badge/M-M-000?logo=npm&style=social) ![](https://img.shields.io/badge/-M-000?logo=npm&style=social) ![](https://img.shields.io/badge/--000?logo=npm&style=social)





pr-11284-badges-shields.fly.dev

![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000) ![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?logo=npm) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?logo=npm) ![](https://pr-11284-badges-shields.fly.dev/badge/--000?logo=npm)\
![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?style=flat-square) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?style=flat-square) ![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?logo=npm&style=flat-square) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?logo=npm&style=flat-square) ![](https://pr-11284-badges-shields.fly.dev/badge/--000?logo=npm&style=flat-square)\
![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?style=plastic) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?style=plastic) ![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?logo=npm&style=plastic) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?logo=npm&style=plastic) ![](https://pr-11284-badges-shields.fly.dev/badge/--000?logo=npm&style=plastic)\
![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?style=for-the-badge) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?style=for-the-badge) ![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?logo=npm&style=for-the-badge) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?logo=npm&style=for-the-badge) ![](https://pr-11284-badges-shields.fly.dev/badge/--000?logo=npm&style=for-the-badge)\
![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?style=social) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?style=social) ![](https://pr-11284-badges-shields.fly.dev/badge/M-M-000?logo=npm&style=social) ![](https://pr-11284-badges-shields.fly.dev/badge/-M-000?logo=npm&style=social) ![](https://pr-11284-badges-shields.fly.dev/badge/--000?logo=npm&style=social)





localhost:8080

GitHub cannot display localhost assets since they have Camo for caching resources. But these links might be helpful for you to review.

Markdown

![](http://localhost:8080/badge/M-M-000) ![](http://localhost:8080/badge/-M-000) ![](http://localhost:8080/badge/M-M-000?logo=npm) ![](http://localhost:8080/badge/-M-000?logo=npm) ![](http://localhost:8080/badge/--000?logo=npm)\
![](http://localhost:8080/badge/M-M-000?style=flat-square) ![](http://localhost:8080/badge/-M-000?style=flat-square) ![](http://localhost:8080/badge/M-M-000?logo=npm&style=flat-square) ![](http://localhost:8080/badge/-M-000?logo=npm&style=flat-square) ![](http://localhost:8080/badge/--000?logo=npm&style=flat-square)\
![](http://localhost:8080/badge/M-M-000?style=plastic) ![](http://localhost:8080/badge/-M-000?style=plastic) ![](http://localhost:8080/badge/M-M-000?logo=npm&style=plastic) ![](http://localhost:8080/badge/-M-000?logo=npm&style=plastic) ![](http://localhost:8080/badge/--000?logo=npm&style=plastic)\
![](http://localhost:8080/badge/M-M-000?style=for-the-badge) ![](http://localhost:8080/badge/-M-000?style=for-the-badge) ![](http://localhost:8080/badge/M-M-000?logo=npm&style=for-the-badge) ![](http://localhost:8080/badge/-M-000?logo=npm&style=for-the-badge) ![](http://localhost:8080/badge/--000?logo=npm&style=for-the-badge)\
![](http://localhost:8080/badge/M-M-000?style=social) ![](http://localhost:8080/badge/-M-000?style=social) ![](http://localhost:8080/badge/M-M-000?logo=npm&style=social) ![](http://localhost:8080/badge/-M-000?logo=npm&style=social) ![](http://localhost:8080/badge/--000?logo=npm&style=social)

HTML

<img src="http://localhost:8080/badge/M-M-000" />
<img src="http://localhost:8080/badge/-M-000" />
<img src="http://localhost:8080/badge/M-M-000?logo=npm" />
<img src="http://localhost:8080/badge/-M-000?logo=npm" />
<img src="http://localhost:8080/badge/--000?logo=npm" />
<br />
<img src="http://localhost:8080/badge/M-M-000?style=flat-square" />
<img src="http://localhost:8080/badge/-M-000?style=flat-square" />
<img src="http://localhost:8080/badge/M-M-000?logo=npm&style=flat-square" />
<img src="http://localhost:8080/badge/-M-000?logo=npm&style=flat-square" />
<img src="http://localhost:8080/badge/--000?logo=npm&style=flat-square" />
<br />
<img src="http://localhost:8080/badge/M-M-000?style=plastic" />
<img src="http://localhost:8080/badge/-M-000?style=plastic" />
<img src="http://localhost:8080/badge/M-M-000?logo=npm&style=plastic" />
<img src="http://localhost:8080/badge/-M-000?logo=npm&style=plastic" />
<img src="http://localhost:8080/badge/--000?logo=npm&style=plastic" />
<br />
<img src="http://localhost:8080/badge/M-M-000?style=for-the-badge" />
<img src="http://localhost:8080/badge/-M-000?style=for-the-badge" />
<img src="http://localhost:8080/badge/M-M-000?logo=npm&style=for-the-badge" />
<img src="http://localhost:8080/badge/-M-000?logo=npm&style=for-the-badge" />
<img src="http://localhost:8080/badge/--000?logo=npm&style=for-the-badge"/>
<br />
<img src="http://localhost:8080/badge/M-M-000?style=social" />
<img src="http://localhost:8080/badge/-M-000?style=social" />
<img src="http://localhost:8080/badge/M-M-000?logo=npm&style=social" />
<img src="http://localhost:8080/badge/-M-000?logo=npm&style=social" />
<img src="http://localhost:8080/badge/--000?logo=npm&style=social" />

@github-actions

github-actions Bot commented Aug 12, 2025

Copy link
Copy Markdown
Contributor
Messages
📖 ✨ Thanks for your contribution to Shields, @LitoMore!

Generated by 🚫 dangerJS against 4940ae7

@jNullj jNullj added the npm-package Badge-maker NPM package label Aug 12, 2025
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Updated review app: https://pr-11284-badges-shields.fly.dev

@jNullj jNullj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's a bit hard to see that changes, i might pick some pixel art editor to try comparing them.
But i do have some feedback about the code, see my changes suggestion.

Regarding the fix, Could you present what exactly is inconsistent here before the change and how this solves the issue?

Comment thread badge-maker/lib/badge-renderers.js Outdated
Comment thread badge-maker/lib/badge-renderers.js
@LitoMore

LitoMore commented Aug 12, 2025

Copy link
Copy Markdown
Member Author

I've attached two 5x zoom level images with measurements to the PR description. It appears that the logo-only in-for-the-badge style still has an alignment issue. I will look into it.

@LitoMore LitoMore requested a review from jNullj August 12, 2025 22:00
@LitoMore

LitoMore commented Aug 12, 2025

Copy link
Copy Markdown
Member Author

I've fixed the style of the for-the-badge. I also updated its 5x preview image. The 1x preview may require your deployment to update.

@jNullj jNullj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the late response.
This review I set some time to try and understand the existing and proposed logic here

Comment thread badge-maker/lib/badge-renderers.js Outdated
messageRectWidth =
2 * TEXT_MARGIN + logoWidth + gutter + messageTextWidth - 1
if (noText) {
messageRectWidth += 5 // Compensate for the extra padding

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tried to warp my head around this one.

If you look closely the original logic seems to say, if we have a logo add the logo width and a separator (gutter) to separate logo and message text, otherwise, only use message text. ofc we add text margin in both sides as well.

You seem to increase our LOGO_MARGIN then add some compensation.
I wonder if we shouldn't add this logic to the gutter to begin with at line 808
const gutter = noText ? LOGO_TEXT_GUTTER - LOGO_MARGIN : LOGO_TEXT_GUTTER

But as i was thinking that i noticed, we get a negative gutter, which hardly makes sense as it should, if im not wrong, be never negative, as it represent a size to space out elements.

in that case, maybe presenting abs would fix our problems and remove the need for the -1 and additional compensation. Even more so that the mentioned line to set gutter already takes into account the noText logic!

please let me know what you think.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for looking so deep. I've made a commit to make these logics cleaner.

You will notice that now some values have base value now:

let labelTextMinX = TEXT_MARGIN
let labelRectWidth = 2 * TEXT_MARGIN
let messageTextMinX = TEXT_MARGIN
let messageRectWidth = 2 * TEXT_MARGIN + messageTextWidth

I also removed the LOGO_MARGIN since its has the same value as TEXT_MARGIN. And the LOGO_MARGIN meaningless because we should actual use TEXT_MARGIN to caculate its size.

I now have a clear idea of ​​how to calculate the size of the badge, and I should be able to optimize the calculation logic of other badges in the same way in the future.

Looking forward to your another review. ✌️

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jNullj would be great if you could take another look at this one :)

@LitoMore LitoMore requested a review from jNullj August 25, 2025 09:48
const gutter = noText ? LOGO_TEXT_GUTTER - LOGO_MARGIN : LOGO_TEXT_GUTTER
let logoMinX, labelTextMinX
let labelTextMinX = TEXT_MARGIN
let labelRectWidth = 2 * TEXT_MARGIN

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixes issue where we add empty space after the badge when there is no label/logo-in-label

Suggested change
let labelRectWidth = 2 * TEXT_MARGIN
let labelRectWidth = needsLabelRect ? 2 * TEXT_MARGIN : undefined

labelTextMinX = logoMinX + logoWidth + gutter
} else {
labelTextMinX = TEXT_MARGIN
if (labelColor) labelRectWidth += logoWidth

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (labelColor) labelRectWidth += logoWidth
if (labelColor || needsLabelRect) labelRectWidth += logoWidth

}
messageTextMinX = labelRectWidth + TEXT_MARGIN
messageRectWidth = 2 * TEXT_MARGIN + messageTextWidth
if (hasLabel) labelRectWidth += labelTextMinX + labelTextWidth

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (hasLabel) labelRectWidth += labelTextMinX + labelTextWidth
if (hasLabel) labelRectWidth += labelTextWidth

@jNullj jNullj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I added a few fixes, there was issues in forTheBadge style with label rect growing outside rendered area, creating empty space.
That also added space to the label area itself.
You can easily apply the fixes if you don't find anything i missed there.

as a side note, i used your html examples but added another case, for labelColor, as it exists in the edited code.
http://localhost:8080/badge/--000?logo=npm&style=for-the-badge&labelColor=blue

@jNullj

jNullj commented Jan 10, 2026

Copy link
Copy Markdown
Member

@LitoMore hey, just a friendly bump. was going over old mail and got this as an open ticket.

@LitoMore

Copy link
Copy Markdown
Member Author

I will take a look this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

npm-package Badge-maker NPM package

Development

Successfully merging this pull request may close these issues.

3 participants