<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/119618/">https://git.reviewboard.kde.org/r/119618/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 5th, 2014, 3:35 p.m. UTC, <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/119618/diff/1/?file=296885#file296885line128" style="color: black; font-weight: bold; text-decoration: underline;">ksmserver/screenlocker/kcm/package/contents/ui/main.qml</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">128</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">Timer</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<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;">what's this for? I assume you're working round something.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(and don't reply here, reply in code comments :) )</p></pre>
</blockquote>
<p>On August 5th, 2014, 3:50 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;">i'll add it in the comments as well ;)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
basically, list.setCurrentIndex doesn't work while the model is getting loaded, so list.currentIndex = index in a component.onCompleted of the delegate, doesn't work, restarting a timer when a delegate gets created, seems the only place where we can approximate "set the property when the view really is done loading"</p></pre>
</blockquote>
<p>On August 5th, 2014, 3:59 p.m. UTC, <b>David Edmundson</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;">One approach I've seen done in the past (not sure if it'll work here) is to have a currentPlugin string property on the root view then in the delegate do</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Component.onCompleted: {<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
if (root.currentPluginString == myString) {<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
view.currentIndex = index<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
}<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
}</p></pre>
</blockquote>
<p>On August 5th, 2014, 4:13 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;">yeah, that was the first thing i tried,<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
but if the current item is not the last, and/or is not the last to be created, view.currentIndex = index will have no effect (yep, pretty weird i know)</p></pre>
</blockquote>
<p>On August 5th, 2014, 9:22 p.m. UTC, <b>David Edmundson</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;">Confirmed. It's deliberate in Qt.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QQuickItemView::setCurrentIndex(int index)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
if (d->inRequest) // currently creating item<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
return;</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Mini unit test if people want it: http://paste.kde.org/patrq6fmm</p></pre>
</blockquote>
<p>On August 5th, 2014, 10:41 p.m. UTC, <b>David Edmundson</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;">Original timer seems to be a sensible workaround, I can't find anything better :(</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We may as well set the time to 0. We only need to treat it like a queued connection. Putting it to a real value makes it look like a far bigger hack than it really is, plus it makes it slower for no reason</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm starting to think we should have a utils singleton in kcoreaddons somewhere with</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Q_INVOKABLE void invokeQueuedMethod(QObject *target, QString slot) {<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
QMetaObject::invokeMethod(target, slot, Qt::QueuedConnection)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
}</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">it'd get rid of a lot timers and the code would be a bit more readable too. This isn't the first time and won't be the last time we'll want to do this. It's not very declarative, but neither is using timers at all.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thoughts?</p></pre>
</blockquote>
<p>On August 6th, 2014, 8:26 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;">could be nice..<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
maybe maybe there could be a way to pass parameters as well? maybe if the parameter is a qjsvalue it can be called with the syntax of a normal callback<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
kcoreAddons.invokeQueued(function("arg") { do stuff})<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
implementation would be a bit dirtier tough</p></pre>
</blockquote>
<p>On August 6th, 2014, 9:15 a.m. UTC, <b>David Edmundson</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;">We can do args on the above version; I avoided suggesting your version because I would have no idea how to do it :D<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
If you could manage it, that'd be super sexy.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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 <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">think</em> i know how would be done.. (not sure yet how make it async tough) only thing, the implementation would be quite ugly</p></pre>
<br />
<p>- Marco</p>
<br />
<p>On August 5th, 2014, 4:14 p.m. UTC, Marco Martin 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 Marco Martin.</div>
<p style="color: grey;"><i>Updated Aug. 5, 2014, 4:14 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</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;">one thing that was requested a lot as well, is an override on the screen locker package as is done in the splash screen as well (master already supports reading it from config, the kcm to set that was missing tough)</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>ksmserver/screenlocker/kcm/CMakeLists.txt <span style="color: grey">(17e4f70)</span></li>
<li>ksmserver/screenlocker/kcm/kcm.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>ksmserver/screenlocker/kcm/kcm.cpp <span style="color: grey">(d74ab75)</span></li>
<li>ksmserver/screenlocker/kcm/kcm.ui <span style="color: grey">(1cc2653)</span></li>
<li>ksmserver/screenlocker/kcm/package/contents/ui/main.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>ksmserver/screenlocker/kcm/package/metadata.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>lookandfeel/contents/lockscreen/screenshot.png <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119618/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/05/1174ed52-d86c-4345-9790-0cb5dd8f75ca__locker.png">locker.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>