Bug 373088 - Current tab highlighting counter-intuitive when using dark themes
Summary: Current tab highlighting counter-intuitive when using dark themes
Status: RESOLVED FIXED
Alias: None
Product: Breeze
Classification: Plasma
Component: QStyle (show other bugs)
Version: 5.8.4
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-29 22:46 UTC by Janek Bevendorff
Modified: 2017-07-10 08:25 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Guess which one is the active tab. Hint: it's not main.cpp (11.12 KB, image/png)
2016-11-29 22:46 UTC, Janek Bevendorff
Details
Comparison of tabs in different widget styles (22.33 KB, image/png)
2016-12-18 01:29 UTC, Francis Herne
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Janek Bevendorff 2016-11-29 22:46:23 UTC
Created attachment 102529 [details]
Guess which one is the active tab. Hint: it's not main.cpp

When using the Breeze Dark theme (or any other dark theme such as Krita Dark), highlighting of the current tab is completely counter-intuitive.

While with the normal (bright) Breeze theme, the current tab has a brighter background color than any of the other tabs, it is the other way round for dark themes (see attached screenshot). This is so absolutely counter-intuitive that even after many weeks of using KDevelop I'm still clicking the wrong tabs again and again, although I know that the colors are wrong.
Comment 1 Francis Herne 2016-12-17 16:02:16 UTC
This is unrelated to KDevelop - the Breeze widget style colours tabs this way in all applications.

Try using Fusion widgets with a dark theme, you'll see the active tab is slightly darker and taller.

And yes, it confuses me too.
Comment 2 Janek Bevendorff 2016-12-17 16:07:27 UTC
Well, no matter which module this bug is in, it should be fixed and I
don't want to change my widget style just because I use a different
color theme.
Comment 3 Hugo Pereira Da Costa 2016-12-17 18:53:21 UTC
Its related to color scheme, not widget style.
Dark themes are usually "reverse video" themes, where brighter becomes darker and darker lighter (see for instance how the "text view" is darker). That's what happening here. 
What's needed is to modify the color scheme. not the widget style.
Comment 4 Hugo Pereira Da Costa 2016-12-17 18:54:17 UTC
I mean: either modify the official breeze-dark theme, or you modify the theme you use to something that suits you better.
Comment 5 Francis Herne 2016-12-18 01:28:44 UTC
I think your statement is incorrect - no other widget style installed on my system displays tabs with reversed colours, using Breeze-dark or other dark schemes.

In the attachment, all using Breeze-dark, top to bottom:

- QtCurve (default settings): active tab is lighter, taller and has red close icon.

- Oxygen: active tab is lighter, taller  all using Breeze-dark, top to bottom:

- QtCurve (default settings): active tab is lighter, taller and has red close icon.

- Win9x (bleh): active tab has same colour, taller, has red close icon.

- Fusion: active tab is lighter and taller. All close buttons identical (red).

- Breeze: active tab is much darker, same height. All close buttons identical (grey).


To be honest, I find Breeze's tabs clearer than the other variants with this colour scheme - ignoring distinct close-icon colours which make a huge difference - but the inverse-colour behaviour is clearly influenced by Breeze QtStyle and is reversed from all other styles.
Comment 6 Francis Herne 2016-12-18 01:29:27 UTC
Created attachment 102846 [details]
Comparison of tabs in different widget styles
Comment 7 Janek Bevendorff 2016-12-18 01:50:51 UTC
I agree that Breeze is the nicest theme of all those. But the inverted
colors are clearly confusing and counter-intuitive.

I'm not quite sure where the error lies, though. This weird tab coloring
also appears with other dark color schemes, such as various Krita color
schemes. So it could either be that they are all just based on each
other and inherit the same bug or that it is really an issue with the
Breeze widget style. The fact that changing the widget style leads to
correct tab coloring lets strongly assume it is.
Comment 8 Francis Herne 2016-12-18 02:08:18 UTC
Tangentially, I looked through the colours in Breeze-dark, and can't find any one that matches the inactive tab colour.

It's not clear where this is colour derived from, or how to modify it.

(also, I messed up my previous comment slightly; part of one entry is duplicated. It's 2am here, time to sleep...)
Comment 9 Christoph Feck 2016-12-21 01:57:46 UTC
According to the code, the color that is used to darken the inactive tabs is the WindowText color.

With reverse (dark) themes, this color is bright, so instead of darkening, the inactive tabs become lighter.

Suggested patch: Change "QPalette::WindowText" to "QPalette::Dark" in breezestyle.cpp, line 5549.
Comment 10 Christoph Feck 2016-12-21 01:59:04 UTC
(or "QPalette::Shadow" at your convenience...)
Comment 11 Hugo Pereira Da Costa 2017-02-18 20:12:53 UTC
Will fix. Thanks Christoph for the suggested patch, it is a good one.
(would have implemented earlier if i had been in the assignee list).
Comment 12 Christoph Feck 2017-02-19 03:55:25 UTC
According to the bug's history, it was you who removed you as the assignee :)

https://bugs.kde.org/show_activity.cgi?id=373088
Comment 13 Hugo Pereira Da Costa 2017-02-21 09:31:41 UTC
(In reply to Christoph Feck from comment #12)
> According to the bug's history, it was you who removed you as the assignee :)
> 
> https://bugs.kde.org/show_activity.cgi?id=373088

Well, I guess that must be the other me, taking action when I turn my back.
More seriously, about to push.
- to master
- to Plasma/5.9 (since this is a bug right ?)
Should it also be pushed to Plasma/5.8 ? (LTS) ?
Comment 14 Hugo Pereira Da Costa 2017-02-21 09:34:57 UTC
Git commit 13c049c6fba95cf58de9bcc3f61df56153b7c54f by Hugo Pereira Da Costa.
Committed on 21/02/2017 at 09:30.
Pushed by hpereiradacosta into branch 'Plasma/5.9'.

Use QPalette::Shadow instead of QPalette::Text to darken inactive tabs

M  +1    -1    kstyle/breezestyle.cpp

https://commits.kde.org/breeze/13c049c6fba95cf58de9bcc3f61df56153b7c54f
Comment 15 Maximiliano Curia 2017-07-10 08:25:43 UTC
(In reply to Hugo Pereira Da Costa from comment #13)
> (In reply to Christoph Feck from comment #12)
> > According to the bug's history, it was you who removed you as the assignee :)
> > 
> > https://bugs.kde.org/show_activity.cgi?id=373088
> 
> Well, I guess that must be the other me, taking action when I turn my back.
> More seriously, about to push.
> - to master
> - to Plasma/5.9 (since this is a bug right ?)
> Should it also be pushed to Plasma/5.8 ? (LTS) ?

Yes, please, the original report was reported against 5.8.4, and the https://bugs.debian.org/866996 follow ups indicate that users of 5.8 are still affected by this.

Happy hacking,