<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/104917/">http://git.reviewboard.kde.org/r/104917/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 11th, 2012, 2:36 p.m., <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;">I think is in the good direction, what would be cool is to insert rows in order of descendednt relevancy, even if a more relevant row arrives after the already existing ones</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;">I see the advantage of sorting by relevancy, but I am a bit afraid of going that way because:
- It changes the way rows are sorted in the model
- It is more complicated to get right
- From a user point of view, it means new items will get inserted between existing items. Thus existing items will move, making them much more difficult to target with the mouse
- A proxy model can be set up to sort the current model by relevancy. If other uses of the model require a different sorting, it feels suboptimal to sort by relevancy only to have another proxy model sort the items using another criteria</pre>
<br />
<p>- Aurélien</p>
<br />
<p>On May 11th, 2012, 2:13 p.m., Aurélien Gâteau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma and Aaron J. Seigo.</div>
<div>By Aurélien Gâteau.</div>
<p style="color: grey;"><i>Updated May 11, 2012, 2:13 p.m.</i></p>
<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;">When RunnerModel receives the matchesChanged() from RunnerManager, it resets itself and replace its copy of matches with the new one. This cause some flicker when the model is used with a QML ListView because the view clears itself each time the model is reset.
Ideally, RunnerManager should provide more fine-grained updates to RunnerModel, but for now I found the below patch to give good results. It basically checks if the new match list is equal to the old one plus some new items. If this is the case, it uses beginInsertRows() and endInsertRows() instead of resetting the model.
Note: the code needs a recent version of kdelibs KDE/4.8 branch because it requires the != operator for Plasma::QueryMatch, which I added earlier today.</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;">Tested with SAL-QML (currently in kde-workspace, in plasma/sreich/sal-qml). Gets lots of flickering because items created by the SAL-QML view fades in when added. Flickering goes away after the patch.</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>plasma/declarativeimports/krunnermodel/runnermodel.cpp <span style="color: grey">(7c5485b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104917/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>