Fix colors from KStatefulBrushes not using application colorschemes
ClosedPublic

Authored by ndavis on Aug 15 2019, 9:05 AM.

Details

Summary

This fixes the colors of things like context menus and application menus

T11124#193132

Test Plan

Old:

New:

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.
ndavis created this revision.Aug 15 2019, 9:05 AM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 15 2019, 9:05 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ndavis requested review of this revision.Aug 15 2019, 9:05 AM
ndavis edited the summary of this revision. (Show Details)EditedAug 15 2019, 9:07 AM

How likely is it for distros using Qt < 5.13 to receive an update to Breeze that isn't a backport?

ndavis edited the test plan for this revision. (Show Details)Aug 15 2019, 9:13 AM
ndavis edited the summary of this revision. (Show Details)

How likely is it for distros using Qt < 5.13 to receive an update to Breeze that isn't a backport?

Do you mean to say that this is fixed already in Qt 5.13?

How likely is it for distros using Qt < 5.13 to receive an update to Breeze that isn't a backport?

Do you mean to say that this is fixed already in Qt 5.13?

No, more code is needed for older versions. See the comment I linked in the task.

As long as Breeze itself doesn't have a hard dependency on ≥ Qt 5.13, you'll need to handle that case, and make sure the added code either still works for 5.13 or gets# ifdef'd out. ...Or you could bump the dep to Qt 5.13, but that's probably not feasible given the relative recency of its release and the fact that it's distributed with Plasma, which is not bumping the Qt dep that high.

mglb added a subscriber: mglb.Aug 15 2019, 7:19 PM

Please add the workaround for a bug fixed in qt 5.13 (eventfilter and stuff). Some people would like to compile it on current systems. In such case non-system Qt is not an option, as the style plugin won't load in older Qt. Also, the code is already written with nice ifdefs and works, so why not.

ndavis updated this revision to Diff 63839.Aug 15 2019, 7:21 PM
ndavis edited the test plan for this revision. (Show Details)

Add event filter for Qt < 5.13

ngraham added inline comments.Aug 15 2019, 7:24 PM
kstyle/breezestyle.cpp
198

You can make it more readable like this:

#if QT_VERSION < QT_VERSION_CHECK(5,13,0)

I thought the hex values were used because it's also supposed to be possible to compile Breeze for Qt 4? I remember reading that Qt 4 needs hex numbers for some reason.

Sounds like you know more than me. Still, the top one is inside a no-KDE4 ifdef, if I'm reading the code right.

Sounds like you know more than me. Still, the top one is inside a no-KDE4 ifdef, if I'm reading the code right.

Ah, that's true. However, unless someone who knows more than me says it's OK, I think I'd like to keep the hex value with a comment to make it more readable.

Anyone want to accept this?

Looking back at the quality of my last Breeze review, maybe @hpereiradacosta could have a go. :)

Considering the changes were already agreed upon before I made this diff, it seem safe to land without further review.

ndavis edited the summary of this revision. (Show Details)Sat, Aug 17, 11:30 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sat, Aug 17, 11:35 PM
This revision was automatically updated to reflect the committed changes.