<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/104258/">http://git.reviewboard.kde.org/r/104258/</a>
</td>
</tr>
</table>
<br />
<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, Aaron J. Seigo, Marco Martin, and Fredrik Höglund.</div>
<div>By Ignat Semenov.</div>
<p style="color: grey;"><i>Updated March 16, 2012, 10:02 a.m.</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;">Discard the scrollbar position restore if the user has manually scrolled the view before the listing is finished.
The patch is for IconView only. There is a big problem with doing the same in ListView, actuallly, it is unnecessary there at all. This is why:
There is no multi-pass layout in ListView; moreover, when the user clicks the icon in the panel, and the listing starts, the popup won't open before the listing is finished. The user has no chance of scrolling the view before the listing is finished, so the problem doens't even exist there.
What botheres me is the Plasma stall on loading a big dir. I think we should port the multiple-pass layout code to the ListView class to ensure responsiveness, but that will be a different review request. Fredrik, what do you think?
This RR is finished now, I think it's ready to go. Please, do nitpick :)</pre>
</td>
</tr>
</table>
<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;">This patch implements scrolbar position saving on plasma exit. The change is fairly trivial, however, due to the fact that the view is not populated and layouted immediately simply scrolling to the desired position on creating the view does not work. Instead a signal is emitted on finishing the item layout, when the view has a valid size and the scrollbar has a valid range. The signal is connected to a slot which scrolls the view to the desired position and then disconnects the signal. For the user, a public function in AbstractItemView is introduced, which performs the connection.
The only problem is that ListView turned out not to have any layout method. It just paints the items one by one, calculating their position on the fly, so I put the signal at the end of updateScrollbar to ensure the scrollbar range is valid. Maybe it should go into the "if (max>0)" branch?</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 both the icon view and the list view, works fine.</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=261139">261139</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plasma/applets/folderview/abstractitemview.h <span style="color: grey">(aa68b90)</span></li>
<li>plasma/applets/folderview/abstractitemview.cpp <span style="color: grey">(3debb70)</span></li>
<li>plasma/applets/folderview/folderview.h <span style="color: grey">(4e441eb)</span></li>
<li>plasma/applets/folderview/folderview.cpp <span style="color: grey">(a94ce87)</span></li>
<li>plasma/applets/folderview/iconview.h <span style="color: grey">(12e93b3)</span></li>
<li>plasma/applets/folderview/iconview.cpp <span style="color: grey">(5c4e086)</span></li>
<li>plasma/applets/folderview/listview.cpp <span style="color: grey">(94efe44)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104258/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>