Zren / breeze-alphablack

Breeze Light theme with minor improvements and a black panel/titlebar.
https://store.kde.org/p/1084931/
35 stars 7 forks source link

Clarification on altering color values #20

Closed wsdfhjxc closed 3 years ago

wsdfhjxc commented 3 years ago

To be more precise, I'm curious about this particular line.

There must have been some reason for doing that, and I can see that the picked highlight color is displayed correctly for items in the default Plasma's Task Manager plasmoid. However, the modified highlight color (not the original one) seems to be present in many other places, for example in KRunner or in the Add Widgets panel. Depending on the picked highlight color value, the modified color might be slightly different, or very different from the picked one (e.g. picked red, but modified is purple).

This seems like a bug, as there is a mismatch between what the user picks, and what is displayed.

After zero-ing the RGB arguments in the hoverEffect method call, the picked color is preserved correctly.

Zren commented 3 years ago

It appears to set the DecorationFocus in all color groups.

DecorationFocus=30,146,255 is the [Colors:Complementary] that breeze-dark used to use:

Doesn't look like it's used now though:

wsdfhjxc commented 3 years ago

Ok, so this is specific to the Breeze's blue highlight color, and it used to make sense to modify the RGB values to match its variant. The issue here is that, this code unconditionally modifies the RGB values of any provided color, e.g. turning red into purple.

I suggest to add some conditionals, or just throw away the -31, -28, 25 arguments for that method call, so there are no more misleading color modifications, and the picked color will be used explicitly as is. What do you think about that?

Zren commented 3 years ago

Can you provide a screenshot of what specifically that color controls?

wsdfhjxc commented 3 years ago

plasmoid

krunner

widgets


As you can see, some elements (sliders, pin button) use the actually picked color (#926262), while others (focused text fields, KRunner items, Add Widgets panel items) use the modified, purplish version of it. The Plasma's Task Manager plasmoid is fine.

I think that elements in the focused state are affected, as the modified highlight color is assigned to the focusColor property, but in some places the focusColor property isn't used, but selectionColor is, as the unmodified color is assigned to that property. Either way, there is a mismatch between what the user picks, and what is displayed, and it makes me sad to see that :slightly_smiling_face: .

To quickly test the suggested fix, you can check out a branch from my testing fork, if you wish so.

Zren commented 3 years ago

Ah, #926262 is a perfect example thank you.

It looks like -31, -28, 25 was only used for the Complementary color. Are you okay with that purple color being generated for Complementary colors and using highlightColor for the rest?

Zren commented 3 years ago

Or should I use deltaColor(highlightColor, 32)?

wsdfhjxc commented 3 years ago

I think I'm fine with your recent changes, as I can no longer see the purple color, so that's terrific.

Thank you for your time, and for the hard work on all these numerous Plasma-related tools and additions.