Remove unneeded media icons
AbandonedPublic

Authored by ndavis on Sep 6 2019, 4:55 PM.

Details

Reviewers
None
Group Reviewers
VDG
Plasma
Summary

These are already provided by most icon themes and there aren't enough sizes in this file to prevent the icons from getting blurry.

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
media-icons (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16188
Build 16206: arc lint + arc unit
ndavis created this revision.Sep 6 2019, 4:55 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 6 2019, 4:55 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ndavis requested review of this revision.Sep 6 2019, 4:55 PM
ndavis added a comment.EditedSep 6 2019, 5:02 PM

A few things to consider:

If a user uses an icon theme besides Breeze, but keeps using the Breeze desktop theme, they could see a mix of icons from their icon theme and icons from the desktop theme. This is actually already the case in different places that this patch doesn't affect, but this would make things a little worse in that regard, unless all icons were removed from the desktop theme (all icons would come from the icon theme).

A more conservative option would be to just add more sizes for the icons in media.svg, but this means we have to maintain the same icons in 2 different repositories. This is also currently the case, but this would make things a little worse in that regard.

unless all icons were removed from the desktop theme (all icons would come from the icon theme)

What's the drawback with that? That not every icon theme has all icons?

ndavis added a comment.EditedSep 6 2019, 5:45 PM

unless all icons were removed from the desktop theme (all icons would come from the icon theme)

What's the drawback with that? That not every icon theme has all icons?

It could cause breakage in 3rd party plasma widgets and a lot of patches to official widgets would be required. This is because some widgets load SVGs from the plasma theme and then use SvgItems (can only load parts of SVGs from the desktop theme) instead of IconItems (can load icons from the plasma theme or icon theme). It's something to consider for Plasma 6, not Plasma 5.

broulik added a subscriber: broulik.Sep 6 2019, 7:43 PM

-1 we've had do much trouble when we touched media icons last, and still get bug reports about "missing icons" so i prefer not touching them again for no good reason.

ndavis abandoned this revision.Sep 6 2019, 9:24 PM

-1 we've had do much trouble when we touched media icons last, and still get bug reports about "missing icons" so i prefer not touching them again for no good reason.

Alright, I guess I'll just add more icons to media.svg instead.