diff options
| -rw-r--r-- | CHANGELOG.md | 8 | ||||
| -rw-r--r-- | CLAUDE.md | 15 | ||||
| -rw-r--r-- | CMakeLists.txt | 2 | ||||
| -rw-r--r-- | assets/icons/marks/attachment.svg | 21 | ||||
| -rw-r--r-- | assets/icons/marks/expander-collapsed.svg | 6 | ||||
| -rw-r--r-- | assets/icons/marks/expander-expanded.svg | 5 | ||||
| -rw-r--r-- | assets/icons/marks/flagged.svg | 3 | ||||
| -rw-r--r-- | assets/icons/marks/passed.svg | 16 | ||||
| -rw-r--r-- | assets/icons/marks/replied.svg | 16 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 55 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 4 | ||||
| -rw-r--r-- | src/carddelegate.cpp | 57 | ||||
| -rw-r--r-- | src/cardlayout.cpp | 85 | ||||
| -rw-r--r-- | src/cardlayout.h | 35 | ||||
| -rw-r--r-- | src/htmlbuilder.h | 10 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 4 | ||||
| -rw-r--r-- | src/marks.cpp | 160 | ||||
| -rw-r--r-- | src/marks.h | 83 | ||||
| -rw-r--r-- | src/messageview.cpp | 59 | ||||
| -rw-r--r-- | src/messageview.h | 5 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 75 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 14 | ||||
| -rw-r--r-- | src/types.h | 21 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_cardlayout.cpp | 173 | ||||
| -rw-r--r-- | tests/test_marks.cpp | 286 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 44 |
27 files changed, 1183 insertions, 80 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eda4e0..a555fb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,14 @@ point at which they are stable. one per thread, and a sync already running is never interrupted or queued behind. Set `auto_sync_delay_ms` in `[general]` to change the delay, or to any negative value to turn the behaviour off and get the previous one back. +- The panes draw their own marks instead of borrowing font glyphs. Flagged, + attachment, forwarded, replied and the thread expander are six SVGs shipped + with the application, recoloured from your palette, so they look the same on + every desktop and cannot turn into a tofu box on a font that lacks a + codepoint. Forwarded and replied were words in the tag strip and are now marks + beside the subject, and the message pane shows the flagged and attachment + marks next to the subject in its header. The toolbar and the menus are + untouched and still follow your icon theme. ### Fixed @@ -74,6 +74,21 @@ colours, the selection and `BackgroundRole` across cells it did not own. With one column there is nothing to span, so the `paintEvent` and its band arithmetic are deleted and none of that applies any more. +**The panes' marks are shipped SVGs, not font glyphs and not a `.qrc`.** `Marks` +(`src/marks.h`) carries six payloads as compiled-in string literals, generated +from `assets/icons/marks/*.svg`, which stay the editable originals. Not a +resource, because `src/CMakeLists.txt` already records that a qrc in the static +library registers itself from a global initialiser the linker drops, and the +tests link the library rather than the executable. Every payload paints with +`fill="currentColor"`, which `QSvgRenderer` renders BLACK rather than resolving; +`Marks::pixmap` composites the real colour with `CompositionMode_SourceIn`, which +is what lets one asset serve a light and a dark palette. The toolbar and menus +still use `QIcon::fromTheme` and must keep doing so: the split between "panes are +ours, chrome is the system's" is item 70's whole point. A tag drawn as a mark +must not also appear as a chip, which `isDrawnAsAMark()` in `threadlistmodel.cpp` +enforces for both roles at once; the duplicate survived every geometry test and +was found only by rendering a card and looking at it. + **A card layout must be testable without a painter.** `CardLayout` computes every rect on a card and touches no `QPainter` and no widget, so the geometry has tests that a blank render cannot defeat. When changing what a card shows, diff --git a/CMakeLists.txt b/CMakeLists.txt index 7ff124a..01f7279 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ set(CMAKE_AUTORCC ON) # that ships. option(QTMAILDIR_BUILD_TESTS "Build the test suite" ON) -set(QTMAILDIR_QT_COMPONENTS Widgets WebEngineWidgets) +set(QTMAILDIR_QT_COMPONENTS Widgets Svg WebEngineWidgets) if(QTMAILDIR_BUILD_TESTS) list(APPEND QTMAILDIR_QT_COMPONENTS Test) endif() diff --git a/assets/icons/marks/attachment.svg b/assets/icons/marks/attachment.svg new file mode 100644 index 0000000..0efd31a --- /dev/null +++ b/assets/icons/marks/attachment.svg @@ -0,0 +1,21 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> + <!-- Vertical clip: outer wall down the left, around the open bottom loop, + back up the inner wall. One closed subpath, so it fills solid with no + even-odd rule to depend on. --> + <path fill="currentColor" d="M 10.4,1.1 + C 8.75,1.1 7.4,2.45 7.4,4.1 + V 11.6 + a 1.1,1.1 0 0 0 2.2,0 + V 4.6 + a 0.85,0.85 0 0 1 1.7,0 + V 11.7 + a 2.75,2.75 0 0 1 -5.5,0 + V 4.35 + a 1.1,1.1 0 0 0 -2.2,0 + V 11.7 + C 3.6,14.4 5.8,16 8.35,16 + 10.9,16 13.1,14.4 13.1,11.7 + V 4.1 + C 13.1,2.45 11.9,1.1 10.4,1.1 + Z"/> +</svg> diff --git a/assets/icons/marks/expander-collapsed.svg b/assets/icons/marks/expander-collapsed.svg new file mode 100644 index 0000000..8cacee7 --- /dev/null +++ b/assets/icons/marks/expander-collapsed.svg @@ -0,0 +1,6 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> + <!-- Collapsed: points right. 6.9 wide by 11.2 tall, scaled up from the + original triangle about the 8,8 centre so it holds its own beside the + star rather than reading as timid. --> + <path fill="currentColor" d="M 5.38,2.38 12.25,8 5.38,13.62 Z"/> +</svg> diff --git a/assets/icons/marks/expander-expanded.svg b/assets/icons/marks/expander-expanded.svg new file mode 100644 index 0000000..6911eeb --- /dev/null +++ b/assets/icons/marks/expander-expanded.svg @@ -0,0 +1,5 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> + <!-- Expanded: the collapsed triangle rotated 90 degrees about the centre, + so the pair cannot drift in weight. --> + <path fill="currentColor" d="M 2.38,5.38 13.62,5.38 8,12.25 Z"/> +</svg> diff --git a/assets/icons/marks/flagged.svg b/assets/icons/marks/flagged.svg new file mode 100644 index 0000000..59dc23f --- /dev/null +++ b/assets/icons/marks/flagged.svg @@ -0,0 +1,3 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> + <path fill="currentColor" d="M 8.00,1.05 9.73,5.96 14.94,6.09 10.81,9.26 12.29,14.26 8.00,11.30 3.71,14.26 5.19,9.26 1.06,6.09 6.27,5.96 Z"/> +</svg> diff --git a/assets/icons/marks/passed.svg b/assets/icons/marks/passed.svg new file mode 100644 index 0000000..47ae438 --- /dev/null +++ b/assets/icons/marks/passed.svg @@ -0,0 +1,16 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> + <!-- Forwarded: arrow head at the right, shaft sweeping down and back to a + tail at lower left. Mirror image of replied.svg about x=8, so the two + read as one pair. --> + <path fill="currentColor" d="M 9.2,2.2 + V 5.0 + H 7.3 + C 4.1,5.0 1.8,7.4 1.8,10.8 + V 13.8 + a 0.9,0.9 0 0 0 1.75,0.28 + C 4.2,12.1 5.6,10.9 7.3,10.9 + H 9.2 + V 13.7 + L 15.0,7.95 + Z"/> +</svg> diff --git a/assets/icons/marks/replied.svg b/assets/icons/marks/replied.svg new file mode 100644 index 0000000..23d2e54 --- /dev/null +++ b/assets/icons/marks/replied.svg @@ -0,0 +1,16 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> + <!-- Replied: the exact mirror of passed.svg about x=8. Every coordinate is + 16 minus its counterpart, so the pair is symmetric by construction + rather than by eye. --> + <path fill="currentColor" d="M 6.8,2.2 + V 5.0 + H 8.7 + C 11.9,5.0 14.2,7.4 14.2,10.8 + V 13.8 + a 0.9,0.9 0 0 1 -1.75,0.28 + C 11.8,12.1 10.4,10.9 8.7,10.9 + H 6.8 + V 13.7 + L 1.0,7.95 + Z"/> +</svg> 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 2663b44..0a1f4dc 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 @@ -125,8 +125,8 @@ taking that too literally. | 66 | Selecting a thread root leaves the message pane blank until a reply has been selected | defect | S | open; needs a reproduction before a fix | | 67 | The placeholder pane counts unread, flagged and inbox, but not sent or drafts | information | XS | **done** 2026-08-11, shipped in 0.15.0 | | 68 | A forwarded subject gets no `passed` tag | workflow | S | open; no subject rule exists, measured 2026-08-11. Decision needed: display mark (XS) or write the flag (S, syncs out) | -| 69 | `passed` and `replied` read as words where every other state is a glyph | presentation | S | open; depends on 68 for what `passed` means | -| 70 | Pane icons are a private set where the main window uses the system theme | presentation | M | open | +| 69 | `passed` and `replied` read as words where every other state is a glyph | presentation | S | **done** 2026-08-11, inside item 70 | +| 70 | Pane icons are a private set where the main window uses the system theme | presentation | M | **done** 2026-08-11; six shipped SVGs | | 71 | A toolbar action does not sync, so the edit sits until the next cron run | workflow | S | **done** 2026-08-11; 2s default, `auto_sync_delay_ms` | | 72 | No khard/khal integration | workflow | ? | open, unspecified; the user places it after send, so v2 at the earliest | | 73 | This backlog is past four thousand lines | maintenance | S | open | @@ -4423,6 +4423,57 @@ covers actions only and will not catch a collision between pane marks. **Size: M**, and it overlaps 69, which should probably be done inside it rather than before it. +**Done 2026-08-11, with item 69 folded in as the Size note predicted.** Six +marks ship with the application in `assets/icons/marks/`: flagged, attachment, +passed, replied and the two expander triangles. The toolbar and menus still +resolve through `QIcon::fromTheme` and were not touched, which is the split the +user stated. + +**Licensing decided the shapes.** The user pointed at the Material-Black-Plum-Suru +theme as the look they wanted. That set is GPL3 (`index.theme` names Sam Hewitt +and the licence) and this project is GPLv2-ONLY (`src/main.cpp:6`, no "or +later"), which are incompatible: GPLv2's "no further restrictions" clause bars +shipping GPL3 assets in a v2-only work. The user chose to have the six drawn +fresh in the same idiom rather than relicense, so no Suru path data was copied. +The idiom itself is generic: solid single-path silhouettes at 16x16, no strokes. + +**Not a .qrc.** `src/CMakeLists.txt` already records that a qrc compiled into the +static library registers itself from a global initialiser the linker drops, so +resources belong to the executable. The tests link the LIBRARY, so a +resource-based mark would be absent exactly where it needs asserting. The +payloads are compiled in as string literals in `src/marks.cpp`, generated from +the assets, which stay the editable originals. + +**One asset per mark, not one per theme.** Every payload paints with +`fill="currentColor"`, which `QSvgRenderer` does not resolve: it renders black. +`Marks::pixmap` composites the wanted colour with `CompositionMode_SourceIn`, +so a mark takes the card's own pen colour and follows selection and the +read/unread dimming for free. Cached by (mark, size, colour, ratio), since a +delegate repaints these per row per frame. + +**`CardLayout` reserves the rects; `CardDelegate` paints them.** The marks were +glyphs inside the subject STRING, so their width came free from the text +metrics; as icons the geometry has to know they exist or the subject runs +underneath them. That is why `Input` grew four bools. The same trap bit the +expander pill, whose triangle was a glyph in `expanderLabel()` and now needs its +width reserved explicitly. + +**What the tests could not catch, and the render did.** Every geometry +assertion passed while a card showed `passed` as BOTH an arrow and a green tag +chip: the chip filter had no reason to know a mark had appeared. Found by +rendering real cards to a PNG and looking at it. `isDrawnAsAMark()` is now one +list consulted by both `PillTagsRole` and `MessageOwnTagsRole`, since two copies +drifting is how a tag ends up drawn twice on one row and not at all on another. + +Nine tests in `test_marks` and four in `test_cardlayout`, plus one in +`test_threadlistmodel` for the de-duplication. Mutation-checked at four points: +the subject ignoring the marks, the flag not indenting the subject, the pill +forgetting the triangle's width, and the recolour composite removed. Each failed +a test. The old `flagGlyph()`/`attachmentGlyph()` and their `*` fallback are +deleted; that fallback was a latent defect of its own, since both collapsed to +the same character and made a flagged thread indistinguishable from one with an +attachment. + ## 71. A toolbar action does not sync, so the edit sits until the next cron run **Observed (user, from the notes):** "clicking one action in the toolbar should be diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index fdac2c1..6f5101e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -6,6 +6,7 @@ add_library(qtmaildir_lib STATIC htmlbuilder.cpp cidschemehandler.cpp cardlayout.cpp + marks.cpp carddelegate.cpp notmuchworker.cpp tagchip.cpp @@ -27,7 +28,8 @@ target_include_directories(qtmaildir_lib ${CMAKE_BINARY_DIR}/generated/qtmaildir) target_link_libraries(qtmaildir_lib - PUBLIC Qt6::Widgets Qt6::WebEngineWidgets PkgConfig::GMIME ${NOTMUCH_LIBRARY}) + PUBLIC Qt6::Widgets Qt6::Svg Qt6::WebEngineWidgets PkgConfig::GMIME + ${NOTMUCH_LIBRARY}) # resources.qrc belongs to the executable, not to the static library. A qrc # compiled into a .a registers itself from a global initialiser, and the linker diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp index 3039853..d7d06aa 100644 --- a/src/carddelegate.cpp +++ b/src/carddelegate.cpp @@ -19,6 +19,7 @@ #include "carddelegate.h" #include "cardlayout.h" +#include "marks.h" #include "threadlistmodel.h" #include <QApplication> @@ -37,6 +38,14 @@ CardLayout::Input inputFor(const QModelIndex &index) in.depth = index.data(ThreadListModel::MessageDepthRole).toInt(); in.replyCount = index.data(ThreadListModel::ReplyCountRole).toInt(); in.dateFormat = index.data(ThreadListModel::DateFormatRole).toString(); + + // Item 70's marks. The layout reserves a rect for each, so these have to + // reach it: a mark drawn without its rect reserved lands on top of the + // subject rather than beside it. + in.flagged = index.data(ThreadListModel::IsFlaggedRole).toBool(); + in.hasAttachment = index.data(ThreadListModel::HasAttachmentRole).toBool(); + in.passed = index.data(ThreadListModel::IsPassedRole).toBool(); + in.replied = index.data(ThreadListModel::IsRepliedRole).toBool(); return in; } @@ -190,16 +199,25 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, QStringLiteral("^\\s*(?:[Rr][Ee]\\s*:\\s*)+")); subject.remove(re); } - QString line2; - if (index.data(ThreadListModel::IsFlaggedRole).toBool()) - line2 += ThreadListModel::flagGlyph() + QLatin1Char(' '); - line2 += subject; - if (index.data(ThreadListModel::HasAttachmentRole).toBool()) - line2 += QLatin1Char(' ') + ThreadListModel::attachmentGlyph(); painter->drawText(card.subjectRect, Qt::AlignVCenter | Qt::AlignLeft, - metrics.elidedText(line2, Qt::ElideRight, + metrics.elidedText(subject, Qt::ElideRight, card.subjectRect.width())); + // Item 70's marks, drawn into the rects the layout reserved rather than + // appended to the subject STRING as glyphs. The colour is the pen's, which + // is already resolved above against selection and the read/unread + // foreground, so a mark follows its card's text exactly: white on a + // selected row, dimmed on a read one. + const QColor markColour = painter->pen().color(); + const auto drawMark = [&](const QRect &rect, Marks::Mark mark) { + if (!rect.isEmpty()) + Marks::paint(painter, rect, mark, markColour); + }; + drawMark(card.flagRect, Marks::Mark::Flagged); + drawMark(card.attachmentRect, Marks::Mark::Attachment); + drawMark(card.passedRect, Marks::Mark::Passed); + drawMark(card.repliedRect, Marks::Mark::Replied); + // The reply count, which is also the expander, drawn as a PILL. // // A bare "3" on the card's own background read as an unexplained number @@ -243,7 +261,30 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, painter->setPen(option.state & QStyle::State_Selected ? option.palette.highlightedText().color() : text); - painter->drawText(card.expanderRect, Qt::AlignCenter, label); + + // The triangle is a drawn mark since item 70, not a glyph in the label, + // so the pill lays out its two pieces itself: the mark, a gap, then the + // count. The layout reserved width for exactly this (ascent + kMarkGap), + // so the two must agree or the text drifts out of its own background. + const QFontMetrics pillMetrics(painter->font()); + const int side = pillMetrics.ascent(); + const int textWidth = pillMetrics.horizontalAdvance(label); + const int contentWidth = side + CardLayout::kMarkGap + textWidth; + const int left = card.expanderRect.left() + + (card.expanderRect.width() - contentWidth) / 2; + const QRect markRect(left, + card.expanderRect.top() + + (card.expanderRect.height() - side) / 2, + side, side); + Marks::paint(painter, markRect, + option.state & QStyle::State_Open + ? Marks::Mark::ExpanderExpanded + : Marks::Mark::ExpanderCollapsed, + painter->pen().color()); + painter->drawText(QRect(markRect.right() + 1 + CardLayout::kMarkGap, + card.expanderRect.top(), textWidth, + card.expanderRect.height()), + Qt::AlignVCenter | Qt::AlignLeft, label); painter->restore(); } diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp index d60bff9..0febb4b 100644 --- a/src/cardlayout.cpp +++ b/src/cardlayout.cpp @@ -33,20 +33,46 @@ QString CardLayout::formatDate(const QDateTime &date, const QString &format) return QLocale::system().toString(date, format); } +int CardLayout::markSide(const QFont &font) +{ + // Derived from the font's ascent rather than fixed, so the marks grow with + // the user's text size. A pixel count settled on one desktop is wrong on + // the next one, and qt6ct sets fonts in PIXELS, where pointSizeF() returns + // -1 (see the note in this file's header) which is why the metric and not + // the size is the thing measured. + // + // Ascent rather than height: height includes the descent, which no mark + // occupies, and marks sized from it read noticeably larger than the text + // beside them. + const int side = QFontMetrics(font).ascent(); + + // Floor of 8: below that the shapes stop being tellable apart, which is the + // defect item 70 exists to fix rather than one to reintroduce at a small + // font size. + return qMax(8, side); +} + QString CardLayout::expanderLabel(int replyCount, bool expanded) { // "3 replies", not a bare "3". The count alone reads as an unexplained // number beside the subject, and the word is what says the card opens. // + // The triangle is NO LONGER part of this string. Item 70 made it a drawn + // mark, so the pill reserves a rect for it and the delegate paints it; a + // glyph left here would be a second triangle beside the drawn one. The + // `expanded` parameter therefore no longer changes the label, and is kept + // because both states are still measured: the pill must not change width + // when the card opens, and a caller that stopped passing the state would + // hide that requirement rather than satisfy it. + // // Not translated through tr() here because CardLayout is a plain struct // rather than a QObject; the delegate is where a translated build would // wrap this, and the string is deliberately kept in one place so there is // exactly one thing to change. - const QString glyph = expanded ? QStringLiteral("\u25be") - : QStringLiteral("\u25b8"); + Q_UNUSED(expanded); const QString word = replyCount == 1 ? QStringLiteral("reply") : QStringLiteral("replies"); - return QStringLiteral("%1 %2 %3").arg(glyph).arg(replyCount).arg(word); + return QStringLiteral("%1 %2").arg(replyCount).arg(word); } QString CardLayout::widestDateSample(const QString &format) @@ -172,17 +198,58 @@ CardLayout CardLayout::compute(const Input &input, const QRect &rect, expanderLabel(input.replyCount, false)); const int expanded = smallMetrics.horizontalAdvance( expanderLabel(input.replyCount, true)); + // The triangle is a drawn mark since item 70, so the pill has to + // reserve its width explicitly. It came free from the text metrics + // while it was a glyph in the label, which is exactly the kind of + // width that disappears silently when the glyph does. + const int triangle = smallMetrics.ascent() + kMarkGap; const int countWidth = - qMax(collapsed, expanded) + kPillPaddingX * 2; + qMax(collapsed, expanded) + triangle + kPillPaddingX * 2; out.expanderRect = QRect(right - countWidth, lineTwoTop, countWidth, metrics.height()); } - const int subjectRight = out.expanderRect.isEmpty() - ? right - : out.expanderRect.left() - kPaddingX; - out.subjectRect = QRect(out.contentLeft, lineTwoTop, - qMax(0, subjectRight - out.contentLeft), + // Item 70's marks. Until it they were glyphs inside the subject string, so + // the text metrics reserved their width without anyone arranging it; as + // icons they need rects, and the subject needs to end before them or it + // runs underneath. + // + // Laid out from the RIGHT, inwards: the expander is already placed, and + // each mark present takes the next slot to its left. The subject then gets + // whatever is left, which is what keeps a card with four marks from eliding + // its subject to nothing on a narrow window: the marks are small and fixed, + // the subject is the elastic part. + const int side = markSide(font); + const int markTop = lineTwoTop + (metrics.height() - side) / 2; + int markRight = out.expanderRect.isEmpty() + ? right + : out.expanderRect.left() - kPaddingX; + + // Order matters and is the drawing order reversed: placing right to left + // here puts attachment nearest the subject and replied furthest right, + // which is the order the delegate then paints them in. + const auto placeMark = [&](bool present, QRect &target) { + if (!present) + return; + target = QRect(markRight - side, markTop, side, side); + markRight = target.left() - kMarkGap; + }; + placeMark(input.replied, out.repliedRect); + placeMark(input.passed, out.passedRect); + placeMark(input.hasAttachment, out.attachmentRect); + + // The flag sits at the START of line two, where the glyph did, so a flagged + // card still reads flagged from the left edge. It indents the subject + // rather than overlapping it. + int subjectLeft = out.contentLeft; + if (input.flagged) { + out.flagRect = QRect(out.contentLeft, markTop, side, side); + subjectLeft = out.flagRect.right() + 1 + kMarkGap; + } + + const int subjectRight = markRight; + out.subjectRect = QRect(subjectLeft, lineTwoTop, + qMax(0, subjectRight - subjectLeft), metrics.height()); out.tagRect = QRect(out.contentLeft, lineThreeTop, diff --git a/src/cardlayout.h b/src/cardlayout.h index 1a2fd18..3512242 100644 --- a/src/cardlayout.h +++ b/src/cardlayout.h @@ -56,6 +56,18 @@ struct CardLayout /// the geometry is exactly how a longer date gets elided into a rect /// sized for a shorter one. QString dateFormat; + + /// Which marks line two carries (item 70). + /// + /// On the INPUT for the same reason dateFormat is: the marks were + /// glyphs inside the subject STRING until item 70, so their width came + /// free from the text metrics. Drawn as icons they occupy rects of + /// their own, and a subject sized as though they were absent would run + /// underneath them. The layout has to know they are there. + bool flagged = false; + bool hasAttachment = false; + bool passed = false; + bool replied = false; }; /// Width of the account accent bar down a thread card's left edge. @@ -94,6 +106,29 @@ struct CardLayout /// Empty when the row has no replies. QRect expanderRect; + /// The flagged mark, at the start of line two before the subject. Empty + /// when the row is not flagged. + QRect flagRect; + + /// The state marks after the subject, in this order: attachment, passed, + /// replied. Each is empty when its state does not apply. + /// + /// Separate rects rather than one strip, because each is independently + /// present or absent and a strip would have to encode which. They are laid + /// out right to left from the expander, so the subject keeps whatever is + /// left. + QRect attachmentRect; + QRect passedRect; + QRect repliedRect; + + /// The side of a square mark on line two, derived from the card's font so + /// the marks scale with the user's text size rather than being pinned to a + /// pixel count that is right on one desktop only. + static int markSide(const QFont &font); + + /// Gap between two adjacent marks, and between a mark and the subject. + static constexpr int kMarkGap = 4; + /// The account accent bar down the card's left edge. /// /// Thread cards only. A reply's account is its thread's, stated once at the diff --git a/src/htmlbuilder.h b/src/htmlbuilder.h index 3ade530..4078065 100644 --- a/src/htmlbuilder.h +++ b/src/htmlbuilder.h @@ -32,6 +32,16 @@ struct ThreadRenderItem /// Matched messages render in full; unmatched collapse to a one-line stub. bool expanded = true; + /// Whether the message is flagged, for the mark beside the subject in the + /// message pane's header (item 70). + /// + /// Carried here rather than derived from `message`, because it comes from + /// the notmuch TAGS and ParsedMessage holds only what the MIME parser found + /// in the file. MessageRef already answers both, so this costs no query. + /// Unused by the generated HTML itself: the header is a QLabel above the + /// web view, not part of the sandboxed document. + bool flagged = false; + /// Disambiguates cid: references. Two newsletters in one thread commonly /// use the same Content-ID (cid:logo@example.org), which would collide in /// a single document, so every reference is rewritten to diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 04e951d..f731511 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2037,6 +2037,10 @@ void MainWindow::onThreadLoaded(const QVector<MessageRef> &messages, // Namespace prefix keeps cid: references distinct across the thread. item.cidPrefix = cidPrefixForIndex(i); + // For the header's marks (item 70). From the REF's tags, since the + // parsed message carries only what was in the file. + item.flagged = ref.isFlagged(); + // Matched messages open; the rest collapse to a stub. The last message // always opens, so a thread never renders as nothing but stubs. item.expanded = ref.matched || i == messages.size() - 1; diff --git a/src/marks.cpp b/src/marks.cpp new file mode 100644 index 0000000..8777f84 --- /dev/null +++ b/src/marks.cpp @@ -0,0 +1,160 @@ +/* + * qtmaildir - a Qt6 GUI for a local notmuch-indexed Maildir + * 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 "marks.h" + +#include <QHash> +#include <QPainter> +#include <QRect> +#include <QSvgRenderer> + +namespace Marks { + +QByteArray svg(Mark mark) +{ + // Compiled in rather than loaded from a .qrc, for the linker reason given + // in marks.h. Generated from assets/icons/marks/*.svg, which stay the + // editable originals: change the asset, regenerate, do not hand-edit here. + switch (mark) { + case Mark::Attachment: + return QByteArray( + "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"16\" height=\"16\" " + "viewBox=\"0 0 16 16\"> <path fill=\"currentColor\" d=\"M 10.4,1.1 C " + "8.75,1.1 7.4,2.45 7.4,4.1 V 11.6 a 1.1,1.1 0 0 0 2.2,0 V 4.6 a " + "0.85,0.85 0 0 1 1.7,0 V 11.7 a 2.75,2.75 0 0 1 -5.5,0 V 4.35 a 1.1,1.1 " + "0 0 0 -2.2,0 V 11.7 C 3.6,14.4 5.8,16 8.35,16 10.9,16 13.1,14.4 " + "13.1,11.7 V 4.1 C 13.1,2.45 11.9,1.1 10.4,1.1 Z\"/> </svg>"); + case Mark::Flagged: + return QByteArray( + "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"16\" height=\"16\" " + "viewBox=\"0 0 16 16\"> <path fill=\"currentColor\" d=\"M 8.00,1.05 " + "9.73,5.96 14.94,6.09 10.81,9.26 12.29,14.26 8.00,11.30 3.71,14.26 " + "5.19,9.26 1.06,6.09 6.27,5.96 Z\"/> </svg>"); + case Mark::Passed: + return QByteArray( + "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"16\" height=\"16\" " + "viewBox=\"0 0 16 16\"> <path fill=\"currentColor\" d=\"M 9.2,2.2 V 5.0 " + "H 7.3 C 4.1,5.0 1.8,7.4 1.8,10.8 V 13.8 a 0.9,0.9 0 0 0 1.75,0.28 C " + "4.2,12.1 5.6,10.9 7.3,10.9 H 9.2 V 13.7 L 15.0,7.95 Z\"/> </svg>"); + case Mark::Replied: + return QByteArray( + "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"16\" height=\"16\" " + "viewBox=\"0 0 16 16\"> <path fill=\"currentColor\" d=\"M 6.8,2.2 V 5.0 " + "H 8.7 C 11.9,5.0 14.2,7.4 14.2,10.8 V 13.8 a 0.9,0.9 0 0 1 -1.75,0.28 C " + "11.8,12.1 10.4,10.9 8.7,10.9 H 6.8 V 13.7 L 1.0,7.95 Z\"/> </svg>"); + case Mark::ExpanderCollapsed: + return QByteArray( + "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"16\" height=\"16\" " + "viewBox=\"0 0 16 16\"> <path fill=\"currentColor\" d=\"M 5.38,2.38 " + "12.25,8 5.38,13.62 Z\"/> </svg>"); + case Mark::ExpanderExpanded: + return QByteArray( + "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"16\" height=\"16\" " + "viewBox=\"0 0 16 16\"> <path fill=\"currentColor\" d=\"M 2.38,5.38 " + "13.62,5.38 8,12.25 Z\"/> </svg>"); } + return {}; +} + +namespace { + +/// Key for the pixmap cache. The colour belongs in it because the mark is +/// recoloured per palette, and the ratio because a pixmap rendered for a 1x +/// screen is blurry on a 2x one. +struct CacheKey +{ + Mark mark; + int width; + int height; + QRgb color; + int ratio; ///< devicePixelRatio scaled by 100, so it can be hashed. + + bool operator==(const CacheKey &other) const + { + return mark == other.mark && width == other.width + && height == other.height && color == other.color + && ratio == other.ratio; + } +}; + +size_t qHash(const CacheKey &key, size_t seed = 0) +{ + return qHashMulti(seed, static_cast<int>(key.mark), key.width, key.height, + key.color, key.ratio); +} + +} // namespace + +QPixmap pixmap(Mark mark, const QSize &size, const QColor &color, + qreal devicePixelRatio) +{ + if (size.isEmpty() || !color.isValid()) + return {}; + + static QHash<CacheKey, QPixmap> cache; + + const CacheKey key{ mark, size.width(), size.height(), color.rgba(), + qRound(devicePixelRatio * 100) }; + const auto cached = cache.constFind(key); + if (cached != cache.constEnd()) + return *cached; + + QPixmap pm(size * devicePixelRatio); + pm.setDevicePixelRatio(devicePixelRatio); + pm.fill(Qt::transparent); + + { + QSvgRenderer renderer(svg(mark)); + QPainter painter(&pm); + painter.setRenderHint(QPainter::Antialiasing, true); + renderer.render(&painter, QRectF(QPointF(0, 0), QSizeF(size))); + + // The payloads paint with fill="currentColor", which QSvgRenderer does + // not resolve: it renders them black. SourceIn keeps the alpha the + // shape just produced and replaces the colour, which is what makes one + // asset serve both a light and a dark palette. + painter.setCompositionMode(QPainter::CompositionMode_SourceIn); + painter.fillRect(QRect(QPoint(0, 0), size), color); + } + + // Unbounded in principle, bounded in practice: the marks are six, the sizes + // come from a handful of font heights, and the colours from the palette. + cache.insert(key, pm); + return pm; +} + +void paint(QPainter *painter, const QRect &rect, Mark mark, const QColor &color) +{ + if (!painter || rect.isEmpty()) + return; + + // Square, sized to the shorter side, so a mark never stretches. The rects + // CardLayout reserves are square already; the message pane's are not + // necessarily. + const int side = qMin(rect.width(), rect.height()); + const QSize size(side, side); + const QPixmap pm = pixmap(mark, size, color, + painter->device()->devicePixelRatioF()); + if (pm.isNull()) + return; + + const QPoint at(rect.left() + (rect.width() - side) / 2, + rect.top() + (rect.height() - side) / 2); + painter->drawPixmap(at, pm); +} + +} // namespace Marks diff --git a/src/marks.h b/src/marks.h new file mode 100644 index 0000000..823c129 --- /dev/null +++ b/src/marks.h @@ -0,0 +1,83 @@ +/* + * qtmaildir - a Qt6 GUI for a local notmuch-indexed Maildir + * 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. + */ + +#ifndef QTMAILDIR_MARKS_H +#define QTMAILDIR_MARKS_H + +#include <QColor> +#include <QPixmap> +#include <QSize> + +class QPainter; +class QRect; + +/// The pane marks: the small state icons drawn on a card and in the message +/// pane's header. +/// +/// These are qtmaildir's own, deliberately, and that is the point of item 70. +/// Toolbar and menu icons come from QIcon::fromTheme and follow the user's icon +/// theme; the panes must not, because a mark that changes shape under a theme +/// change is a mark whose meaning the application cannot state. They are also +/// no longer font glyphs: U+1F4CE and U+2605 render at the mercy of whatever +/// font the desktop supplies, and both fell back to a bare "*" on a font that +/// lacked them, which made a flagged thread and one with an attachment +/// indistinguishable. +/// +/// The SVG payloads are compiled in as string literals rather than loaded from +/// a .qrc, and the reason is in src/CMakeLists.txt: a qrc compiled into the +/// static library registers itself from a global initialiser that the linker +/// drops, so resources belong to the executable. The tests link the library, +/// not the executable, so a resource-based mark would be absent exactly where +/// it needs asserting. The editable originals live in assets/icons/marks/ and +/// are the source these were taken from. +namespace Marks { + +enum class Mark { + Attachment, + Flagged, + Passed, + Replied, + ExpanderCollapsed, + ExpanderExpanded, +}; + +/// The mark rendered at `size`, filled with `color`. +/// +/// Recoloured rather than shipped in light and dark variants: every payload +/// paints with fill="currentColor", which QSvgRenderer does not resolve, so the +/// colour is composited in. One asset then serves both palettes and cannot fall +/// out of step with itself. +/// +/// Cached by (mark, size, colour, devicePixelRatio). A delegate paints these on +/// every row of every repaint, and re-parsing six XML documents per frame is +/// the kind of cost that does not show up until a list is long. +QPixmap pixmap(Mark mark, const QSize &size, const QColor &color, + qreal devicePixelRatio = 1.0); + +/// Paints `mark` centred in `rect`, scaled to fit its shorter side. +void paint(QPainter *painter, const QRect &rect, Mark mark, + const QColor &color); + +/// The raw SVG payload, exposed for tests and for the message pane, which +/// embeds marks as data: URIs in the header label's rich text rather than +/// painting them. +QByteArray svg(Mark mark); + +} // namespace Marks + +#endif // QTMAILDIR_MARKS_H diff --git a/src/messageview.cpp b/src/messageview.cpp index b6c3fa3..d2be380 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -25,6 +25,7 @@ #include <QFontDatabase> #include <QPlainTextEdit> #include <QDir> +#include <QBuffer> #include <QFileDialog> #include <QHBoxLayout> #include <QLabel> @@ -392,6 +393,39 @@ void MessageView::showError(const QString &text, const QString &filePath) setDocument(html); } +QString MessageView::headerMark(Marks::Mark mark) const +{ + // A data: URI rather than a resource path, for the same reason the marks + // are compiled in rather than shipped in a .qrc, and one more besides: this + // string goes into a QLabel's rich text, and Qt resolves a src= against the + // resource system only when one is registered. The image travels with the + // markup instead. + // + // Rendered at the label's OWN text colour so the mark tracks the palette + // exactly as the subject beside it does, on a light or a dark theme. + const int side = QFontMetrics(m_headerLabel->font()).ascent(); + const QPixmap pm = Marks::pixmap(mark, QSize(side, side), + m_headerLabel->palette().color( + QPalette::WindowText), + m_headerLabel->devicePixelRatioF()); + if (pm.isNull()) + return {}; + + QByteArray png; + QBuffer buffer(&png); + buffer.open(QIODevice::WriteOnly); + if (!pm.save(&buffer, "PNG")) + return {}; + + // A hair of margin on both sides, so a mark does not touch the subject. + return QStringLiteral( + "<img src=\"data:image/png;base64,%1\" width=\"%2\" " + "height=\"%3\" style=\"vertical-align: middle;\"> ") + .arg(QString::fromLatin1(png.toBase64())) + .arg(side) + .arg(side); +} + void MessageView::updateHeader() { if (m_items.isEmpty()) { @@ -406,7 +440,30 @@ void MessageView::updateHeader() // Re: prefixes that add nothing. const QString subject = m_items.first().message.subject; - QString text = QStringLiteral("<b>%1</b>").arg(subject.toHtmlEscaped()); + // Item 70's marks, beside the subject and OUTSIDE the message area. The + // user asked for these two only: whether the thread is flagged and whether + // it carries an attachment, which are the two states worth knowing before + // reading. They belong to the header label, which is application chrome, + // rather than to the generated document, which is untrusted content in a + // sandboxed web view. + // + // Any message in the thread having the state marks the whole thread, since + // the header describes the thread: an attachment on reply four is still an + // attachment the reader wants to know about. + const bool anyFlagged = std::any_of( + m_items.cbegin(), m_items.cend(), + [](const ThreadRenderItem &item) { return item.flagged; }); + const bool anyAttachment = std::any_of( + m_items.cbegin(), m_items.cend(), [](const ThreadRenderItem &item) { + return !item.message.attachments.isEmpty(); + }); + + QString text; + if (anyFlagged) + text += headerMark(Marks::Mark::Flagged); + text += QStringLiteral("<b>%1</b>").arg(subject.toHtmlEscaped()); + if (anyAttachment) + text += headerMark(Marks::Mark::Attachment); // The header adapts to what it can say honestly. From, To and Cc are // per-message, and the pane shows a whole thread, so they are only diff --git a/src/messageview.h b/src/messageview.h index 63fd6d8..422869a 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -23,6 +23,7 @@ #include <QWidget> #include "htmlbuilder.h" +#include "marks.h" #include "mimeparser.h" class QLabel; @@ -175,6 +176,10 @@ protected: private: void render(); void updateHeader(); + + /// One header mark as an <img> data: URI, sized and coloured to the header + /// label's own font and palette. Empty when the mark cannot be rendered. + QString headerMark(Marks::Mark mark) const; void setDocument(const QString &html); /// Rebuilds the attachment bar from m_items. Called from render(), so a diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 5874648..20d6915 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -27,25 +27,27 @@ #include <QFontDatabase> #include <QFontMetrics> -QString ThreadListModel::attachmentGlyph() +namespace { + +/// Whether a tag is already drawn on the card as a mark, and so must not also +/// appear as a chip. +/// +/// One list, consulted by both PillTagsRole (a thread's chips) and +/// MessageOwnTagsRole (a reply's own chips). Two copies drifted apart is +/// exactly how a tag ends up drawn twice on one row and not at all on another. +bool isDrawnAsAMark(const QString &tag) { - // U+1F4CE PAPERCLIP, with a fallback for a system whose default font - // cannot draw it: an unrenderable codepoint shows as a tofu box, which - // reads as "something is broken" rather than "this has an attachment". - // Computed once; the font does not change under a running application. - static const QString glyph = [] { - const char32_t paperclip = 0x1F4CE; - const QString preferred = QString::fromUcs4(&paperclip, 1); - const QFontMetrics metrics{QFontDatabase::systemFont( - QFontDatabase::GeneralFont)}; - // "*" as the fallback: ASCII, present in every practical font, and - // unambiguous in a column that shows nothing else. - return metrics.inFontUcs4(paperclip) ? preferred - : QStringLiteral("*"); - }(); - return glyph; + static const QStringList marks = { + QStringLiteral("flagged"), + QStringLiteral("attachment"), + QStringLiteral("passed"), + QStringLiteral("replied"), + }; + return marks.contains(tag); } +} // namespace + QColor ThreadListModel::deletedColour() { // Desaturated crimson: legible under white text on a dark theme, and calm @@ -61,22 +63,6 @@ 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 small size 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::replyBackground() { // Mixed from the palette rather than fixed, for the same reason as @@ -308,6 +294,13 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const : QStringList(); QStringList own; for (const QString &tag : node.tags) { + // Drawn as a mark on this row's own line two since items 69 + // and 70, so a chip would repeat it. Filtered here as well as + // in PillTagsRole because "own" is a difference against the + // THREAD, and a reply that is flagged where its thread is not + // would otherwise show both the mark and the word. + if (isDrawnAsAMark(tag)) + continue; if (!threadTags.contains(tag)) own.append(tag); } @@ -343,6 +336,10 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const return node.hasAttachment(); case IsFlaggedRole: return node.isFlagged(); + case IsPassedRole: + return node.isPassed(); + case IsRepliedRole: + return node.isReplied(); case ReplyCountRole: // A reply never offers an expander: nesting past the first level is // drawn from depth, not from further parent-child structure. @@ -425,16 +422,20 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const // // 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. + // passed and replied joined this list with item 69: they are drawn + // marks on line two now, so a chip repeating the word is the same + // duplication flagged and attachment were already dropped for. This is + // what item 69 asked for, "tags like Passed and Replied should use + // icons instead", and the chip has to go or both appear at once. 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)) + if (hidden.contains(tag) || isDrawnAsAMark(tag) + || TagColors::isAccountTag(tag)) continue; pills.append(tag); } @@ -512,6 +513,10 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const return thread.hasAttachment(); case IsFlaggedRole: return thread.isFlagged(); + case IsPassedRole: + return thread.isPassed(); + case IsRepliedRole: + return thread.isReplied(); case ReplyCountRole: // Zero in a flat list, so the card draws no expander pill. The count // and hasChildren() must agree: a card advertising "3 replies" that diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 8b8f414..2b8d2b0 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -115,6 +115,14 @@ public: DateRole, ///< A QDateTime. The delegate formats it. HasAttachmentRole, ///< bool IsFlaggedRole, ///< bool + + /// bool; the message was forwarded, from the Maildir "P" flag. + /// Item 69 draws this as a mark where it used to read as the word + /// "passed" in the tag strip. + IsPassedRole, + + /// bool; the message was replied to, from the Maildir "R" flag. + IsRepliedRole, ReplyCountRole, ///< int; 0 when a thread has no replies. /// The [general] date_format pattern, or empty for the system's short @@ -126,13 +134,7 @@ public: DateFormatRole, }; - /// The mark drawn on a card's second line when the message has an - /// attachment. A paperclip when the system font can draw it, "*" otherwise. - static QString attachmentGlyph(); - /// The mark drawn on a card's second line when the message is flagged. - /// A star when the system font can draw it, "*" otherwise. - static QString flagGlyph(); /// Row fill for a thread tagged `deleted`, and for one tagged `spam`. /// Muted rather than saturated: a bulk delete paints every selected row, diff --git a/src/types.h b/src/types.h index 2211619..97ab43f 100644 --- a/src/types.h +++ b/src/types.h @@ -47,6 +47,14 @@ struct ThreadSummary bool isUnread() const { return tags.contains(QStringLiteral("unread")); } bool isFlagged() const { return tags.contains(QStringLiteral("flagged")); } + /// True when this message was forwarded. The Maildir "P" (passed) flag, + /// which notmuch translates to a tag under maildir.synchronize_flags. + /// Item 68 measured the whole database: nothing derives this from a + /// subject line, so a "Fwd:" subject with no flag is correctly unmarked. + bool isPassed() const { return tags.contains(QStringLiteral("passed")); } + + /// True when this message was replied to. The Maildir "R" flag. + bool isReplied() const { return tags.contains(QStringLiteral("replied")); } bool isDeleted() const { return tags.contains(QStringLiteral("deleted")); } bool isSpam() const { return tags.contains(QStringLiteral("spam")); } @@ -73,6 +81,11 @@ struct MessageRef /// being pulled in only because a sibling in its thread matched. Drives /// whether it renders expanded or as a stub. bool matched = true; + + /// For the message pane header's flagged mark (item 70). The tags are + /// already carried, so this is the same predicate MessageNode and + /// ThreadSummary offer rather than new state. + bool isFlagged() const { return tags.contains(QStringLiteral("flagged")); } }; /// One message as a row in the thread list. @@ -97,6 +110,14 @@ struct MessageNode bool isUnread() const { return tags.contains(QStringLiteral("unread")); } bool isFlagged() const { return tags.contains(QStringLiteral("flagged")); } + /// True when this message was forwarded. The Maildir "P" (passed) flag, + /// which notmuch translates to a tag under maildir.synchronize_flags. + /// Item 68 measured the whole database: nothing derives this from a + /// subject line, so a "Fwd:" subject with no flag is correctly unmarked. + bool isPassed() const { return tags.contains(QStringLiteral("passed")); } + + /// True when this message was replied to. The Maildir "R" flag. + bool isReplied() const { return tags.contains(QStringLiteral("replied")); } /// notmuch applies "attachment" while indexing, so this needs no MIME /// parsing, exactly as on ThreadSummary. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 13063e9..06cc739 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -42,6 +42,7 @@ add_qtmaildir_test(htmlbuilder) add_qtmaildir_test(notmuchworker) add_qtmaildir_test(tagcolors) add_qtmaildir_test(cardlayout) +add_qtmaildir_test(marks) add_qtmaildir_test(carddelegate) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) diff --git a/tests/test_cardlayout.cpp b/tests/test_cardlayout.cpp index bce6a0f..f5f40ab 100644 --- a/tests/test_cardlayout.cpp +++ b/tests/test_cardlayout.cpp @@ -35,6 +35,10 @@ private slots: void expanderSitsOnTheSecondLine(); void expanderIsEmptyWithoutReplies(); void theExpanderReadsAsAPillWithAWord(); + void marksReserveTheirOwnSpaceRatherThanOverlappingTheSubject(); + void anAbsentMarkReservesNothing(); + void theFlagIndentsTheSubjectRatherThanSittingOnIt(); + void marksDoNotCollideWithEachOtherOrTheExpander(); void dateIsFlushRight(); void threadCardCarriesAnAccentBar(); void replyCardCarriesNoAccentBar(); @@ -228,15 +232,22 @@ void TestCardLayout::theExpanderReadsAsAPillWithAWord() // A bare "3" beside the subject reads as an unexplained number and gives // no hint that it can be clicked. The label carries the word, and the rect // carries padding for the pill drawn behind it. - QCOMPARE(CardLayout::expanderLabel(3, false), - QStringLiteral("\u25b8 3 replies")); - QCOMPARE(CardLayout::expanderLabel(3, true), - QStringLiteral("\u25be 3 replies")); + // + // NO triangle in the label since item 70: it is a drawn mark now, and a + // glyph left here would be a second triangle beside the drawn one. The + // label is the words alone, and the state no longer changes it. + QCOMPARE(CardLayout::expanderLabel(3, false), QStringLiteral("3 replies")); + QCOMPARE(CardLayout::expanderLabel(3, true), QStringLiteral("3 replies")); // Singular, because "1 replies" is the kind of detail that makes an // interface look unfinished. - QCOMPARE(CardLayout::expanderLabel(1, false), - QStringLiteral("\u25b8 1 reply")); + QCOMPARE(CardLayout::expanderLabel(1, false), QStringLiteral("1 reply")); + + // The glyphs are gone from the label entirely. Asserted rather than assumed, + // because a stray one would draw underneath the mark and look like a + // rendering fault rather than like a stale string. + QVERIFY(!CardLayout::expanderLabel(3, false).contains(QChar(0x25b8))); + QVERIFY(!CardLayout::expanderLabel(3, true).contains(QChar(0x25be))); const QFont font; const int h = CardLayout::heightFor(font); @@ -244,12 +255,17 @@ void TestCardLayout::theExpanderReadsAsAPillWithAWord() CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font); const QFontMetrics small(CardLayout::smallFont(font)); - // The rect must hold the label AND its padding, or the pill's background - // is narrower than the text sitting on it. + // The rect must hold the label, the drawn triangle, the gap between them + // AND the padding, or the pill's background is narrower than what sits on + // it. The triangle's width came free from the text metrics while it was a + // glyph in the label; since item 70 it is reserved explicitly, and this is + // what would catch it being forgotten. QVERIFY2(card.expanderRect.width() >= small.horizontalAdvance(CardLayout::expanderLabel(3, false)) + + small.ascent() + CardLayout::kMarkGap + CardLayout::kPillPaddingX * 2, - "the expander rect is too narrow for its own label and padding"); + "the expander rect is too narrow for its label, its triangle and " + "its padding"); // And it must NOT change width when the card opens: a pill that resized on // click would shift the subject's elision under the pointer. @@ -259,6 +275,145 @@ void TestCardLayout::theExpanderReadsAsAPillWithAWord() QCOMPARE(expanded.expanderRect.width(), card.expanderRect.width()); } +void TestCardLayout::marksReserveTheirOwnSpaceRatherThanOverlappingTheSubject() +{ + // Item 70. The marks were glyphs INSIDE the subject string until then, so + // their width came free from the text metrics and no arrangement was + // needed. As drawn icons they occupy rects, and a subject sized as though + // they were absent runs underneath them. This is the assertion that would + // catch that, and it cannot be made anywhere else: a rendering probe over + // the delegate would show overlapping ink as a plausible-looking card. + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + CardLayout::Input bare = threadInput(); + CardLayout::Input marked = threadInput(); + marked.hasAttachment = true; + marked.passed = true; + marked.replied = true; + + const CardLayout without = CardLayout::compute(bare, rect, font); + const CardLayout with = CardLayout::compute(marked, rect, font); + + QVERIFY(!with.attachmentRect.isEmpty()); + QVERIFY(!with.passedRect.isEmpty()); + QVERIFY(!with.repliedRect.isEmpty()); + + // The subject gives up exactly the room the marks take. + QVERIFY2(with.subjectRect.width() < without.subjectRect.width(), + "the marks reserved no space, so the subject is sized as though " + "they were not there and its text runs underneath them"); + + // And every mark begins after the subject ends. Compared as exclusive + // edges: QRect::right() is inclusive, which is the trap this file already + // documents for the date. + const int subjectEnd = with.subjectRect.left() + with.subjectRect.width(); + QVERIFY2(with.attachmentRect.left() >= subjectEnd, + "the attachment mark overlaps the subject"); + QVERIFY2(with.passedRect.left() >= subjectEnd, "passed overlaps the subject"); + QVERIFY2(with.repliedRect.left() >= subjectEnd, + "replied overlaps the subject"); + + // Square, so nothing is drawn stretched. + QCOMPARE(with.attachmentRect.width(), with.attachmentRect.height()); +} + +void TestCardLayout::anAbsentMarkReservesNothing() +{ + // A card with no attachment must not leave a hole where the mark would be: + // the subject is the elastic part of line two and every reserved-but-unused + // pixel comes out of it. + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + const CardLayout card = CardLayout::compute(threadInput(), rect, font); + + QVERIFY(card.flagRect.isEmpty()); + QVERIFY(card.attachmentRect.isEmpty()); + QVERIFY(card.passedRect.isEmpty()); + QVERIFY(card.repliedRect.isEmpty()); + + // Guard: the same input WITH a mark must produce one, or the assertions + // above pass against a layout that never draws marks at all. + CardLayout::Input marked = threadInput(); + marked.hasAttachment = true; + QVERIFY(!CardLayout::compute(marked, rect, font).attachmentRect.isEmpty()); +} + +void TestCardLayout::theFlagIndentsTheSubjectRatherThanSittingOnIt() +{ + // The flag is the one mark on the LEFT, where its glyph was, so a flagged + // card still reads flagged from the left edge. + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + CardLayout::Input flagged = threadInput(); + flagged.flagged = true; + + const CardLayout plain = CardLayout::compute(threadInput(), rect, font); + const CardLayout marked = CardLayout::compute(flagged, rect, font); + + QVERIFY(!marked.flagRect.isEmpty()); + QCOMPARE(marked.flagRect.left(), marked.contentLeft); + + // The subject starts after the flag, rather than at contentLeft with the + // flag drawn over it. + QVERIFY2(marked.subjectRect.left() > plain.subjectRect.left(), + "the flag did not move the subject, so it is drawn on top of it"); + QVERIFY(marked.subjectRect.left() + >= marked.flagRect.left() + marked.flagRect.width()); +} + +void TestCardLayout::marksDoNotCollideWithEachOtherOrTheExpander() +{ + // All four marks at once on a card that also has an expander, which is the + // densest line two can get. Nothing may overlap anything. + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + CardLayout::Input in = threadInput(); + in.flagged = true; + in.hasAttachment = true; + in.passed = true; + in.replied = true; + + const CardLayout card = CardLayout::compute(in, rect, font); + + QVERIFY(!card.expanderRect.isEmpty()); + + // Left to right: flag, subject, attachment, passed, replied, expander. + const QList<QRect> ordered = { card.flagRect, card.subjectRect, + card.attachmentRect, card.passedRect, + card.repliedRect, card.expanderRect }; + for (int i = 0; i + 1 < ordered.size(); ++i) { + const QRect &left = ordered.at(i); + const QRect &right = ordered.at(i + 1); + QVERIFY2(left.left() + left.width() <= right.left(), + qPrintable(QStringLiteral("rect %1 (x %2 w %3) overlaps rect " + "%4 (x %5)") + .arg(i) + .arg(left.left()) + .arg(left.width()) + .arg(i + 1) + .arg(right.left()))); + } + + // And the whole line stays inside the card. + QVERIFY(card.repliedRect.left() + card.repliedRect.width() + <= card.expanderRect.left()); + QVERIFY(card.expanderRect.left() + card.expanderRect.width() + <= rect.right() + 1); + + // The subject survives at a usable width rather than being squeezed to + // nothing by four marks: they are small and fixed, it is the elastic part. + QVERIFY2(card.subjectRect.width() > 100, + "four marks left the subject with almost no room on a 400px card"); +} + void TestCardLayout::dateIsFlushRight() { const QFont font; diff --git a/tests/test_marks.cpp b/tests/test_marks.cpp new file mode 100644 index 0000000..2ed6580 --- /dev/null +++ b/tests/test_marks.cpp @@ -0,0 +1,286 @@ +/* + * qtmaildir - a Qt6 GUI for a local notmuch-indexed Maildir + * 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 "marks.h" + +#include <QImage> +#include <QPainter> +#include <QtTest> + +/// Counts pixels with any alpha at all. +/// +/// "Rendering probes lie" in CLAUDE.md is about probes over widgets, where a +/// blank result is more likely a broken probe than broken code. This one is +/// safe for the opposite reason: the input is a fixed SVG payload and a +/// transparent pixmap this test creates itself, with no widget, no exposure and +/// no viewport to come out empty. Every assertion below still states the ink it +/// expects to find before drawing a conclusion from ink it does not. +static int inkPixels(const QImage &image) +{ + int count = 0; + for (int y = 0; y < image.height(); ++y) { + for (int x = 0; x < image.width(); ++x) { + if (qAlpha(image.pixel(x, y)) > 0) + ++count; + } + } + return count; +} + +static QImage renderMark(Marks::Mark mark, int side = 64, + const QColor &color = Qt::black) +{ + return Marks::pixmap(mark, QSize(side, side), color).toImage(); +} + +class TestMarks : public QObject +{ + Q_OBJECT + +private slots: + void everyMarkHasAPayload(); + void everyMarkDrawsSomething(); + void marksAreRecolouredRatherThanShippedPerTheme(); + void theExpanderPairIsTheSameWeightInBothStates(); + void passedAndRepliedAreMirrorsOfEachOther(); + void aMarkIsDistinguishableFromEveryOther(); + void paintCentresTheMarkInItsRect(); + void anEmptySizeOrInvalidColourYieldsNothing(); +}; + +void TestMarks::everyMarkHasAPayload() +{ + // A missing case in the switch returns an empty QByteArray, which + // QSvgRenderer accepts and renders as nothing. That failure is silent + // everywhere else, so it is caught here first. + const QList<Marks::Mark> all = { + Marks::Mark::Attachment, Marks::Mark::Flagged, + Marks::Mark::Passed, Marks::Mark::Replied, + Marks::Mark::ExpanderCollapsed, Marks::Mark::ExpanderExpanded, + }; + + for (const Marks::Mark mark : all) { + const QByteArray payload = Marks::svg(mark); + QVERIFY2(!payload.isEmpty(), + qPrintable(QStringLiteral("mark %1 has no payload") + .arg(static_cast<int>(mark)))); + QVERIFY(payload.contains("<svg")); + // The recolouring in pixmap() depends on this: a payload that named a + // literal colour would ignore the palette and stay that colour on both + // themes. + QVERIFY2(payload.contains("currentColor"), + qPrintable(QStringLiteral("mark %1 does not paint with " + "currentColor, so it cannot be " + "recoloured") + .arg(static_cast<int>(mark)))); + } +} + +void TestMarks::everyMarkDrawsSomething() +{ + // The guard the rest of this file needs: a probe that cannot find ink where + // ink certainly exists is broken, and would pass every "differs from" + // assertion below by finding nothing anywhere. + const QList<QPair<Marks::Mark, QString>> all = { + { Marks::Mark::Attachment, QStringLiteral("attachment") }, + { Marks::Mark::Flagged, QStringLiteral("flagged") }, + { Marks::Mark::Passed, QStringLiteral("passed") }, + { Marks::Mark::Replied, QStringLiteral("replied") }, + { Marks::Mark::ExpanderCollapsed, QStringLiteral("expander-collapsed") }, + { Marks::Mark::ExpanderExpanded, QStringLiteral("expander-expanded") }, + }; + + for (const auto &[mark, name] : all) { + const QImage image = renderMark(mark); + QVERIFY2(!image.isNull(), qPrintable(name + QStringLiteral(" is null"))); + const int ink = inkPixels(image); + QVERIFY2(ink > 100, + qPrintable(QStringLiteral("%1 drew %2 ink pixels at 64x64, " + "which is a blank or near-blank " + "render") + .arg(name) + .arg(ink))); + } +} + +void TestMarks::marksAreRecolouredRatherThanShippedPerTheme() +{ + // One asset serves a light and a dark palette. The payload paints with + // currentColor, which QSvgRenderer renders BLACK rather than resolving, so + // without the SourceIn composite every mark would be black on both themes + // and invisible on a dark one. + const QImage light = renderMark(Marks::Mark::Flagged, 64, QColor(Qt::white)); + const QImage dark = renderMark(Marks::Mark::Flagged, 64, QColor(Qt::black)); + + QCOMPARE(inkPixels(light), inkPixels(dark)); // same shape + + // Find a pixel the shape actually covers and compare the colour there. + // Sampling a fixed coordinate would risk landing outside the star. + bool sampled = false; + for (int y = 0; y < light.height() && !sampled; ++y) { + for (int x = 0; x < light.width() && !sampled; ++x) { + if (qAlpha(light.pixel(x, y)) != 255) + continue; + const QRgb lit = light.pixel(x, y); + const QRgb unlit = dark.pixel(x, y); + QVERIFY2(qRed(lit) > 200 && qGreen(lit) > 200 && qBlue(lit) > 200, + "the white request did not produce a white mark"); + QVERIFY2(qRed(unlit) < 50 && qGreen(unlit) < 50 && qBlue(unlit) < 50, + "the black request did not produce a black mark"); + sampled = true; + } + } + QVERIFY2(sampled, "no fully opaque pixel found, so nothing was compared"); +} + +void TestMarks::theExpanderPairIsTheSameWeightInBothStates() +{ + // The expanded triangle is the collapsed one rotated 90 degrees about the + // centre, so neither state can read as heavier than the other. Asserted as + // equal ink rather than by eye, and it is the property most easily lost by + // hand-editing one of the two paths. + const int collapsed = inkPixels(renderMark(Marks::Mark::ExpanderCollapsed)); + const int expanded = inkPixels(renderMark(Marks::Mark::ExpanderExpanded)); + + QVERIFY2(collapsed > 0 && expanded > 0, "an expander drew nothing"); + + // Not exactly equal: antialiasing along a rotated edge differs by a few + // pixels. 2% is far tighter than any real weight difference would be. + const double ratio = double(qAbs(collapsed - expanded)) + / double(qMax(collapsed, expanded)); + QVERIFY2(ratio < 0.02, + qPrintable(QStringLiteral("expander states differ in weight: %1 " + "against %2 ink pixels") + .arg(collapsed) + .arg(expanded))); +} + +void TestMarks::passedAndRepliedAreMirrorsOfEachOther() +{ + // Item 69 wants these two to read as one pair. They are mirrors about + // x = 8, so mirroring one must reproduce the other; a hand edit to one + // alone would break the pairing while leaving both looking plausible. + const QImage passed = renderMark(Marks::Mark::Passed); + const QImage replied = renderMark(Marks::Mark::Replied); + + QVERIFY(inkPixels(passed) > 100); + + // Near-equal, not equal. These are mirrored CURVES, and the rasteriser + // antialiases a curve and its mirror slightly differently: measured 1383 + // against 1397 at 64x64, a 1% difference that says nothing about the + // shapes. The pixel-by-pixel comparison below is the assertion that would + // actually catch a broken pair; this one only rejects a gross weight + // difference. + const int passedInk = inkPixels(passed); + const int repliedInk = inkPixels(replied); + const double weightRatio = double(qAbs(passedInk - repliedInk)) + / double(qMax(passedInk, repliedInk)); + QVERIFY2(weightRatio < 0.02, + qPrintable(QStringLiteral("passed and replied differ in weight: " + "%1 against %2 ink pixels") + .arg(passedInk) + .arg(repliedInk))); + + const QImage mirrored = passed.mirrored(true, false); + QCOMPARE(mirrored.size(), replied.size()); + + // Compared on alpha rather than on exact pixels: mirroring resamples the + // antialiased edges, so a strict image equality would fail on a correct + // pair. A shape mismatch shows up as a large disagreeing area, not a few + // edge pixels. + int disagreeing = 0; + for (int y = 0; y < replied.height(); ++y) { + for (int x = 0; x < replied.width(); ++x) { + const int a = qAlpha(mirrored.pixel(x, y)) > 127 ? 1 : 0; + const int b = qAlpha(replied.pixel(x, y)) > 127 ? 1 : 0; + if (a != b) + ++disagreeing; + } + } + const double fraction = double(disagreeing) + / double(replied.width() * replied.height()); + QVERIFY2(fraction < 0.02, + qPrintable(QStringLiteral("passed mirrored does not match replied: " + "%1% of pixels disagree") + .arg(fraction * 100, 0, 'f', 1))); +} + +void TestMarks::aMarkIsDistinguishableFromEveryOther() +{ + // The defect the glyphs had: an unrenderable codepoint fell back to "*" for + // BOTH the star and the paperclip, so a flagged thread and one carrying an + // attachment looked identical. Whatever else changes about these marks, no + // two may render the same. + const QList<QPair<Marks::Mark, QString>> all = { + { Marks::Mark::Attachment, QStringLiteral("attachment") }, + { Marks::Mark::Flagged, QStringLiteral("flagged") }, + { Marks::Mark::Passed, QStringLiteral("passed") }, + { Marks::Mark::Replied, QStringLiteral("replied") }, + { Marks::Mark::ExpanderCollapsed, QStringLiteral("expander-collapsed") }, + { Marks::Mark::ExpanderExpanded, QStringLiteral("expander-expanded") }, + }; + + for (int i = 0; i < all.size(); ++i) { + for (int j = i + 1; j < all.size(); ++j) { + const QImage a = renderMark(all.at(i).first); + const QImage b = renderMark(all.at(j).first); + QVERIFY2(a != b, + qPrintable(QStringLiteral("%1 and %2 render identically") + .arg(all.at(i).second, all.at(j).second))); + } + } +} + +void TestMarks::paintCentresTheMarkInItsRect() +{ + // paint() is what the delegate calls, and it must not stretch a mark to a + // non-square rect: the message pane's rects are not square. + QImage canvas(80, 40, QImage::Format_ARGB32_Premultiplied); + canvas.fill(Qt::transparent); + + { + QPainter painter(&canvas); + Marks::paint(&painter, QRect(0, 0, 80, 40), Marks::Mark::Flagged, + QColor(Qt::black)); + } + + const int ink = inkPixels(canvas); + QVERIFY2(ink > 50, "paint() drew nothing into the canvas"); + + // Sized to the SHORTER side, so nothing is drawn outside a centred 40x40 + // square. Columns outside it must be empty. + for (int y = 0; y < canvas.height(); ++y) { + for (int x = 0; x < 20; ++x) { + QVERIFY2(qAlpha(canvas.pixel(x, y)) == 0, + "the mark was stretched past its square, so a non-square " + "rect distorts it"); + } + for (int x = 60; x < canvas.width(); ++x) + QVERIFY(qAlpha(canvas.pixel(x, y)) == 0); + } +} + +void TestMarks::anEmptySizeOrInvalidColourYieldsNothing() +{ + // Rather than asserting or painting at a garbage size. + QVERIFY(Marks::pixmap(Marks::Mark::Flagged, QSize(0, 0), Qt::black).isNull()); + QVERIFY(Marks::pixmap(Marks::Mark::Flagged, QSize(16, 16), QColor()).isNull()); +} + +QTEST_MAIN(TestMarks) +#include "test_marks.moc" diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 1fb8a1f..e338661 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -54,6 +54,7 @@ private slots: void readThreadsAreDimmedAndUnreadAreNot(); void flaggedThreadsShowAStar(); void pillTagsExcludeWhatTheRowAlreadyShows(); + void aTagDrawnAsAMarkIsNotAlsoAChip(); void theUnreadCueDoesNotDependOnFontWeight(); void aDoomedThreadKeepsItsContrastEvenWhenRead(); void accountTagBecomesAChipLabel(); @@ -583,10 +584,6 @@ void TestThreadListModel::flaggedThreadsShowAStar() ThreadListModel::IsFlaggedRole).toBool(), "a flagged thread does not report itself flagged"); - // The glyph the delegate draws from that flag must be something a font can - // render: an unrenderable codepoint shows as tofu, which reads as - // breakage rather than as a mark. - QVERIFY(!ThreadListModel::flagGlyph().isEmpty()); } void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows() @@ -1001,16 +998,47 @@ void TestThreadListModel::attachmentIsMarkedOnlyOnTaggedThreads() QVERIFY(!model.data(plainCell, ThreadListModel::HasAttachmentRole).toBool()); QVERIFY(model.data(fileCell, ThreadListModel::HasAttachmentRole).toBool()); - // The glyph must be something a font can draw. An unrenderable codepoint - // shows as a tofu box, which reads as breakage rather than as a marker. - QVERIFY(!ThreadListModel::attachmentGlyph().isEmpty()); - // Only the marked thread gets a tooltip, or an empty cell would claim to // have an attachment on hover. QVERIFY(model.data(plainCell, Qt::ToolTipRole).toString().isEmpty()); QVERIFY(!model.data(fileCell, Qt::ToolTipRole).toString().isEmpty()); } +void TestThreadListModel::aTagDrawnAsAMarkIsNotAlsoAChip() +{ + // Items 69 and 70. passed and replied are drawn marks on line two now, so a + // chip repeating the word puts the same fact on the card twice. Caught by + // rendering a real card rather than by any existing test, which is why this + // one exists: every geometry assertion passed while the row said "passed" + // as both an arrow and a green pill. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), + QStringLiteral("subject")); + thread.tags = { QStringLiteral("inbox"), QStringLiteral("flagged"), + QStringLiteral("attachment"), QStringLiteral("passed"), + QStringLiteral("replied"), QStringLiteral("project") }; + model.appendBatch({ thread }); + + const QStringList pills = + model.data(model.index(0, 0), ThreadListModel::PillTagsRole) + .toStringList(); + + for (const QString &drawn : { QStringLiteral("flagged"), + QStringLiteral("attachment"), + QStringLiteral("passed"), + QStringLiteral("replied") }) { + QVERIFY2(!pills.contains(drawn), + qPrintable(QStringLiteral("'%1' is drawn as a mark and still " + "appears as a chip") + .arg(drawn))); + } + + // Guard: a tag with no mark still becomes a chip, or the assertions above + // would pass against a model that dropped every pill. + QVERIFY2(pills.contains(QStringLiteral("project")), + "an ordinary tag lost its chip, so the filter is too broad"); +} + void TestThreadListModel::modelHasOneColumn() { ThreadListModel model; |
