fix: support logoColor to shield icons.

[regseb]

This pull request fixes #6208 (and its duplicates: #7576, #8139, #8248) and finds a compromise for Simple Icons or custom Shields logos? #7684.

• Without logoColor query parameter: display custom Shields logo multiple colors;
• With logoColor query parameter: display Simple Icon by changing the color.

If you want:

• custom Shields logo multiple colors: https://img.shields.io/badge/shields-gitlab-blue?logo=gitlab
• Simple Icon with the default color (you must specify color): https://img.shields.io/badge/shields-gitlab-blue?logo=gitlab&logoColor=FC6D26
• Simple Icon with specific color: https://img.shields.io/badge/shields-gitlab-blue?logo=gitlab&logoColor=white

There is one exception for the Dependabot logo. The custom Shields logo version is monochrome and its color is customizable. With logoColor query parameter, it is the custom Shields logo that is displayed by changing its color.

[shields-ci]

	Messages
📖	✨ Thanks for your contribution to Shields, @regseb!
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS against b741d52

[chris48s]

I haven't been massively following all of the discussion in #7684 but I think I'd be fine with this as a solution: Seems like a reasonable way to reduce our support burden without taking on much additional complexity (which are really the 2 things I think we want to optimise for here).

The one issue is travis (because in simpleicons the slug is travis-ci).

[calebcartwright]

I haven't been massively following all of the discussion in #7684 but I think I'd be fine with this as a solution: Seems like a reasonable way to reduce our support burden without taking on much additional complexity (which are really the 2 things I think we want to optimise for here).

The one issue is travis (because in simpleicons the slug is travis-ci).

IMO this isn't directly related to #7684 and the fundamental question of whether users still want the custom logos. I view this as simply adding new functionality to Shields to give a minor assist to users that want to use the SI version instead of one of the corresponding ten custom logos we have, without having to base64 encode the SI icon themselves to be used with ?customLogo. I realize we could use this to decide to settle the matter since both camps could rely on us to do their logo work for them (and perhaps that's what you had in mind too), but I just wanted to also underscore that we could still drop the custom logos before or even after potentially making a change like this.

There's also the potential for a small slice of our existing users to be impacted by a change like this, if they happen to have a logoColor param in their badge urls. That's most likely a fairly small portion given that param is a functional no-op today, but it's still the case that the same badge url would produce a different badge and potentially require unhappy users to update their badge urls. Obviously that doesn't need to be treated as a hard blocker, but it's something to be cognizant of, and transparent about, if we were to move forward.

[chris48s]

IMO this isn't directly related to #7684

Yeah - sorry. I think @regseb 's description:

This pull request fixes #6208 (and its duplicates: #7576, #8139, #8248) and finds a compromise for #7684.

is better.

but I just wanted to also underscore that we could still drop the custom logos before or even after potentially making a change like this

Agreed. We could. Tbh though, as much as I'd like to bin them all and simplify things, I think #7684 tells us there isn't really a clear consensus for it.

---

Given the point we're starting from, It seems like merging some variation of this solution (accepting it probably needs an additional special-case for the travis logo) would:

• reduce support requests/confusion like in the linked issues
• not add much additional complexity to the code
• keep users who like the custom logos happy
• keep users who want to use logoColor= with all the logos happy
• maybe require some existing users to remove an unwanted/spurious logoColor param from some existing badge URLs
• not make anything else worse than it already is (I think?)

which seems like a pretty decent set of tradeoffs to me, all things considered.

[calebcartwright]

I think I largely agree, though do you think it'd be better to shift the discussion over to #6208 or to at least recap? Both so that those involved/subscribed to that convo can see this articulated position and for posterity so that thread will contain all relevant parts?

[regseb]

@chris48s @calebcartwright Is there anything I should do to advance this pull request?

[chris48s]

I think the thing that stalled this is not "the code is bad and it needs changes", its more "do we agree this idea is the right solution?". Personally, I think yes it is.

