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

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

Closed
paulmelnikow opened this issue Nov 29, 2018 · 21 comments · Fixed by #2833
Closed

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

paulmelnikow opened this issue Nov 29, 2018 · 21 comments · Fixed by #2833
Labels
core Server, BaseService, GitHub auth

Comments

@paulmelnikow
Copy link
Member

As was variously suggested in this thread…

#1535 (comment)
#1535 (comment)
#1535 (comment)

it would be nice to support light and dark variants of logos. Perhaps these could be chosen automatically based on the template's left color.

The light and dark versions could be curated or determined programmatically.

It's already possible to specify an explicit color thanks to @RedSparr0w's work in #1810; this is a refinement that would make it automatic.

@RedSparr0w
Copy link
Member

RedSparr0w commented Jan 14, 2019

How would we go about current logos with colors/multi colors?
Will we just change them all to be automatic, or should we have a list of excluded logos?

Also would we want to add an option like ?logoColor=default or ?logoColor=brand to allow for the default simple icons colors?

@paulmelnikow
Copy link
Member Author

Since we only support two colors for the left side, whatever the social template uses (white?) and gray, and are likely to want to customize some of these anyway, it seems simplest to just explicitly override the ones that need overriding.

We could build a diagnostic page that makes it easy to see all the logo colors. That would help with diagnosing this.

Also would we want to add an option like ?logoColor=default or ?logoColor=brand to allow for the default simple icons colors?

At the moment it doesn't seem necessary. I think we should pick good default colors for the two background colors we support in the templates. If someone is customizing the background color, they are way out on a limb, and can inspect and specify the brand color easily enough.

By the way I started refactoring the logo code as a sub-task of #2473 but it depends on the color code so I had to refactor that first. After that's merged (#2742) I'll take a pass at the existing logo functionality. I won't implement this, but I'll keep it in mind.

@chris48s
Copy link
Member

So my thought was that we want to avoid a situation where we've got light logo on light background or dark logo on dark background, preferring the brand colour if possible. I'd also like to avoid having to individually store our preferred colour for each icon in the simple-icons set (they hold over 500 icons so far). We can work out if a colour is 'light' or 'dark' using something like https://awik.io/determine-color-bright-dark-using-javascript/ (or there might be a npm package for it). Then my thought was we do something like (pseudo-code):

if we're using our own icon, assume its multi-coloured and just use it as-is

if we're using simpleicons {
    if isLight(backgroundColor) and isLight(brandColor) {
        iconColor = black
    }
    if isDark(backgroundColor) and isDark(brandColor) {
        iconColor = white
    } else {
        iconColor = brandColor
    }
}

What do you think to that?

@chris48s
Copy link
Member

I suppose the downside of that approach is there are some logos which sit somewhere in the middle of the brightness spectrum which look good enough on both of our default backgrounds e.g:

- https://img.shields.io/badge/foo-bar-blue.svg?logo=twitter
- https://img.shields.io/badge/foo-bar-blue.svg?logo=twitter&style=social

Doing something like what Ive described there would probably end up making that either black or white in one of those cases :/

@paulmelnikow
Copy link
Member Author

Urgh, I forgot about needing to recolor everything in simple-icons.

There are a small range of colors which look good on both light and dark background. The only one I'm personally familiar with is that azure color close to the Twitter logo, though there may be some reds for which that is also true.

Perhaps we could improve on the algorithm to detect those "midrange" colors?

@RedSparr0w
Copy link
Member

Yeah that was one of my main concerns with this approach.


Although it doesn't look bad, it would be pretty unexpected for all your logos to suddenly be black/white.
The only way i can think of to get around it is using an ignore/blacklist, but that also adds more maintenance.

@RedSparr0w
Copy link
Member

RedSparr0w commented Jan 16, 2019

Maybe we could just recolor any logos darker than 0.33 when using the default colorA,
or brighter than 0.85 when using the social style badge.

function brightness(r, g, b){
  return +((r * 299 + g * 587 + b * 114) / 255000).toFixed(2)
}

