Review Request 124156: KItemModels: new proxy KRearrangeColumnsProxyModel

David Faure faure at kde.org
Tue Jun 23 14:55:58 UTC 2015



On June 23, 2015, 12:26 p.m., David Faure wrote:
> > Regarding the style. AStyle time i think ;)
> > There is lots of function( arg ) instead of function(arg). ?besides that, some places could use more line breaks for readability (marked above).
> > 
> > Some API questions.
> > setSourceColumns seems to be used for both rearranging and filtering with no way to change individual columns (that i see). Eg, to toogle them or show/hide columns on demand.
> > Would it make sense to add the following API functions as well?
> > 
> > // toggle the visibility of a column
> > void toggleColumnVisibility(int column)
> > {
> >     // pseudo code
> >     setColumnVisible(column, !isColumnVisible(column));
> > }
> > 
> > // set the column visible or hidden, depending on the bool.
> > void setColumnVisible(int column, bool visible)
> > 
> > // returns if a column is visible
> > bool isColumnVisible(int column)
> > 
> > Adding the above functionality would make it much more convenient to use in cases where you want to hide/show a column on the fly.
> 
> David Faure wrote:
>     Thanks for the reminder about astyle-kdelibs, forgot to do that. Done now.
>     
>     Toggle seems overkill (even QHeaderView doesn't have that), but the other two make sense. I'll add that (with QHeaderView naming).

Hmm, it gets a bit hairy.

If I do setSourceColumns(QVector<int>() << 2 << 1) and then setColumnHidden(2, true), I assume I'm hiding source-column-2, so the underlying vector becomes [1].
But if I now do setColumnHidden(2, false), where should column 2 reappear?
For the position to be kept, we would need (like QHeaderView) to distinguish reordering (visual<->logical mapping) and hiding (happens on top).
This makes the implementation more complex and error-prone IMHO. I think I'd rather let the program decide how it wants to handle that "vector of source columns". Maybe it's statically known (like in the first app I wrote this for), or it's replicating what a QHeaderView does at report-generation time (like in the app I'm writing this for right now, see FatCRM on github) (*), and only in a possible third case there would be a need for separating reordering and hiding...

(*) this reminds me, I wanted to add docu about how to replicate what QHeaderView does, using this proxy, but maybe this is only useful for use with KDReports...


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124156/#review81702
-----------------------------------------------------------


On June 23, 2015, 11:33 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124156/
> -----------------------------------------------------------
> 
> (Updated June 23, 2015, 11:33 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> -------
> 
> It supports reordering and hiding columns from the source model.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 4e604eeb6bd64529ec1fba983e3f58e2f8d0bd2c 
>   autotests/krearrangecolumnsproxymodeltest.cpp PRE-CREATION 
>   autotests/test_model_helpers.h PRE-CREATION 
>   src/CMakeLists.txt dc7d5f04c650f8d21784f16f8d7676e5c74c93e1 
>   src/krearrangecolumnsproxymodel.h PRE-CREATION 
>   src/krearrangecolumnsproxymodel.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124156/diff/
> 
> 
> Testing
> -------
> 
> Wrote it for a customer who allowed me to keep copyright and publish it under the LGPL.
> 
> Unit-tested, and used in two real projects.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150623/325b52fb/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list