@calebcartwright - I'm going to invoke your suggestion about making decisions here and say: If you want to say "no its not", object in the next week.

@regseb - In terms of the code, the one thing we'd need to do is add a special case to fix travis. This one breaks because our custom logo is travis and in simpleicons the slug is travis-ci (?logo=travis&logoColor=red show no logo at all). If you want to do it straight away, fine but if you want to hold off on making that change then that's reasonable. I don't like to see people spend more time than needed on a PR that doesn't get merged.

@SnoozeThis wait 7 days

[SnoozeThis]

(https://snoozeth.is/bRTxKqHPz6o) I will wait until Tue, 29 Nov 2022 16:50:08 UTC and then add a comment.

[regseb]

I handled the Travis case.

[calebcartwright]

@calebcartwright - I'm going to invoke your suggestion about making decisions here and say: If you want to say "no its not", object in the next week.

Agreed. Hopefully already evident from the above thread, but I'm not personally going to object. I do think the code will need another pass through (and I can't promise I'll have a chance to do so particularly given the holiday this week in the US), and would also need some accompanying items e.g. solid documentation, both in readme docs and on the site, and a communication plan

[chris48s]

The code is fine so far and I'm happy to review any further commits.

Documentation is a good shout - thanks 👍

At the moment we don't mention anything about logoColor in https://github.com/badges/shields/blob/master/doc/logos.md so I am fine with leaving that as it is.

We do need to update

shields/frontend/components/usage.tsx

Lines 362 to 372 in c5476b4

	<QueryParam
	documentation={
	<span>
	Set the color of the logo (hex, rgb, rgba, hsl, hsla and css
	named colors supported). Supported for named logos but not for
	custom logos.
	</span>
	}
	key="logoColor"
	snippet="?logoColor=violet"
	/>

and

shields/frontend/pages/endpoint.tsx

Lines 210 to 214 in c5476b4

	<dt>logoColor</dt>
	<dd>
	Default: none. Same meaning as the query string. Can be overridden by
	the query string. Only works for named logos.
	</dd>

Also the comment in

shields/core/base-service/coalesce-badge.js

Lines 9 to 34 in c5476b4

	// Translate modern badge data to the legacy schema understood by the badge
	// maker. Allow the user to override the label, color, logo, etc. through the
	// query string. Provide support for most badge options via `serviceData` so
	// the Endpoint badge can specify logos and colors, though allow that the
	// user's logo or color to take precedence. A notable exception is the case of
	// errors. When the service specifies that an error has occurred, the user's
	// requested color does not override the error color.
	//
	// Logos are resolved in this manner:
	//
	// 1. When `?logo=` contains the name of one of the Shields logos, or contains
	// base64-encoded SVG, that logo is used. In the case of a named logo, when
	// a `&logoColor=` is specified, that color is used. Otherwise the default
	// color is used. `logoColor` will not be applied to a custom
	// (base64-encoded) logo; if a custom color is desired the logo should be
	// recolored prior to making the request. The appearance of the logo can be
	// customized using `logoWidth`, and in the case of the popout badge,
	// `logoPosition`. When `?logo=` is specified, any logo-related parameters
	// specified dynamically by the service, or by default in the service, are
	// ignored.
	// 2. The second precedence is the dynamic logo returned by a service. This is
	// used only by the Endpoint badge. The `logoColor` can be overridden by the
	// query string.
	// 3. In the case of the `social` style only, the last precedence is the
	// service's default logo. The `logoColor` can be overridden by the query
	// string.

could do with an update.

In terms of communicating the change, I think posting a comment in the various linked issues when we merge this and close them will be fine. #7684 is already pinned and it can stay pinned. I've not drafted a post yet, but I'm happy to cover that.

[regseb]

I have updated the documentation. English is not my first language, so I'm open to any changes.

[chris48s]

Going to merge this with a 1 word tweak to the docs. I'll update the issue threads once I've deployed

[SnoozeThis]

Resolved.