diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-08 10:35:06 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-10 08:23:23 +0200 |
| commit | 5487d581069333a64e0e0480f53f06a7b64e486d (patch) | |
| tree | 5471690779c57324cd40fec1a7369c846a9b8f34 | |
| parent | c80c060a593fb802f7149223d0d2f8495bc1f443 (diff) | |
| download | qtmaildir-5487d581069333a64e0e0480f53f06a7b64e486d.tar.gz qtmaildir-5487d581069333a64e0e0480f53f06a7b64e486d.zip | |
refactor(view): make ThreadListView a QTreeView for message rows
The strip survived the port because every geometry call it needs exists on both
classes. What did not survive is anything keyed on a row NUMBER: a tree numbers
rows per parent, so row 0 exists once per expanded thread and the old flat 0..N
walk would paint the first thread's strip over every one of them. The walk now
goes by index, and the alternating colour follows visual position rather than
index.row() for the same reason.
QTableView::isRowSelected(int) has no QTreeView equivalent; isSelected on the
index replaces it. MainWindow loses verticalHeader and selectRow, so row height
comes from uniformRowHeights and three helpers replace the row arithmetic.
next_thread and prev_thread now resolve the containing thread first: in a tree
current.row() + 1 is the next SIBLING, which under an expanded thread is the
next reply, not the next thread.
Two test defects found by mutation and worth recording, since both produced a
green suite over a broken assertion:
The indent test asserted on column 0. A QTreeView indents only the column
holding the expander, verified against Qt 6.11: with setTreePosition(4), column
0 reports the same left edge for a thread and its reply while column 4 reports
420 against 440. It was failing against a correctly indented tree.
The strip test passed with the view's skip deleted, because the real model
already returns no pills for a child row, so the view's guard was never the
thing under test. It now runs against a stub model that hands pills to every
row, which leaves the view's skip as the only thing that can keep replies clean.
That rewrite then failed for a third reason: without the delegates MainWindow
installs, rows take the default height, the band is measured against
SubjectDelegate::rowHeightFor and overflows into the row below, and the thread's
own strip paints across the reply. Reads exactly like a missing skip and is not
one.
| -rw-r--r-- | src/mainwindow.cpp | 85 | ||||
| -rw-r--r-- | src/mainwindow.h | 16 | ||||
| -rw-r--r-- | src/threadlistview.cpp | 45 | ||||
| -rw-r--r-- | src/threadlistview.h | 18 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 293 |
5 files changed, 372 insertions, 85 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 13406e4..740edbe 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -99,6 +99,40 @@ QString MainWindow::locksPath() return g_locksPath; } +/// The thread row containing an index: the index itself when it is already a +/// thread row, its parent when it is a message row. +/// +/// Replaces the arithmetic on row numbers that a table permitted. In a tree a +/// row number only identifies a row within one parent, so "current.row() + 1" +/// means the next SIBLING, which under an expanded thread is the next reply. +QModelIndex MainWindow::threadRowOf(const QModelIndex &index) const +{ + if (!index.isValid()) + return {}; + return index.parent().isValid() ? index.parent() : index; +} + +/// Selects a whole row, the way QTableView::selectRow did. +/// +/// QTreeView has no selectRow, and SelectRows on the selection model is not a +/// substitute: it governs what a click extends to, not what a programmatic +/// select() covers. +void MainWindow::selectRowAt(const QModelIndex &index) +{ + if (!index.isValid()) + return; + + m_threadView->selectionModel()->select( + index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + m_threadView->setCurrentIndex(index); +} + +/// Selects the top-level thread row at `row`. +void MainWindow::selectThreadRow(int row) +{ + selectRowAt(m_model->index(row, 0, QModelIndex())); +} + void MainWindow::restoreUiState() { QSettings state(uiStatePath(), QSettings::IniFormat); @@ -135,7 +169,7 @@ void MainWindow::restoreUiState() const int savedColumns = state.value(QStringLiteral("threadlist/columns")).toInt(); if (!header.isEmpty() && savedColumns == ThreadListModel::ColumnCount) { - m_threadView->horizontalHeader()->restoreState(header); + m_threadView->header()->restoreState(header); } // The config value is the starting point for a profile that has never @@ -154,7 +188,7 @@ void MainWindow::saveUiState() const state.setValue(QStringLiteral("window/state"), saveState()); state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState()); state.setValue(QStringLiteral("threadlist/header"), - m_threadView->horizontalHeader()->saveState()); + m_threadView->header()->saveState()); // Guards the blob above: see restoreUiState(). state.setValue(QStringLiteral("threadlist/columns"), int(ThreadListModel::ColumnCount)); @@ -511,16 +545,24 @@ void MainWindow::buildUi() m_threadView->setModel(m_model); m_threadView->setSelectionBehavior(QAbstractItemView::SelectRows); m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection); - m_threadView->verticalHeader()->hide(); - m_threadView->horizontalHeader()->setStretchLastSection(false); + m_threadView->header()->setStretchLastSection(false); // Every column Interactive, Subject included: Stretch and ResizeToContents // both compute a width and discard the user's drag. Nothing absorbs spare // width as a result, so the columns end where they end. for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { - m_threadView->horizontalHeader()->setSectionResizeMode( + m_threadView->header()->setSectionResizeMode( column, QHeaderView::Interactive); } + // The expander goes on the subject column, not on column 0. Column 0 is the + // narrow attachment marker, and an expander there has no room: it pushes the + // paperclip out of a 28px column entirely. + m_threadView->setTreePosition(ThreadListModel::SubjectColumn); + + // The root thread rows are the top level, so no decoration for them beyond + // the expander a thread with replies gets on its own. + m_threadView->setRootIsDecorated(true); + // Two delegates, and the split is not cosmetic. RowStyleDelegate carries // only the selection fix every column needs: the read/unread dimming // arrives as a Qt::ForegroundRole, which Qt's painting prefers over the @@ -535,11 +577,13 @@ void MainWindow::buildUi() m_threadView->setItemDelegateForColumn(ThreadListModel::SubjectColumn, new SubjectDelegate(this)); - // One height for every row, set here rather than left to a column's - // sizeHint: a QTableView takes a single height per row, so a hint from the - // subject column alone would only apply if the view happened to ask it. - m_threadView->verticalHeader()->setDefaultSectionSize( - SubjectDelegate::rowHeightFor(m_threadView->font())); + // One height for every row. A QTreeView has no vertical header to carry a + // default section size, so the height comes from uniformRowHeights plus the + // delegate's own sizeHint. uniformRowHeights is not merely an optimisation + // here: without it the tree measures every row separately and the tag strip, + // which is painted OUTSIDE any cell, is not accounted for in any of those + // measurements, so rows collapse to text height and the strip is clipped. + m_threadView->setUniformRowHeights(true); // Widening a column past the viewport scrolls rather than squeezing the // others. Per-pixel so the scroll does not jump a whole column at a time. // Banding, so the eye can follow a row across four columns and a pill @@ -555,7 +599,7 @@ void MainWindow::buildUi() // Without this the attachment column cannot be narrow at all: the default // minimum section size is 58px on this platform, and setColumnWidth() // clamps to it silently rather than reporting the smaller value back. - m_threadView->horizontalHeader()->setMinimumSectionSize(24); + m_threadView->header()->setMinimumSectionSize(24); m_threadView->setColumnWidth(ThreadListModel::AttachmentColumn, 28); m_threadView->setColumnWidth(ThreadListModel::FlagColumn, 28); m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130); @@ -645,16 +689,23 @@ void MainWindow::registerActions() }); addAction(QStringLiteral("next_thread"), tr("&Next thread"), tr("Select the next thread"), [this]() { + // The THREAD after this one, which is not "the next row" once replies + // are expanded: from a thread row the next row may be its own first + // reply, and from a reply row the row number counts siblings, not + // threads. Both are resolved by walking up to the containing thread + // first. const QModelIndex current = m_threadView->currentIndex(); - const int row = current.isValid() ? current.row() + 1 : 0; + const QModelIndex thread = threadRowOf(current); + const int row = thread.isValid() ? thread.row() + 1 : 0; if (row < m_model->rowCount()) - m_threadView->selectRow(row); + selectThreadRow(row); }); addAction(QStringLiteral("prev_thread"), tr("&Previous thread"), tr("Select the previous thread"), [this]() { const QModelIndex current = m_threadView->currentIndex(); - if (current.isValid() && current.row() > 0) - m_threadView->selectRow(current.row() - 1); + const QModelIndex thread = threadRowOf(current); + if (thread.isValid() && thread.row() > 0) + selectThreadRow(thread.row() - 1); }); addAction(QStringLiteral("open_thread"), tr("&Open thread"), tr("Focus the thread list"), [this]() { @@ -1521,8 +1572,8 @@ void MainWindow::showThreadContextMenu(const QPoint &pos) // collapsing to the clicked row here would silently narrow a deliberate // multi-row selection to one. Right-clicking outside it selects that row // instead, which is what every other list does. - if (!m_threadView->selectionModel()->isRowSelected(index.row())) - m_threadView->selectRow(index.row()); + if (!m_threadView->selectionModel()->isSelected(index)) + selectRowAt(index); m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos)); } diff --git a/src/mainwindow.h b/src/mainwindow.h index d8401a3..992cdd6 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -41,7 +41,7 @@ class QAction; class QLineEdit; class QMenu; -class QTableView; +class ThreadListView; class QLabel; class QPushButton; class QComboBox; @@ -212,6 +212,15 @@ private: /// A missing or rejected blob leaves the buildUi() defaults in place. void restoreUiState(); + /// The thread row containing an index: itself for a thread row, its parent + /// for a message row. + QModelIndex threadRowOf(const QModelIndex &index) const; + + /// Selects a whole row. QTreeView has no selectRow of its own. + void selectRowAt(const QModelIndex &index); + + /// Selects the top-level thread row at `row`. + void selectThreadRow(int row); void saveUiState() const; void registerActions(); @@ -410,7 +419,10 @@ private: QLineEdit *m_queryEdit = nullptr; QueryCompleter *m_queryCompleter = nullptr; - QTableView *m_threadView = nullptr; + /// Its own type, not the QTreeView base. The strip painting and the + /// expander column are ThreadListView's, and holding the base here only + /// hid that from every reader. + ThreadListView *m_threadView = nullptr; /// Right-click menu for the thread list, holding the same QActions the /// menu bar does. diff --git a/src/threadlistview.cpp b/src/threadlistview.cpp index ef80e09..ebca4cc 100644 --- a/src/threadlistview.cpp +++ b/src/threadlistview.cpp @@ -27,7 +27,7 @@ void ThreadListView::paintEvent(QPaintEvent *event) { - QTableView::paintEvent(event); + QTreeView::paintEvent(event); if (!model()) return; @@ -43,18 +43,34 @@ void ThreadListView::paintEvent(QPaintEvent *event) const QFontMetrics metrics(pillFont); painter.setFont(pillFont); - // Only the rows actually on screen. Walking the whole model would paint - // thousands of strips outside the viewport on a large query. - const int first = rowAt(0); - const int last = rowAt(viewport()->height() - 1); - const int lastRow = last >= 0 ? last : model()->rowCount() - 1; + // Only the rows actually on screen, walked by INDEX rather than by row + // number. A tree numbers rows per parent, so row 0 exists once per expanded + // thread and the old flat 0..N walk would paint the first thread's strip + // over every one of them. + QModelIndex walk = indexAt(QPoint(0, 0)); + + // Counts the rows actually painted, for the alternating colour. In a tree + // that has to follow VISUAL position: row 0 under three different threads + // is three different stripes, and using index.row() would give all three + // the same one. + int visualRow = 0; + + for (; walk.isValid(); walk = indexBelow(walk), ++visualRow) { + const QRect rowRect = visualRect(walk); + if (rowRect.top() > viewport()->height()) + break; + + // No strip under a message row. The strip carries the THREAD's tags, so + // one under each reply would stripe the list and repeat identical tags + // down the whole expansion. + if (walk.parent().isValid()) + continue; - for (int row = qMax(0, first); row <= lastRow; ++row) { - const QModelIndex index = - model()->index(row, ThreadListModel::SubjectColumn); + const QModelIndex index = walk.siblingAtColumn( + ThreadListModel::SubjectColumn); - const int rowTop = rowViewportPosition(row); - const int height = rowHeight(row); + const int rowTop = rowRect.top(); + const int height = rowRect.height(); if (height <= 0) continue; @@ -86,9 +102,12 @@ void ThreadListView::paintEvent(QPaintEvent *event) if (background.isValid()) painter.fillRect(band, background.value<QBrush>()); - else if (selectionModel() && selectionModel()->isRowSelected(row)) + // isSelected on the index, not isRowSelected(int): a QTreeView has no + // such overload, and a row number alone cannot name a row in a tree + // anyway since it is only unique under one parent. + else if (selectionModel() && selectionModel()->isSelected(index)) painter.fillRect(band, palette().brush(QPalette::Highlight)); - else if (alternatingRowColors() && (row % 2)) + else if (alternatingRowColors() && (visualRow % 2)) painter.fillRect(band, palette().brush(QPalette::AlternateBase)); else painter.fillRect(band, palette().brush(QPalette::Base)); diff --git a/src/threadlistview.h b/src/threadlistview.h index 0b4eafc..520d610 100644 --- a/src/threadlistview.h +++ b/src/threadlistview.h @@ -18,7 +18,7 @@ #pragma once -#include <QTableView> +#include <QTreeView> /// The thread list, with a row-wide strip of tag chips under each row's cells. /// @@ -36,11 +36,23 @@ /// The cells confine themselves to the upper band so the lower one is free; /// SubjectDelegate::kRowPadding and rowHeightFor() are the shared measurements /// that keep the two halves agreeing. -class ThreadListView : public QTableView +/// +/// A QTreeView rather than a QTableView since item 20: a thread's replies are +/// child rows, and a table can neither indent nor expand. The strip survived +/// the port because every geometry call it needs (visualRect, +/// columnViewportPosition, indexAt, indexBelow) exists on both. What did NOT +/// survive is anything keyed on a row NUMBER: a tree numbers rows per parent, +/// so row 0 exists once per expanded thread and a flat 0..N walk paints the +/// first thread's strip over every one of them. The walk below goes by index. +/// +/// The strip is painted for THREAD rows only. It carries the thread's tags, so +/// one under each reply would stripe the list and repeat identical tags down +/// the whole expansion. +class ThreadListView : public QTreeView { Q_OBJECT public: - using QTableView::QTableView; + using QTreeView::QTreeView; protected: void paintEvent(QPaintEvent *event) override; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 5031ead..7b6e55d 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -38,6 +38,7 @@ #include <QSplitter> #include <QTableView> #include <QToolBar> +#include <QTreeView> #include <QTimer> #include "config.h" @@ -47,6 +48,7 @@ #include "notmuchworker.h" #include "tagchip.h" #include "threadlistmodel.h" +#include "threadlistview.h" /// MainWindow is mostly wiring, and the parts that need a real database are /// still verified manually. What is checked here is the action registry: the @@ -94,6 +96,8 @@ private slots: void theStatusBarFollowsTheSyncPhase(); void aSelectedReadThreadIsNotDimmedIntoTheHighlight(); void thePillRowSpansTheWholeWidthNotOneColumn(); + void childRowsAreIndentedUnderTheirThread(); + void noTagStripIsPaintedUnderAMessageRow(); void markAllReadIsDisabledUntilTheQueryFinishes(); void markAllReadActsOnEveryRowAndUndoesInOneStep(); void markAllReadDoesNothingWhenNothingIsUnread(); @@ -452,7 +456,7 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded() const Config config; MainWindow reopened(config); - auto *view = reopened.findChild<QTableView *>(); + auto *view = reopened.findChild<QTreeView *>(); QVERIFY(view); QCOMPARE(view->columnWidth(ThreadListModel::AttachmentColumn), 28); QCOMPARE(view->columnWidth(ThreadListModel::DateColumn), 130); @@ -503,6 +507,27 @@ void TestMainWindow::returnInTheQueryBarRunsTheQueryNotOpenThread() QVERIFY(!actionFired); } +/// Selects a top-level THREAD row, replacing QTableView::selectRow which a +/// QTreeView does not have. +/// +/// Not merely a rename: setCurrentIndex alone leaves the selection model empty, +/// and select() alone leaves current invalid, so every test asserting on either +/// would break in a different way. Both are set here, exactly as +/// QTableView::selectRow did. +static void selectThreadRow(QTreeView *view, int row) +{ + const QModelIndex index = view->model()->index(row, 0, QModelIndex()); + view->selectionModel()->select( + index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(index); +} + +/// The height of a top-level row, replacing QTableView::rowHeight(int). +static int threadRowHeight(QTreeView *view, int row) +{ + return view->visualRect(view->model()->index(row, 0, QModelIndex())).height(); +} + /// A thread summary carrying the tags a test needs. Enough to drive selection; /// nothing here touches a database. static ThreadSummary makeThread(const QString &id, const QStringList &tags) @@ -530,7 +555,7 @@ void TestMainWindow::thePillRowSpansTheWholeWidthNotOneColumn() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); ThreadSummary thread = makeThread(QStringLiteral("t1"), {}); @@ -576,7 +601,7 @@ void TestMainWindow::thePillRowSpansTheWholeWidthNotOneColumn() for (const QVariant &colour : colours) pillColours.insert(colour.value<QColor>().rgb()); - const int rowHeight = view->rowHeight(0); + const int rowHeight = threadRowHeight(view, 0); QVERIFY(rowHeight > 0); int chipPixels = 0; @@ -592,6 +617,174 @@ void TestMainWindow::thePillRowSpansTheWholeWidthNotOneColumn() "still confined to that cell rather than spanning the row"); } +void TestMainWindow::childRowsAreIndentedUnderTheirThread() +{ + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY2(view, "the thread list is not a QTreeView, so it cannot indent"); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); + + MessageNode first; + first.messageId = QStringLiteral("m0@example.org"); + first.threadId = QStringLiteral("t1"); + first.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t1"); + reply.from = QStringLiteral("A Replier <replier@example.org>"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t1"), { first, reply }); + + window.resize(1400, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + view->expand(root); + QApplication::processEvents(); + + // Measured on the TREE POSITION column, not on column 0. A QTreeView + // indents only the column carrying the expander, verified against Qt 6.11: + // with setTreePosition(4), column 0 reports the same left edge for a thread + // and its reply (0 and 0) while column 4 reports 420 and 440. Asserting on + // column 0 therefore fails against a perfectly indented tree. + const int treeColumn = ThreadListModel::SubjectColumn; + const QModelIndex rootCell = model->index(0, treeColumn, QModelIndex()); + const QModelIndex child = model->index(0, treeColumn, root); + QVERIFY(child.isValid()); + + // Guards before the claim: a probe that cannot see both rows can report + // anything it likes about their relative position. + QVERIFY2(view->visualRect(rootCell).height() > 0, + "the thread row has no height, so nothing about it is measurable"); + QVERIFY2(view->visualRect(child).height() > 0, + "the reply row has no height: it is collapsed or off-screen, and " + "an indent test against it would pass without drawing anything"); + + QVERIFY2(view->visualRect(child).left() > view->visualRect(rootCell).left(), + "the reply is not indented relative to its thread"); +} + +void TestMainWindow::noTagStripIsPaintedUnderAMessageRow() +{ + // The strip is a row-wide band of the THREAD's tags. Painted under every + // reply as well it would stripe the list and repeat identical tags down the + // whole expansion. + // + // TWO independent guards stop that, and this test is aimed at the SECOND: + // the model returns no pills for a child row, and the view skips child rows + // in its walk. Asserting against the real model tests only the first, and + // the view's guard can be deleted without the test noticing: verified by + // mutation, which passed with the skip removed. So the model is replaced + // here by one that hands out pills for EVERY row, thread and reply alike, + // leaving the view's own skip as the only thing that can keep the reply + // rows clean. + /// Hands out the same pills for a message row as for a thread row, which + /// the real model never does. Without this the view's skip is unobservable. + class PillsEverywhereModel : public ThreadListModel + { + public: + QVariant data(const QModelIndex &index, int role) const override + { + if (role == PillTagsRole) { + return QStringList{ QStringLiteral("mailing-list/SBo"), + QStringLiteral("signed") }; + } + if (role == PillColoursRole) { + return QVariantList{ QVariant::fromValue(QColor(Qt::magenta)), + QVariant::fromValue(QColor(Qt::cyan)) }; + } + return ThreadListModel::data(index, role); + } + }; + + PillsEverywhereModel model; + ThreadListView view; + view.setModel(&model); + view.setTreePosition(ThreadListModel::SubjectColumn); + view.setUniformRowHeights(true); + + // The delegates MainWindow installs, and not optional here. The strip's + // band is measured against SubjectDelegate::rowHeightFor; without the + // delegate the rows take the default height, the band overflows into the + // row below, and the thread's own strip paints across the reply. That + // reads exactly like a missing skip in the walk and is not one. + view.setItemDelegate(new RowStyleDelegate(&view)); + view.setItemDelegateForColumn(ThreadListModel::SubjectColumn, + new SubjectDelegate(&view)); + view.setColumnWidth(ThreadListModel::AttachmentColumn, 28); + view.setColumnWidth(ThreadListModel::FlagColumn, 28); + view.setColumnWidth(ThreadListModel::DateColumn, 130); + view.setColumnWidth(ThreadListModel::AuthorsColumn, 180); + view.setColumnWidth(ThreadListModel::SubjectColumn, 520); + + ThreadSummary thread = makeThread(QStringLiteral("t1"), {}); + thread.tags = QStringList{ QStringLiteral("mailing-list/SBo"), + QStringLiteral("signed") }; + model.appendBatch({ thread }); + + MessageNode first; + first.messageId = QStringLiteral("m0@example.org"); + first.threadId = QStringLiteral("t1"); + first.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t1"); + reply.depth = 1; + model.setThreadMessages(QStringLiteral("t1"), { first, reply }); + + view.resize(1400, 300); + view.show(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + const QModelIndex root = model.index(0, 0, QModelIndex()); + view.expand(root); + QApplication::processEvents(); + + const QModelIndex child = model.index(0, 0, root); + const QRect childRect = view.visualRect(child); + QVERIFY2(childRect.height() > 0, "the reply row is not on screen"); + + // The exact colours the stub supplies, so an antialiased edge of anything + // else cannot be counted as a pill. + QSet<QRgb> pillColours; + pillColours.insert(QColor(Qt::magenta).rgb()); + pillColours.insert(QColor(Qt::cyan).rgb()); + + QImage shot(view.viewport()->size(), QImage::Format_ARGB32); + shot.fill(Qt::transparent); + view.viewport()->render(&shot); + + // Guard proving the probe can see pills at all: the THREAD row must have + // them, or a zero count under the reply proves nothing about the reply. + const QRect rootRect = view.visualRect(root); + int threadPills = 0; + for (int y = rootRect.top(); y < qMin(rootRect.bottom(), shot.height()); ++y) { + for (int x = 0; x < shot.width(); ++x) { + if (pillColours.contains(shot.pixel(x, y) | 0xff000000)) + ++threadPills; + } + } + QVERIFY2(threadPills > 0, + "no pill pixels under the THREAD row either, so this probe cannot " + "tell a missing strip from a broken render"); + + int replyPills = 0; + for (int y = childRect.top(); y < qMin(childRect.bottom(), shot.height()); ++y) { + for (int x = 0; x < shot.width(); ++x) { + if (pillColours.contains(shot.pixel(x, y) | 0xff000000)) + ++replyPills; + } + } + + QCOMPARE(replyPills, 0); +} + void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() { // Read threads carry a dimmed Qt::ForegroundRole, blended against the @@ -607,7 +800,7 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); // Both rows READ, so both are dimmed and neither is bold: the only thing @@ -633,10 +826,10 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() // dimmed row switches it to the highlight's own text colour, so the two // rows MUST differ; comparing two identically-styled rows would pass // against a delegate that did nothing at all. - view->selectRow(0); + selectThreadRow(view, 0); QApplication::processEvents(); - const int rowHeight = view->rowHeight(0); + const int rowHeight = threadRowHeight(view, 0); QVERIFY(rowHeight > 0); QImage shot(view->viewport()->size(), QImage::Format_ARGB32); @@ -724,7 +917,7 @@ void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("mark_all_read")); QVERIFY(action); @@ -746,7 +939,7 @@ void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep() // One row selected, to prove the action ignores the selection rather than // acting on it. - view->selectRow(0); + selectThreadRow(view, 0); action->trigger(); @@ -831,7 +1024,7 @@ void TestMainWindow::markReadTimerRestartsRatherThanStacking() QVERIFY(model); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), @@ -841,13 +1034,13 @@ void TestMainWindow::markReadTimerRestartsRatherThanStacking() makeThread(QStringLiteral("t3"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(timer->isActive(), "no timer armed for an unread thread"); // Move on before it can fire. One timer stays armed, not three. - view->selectRow(1); + selectThreadRow(view, 1); QVERIFY(timer->isActive()); - view->selectRow(2); + selectThreadRow(view, 2); QVERIFY(timer->isActive()); // Exactly one timer exists at all, which is what "restarted, not stacked" @@ -867,7 +1060,7 @@ void TestMainWindow::markReadTimerIsNotArmedForAReadThread() QVERIFY(model); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("read"), @@ -875,16 +1068,16 @@ void TestMainWindow::markReadTimerIsNotArmedForAReadThread() makeThread(QStringLiteral("unread"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(!timer->isActive(), "armed a timer for an already-read thread"); // And the unread one still arms, so this is not "never arms". - view->selectRow(1); + selectThreadRow(view, 1); QVERIFY(timer->isActive()); // Moving back to a read thread disarms it again, rather than leaving the // previous thread's timer running to fire against the wrong row. - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY(!timer->isActive()); } @@ -909,12 +1102,12 @@ void TestMainWindow::markReadCanBeDisabled() QVERIFY(model); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(!timer->isActive(), "a negative mark_read_delay_ms must disable the timer"); @@ -1091,7 +1284,7 @@ void TestMainWindow::selectAllIsBoundAndSelectsEveryRow() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), @@ -1120,7 +1313,7 @@ void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1134,7 +1327,7 @@ void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer() // Sweep down as Shift+arrow does: current moves onto a row while the // selection already spans more than one. - view->selectRow(0); + selectThreadRow(view, 0); view->selectionModel()->select( model->index(1, 0), QItemSelectionModel::Select | QItemSelectionModel::Rows); @@ -1159,7 +1352,7 @@ void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1169,7 +1362,7 @@ void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer() makeThread(QStringLiteral("t2"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(timer->isActive(), "no timer armed for a single unread thread"); // Extend to a second row, as Shift+click would. @@ -1191,7 +1384,7 @@ void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1205,7 +1398,7 @@ void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain() QVERIFY(!timer->isActive()); // Back to one row, as a plain click would leave it. - view->selectRow(1); + selectThreadRow(view, 1); QVERIFY2(timer->isActive(), "collapsing back to one row did not resume mark-read"); @@ -1221,7 +1414,7 @@ void TestMainWindow::theStatusBarReportsAMultiRowSelection() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); QVERIFY2(status, "no status label to report into"); @@ -1246,7 +1439,7 @@ void TestMainWindow::clearSelectionBlanksThePaneAndDeselects() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), @@ -1256,7 +1449,7 @@ void TestMainWindow::clearSelectionBlanksThePaneAndDeselects() // and what CLAUDE.md requires: selectAll() on a fresh view emits no // currentRowChanged at all, so a test starting there passes against a // missing guard. - view->selectRow(0); + selectThreadRow(view, 0); QCOMPARE(view->selectionModel()->selectedRows().size(), 1); auto *action = window.findChild<QAction *>(QStringLiteral("clear_selection")); @@ -1302,13 +1495,13 @@ void TestMainWindow::clearPaneLeavesTheSelectionAlone() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), makeThread(QStringLiteral("t2"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QCOMPARE(view->selectionModel()->selectedRows().size(), 1); auto *action = window.findChild<QAction *>(QStringLiteral("clear_pane")); @@ -1421,7 +1614,7 @@ void TestMainWindow::theThreadListOffersAContextMenu() const Config config; MainWindow window(config); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); QCOMPARE(view->contextMenuPolicy(), Qt::CustomContextMenu); @@ -1464,7 +1657,7 @@ void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1477,7 +1670,7 @@ void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird() { QStringLiteral("unread") }) }); // One row: ordinary reading, so a timer is armed and a thread is current. - view->selectRow(0); + selectThreadRow(view, 0); QCOMPARE(view->selectionModel()->selectedRows().size(), 1); QVERIFY(timer->isActive()); @@ -1829,13 +2022,13 @@ void TestMainWindow::escapeBlanksTheMessagePane() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), makeThread(QStringLiteral("t2"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(!window.currentThreadId().isEmpty(), "no thread was opened to blank"); @@ -1856,14 +2049,14 @@ void TestMainWindow::deleteTogglesOnAnAlreadyDeletedThread() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("delete")); QVERIFY(action); model->appendBatch({ makeThread(QStringLiteral("t1"), { QStringLiteral("deleted") }) }); - view->selectRow(0); + selectThreadRow(view, 0); action->trigger(); @@ -1884,7 +2077,7 @@ void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("delete")); QVERIFY(action); @@ -1935,7 +2128,7 @@ void TestMainWindow::theSelectionCountIsStateAndDoesNotExpire() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); QVERIFY(status); @@ -2059,13 +2252,13 @@ void TestMainWindow::anEditDuringABackgroundSyncIsNotSentYet() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY2(action, "no flag action registered"); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); // A cron sync takes the lock. QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", @@ -2090,13 +2283,13 @@ void TestMainWindow::aHeldEditIsSentWhenTheBackgroundSyncEnds() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, @@ -2125,7 +2318,7 @@ void TestMainWindow::aHeldEditCountsAsUnsynced() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); @@ -2134,7 +2327,7 @@ void TestMainWindow::aHeldEditCountsAsUnsynced() QVERIFY2(label->isHidden(), "the indicator starts hidden at zero"); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, @@ -2162,7 +2355,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); @@ -2173,7 +2366,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit() QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, SyncMonitor::State::Running)); - view->selectRow(0); + selectThreadRow(view, 0); action->trigger(); QVERIFY2(window.hasEditAwaitingSend(), "the edit was not held during a running sync, so this test is not " @@ -2189,7 +2382,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit() "platform without /proc/locks"); // And a NEW edit is sent rather than held. - view->selectRow(1); + selectThreadRow(view, 1); action->trigger(); QVERIFY2(!window.hasEditAwaitingSend(), "an unreadable lock table held a new edit, so writes never resume"); @@ -2206,7 +2399,7 @@ void TestMainWindow::aRejectedWriteKeepsEarlierUndoHistory() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *flag = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(flag); @@ -2215,7 +2408,7 @@ void TestMainWindow::aRejectedWriteKeepsEarlierUndoHistory() model->appendBatch({ makeThread(QStringLiteral("t1"), { QStringLiteral("inbox") }) }); - view->selectRow(0); + selectThreadRow(view, 0); // One edit that succeeds, so there is history worth keeping. archive->trigger(); |
