Bug 426651 - KStyle Breeze uses a wrong KColorScheme for Button colors focus and hover
Summary: KStyle Breeze uses a wrong KColorScheme for Button colors focus and hover
Status: RESOLVED FIXED
Alias: None
Product: Breeze
Classification: Unclassified
Component: QStyle (show other bugs)
Version: 5.17.5
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Jan Blackquill
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-17 19:17 UTC by Jaakko Kantojärvi
Modified: 2020-09-21 20:38 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jaakko Kantojärvi 2020-09-17 19:17:22 UTC
SUMMARY

The KStyle Breeze uses the color set View for Button colors focus and hover, when it should use color set Button instead.

The style uses helper functions [1] to retrieve focus and hover colors and uses these for buttons too [2]. However, the functions use cached KStatefulBrush, which is constructed using KColorScheme::View (i.e. not KColorScheme::Button) [3].

Based on the KColosScheme API doc [4], it seems the Breeze should use the different color set for these colors.

It's likely that there are other function rendering buttons. which use the wrong brush too.

[1]: https://invent.kde.org/plasma/breeze/-/blob/master/kstyle/breezehelper.h#L55
[2]: https://invent.kde.org/plasma/breeze/-/blob/master/kstyle/breezehelper.cpp#L219 
[3]: https://invent.kde.org/plasma/breeze/-/blob/master/kstyle/breezehelper.cpp#L42
[4]: https://api.kde.org/frameworks/kconfigwidgets/html/classKColorScheme.html#a5b3eef45718f795b80ea24bf3b364b7d


STEPS TO REPRODUCE
1. Select Breeze application style and e.g. Breeze Dark color theme
2. Edit Focus Decoration and Hover Decoration for color set View (set something wild)
3. Save and apply the new color scheme
4. Hover and focus some buttons


OBSERVED RESULT

Buttons use focus and hover colors from the View color set.


EXPECTED RESULT

Buttons would use the colors from the Button color set (which were not edited by the reproduce step).


SOFTWARE/OS VERSIONS
Operating System: Debian GNU/Linux 
KDE Plasma Version: 5.17.5
KDE Frameworks Version: 5.70.0
Qt Version: 5.14.2
Kernel Version: 5.8.0-1-amd64
OS Type: 64-bit


ADDITIONAL INFORMATION

If someone can confirm that I'm not totally off with this, then I might look into fixing this. However, I'm not sure about how much time I have or if I can make the code good enough (I asumme the brush is cached for some reason).
Comment 1 Nate Graham 2020-09-21 03:26:11 UTC
Noah and Carson, any thoughts on this?
Comment 2 Jan Blackquill 2020-09-21 03:29:37 UTC
Seems like something to fix. Will add to my to-do list for tomorrow.
Comment 3 Nate Graham 2020-09-21 03:34:27 UTC
<3
Comment 4 Jaakko Kantojärvi 2020-09-21 10:32:16 UTC
I have been using different colors for different color sets, so I can see which locations use what colors for past few days.

I think one challenge is that there is only decoration colors and not focus and hover background colors. To me, the decoration colors seem to be more highlight colors and not suitable for the background. However, the Breeze requires background highlight colors too, which is good for the look'n'feel of course.

For example, selecting a nice bright (high contrast) color for the hover effect, makes the contrast low when it's used as a background (bright color behind bright text), as you can see in the video in shared imgur post [1].

The Plasma panel seems to use the decoration color for the edge as expected, but it also uses a darkened version of the color for the background (see image 2 in [1]), thus kind of fixing the issue. Should the same be done here too? Or should we blend/mix the decoration color with the background color? That could mean that if the background color is white and text is black, the resulting background still yield a good contrast for the text?. As a side note, the panel uses correct color set for buttons.

I guess the best would be adding background colors for hover and focus cases in the color set, but I guess that would be way harder or more complicated to change than working with the current color sets.

[1]: https://imgur.com/a/ED6VD9D

p.s. I was working on with a generic color scheme, so I could give a list of major colors and get a list of color shemes out, which would mimic the "highlight" color feature from other operating systems.
Comment 5 Bug Janitor Service 2020-09-21 20:21:37 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/breeze/-/merge_requests/31
Comment 6 Jan Blackquill 2020-09-21 20:27:46 UTC
Git commit 659e3d35dca93b600b1492b653ea641e5158824f by Carson Black.
Committed on 21/09/2020 at 20:21.
Pushed by cblack into branch 'master'.

[kstyle]: Make buttons use KColorScheme::Button for hover and focus colours

M  +13   -11   kstyle/breezehelper.cpp
M  +10   -0    kstyle/breezehelper.h

https://invent.kde.org/plasma/breeze/commit/659e3d35dca93b600b1492b653ea641e5158824f
Comment 7 Nate Graham 2020-09-21 20:38:15 UTC
Git commit dd3b18930e08638e27eeff7e382a3854036140b6 by Nate Graham, on behalf of Carson Black.
Committed on 21/09/2020 at 20:38.
Pushed by ngraham into branch 'Plasma/5.20'.

[kstyle]: Make buttons use KColorScheme::Button for hover and focus colours


(cherry picked from commit 659e3d35dca93b600b1492b653ea641e5158824f)

M  +13   -11   kstyle/breezehelper.cpp
M  +10   -0    kstyle/breezehelper.h

https://invent.kde.org/plasma/breeze/commit/dd3b18930e08638e27eeff7e382a3854036140b6