diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-07 12:48:30 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-07 12:48:30 +0200 |
| commit | de884b036689b253d10ff48daa3a05cca20ba61d (patch) | |
| tree | 1a5a5cf5020996d9f751b107e9fac9b9c466e40c /tests | |
| parent | 3826759d5167fb7a6f9449f9a39814c75771f449 (diff) | |
| download | qtmaildir-de884b036689b253d10ff48daa3a05cca20ba61d.tar.gz qtmaildir-de884b036689b253d10ff48daa3a05cca20ba61d.zip | |
fix(ui): tell read threads from unread without relying on bold
Bold was unread's only cue, and it renders identically to regular on the
user's system: confirmed by eye against a bare QTableView holding a
plain QStandardItemModel, with no code from this project involved. The
fault is in Qt or fontconfig, below this application, and nothing in the
model could ever have reached it. Read and unread mail looked exactly
alike.
The emphasis is inverted instead. Unread rows keep the palette's own
text colour and read rows are dimmed toward the background, so the cue
rides on Qt::ForegroundRole, which the delegate already honours, and
costs no column. It also suits the real ratio, measured at 99 unread
against 4220 read: dimming the bulk is calmer than highlighting it. The
dim colour is derived from the palette, never hardcoded, per the rule
item 12 established. Bold is kept for systems where it works, but
nothing depends on it now.
That exposed a second defect, visible the moment it shipped. Qt resolves
ForegroundRole into the palette and then prefers it over
HighlightedText, so a model-supplied colour wins on a SELECTED row too.
The dim is blended against the unselected background, so a selected read
row painted grey on the selection colour, near unreadable.
SubjectDelegate::initStyleOption now reverses that, and the delegate is
installed view-wide rather than on the subject column alone, so every
column gets the same handling instead of three of them keeping Qt's
ordering.
The guarding tests state the property rather than the mechanism: strip
the font from the model's answer and the two states must still differ.
A test asserting only that bold is set passes on a system where bold
paints like regular, which is exactly how this survived. The selection
test renders two rows identical but for the unread tag, selects both,
and requires zero differing pixels.
Part of item 5; the density work and the star column remain.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_mainwindow.cpp | 55 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 86 |
2 files changed, 140 insertions, 1 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 1fdeaf2..1af95ed 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -81,6 +81,7 @@ private slots: void aSkippedLocalSyncStillReportsTheOtherRunFinishing(); void anUnobservableLockTableLeavesTheSyncButtonUsable(); void theStatusBarFollowsTheSyncPhase(); + void aSelectedReadThreadIsNotDimmedIntoTheHighlight(); void markAllReadIsDisabledUntilTheQueryFinishes(); void markAllReadActsOnEveryRowAndUndoesInOneStep(); void markAllReadDoesNothingWhenNothingIsUnread(); @@ -370,6 +371,60 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags) return thread; } +void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() +{ + // Read threads carry a dimmed Qt::ForegroundRole, blended against the + // UNSELECTED background. Qt's own painting prefers a model foreground over + // HighlightedText, so without SubjectDelegate::initStyleOption reversing + // that, selecting a read row paints it grey on the selection colour, which + // is close to unreadable. Seen in a screenshot before it was caught here. + // + // Rendered rather than asserted on roles: the model is right either way, + // and the defect lives entirely in how the delegate resolves them. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + + // Identical but for the unread tag, so any pixel difference between the + // two selected rows is the dimming leaking through. + ThreadSummary read = makeThread(QStringLiteral("t1"), {}); + ThreadSummary unread = + makeThread(QStringLiteral("t2"), { QStringLiteral("unread") }); + read.subject = unread.subject = QStringLiteral("Same subject both rows"); + read.authors = unread.authors = QStringLiteral("Someone <s@example.org>"); + model->appendBatch({ read, unread }); + + window.resize(900, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + view->selectAll(); + QApplication::processEvents(); + + const int rowHeight = view->rowHeight(0); + QVERIFY(rowHeight > 0); + + QImage shot(view->viewport()->size(), QImage::Format_ARGB32); + shot.fill(Qt::transparent); + view->viewport()->render(&shot); + + int differing = 0; + for (int y = 0; y < rowHeight && y + rowHeight < shot.height(); ++y) + for (int x = 0; x < shot.width(); ++x) + if (shot.pixel(x, y) != shot.pixel(x, y + rowHeight)) + ++differing; + + QVERIFY2(differing == 0, + qPrintable(QStringLiteral("a selected read row paints differently " + "from a selected unread one (%1 pixels): " + "the dimming is overriding the selection " + "highlight").arg(differing))); +} + void TestMainWindow::markAllReadIsDisabledUntilTheQueryFinishes() { // Threads arrive in batches, so acting mid-load would silently skip diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index a9cdb73..b880c29 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -33,6 +33,9 @@ private slots: void reportsSubjectAndAuthors(); void subjectShowsMessageCountOnlyForRealThreads(); void unreadThreadsRenderBold(); + void readThreadsAreDimmedAndUnreadAreNot(); + void theUnreadCueDoesNotDependOnFontWeight(); + void aDoomedThreadKeepsItsContrastEvenWhenRead(); void tagsAreTheFirstColumnAndSubjectTheLast(); void accountTagBecomesAChipLabel(); void unreadStylingSurvivesAnAccountChip(); @@ -164,6 +167,79 @@ void TestThreadListModel::unreadThreadsRenderBold() QVERIFY(unreadFont.value<QFont>().bold()); } +void TestThreadListModel::readThreadsAreDimmedAndUnreadAreNot() +{ + // Bold was unread's ONLY cue, and on the user's system it renders + // identically to regular: verified with a bare QTableView and a plain + // QStandardItemModel, so the fault is below this application, in Qt or + // fontconfig, and no model change can reach it. Bold is kept, since it + // works elsewhere, but the state can no longer depend on it. + // + // Read rows are dimmed instead, which inverts the emphasis: unread sits at + // full contrast and the bulk of a mostly-read list recedes. + ThreadListModel model; + ThreadSummary read = makeThread(QStringLiteral("t1"), QStringLiteral("read")); + read.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch( + { read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) }); + + const QVariant readFg = + model.data(model.index(0, ThreadListModel::SubjectColumn), + Qt::ForegroundRole); + const QVariant unreadFg = + model.data(model.index(1, ThreadListModel::SubjectColumn), + Qt::ForegroundRole); + + QVERIFY2(readFg.isValid(), "a read thread carries no dimming"); + QVERIFY2(!unreadFg.isValid(), + "an unread thread must be left at the palette's own colour, so it " + "is the one that stands out"); +} + +void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight() +{ + // The property that matters, stated directly: strip every font from the + // model's answer and the two states must still be distinguishable. A test + // asserting only that bold is set passes on a system where bold paints + // exactly like regular, which is precisely how this went unnoticed. + ThreadListModel model; + ThreadSummary read = makeThread(QStringLiteral("t1"), QStringLiteral("read")); + read.tags = QStringList{ QStringLiteral("inbox") }; + 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); + + QVERIFY2(readFg != unreadFg, + qPrintable(QStringLiteral("column %1 renders read and unread " + "identically once the font is " + "ignored").arg(column))); + } +} + +void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead() +{ + // Both cues write ForegroundRole, so they share one channel and the order + // matters. A deleted row forces white text onto its crimson fill; dimming + // it because it also happens to be read would drop that contrast to + // unreadable. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), + QStringLiteral("doomed and read")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + 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 @@ -325,9 +401,17 @@ void TestThreadListModel::ordinaryThreadsCarryNoRowColour() const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); - QVERIFY(!model.data(subject, Qt::ForegroundRole).isValid()); const QVariant font = model.data(subject, Qt::FontRole); QVERIFY(!font.isValid() || !font.value<QFont>().strikeOut()); + + // The foreground goes back to the dimming a read thread carries, NOT to + // nothing: this thread has no unread tag, so plain for it means dimmed. + // What matters is that the doomed white is gone. + const QVariant foreground = model.data(subject, Qt::ForegroundRole); + if (foreground.isValid()) { + QVERIFY2(foreground.value<QBrush>().color() != QColor(Qt::white), + "the doomed white text survived the undo"); + } } void TestThreadListModel::threadIdIsReachableFromAnIndex() |
