Use the same icon mode calculation for comboboxes as for buttons
ClosedPublic

Authored by davidre on Apr 17 2020, 9:31 AM.

Details

Summary

When hovering a focused combobox the icon was in Selected state resulting in a wrong color.

Test Plan

Hover over a focused combobox that has an icon

Diff Detail

Repository
R31 Breeze
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidre created this revision.Apr 17 2020, 9:31 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 17 2020, 9:31 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidre requested review of this revision.Apr 17 2020, 9:31 AM
broulik added a comment.EditedApr 17 2020, 9:51 AM

Now that we can properly recolorize all the icons, maybe we should drop the State_Selected handling altogether?

Though probably qqc2 desktop style or Kirigami still relies on this, or maybe not since they can also set a proper palette

I don't understand. The palette doesn't change when a widget is selected, does it? The selected state is used by KIconLoader to decide which color to use for text color, or highlight

state == KIconLoader::SelectedState ? pal.highlightedText().color().name() : pal.windowText().color().name(),
ndavis added a subscriber: ndavis.EditedApr 18 2020, 2:42 PM

Oddly, I can't reproduce the bug this fixes in all comboboxes. KSysGuard's process filtering and tools comboboxes are right next to each other and have different behavior.

This patch causes another problem with all monochrome icons on the Breeze colorscheme though:

Oddly, I can't reproduce the bug this fixes in all comboboxes. KSysGuard's process filtering and tools comboboxes are right next to each other and have different behavior.

This patch causes another problem with all monochrome icons on the Breeze colorscheme though:

Isn't one of them a toolbuttons with menu, not a combobox?
I don't think this patches causes that issue. Do you have
https://phabricator.kde.org/R31:66d0b0b4e3e1adc389dbd4ce1976d81860d1880d ?

ndavis added a comment.EditedApr 18 2020, 7:29 PM

Oddly, I can't reproduce the bug this fixes in all comboboxes. KSysGuard's process filtering and tools comboboxes are right next to each other and have different behavior.

This patch causes another problem with all monochrome icons on the Breeze colorscheme though:

Isn't one of them a toolbuttons with menu, not a combobox?
I don't think this patches causes that issue. Do you have
https://phabricator.kde.org/R31:66d0b0b4e3e1adc389dbd4ce1976d81860d1880d ?

You're right, it looks like this patch doesn't cause the problem. I thought it did because the problem doesn't appear until I change color schemes and I normally change color schemes when testing Breeze patches. It's also only happening to KSysGuard as far as I can tell.

ndavis accepted this revision.Apr 18 2020, 7:34 PM

Ok, it took me a while to fully understand what this patch does, but it seems to be working fine.

This revision is now accepted and ready to land.Apr 18 2020, 7:34 PM
This revision was automatically updated to reflect the committed changes.