----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101219/#review2924 -----------------------------------------------------------
Ship it! That's a no brainer. Please, ship it. As a side note: where you placed the comment for KDE5, please add "###" and remove the note from the documentation that is generated (you could add this comment before the implementation on the cpp file). When working on KDE5 I will do a fast grep on ### and fix those issues (mostly public API that needs to be fixed). Thank you very much for the patch. - Rafael Fernández On April 24, 2011, 7:49 a.m., Eli MacKenzie wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101219/ > ----------------------------------------------------------- > > (Updated April 24, 2011, 7:49 a.m.) > > > Review request for kdelibs and Rafael Fernández López. > > > Summary > ------- > > Ideally KWidgetItemDelegate::createItemWidgets would take a const > QModelIndex& argument. Unfortunately its public API, so the signature cannot > be changed until KDE5. Additionally, its pure virtual so the option of adding > a createItemWidgets(const QModelIndex&) method won't work either. > > Instead, I've added a dynamic property called "creatingWidgetForIndex" to the > delegate, that is only present while the widget pool is creating widgets. > This means the only change kwidgetitemdelegate.h is documentation. The > QModelIndex could be stashed on one of the d-objects and made available > through an accessor, but that would mean that d-object would have to carry a > QModelIndex that would be invalid almost all of the time. > > Another benefit is that an application where this feature is desired (for KDE > < 4.7) only has to include an the new version of kwidgetitemdelegatepool.cpp, > which is easy to keep in sync with kdelibs. > > This change allows you to set a delegate for a particular column in the > model, and put a type of widget in each row that works best for the data. > > > Diffs > ----- > > kdeui/itemviews/kwidgetitemdelegate.h > 58dd60868f476d925f3abd53e67b22c1ed7149ac > kdeui/itemviews/kwidgetitemdelegatepool.cpp > b287584a594e97a091f96447386a588acb08c59e > > Diff: http://git.reviewboard.kde.org/r/101219/diff > > > Testing > ------- > > In my test app I started with a model that only included 5 rows, and would > only create a widget for row 2. As long as KWidgetItemDelegate::updateWidgets > doesn't assume that the QList<QWidget*> argument has something in it, all is > well. After implementing this change I caused the model to change the type of > widget for each row per the index pulled out of the property, again keeping > updateWidgets in sync, and have encountered no problems. > > > Screenshots > ----------- > > with no filtering > http://git.reviewboard.kde.org/r/101219/s/136/ > With filtering > http://git.reviewboard.kde.org/r/101219/s/137/ > > > Thanks, > > Eli > >
