Try to apply the colorscheme of the current theme to QIcons
ClosedPublic
Actions

Authored by davidre on Feb 22 2020, 10:15 PM.

Details

Reviewers

cblack
ngraham
mart

Group Reviewers

Plasma

Commits

R242:b7fa6e0e916b: Try to apply the colorscheme of the current theme to QIcons

Summary

Before icons loaded internally with QIcon::fromTheme were being colored with the
colors from the current global color scheme instead of the ones from the current
Plasma Theme. Leading to visual bugs when the two differ. This happened because
KIconLoader uses the global color scheme by default.
A prominent case is the notification send by the network plasmoid when one
successfully connected to a wireless network. It sets the icon
"network-wireless-on" which is not included in Breeze icons (but is included in
Breeze Plasma Theme). If the current icon theme is indeed Breeze, IconItem
resorts to using QIcon::fromTheme and we end up with a wrong colored
"network-wireless" icon.

BUG: 417780

Test Plan

Diff Detail

Repository

R242 Plasma Framework (Library)

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.Feb 22 2020, 10:15 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 22 2020, 10:15 PM

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

davidre requested review of this revision.Feb 22 2020, 10:15 PM

Harbormaster completed remote builds in B22804: Diff 76197.Feb 22 2020, 10:15 PM

davidre edited the test plan for this revision. (Show Details)Feb 22 2020, 10:16 PM

davidre added a reviewer: Plasma.

Patch works as advertised and the code LGTM.

This revision is now accepted and ready to land.Feb 22 2020, 10:26 PM

Sounds like it would fix https://bugs.kde.org/show_bug.cgi?id=417780 ?

In D27589#616106, @wbauer wrote:

Sounds like it would fix https://bugs.kde.org/show_bug.cgi?id=417780 ?

Yes seems like the exact bug

davidre edited the summary of this revision. (Show Details)Feb 23 2020, 8:50 AM

Confirmed. I tried out the patch now, and that problem is gone.

ngraham accepted this revision.Feb 23 2020, 4:32 PM

mart requested changes to this revision.Feb 24 2020, 11:09 AM

mart added a subscriber: mart.

mart added inline comments.

src/declarativeimports/core/iconitem.cpp
600	maybe Plasma::Theme should have a qpalette getter?

This revision now requires changes to proceed.Feb 24 2020, 11:09 AM

Introduce Plasma::Theme::palette

Harbormaster completed remote builds in B22850: Diff 76284.Feb 24 2020, 1:37 PM

mart accepted this revision.Mar 2 2020, 10:52 AM

This revision is now accepted and ready to land.Mar 2 2020, 10:52 AM

Closed by commit R242:b7fa6e0e916b: Try to apply the colorscheme of the current theme to QIcons (authored by davidre). · Explain WhyMar 2 2020, 11:00 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			src/declarativeimports/core/iconitem.cpp (4 lines)
M			src/plasma/private/theme_p.h (1 line)
M			src/plasma/private/theme_p.cpp (2 lines)
M			src/plasma/theme.h (8 lines)
M			src/plasma/theme.cpp (6 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	76197	667dce5		Feb 22 2020, 10:15 PM	★	★
Diff 2	76284	667dce5	- Introduce Plasma::Theme::palette	Feb 24 2020, 1:37 PM	★	★
Diff 3	76760	a08449a	R242:b7fa6e0e916bd8856cbf85b2d3886bc1e7b0e278	Mar 2 2020, 10:55 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
db70accf4947	e0027fc20037	dd30908de5a9	David Redondo	Introduce Plasma::Theme::palette	Feb 24 2020, 1:34 PM
dd30908de5a9	bbf1c0c8493e	667dce502acf	David Redondo	Try to apply the colorscheme of the current theme to QIcons (Show More…)	Feb 22 2020, 10:00 PM