<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/125307/">https://git.reviewboard.kde.org/r/125307/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 22nd, 2015, 11:55 a.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;">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</p></pre>
</blockquote>
<p>On September 22nd, 2015, 12:03 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;">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). </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p></pre>
</blockquote>
<p>On September 22nd, 2015, 12:07 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;">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</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;">Also I can see in that case bugreports of people complaining that plasma refuses to use their $favoriteicontheme they did set on systemsettings</p></pre>
<br />
<p>- Marco</p>
<br />
<p>On September 19th, 2015, 8:48 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.</div>
<div>By David Rosca.</div>
<p style="color: grey;"><i>Updated Sept. 19, 2015, 8:48 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;">It is now possible to set preferred icon theme in desktoptheme metadata.desktop.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As discussed on ML, this fixes using light breeze icon theme with breeze dark desktoptheme.</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;">Icon theme is picked correctly with PlasmaCore.IconItem. Changing the desktoptheme updates the icons.</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/plasma/theme.cpp <span style="color: grey">(c49ad4c)</span></li>
<li>src/plasma/private/theme_p.cpp <span style="color: grey">(31a6512)</span></li>
<li>src/plasma/theme.h <span style="color: grey">(3f49719)</span></li>
<li>src/desktoptheme/breeze/metadata.desktop <span style="color: grey">(07bbfc3)</span></li>
<li>src/plasma/private/theme_p.h <span style="color: grey">(5b8f71c)</span></li>
<li>src/declarativeimports/core/iconitem.h <span style="color: grey">(3ef0306)</span></li>
<li>src/declarativeimports/core/iconitem.cpp <span style="color: grey">(692cd8d)</span></li>
<li>src/desktoptheme/breeze-dark/metadata.desktop <span style="color: grey">(77647a4)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125307/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>