diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-11 20:24:05 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-11 20:24:05 +0200 |
| commit | 1faf94eb35e8270a659f215d260db73bcaa3f8d7 (patch) | |
| tree | 6305f6532cf1abc000eb261ce85c3ce12bc0a8d3 | |
| parent | 64d3138ba923071069da6c9bc458a25a9cc7d27f (diff) | |
| download | qtmaildir-1faf94eb35e8270a659f215d260db73bcaa3f8d7.tar.gz qtmaildir-1faf94eb35e8270a659f215d260db73bcaa3f8d7.zip | |
feat(panes): draw the pane marks from shipped SVGs, not font glyphs
Items 70 and 69, the second folded into the first as item 70's own size note
predicted it should be.
The panes drew their state marks as font glyphs: U+1F4CE for an attachment and
U+2605 for a flagged thread, each with a fallback for a font that cannot render
it. Both fell back to "*", so on such a font a flagged thread and one carrying
an attachment were indistinguishable, which is a defect the fallback introduced
rather than prevented. What a mark looks like was also the desktop's decision
rather than this application's, and the panes are exactly where it should not
be: the user asked for the toolbar and menus to keep following their icon theme
while the panes stop.
Six marks now ship in assets/icons/marks/: flagged, attachment, passed, replied
and the two expander triangles. QIcon::fromTheme still resolves every toolbar
and menu icon and was not touched.
Licensing chose the shapes. The look came from a GPL3 icon theme, and this
project is GPLv2-only, which are incompatible: GPLv2's "no further
restrictions" clause bars shipping GPL3 assets in a v2-only work. The six were
drawn fresh in the same idiom instead, with no path data copied. The idiom is
generic: solid single-path silhouettes at 16x16 with no strokes.
They are compiled in as string literals rather than loaded from a .qrc.
src/CMakeLists.txt already records why resources belong to the executable: a
qrc in the static library registers itself from a global initialiser the linker
drops. The tests link the library, so a resource-based mark would be missing
exactly where it needs asserting. assets/icons/marks/ stays the editable
source.
One asset serves both palettes. Every payload paints with fill="currentColor",
which QSvgRenderer renders black rather than resolving, so Marks::pixmap
composites the wanted colour with CompositionMode_SourceIn. A mark then takes
the card's own pen colour and follows selection and the read/unread dimming
without a second variant to keep in step.
CardLayout reserves a rect per mark and CardDelegate paints into it. 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 are there or the subject runs
underneath them. The expander pill had the same trap, its triangle being a
glyph in expanderLabel(), and now reserves that width explicitly.
Item 69's part: passed and replied were words in the tag strip and are marks
beside the subject now. The message pane's header carries the flagged and
attachment marks next to the subject, per the user's decision that the right
pane needs those two and only outside the message area.
A duplicate that no test caught is worth recording. 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. It was found by
rendering real cards to an image and looking at them. isDrawnAsAMark() is now
one list consulted by both PillTagsRole and MessageOwnTagsRole, since two
copies drifting apart is how a tag ends up drawn twice on one row and not at
all on another.
Fourteen tests: nine in test_marks, four in test_cardlayout, one in
test_threadlistmodel. Mutation-checked at four points, each failing a test: the
subject ignoring the marks, the flag not indenting the subject, the pill
forgetting the triangle's width, and the recolour composite removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| -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; |
