From 019117aa8e52ce39cab58f77b57a9a67f510696f Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sun, 16 Aug 2026 21:58:18 +0200 Subject: feat(ui): act on the message a row displays, not its whole thread A thread's card has rendered one message since item 66, but every tag action still acted on the entire conversation. Delete, Archive, Important, Mark spam and Toggle unread now act on the message the card shows; the whole-thread versions move to a "Whole thread" submenu in the Message menu and the thread list's context menu, on Ctrl+Alt+. Closes items 87, 88, 105, 106, 107, 108, 109, 110 and 111. The defects fixed along the way, several found by reading rather than by report: - threadAt(current.row()) answered about the wrong thread for a reply row, because a tree numbers rows per parent. The audit found four live sites, not the one reported: Delete and Toggle unread each chose their DIRECTION from an unrelated thread, and the tag dialog counted the wrong thread's tags. threadFor(index) replaces them. - A message-scoped write made no optimistic model update and no reply row carried a doomed cue, so acting on a reply moved the pending-edit count and changed nothing on screen. - Both toggles read the state of a reply's THREAD, which a message-scoped write never changes, so they were one-way: the second press re-sent a tag the message already had. - flushHeldEdits() re-sent only thread-scoped edits, so a tag change made on one message during a sync was applied to the row, counted as unsynced, and then dropped without ever being written. - applyTagChange() updated a thread's summary but not its loaded replies, leaving an expanded thread's rows describing a state the database no longer held. - A thread's first message is not among its children, so both message-scoped lookups missed it: acting on a root card repainted nothing and emptied the message pane's chip row. - ThreadSummary::tags is notmuch's union over the thread, so a card standing for one message drew tags belonging to its siblings. The worker now reads that message's own tags in the walk that already finds its id, so the split is known before a row is ever opened. The card shows both tiers: its own message's tags at full size, the rest of the conversation's smaller and muted, so nothing appears to vanish when a row is selected. Auto mark-read is message-scoped as a result, and now arms for a reply, which it never did. With maildir.synchronize_flags on, the old thread-wide write reached the server for mail that had never been displayed. Co-Authored-By: Claude Opus 5 --- tests/test_carddelegate.cpp | 128 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) (limited to 'tests/test_carddelegate.cpp') diff --git a/tests/test_carddelegate.cpp b/tests/test_carddelegate.cpp index d5e55b5..a14671d 100644 --- a/tests/test_carddelegate.cpp +++ b/tests/test_carddelegate.cpp @@ -18,6 +18,9 @@ #include "carddelegate.h" +#include "cardlayout.h" +#include "tagchip.h" +#include "tagcolors.h" #include "threadlistmodel.h" #include @@ -30,6 +33,9 @@ private slots: void theAccentLiftsAMutedAccountColour(); void theAccentKeepsEachAccountTellableApart(); void anAccountWithNoColourFallsBackToTheNeutralLine(); + void aSiblingChipIsMutedButStaysLegibleAndRecognisable(); + void aSiblingChipFontIsSmallerThanItsOwnTier(); + void aSiblingChipsPaddingShrinksWithItsFont(); }; namespace { @@ -131,5 +137,127 @@ void TestCardDelegate::anAccountWithNoColourFallsBackToTheNeutralLine() ThreadListModel::threadLineColour()); } +void TestCardDelegate::aSiblingChipIsMutedButStaysLegibleAndRecognisable() +{ + // Item 111: a card shows its own tags at full size and the rest of the + // conversation's smaller and muted. "Muted" has two hard requirements that + // a look at the screen will not catch, so they are asserted here. + for (const QColor &colour : sampleAccounts()) { + const QColor muted = CardDelegate::mutedChipColour(colour); + + // Actually muted, or the tier is not distinguishable at all. + QVERIFY2(saturationOf(muted) < saturationOf(colour), + qPrintable(QStringLiteral("%1 was not drained at all") + .arg(colour.name()))); + + // Same HUE. A sibling's `signed` has to stay recognisably the same + // colour as a full-size `signed` elsewhere in the list, or the muting + // reads as a different tag rather than a quieter one. + float h1 = 0, h2 = 0, s = 0, l = 0, a = 0; + colour.getHslF(&h1, &s, &l, &a); + muted.getHslF(&h2, &s, &l, &a); + QVERIFY2(qAbs(h1 - h2) < 0.001f, + qPrintable(QStringLiteral("%1 changed hue when muted") + .arg(colour.name()))); + + // Same LIGHTNESS, which is what keeps the text legible: TagColors + // picks the text colour from the fill, and a fill that drifted toward + // black or white could flip that choice or land mid-grey where neither + // works. Blending toward the background would do exactly that, which + // is the mistake accentLineColour() records. + QCOMPARE(lightnessOf(muted), lightnessOf(colour)); + QCOMPARE(TagColors::textColourOn(muted), + TagColors::textColourOn(colour)); + } + + // An invalid colour stays invalid rather than becoming a real one. + QVERIFY(!CardDelegate::mutedChipColour(QColor()).isValid()); +} + +void TestCardDelegate::aSiblingChipFontIsSmallerThanItsOwnTier() +{ + // Size is what says whose tag a chip is, so the two tiers must differ, and + // by enough to SEE. The first version subtracted a point from smallFont(), + // and the user reported the tiers as indistinguishable: on their 14pt + // desktop that gave 13 and 12, a 7% step. + // + // The step is now a fraction of the card font, so it does not shrink as + // the desktop's font grows. Asserted as a ratio rather than as a size, to + // keep this about the DISTINCTION rather than about the constant. + QFont card; + card.setPointSizeF(14.0); // The user's own desktop size. + const qreal own = CardLayout::smallFont(card).pointSizeF(); + const qreal sibling = CardLayout::siblingFont(card).pointSizeF(); + + QVERIFY(sibling < own); + QVERIFY2(sibling < own * 0.85, + qPrintable(QStringLiteral("sibling %1pt against own %2pt is under " + "a 15%% step, which reads as the same " + "size") + .arg(sibling) + .arg(own))); + + // Proportional, not a fixed subtraction: the step must survive a larger + // desktop font rather than becoming proportionally smaller. + QFont big; + big.setPointSizeF(28.0); + QVERIFY(CardLayout::siblingFont(big).pointSizeF() + < CardLayout::smallFont(big).pointSizeF() * 0.85); + + // The pixel branch too: qt6ct sets fonts in PIXELS, and pointSizeF() is -1 + // for those, so a point-only implementation silently returns the original + // size and both tiers render identically. CLAUDE.md records this trap. + QFont pixels; + pixels.setPixelSize(14); + QVERIFY(pixels.pointSizeF() < 0); + QVERIFY2(CardLayout::siblingFont(pixels).pixelSize() + < CardLayout::smallFont(pixels).pixelSize(), + "a pixel-sized desktop font gives both tiers the same size, so " + "the distinction disappears entirely"); + + // Floored rather than shrinking without limit. + QFont tiny; + tiny.setPointSizeF(6.0); + QVERIFY(CardLayout::siblingFont(tiny).pointSizeF() >= 6.0); +} + +void TestCardDelegate::aSiblingChipsPaddingShrinksWithItsFont() +{ + // Half of "smaller" is the padding, and leaving it fixed is why the first + // version still looked the same size. kPaddingX is 9 a side: on a sibling + // chip that is 18px of padding around roughly 30px of text, so the chip + // stayed wide while its letters shrank, which reads as "same chip, smaller + // text" rather than as a smaller chip. + QFont card; + card.setPointSizeF(14.0); + const QFontMetrics ownMetrics(CardLayout::smallFont(card)); + const QFontMetrics siblingMetrics(CardLayout::siblingFont(card)); + + const QString tag = QStringLiteral("signed"); + // Through CardDelegate::chipSize(), which is what the paint loop calls. + // Calling TagChip::sizeFor() directly here proved what THAT function does + // and nothing about whether the delegate asks it for a scaled padding: a + // mutation dropping the scale at the call site survived that version of + // this test. + const QSize own = CardDelegate::chipSize(ownMetrics, tag, true); + const QSize scaled = CardDelegate::chipSize(siblingMetrics, tag, false); + const QSize unscaled = TagChip::sizeFor(siblingMetrics, tag); + + // The font alone is not enough: scaling the padding as well takes off + // measurably more width. + QVERIFY2(scaled.width() < unscaled.width(), + "the padding did not scale, so the chip keeps full-size margins " + "around smaller letters"); + QVERIFY(scaled.width() < own.width()); + QVERIFY(scaled.height() < own.height()); + + // Floored rather than collapsing to nothing: the corner radius is half the + // height, so a chip with no horizontal padding has its text on the curve. + const QSize tiny = TagChip::sizeFor(siblingMetrics, tag, 0.0); + QVERIFY2(tiny.width() > siblingMetrics.horizontalAdvance(tag), + "a zero scale left no horizontal padding at all, so the text sits " + "on the chip's rounded end"); +} + QTEST_MAIN(TestCardDelegate) #include "test_carddelegate.moc" -- cgit v1.2.3