Add support for simple-icons, colored icons with ?logoColor #1810

RedSparr0w · 2018-07-25T22:19:04Z

Support more icons without the overhead of reviewing them ourselves.
I also added support for colored icons.

It will use our icon > simple-icon if available.

Examples:

params	badge
`?logo=github&style=social`
`?logo=github&logoColor=blue`
`?logo=react`
`?logo=microsoft%20excel`
`?logo=microsoft-excel`
`?logo=microsoft%20excel&logoColor=blue`
`?logo=apple`
NOTE:	Some badges wont look good with `?logoColor`
`?logo=npm`
`?logo=npm&logoColor=blue`

the npm & bitcoin logos (maybe others also) will be completely filled in when you change the logo color due to the background being solid

TODO:

Add logoColor information
Add simple-icon information
Add snapshot tests

shields-ci · 2018-07-25T22:20:23Z

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

Generated by 🚫 dangerJS

paulmelnikow

This looks really good!

We've just gotten a list of named logos into the frontend in #1794. Should we add a link to a versioned list of theirs? Or just display the complete list, including simple-icons?

It would also be good to get a static badge test that exercises this feature.

paulmelnikow · 2018-07-26T00:27:29Z

lib/badge-data.js

    return color;
+  } else {
+    return undefined;


Is this change related to simpleicons?

Kind of, I wasn't sure whether to make a new function for the color or just modify this function as it seemed to still fit quite well, I think it makes more sense to only return a valid color rather than just assume anything that's not hex is a valid color/colorscheme, and the current test were expecting any non valid color to return undefined.

Also there wasn't any test for this function so i've added these too, which are very similar to setBadgeColor().

The main differences between setBadgeColor() and makeColor() are:

setBadgeColor():

will set the either badgeData.colorB or badgeData.colorscheme(which i think may be redundant now, but will look at that in another PR) to the desired color.

makeColor():

returns a valid color.

paulmelnikow · 2018-07-26T00:30:26Z

lib/badge-data.js

 function makeLogo(defaultNamedLogo, overrides) {
  if (overrides.logo === undefined){
-    return logos[defaultNamedLogo];
+    return logos[defaultNamedLogo] || getSimpleIcon(defaultNamedLogo, overrides.logoColor);


For parity, is it possible to support logoColor in our own logos?

Should be able to, Do you know if there is a reason that we load the logos into memory on launch?
(in load-logos.js)
As that is the main reason i haven't tried yet, because it loads them and keeps them as base64.

Seems to be okay 👍

It’s a performance optimization, to load and encode them once instead of per request. We could cache the unencoded logos instead, or keep both in memory, as a small optimization for the cases where logoColor is not specified.

Ah, yeah that makes sense, should I also do the same for the simple-icons and store them as their own default colors?

Sure, that couldn't hurt!

RedSparr0w · 2018-07-26T23:24:06Z

use the base64 logos from memory (ours & simple-icons),
Updated descriptions for ?logoColor= & ?logo= (see image in main comment)
Simple-icons by default use their own color instead of whitesmoke:

gitlab: E24329
react: 61DAFB
etc..

Updated main comment to show some examples.

If i could get another review please. @paulmelnikow
Note: Unsure what the best way to show the logo options was, let me know if you think it should be changed back/adjusted at all.
😄

update to 1.7.1

paulmelnikow · 2018-07-27T20:54:54Z

I'd prefer to keep the list in the frontend so it stays in sync with the deployed code. Maybe the simplest thing would be to update lib/export-supported-features-cli.js so it includes both the simpleicons and internal icons.

What do you think about my suggestion to add a test that exercises the simple logo feature on a static badge? It could be a snapshot test. I don't want us to break this stuff by accident.

Otherwise this looks good to me!

RedSparr0w · 2018-07-28T04:43:03Z

I'd prefer to keep the list in the frontend so it stays in sync with the deployed code. Maybe the simplest thing would be to update lib/export-supported-features-cli.js so it includes both the simpleicons and internal icons.

I think it may end up making it look a little too busy if we included the simple-icons in the list as its already quite large just with the icons we have, what about if we just have a link to simple icons at the end of our list?

What do you think about my suggestion to add a test that exercises the simple logo feature on a static badge? It could be a snapshot test. I don't want us to break this stuff by accident.

No worries, sounds like a good idea to me 👍

paulmelnikow · 2018-07-28T17:51:28Z

what about if we just have a link to simple icons at the end of our list?

Works for me!

…our logos" This reverts commit 5e99d5f.

RedSparr0w · 2018-07-31T22:44:11Z

Added snapshot test for:

shields GitHub logo
shields GitHub logo w/custom color
simple-icons javascript logo
simple-icons javascript logo w/custom color

Should be ready for another review now @paulmelnikow
🤞

Edit:

w‮?tsil‭hat ‮ ruo ‭abo‮fo ‭ut if‮dne ‭ ‮eht t‭we‮a sno‭ j‮ci ‭ust h‮elpm‭ave‮is‭ ‮ ot‭a‮ kni‭ l

‭Also ‮.s‭up‮koo‭da‮l ti ‭te‮woh ‭d w‮fo ‭ith t‮elp‭hi‮max‭s ch‮e ht‭ang‮i‭e,‮w tn‭ an‮em‭d ‮m‭adde‮o‭d sc‮c ‭reen‮pot‭sho‮ ni ‭t

paulmelnikow · 2018-08-01T18:31:39Z

?logo=microsoft%20excel

To make the URLs friendlier, what do you think about converting the spaces to dashes, similar to the way you've lowercased them?

RedSparr0w · 2018-08-01T20:59:15Z

Sounds like a great idea, will definitely look cleaner.

RedSparr0w · 2018-08-01T21:06:46Z

Now supports using a dash or space ?logo=microsoft%20excel or ?logo=microsoft-excel

shields-deployment · 2018-08-01T21:07:26Z

This pull request was merged to master branch. Now this change is waiting for deployment.
Deploys usually happen every few weeks. After deployment changes are copied to gh-pages branch.

This badge displays deployment status:

JeanMertz · 2018-09-10T21:37:55Z

Pretty sweet that all those icons are now available 👍. One question; is it correct that not all icon colours can be manipulated for every possible badge? For example, the Linux or Arch Linux icons don't seem to change color when using a URL like this:

https://img.shields.io/circleci/project/github/rusty-rockets/sm/master.svg?style=flat-square&logo=linux&logoColor=white

While using the static image does work:

https://img.shields.io/badge/style-flat--square-green.svg?style=flat-square&logo=linux&logoColor=white

chris48s · 2018-09-11T19:25:13Z

@JeanMertz - it isn't intentional. This issue has already been reported and fixed in #2059 . It hasn't been deployed yet, but it will be fixed when we next deploy. Cheers

RedSparr0w added 4 commits July 26, 2018 10:07

add simple-icons

4c72b8d

handle undefined

7a219a3

support logoColor param

4c35721

our icon > simple-icon

996a524

RedSparr0w added 3 commits July 26, 2018 10:57

dont crash ✅

ff6bc0d

return false → undefined

84b677f

update test

f79b0ce

RedSparr0w force-pushed the simple-icons branch from 42e8c38 to 414ec9b Compare July 25, 2018 23:22

add test

961b50a

RedSparr0w force-pushed the simple-icons branch from 414ec9b to 961b50a Compare July 25, 2018 23:26

paulmelnikow added the core Server, BaseService, GitHub auth label Jul 25, 2018

paulmelnikow reviewed Jul 26, 2018

View reviewed changes

support logoColor on our logos

6af555c

RedSparr0w force-pushed the simple-icons branch from 3ce9bef to 6af555c Compare July 26, 2018 03:15

RedSparr0w changed the title ~~Add support for simple-icons~~ Add support for simple-icons, colored icons with ?logoColor Jul 26, 2018

RedSparr0w force-pushed the simple-icons branch from 90d2723 to 02dbc26 Compare July 26, 2018 21:50

cache as base64, pre-load-simple-icons, logo-helper

c8a9157

RedSparr0w force-pushed the simple-icons branch from 4f25820 to c8a9157 Compare July 26, 2018 22:40

RedSparr0w added 2 commits July 27, 2018 10:45

add ?logoColor information

1fddca8

and simple-icons reference, link to github master branch for our logos

5e99d5f

update simple-icons

665668a

update to 1.7.1

RedSparr0w and others added 4 commits July 30, 2018 15:17

Merge branch 'master' into simple-icons

2b1b94c

Revert "and simple-icons reference, link to github master branch for …

f6acebb

…our logos" This reverts commit 5e99d5f.

add link to simple-icons

c1ac752

Add snapshot test

d3bc226

paulmelnikow approved these changes Aug 1, 2018

View reviewed changes

support dash in place of space for logo name

87bc01a

paulmelnikow approved these changes Aug 1, 2018

View reviewed changes

RedSparr0w merged commit 317e19e into badges:master Aug 1, 2018

paulmelnikow mentioned this pull request Aug 11, 2018

dark logos on dark backgrounds: Provide light version #1535

Closed

paulmelnikow mentioned this pull request Nov 29, 2018

Automatically show light or dark logos depending on left color #2431

Closed

RedSparr0w deleted the simple-icons branch January 21, 2019 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for simple-icons, colored icons with ?logoColor #1810

Add support for simple-icons, colored icons with ?logoColor #1810

RedSparr0w commented Jul 25, 2018 •

edited

shields-ci commented Jul 25, 2018 •

edited

paulmelnikow left a comment

paulmelnikow Jul 26, 2018

RedSparr0w Jul 26, 2018

paulmelnikow Jul 26, 2018

RedSparr0w Jul 26, 2018

RedSparr0w Jul 26, 2018

paulmelnikow Jul 26, 2018

RedSparr0w Jul 26, 2018

paulmelnikow Jul 26, 2018

RedSparr0w commented Jul 26, 2018

paulmelnikow commented Jul 27, 2018

RedSparr0w commented Jul 28, 2018

paulmelnikow commented Jul 28, 2018

RedSparr0w commented Jul 31, 2018 •

edited

paulmelnikow commented Aug 1, 2018

RedSparr0w commented Aug 1, 2018

RedSparr0w commented Aug 1, 2018

shields-deployment bot commented Aug 1, 2018

JeanMertz commented Sep 10, 2018

chris48s commented Sep 11, 2018

Add support for simple-icons, colored icons with ?logoColor #1810

Add support for simple-icons, colored icons with ?logoColor #1810

Conversation

RedSparr0w commented Jul 25, 2018 • edited

Examples:

TODO:

shields-ci commented Jul 25, 2018 • edited

paulmelnikow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

setBadgeColor():

makeColor():

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RedSparr0w commented Jul 26, 2018

paulmelnikow commented Jul 27, 2018

RedSparr0w commented Jul 28, 2018

paulmelnikow commented Jul 28, 2018

RedSparr0w commented Jul 31, 2018 • edited

paulmelnikow commented Aug 1, 2018

RedSparr0w commented Aug 1, 2018

RedSparr0w commented Aug 1, 2018

shields-deployment bot commented Aug 1, 2018

JeanMertz commented Sep 10, 2018

chris48s commented Sep 11, 2018

RedSparr0w commented Jul 25, 2018 •

edited

shields-ci commented Jul 25, 2018 •

edited

`setBadgeColor()`:

`makeColor()`:

RedSparr0w commented Jul 31, 2018 •

edited