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
>
>
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
