summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-07 16:26:22 +0200
committerDanilo M. <danix@danix.xyz>2026-08-07 16:26:22 +0200
commit39cbde74a560407e24b05e171df883421aa2153e (patch)
treea7390b036f57507b17aa4291511ce33d3f297f81
parentde884b036689b253d10ff48daa3a05cca20ba61d (diff)
downloadqtmaildir-39cbde74a560407e24b05e171df883421aa2153e.tar.gz
qtmaildir-39cbde74a560407e24b05e171df883421aa2153e.zip
feat(ui): show each thread's tags under its row
The thread list was uniform and cramped: every row one line tall, with nothing to say what a thread was about before opening it. Rows are now roughly double height, carrying a strip of tag chips beneath the text, with alternating row colours and a star column for flagged threads beside the existing paperclip. The strip is painted by the VIEW rather than by a delegate, which is why ThreadListView exists. A delegate is handed one cell's rectangle and cannot paint outside its column, so a strip drawn from the subject column stops at that column's edge, losing the last tags of a well-tagged thread, and starts at its left edge, putting the chips under the subject instead of under the row. Tags the row already shows another way are left out: inbox as structure, unread as the dimming, flagged as the star, attachment as the paperclip, and the account as the chip in the subject cell. Sorted, since notmuch's order is not guaranteed stable and a row whose chips reordered between repaints would flicker. Six defects were introduced and fixed on the way here, all of them one consequence: a QTableView paints per cell, and a row-wide strip is not a cell. SubjectDelegate installed view-wide drew the account chip into every column, since AccountLabelRole belongs to the row; it is split into RowStyleDelegate for every column and SubjectDelegate for the subject alone, with a Q_ASSERT guarding that. Row height returned from sizeHint did nothing, because a table takes one height per row. The strip painted from x=0 over the marker columns, via a protected viewportMargins() that returns 0. Measuring the text band and the strip with one font put the pills over the date. Alternating colours and the selection are per-cell too, so the band showed bare viewport background until the view filled it, honouring the model's own BackgroundRole first so a deleted row is not cut in half. And that fill spanned the full width, cutting the centred marker glyphs at their midpoint. Closes item 5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--CHANGELOG.md20
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md123
-rw-r--r--src/CMakeLists.txt1
-rw-r--r--src/mainwindow.cpp39
-rw-r--r--src/tagchip.cpp110
-rw-r--r--src/tagchip.h62
-rw-r--r--src/threadlistmodel.cpp97
-rw-r--r--src/threadlistmodel.h27
-rw-r--r--src/threadlistview.cpp140
-rw-r--r--src/threadlistview.h47
-rw-r--r--tests/test_mainwindow.cpp153
-rw-r--r--tests/test_threadlistmodel.cpp118
12 files changed, 836 insertions, 101 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 096eb19..1c1e87b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,15 @@ point at which they are stable.
### Added
+- **The thread list shows each thread's tags**, as small coloured chips in a
+ strip under the row, using the same colours as the message pane. Rows are
+ taller to make room, and alternate in colour so one can be followed across
+ the width. The strip spans the whole row rather than sitting inside the
+ subject column, so a well-tagged thread does not lose its last tags off the
+ edge. Tags the row already shows another way are left out: the account, the
+ flag, the attachment, and read state.
+- **A star column for flagged threads**, beside the existing attachment
+ paperclip.
- **Mark all read**, on the toolbar, the Message menu and `Ctrl+Shift+U`. It
acts on every thread in the current view rather than the selection, as one
write and one undo entry, so a single `Ctrl+Z` puts back a view of 400
@@ -30,12 +39,11 @@ 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.
+- **Read threads are dimmed in the list**, so unread mail stands out by colour
+ as well as by weight. Bold alone was the only distinction, which leaves
+ nothing to see when the desktop's own font is configured bold. Unread keeps
+ the palette's text colour and read recedes toward the background; bold still
+ applies on top.
### 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 21e7a91..8b3858b 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
@@ -53,7 +53,7 @@ taking that too literally.
| 2 | No way to see full message details (From/To/Cc/Subject) | information | M | **done** |
| 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | **done** |
| 4 | Message-pane font size does not survive restart | persistence | S | **done** |
-| 5 | Thread list is cramped, poor readability | presentation | S | open |
+| 5 | Thread list is cramped, poor readability | presentation | S | **done** |
| 6 | Opened message stays unread | behavior | S | **done** |
| 7 | HTML view should be default for HTML messages | behavior | XS | **done** (already worked) |
| 8 | No buttons or menu entries for archive, undo, etc | discoverability | M | **done** |
@@ -446,20 +446,36 @@ 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?"**
- **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.
+ **Resolved 2026-08-07. The cause was a misconfigured desktop font, not code.**
+
+ The user's Qt font was set to **Bold in qt6ct**, so every row rendered bold
+ and nothing could stand out. Bold in the model was working correctly the
+ whole time. Correcting the qt6ct setting fixed the original complaint on its
+ own.
+
+ **Read this before trusting any measurement in this file.** Three wrong
+ conclusions were reached before that came out, and the reasoning behind each
+ is worth keeping, because the same mistakes are easy to repeat.
+
+ 1. Dismissed from thread counts (99 unread against 4220 read), which
+ explained why two screenshots looked alike but said nothing about whether
+ bold rendered.
+ 2. Dismissed again by a probe counting lit pixels. Antialiasing makes a bold
+ and a regular glyph light a similar number, so the metric read "identical"
+ regardless of the truth. **Text width is the honest measure**: with the
+ font misconfigured both weights measured 277px, and once corrected they
+ measured 277px against 306px.
+ 3. Concluded that Qt or fontconfig was broken, from a bare `QTableView` with
+ a plain `QStandardItemModel` painting two rows identically. That test was
+ correct and its conclusion was wrong: the baseline font was already bold,
+ so `setBold(true)` genuinely changed nothing.
+
+ **The dimming was kept anyway**, and stands on its own merits rather than on
+ that mistaken diagnosis. `ThreadListModel::readColour()` dims READ rows
+ toward the background while unread keeps the palette's text colour. With 99
+ unread among 4220 read, dimming the bulk carries the list better than
+ emphasising the few, and it is a second cue that survives a font setting like
+ the one that caused this. Bold still applies on top.
**A caution for anyone adding another `ForegroundRole` cue.** Qt resolves
that role into the palette and then prefers it over `HighlightedText`, so a
@@ -469,21 +485,78 @@ Two concrete sub-items, from using the list rather than looking at it:
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.
+ **One test had to be rewritten when the font was corrected.**
+ `aSelectedReadThreadIsNotDimmedIntoTheHighlight` originally compared a
+ selected read row against a selected unread one and required them to paint
+ identically. That only held because every row was bold; with bold working,
+ the unread row differs legitimately. It now asserts the resolved palette
+ rather than pixels, which is the property the fix actually changes.
- **A star column for flagged threads**, mirroring the paperclip column that
already exists for attachments. `ThreadSummary` carries the tags and
`flagged` is an ordinary notmuch tag, so this needs no new worker query, the
same way item 15's paperclip did not. Keep it narrow: an icon column, no
- text.
+ text. **Done 2026-08-07**, as `FlagColumn` beside `AttachmentColumn`, using
+ the same glyph-with-ASCII-fallback pattern (`flagGlyph()`).
+
+### Built 2026-08-07, and what the layout cost
+
+The row is roughly doubled in height, with the tags shown as chips beneath the
+subject, alternating row colours, and the star column above.
+
+**The tag strip is painted by the VIEW, not by a delegate**, which is why
+`ThreadListView` exists at all. A delegate is handed one cell's rectangle and
+cannot paint outside its column, so a strip drawn from the subject column's
+delegate stops at that column's edge, losing the last tags of a well-tagged
+thread, and starts at that column's left edge, which puts it under the subject
+rather than under the row. The user asked for it under the whole row:
+
+```
+[ date ][ from ][ subject ...................... ]
+ [ pill ][ pill ][ pill ]
+```
+
+**Which tags appear.** Everything except `inbox`, `unread`, `flagged`,
+`attachment` and the account tag, since the row already shows those as
+structure, dimming, the star, the paperclip and the chip. Sorted, because
+notmuch's order is not guaranteed stable and a row whose chips reordered
+between repaints would flicker.
+
+**Six defects were introduced and fixed while building this**, every one of
+them a consequence of the same thing: a `QTableView` paints PER CELL, and a
+row-wide strip is not a cell. Worth listing, because each is easy to
+reintroduce.
+
+1. `SubjectDelegate` was installed view-wide to spread the selection fix
+ across every column. It reads `AccountLabelRole`, which belongs to the row,
+ so every column drew the account chip. Split into `RowStyleDelegate` (the
+ selection fix, every column) and `SubjectDelegate` (chip, subject column
+ only), with a `Q_ASSERT` guarding the latter.
+2. Row height was returned from `sizeHint`, which does nothing: a table takes
+ ONE height per row, so a hint from a single column applies only if the view
+ happens to ask that column. Set on the vertical header instead.
+3. The strip used `viewportMargins().left()`, which is 0, so it painted from
+ the viewport edge across the marker columns. It compiled because the method
+ is protected and the call was inside the subclass.
+4. The text band and the strip were measured with one font, so the pills rode
+ up over the date and sender.
+5. Alternating colours and the selection are painted per cell, so the strip's
+ band showed the bare viewport background as a stripe across every other
+ row. The view now fills that band itself, and must honour three cases: the
+ model's own `BackgroundRole` first (a deleted thread's fill would otherwise
+ be cut in half), then the selection, then the alternating colour.
+6. That fill spanned the full width, and the marker glyphs are centred in the
+ full row height, so its top edge cut the paperclip and star at their
+ midpoint. The band starts at the date column now.
+
+**A note on verifying any of this.** Several rendering probes written during
+this work returned results that were confidently wrong: counting "lit" pixels
+cannot tell bold from regular, since antialiasing lights a similar number
+either way, and `viewport()->render()` returned a blank image more than once.
+Text width distinguishes weights; a strict pixel diff distinguishes renders;
+an ink count distinguishes nothing. Two versions of the strip's own test passed
+under mutation before one was written that matched the exact chip colours the
+model supplies.
## 6. Opened message stays unread
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index a71a6f1..cae3bd4 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -11,6 +11,7 @@ add_library(qtmaildir_lib STATIC
tagdialog.cpp
tagstrip.cpp
threadlistmodel.cpp
+ threadlistview.cpp
mailsync.cpp
syncmonitor.cpp
threadcidmap.cpp
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 30de89c..77322bc 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -55,6 +55,7 @@
#include "tagchip.h"
#include "tagdialog.h"
#include "threadlistmodel.h"
+#include "threadlistview.h"
#include "version.h"
QStringList MainWindow::registeredActionNames() const
@@ -500,7 +501,10 @@ void MainWindow::buildUi()
// Thread list and message pane.
m_model = new ThreadListModel(this);
m_model->setTagColors(&m_tagColors);
- m_threadView = new QTableView(central);
+ // ThreadListView, not a plain QTableView: it paints the row-wide tag
+ // strip under each row's cells, which no delegate can do because a
+ // delegate is confined to one column's rectangle.
+ m_threadView = new ThreadListView(central);
m_threadView->setModel(m_model);
m_threadView->setSelectionBehavior(QAbstractItemView::SelectRows);
m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection);
@@ -514,16 +518,32 @@ void MainWindow::buildUi()
column, QHeaderView::Interactive);
}
- // 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));
+ // 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));
+
+ // One height for every row, set here rather than left to a column's
+ // sizeHint: a QTableView takes a single height per row, so a hint from the
+ // subject column alone would only apply if the view happened to ask it.
+ m_threadView->verticalHeader()->setDefaultSectionSize(
+ SubjectDelegate::rowHeightFor(m_threadView->font()));
// 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.
+ m_threadView->setAlternatingRowColors(true);
+
m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded);
m_threadView->setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel);
@@ -534,6 +554,7 @@ void MainWindow::buildUi()
// clamps to it silently rather than reporting the smaller value back.
m_threadView->horizontalHeader()->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);
diff --git a/src/tagchip.cpp b/src/tagchip.cpp
index 1ac7623..1f4ad79 100644
--- a/src/tagchip.cpp
+++ b/src/tagchip.cpp
@@ -40,7 +40,13 @@ void paint(QPainter *painter, const QRect &rect, const QString &text,
painter->setRenderHint(QPainter::Antialiasing, true);
painter->setPen(Qt::NoPen);
painter->setBrush(background);
- painter->drawRoundedRect(rect, kRadius, kRadius);
+ // Radius from the chip's own height rather than the fixed kRadius: a 3px
+ // corner on a 17px chip reads as a slightly-softened rectangle, which is
+ // hard to tell from the square cells of the columns behind it. Half the
+ // height gives fully rounded ends, so a chip reads as an object sitting on
+ // the row instead of as another compartment of it.
+ const qreal radius = rect.height() / 2.0;
+ painter->drawRoundedRect(rect, radius, radius);
painter->setPen(TagColors::textColourOn(background));
painter->drawText(rect, Qt::AlignCenter, text);
@@ -49,8 +55,44 @@ void paint(QPainter *painter, const QRect &rect, const QString &text,
} // namespace TagChip
-void SubjectDelegate::initStyleOption(QStyleOptionViewItem *option,
- const QModelIndex &index) const
+int SubjectDelegate::subjectBandHeight(const QStyleOptionViewItem &option)
+{
+ return QFontMetrics(option.font).height();
+}
+
+QFont SubjectDelegate::pillFont(const QFont &rowFont)
+{
+ QFont font = rowFont;
+
+ // Two points down, floored. One point was measured to change nothing at a
+ // 12pt desktop font: 12 and 11 both render 17px tall, so the pills came
+ // out the same size as the subject and read as competing content rather
+ // than as annotation.
+ //
+ // pointSize() is -1 when the font was specified in pixels, which
+ // subtracting from would be nonsense, hence the two branches.
+ if (rowFont.pointSize() > 0)
+ font.setPointSize(qMax(6, rowFont.pointSize() - 2));
+ else if (rowFont.pixelSize() > 0)
+ font.setPixelSize(qMax(8, rowFont.pixelSize() - 3));
+
+ return font;
+}
+
+int SubjectDelegate::rowHeightFor(const QFont &rowFont)
+{
+ // The text band uses the ROW's font and the strip its own smaller one.
+ // Measuring both with one font is what put the pills over the date text.
+ const QFontMetrics rowMetrics(rowFont);
+ const QFontMetrics pillMetrics(pillFont(rowFont));
+
+ return rowMetrics.height()
+ + TagChip::sizeFor(pillMetrics, QStringLiteral("x")).height()
+ + kRowPadding * 2 + TagChip::kSpacing;
+}
+
+void RowStyleDelegate::initStyleOption(QStyleOptionViewItem *option,
+ const QModelIndex &index) const
{
QStyledItemDelegate::initStyleOption(option, index);
@@ -72,15 +114,57 @@ void SubjectDelegate::initStyleOption(QStyleOptionViewItem *option,
option->palette.setColor(QPalette::Text, highlighted);
option->palette.setColor(QPalette::WindowText, highlighted);
}
+
+ // Top-aligned and on one line, matching the subject beside them.
+ //
+ // The row is tall enough for a pill strip under the text, and Qt centres a
+ // cell's text in the whole rectangle by default: date and sender floated
+ // into the middle while the subject sat at the top, so the three did not
+ // share a baseline. Confining the rectangle to the text band puts them all
+ // on one.
+ //
+ // Wrapping matters more than it looks. A long sender ran to a second line,
+ // which reached down into the strip's band and collided with the pills; a
+ // cell cannot know they are there, since the view paints them afterwards.
+ // Eliding keeps every row's text inside its own band whatever it holds.
+ // Top of the row rather than centre of it, so the alignment is expressed
+ // without shrinking the rectangle: the rect is also what the background
+ // and selection fill are drawn into, and clipping it to the text band
+ // would leave the highlight covering only the upper part of the row.
+ option->features &= ~QStyleOptionViewItem::WrapText;
+ option->textElideMode = Qt::ElideRight;
+
+ // The marker columns keep their centring. Their glyphs are the row's
+ // symbols rather than its text, so aligning them with the subject's
+ // baseline would strand them at the top of a tall row with the pill strip
+ // empty beneath; centred, they read as marking the whole row.
+ const bool marker = index.column() == ThreadListModel::AttachmentColumn
+ || index.column() == ThreadListModel::FlagColumn;
+ option->displayAlignment = marker
+ ? Qt::AlignCenter
+ : (Qt::AlignLeft | Qt::AlignTop);
}
void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option,
const QModelIndex &index) const
{
+ // AccountLabelRole is a property of the ROW, not of a cell, so this
+ // delegate must only ever be installed on the subject column. Installed
+ // view-wide it draws the account chip into every column, which is exactly
+ // what happened when that was tried.
+ Q_ASSERT(index.column() == ThreadListModel::SubjectColumn);
+
const QString account =
index.data(ThreadListModel::AccountLabelRole).toString();
if (account.isEmpty()) {
- QStyledItemDelegate::paint(painter, option, index);
+ // No chip to draw, so the base class renders the text, confined to the
+ // upper band: the lower one belongs to the row-wide pill strip that
+ // ThreadListView paints after every cell.
+ QStyleOptionViewItem chrome = option;
+ initStyleOption(&chrome, index);
+ chrome.rect.setHeight(subjectBandHeight(option));
+ QStyledItemDelegate::paint(painter, chrome, index);
+
return;
}
@@ -95,9 +179,15 @@ void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &optio
const QFontMetrics metrics(option.font);
const QSize chipSize = TagChip::sizeFor(metrics, account);
+
+ // The subject and its chip occupy the upper band; ThreadListView paints
+ // the pill strip across the lower one. Centring the chip in the whole row
+ // would leave it floating beside that gap rather than beside its text.
+ const int textBandHeight = subjectBandHeight(option);
+ const int textTop = option.rect.top() + kRowPadding;
+
const QRect chipRect(option.rect.left() + TagChip::kSpacing,
- option.rect.top()
- + (option.rect.height() - chipSize.height()) / 2,
+ textTop + (textBandHeight - chipSize.height()) / 2,
chipSize.width(), chipSize.height());
const QColor colour =
@@ -108,6 +198,8 @@ void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &optio
// The subject follows the chip, elided so a long one cannot overflow.
QRect textRect = option.rect;
textRect.setLeft(chipRect.right() + TagChip::kSpacing * 2);
+ textRect.setTop(textTop);
+ textRect.setHeight(textBandHeight);
if (textRect.width() <= 0)
return;
@@ -155,5 +247,11 @@ QSize SubjectDelegate::sizeHint(const QStyleOptionViewItem &option,
size.setWidth(size.width() + TagChip::sizeFor(metrics, account).width()
+ TagChip::kSpacing * 3);
}
+
+ // Height comes from rowHeightFor(), applied by the view to every row at
+ // once. A QTableView takes ONE height per row, so a hint returned here
+ // would only win if the view happened to ask this column, and this
+ // delegate is on the subject column alone.
+ size.setHeight(rowHeightFor(option.font));
return size;
}
diff --git a/src/tagchip.h b/src/tagchip.h
index f43e3ff..cc3b5de 100644
--- a/src/tagchip.h
+++ b/src/tagchip.h
@@ -19,6 +19,7 @@
#pragma once
#include <QColor>
+#include <QFont>
#include <QRect>
#include <QSize>
#include <QString>
@@ -32,10 +33,13 @@ class QFontMetrics;
namespace TagChip {
/// Padding inside a chip and the gap between two of them.
-constexpr int kPaddingX = 6;
+///
+/// kPaddingX allows for the rounded ends: the corner radius is half the chip's
+/// height, so the leftmost and rightmost few pixels of the fill are curve
+/// rather than usable width, and text set closer would touch it.
+constexpr int kPaddingX = 9;
constexpr int kPaddingY = 1;
constexpr int kSpacing = 4;
-constexpr int kRadius = 3;
QSize sizeFor(const QFontMetrics &metrics, const QString &text);
@@ -46,25 +50,63 @@ void paint(QPainter *painter, const QRect &rect, const QString &text,
} // namespace TagChip
+/// Makes the selection highlight outrank a model-supplied foreground colour.
+///
+/// Qt resolves Qt::ForegroundRole into the palette's Text roles and its
+/// painting then prefers those over HighlightedText, so a model that supplies
+/// a foreground wins even on a selected row. That is wrong for the read/unread
+/// dimming, whose colour is blended against the UNSELECTED background: over
+/// the selection highlight it lands as grey on the highlight colour, close to
+/// unreadable.
+///
+/// Applied to the columns that have no delegate of their own; SubjectDelegate
+/// inherits it for the subject column.
+class RowStyleDelegate : public QStyledItemDelegate
+{
+ Q_OBJECT
+public:
+ using QStyledItemDelegate::QStyledItemDelegate;
+
+protected:
+ void initStyleOption(QStyleOptionViewItem *option,
+ const QModelIndex &index) const override;
+};
+
/// Item delegate for the subject column: draws the account chip in front of
/// the subject text, so which mailbox a thread came from reads at a glance
/// without a tags column spelling it out.
-class SubjectDelegate : public QStyledItemDelegate
+/// **Install on the subject column only.** It reads AccountLabelRole, which is
+/// a property of the row rather than of a cell, so as a view-wide delegate it
+/// draws the account chip into every column.
+class SubjectDelegate : public RowStyleDelegate
{
Q_OBJECT
public:
- using QStyledItemDelegate::QStyledItemDelegate;
+ using RowStyleDelegate::RowStyleDelegate;
void paint(QPainter *painter, const QStyleOptionViewItem &option,
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.
+ /// Vertical breathing room above the subject and below the pill row.
+ static constexpr int kRowPadding = 4;
+
+ /// The font the pill strip is drawn in: a size down from the row's own.
///
- /// 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;
+ /// At the same size the pills read as a second row of content competing
+ /// with the subject, rather than as annotation beneath it. Derived from
+ /// the row font rather than fixed, so it follows the desktop's font size.
+ static QFont pillFont(const QFont &rowFont);
+
+ /// The height every row gets, tall enough for the subject and a pill strip
+ /// beneath it. The view applies this itself: a QTableView takes one height
+ /// for the whole row, so leaving it to a single column's sizeHint would
+ /// let whichever column the view happens to ask decide.
+ static int rowHeightFor(const QFont &rowFont);
+
+protected:
+ /// The height of the band the subject text occupies. Everything below it
+ /// belongs to ThreadListView's row-wide pill strip.
+ static int subjectBandHeight(const QStyleOptionViewItem &option);
};
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index 7e0477d..4794d8c 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -59,6 +59,22 @@ QColor ThreadListModel::spamColour()
return QColor(0xa8, 0x5c, 0x18);
}
+QString ThreadListModel::flagGlyph()
+{
+ // U+2605 BLACK STAR, with the same fallback reasoning as the paperclip: an
+ // unrenderable codepoint shows as tofu, which reads as breakage rather
+ // than as "flagged". The solid star, not the outlined U+2606, since it has
+ // to register at column width beside a paperclip.
+ static const QString glyph = [] {
+ const char32_t star = 0x2605;
+ const QString preferred = QString::fromUcs4(&star, 1);
+ const QFontMetrics metrics{QFontDatabase::systemFont(
+ QFontDatabase::GeneralFont)};
+ return metrics.inFontUcs4(star) ? preferred : QStringLiteral("*");
+ }();
+ return glyph;
+}
+
QColor ThreadListModel::readColour()
{
// Derived from the palette, never hardcoded: a fixed grey that reads as
@@ -114,6 +130,45 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
if (role == TagsRole)
return thread.tags;
+ if (role == PillTagsRole || role == PillColoursRole) {
+ // Everything the row already says another way is dropped: the account
+ // is the chip in the subject cell, flagged is the star column,
+ // attachment is the paperclip, unread is the row not being dimmed, and
+ // inbox is structural rather than informative. Spending the pill row on
+ // any of those would repeat what is already on screen.
+ //
+ // deleted and spam are kept: they repaint the whole row, so a pill is
+ // redundant there too, but a doomed thread is rare and worth naming.
+ static const QStringList hidden = {
+ QStringLiteral("inbox"),
+ QStringLiteral("unread"),
+ QStringLiteral("flagged"),
+ QStringLiteral("attachment"),
+ };
+
+ QStringList pills;
+ for (const QString &tag : thread.tags) {
+ if (hidden.contains(tag) || TagColors::isAccountTag(tag))
+ continue;
+ pills.append(tag);
+ }
+ // Sorted rather than in notmuch's order, which is not guaranteed
+ // stable: a row whose pills reordered between repaints would flicker.
+ pills.sort();
+
+ if (role == PillTagsRole)
+ return pills;
+
+ // Same order as the names, so the delegate can walk the two together.
+ QVariantList colours;
+ colours.reserve(pills.size());
+ for (const QString &tag : pills) {
+ colours.append(m_tagColors ? m_tagColors->colourFor(tag)
+ : TagColors().colourFor(tag));
+ }
+ return colours;
+ }
+
if (role == AccountLabelRole || role == AccountColourRole) {
// At most one account tag per thread in practice, but a thread whose
// messages landed in two mailboxes carries both; the first is shown.
@@ -134,8 +189,15 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
if (role == Qt::ToolTipRole && index.column() == AttachmentColumn)
return thread.hasAttachment() ? tr("Has an attachment") : QVariant();
- if (role == Qt::TextAlignmentRole && index.column() == AttachmentColumn)
+ if (role == Qt::ToolTipRole && index.column() == FlagColumn)
+ return thread.isFlagged() ? tr("Flagged") : QVariant();
+
+ // Both marker columns: a glyph reads as a marker only when it sits in the
+ // middle of its column rather than against the text beside it.
+ if (role == Qt::TextAlignmentRole
+ && (index.column() == AttachmentColumn || index.column() == FlagColumn)) {
return QVariant::fromValue(Qt::AlignCenter);
+ }
if (role == Qt::DisplayRole) {
switch (index.column()) {
@@ -144,6 +206,11 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
// it inherits the row's font, so it strikes through with a doomed
// thread like every other cell.
return thread.hasAttachment() ? attachmentGlyph() : QString();
+ case FlagColumn:
+ // A glyph rather than an icon, for the same reasons as the
+ // paperclip: no asset to ship, and it inherits the row's font so
+ // it strikes through with a doomed thread.
+ return thread.isFlagged() ? flagGlyph() : QString();
case DateColumn:
return thread.date.toString(QStringLiteral("yyyy-MM-dd hh:mm"));
case AuthorsColumn:
@@ -170,19 +237,20 @@ 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.
+ // Unread's second cue, independent of 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.
+ // Bold alone was the only distinction until 2026-08-07, which leaves
+ // nothing to see when the desktop's own font is configured bold: every row
+ // renders bold and setBold() changes nothing. That is a font setting
+ // rather than a defect here, but a cue with a single point of failure is
+ // worth reinforcing.
//
- // So the emphasis is inverted instead. Unread rows are left at the
+ // So the emphasis is inverted as well. 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.
+ // background. The cue rides on ForegroundRole, which the delegate already
+ // honours, and 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
@@ -222,9 +290,10 @@ QVariant ThreadListModel::headerData(int section, Qt::Orientation orientation,
// No label: any text would set a minimum width far wider than the icon,
// which defeats the point of a narrow column.
case AttachmentColumn: return QString();
- case DateColumn: return QStringLiteral("Date");
- case AuthorsColumn: return QStringLiteral("From");
- case SubjectColumn: return QStringLiteral("Subject");
+ case FlagColumn: return QString();
+ case DateColumn: return tr("Date");
+ case AuthorsColumn: return tr("From");
+ case SubjectColumn: return tr("Subject");
default: return {};
}
}
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index 152730f..461e467 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -40,6 +40,11 @@ public:
/// without opening the thread. Icon only and deliberately narrow;
/// it carries no text.
AttachmentColumn = 0,
+
+ /// A star when the thread carries the flagged tag. Beside the
+ /// paperclip and the same shape: icon only, narrow, no text.
+ FlagColumn,
+
DateColumn,
AuthorsColumn,
SubjectColumn,
@@ -62,6 +67,16 @@ public:
/// Every tag on the thread, for the strip under the message pane.
TagsRole,
+
+ /// The tags worth drawing as pills under the subject: every tag except
+ /// the ones the row already shows another way. Sorted, so a row does
+ /// not reshuffle its own pills between repaints.
+ PillTagsRole,
+
+ /// The colours for PillTagsRole, in the same order. Supplied by the
+ /// model because it owns the TagColors instance; a delegate reading
+ /// config itself would be a second source of truth.
+ PillColoursRole,
};
/// Row fill for a thread tagged `deleted`, and for one tagged `spam`.
@@ -72,16 +87,20 @@ public:
/// A paperclip when the system font can draw it, "*" otherwise.
static QString attachmentGlyph();
+ /// The character shown in FlagColumn for a flagged thread.
+ /// A star when the system font can draw it, "*" otherwise.
+ static QString flagGlyph();
+
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.
+ /// rather than unread being emphasised. Bold alone used to be the only
+ /// cue, which leaves nothing to see when the desktop font is itself
+ /// configured bold; colour is a second cue that survives that. Derived
+ /// from the palette, never hardcoded.
static QColor readColour();
explicit ThreadListModel(QObject *parent = nullptr);
diff --git a/src/threadlistview.cpp b/src/threadlistview.cpp
new file mode 100644
index 0000000..ef80e09
--- /dev/null
+++ b/src/threadlistview.cpp
@@ -0,0 +1,140 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "threadlistview.h"
+
+#include "tagchip.h"
+#include "threadlistmodel.h"
+
+#include <QPaintEvent>
+#include <QPainter>
+#include <QScrollBar>
+
+void ThreadListView::paintEvent(QPaintEvent *event)
+{
+ QTableView::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. Walking the whole model would paint
+ // thousands of strips outside the viewport on a large query.
+ const int first = rowAt(0);
+ const int last = rowAt(viewport()->height() - 1);
+ const int lastRow = last >= 0 ? last : model()->rowCount() - 1;
+
+ for (int row = qMax(0, first); row <= lastRow; ++row) {
+ const QModelIndex index =
+ model()->index(row, ThreadListModel::SubjectColumn);
+
+ const int rowTop = rowViewportPosition(row);
+ const int height = rowHeight(row);
+ 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>());
+ else if (selectionModel() && selectionModel()->isRowSelected(row))
+ painter.fillRect(band, palette().brush(QPalette::Highlight));
+ else if (alternatingRowColors() && (row % 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;
+ }
+ }
+}
diff --git a/src/threadlistview.h b/src/threadlistview.h
new file mode 100644
index 0000000..0b4eafc
--- /dev/null
+++ b/src/threadlistview.h
@@ -0,0 +1,47 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#pragma once
+
+#include <QTableView>
+
+/// The thread list, with a row-wide strip of tag chips under each row's cells.
+///
+/// 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:
+///
+/// [ 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.
+class ThreadListView : public QTableView
+{
+ Q_OBJECT
+public:
+ using QTableView::QTableView;
+
+protected:
+ void paintEvent(QPaintEvent *event) override;
+};
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 1af95ed..7ee8a5a 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -29,6 +29,7 @@
#include <QPushButton>
#include <QProgressBar>
#include <QFile>
+#include <QSet>
#include <QSettings>
#include <QStandardPaths>
#include <QTemporaryDir>
@@ -41,6 +42,7 @@
#include "mainwindow.h"
#include "messageview.h"
#include "notmuchworker.h"
+#include "tagchip.h"
#include "threadlistmodel.h"
/// MainWindow is mostly wiring, and the parts that need a real database are
@@ -82,6 +84,7 @@ private slots:
void anUnobservableLockTableLeavesTheSyncButtonUsable();
void theStatusBarFollowsTheSyncPhase();
void aSelectedReadThreadIsNotDimmedIntoTheHighlight();
+ void thePillRowSpansTheWholeWidthNotOneColumn();
void markAllReadIsDisabledUntilTheQueryFinishes();
void markAllReadActsOnEveryRowAndUndoesInOneStep();
void markAllReadDoesNothingWhenNothingIsUnread();
@@ -371,6 +374,83 @@ 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<QTableView *>();
+ 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 = view->rowHeight(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::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
{
// Read threads carry a dimmed Qt::ForegroundRole, blended against the
@@ -389,20 +469,30 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
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 });
+ // Both rows READ, so both are dimmed and neither is bold: the only thing
+ // that could differ is how the dimming composites against the selection.
+ //
+ // Comparing a read row against an unread one would not work, and an
+ // earlier version of this test did exactly that. Unread also paints bold,
+ // so the rows differ legitimately and the comparison says nothing about
+ // the selection. That version passed only because the machine it was
+ // written on had its Qt font configured Bold, which made every row bold
+ // and hid the difference.
+ ThreadSummary first = makeThread(QStringLiteral("t1"), {});
+ ThreadSummary second = makeThread(QStringLiteral("t2"), {});
+ first.subject = second.subject = QStringLiteral("Same subject both rows");
+ first.authors = second.authors = QStringLiteral("Someone <s@example.org>");
+ model->appendBatch({ first, second });
window.resize(900, 300);
window.show();
QVERIFY(QTest::qWaitForWindowExposed(&window));
- view->selectAll();
+ // Row 0 selected, row 1 not. The property under test is that selecting a
+ // dimmed row switches it to the highlight's own text colour, so the two
+ // rows MUST differ; comparing two identically-styled rows would pass
+ // against a delegate that did nothing at all.
+ view->selectRow(0);
QApplication::processEvents();
const int rowHeight = view->rowHeight(0);
@@ -412,17 +502,40 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight()
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)));
+ // What the delegate resolves for each row, which is the thing the fix
+ // changes. Rendering alone cannot separate "used the highlight colour"
+ // from "used the dim over a highlighted background".
+ QStyleOptionViewItem selected;
+ selected.initFrom(view);
+ selected.state |= QStyle::State_Selected;
+ QStyleOptionViewItem unselected;
+ unselected.initFrom(view);
+ unselected.state &= ~QStyle::State_Selected;
+
+ auto *delegate = qobject_cast<QStyledItemDelegate *>(view->itemDelegate());
+ QVERIFY2(delegate, "the thread view has no styled delegate");
+
+ const QModelIndex index =
+ model->index(0, ThreadListModel::SubjectColumn);
+
+ // 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;
+ };
+ const auto *probe = static_cast<const Probe *>(
+ static_cast<const SubjectDelegate *>(delegate));
+
+ probe->initStyleOption(&selected, index);
+ probe->initStyleOption(&unselected, index);
+
+ QVERIFY2(selected.palette.color(QPalette::Text)
+ == selected.palette.color(QPalette::HighlightedText),
+ "a selected row still resolves to the dimmed text colour, so the "
+ "dimming will paint over the selection highlight");
+ QVERIFY2(unselected.palette.color(QPalette::Text)
+ != selected.palette.color(QPalette::Text),
+ "an unselected read row lost its dimming");
}
void TestMainWindow::markAllReadIsDisabledUntilTheQueryFinishes()
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index b880c29..82686e5 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -20,6 +20,7 @@
#include <QSignalSpy>
#include <QtTest>
+#include "tagcolors.h"
#include "threadlistmodel.h"
class TestThreadListModel : public QObject
@@ -34,6 +35,9 @@ private slots:
void subjectShowsMessageCountOnlyForRealThreads();
void unreadThreadsRenderBold();
void readThreadsAreDimmedAndUnreadAreNot();
+ void flaggedThreadsShowAStar();
+ void pillTagsExcludeWhatTheRowAlreadyShows();
+ void theStarColumnIsNarrowAndCarriesNoText();
void theUnreadCueDoesNotDependOnFontWeight();
void aDoomedThreadKeepsItsContrastEvenWhenRead();
void tagsAreTheFirstColumnAndSubjectTheLast();
@@ -169,14 +173,15 @@ void TestThreadListModel::unreadThreadsRenderBold()
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.
+ // Bold was unread's ONLY cue, which leaves nothing to see when the
+ // desktop's own font is configured bold: every row renders bold and
+ // setBold() changes nothing. That is what the original report turned out
+ // to be, a qt6ct setting rather than a defect here, but a cue with one
+ // point of failure is worth reinforcing.
//
- // Read rows are dimmed instead, which inverts the emphasis: unread sits at
- // full contrast and the bulk of a mostly-read list recedes.
+ // Read rows are dimmed as well, which inverts the emphasis: unread sits at
+ // full contrast and the bulk of a mostly-read list recedes. Bold still
+ // applies on top.
ThreadListModel model;
ThreadSummary read = makeThread(QStringLiteral("t1"), QStringLiteral("read"));
read.tags = QStringList{ QStringLiteral("inbox") };
@@ -196,6 +201,105 @@ void TestThreadListModel::readThreadsAreDimmedAndUnreadAreNot()
"is the one that stands out");
}
+void TestThreadListModel::flaggedThreadsShowAStar()
+{
+ // "flagged" is an ordinary notmuch tag already carried in ThreadSummary,
+ // so this needs no worker query, exactly as the paperclip did not.
+ ThreadListModel model;
+ ThreadSummary plain = makeThread(QStringLiteral("t1"), QStringLiteral("plain"));
+ plain.tags = QStringList{ QStringLiteral("inbox") };
+ ThreadSummary starred = makeThread(QStringLiteral("t2"),
+ QStringLiteral("starred"));
+ starred.tags = QStringList{ QStringLiteral("inbox"),
+ 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());
+}
+
+void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows()
+{
+ // The pills exist to say what the row does not already say. Repeating the
+ // account, the flag, the attachment or the read state as text beside the
+ // chip, the star, the paperclip and the dimming would spend the new space
+ // on things already visible.
+ ThreadListModel model;
+ ThreadSummary thread = makeThread(QStringLiteral("t1"),
+ QStringLiteral("noisy"));
+ thread.tags = QStringList{
+ QStringLiteral("inbox"), // structural, always true here
+ QStringLiteral("unread"), // shown by not being dimmed
+ QStringLiteral("flagged"), // shown by the star column
+ QStringLiteral("attachment"), // shown by the paperclip column
+ QStringLiteral("account-work"), // shown as the chip
+ QStringLiteral("SBo"), // worth showing
+ QStringLiteral("shopping/amazon"),
+ };
+ model.appendBatch({ thread });
+
+ const QStringList pills =
+ model.data(model.index(0, ThreadListModel::SubjectColumn),
+ ThreadListModel::PillTagsRole).toStringList();
+
+ QVERIFY2(pills.contains(QStringLiteral("SBo")), qPrintable(pills.join(',')));
+ QVERIFY2(pills.contains(QStringLiteral("shopping/amazon")),
+ qPrintable(pills.join(',')));
+
+ for (const QString &hidden : { QStringLiteral("inbox"),
+ QStringLiteral("unread"),
+ QStringLiteral("flagged"),
+ QStringLiteral("attachment") }) {
+ QVERIFY2(!pills.contains(hidden),
+ qPrintable(QStringLiteral("'%1' is repeated as a pill")
+ .arg(hidden)));
+ }
+
+ // The account tag is matched by shape rather than by name, since the key
+ // varies per user: whatever TagColors calls an account tag is excluded.
+ for (const QString &tag : pills) {
+ QVERIFY2(!TagColors::isAccountTag(tag),
+ qPrintable(QStringLiteral("account tag '%1' repeated as a pill")
+ .arg(tag)));
+ }
+
+ // Stable order, so a row does not reshuffle its own pills between repaints.
+ QStringList sorted = pills;
+ sorted.sort();
+ 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