Instead of always adding space for the sorting indicator in item view headers, only add it if sorting is enabled. Without this fix, operations such as QTreeView::resizeColumnToContents(..) will not result in a snug fit when the header section is wider than all items in the column.
Details
Details
- Reviewers
hpereiradacosta - Group Reviewers
Breeze - Commits
- R31:54e43f695d34: Fix header content size when sorting is disabled
#include <QApplication> #include <QTreeView> #include <QStandardItemModel> #include <QStandardItem> int main(int argc, char *argv[]) { QApplication app(argc, argv); // Test model QStandardItemModel model(3, 2); model.setHorizontalHeaderLabels({ "Header 1", "Header 2" }); for (int row = 0; row < 3; ++row) { for (int column = 0; column < 2; ++column) { model.setItem(row, column, new QStandardItem("Foo")); } } // View with sorting disabled QTreeView view; view.setWindowTitle("Sorting Disabled"); view.setModel(&model); view.show(); view.resizeColumnToContents(0); // View with sorting enabled QTreeView viewWithSorting; viewWithSorting.setWindowTitle("Sorting Enabled"); viewWithSorting.setModel(&model); viewWithSorting.setSortingEnabled(true); viewWithSorting.show(); viewWithSorting.resizeColumnToContents(0); return app.exec(); }
Notice how before applying this fix, there's space reserved for the sorting indicator even on the QTreeView that has sorting disabled.
Before the fix:
After the fix:
Diff Detail
Diff Detail
- Repository
- R31 Breeze
- Branch
- fix-unsortable-item-view-header-size (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
@cfeck: Right, and so does Fusion. After a long hiatus from KDE, this is the first time I use Phabricator (really like it!): Is a +1 enough to push, or is there a more formal approval mechanism?
Comment Actions
Thanks for the patch ! Ship it !
Also, it would be great if you could see if it also applies to oxygen
Comment Actions
@hpereiradacosta: The problem and fix was the same in Oxygen. I opened https://phabricator.kde.org/D4410 for that.