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 /tests | |
| 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>
Diffstat (limited to 'tests')
| -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 |
4 files changed, 487 insertions, 17 deletions
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; |
