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

Title bar text color doesn't respect the color theme setting #63

Closed
nwwdles opened this issue Dec 12, 2019 · 22 comments
Closed

Title bar text color doesn't respect the color theme setting #63

nwwdles opened this issue Dec 12, 2019 · 22 comments
Labels
bug Something isn't working fixed

Comments

@nwwdles
Copy link

nwwdles commented Dec 12, 2019

Latest versions (after d4b4947) seem not to respect the color setting. It's either white or black and you can't even choose which.

Screenshot_20191212_213922

Screenshot_20191212_213226

@kupiqu
Copy link
Owner

kupiqu commented Dec 12, 2019

There is a heuristic to always show a color that has good contrast indep of the background color

@kupiqu kupiqu closed this as completed Dec 12, 2019
@nwwdles
Copy link
Author

nwwdles commented Dec 12, 2019

This heuristic doesn't really work well, as depicted in the second screenshot. White on orange is harder to read than black on orange. Is there a way to disable it?

@kupiqu
Copy link
Owner

kupiqu commented Dec 12, 2019

There is always room for improvement... but please note that I need to make it work for all possible colors, which is tricky.

There is no way to disable it. I guess I could use that of the color theme, and the heuristic when matchtitlebartowindowcolor is enabled, or set an option to enable the heuristic.

I'll think about it...

@kupiqu kupiqu reopened this Dec 12, 2019
@kupiqu kupiqu added enhancement New feature or request low priority labels Dec 21, 2019
@ray-holmium
Copy link

I would also appreciate an option to disable the heuristic! In my case, I want to hide the titlebar text by matching its color to the background of the titlebar. I.e. black text on a black title bar = no more text, clean bars. Cheers!

@kupiqu
Copy link
Owner

kupiqu commented Feb 26, 2020

I don't have time to work on this. If someone wants to submit a pull request, I will review it.

@kupiqu kupiqu added community bug Something isn't working and removed community enhancement New feature or request low priority labels Apr 4, 2020
@kupiqu
Copy link
Owner

kupiqu commented Apr 7, 2020

I'll try to find time to work on this issue

@nwwdles
Copy link
Author

nwwdles commented Apr 8, 2020

this would be great, thanks!
i wanted to fix it but, basically, i just copy-pasted some code back from d4b4947 and called it a day

