Review Request: Fix KWidgetItemeDelegate to correctly update widget positions when a row gets removed

[Kevin Ottens]

> On Jan. 25, 2011, 6:59 a.m., Kevin Ottens wrote:
> > 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?
> 
> Thomas Richard wrote:
>     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 ;))

Ah right, I missed the fact that the flag was positionned differently in both calls. I assumed they were passing true in both cases. Feel free to ignore that part of the review then.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6401/#review9714
-----------------------------------------------------------


On Jan. 21, 2011, 6:21 p.m., Thomas Richard wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6401/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2011, 6:21 p.m.)
> 
> 
> Review request for kdelibs, Kevin Ottens and Rafael Fernández López.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegate.h 1214155 
>   trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegate.cpp 1214155 
>   trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegate_p.h 1214155 
>   trunk/KDE/kdelibs/kdeui/itemviews/kwidgetitemdelegatepool_p.h 1214155 
> 
> Diff: http://svn.reviewboard.kde.org/r/6401/diff
> 
> 
> Testing
> -------
> 
> *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.
> 
> 
> Thanks,
> 
> Thomas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110125/dddb5036/attachment.htm>