From 5f31a037a37a928a8e21fdf0950568a689e29fb8 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 28 Aug 2026 11:44:06 +0200 Subject: refactor: remove the sibling chip tier Nothing is a sibling any more: a conversation row draws the thread's tags and a message row draws its own. --- src/carddelegate.cpp | 67 ++++-------------------- src/carddelegate.h | 17 ++---- src/cardlayout.cpp | 26 --------- src/cardlayout.h | 13 ----- tests/test_carddelegate.cpp | 125 -------------------------------------------- 5 files changed, 12 insertions(+), 236 deletions(-) diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp index d0ee957..fbdf13f 100644 --- a/src/carddelegate.cpp +++ b/src/carddelegate.cpp @@ -35,12 +35,6 @@ namespace { -/// How much of a full-size chip's padding a SIBLING chip keeps. -/// -/// Matched to CardLayout::siblingFont()'s own scale, so the chip shrinks as a -/// whole rather than keeping full-size margins around smaller letters. -constexpr qreal kSiblingPaddingScale = 0.70; - CardLayout::Input inputFor(const QModelIndex &index) { CardLayout::Input in; @@ -70,37 +64,9 @@ QRect CardDelegate::expanderRectFor(const QStyleOptionViewItem &option, .expanderRect; } -QSize CardDelegate::chipSize(const QFontMetrics &metrics, const QString &text, - bool own) +QSize CardDelegate::chipSize(const QFontMetrics &metrics, const QString &text) { - // The padding shrinks with the font for a sibling chip. Left fixed it is - // 18px around roughly 30px of text, so the chip stays wide while its - // letters shrink and the tier reads as "same chip, smaller text". - return own ? TagChip::sizeFor(metrics, text) - : TagChip::sizeFor(metrics, text, kSiblingPaddingScale); -} - -QColor CardDelegate::mutedChipColour(const QColor &chipColour) -{ - if (!chipColour.isValid()) - return chipColour; - - // Saturation only, and NOT a blend toward the background. The accent bar - // above records what blending toward Base costs: on a dark theme it lands - // on the background and the thing disappears. A chip is worse, because its - // fill also has to carry legible text on top of it. - // - // Hue is untouched, so a muted `signed` is still recognisably the same - // colour as a full-size `signed` elsewhere in the list. Lightness is - // untouched too, which is what keeps TagColors::textColourOn() picking the - // same text colour: draining saturation alone moves the fill toward grey - // without moving it toward either black or white, so contrast is preserved - // by construction rather than by hoping. - constexpr float kSaturationScale = 0.45f; - - float h = 0, s = 0, l = 0, a = 0; - chipColour.getHslF(&h, &s, &l, &a); - return QColor::fromHslF(h, s * kSaturationScale, l, a); + return TagChip::sizeFor(metrics, text); } QRect CardDelegate::fadeRectFor(const QRect &card, const QRect &innermostSpine) @@ -427,39 +393,24 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, // One tier since item 177: a conversation row draws the thread's own tags // and a message row draws its message's, so nothing on a card belongs to - // anything but the row. The sibling tier this switched fonts at is Task - // 3's to remove. - const int ownCount = tags.size(); - + // anything but the row. const QFont ownFont = CardLayout::smallFont(chrome.font); - const QFont siblingFont = CardLayout::siblingFont(chrome.font); const QFontMetrics ownMetrics(ownFont); - const QFontMetrics siblingMetrics(siblingFont); painter->save(); + painter->setFont(ownFont); int x = card.tagRect.left(); for (int i = 0; i < tags.size(); ++i) { - const bool own = i < ownCount; - const QFontMetrics &metrics = own ? ownMetrics : siblingMetrics; - - const QSize size = chipSize(metrics, tags.at(i), own); + const QSize size = chipSize(ownMetrics, tags.at(i)); if (x + size.width() > card.tagRect.right()) break; // Out of room; a clipped chip reads as a rendering fault. QColor colour = i < colours.size() ? colours.at(i).value() : QColor(0x55, 0x55, 0x5f); - if (!own) - colour = mutedChipColour(colour); - - // Bottom-aligned, so a smaller chip sits on the same baseline as its - // neighbours rather than floating in the middle of the row. Top - // alignment would step the tier down and read as a layout fault. - const int top = card.tagRect.top() - + (ownMetrics.height() - metrics.height()); - - painter->setFont(own ? ownFont : siblingFont); - TagChip::paint(painter, QRect(QPoint(x, top), size), tags.at(i), - colour); + + TagChip::paint(painter, + QRect(QPoint(x, card.tagRect.top()), size), + tags.at(i), colour); x += size.width() + TagChip::kSpacing; } painter->restore(); diff --git a/src/carddelegate.h b/src/carddelegate.h index cfbd23e..528381c 100644 --- a/src/carddelegate.h +++ b/src/carddelegate.h @@ -95,23 +95,12 @@ public: /// colour reaches. static constexpr qreal kFadeFraction = 0.60; - /// A tag chip's colour, drained for the SIBLING tier (item 111). - /// - /// Saturation only: hue stays so the tag is still recognisable, and - /// lightness stays so `TagColors::textColourOn()` keeps choosing the same - /// text colour and the chip cannot become unreadable. Exposed for a test, - /// since "muted" has to be asserted on rather than eyeballed. - static QColor mutedChipColour(const QColor &chipColour); - - /// The size of one tag chip on a card, for either tier. + /// The size of one tag chip on a card, at full size. /// /// The delegate's own arithmetic rather than a duplicate of it: a test /// calling `TagChip::sizeFor` directly proves what that function does and - /// nothing about what the delegate ASKS for, which is where the padding - /// scale is chosen. A mutation dropping the scale at the call site - /// survived exactly that kind of test. - static QSize chipSize(const QFontMetrics &metrics, const QString &text, - bool own); + /// nothing about what the delegate ASKS for. + static QSize chipSize(const QFontMetrics &metrics, const QString &text); /// The squircle for one row, resolved from what the model supplies. /// diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp index a9bbe52..ec22635 100644 --- a/src/cardlayout.cpp +++ b/src/cardlayout.cpp @@ -107,32 +107,6 @@ QFont CardLayout::smallFont(const QFont &cardFont) return small; } -QFont CardLayout::siblingFont(const QFont &cardFont) -{ - // A FRACTION of the card's font, not a fixed number of points off it. - // - // Subtracting one point was the first attempt and the user reported the - // tiers as indistinguishable. The reason is arithmetic: their desktop is - // 14pt, so the two chip tiers were 13 and 12, a 7% step. Subtraction gives - // a step whose size depends on the desktop font, which is exactly backwards - // — it is largest where the text is already small enough to be fragile. - // - // 0.70 of the card font, against smallFont()'s one point off, so on a 14pt - // desktop the tiers are 13 and 10. Chosen with the user against rendered - // sizes rather than picked. - constexpr qreal kSiblingScale = 0.70; - - QFont small = cardFont; - // pointSizeF() returns -1 for a font set in PIXELS, which qt6ct does, and - // scaling -1 asks for an invalid size that Qt silently ignores, leaving - // both tiers identical. Same split as smallFont(), same reason. - if (small.pointSizeF() > 0.0) - small.setPointSizeF(qMax(6.0, cardFont.pointSizeF() * kSiblingScale)); - else if (small.pixelSize() > 0) - small.setPixelSize(qMax(8, qRound(cardFont.pixelSize() * kSiblingScale))); - return small; -} - int CardLayout::heightFor(const QFont &font) { const QFontMetrics metrics(font); diff --git a/src/cardlayout.h b/src/cardlayout.h index cd4eb76..57edca7 100644 --- a/src/cardlayout.h +++ b/src/cardlayout.h @@ -174,19 +174,6 @@ struct CardLayout /// column of content. static QFont smallFont(const QFont &cardFont); - /// The font a SIBLING's tag chip is drawn in: a size down again from - /// smallFont(). - /// - /// A card stands for one message but sits above a conversation, and shows - /// both tiers (item 111). Size is what says which is which, so the two - /// must be visibly different; taking one more step from the same base - /// keeps it following the desktop's font rather than being fixed. - /// - /// Floored like smallFont(), and the floor really is reachable: a desktop - /// at the minimum size gives both tiers the same size, which is a legible - /// degradation rather than an illegible chip. - static QFont siblingFont(const QFont &cardFont); - static CardLayout compute(const Input &input, const QRect &rect, const QFont &font); diff --git a/tests/test_carddelegate.cpp b/tests/test_carddelegate.cpp index bb21ae1..e23fa77 100644 --- a/tests/test_carddelegate.cpp +++ b/tests/test_carddelegate.cpp @@ -33,9 +33,6 @@ private slots: void theAccentLiftsAMutedAccountColour(); void theAccentKeepsEachAccountTellableApart(); void anAccountWithNoColourFallsBackToTheNeutralLine(); - void aSiblingChipIsMutedButStaysLegibleAndRecognisable(); - void aSiblingChipFontIsSmallerThanItsOwnTier(); - void aSiblingChipsPaddingShrinksWithItsFont(); void theFadeEndsAtSixtyPercentOfTheCard(); void aReplyFadeStartsAtItsOwnSpine(); void theDelegateAsksForAScaledSquircle(); @@ -141,128 +138,6 @@ 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"); -} - void TestCardDelegate::theFadeEndsAtSixtyPercentOfTheCard() { const QRect card(0, 0, 500, 60); -- cgit v1.2.3