Review Request 126637: IconItem: Add animated property

[Eike Hein]

> On Jan. 6, 2016, 12:14 p.m., Kai Uwe Broulik wrote:
> > I'm not arguing against the addition of this property, my patch should significantly reduce the need for it and prevent abusing it, however.
> > 
> > Most cases where you don't want to fade the pixmap is when the view it's in was previously hidden, like a tooltip or the OSD because when it shows up again the context will be vastly different and a fade animation just confusing. However, when the view is still there, a fade animation might still be desired, and then you need to do ugly tricks to get the "animated" property do what you want: eg, animated: !foo.Window.visibility === Window.Visible or something like that
> 
> Marco Martin wrote:
>     is your patch already in?
>     can we have it running and tested for a while before deciding if this one is needed as well?
> 
> David Rosca wrote:
>     This is not meant to be used instead of Kai's patch.
>     I think it's still useful to be able to completely disable the animation, even if it won't be used for tooltips.
>     
>     But in the case of tooltips, for me it looks better if the icon is not animated at all. When the contents of tooltip changes, only icon change is animated while everything else (be it text, window thumbnail, ...) is changed immediately. IMO we should either animate the whole tooltip contents, or nothing.

As David mentioned, if we want UI's like Kicker to use IconItem something like this prop is needed, and in fact Kai's patch makes it more needed. In Kicker, when you switch menu categories some delegates are preserved and some may be added, so you can't have cross-fade in the former and no fade-in in the latter or vice-versa. The last thing needed to make IconItem a full replacement for QIconItem is being able to disable the Plasma theme icon overlay.


- Eike


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


On Jan. 6, 2016, 11 a.m., David Rosca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126637/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2016, 11 a.m.)
> 
> 
> Review request for Plasma and Kai Uwe Broulik.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Add property animated, that allows to disable cross-fade animation when changing icon (enabled by default).
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/iconitem.h 366edf3 
>   src/declarativeimports/core/iconitem.cpp db15d0f 
> 
> Diff: https://git.reviewboard.kde.org/r/126637/diff/
> 
> 
> Testing
> -------
> 
> Setting `animated: false` disables the animation.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160106/46937d9b/attachment-0001.html>