It converts a model's column headers into a simple list model. It is
mainly useful as a helper to create headers for QtQuick TableView but
may also be useful for other things.
Details
- Reviewers
davidedmundson - Commits
- R275:241e9dec6896: Add KColumnHeadersProxyModel
The added test QML file shows a single 1 as described in the test.
Diff Detail
- Repository
- R275 KItemModels
- Branch
- columnheadersmodel
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 18803 Build 18821: arc lint + arc unit
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.
src/core/kcolumnheadersproxymodel.h | ||
---|---|---|
30 | Worth clarifying that the list isn't across columns. i.e |
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 for it, so the mapping functions are probably not even called, until the source model changes, or you add a selection model on top.
- Forward column change singals as row change signals
- Forward header data change signals as dataChanged
- Add a unit test for KColumnHeadersProxyModel
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 intention. Fixed.
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.
Yeah I'm not entirely sure. If it was possible I would probably have based this on QTransposeProxyModel, but that is Qt 5.13+ stuff.
It works in the example as you implement the entire QAIM interface needed for it, so the mapping functions are probably not even called, until the source model changes, or you add a selection model on top.
Most of what I needed I needed to implement because QAbstractProxyModel does not reimplement them.
I don't think that would help much either, as that assumes you are mapping cells to cells, not cells to header data.
It works in the example as you implement the entire QAIM interface needed for it, so the mapping functions are probably not even called, until the source model changes, or you add a selection model on top.
Most of what I needed I needed to implement because QAbstractProxyModel does not reimplement them.
Right. My point is that if you'd inherit from QAbstractListModel and drop parent, index, columnCount and mapTo/FromSource you should end up with the same result. The sourceModel property might need to be added explicitly in that case though.
src/core/kcolumnheadersmodel.cpp | ||
---|---|---|
22 ↗ | (On Diff #70359) | Instead of using a nested class Private, consider using a non-nested KColumnHeadersModelPrivate class. |
Two more nitpicks while this has my attention (sadly no time/idea to comment on the general usefulness) :)
src/core/kcolumnheadersmodel.h | ||
---|---|---|
57 ↗ | (On Diff #70405) | For consistency I would prefer having a full Q_SIGNALS section: Q_SIGNALS: void sourceModelChanged(); That matches the usual pattern and helps when reading raw headers to learn about API of classes. |
60 ↗ | (On Diff #70405) | Why not QScopedPointer (for consistency in KF code)? Is that one going to be deprecated in favour of std::unique_ptr? |
src/core/kcolumnheadersmodel.h | ||
---|---|---|
57 ↗ | (On Diff #70405) | I actually prefer the combined style since it groups together all related bits regarding a single property. But I've changed it to a separate section now. |
60 ↗ | (On Diff #70405) | Mostly force of habit. I also see little point in using QScopedPointer nowadays, as it does not offer anything over std::unique_ptr. |
src/core/kcolumnheadersmodel.cpp | ||
---|---|---|
83 ↗ | (On Diff #70437) | connect(modelAboutToBeReset |