> On April 26, 2011, 10:08 p.m., Rafael Fernández López wrote:
> > Maybe I am misunderstanding the goal of the patch. I agree having this is 
> > cool, but it is not strictly necessary. In your use case, you can return 
> > the list of widgets in the createItemWidgets method with just one widget: 
> > the button. Afterwards, in updateItemWidgets you can hide or show it, 
> > depending on the index, in which you can check the row, column, or whatever 
> > property is needed.
> > 
> > The idea has always been to create a common set of widgets, and afterwards 
> > perform different actions depending on the index being drawn (in this case, 
> > checking column too).
> > 
> > Please, if this still doesn't work for you let me know. If this patch is 
> > really a need, I am not against accepting it, but only in the case that it 
> > is extremely necessary and there is no other way of performing the task you 
> > want to accomplish.

I don't think that is going to work very well, but I'll try writing it that 
way. My model has 4 columns and (in the common case) 20 children and 3 parents. 
Only one row actually needs widgets, but KWidgetItemDelegate has no way to find 
out whether it has been set as a single row or column delegate. There are 6 
different controls, so there will be 552 widgets when I really only need 20. In 
the extreme case there can be 104 child rows, same 3 parents, and 4 columns - 
so 2568 widgets instead of 104.


- Eli


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101219/#review2913
-----------------------------------------------------------


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.
> 
> 
> Thanks,
> 
> Eli
> 
>

Reply via email to