Use black font when background color is too light #5467
Conversation
PyvesB
added
core
|
|
@PyvesB Great! |
|
@PyvesB can there be an option to override the auto-detection? Like ... Why? Well .. in #5455 .. I would have liked the badges having Black text. The current threshold still keeps it white. So ... I think that the current threshold should be kept as it is. It seems well-thought-of. But it's always the edge cases that annoy users. So why not provide an override for them? Just my two cents |
|
This is really nice. https://shields-staging-pr-5467.herokuapp.com/badge/foo-bar-white I appreciate the sentiment of setting a very conservative threshold so we don't change the appearance for any of the standard badge colours. Do you have any idea what sort of threshold we'd be looking at to start changing the text colour on something like https://shields-staging-pr-5467.herokuapp.com/badge/foo-bar-9cf and whether that would start impacting any of the standard colours? Also did you consider using the dark grey colour we use when we re-colour light logos for social badges ( |
badge-maker/lib/badge-renderers.js
Outdated
| @@ -10,6 +11,19 @@ function capitalize(s) { | |||
| return `${s.charAt(0).toUpperCase()}${s.slice(1)}` | |||
| } | |||
|
|
|||
| function colorsForBackground(color) { | |||
| if (brightness(color) <= 0.8) { | |||
There was a problem hiding this comment.
It would be clearer what we're doing here if we gave this a name e.g:
const brightnessThreshold = 0.8;We should probably do the same thing for the logo thresholds too - those are also magic numbers.
Thanks for your thoughts @subnut! You've touched upon a classic problematic within the Shields project, namely how much customisation should we allow within our badges? Shields makes two main promises, readability and consistency. There's always a careful balance to strike and we need to make sure new customisation options don't impede too much on one or the other - for example we got the balance wrong in #1478. Here, this PR trades off some consistency (i.e. text is no longer always white) for some readability (i.e. badges are easier to read on light backgrounds). However, I'm concerned that adding a Beyond this debate about readability, consistency and customisation, I think we should be careful when it comes to adding new options, as having too many of them doesn't necessarily help users and may even overwhelm some of them. There are already approximately ten query parameters available for all badges! If a lot of users end up requesting an override and we can't make them happy by tweaking the threshold, then I'd personally be open to considering such an option, but I'm not keen on implementing it initially as part of this PR. 😉
0.75 would make the first one go black, 0.69 the second one. With 0.69 being used, I don't think our standard named colors change, but a few of the default badges displayed on the homepage do, "CII Best Practices Level", "SymfonyInsight Stars" and "9cf" from what I can tell from a quick browse through. With 0.75 only the "9cf" badge on the homepage changes from what I can see. Do you want me to lower the threshold to something in between so that only "9cf" is affected?
Good point, I've pushed a commit with that change. |
|
Sorry for letting this rot for a bit. The code as it stands looks good. I think we could go to 0.69 no problem (sorry I should have just played with this locally myself). That said, this PR has got me thinking about the level of contrast on our standard colour schemes from the perspective of accessibility. Some of our standard colour schemes actually score quite poorly and could be improved. For example
I'm aware this is scope creep, so unless you really want to dive into that now I think we should probably switch the threshold to 0.69 (if you agree with that) and merge this because its an improvement over what we have now. Then as another issue it might be worth reviewing our most commonly used styles from this perspective. We probably can't rely on only brightness to get amazing contrast on every bespoke colour, but if we can use 'presets' to ensure all of these have a good accessibility score that would be beneficial.
|
Alternatively there is a color-contrast-checker package available, I'm wondering whether we could make use of that or another similar package.
I've pushed the change. Agreed, let's continue to iterate over contrast in a separate issue. |
badges will become readable badges/shields#5467 (comment)
|
Before:
Now:
That just ruined my carefully crafted band-aid badge 😢 |
|
@laowantong if you make the background a bit darker, the font will automatically swap to white. 😉 |
|
@PyvesB Sure, but the background color would no more evoke a classic band aid. Thanks anyway! |
|
@PyvesB I have just switched to another service, sorry about that. |
|
Thanks, much improved! Before
After
|
badges/shields#5467 unfortunately doesn't provide an override so we just have to make some of the colors darker
Fixes #4661.
I've chosen quite a conservative threshold for switching to black text. None of the badges advertised on our homepage will be affected, this functionality will only kick in with very light background colours.
Let me know what you think!