D25311: Add KColumnHeadersProxyModel

This revision was automatically updated to reflect the committed changes. Closed by commit R275:241e9dec6896: Add KColumnHeadersProxyModel (authored by ahiemstra). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D25311?vs=70948&id=71204#toc REPOSITORY R275 KItemModels CHANGES SINCE LAST

D25311: Add KColumnHeadersProxyModel

ahiemstra added a comment. Sure, I can land it after saturday. REPOSITORY R275 KItemModels BRANCH columnheadersmodel REVISION DETAIL https://phabricator.kde.org/D25311 To: ahiemstra, davidedmundson Cc: kossebau, vkrause, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh

D25311: Add KColumnHeadersProxyModel

davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. Though maybe we should wait till after tagging (Saturday) ? REPOSITORY R275 KItemModels BRANCH columnheadersmodel REVISION DETAIL https://phabricator.kde.org/D25311

D25311: Add KColumnHeadersProxyModel

ahiemstra marked an inline comment as done. REPOSITORY R275 KItemModels REVISION DETAIL https://phabricator.kde.org/D25311 To: ahiemstra Cc: kossebau, vkrause, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25311: Add KColumnHeadersProxyModel

ahiemstra updated this revision to Diff 70948. ahiemstra added a comment. - Add missing layoutChanged/reset signals REPOSITORY R275 KItemModels CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25311?vs=70437&id=70948 BRANCH columnheadersmodel REVISION DETAIL https://phabricato

D25311: Add KColumnHeadersProxyModel

davidedmundson added inline comments. INLINE COMMENTS > kcolumnheadersmodel.cpp:83 > + > +if (newSourceModel) { > +connect(newSourceModel, > &QAbstractItemModel::columnsAboutToBeInserted, this, [this](const > QModelIndex&, int first, int last) { connect(modelAboutToBeReset modelRes

D25311: Add KColumnHeadersProxyModel

vkrause added a comment. In D25311#570700 , @ahiemstra wrote: > @vkrause Is the updated version more what you expected? Yep, thanks, this addresses my concerns. So +1 from me on this. REPOSITORY R275 KItemModels REVISION DETAIL http

D25311: Add KColumnHeadersProxyModel

ahiemstra marked an inline comment as done. ahiemstra added a comment. @vkrause Is the updated version more what you expected? REPOSITORY R275 KItemModels REVISION DETAIL https://phabricator.kde.org/D25311 To: ahiemstra Cc: kossebau, vkrause, davidedmundson, kde-frameworks-devel, LeGast0

D25311: Add KColumnHeadersProxyModel

ahiemstra marked an inline comment as done. ahiemstra added inline comments. INLINE COMMENTS > kossebau wrote in kcolumnheadersmodel.h:57 > For consistency I would prefer having a full Q_SIGNALS section: > > Q_SIGNALS: > void sourceModelChanged(); > > That matches the usual pattern and

D25311: Add KColumnHeadersProxyModel

ahiemstra updated this revision to Diff 70437. ahiemstra added a comment. - Use separate Q_SIGNALS for consistency REPOSITORY R275 KItemModels CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25311?vs=70405&id=70437 BRANCH columnheadersmodel REVISION DETAIL https://phabricator

D25311: Add KColumnHeadersProxyModel

kossebau added a comment. Two more nitpicks while this has my attention (sadly no time/idea to comment on the general usefulness) :) INLINE COMMENTS > kcolumnheadersmodel.h:57 > +void setSourceModel(QAbstractItemModel *newSourceModel); > +Q_SIGNAL void sourceModelChanged(); > + For

D25311: Add KColumnHeadersProxyModel

ahiemstra updated this revision to Diff 70405. ahiemstra added a comment. - Do not use nested Private class REPOSITORY R275 KItemModels CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25311?vs=70359&id=70405 BRANCH columnheadersmodel REVISION DETAIL https://phabricator.kde.or

D25311: Add KColumnHeadersProxyModel

kossebau added inline comments. INLINE COMMENTS > kcolumnheadersmodel.cpp:22 > + > +class KColumnHeadersModel::Private > +{ Instead of using a nested class Private, consider using a non-nested KColumnHeadersModelPrivate class. Otherwise you want to tag this class to not inherit the symbol visib

D25311: Add KColumnHeadersProxyModel

ahiemstra updated this revision to Diff 70359. ahiemstra added a comment. - Change to using QAbstractListModel as base REPOSITORY R275 KItemModels CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25311?vs=69807&id=70359 BRANCH columnheadersmodel REVISION DETAIL https://phabric

D25311: Add KColumnHeadersProxyModel

vkrause added a comment. In D25311#563011 , @ahiemstra wrote: > > I'm wondering if this is technically a proxy model, or rather a QAbstractListModel? Its content is not representing cells in the source model after all, which mapTo/FromSource as

D25311: Add KColumnHeadersProxyModel

ahiemstra marked an inline comment as done. ahiemstra added a comment. > We should probably have a unit test, even if it just covers the basic case. Done. > This currently assumes the source model's columns are static. At a minimum that needs a comment. Oops. That was not the in

D25311: Add KColumnHeadersProxyModel

ahiemstra updated this revision to Diff 69807. ahiemstra added a comment. - Add a unit test for KColumnHeadersProxyModel REPOSITORY R275 KItemModels CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25311?vs=69806&id=69807 BRANCH columnheadersmodel REVISION DETAIL https://phabr

D25311: Add KColumnHeadersProxyModel

ahiemstra updated this revision to Diff 69806. ahiemstra added a comment. - Forward column change singals as row change signals - Forward header data change signals as dataChanged - Add a unit test for KColumnHeadersProxyModel REPOSITORY R275 KItemModels CHANGES SINCE LAST UPDATE http

D25311: Add KColumnHeadersProxyModel

vkrause added a comment. I'm wondering if this is technically a proxy model, or rather a QAbstractListModel? Its content is not representing cells in the source model after all, which mapTo/FromSource assumes I think. It works in the example as you implement the entire QAIM interface needed

D25311: Add KColumnHeadersProxyModel

davidedmundson added a comment. We should probably have a unit test, even if it just covers the basic case. This currently assumes the source model's columns are static. At a minimum that needs a comment. INLINE COMMENTS > kcolumnheadersproxymodel.h:30 > + * > + * This model will expos

D25311: Add KColumnHeadersProxyModel

ahiemstra updated this revision to Diff 69765. ahiemstra added a comment. - Add @since for the class REPOSITORY R275 KItemModels CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25311?vs=69764&id=69765 BRANCH columnheadersmodel REVISION DETAIL https://phabricator.kde.org/D2531

D25311: Add KColumnHeadersProxyModel

ahiemstra created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ahiemstra requested review of this revision. REVISION SUMMARY It converts a model's column headers into a simple list model. It is mainly useful as a helper to create headers f