<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/129204/">https://git.reviewboard.kde.org/r/129204/</a>
</td>
</tr>
</table>
<br />
<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 Martin Gräßlin.</div>
<div>By Roman Gilg.</div>
<p style="color: grey;"><i>Updated Okt. 17, 2016, 8:40 nachm.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">Changed structure completely in order to support legacy applets and the asynced releases of Plasma and Frameworks. The new structure also reduces the extent of changes to other components. For example we don't need any changes anymore in KWin and Plasma-Workspace. The solution now works like this:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">A qml based applet can decide if it wants to use legacy behaviour of activation, which means while being activated it will NOT trigger shrinking or if it wants to let the shortcut work like a switch of the expanded state</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">The default value of this behaviour is false in order to not break legacy applets</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">The launchers set it to true, in case of kicker only if it's not in Dash mode (maybe we could use the new system here too, but it shouldn't be part of this diff)</li>
</ul></pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=367685">367685</a>
</div>
<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;"><h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">The new Meta-key support for launcher opening doesn't work for closing at the moment. My analysis of the problem was as follows:</h1>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">KWin calls Applet::activated() over DBus</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Applet::activated() is connected to AppletInterface::activated()</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">AppletInterface::activated() is connected to setExpanded(true), which can only expand the launcher, but not the other way around</li>
</ul>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Q: Why is Alt+F1 working though?</h1>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">A: The launchers seem to inherit the reimplemented function Dialog::focusOutEvent(..). Atleast when you delete line 1094 in dialog.cpp, which sets the visibility to false, it's not working anymore. Alt+F1 triggers the focusOutEvent(..) as a global shortcut.</p>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Q: Why is it working with the Dashboard though?</h1>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">A: The dashboard doesn't use the expanded feature of the plasmoid, but rather connects directly to the AppletInterface::activated() signal and shows/hides an independent widget when this signal gets triggered.</p>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Solution:</h1>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Create new toggled() signal chain in KWin, Applet, AppletInterface, which tests the current expanded state and sets it afterwards accordingly to the opposite. This diff here in plasma-framework is the first one, which the others depend on. The others are on Phabricator:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">kwin: https://phabricator.kde.org/D3077</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">plasma-workspace: https://phabricator.kde.org/D3078</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">plasma-desktop: https://phabricator.kde.org/D3079</li>
</ul>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Need feedback regarding:</h1>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Should we remove the activated() signal chain? Is it used somewhere else than KWin?</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Could a race condition occur if we deexpand the applet while at the same time setting the visibility to false through focusOutEvent()? My tests until now don't suggest it, but I haven't yet looked into it extensively.</li>
</ul></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/plasmaquick/appletquickitem.h <span style="color: grey">(943e227)</span></li>
<li>src/plasmaquick/appletquickitem.cpp <span style="color: grey">(2f100b8)</span></li>
<li>src/plasmaquick/private/appletquickitem_p.h <span style="color: grey">(1436935)</span></li>
<li>src/scriptengines/qml/plasmoid/appletinterface.cpp <span style="color: grey">(1cd6934)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129204/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>