diff options
| -rw-r--r-- | CHANGELOG.md | 20 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 123 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 39 | ||||
| -rw-r--r-- | src/tagchip.cpp | 110 | ||||
| -rw-r--r-- | src/tagchip.h | 62 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 97 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 27 | ||||
| -rw-r--r-- | src/threadlistview.cpp | 140 | ||||
| -rw-r--r-- | src/threadlistview.h | 47 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 153 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 118 |
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 |