diff --git a/breezedecoration.cpp b/breezedecoration.cpp
index b78e22b..836ed02 100644
--- a/breezedecoration.cpp
+++ b/breezedecoration.cpp
@@ -309,6 +309,17 @@ namespace Breeze
     {
         auto c = client().toStrongRef().data();
 
+        if ( !matchColorForTitleBar() ) {
+            if ( m_animation->state() == QAbstractAnimation::Running ) {
+                return KColorUtils::mix(
+                    c->color( ColorGroup::Inactive, ColorRole::Foreground ),
+                    c->color( ColorGroup::Active, ColorRole::Foreground ),
+                    m_opacity );
+            }
+
+            return  c->color( c->isActive() ? ColorGroup::Active : ColorGroup::Inactive, ColorRole::Foreground );
+        }
+
         QColor darkTextColor( !c->isActive() && matchColorForTitleBar() ? QColor(81, 102, 107) : QColor(34, 45, 50) );
         QColor lightTextColor( !c->isActive() && matchColorForTitleBar() ? QColor(192, 193, 194) : QColor(250, 251, 252) );
 

as i was too lazy to go around and fix monochrome icon colors too (as IMO it feels weird when you have white buttons and dark text or vice versa (e.g with #eda771 titlebar color)). if you're fine with the diff (at least as a temporary solution), i could make a PR

@StarrKiss
Copy link

Can you add this as a PR please? The author doesn't seemt o be active...

@StarrKiss
Copy link

@nwwdles I tried this out on my own machine, but for some reason it only works if Match Titlebar to WIndow is ACTIVE, even though the code specifically says if it's NOT ACTIVE... I don't know why.
image
So idk about that. Works if I check it though!

@StarrKiss
Copy link

I tried changing the code to remove the ! in front of the boolean check- for some reason it didn't seem to change anything at all once built. SO I have no idea about that..

@nwwdles
Copy link
Author

nwwdles commented Dec 15, 2020

@StarrKiss, the patch seems to be still working for me.

screenshot

Screenshot-20201215193352-1238x913

It's not really good for PR, because it doesn't fix colors for symbolic sierra*-style buttons (but Gnome and Plasma styles are fine). I'm not going to use KDE for the time being, so I don't want to spend time on fixing it. But here's the project with the patch applied https://github.com/nwwdles/SierraBreezeEnhanced

@kupiqu
Copy link
Owner

kupiqu commented Dec 15, 2020

I'll try to find time in christmas time to take a look (running very busy lately)

@W54J04S07T
Copy link

A simple "follow color scheme exactly" entry
along with associated logic would probably be
your fastest approach.

Although, I get the impression most here would be happier
with a "Match TitleBar to Color Scheme" entry that might be even simpler.

BTW, this project is awesome!!

@kupiqu
Copy link
Owner

kupiqu commented Dec 18, 2020

@nwwdles could you share this colortheme for testing purposes?

Screenshot_20191212_213226

@nwwdles
Copy link
Author

nwwdles commented Dec 18, 2020

@kupiqu sure, I think it should be this one https://store.kde.org/p/1314024/ (the screenshot seems to be mismatched)

@kupiqu
Copy link
Owner

kupiqu commented Dec 26, 2020

I fail to reproduce. I see the same behavior in Breeze:

image

@kupiqu
Copy link
Owner

kupiqu commented Dec 26, 2020

In SBE:

image

@nwwdles
Copy link
Author

nwwdles commented Dec 27, 2020

seems like i might have customized the theme and set the active fg color to black manually, this was a while ago. that's pretty much a one line edit

--- CommonalityClassic.colors	2020-12-27 03:47:08.357529884 +0300
+++ CommonalityClassic2.colors	2020-12-27 03:47:25.817530218 +0300
@@ -112,7 +112,7 @@
 [WM]
 activeBackground=237,167,113
 activeBlend=138,94,131
-activeForeground=248,248,248
+activeForeground=0,0,0
 inactiveBackground=151,153,155
 inactiveBlend=137,143,154
 inactiveForeground=248,248,248

@kupiqu
Copy link
Owner

kupiqu commented Jan 5, 2021

It seems there's no more solution than adding an option to strictly follow the colortheme for the color buttons and titlebar text

Update: working on it

@kupiqu
Copy link
Owner

kupiqu commented Jan 29, 2021

I created a branch for this, called issue63 (https://github.com/kupiqu/SierraBreezeEnhanced/tree/issue63). In its current state, I set true's and false's in the right places so the color theme is applied to titlebar text and markers instead of the heuristic.

What needs to be done:

  1. create the options in settings and pass them so true's/false's can be replaced with the proper option state

In addition, although less 'urgent', it would be nice if the monochrome symbols, sierra and dark-aurorae styles, could follow the same behavior of that of the plasma style.

Note: I'm super busy at the moment and I am not sure when I will be able to finish point (1) above, so if anyone wants to help on this, I am very much open to review pull requests on this. This shouldn't be that difficult as one just needs to see how options are managed and add a new one for using the default color theme vs the heuristic.

@kupiqu
Copy link
Owner

kupiqu commented Apr 9, 2021

I'll set this as a low priority bug for now, maybe somebody picks it up.

@kupiqu
Copy link
Owner

kupiqu commented Nov 1, 2021

My aim is that SBE is maintained by the community. This shouldn't be hard to fix. I hope someone in the community cares about SBE and submits a pull request about it... I could help if somebody wants to try but is lost where to start.

kupiqu pushed a commit that referenced this issue Sep 2, 2022
@kupiqu kupiqu closed this as completed Sep 2, 2022
@kupiqu kupiqu added the fixed label Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed
Projects
None yet
Development

No branches or pull requests

5 participants