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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove logoWidth param #10878

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

jNullj
Copy link
Member

@jNullj jNullj commented Feb 8, 2025

Fixes #10285

notice - i moved the auto logo size logic into badge-maker as it makes sense here, my logic was that its related to badge render, let me know if that was the right move.

Copy link
Contributor

github-actions bot commented Feb 8, 2025

Messages
📖 ✨ Thanks for your contribution to Shields, @jNullj!

Generated by 🚫 dangerJS against 1ea62b5

@@ -4,6 +4,7 @@ const { normalizeColor, toSvgColor } = require('./color')
const badgeRenderers = require('./badge-renderers')
const { stripXmlWhitespace } = require('./xml')
const { DEFAULT_LOGO_HEIGHT } = require('./constants')
const { getIconSize } = require('./svg-helpers.js')
Copy link
Member Author

Choose a reason for hiding this comment

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

svg-helpers.js resides at /lib and not inside the lib of badge-maker /badge-maker/lib
😵

@LitoMore
Copy link
Contributor

There are some projects that use the logoWidth for adjusting the icon size, like https://github.com/xojs/xo?tab=readme-ov-file#badge.

Should we use a new parameter to allow users to customize the icon size?

@jNullj
Copy link
Member Author

jNullj commented Feb 15, 2025

There are some projects that use the logoWidth for adjusting the icon size, like https://github.com/xojs/xo?tab=readme-ov-file#badge.

Should we use a new parameter to allow users to customize the icon size?

Wouldn't this example still work as it uses logoSize=auto?
this
https://shields.io/badge/code_style-5ed9c7?logo=xo&labelColor=gray&logoSize=auto&logoWidth=20
before
would render as this in backend
https://shields.io/badge/code_style-5ed9c7?logo=xo&labelColor=gray&logoSize=auto&logoSize=auto
after

the logo is a bit bigger but doesnt seem to change drastically.

But i do get that this might break other badges using logos without logoSize=auto

@chris48s
Copy link
Member

The purpose of deleting it is to remove it.

logoWidth has been an undocumented param since at least 2023, which is before logoSize=auto was added. If projects are using it with logoSize=auto, they're relying on undocumented internals so we can consider this non-breaking.

If people are using it without, its probably a hangover from the popout style which was removed in 2019 or just inserting strange spacing.

Either way, its time for this to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove logoWidth param
3 participants