<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://svn.reviewboard.kde.org/r/6401/">http://svn.reviewboard.kde.org/r/6401/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 25th, 2011, 6:59 a.m., <b>Kevin Ottens</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;">Sounds mostly good, I'm just wondering why you keep calling the updateRowRange in the slots connected to both rowsRemoved and rowsAboutToBeRemoved. Wouldn't doing it only once for one of those signals only be enough?</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 think it's needed because in the rowsAboutToBeRemoved you actually delete the widgets that belong to those rows (you pass the flag isRemoving = true). You need to do this before removing the rows because you need a valid index to find the widgets you want to remove. The other widgets can't be updated yet because the rows are not really removed and thus they would not be moved. You have to move those widgets after the rows have been deleted. (I hope you understand this poor explanation ;))</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 25th, 2011, 6:59 a.m., <b>Kevin Ottens</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="http://svn.reviewboard.kde.org/r/6401/diff/1/?file=44423#file44423line90" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegate.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KWidgetItemDelegatePrivate::_k_slotRowsRemoved(const QModelIndex &parent, int start, int end)</pre></td>
</tr>
</tbody>
<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">90</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew"> </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;">This whitespace should go away.</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;">Oops, will remove it</pre>
<br />
<p>- Thomas</p>
<br />
<p>On January 21st, 2011, 6:21 p.m., Thomas Richard wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 kdelibs, Kevin Ottens and Rafael Fernández López.</div>
<div>By Thomas Richard.</div>
<p style="color: grey;"><i>Updated Jan. 21, 2011, 6:21 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 a row other then the last got removed from a model, the widgets that are created by the itemdelegate are not moved. This is obviously a big problem when you start removing rows in between. The wrong widgets will show up in the wrong rows. This also happens when you insert a new row instead of appending it.
This patch will update the widgets position of the rows that come behind the one that is inserted/removed. I also had to change QModelIndex to QPersistentModelIndex at one place because the modelindex changes when your remove/insert a row in the middle and new widgets would get created.</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;">*Compiling
*Insert a row at the start/in the middle/at the end
*Remove a row at the start/in the middle/at the end
Everything seems to work now.</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>trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegate.h <span style="color: grey">(1214155)</span></li>
<li>trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegate.cpp <span style="color: grey">(1214155)</span></li>
<li>trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegate_p.h <span style="color: grey">(1214155)</span></li>
<li>trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegatepool_p.h <span style="color: grey">(1214155)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6401/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>