<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126637/">https://git.reviewboard.kde.org/r/126637/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 6th, 2016, 12:14 p.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm not arguing against the addition of this property, my patch should significantly reduce the need for it and prevent abusing it, however.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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</p></pre>
</blockquote>
<p>On January 6th, 2016, 12:30 p.m. UTC, <b>Marco Martin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">is your patch already in?
can we have it running and tested for a while before deciding if this one is needed as well?</p></pre>
</blockquote>
<p>On January 6th, 2016, 1:06 p.m. UTC, <b>David Rosca</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
<br />
<p>- Eike</p>
<br />
<p>On January 6th, 2016, 11 a.m. UTC, David Rosca wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma and Kai Uwe Broulik.</div>
<div>By David Rosca.</div>
<p style="color: grey;"><i>Updated Jan. 6, 2016, 11 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-framework
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Add property animated, that allows to disable cross-fade animation when changing icon (enabled by default).</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Setting <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">animated: false</code> disables the animation.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/declarativeimports/core/iconitem.h <span style="color: grey">(366edf3)</span></li>
<li>src/declarativeimports/core/iconitem.cpp <span style="color: grey">(db15d0f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126637/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>