Theming: invert foreground color on bright backgrounds #412
Conversation
|
@juliushaertl, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke, @oparoz and @Henni to be potential reviewers |
| function calculateLuminance(rgb) { | ||
| var hexValue = rgb.replace(/[^0-9A-Fa-f]/,''); | ||
| var r,g,b; | ||
| if(hexValue.length === 3) { |
There was a problem hiding this comment.
Missing space between if and (.
But it would be worth parsing the whole code so that all such errors are automatically fixed.
|
Looks good. The logo part can be taken care of in another PR, simply detect if a custom one is used and if there is a dark version. I'm not a fan of static methods, but everything seems tested, so it's not a show stopper. |
|
All right, thanks. Spaces should be fixed now. |
|
Wow, awesome! :) Really good stuff @juliushaertl @nextcloud/designers @Lord-Protector @schiessle let’s give this a good test! Also with the installation process etc. @juliushaertl @oparoz the logo we will leave entirely untouched, yes. Because when you change the theming, you will see that a light logo will not work. That’s not something we should automatically modify. |
|
|
The JavaScript console shows this error: the correct path would be |
|
Good point @jancborchardt. |
|
@jancborchardt theoretically logo will have a good look with some shade of gray. |
juliusknorr
force-pushed
the
theming-foreground-color
branch
from
30447ca to
d8bc52c
Compare
juliusknorr
force-pushed
the
theming-foreground-color
branch
from
d8bc52c to
48ac845
Compare
|
The disappearing icon should be fixed now. |
|
everything works now... Great work! 👍 |
|
Works and looks good 👍 Regarding:
Not sure about this, maybe @karlitschek can help, because in this case the logo was already recolored (it's white not light-blue) |
|
@juliushaertl want to make a PR against stable9 as well? |
|
Not sure if we should backport it... It is not a critical bug. I would prefer to keep it for the next release |
|
True. @jospoortvliet We should update our trademark policy to also allow white on blue. |
|
@karlitschek theming has nothing to do with the trademark policy though I would say. When theming your Nextcloud, you should also use a different logo. :) |
|
true :-) |
As discussed in #378, this pull request extends the theming app to invert the text/icon color inside
the header for bright colors.
Algorithm for luminance calculation: https://www.w3.org/TR/AERT#color-contrast
Before:
After:
By now the Nextcloud logo will not be touched, as specified by the Nextcloud Trademark Guidelines:
cc @nextcloud/designers @schiessle