summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-07 12:48:30 +0200
committerDanilo M. <danix@danix.xyz>2026-08-07 12:48:30 +0200
commitde884b036689b253d10ff48daa3a05cca20ba61d (patch)
tree1a5a5cf5020996d9f751b107e9fac9b9c466e40c
parent3826759d5167fb7a6f9449f9a39814c75771f449 (diff)
downloadqtmaildir-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>
-rw-r--r--CHANGELOG.md6
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md40
-rw-r--r--src/mainwindow.cpp11
-rw-r--r--src/tagchip.cpp43
-rw-r--r--src/tagchip.h8
-rw-r--r--src/threadlistmodel.cpp46
-rw-r--r--src/threadlistmodel.h9
-rw-r--r--tests/test_mainwindow.cpp55
-rw-r--r--tests/test_threadlistmodel.cpp86
9 files changed, 287 insertions, 17 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ef2b5be..096eb19 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,6 +30,12 @@ point at which they are stable.
until it exits, then a single summary line, so a run of over a minute was
silent and there was nothing for the status bar to report. This is not a
buffering problem and `stdbuf` does not help.
+- **Read threads are dimmed in the list, rather than unread being bold.** Bold
+ was the only thing distinguishing the two, and on some systems it renders
+ identically to regular, which is a Qt or fontconfig matter this application
+ cannot reach: read and unread mail looked exactly alike. The distinction now
+ rides on colour, with unread left at the palette's own text colour and read
+ receding toward the background. Bold is still applied where it works.
### Fixed
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index faf510a..21e7a91 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -446,14 +446,38 @@ to copy.
Two concrete sub-items, from using the list rather than looking at it:
- **"All items look unread (bold), maybe use regular for read items?"**
- **Check this before changing anything.** Bold is ALREADY conditional:
- `ThreadListModel::data()` sets it under `Qt::FontRole` only when
- `thread.isUnread()` (`src/threadlistmodel.cpp:148-152`). So either the
- observation is that the user's list genuinely is mostly unread, in which case
- there is no bug and the fix is elsewhere (the density work below), or bold is
- leaking onto read rows through some path the model does not control. Reproduce
- against a query with a known mix, e.g. `tag:inbox and not tag:unread`, before
- touching the font logic.
+ **Done 2026-08-07, and the guess written here was wrong on both branches.**
+
+ Bold was indeed already conditional, and the user's list was not mostly
+ unread, and bold was not leaking. The actual cause: **bold renders
+ identically to regular on the user's system.** Confirmed by eye against a
+ bare `QTableView` holding a plain `QStandardItemModel` with no qtmaildir code
+ involved, so the fault is in Qt or fontconfig, below this application, and
+ nothing in the model could ever have reached it. Bold was unread's ONLY cue.
+
+ The fix inverts the emphasis instead: unread rows keep the palette's text
+ colour and READ rows are dimmed toward the background, via
+ `ThreadListModel::readColour()`. The cue rides on `Qt::ForegroundRole`, costs
+ no column, and suits the real ratio, which was 99 unread against 4220 read.
+ Bold is kept, since it works on other systems, but nothing depends on it.
+
+ **A caution for anyone adding another `ForegroundRole` cue.** Qt resolves
+ that role 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 it landed as grey on the selection highlight,
+ near unreadable. `SubjectDelegate::initStyleOption` reverses that, and the
+ delegate is installed view-wide rather than on the subject column alone so
+ every column gets the same handling.
+
+ **How this was nearly missed twice.** It was first dismissed from thread
+ counts, which explained why two screenshots looked alike but said nothing
+ about rendering. It was then dismissed again by a probe that counted lit
+ pixels: antialiasing makes a bold and a regular glyph light a similar number,
+ so the metric read "identical" regardless. Text WIDTH distinguishes them
+ (277px against 288px for the same string) and a strict pixel diff does; an
+ ink count does not. The tests that now guard this strip the font from the
+ model's answer entirely and require the two states to still differ, which is
+ the assertion that was missing all along.
- **A star column for flagged threads**, mirroring the paperclip column that
already exists for attachments. `ThreadSummary` carries the tags and
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index bb11261..30de89c 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -514,9 +514,14 @@ void MainWindow::buildUi()
column, QHeaderView::Interactive);
}
- // The subject cell carries the account chip in front of its text.
- m_threadView->setItemDelegateForColumn(ThreadListModel::SubjectColumn,
- new SubjectDelegate(this));
+ // The subject cell carries the account chip in front of its text, and
+ // every cell needs the delegate's selection handling: the read/unread
+ // dimming arrives as a Qt::ForegroundRole, which Qt's default painting
+ // prefers over the highlight, leaving a selected read row grey on the
+ // selection colour. SubjectDelegate::initStyleOption reverses that, and
+ // its paint() falls through to the base class wherever there is no chip,
+ // so the other columns keep their ordinary rendering.
+ m_threadView->setItemDelegate(new SubjectDelegate(this));
// 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.
m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded);
diff --git a/src/tagchip.cpp b/src/tagchip.cpp
index 2e21419..1ac7623 100644
--- a/src/tagchip.cpp
+++ b/src/tagchip.cpp
@@ -49,6 +49,31 @@ void paint(QPainter *painter, const QRect &rect, const QString &text,
} // namespace TagChip
+void SubjectDelegate::initStyleOption(QStyleOptionViewItem *option,
+ const QModelIndex &index) const
+{
+ QStyledItemDelegate::initStyleOption(option, index);
+
+ // Qt resolves Qt::ForegroundRole into the palette's Text roles, and its
+ // own painting then prefers those over HighlightedText: a model that
+ // supplies a foreground wins even on a selected row.
+ //
+ // That is wrong for the read/unread dimming. A read thread's colour is
+ // blended against the UNSELECTED background, so over the selection
+ // highlight it lands as grey on purple, near unreadable. The highlight
+ // already says "this row", so the dimming can yield to it while selected.
+ //
+ // Doomed threads are unaffected in practice: their fill is drawn beneath
+ // the selection and their white is a contrast requirement, which
+ // HighlightedText also satisfies.
+ if (option->state & QStyle::State_Selected) {
+ const QColor highlighted =
+ option->palette.color(QPalette::HighlightedText);
+ option->palette.setColor(QPalette::Text, highlighted);
+ option->palette.setColor(QPalette::WindowText, highlighted);
+ }
+}
+
void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option,
const QModelIndex &index) const
{
@@ -87,13 +112,21 @@ void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &optio
return;
painter->save();
- // The model supplies the row's colours; honouring them keeps a deleted
- // thread white-on-red here as everywhere else.
+ // Selection outranks the model's colour, and that order matters. A read
+ // thread carries a dimmed foreground blended against the UNSELECTED
+ // background, so painting it over the highlight leaves grey-on-purple,
+ // which is close to unreadable. The highlight already carries the "this
+ // row" signal, so the read/unread distinction can yield to it for as long
+ // as the row is selected.
+ //
+ // A doomed thread is the exception that proves the rule: its white is not
+ // a dimming but a contrast requirement against its own fill, and the fill
+ // is drawn under the selection too.
const QVariant foreground = index.data(Qt::ForegroundRole);
- if (foreground.isValid())
- painter->setPen(foreground.value<QBrush>().color());
- else if (option.state & QStyle::State_Selected)
+ if (option.state & QStyle::State_Selected)
painter->setPen(option.palette.highlightedText().color());
+ else if (foreground.isValid())
+ painter->setPen(foreground.value<QBrush>().color());
else
painter->setPen(option.palette.text().color());
diff --git a/src/tagchip.h b/src/tagchip.h
index 9bd4e78..f43e3ff 100644
--- a/src/tagchip.h
+++ b/src/tagchip.h
@@ -59,4 +59,12 @@ public:
const QModelIndex &index) const override;
QSize sizeHint(const QStyleOptionViewItem &option,
const QModelIndex &index) const override;
+
+protected:
+ /// Makes the selection highlight outrank a model-supplied foreground.
+ ///
+ /// Qt's own resolution does the opposite, which leaves a dimmed read
+ /// thread painting grey over the selection colour.
+ void initStyleOption(QStyleOptionViewItem *option,
+ const QModelIndex &index) const override;
};
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index 2289e6c..7e0477d 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -20,6 +20,8 @@
#include <QBrush>
#include <QFont>
+#include <QGuiApplication>
+#include <QPalette>
#include <QFontDatabase>
#include <QFontMetrics>
@@ -57,6 +59,29 @@ QColor ThreadListModel::spamColour()
return QColor(0xa8, 0x5c, 0x18);
}
+QColor ThreadListModel::readColour()
+{
+ // Derived from the palette, never hardcoded: a fixed grey that reads as
+ // "quiet" on a light theme is nearly invisible on a dark one, which is the
+ // rule item 12 established for the message pane.
+ //
+ // Mixed toward the background rather than simply made transparent, so it
+ // composites the same over a selected row as over an unselected one.
+ const QPalette palette = QGuiApplication::palette();
+ const QColor text = palette.color(QPalette::Text);
+ const QColor background = palette.color(QPalette::Base);
+
+ // 0.55 of the text colour: clearly recessive beside an undimmed row, and
+ // still comfortably readable on its own. A read thread is not disabled,
+ // it is simply not the thing being pointed at.
+ constexpr qreal kWeight = 0.55;
+ const qreal inverse = 1.0 - kWeight;
+ return QColor::fromRgbF(
+ text.redF() * kWeight + background.redF() * inverse,
+ text.greenF() * kWeight + background.greenF() * inverse,
+ text.blueF() * kWeight + background.blueF() * inverse);
+}
+
ThreadListModel::ThreadListModel(QObject *parent)
: QAbstractTableModel(parent)
{
@@ -145,6 +170,27 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
return QBrush(QColor(Qt::white));
}
+ // Unread's cue, and it deliberately does NOT rely on the bold below.
+ //
+ // Bold was the only cue until 2026-08-07, when it turned out to render
+ // identically to regular on the user's system: confirmed with a bare
+ // QTableView and a plain QStandardItemModel, so the fault is in Qt or
+ // fontconfig, below this application, and nothing here can reach it.
+ //
+ // So the emphasis is inverted instead. Unread rows are left at the
+ // palette's own text colour, and READ rows are dimmed toward the
+ // background. That way the cue rides on ForegroundRole, which the delegate
+ // already honours, and it costs no column. It also suits the real ratio:
+ // with a few dozen unread among thousands read, dimming the bulk is calmer
+ // than highlighting it.
+ //
+ // BELOW the doomed branch on purpose, and that ordering is the whole
+ // protection: a deleted or spam thread has already returned white text for
+ // this role above, and dimming it because it is also read would drop that
+ // to unreadable against the crimson. Do not hoist this.
+ if (role == Qt::ForegroundRole && !thread.isUnread())
+ return QBrush(readColour());
+
if (role == Qt::FontRole) {
QFont font;
bool styled = false;
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index ab9378e..152730f 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -75,6 +75,15 @@ public:
static QColor deletedColour();
static QColor spamColour();
+ /// The dimmed text colour a READ thread carries.
+ ///
+ /// Unread rows are left at the palette's own colour and read ones recede,
+ /// rather than unread being emphasised. Bold used to be the only cue and
+ /// cannot be relied on: on at least one system it renders identically to
+ /// regular, which is a Qt or fontconfig matter this application cannot
+ /// reach. Derived from the palette, never hardcoded.
+ static QColor readColour();
+
explicit ThreadListModel(QObject *parent = nullptr);
/// Supplies the account chip colours. Not owned; must outlive the model.
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()