Bug 388261 - App icons should respect active icon theme
Summary: App icons should respect active icon theme
Status: RESOLVED FIXED
Alias: None
Product: Discover
Classification: Applications
Component: discover (show other bugs)
Version: 5.11.4
Platform: Other Linux
: NOR wishlist
Target Milestone: ---
Assignee: Aleix Pol
URL:
Keywords: usability
Depends on:
Blocks:
 
Reported: 2017-12-26 23:04 UTC by Nate Graham
Modified: 2018-07-13 19:59 UTC (History)
7 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.13


Attachments
Clementine: Themed once installed, un-themed in Discover (20.50 KB, image/png)
2017-12-26 23:04 UTC, Nate Graham
Details
LibreOffice Writer: Themed once installed, un-themed in Discover (22.42 KB, image/png)
2017-12-26 23:04 UTC, Nate Graham
Details
Dolphin: Themed once installed, un-themed in Discover (also just plain wrong upstream) (21.59 KB, image/png)
2017-12-26 23:07 UTC, Nate Graham
Details
SMPlayer: Themed once installed, un-themed in Discover (22.74 KB, image/png)
2017-12-26 23:09 UTC, Nate Graham
Details
Kate: Themed once installed, un-themed in Discover (20.00 KB, image/png)
2017-12-26 23:12 UTC, Nate Graham
Details
Icon differs between sources (1.02 MB, video/webm)
2018-02-24 15:56 UTC, Nate Graham
Details
Kolourpaint: Themed once installed, un-themed in Discover (35.67 KB, image/png)
2018-02-25 03:37 UTC, Nate Graham
Details
discover uses stange icon for kolourpaint (102.07 KB, image/jpeg)
2018-07-02 06:41 UTC, PK
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Graham 2017-12-26 23:04:04 UTC
Created attachment 109536 [details]
Clementine: Themed once installed, un-themed in Discover

The app icons that Discover shows come directly upstream, without being taking into account the current icon theme. This can create awkward moments where users might have trouble fining newly-installed apps if their themed icons differ substantially from the ones shown in Discover. I've attached a few examples; the icon on the left is what appears in my Icons-Only Task Manager, while the icon on the right is what Discover shows.
Comment 1 Nate Graham 2017-12-26 23:04:19 UTC
Created attachment 109537 [details]
LibreOffice Writer: Themed once installed, un-themed in Discover
Comment 2 Nate Graham 2017-12-26 23:07:16 UTC
Created attachment 109538 [details]
Dolphin: Themed once installed, un-themed in Discover (also just plain wrong upstream)

This one shows an upstream icon that's just plain wrong in my distro (Kubuntu 17.10): Dolphin is using Nautilus's old icon, for some reason. While this should be fixed upstream, it would also be avoided if Discover could substitute the Breeze icon for Dolphin from my active theme.
Comment 3 Nate Graham 2017-12-26 23:09:00 UTC
Created attachment 109539 [details]
SMPlayer: Themed once installed, un-themed in Discover
Comment 4 Nate Graham 2017-12-26 23:11:23 UTC
This would also work around the issue of upstream sources that only provide poor quality quality low resolution icons, as reported in https://www.reddit.com/r/kde/comments/7ltc4h/discover_using_low_resolution_icons_for_apps_on/. The icons from your theme are virtually guaranteed to be better, and most are scalable SVGs too.
Comment 5 Nate Graham 2017-12-26 23:12:14 UTC
Created attachment 109540 [details]
Kate: Themed once installed, un-themed in Discover
Comment 6 Nate Graham 2017-12-26 23:12:43 UTC
I could go on attaching even more screenshots, but I think you get the idea. :)
Comment 7 Aleix Pol 2017-12-27 14:54:59 UTC
Mh, I'm not convinced TBH. The icon in Discover context is part of the application branding. Overriding it is weird, especially on the software center.
Comment 8 Nate Graham 2018-02-24 15:56:02 UTC
Created attachment 110969 [details]
Icon differs between sources

