aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mainwindow.cpp121
-rw-r--r--src/threadlistview.cpp243
-rw-r--r--src/threadlistview.h52
-rw-r--r--tests/test_mainwindow.cpp464
-rw-r--r--tests/test_threadlistmodel.cpp224
5 files changed, 311 insertions, 793 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 423aa7b..30148d4 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -42,7 +42,7 @@
#include <QSplitter>
#include <QStandardPaths>
#include <QStatusBar>
-#include <QTableView>
+#include <QScrollBar>
#include <QTimer>
#include <QToolBar>
#include <QVBoxLayout>
@@ -52,6 +52,8 @@
#include "mimeparser.h"
#include "notmuchworker.h"
#include "querycompleter.h"
+#include "carddelegate.h"
+#include "cardlayout.h"
#include "tagchip.h"
#include "tagdialog.h"
#include "threadlistmodel.h"
@@ -157,20 +159,9 @@ void MainWindow::restoreUiState()
m_splitter->restoreState(splitter);
}
- // A header blob saved against a different set of columns must be
- // discarded, not restored. QHeaderView::restoreState() returns TRUE for a
- // blob with fewer sections than the model and applies the old widths to
- // the wrong columns: adding the attachment column in front shifted every
- // saved width one place right, silently mangling the layout with no error
- // to detect it by (verified on Qt 6.11). The column count is stored
- // alongside and the blob is only used when it still matches.
- const QByteArray header = state.value(QStringLiteral("threadlist/header"))
- .toByteArray();
- const int savedColumns =
- state.value(QStringLiteral("threadlist/columns")).toInt();
- if (!header.isEmpty() && savedColumns == ThreadListModel::ColumnCount) {
- m_threadView->header()->restoreState(header);
- }
+ // No thread-list header state is read. The pane is one column drawn whole
+ // by CardDelegate, so there are no widths to restore; a blob saved by an
+ // older version is simply ignored (item 53's Upgrading note).
// The config value is the starting point for a profile that has never
// zoomed; once the user does, the state file is what they last had.
@@ -187,11 +178,6 @@ void MainWindow::saveUiState() const
state.setValue(QStringLiteral("window/geometry"), saveGeometry());
state.setValue(QStringLiteral("window/state"), saveState());
state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState());
- state.setValue(QStringLiteral("threadlist/header"),
- m_threadView->header()->saveState());
- // Guards the blob above: see restoreUiState().
- state.setValue(QStringLiteral("threadlist/columns"),
- int(ThreadListModel::ColumnCount));
state.setValue(QStringLiteral("message/zoom"), m_messageView->zoomFactor());
}
@@ -543,83 +529,42 @@ void MainWindow::buildUi()
// delegate is confined to one column's rectangle.
m_threadView = new ThreadListView(central);
m_threadView->setModel(m_model);
+ m_threadView->setItemDelegate(new CardDelegate(this));
+ m_threadView->setHeaderHidden(true);
m_threadView->setSelectionBehavior(QAbstractItemView::SelectRows);
m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection);
- 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->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);
-
- // No style-drawn branch decoration. SubjectDelegate draws the expander
- // itself, because drawBranches runs BEFORE the row's cells and the
- // delegate's own background paints straight over anything put there: a
- // 60-pixel triangle survived as 8 pixels, indistinguishable from the
- // near-invisible dot this replaces. Leaving both enabled would draw the
- // theme's dot underneath the delegate's glyph.
+ // No style-drawn branch decoration. CardDelegate draws the expander itself,
+ // because drawBranches runs BEFORE the row's cells and the delegate's own
+ // background paints straight over anything put there: a 60-pixel triangle
+ // once survived as 8. Leaving both enabled would draw the theme's dot
+ // underneath the delegate's glyph.
m_threadView->setRootIsDecorated(false);
- // Wider than Qt's default 20px, and the reason is specific rather than
- // aesthetic. A thread row carries an account chip in front of its subject
- // and a reply row does not, so a reply's text already starts roughly a
- // chip's width (~60px) to the LEFT of its thread's. At the default indent
- // the 20px shift is swallowed by that difference and the replies read as
- // flush with the thread, or even outdented. Verified against the running
- // app, not assumed: visualRect reported a correct 20px indent while the
- // rendered text showed none, because the geometry is indented and the
- // delegate then lays the text out from its own left edge.
- m_threadView->setIndentation(SubjectDelegate::kReplyIndent);
-
- // 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
- // highlight, leaving a selected read row grey on the selection colour.
- //
- // SubjectDelegate adds the account chip and the tag pills, and must go on
- // the subject column ALONE. It reads AccountLabelRole, a property of the
- // row rather than of a cell, so installed view-wide it draws the chip into
- // every column: tried once, and the list came out with a chip repeated
- // four times per row.
- m_threadView->setItemDelegate(new RowStyleDelegate(this));
- m_threadView->setItemDelegateForColumn(ThreadListModel::SubjectColumn,
- new SubjectDelegate(this));
+ // Zero, because CardLayout draws the indent itself. Qt's own indentation
+ // would shift the card's rect, and every rect on the card is measured from
+ // that rect's left edge, so the two would compound.
+ m_threadView->setIndentation(0);
// 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.
+ // default section size, so the height comes from uniformRowHeights plus
+ // CardDelegate::sizeHint.
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
- // strip without losing it. The colour comes from the palette's
- // AlternateBase, so it follows the desktop theme.
+
+ // Banding, so the eye can follow a card across the pane. The colour comes
+ // from the palette's AlternateBase, so it follows the desktop theme.
m_threadView->setAlternatingRowColors(true);
- m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded);
- m_threadView->setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel);
-
- // Starting widths only; a drag overrides them, and they are what the
- // saved-widths item will persist.
- // 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->header()->setMinimumSectionSize(24);
- m_threadView->setColumnWidth(ThreadListModel::AttachmentColumn, 28);
- m_threadView->setColumnWidth(ThreadListModel::FlagColumn, 28);
- m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130);
- m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180);
- m_threadView->setColumnWidth(ThreadListModel::SubjectColumn, 520);
+ // A card is exactly viewport width, so there is nothing to scroll to
+ // sideways. Turning the bar off is what closes item 51: a click used to
+ // scroll the list horizontally, because the subject column was wider than
+ // the viewport and auto-scroll brought the clicked index fully into view.
+ m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
+
+ // Scrolling a whole card at a time rather than a fraction of one, so a
+ // card is never left half above the top edge.
+ m_threadView->verticalScrollBar()->setSingleStep(
+ CardLayout::heightFor(m_threadView->font()));
// Replies are loaded when a thread is expanded, not with the query.
// Walking the reply tree of every thread in a 10k-thread result would cost
@@ -1061,7 +1006,7 @@ void MainWindow::buildMenus()
m_threadContextMenu->addAction(m_actions.value(QStringLiteral("select_all")));
m_threadView->setContextMenuPolicy(Qt::CustomContextMenu);
- connect(m_threadView, &QTableView::customContextMenuRequested,
+ connect(m_threadView, &QWidget::customContextMenuRequested,
this, &MainWindow::showThreadContextMenu);
// The frequent subset only. A toolbar holding every action is as
diff --git a/src/threadlistview.cpp b/src/threadlistview.cpp
index 1ffec04..5a3ce77 100644
--- a/src/threadlistview.cpp
+++ b/src/threadlistview.cpp
@@ -18,39 +18,34 @@
#include "threadlistview.h"
-#include "tagchip.h"
+#include "carddelegate.h"
#include "threadlistmodel.h"
#include <QMouseEvent>
-#include <QPaintEvent>
-#include <QPainter>
-#include <QScrollBar>
void ThreadListView::mousePressEvent(QMouseEvent *event)
{
const QModelIndex index = indexAt(event->pos());
- // Only a thread row, only the subject column, only the strip the delegate
- // reserved for the glyph. Anything wider would swallow clicks meant to
- // select the row, which is what the rest of the subject cell is for.
+ // The reply count IS the expander. Anything outside its rect selects the
+ // card and opens it, which is what the rest of the card is for.
if (event->button() == Qt::LeftButton && index.isValid()
- && !index.parent().isValid()
- && index.column() == ThreadListModel::SubjectColumn
- && index.data(ThreadListModel::HasRepliesRole).toBool()) {
-
- const QRect rect = visualRect(index);
- if (event->pos().x() >= rect.left()
- && event->pos().x() < rect.left() + SubjectDelegate::kExpanderWidth) {
- // Column 0, not the clicked index. Expansion state belongs to the
- // ROW, and QTreeView keys it on the first column: asking
- // isExpanded() about the subject-column index always answers false,
- // so every click expanded again instead of toggling.
- const QModelIndex row = index.siblingAtColumn(0);
- setExpanded(row, !isExpanded(row));
-
- // Swallowed, so the click that opened a thread does not also load
- // it into the message pane: expanding is a request to see the
- // thread's shape, not to read it.
+ && index.data(ThreadListModel::ReplyCountRole).toInt() > 0) {
+
+ QStyleOptionViewItem option;
+ initViewItemOption(&option);
+ option.rect = visualRect(index);
+ // State_Open decides which way the glyph points, and the rect is the
+ // same either way, but pass it so the layout sees the true state.
+ if (isExpanded(index))
+ option.state |= QStyle::State_Open;
+
+ if (CardDelegate::expanderRectFor(option, index)
+ .contains(event->pos())) {
+ setExpanded(index, !isExpanded(index));
+ // Swallowed, so expanding does not also load the thread into the
+ // message pane: it is a request to see the thread's shape, not to
+ // read it.
event->accept();
return;
}
@@ -58,203 +53,3 @@ void ThreadListView::mousePressEvent(QMouseEvent *event)
QTreeView::mousePressEvent(event);
}
-
-void ThreadListView::paintEvent(QPaintEvent *event)
-{
- QTreeView::paintEvent(event);
-
- if (!model())
- return;
-
- QPainter painter(viewport());
-
- // Two fonts, deliberately. The row's own font fixes where the text band
- // ends, and the pills are drawn a size smaller: at the same size they read
- // as a second row of content competing with the subject, rather than as
- // annotation beneath it.
- const QFontMetrics rowMetrics(font());
- const QFont pillFont = SubjectDelegate::pillFont(font());
- const QFontMetrics metrics(pillFont);
- painter.setFont(pillFont);
-
- // 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;
-
- // The spine's extent, collected across the reply rows and drawn once after
- // the loop. Per-row segments leave a gap at every row boundary and read as
- // a column of dashes rather than as one line.
- int spineX = -1;
- int spineTop = std::numeric_limits<int>::max();
- int spineBottom = std::numeric_limits<int>::min();
-
- for (; walk.isValid(); walk = indexBelow(walk), ++visualRow) {
- const QRect rowRect = visualRect(walk);
- if (rowRect.top() > viewport()->height())
- break;
-
- // A message row: no tag strip, but it does get the band filled to its
- // own tint and a thread line down its left.
- //
- // The band has to be filled here for the same reason a thread row's is.
- // The cells paint the tint per cell, so nothing covers the width to the
- // right of the last column or the lower band the strip normally
- // occupies, and an untouched reply row comes out tinted across its text
- // and bare underneath it.
- if (walk.parent().isValid()) {
- // Only the band BELOW the text, never the whole row. paintEvent
- // runs after the cells, so filling the row's full height paints
- // over the sender and subject the delegate just drew: measured at
- // zero surviving text pixels, a block of blank tinted rows.
- const int bandTop = rowRect.top() + SubjectDelegate::kRowPadding
- + rowMetrics.height();
- const QRect band(columnViewportPosition(ThreadListModel::DateColumn),
- bandTop,
- viewport()->width()
- - columnViewportPosition(
- ThreadListModel::DateColumn),
- rowRect.bottom() - bandTop + 1);
-
- if (selectionModel()
- && selectionModel()->isSelected(
- walk.siblingAtColumn(ThreadListModel::SubjectColumn))) {
- painter.fillRect(band, palette().brush(QPalette::Highlight));
- } else {
- painter.fillRect(band, ThreadListModel::replyBackground());
- }
-
- // The spine is NOT drawn here. Drawing it per row leaves a gap
- // wherever consecutive rows do not abut exactly, which is every row
- // boundary once the rows carry padding: the result reads as a
- // column of dashes rather than as one line. It is drawn as a single
- // continuous run after this loop, from the collected extents below.
- const int subjectLeft =
- columnViewportPosition(ThreadListModel::SubjectColumn);
- const int lineX = subjectLeft + SubjectDelegate::kExpanderWidth / 2;
-
- if (spineX < 0)
- spineX = lineX;
- spineTop = qMin(spineTop, rowRect.top());
- spineBottom = qMax(spineBottom, rowRect.bottom() + 1);
-
- // A stub out to the row, so each reply is visibly attached to the
- // spine rather than merely beside it. Drawn in the LOWER band, not
- // at the row's midpoint: the midpoint crosses the sender text, and
- // this paints after the cells.
- const int stubY = bandTop + (rowRect.bottom() - bandTop) / 2;
- painter.setPen(QPen(ThreadListModel::threadLineColour(), 2));
- painter.drawLine(lineX, stubY,
- subjectLeft + SubjectDelegate::kReplyIndent
- - TagChip::kSpacing * 2,
- stubY);
- continue;
- }
-
- const QModelIndex index = walk.siblingAtColumn(
- ThreadListModel::SubjectColumn);
-
- const int rowTop = rowRect.top();
- const int height = rowRect.height();
- if (height <= 0)
- continue;
-
- // The strip's band, filled to match the row before anything is drawn
- // on it.
- //
- // A QTableView paints alternating colours and the selection PER CELL,
- // so nothing paints the width to the right of the last column, and
- // nothing paints the band at all where a column does not reach. Left
- // unfilled, an alternate-coloured or selected row shows the viewport
- // background in a strip across its lower half. Filled for every
- // visible row, not only tagged ones, since an untagged row has the
- // same band to account for.
- // Starting at the date column, NOT at the viewport edge. The two
- // leading columns hold the attachment and flag glyphs, centred in the
- // full row height, so a band drawn over them cuts those glyphs in half.
- const int bandLeft =
- columnViewportPosition(ThreadListModel::DateColumn);
- const QRect band(bandLeft, rowTop + SubjectDelegate::kRowPadding
- + rowMetrics.height(),
- viewport()->width() - bandLeft,
- height - SubjectDelegate::kRowPadding
- - rowMetrics.height());
-
- // The model's own row colour wins where it has one: a deleted or spam
- // thread fills its cells with crimson or orange, and painting the base
- // colour across the band beneath them would cut the row in half.
- const QVariant background = index.data(Qt::BackgroundRole);
-
- if (background.isValid())
- painter.fillRect(band, background.value<QBrush>());
- // 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() && (visualRow % 2))
- painter.fillRect(band, palette().brush(QPalette::AlternateBase));
- else
- painter.fillRect(band, palette().brush(QPalette::Base));
-
- const QStringList tags =
- index.data(ThreadListModel::PillTagsRole).toStringList();
- if (tags.isEmpty())
- continue;
-
- const QVariantList colours =
- index.data(ThreadListModel::PillColoursRole).toList();
-
- // The band the cells leave free, below the text they draw in the
- // upper one. Measured from SubjectDelegate by both sides, so neither
- // can drift into the other's half. The row's own font metrics set the
- // text band; the strip's smaller font must not be used for it, or the
- // pills ride up over the date and sender.
- const int top = rowTop + SubjectDelegate::kRowPadding
- + rowMetrics.height() + TagChip::kSpacing;
-
- // Aligned with the first text column rather than the viewport edge:
- // the two leading columns are narrow markers for the attachment and
- // flag glyphs, and a strip starting at x=0 paints straight over them.
- // Indented past the date column's own left edge rather than flush with
- // it: a chip starting exactly where the column does reads as part of
- // the column rather than as a strip laid under the row.
- int x = columnViewportPosition(ThreadListModel::DateColumn)
- + TagChip::kSpacing * 2;
- const int available = viewport()->width() - TagChip::kSpacing;
-
- for (int i = 0; i < tags.size(); ++i) {
- const QSize size = TagChip::sizeFor(metrics, tags.at(i));
-
- // Stop rather than wrap or elide. A row that grew to fit its tags
- // would break the uniform height the list depends on, and half a
- // chip reads as a rendering fault.
- if (x + size.width() > available)
- break;
-
- const QColor colour = i < colours.size()
- ? colours.at(i).value<QColor>()
- : QColor(0x55, 0x55, 0x5f);
-
- TagChip::paint(&painter, QRect(x, top, size.width(), size.height()),
- tags.at(i), colour);
- x += size.width() + TagChip::kSpacing;
- }
- }
-
- // One continuous spine over every visible reply row, drawn last so no
- // cell fill can break it. Segments drawn per row left a dash at every row
- // boundary, which read as a dotted decoration rather than as the structure
- // holding the block together.
- if (spineX >= 0 && spineBottom > spineTop) {
- painter.setPen(QPen(ThreadListModel::threadLineColour(), 2));
- painter.drawLine(spineX, spineTop, spineX, spineBottom);
- }
-}
diff --git a/src/threadlistview.h b/src/threadlistview.h
index 0ebe3ea..3215153 100644
--- a/src/threadlistview.h
+++ b/src/threadlistview.h
@@ -20,34 +20,19 @@
#include <QTreeView>
-/// The thread list, with a row-wide strip of tag chips under each row's cells.
+/// The thread list.
///
-/// The strip is painted by the VIEW rather than by a delegate, and that is the
-/// whole reason this class exists. A delegate is handed one cell's rectangle
-/// and cannot paint outside its column, so pills drawn from the subject
-/// column's delegate stop at that column's edge, losing the last tags of a
-/// well-tagged thread, and start at that column's left edge, which puts them
-/// under the subject instead of under the row. Painting after the cells lets
-/// the strip run the full width, which is what the layout asks for:
+/// It exists for ONE reason now: the expander is drawn by CardDelegate, and a
+/// delegate gets no click of its own without an editor, so the view has to own
+/// the hit-test. Everything else it used to do is gone.
///
-/// [ date ][ from ][ subject ...................... ]
-/// [ pill ][ pill ][ pill ]
-///
-/// 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.
-///
-/// 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.
+/// Until item 53 this class also painted a row-wide strip of tag chips after
+/// the cells, because a delegate cannot paint outside its column and the strip
+/// spanned all five. With one column and one delegate painting the whole card,
+/// that reason is gone and so is the paintEvent, along with the two faults it
+/// kept producing: a deleted row cut in half, and every other row showing a
+/// bare stripe, both from the view having to re-honour alternating colours,
+/// the selection and BackgroundRole across cells it did not own.
class ThreadListView : public QTreeView
{
Q_OBJECT
@@ -55,16 +40,11 @@ public:
using QTreeView::QTreeView;
protected:
- void paintEvent(QPaintEvent *event) override;
-
- /// Toggles a thread when its expander glyph is clicked.
+ /// Toggles a thread when its reply count is clicked.
///
- /// The view owns this because the glyph is drawn by SubjectDelegate and a
- /// delegate gets no click of its own without an editor. Being VISIBLE and
- /// being CLICKABLE are separate properties: setRootIsDecorated(false),
- /// needed to stop the style drawing its own indicator underneath ours, also
- /// removed the style's hit area, so the expander painted correctly and did
- /// nothing at all.
+ /// Being VISIBLE and being CLICKABLE are separate properties:
+ /// setRootIsDecorated(false), needed to stop the style drawing its own
+ /// indicator underneath, also removed the style's hit area, so an expander
+ /// once painted correctly and did nothing at all.
void mousePressEvent(QMouseEvent *event) override;
-
};
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 740e7fa..d77cee3 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -46,6 +46,12 @@
#include "mainwindow.h"
#include "messageview.h"
#include "notmuchworker.h"
+#include "carddelegate.h"
+#include "cardlayout.h"
+
+#include <QImage>
+#include <QPainter>
+#include <QScrollBar>
#include "tagchip.h"
#include "threadlistmodel.h"
#include "threadlistview.h"
@@ -95,10 +101,9 @@ private slots:
void anUnobservableLockTableLeavesTheSyncButtonUsable();
void theStatusBarFollowsTheSyncPhase();
void aSelectedReadThreadIsNotDimmedIntoTheHighlight();
- void thePillRowSpansTheWholeWidthNotOneColumn();
void childRowsAreIndentedUnderTheirThread();
void aThreadWithRepliesDrawsAVisibleExpander();
- void noTagStripIsPaintedUnderAMessageRow();
+ void cardsNeverScrollSideways();
void replyRowsKeepTheirTextUnderTheThreadLine();
void clickingTheExpanderTogglesTheThread();
void selectingAMessageRowTargetsThatMessageNotItsThread();
@@ -451,11 +456,12 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded()
window.close();
}
- // Forge a state file from an older layout: same blob, wrong column count.
+ // Forge a state file from the five-column layout. Nothing reads these keys
+ // any more, and that is exactly what must be verified: a blob saved by an
+ // older version has to be ignored rather than applied to a one-column view.
{
QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat);
- state.setValue(QStringLiteral("threadlist/columns"),
- int(ThreadListModel::ColumnCount) - 1);
+ state.setValue(QStringLiteral("threadlist/columns"), 5);
state.setValue(QStringLiteral("threadlist/header"),
QByteArray("not a header this model could have saved"));
}
@@ -466,9 +472,7 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded()
auto *view = reopened.findChild<QTreeView *>();
QVERIFY(view);
- QCOMPARE(view->columnWidth(ThreadListModel::AttachmentColumn), 28);
- QCOMPARE(view->columnWidth(ThreadListModel::DateColumn), 130);
- QCOMPARE(view->columnWidth(ThreadListModel::SubjectColumn), 520);
+ QCOMPARE(view->model()->columnCount(), 1);
QFile::remove(MainWindow::uiStatePath());
QStandardPaths::setTestModeEnabled(false);
@@ -548,83 +552,6 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags)
return thread;
}
-void TestMainWindow::thePillRowSpansTheWholeWidthNotOneColumn()
-{
- // The pills are a row-wide strip under the cells, not content of the
- // subject cell. Drawn from the subject column's delegate they stop at that
- // column's edge, so a thread with several tags loses the last of them; and
- // they inherit the column's left edge, which puts them under the subject
- // rather than under the row.
- //
- // The property: pills appear to the LEFT of where the subject column
- // starts, which no per-cell delegate on that column could produce.
- const Config config;
- MainWindow window(config);
-
- auto *model = window.findChild<ThreadListModel *>();
- QVERIFY(model);
- auto *view = window.findChild<QTreeView *>();
- QVERIFY(view);
-
- ThreadSummary thread = makeThread(QStringLiteral("t1"), {});
- thread.tags = QStringList{ QStringLiteral("mailing-list/SBo"),
- QStringLiteral("signed") };
- model->appendBatch({ thread });
-
- window.resize(1400, 300);
- window.show();
- QVERIFY(QTest::qWaitForWindowExposed(&window));
- QApplication::processEvents();
-
- const int subjectLeft =
- view->columnViewportPosition(ThreadListModel::SubjectColumn);
- QVERIFY2(subjectLeft > 40,
- qPrintable(QStringLiteral("the subject column starts at x=%1, too "
- "close to the left edge to tell a "
- "row-wide strip from a subject-cell one")
- .arg(subjectLeft)));
- // The strip must have somewhere to paint that the subject cell does not
- // reach, or this test cannot fail.
- QVERIFY2(subjectLeft < view->viewport()->width(),
- qPrintable(QStringLiteral("the subject column is off-screen "
- "(x=%1, viewport %2), so nothing it "
- "draws is measurable")
- .arg(subjectLeft)
- .arg(view->viewport()->width())));
-
- QImage shot(view->viewport()->size(), QImage::Format_ARGB32);
- shot.fill(Qt::transparent);
- view->viewport()->render(&shot);
-
- // Count pixels matching the tag colours EXACTLY, not "saturated" pixels.
- // A looser test counts the antialiased edge of the selection highlight
- // blending into the background, which is several hundred distinct
- // near-background colours and passes whatever the strip does. Both earlier
- // versions of this test did precisely that.
- QSet<QRgb> pillColours;
- const QVariantList colours =
- model->index(0, ThreadListModel::SubjectColumn)
- .data(ThreadListModel::PillColoursRole).toList();
- QVERIFY2(!colours.isEmpty(), "the model supplied no pill colours");
- for (const QVariant &colour : colours)
- pillColours.insert(colour.value<QColor>().rgb());
-
- const int rowHeight = threadRowHeight(view, 0);
- QVERIFY(rowHeight > 0);
-
- int chipPixels = 0;
- for (int y = 0; y < qMin(rowHeight, shot.height()); ++y) {
- for (int x = 0; x < qMin(subjectLeft, shot.width()); ++x) {
- if (pillColours.contains(shot.pixel(x, y) | 0xff000000))
- ++chipPixels;
- }
- }
-
- QVERIFY2(chipPixels > 0,
- "no pill-coloured pixels left of the subject column: the strip is "
- "still confined to that cell rather than spanning the row");
-}
-
void TestMainWindow::childRowsAreIndentedUnderTheirThread()
{
const Config config;
@@ -661,14 +588,14 @@ void TestMainWindow::childRowsAreIndentedUnderTheirThread()
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);
+ // One column, and setIndentation(0): Qt indents nothing, CardLayout draws
+ // the indent itself. So visualRect reports the SAME rect for a thread and
+ // its reply, and the indent has to be read off the layout rather than off
+ // the geometry. That is the trap CLAUDE.md records in reverse: there,
+ // visualRect reported an indent the text did not have; here it reports
+ // none while the text is indented.
+ const QModelIndex rootCell = model->index(0, 0, QModelIndex());
+ const QModelIndex child = model->index(0, 0, root);
QVERIFY(child.isValid());
// Guards before the claim: a probe that cannot see both rows can report
@@ -679,55 +606,84 @@ void TestMainWindow::childRowsAreIndentedUnderTheirThread()
"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");
-
- // The geometry being indented is NOT the same as the reply LOOKING
- // indented, and asserting only the former shipped a build with no visible
- // nesting at all. A thread row draws an account chip before its subject and
- // a reply row does not, so the reply's text starts about a chip's width to
- // the left of the thread's; at Qt's default 20px indent that difference
- // swallows the shift entirely.
+ // The indent is NOT in the geometry. setIndentation(0) means visualRect
+ // reports the same left edge for both rows, deliberately: CardLayout draws
+ // the indent inside the card's own rect. Asserting on visualRect here
+ // would fail against a perfectly indented list, which is the mirror of the
+ // trap CLAUDE.md records for item 20, where visualRect reported an indent
+ // the text did not have.
//
- // So the real property: where the TEXT lands. The reply's subject must
- // begin to the right of the thread's, which is what the eye reads as
- // nesting.
- const int chipWidth =
- TagChip::sizeFor(QFontMetrics(view->font()),
- model->data(rootCell, ThreadListModel::AccountLabelRole)
- .toString()).width();
- QVERIFY2(chipWidth > 0,
- "the thread row has no account chip, so this test cannot measure "
- "the offset it is meant to compensate for");
-
- const int threadTextLeft = view->visualRect(rootCell).left() + chipWidth;
- QVERIFY2(view->visualRect(child).left() > threadTextLeft,
+ // So the real property, as before: where the TEXT lands. It is read off
+ // the layout, which is what the delegate paints from.
+ CardLayout::Input threadIn;
+ threadIn.isMessage = false;
+ threadIn.depth = 0;
+ CardLayout::Input replyIn;
+ replyIn.isMessage = true;
+ replyIn.depth =
+ model->data(child, ThreadListModel::MessageDepthRole).toInt();
+ QVERIFY2(replyIn.depth > 0,
+ "the reply reports depth 0, so there is no nesting to measure");
+
+ const QRect rect = view->visualRect(rootCell);
+ const CardLayout threadCard =
+ CardLayout::compute(threadIn, rect, view->font());
+ const CardLayout replyCard =
+ CardLayout::compute(replyIn, rect, view->font());
+
+ QVERIFY2(replyCard.contentLeft > threadCard.contentLeft,
qPrintable(QStringLiteral("the reply's text starts at x=%1, not "
- "right of the thread's text at x=%2: the "
- "indent does not beat the account chip "
- "and the nesting is invisible")
- .arg(view->visualRect(child).left())
- .arg(threadTextLeft)));
+ "right of the thread's at x=%2: the "
+ "nesting is invisible")
+ .arg(replyCard.contentLeft)
+ .arg(threadCard.contentLeft)));
+
+ // And the spine that makes the nesting read as one block rather than as an
+ // arbitrary offset.
+ QCOMPARE(replyCard.spines.size(), replyIn.depth);
+}
+
+void TestMainWindow::cardsNeverScrollSideways()
+{
+ const Config config;
+ MainWindow window(config);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ // A long subject, so the guard below is not vacuous: this is exactly the
+ // content that used to make the subject column wider than the viewport.
+ model->appendBatch({ makeThread(
+ QStringLiteral("t1"),
+ QStringList{ QStringLiteral("inbox") }) });
+ QApplication::processEvents();
+
+ // Item 51: clicking a row used to scroll the list sideways, because the
+ // subject column was wider than the viewport and auto-scroll brought the
+ // clicked index fully into view. A card is exactly viewport width, so
+ // there is nowhere to scroll to.
+ QVERIFY2(view->visualRect(model->index(0, 0)).height() > 0,
+ "no card is drawn, so there is no layout to assert about");
+ QCOMPARE(view->horizontalScrollBar()->minimum(),
+ view->horizontalScrollBar()->maximum());
}
void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander()
{
// The expander is the ONLY thing saying a thread can be opened, and it took
- // four wrong attempts to get on screen, each of which looked correct in
- // code:
- //
- // - QTreeView::drawBranches, the documented hook, runs BEFORE the row's
- // cells, so with the expander on a content column the delegate's own
- // background paints over it. A 60-pixel triangle survived as 8.
- // - Sizing it from the row rather than the branch rect put most of it
- // outside that rect.
- // - Moving it into the delegate but calling it from only one of the two
- // branches left every real row without one, since every real row has an
- // account chip and takes the other branch.
+ // four wrong attempts to get on screen before item 53, each of which looked
+ // correct in code and none of which a geometry or role assertion could see.
+ // So this counts painted pixels.
//
- // None of those is visible to a test that asserts on geometry or on model
- // roles, so this one counts painted pixels of the palette colour the glyph
- // is drawn in.
+ // Painted through the DELEGATE rather than through viewport()->render().
+ // The viewport render returns a blank image here: CLAUDE.md records that it
+ // does so in several ordinary situations, and this test proved it again,
+ // reporting zero ink over a card the delegate demonstrably paints 2183
+ // pixels into. A probe that sees nothing cannot report on anything.
const Config config;
MainWindow window(config);
@@ -737,7 +693,7 @@ void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander()
QVERIFY(view);
// Two threads: one with replies, one without. The second is the control,
- // and without it a test that counts text pixels would pass on any row.
+ // and without it a test that counts ink would pass on any card.
ThreadSummary withReplies = makeThread(
QStringLiteral("t1"),
QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) });
@@ -748,61 +704,66 @@ void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander()
lone.totalCount = 1;
model->appendBatch({ withReplies, lone });
- window.resize(1400, 300);
- window.show();
- QVERIFY(QTest::qWaitForWindowExposed(&window));
- QApplication::processEvents();
-
- const QModelIndex first =
- model->index(0, ThreadListModel::SubjectColumn, QModelIndex());
- const QModelIndex second =
- model->index(1, ThreadListModel::SubjectColumn, QModelIndex());
-
- // Guards: both rows on screen, and the model agreeing about which has
- // replies. Without these a zero count could mean anything.
- QVERIFY2(view->visualRect(first).height() > 0, "the first row is not drawn");
- QVERIFY2(view->visualRect(second).height() > 0,
- "the control row is not drawn");
- QVERIFY(model->data(first, ThreadListModel::HasRepliesRole).toBool());
- QVERIFY(!model->data(second, ThreadListModel::HasRepliesRole).toBool());
-
- QImage shot(view->viewport()->size(), QImage::Format_ARGB32);
- shot.fill(Qt::transparent);
- view->viewport()->render(&shot);
-
- // The exact colour the glyph is filled with, matched exactly rather than by
- // a brightness threshold, which would count antialiased subject text.
- const QRgb glyph = view->palette().color(QPalette::Text).rgb();
-
- // Only the strip in front of the subject text, so the subject's own glyphs
- // cannot be counted. kExpanderWidth is the room the delegate reserves.
- const auto countGlyphPixels = [&](const QModelIndex &index) {
- const QRect rect = view->visualRect(index);
+ const QModelIndex first = model->index(0, 0, QModelIndex());
+ const QModelIndex second = model->index(1, 0, QModelIndex());
+
+ // Guards: the model agrees about which thread has replies, and only that
+ // one is offered an expander at all.
+ QCOMPARE(model->data(first, ThreadListModel::ReplyCountRole).toInt(), 2);
+ QCOMPARE(model->data(second, ThreadListModel::ReplyCountRole).toInt(), 0);
+
+ const QFont font = view->font();
+ const int height = CardLayout::heightFor(font);
+
+ const auto inkInExpander = [&](const QModelIndex &index) {
+ QImage shot(400, height, QImage::Format_ARGB32);
+ shot.fill(Qt::white);
+ QPainter painter(&shot);
+ QStyleOptionViewItem option;
+ option.rect = QRect(0, 0, 400, height);
+ option.font = font;
+ option.palette = QApplication::palette();
+ option.state = QStyle::State_Enabled;
+ CardDelegate delegate;
+ delegate.paint(&painter, option, index);
+ painter.end();
+
+ const QRect rect = CardDelegate::expanderRectFor(option, index);
int found = 0;
- for (int y = rect.top(); y < qMin(rect.bottom(), shot.height()); ++y) {
- for (int x = rect.left();
- x < qMin(rect.left() + SubjectDelegate::kExpanderWidth,
- shot.width());
+ for (int y = rect.top(); y <= rect.bottom() && y < shot.height(); ++y) {
+ for (int x = rect.left(); x <= rect.right() && x < shot.width();
++x) {
- if ((shot.pixel(x, y) | 0xff000000) == (glyph | 0xff000000))
+ if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu)
++found;
}
}
- return found;
+
+ // Guard on the probe itself: prove it can see the card's own text
+ // before trusting it about the expander. A probe that finds no ink
+ // anywhere reports "nothing was drawn" whatever the delegate did.
+ int anyInk = 0;
+ for (int y = 0; y < shot.height(); ++y)
+ for (int x = 0; x < shot.width(); ++x)
+ if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu)
+ ++anyInk;
+ return std::pair<int, int>(found, anyInk);
};
- const int drawn = countGlyphPixels(first);
- const int control = countGlyphPixels(second);
+ const auto [drawn, drawnAnywhere] = inkInExpander(first);
+ const auto [control, controlAnywhere] = inkInExpander(second);
- QVERIFY2(drawn > 12,
- qPrintable(QStringLiteral("only %1 expander pixels: the glyph is "
- "clipped or painted over, which is how "
- "it shipped as an invisible dot")
- .arg(drawn)));
+ QVERIFY2(drawnAnywhere > 0 && controlAnywhere > 0,
+ "the probe finds no ink on either card, so it cannot report on "
+ "the expander either");
- // The control must have none, or the count above is measuring something
- // every row draws.
- QCOMPARE(control, 0);
+ QVERIFY2(drawn > 12,
+ qPrintable(QStringLiteral("only %1 pixels in the expander's rect: "
+ "the reply count is clipped or painted "
+ "over").arg(drawn)));
+ QVERIFY2(control == 0,
+ qPrintable(QStringLiteral("a thread with no replies drew %1 "
+ "pixels where an expander would go")
+ .arg(control)));
}
void TestMainWindow::selectingAThreadRowNamesHowManyMessagesItStandsFor()
@@ -1110,7 +1071,7 @@ void TestMainWindow::clickingTheExpanderTogglesTheThread()
const QModelIndex root = model->index(0, 0, QModelIndex());
const QModelIndex subject =
- model->index(0, ThreadListModel::SubjectColumn, QModelIndex());
+ model->index(0, 0, QModelIndex());
const QRect rect = view->visualRect(subject);
// Guards: the row is drawn, it claims to have replies, and it starts
@@ -1119,11 +1080,15 @@ void TestMainWindow::clickingTheExpanderTogglesTheThread()
QVERIFY(model->data(subject, ThreadListModel::HasRepliesRole).toBool());
QVERIFY(!view->isExpanded(root));
- // Aimed at the glyph itself: the delegate reserves kExpanderWidth at the
- // left of the subject cell and centres the triangle in it.
- const QPoint hit(rect.left() + SubjectDelegate::kExpanderWidth / 2,
- rect.top() + SubjectDelegate::kRowPadding
- + QFontMetrics(view->font()).height() / 2);
+ // Aimed at the rect the delegate reports, not at one reconstructed here:
+ // the drawn target and the clickable one cannot drift if both come from
+ // the same call.
+ QStyleOptionViewItem option;
+ option.rect = rect;
+ option.font = view->font();
+ const QRect expander = CardDelegate::expanderRectFor(option, subject);
+ QVERIFY2(!expander.isEmpty(), "the card offers no expander to click");
+ const QPoint hit = expander.center();
QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit);
QApplication::processEvents();
@@ -1177,7 +1142,7 @@ void TestMainWindow::replyRowsKeepTheirTextUnderTheThreadLine()
QApplication::processEvents();
const QModelIndex child =
- model->index(0, ThreadListModel::AuthorsColumn, root);
+ model->index(0, 0, root);
const QRect rect = view->visualRect(child);
QVERIFY2(rect.height() > 0, "the reply row is not on screen");
@@ -1203,121 +1168,6 @@ void TestMainWindow::replyRowsKeepTheirTextUnderTheThreadLine()
.arg(textPixels)));
}
-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
@@ -1383,15 +1233,15 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
QVERIFY2(delegate, "the thread view has no styled delegate");
const QModelIndex index =
- model->index(0, ThreadListModel::SubjectColumn);
+ model->index(0, 0);
// initStyleOption is protected, so the resolved palette is reached the way
// the painter does: through a subclass that exposes it.
- struct Probe : SubjectDelegate {
- using SubjectDelegate::initStyleOption;
+ struct Probe : CardDelegate {
+ using CardDelegate::initStyleOption;
};
const auto *probe = static_cast<const Probe *>(
- static_cast<const SubjectDelegate *>(delegate));
+ static_cast<const CardDelegate *>(delegate));
probe->initStyleOption(&selected, index);
probe->initStyleOption(&unselected, index);
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index 2e3eede..49b8894 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -47,22 +47,20 @@ private slots:
void appendingEmptyBatchIsNoOp();
void clearResetsModel();
void reportsSubjectAndAuthors();
- void subjectShowsMessageCountOnlyForRealThreads();
+ void theReplyCountExcludesTheRootMessage();
void unreadThreadsRenderBold();
void readThreadsAreDimmedAndUnreadAreNot();
void flaggedThreadsShowAStar();
void pillTagsExcludeWhatTheRowAlreadyShows();
- void theStarColumnIsNarrowAndCarriesNoText();
void theUnreadCueDoesNotDependOnFontWeight();
void aDoomedThreadKeepsItsContrastEvenWhenRead();
- void tagsAreTheFirstColumnAndSubjectTheLast();
void accountTagBecomesAChipLabel();
void unreadStylingSurvivesAnAccountChip();
void accountChipUsesTheConfiguredColour();
void deletedThreadsAreRedAndStruckThrough();
- void attachmentColumnIsFirstAndMarksOnlyTaggedThreads();
+ void attachmentIsMarkedOnlyOnTaggedThreads();
void spamThreadsAreOrangeAndStruckThrough();
- void doomedStylingCoversEveryColumn();
+ void doomedStylingCoversTheWholeCard();
void ordinaryThreadsCarryNoRowColour();
void threadIdIsReachableFromAnIndex();
void invalidIndexesReturnNothing();
@@ -120,7 +118,7 @@ void TestThreadListModel::repliesBecomeChildRowsUnderTheirThread()
QCOMPARE(model.rowCount(root), 2);
const QModelIndex child =
- model.index(0, ThreadListModel::SubjectColumn, root);
+ model.index(0, 0, root);
QVERIFY(child.isValid());
QCOMPARE(model.parent(child), model.index(0, 0, QModelIndex()));
@@ -159,20 +157,17 @@ void TestThreadListModel::messageRowsShowTheirOwnSenderAndSubject()
QStringLiteral("Re: A subject")) });
const QModelIndex root = model.index(0, 0, QModelIndex());
- const QModelIndex authors =
- model.index(0, ThreadListModel::AuthorsColumn, root);
- const QModelIndex subject =
- model.index(0, ThreadListModel::SubjectColumn, root);
+ const QModelIndex reply = model.index(0, 0, root);
- QCOMPARE(model.data(authors, Qt::DisplayRole).toString(),
+ QCOMPARE(model.data(reply, ThreadListModel::SendersRole).toString(),
QStringLiteral("Bob <bob@example.org>"));
- QCOMPARE(model.data(subject, Qt::DisplayRole).toString(),
+ QCOMPARE(model.data(reply, ThreadListModel::SubjectRole).toString(),
QStringLiteral("Re: A subject"));
// No tag strip under a child row. The strip is a row-wide band carrying the
// THREAD's tags; one under every reply would stripe the list and repeat the
// same tags down the whole expansion.
- QVERIFY(model.data(subject, ThreadListModel::PillTagsRole)
+ QVERIFY(model.data(reply, ThreadListModel::PillTagsRole)
.toStringList().isEmpty());
}
@@ -397,10 +392,10 @@ void TestThreadListModel::rootRowsSurviveTheTreeConversion()
// A tree model reports its roots under an INVALID parent.
QCOMPARE(model.rowCount(QModelIndex()), 1);
- QCOMPARE(model.columnCount(QModelIndex()), ThreadListModel::ColumnCount);
+ QCOMPARE(model.columnCount(QModelIndex()), 1);
const QModelIndex root =
- model.index(0, ThreadListModel::SubjectColumn, QModelIndex());
+ model.index(0, 0, QModelIndex());
QVERIFY(root.isValid());
QVERIFY(!model.parent(root).isValid());
QCOMPARE(model.data(root, ThreadListModel::ThreadIdRole).toString(),
@@ -422,7 +417,7 @@ void TestThreadListModel::startsEmpty()
{
ThreadListModel model;
QCOMPARE(model.rowCount(), 0);
- QCOMPARE(model.columnCount(), ThreadListModel::ColumnCount);
+ QCOMPARE(model.columnCount(), 1);
}
void TestThreadListModel::appendsBatches()
@@ -467,21 +462,21 @@ void TestThreadListModel::reportsSubjectAndAuthors()
ThreadListModel model;
model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) });
- const QModelIndex authors = model.index(0, ThreadListModel::AuthorsColumn);
- QCOMPARE(model.data(authors, Qt::DisplayRole).toString(),
+ // One index, every field, by role. The card draws them all at once, so
+ // reading them through Qt::DisplayRole as five columns did is no longer
+ // possible: DisplayRole answers the subject alone.
+ const QModelIndex card = model.index(0, 0);
+ QCOMPARE(model.data(card, ThreadListModel::SendersRole).toString(),
QStringLiteral("Alice"));
+ QVERIFY(model.data(card, ThreadListModel::DateRole).toDateTime().isValid());
+ QCOMPARE(model.data(card, ThreadListModel::SubjectRole).toString(),
+ QStringLiteral("hello"));
- const QModelIndex date = model.index(0, ThreadListModel::DateColumn);
- QVERIFY(!model.data(date, Qt::DisplayRole).toString().isEmpty());
-
- // Tags are no longer a column; they reach the strip under the message
- // pane through a role instead.
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
- QCOMPARE(model.data(subject, ThreadListModel::TagsRole).toStringList(),
+ QCOMPARE(model.data(card, ThreadListModel::TagsRole).toStringList(),
QStringList({ QStringLiteral("inbox"), QStringLiteral("unread") }));
}
-void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads()
+void TestThreadListModel::theReplyCountExcludesTheRootMessage()
{
ThreadListModel model;
@@ -491,12 +486,18 @@ void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads()
multi.totalCount = 4;
model.appendBatch({ single, multi });
- QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn),
- Qt::DisplayRole).toString(),
- QStringLiteral("alone"));
- QCOMPARE(model.data(model.index(1, ThreadListModel::SubjectColumn),
- Qt::DisplayRole).toString(),
- QStringLiteral("group (4)"));
+ // The count used to be a "(4)" suffix on the subject. It is the expander
+ // on the card's second line now, and it counts REPLIES: totalCount
+ // includes the root message, which is the card itself.
+ QCOMPARE(model.data(model.index(0, 0),
+ ThreadListModel::ReplyCountRole).toInt(), 0);
+ QCOMPARE(model.data(model.index(1, 0),
+ ThreadListModel::ReplyCountRole).toInt(), 3);
+
+ // And the subject is bare, with no count spliced into it.
+ QCOMPARE(model.data(model.index(1, 0),
+ ThreadListModel::SubjectRole).toString(),
+ QStringLiteral("group"));
}
void TestThreadListModel::unreadThreadsRenderBold()
@@ -507,11 +508,11 @@ void TestThreadListModel::unreadThreadsRenderBold()
model.appendBatch({ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) });
const QVariant readFont =
- model.data(model.index(0, ThreadListModel::SubjectColumn), Qt::FontRole);
+ model.data(model.index(0, 0), Qt::FontRole);
QVERIFY(!readFont.isValid());
const QVariant unreadFont =
- model.data(model.index(1, ThreadListModel::SubjectColumn), Qt::FontRole);
+ model.data(model.index(1, 0), Qt::FontRole);
QVERIFY(unreadFont.isValid());
QVERIFY(unreadFont.value<QFont>().bold());
}
@@ -534,10 +535,10 @@ void TestThreadListModel::readThreadsAreDimmedAndUnreadAreNot()
{ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) });
const QVariant readFg =
- model.data(model.index(0, ThreadListModel::SubjectColumn),
+ model.data(model.index(0, 0),
Qt::ForegroundRole);
const QVariant unreadFg =
- model.data(model.index(1, ThreadListModel::SubjectColumn),
+ model.data(model.index(1, 0),
Qt::ForegroundRole);
QVERIFY2(readFg.isValid(), "a read thread carries no dimming");
@@ -559,16 +560,17 @@ void TestThreadListModel::flaggedThreadsShowAStar()
QStringLiteral("flagged") };
model.appendBatch({ plain, starred });
- const QString none =
- model.data(model.index(0, ThreadListModel::FlagColumn),
- Qt::DisplayRole).toString();
- const QString star =
- model.data(model.index(1, ThreadListModel::FlagColumn),
- Qt::DisplayRole).toString();
-
- QVERIFY2(none.isEmpty(), "an unflagged thread shows something in the column");
- QVERIFY2(!star.isEmpty(), "a flagged thread shows nothing");
- QCOMPARE(star, ThreadListModel::flagGlyph());
+ QVERIFY2(!model.data(model.index(0, 0),
+ ThreadListModel::IsFlaggedRole).toBool(),
+ "an unflagged thread reports itself flagged");
+ QVERIFY2(model.data(model.index(1, 0),
+ ThreadListModel::IsFlaggedRole).toBool(),
+ "a flagged thread does not report itself flagged");
+
+ // The glyph the delegate draws from that flag must be something a font can
+ // render: an unrenderable codepoint shows as tofu, which reads as
+ // breakage rather than as a mark.
+ QVERIFY(!ThreadListModel::flagGlyph().isEmpty());
}
void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows()
@@ -592,7 +594,7 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows()
model.appendBatch({ thread });
const QStringList pills =
- model.data(model.index(0, ThreadListModel::SubjectColumn),
+ model.data(model.index(0, 0),
ThreadListModel::PillTagsRole).toStringList();
QVERIFY2(pills.contains(QStringLiteral("SBo")), qPrintable(pills.join(',')));
@@ -622,29 +624,6 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows()
QCOMPARE(pills, sorted);
}
-void TestThreadListModel::theStarColumnIsNarrowAndCarriesNoText()
-{
- // A marker column, like the paperclip beside it: centred, and never
- // carrying the subject or anything else that would want width.
- ThreadListModel model;
- ThreadSummary starred = makeThread(QStringLiteral("t1"),
- QStringLiteral("starred"));
- starred.tags = QStringList{ QStringLiteral("flagged") };
- model.appendBatch({ starred });
-
- const QModelIndex index = model.index(0, ThreadListModel::FlagColumn);
- QCOMPARE(model.data(index, Qt::TextAlignmentRole).toInt(),
- int(Qt::AlignCenter));
-
- // The glyph is one character, whether it is the star or its fallback: a
- // column sized for a marker cannot hold a word.
- QCOMPARE(ThreadListModel::flagGlyph().size(), 1);
-
- // And it says what it means, for anyone who cannot tell the glyph apart
- // from the paperclip beside it.
- QVERIFY(!model.data(index, Qt::ToolTipRole).toString().isEmpty());
-}
-
void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight()
{
// The property that matters, stated directly: strip every font from the
@@ -657,17 +636,14 @@ void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight()
model.appendBatch(
{ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) });
- for (int column = 0; column < ThreadListModel::ColumnCount; ++column) {
- const QVariant readFg =
- model.data(model.index(0, column), Qt::ForegroundRole);
- const QVariant unreadFg =
- model.data(model.index(1, column), Qt::ForegroundRole);
+ const QVariant readFg =
+ model.data(model.index(0, 0), Qt::ForegroundRole);
+ const QVariant unreadFg =
+ model.data(model.index(1, 0), Qt::ForegroundRole);
- QVERIFY2(readFg != unreadFg,
- qPrintable(QStringLiteral("column %1 renders read and unread "
- "identically once the font is "
- "ignored").arg(column)));
- }
+ QVERIFY2(readFg != unreadFg,
+ "read and unread cards render identically once the font is "
+ "ignored");
}
void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead()
@@ -684,32 +660,11 @@ void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QCOMPARE(model.data(subject, Qt::ForegroundRole).value<QBrush>().color(),
QColor(Qt::white));
}
-void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast()
-{
- // Subject stretches to fill the view, so whatever sits after it is pushed
- // off-screen. Tags used to be there, which is why acting on a thread
- // looked like it did nothing: the only column that changed was invisible.
- QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1);
-
- ThreadListModel model;
- model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) });
- QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal,
- Qt::DisplayRole).toString(),
- QStringLiteral("Subject"));
-
- // No tags column at all: spelling out a dozen tags per row consumed most
- // of the list's width and was unreadable.
- for (int column = 0; column < ThreadListModel::ColumnCount; ++column) {
- QVERIFY(model.headerData(column, Qt::Horizontal, Qt::DisplayRole)
- .toString() != QStringLiteral("Tags"));
- }
-}
-
void TestThreadListModel::accountTagBecomesAChipLabel()
{
// The account tag is a different taxonomy from a functional one: which
@@ -721,7 +676,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel()
QStringLiteral("account-webmail-personal") };
model.appendBatch({ thread });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QCOMPARE(model.data(subject, ThreadListModel::AccountLabelRole).toString(),
QStringLiteral("webmail-personal"));
QVERIFY(model.data(subject, ThreadListModel::AccountColourRole)
@@ -732,7 +687,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel()
ThreadSummary untagged = makeThread(QStringLiteral("t2"), QStringLiteral("hi"));
untagged.tags = QStringList{ QStringLiteral("inbox") };
plain.appendBatch({ untagged });
- QVERIFY(plain.data(plain.index(0, ThreadListModel::SubjectColumn),
+ QVERIFY(plain.data(plain.index(0, 0),
ThreadListModel::AccountLabelRole).toString().isEmpty());
}
@@ -748,7 +703,7 @@ void TestThreadListModel::unreadStylingSurvivesAnAccountChip()
QStringLiteral("account-webmail-personal") };
model.appendBatch({ thread });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QVERIFY(!model.data(subject, ThreadListModel::AccountLabelRole)
.toString().isEmpty());
@@ -771,7 +726,7 @@ void TestThreadListModel::accountChipUsesTheConfiguredColour()
thread.tags = QStringList{ QStringLiteral("account-webmail-personal") };
model.appendBatch({ thread });
- QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn),
+ QCOMPARE(model.data(model.index(0, 0),
ThreadListModel::AccountColourRole).value<QColor>(),
QColor(QStringLiteral("#cc0000")));
}
@@ -783,7 +738,7 @@ void TestThreadListModel::deletedThreadsAreRedAndStruckThrough()
thread.tags = QStringList{ QStringLiteral("inbox") };
model.appendBatch({ thread });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid());
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
@@ -808,7 +763,7 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("spam") }, {});
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QCOMPARE(model.data(subject, Qt::BackgroundRole).value<QBrush>().color(),
ThreadListModel::spamColour());
QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut());
@@ -817,10 +772,12 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough()
QVERIFY(ThreadListModel::spamColour() != ThreadListModel::deletedColour());
}
-void TestThreadListModel::doomedStylingCoversEveryColumn()
+void TestThreadListModel::doomedStylingCoversTheWholeCard()
{
- // A cue on one column would vanish the moment that column scrolled out of
- // view, which is the bug this whole change exists to fix.
+ // The cue is on the card itself. It used to be asserted per column,
+ // because a cue on one column vanished the moment that column scrolled out
+ // of view; one column cannot scroll away, but the roles still have to be
+ // answered or a deleted card looks untouched.
ThreadListModel model;
ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed"));
thread.tags = QStringList{ QStringLiteral("inbox") };
@@ -828,13 +785,11 @@ void TestThreadListModel::doomedStylingCoversEveryColumn()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
- for (int column = 0; column < ThreadListModel::ColumnCount; ++column) {
- const QModelIndex index = model.index(0, column);
- QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(),
- qPrintable(QStringLiteral("column %1 has no background").arg(column)));
- QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(),
- qPrintable(QStringLiteral("column %1 is not struck through").arg(column)));
- }
+ const QModelIndex index = model.index(0, 0);
+ QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(),
+ "a deleted card has no background");
+ QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(),
+ "a deleted card is not struck through");
}
void TestThreadListModel::ordinaryThreadsCarryNoRowColour()
@@ -848,7 +803,7 @@ void TestThreadListModel::ordinaryThreadsCarryNoRowColour()
model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {});
model.applyTagChange(QStringLiteral("t1"), {}, { QStringLiteral("deleted") });
- const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn);
+ const QModelIndex subject = model.index(0, 0);
QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid());
const QVariant font = model.data(subject, Qt::FontRole);
QVERIFY(!font.isValid() || !font.value<QFont>().strikeOut());
@@ -871,7 +826,7 @@ void TestThreadListModel::threadIdIsReachableFromAnIndex()
model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")),
makeThread(QStringLiteral("t2"), QStringLiteral("two")) });
- const QModelIndex index = model.index(1, ThreadListModel::SubjectColumn);
+ const QModelIndex index = model.index(1, 0);
QCOMPARE(model.data(index, ThreadListModel::ThreadIdRole).toString(),
QStringLiteral("t2"));
}
@@ -889,7 +844,7 @@ void TestThreadListModel::invalidIndexesReturnNothing()
// the reset in clear() before data() ever sees it. data() still checks its
// own bounds, but that guard is unreachable defence, not something these
// assertions can falsify.
- QVERIFY(!model.index(0, ThreadListModel::ColumnCount).isValid());
+ QVERIFY(!model.index(0, 1).isValid());
QVERIFY(!model.index(5, 0).isValid());
QVERIFY(!model.index(-1, 0).isValid());
@@ -953,9 +908,9 @@ void TestThreadListModel::tagChangeSignalsExactlyTheChangedRow()
const QModelIndex bottomRight = changed.first().at(1).value<QModelIndex>();
QCOMPARE(topLeft.row(), 1);
QCOMPARE(bottomRight.row(), 1);
+ // One column, so the range is a single index: the card repaints whole.
QCOMPARE(topLeft.column(), 0);
- // The whole row repaints: unread state changes the font of every column.
- QCOMPARE(bottomRight.column(), ThreadListModel::ColumnCount - 1);
+ QCOMPARE(bottomRight.column(), 0);
}
void TestThreadListModel::tagChangeForUnknownThreadIsIgnored()
@@ -1006,12 +961,11 @@ void TestThreadListModel::modelPassesQtTester()
model.clear();
}
-void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads()
+void TestThreadListModel::attachmentIsMarkedOnlyOnTaggedThreads()
{
- // Leftmost, and narrow: the point is to see an attachment without opening
- // the thread, which only works if the column is never scrolled away.
- QCOMPARE(ThreadListModel::AttachmentColumn, 0);
-
+ // The mark is drawn on the card's second line by CardDelegate. What the
+ // model owes it is the flag and the glyph, which is what this asserts:
+ // the column that used to carry it is gone.
ThreadSummary plain = makeThread(QStringLiteral("t1"),
QStringLiteral("no attachment"));
ThreadSummary withFile = makeThread(QStringLiteral("t2"),
@@ -1024,13 +978,12 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads()
model.appendBatch({ plain, withFile });
const QModelIndex plainCell =
- model.index(0, ThreadListModel::AttachmentColumn);
+ model.index(0, 0);
const QModelIndex fileCell =
- model.index(1, ThreadListModel::AttachmentColumn);
+ model.index(1, 0);
- QVERIFY(model.data(plainCell, Qt::DisplayRole).toString().isEmpty());
- QCOMPARE(model.data(fileCell, Qt::DisplayRole).toString(),
- ThreadListModel::attachmentGlyph());
+ QVERIFY(!model.data(plainCell, ThreadListModel::HasAttachmentRole).toBool());
+ QVERIFY(model.data(fileCell, ThreadListModel::HasAttachmentRole).toBool());
// The glyph must be something a font can draw. An unrenderable codepoint
// shows as a tofu box, which reads as breakage rather than as a marker.
@@ -1040,11 +993,6 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads()
// have an attachment on hover.
QVERIFY(model.data(plainCell, Qt::ToolTipRole).toString().isEmpty());
QVERIFY(!model.data(fileCell, Qt::ToolTipRole).toString().isEmpty());
-
- // The header carries no text: a label would set a minimum width far wider
- // than the icon and defeat the narrow column.
- QVERIFY(model.headerData(ThreadListModel::AttachmentColumn, Qt::Horizontal,
- Qt::DisplayRole).toString().isEmpty());
}
void TestThreadListModel::modelHasOneColumn()