Review Request 125307: Add option for desktoptheme to set preferred icon theme

[Marco Martin]

> On Sept. 22, 2015, 11:55 a.m., Marco Martin wrote:
> > not sure about it, as it would only change the behavior of IconItem but not QIconItem, that is the most used for various reasons, so the two would end up using different themes
> 
> David Rosca wrote:
>     I found only 8 instances: plasma-desktop/applets (3 - kickoff, kicker), plasma-workspace/applets (2 - only in config ui), kdeplasma-addons/applets (3 - only in QtQuick1 code). 
>     
>     So it is only used in kickoff/kicker and it should be safe to change it to PlasmaCore.IconItem there (if there is not some special reason to use QIconItem).
> 
> Marco Martin wrote:
>     no, it should stay qiconitem.
>     since in those cases shouldn't use the svg icons from the paasma theme and being iconitem more complicated, the difference in time in loading submenus in kicker is distinguishable
> 
> Marco Martin wrote:
>     Also I can see in that case bugreports of people complaining that plasma refuses to use their $favoriteicontheme they did set on systemsettings
> 
> andreas kainz wrote:
>     can you link to the mailing list or the bugreports please
> 
> David Rosca wrote:
>     > since in those cases shouldn't use the svg icons from the paasma theme 
>     
>     It wouldn't use svg icons, because those QIconItems are given QIcons and not QStrings, so the performance penalty there would be only if the icon name wasn't found in "desktop icon theme" (eg. the icon was in Breeze but not in Breeze Dark). But yeah, using IconItem brings that fade animation, which doesn't look good when listing through app menu.
>     
>     > Also I can see in that case bugreports of people complaining that plasma refuses to use their $favoriteicontheme they did set on systemsettings
>     
>     Right now, the plasma is using part of icons from plasma theme and part of the icons from $favoriteicontheme, which I think is worse than using consistently just one icon theme.
>     
>     > can you link to the mailing list or the bugreports please
>     
>     ML: https://mail.kde.org/pipermail/plasma-devel/2015-September/044438.html
>     I didn't find any open bugs with similar issue, but I'm sure I can't be the only one using dark plasma theme with light applications theme.
> 
> Marco Martin wrote:
>     It may make sense to add to iconitem a property to disable plasma icons and maybe even the animation.
>     i don't think it would improve loading times much tough as is just a "bigger" class that just takes more to be instantiated due to just its size
> 
> David Rosca wrote:
>     Well, kicker can stay as it is. There are bunch of icon themes mixed (some from icon theme and some are own application icons) anyway.
>     
>     The issue is more evident in other parts of desktop (eg. icons in some applets, desktop toolbox, etc.).

in general i like the idea, and most of the patch on this part is valid i think.
But i still think that having a fixed icon theme in the desktop is quite a no go.
what's missing here i think is for themes to say "i'm a dark version of the theme X"  then the palsma theme would tell to use a dark version where available, not forcing a particular theme tough


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125307/#review85747
-----------------------------------------------------------


On Sept. 19, 2015, 8:48 a.m., David Rosca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125307/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 8:48 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> It is now possible to set preferred icon theme in desktoptheme metadata.desktop.
> 
> As discussed on ML, this fixes using light breeze icon theme with breeze dark desktoptheme.
> 
> 
> Diffs
> -----
> 
>   src/plasma/theme.cpp c49ad4c 
>   src/plasma/private/theme_p.cpp 31a6512 
>   src/plasma/theme.h 3f49719 
>   src/desktoptheme/breeze/metadata.desktop 07bbfc3 
>   src/plasma/private/theme_p.h 5b8f71c 
>   src/declarativeimports/core/iconitem.h 3ef0306 
>   src/declarativeimports/core/iconitem.cpp 692cd8d 
>   src/desktoptheme/breeze-dark/metadata.desktop 77647a4 
> 
> Diff: https://git.reviewboard.kde.org/r/125307/diff/
> 
> 
> Testing
> -------
> 
> Icon theme is picked correctly with PlasmaCore.IconItem. Changing the desktoptheme updates the icons.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150922/8d12fe47/attachment.html>