Another side effect of not doing this: the icon can change when you switch the source. For example, the KDEapps Flatpak repo uses Breeze icons, whereas KDE Neon and Kubuntu packaging (and maybe everyone's packaging) uses Oxygen (or some other theme) icons for KDE apps. Switching sources changes the icon, which is really weird and jarring. See attached screen recording

This affects KDE software very strongly because we make use of icon themes much more than GNOME and other DEs do. So by not doing this, we're mostly hurting ourselves.
Comment 9 Nate Graham 2018-02-25 03:37:59 UTC
Created attachment 110983 [details]
Kolourpaint: Themed once installed, un-themed in Discover

Another example from Kolourpaint. This one surprised me since the Breeze icon is nothing at all like the one shown in Discover. My son opened the app to do some art and I didn't know what it even was just by looking at the icon.
Comment 10 Aleix Pol 2018-02-26 11:13:53 UTC
That's not a side-effect, that's straight out a bug in our applications.
Comment 11 Nate Graham 2018-02-26 13:59:25 UTC
What's the bug in our applications? That Breeze and Oxygen icons are different, or that packagers use Oxygen icons by default?
Comment 12 Andrew Crouthamel 2018-02-26 16:54:14 UTC
+1 for this suggestion.

I'd much rather have my matching SVG's load from the my theme, than the sub-HiDPI, non-matching icons load all stretched and pixelated from the packagers. It just doesn't look nice.

Providing visual consistency will really help make the Plasma desktop look beautiful. It's the little things like this, all added up that make a something like Mac OS so well regarded for its design.
Comment 13 Igor Serwin 2018-03-03 16:42:35 UTC
I strongly agree with this suggestion. With this one change few issues will be easily solved,problems with low res icon's and problem with inconsistent look will disappear. Those two are most visible things for me and my friends who use Discover everyday. I think it's worth sacrificing a little of app branding for consistent qualitative look.
Comment 14 Andres Betts 2018-04-02 19:19:18 UTC
+1
Comment 15 Aleix Pol 2018-04-05 00:20:39 UTC
Git commit 7b3e6de68efaeefa9eb9cb2ffe0fa8c443091202 by Aleix Pol.
Committed on 05/04/2018 at 00:20.
Pushed by apol into branch 'master'.

Prefer the icon from the local theme to upstream's

Overrides the project's icon by the one in the icon theme, provided the
icon
name is offered in the metadata.

M  +5    -7    libdiscover/backends/FlatpakBackend/FlatpakResource.cpp
M  +5    -9    libdiscover/backends/PackageKitBackend/AppPackageKitResource.cpp

https://commits.kde.org/discover/7b3e6de68efaeefa9eb9cb2ffe0fa8c443091202
Comment 16 Andrew Crouthamel 2018-04-05 00:41:20 UTC
Very cool, does this work for all three package methods? (distro, snap, flatpak)
Comment 17 Aleix Pol 2018-04-05 01:04:40 UTC
Not snap yet, as snap doesn't support appstream information just yet. On snap we show the icon the devs uploaded to the store.
Comment 18 Andrew Crouthamel 2018-04-05 01:06:32 UTC
Fair enough, I didn't realize they still don't provide appstream. Thanks!
Comment 19 PK 2018-05-30 15:30:27 UTC
When am I supposed to see my system icon-theme in plasma discover? It is end May and I don't think it has happened already. 
Or, " the icon name is (not) offered in the metadata"... Would that be a common thing? I am using Neon User edition.
Comment 20 Nate Graham 2018-05-30 16:36:40 UTC
This feature will land in KDE Plasma 5.13, which hasn't been released yet.
Comment 21 PK 2018-05-30 16:38:52 UTC
Oh, great! Thank you for your answer Nate...
Comment 22 PK 2018-07-02 06:41:58 UTC
Created attachment 113696 [details]
discover uses stange icon for kolourpaint

Discover now respects my installed and preferred icon theme! Great! Only, for the appliction KolourPaint discover chooses an icon that is not in my preferred theme. I did "locate kolourpaint.svg" and I found the icon that Discover uses in the folder /usr/share/icons/hycolor. So I renamed that folder, deleted the cache, rebooted and still discover uses this icon.
Comment 23 Nate Graham 2018-07-03 21:00:51 UTC
Thanks for finding this issue!

Since the bug report you're commenting on was tracking the implementation of the feature itself, and because that feature was indeed implemented and mostly works, I'd prefer a new bug report to track the issue you've discovered. Would you mind filing it? Thanks!
Comment 24 Nate Graham 2018-07-13 19:59:17 UTC
I see you filed Bug 396149; thanks!