function hexToRgb(hex) {
  const result = (/^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex) || (/^#?([a-f\d]{1})([a-f\d]{1})([a-f\d]{1})$/i.exec(hex) || [0,0,0,0]).map(h=>`${h}${h}`)).slice(1)
  const [r, g, b] = result.map(h=>parseInt(h, 16))
  return {r, g, b}
}

@chris48s
Copy link
Member

Yeah - something like that sounds good. Its difficult to know if 0.33 and 0.85 are the exact right thresholds without seeing some examples, but splitting into light/mid/dark instead of just light/dark seems like the way to go.

@RedSparr0w
Copy link
Member

RedSparr0w commented Jan 16, 2019

I created a quick little script that you can run on https://simpleicons.org/ and it will show you how the logos would look on the default badge background color (#555),
Although its still quite hard to decide where the cut off point should be.

const darkestColor = 0.4;

function brightness(r, g, b){
  return +((r * 299 + g * 587 + b * 114) / 255000).toFixed(2)
}

document.body.style.backgroundColor = '#555';
[...document.getElementsByClassName('grid-item grid-item--light'), ...document.getElementsByClassName('grid-item grid-item--dark')].forEach((el)=>{
	let [match, r, g, b] = el.style.backgroundColor.match(/rgb\((\d+),\s(\d+),\s(\d+)\)/);
	let bright = brightness(r,g,b);
	el.style.color = bright <= darkestColor ? 'whitesmoke' : el.style.backgroundColor;
	el.style.backgroundColor = '#555555'
});

or with a range:

const range = [0.22, 0.4];

function brightness(r, g, b){
  return +((r * 299 + g * 587 + b * 114) / 255000).toFixed(2)
}

document.body.style.backgroundColor = '#555';
[...document.getElementsByClassName('grid-item grid-item--light'), ...document.getElementsByClassName('grid-item grid-item--dark')].forEach((el)=>{
	let [match, r, g, b] = el.style.backgroundColor.match(/rgb\((\d+),\s(\d+),\s(\d+)\)/);
	let bright = brightness(r,g,b);
	el.style.color = bright >= range[0] && bright <= range[1]  ? 'whitesmoke' : el.style.backgroundColor;
	el.style.backgroundColor = '#555555'
});

@paulmelnikow
Copy link
Member Author

Wow, you come up with such unexpected solutions to these kind of challenges @RedSparr0w!

@paulmelnikow
Copy link
Member Author

I prefer the first version (i.e. darkestColor = 0.4, where everything dark gets turned into light. Especially when these logos are smaller.





In that version, the ones that seem borderline are read the docs, automatic, bandcamp, and circle:




I think I can live with all of those except circle which looks better in white:

I wonder, if we nudge the cutoff slightly, do we pull in circle, or do we pull in more?

@paulmelnikow
Copy link
Member Author

Do they all look okay on the light background?

@RedSparr0w
Copy link
Member

RedSparr0w commented Jan 16, 2019

0.6 seems to be a decent value:

const brightest = 0.6;

function brightness(r, g, b){
  return +((r * 299 + g * 587 + b * 114) / 255000).toFixed(2)
}

document.body.style.backgroundColor = '#fcfcfc';
[...document.getElementsByClassName('grid-item grid-item--light'), ...document.getElementsByClassName('grid-item grid-item--dark')].forEach((el)=>{
	let [, r, g, b] = el.style.backgroundColor.match(/rgb\((\d+),\s(\d+),\s(\d+)\)/);
	let bright = brightness(r,g,b);
	el.style.color = bright >= brightest  ? '#333' : el.style.backgroundColor;
	el.style.backgroundColor = '#fcfcfc'
});

specifically for dreamweaver which has a brightness value of 0.64, 0.8 didn't quite cover it.

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Jan 16, 2019

I wonder if circle should be a manual override, or if we should just ignore it. I think part of why it doesn't work is that the lines are so thin. It looks like 0.48 will pick up Circle. Not sure if we make anything else worse, though.

@RedSparr0w
Copy link
Member

RedSparr0w commented Jan 16, 2019

I think part of why it doesn't work is that the lines are so thin.

Agreed, that's probably the main problem with that logo.

Not sure if we make anything else worse, though.

It's hard to tell if any of the logos look worse than before, but i think it would be better to use this approach than the ignore/blacklist, which just creates more overhead.

@chris48s pseudo code from above, with some minor adjustments:

//if we're using our own icon, assume its multi-coloured and just use it as-is

if (simpleicons) {
    if (colorA == '#555' /* Default Color Not Social */ && brightness(brandColor) <= 0.4 /* or higher  as the background color has a value of 0.33 */) {
        iconColor = 'whitesmoke'
    }
    if (colorA == '#FCFCFC' /* Default Color Social */ && brightness(brandColor) >= 0.6 /* should be low enough as the background color has a value of 0.99 */) {
        iconColor = '#333'
    } else {
        iconColor = brandColor
    }
}

If people are changing the default colorA of the badge, I think its safe to assume they will also change the logoColor if needed.

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Jan 17, 2019

After looking at .4 and .48, I'm much happier with .4 and think we should either make an exception for circle or ignore it.

@chris48s
Copy link
Member

After looking at .4 and .48, I'm much happier with .4 and think we should either make an exception for circle or ignore it.

Yep. 4 looks good to me. I think if we can arrive at something that looks good in most cases, we should leave the odd edge cases to the user to customise. Bear in mind simple-icons is knocking out a minor release 2 or 3 times a month with new icons in each one. Trying to maintain manual overrides for every edge cases is a losing battle.

If people are changing the default colorA of the badge, I think its safe to assume they will also change the logoColor if needed.

Agreed. If we can come up with something that gives us a sensible amount of contrast in most cases on our 2 default backgrounds, that's pretty good going (and a massive improvement on where we are now).

Are you good to work this up into a PR @RedSparr0w ? If not I don't mind picking it up

@RedSparr0w
Copy link
Member

I think if we can arrive at something that looks good in most cases, we should leave the odd edge cases to the user to customise.

👍

Are you good to work this up into a PR @RedSparr0w ? If not I don't mind picking it up

I can, but won't be able to get around to doing anything with this until around the 28th, so if you have some time feel free to take this on 😄

@chris48s
Copy link
Member

chris48s commented Jan 19, 2019

I was going to have a go at it after #2742 was merged to avoid merge conflicts, but now #2796 refactors all the logo code so probably not sensible to do it just yet. I'll try and sneak in a PR for it after #2796 but before @paulmelnikow opens another PR to move it all to /core or /gh-badges ;)

@paulmelnikow
Copy link
Member Author

After #2833 is merged will this be done? Or do we need to do something similar for the remaining Shields logos?

@chris48s
Copy link
Member

I'm deliberately only focussing on simple-icons for the moment because they all respond correctly to setting a colour. logoColor doesn't necessarily do anything sensible if you try to use it on our logos. For example:

- https://img.shields.io/badge/logo-npm-blue.svg?logo=npm
- https://img.shields.io/badge/logo-npm-blue.svg?logo=npm&logoColor=green

Once #2833 is merged, the next thing I'd like to do is get rid of as many of our logos as possible (once we abstract the issue of default colours, I'm happy to bin a lot more of ours in favour of simple-icons). Then I'd like to revisit what we're left with and what's sensible to do with them and think about guidance for future etc. If we're left with mostly multi-coloured logos where logoColor doesn't make sense anyway, that possibly changes the game.

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

Successfully merging a pull request may close this issue.

3 participants