diff options
| -rw-r--r-- | src/carddelegate.cpp | 34 | ||||
| -rw-r--r-- | src/carddelegate.h | 31 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_carddelegate.cpp | 135 |
4 files changed, 180 insertions, 21 deletions
diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp index f13205e..bce9eec 100644 --- a/src/carddelegate.cpp +++ b/src/carddelegate.cpp @@ -58,16 +58,32 @@ QColor CardDelegate::accentLineColour(const QColor &accountColour) // at (0.18, 0.22, 0.26), which is the background. The weight is a fraction // OF THE ACCOUNT COLOUR, so a low one keeps the background, not the hue. // - // The bar is the account's colour, undiluted. Blending it toward Base at - // all was the mistake: a chip's colour is chosen to carry text on top and - // is therefore already muted, and three pixels of a muted colour on a dark - // background is nothing at all. There is no text on this bar, so nothing - // needs the contrast a chip's fill was picked for. + // The account's own hue, lifted to a floor of saturation and lightness. // - // What DOES step back is the spine, below: a line running the height of a - // whole expansion has to be followable without competing with the senders - // beside it, which is a different problem from a 3px edge marker. - return accountColour; + // Blending toward Base was the first mistake and is long gone: a chip's + // colour is already muted, since it is chosen to carry legible text on top, + // and three pixels of a muted colour is nothing. Handing the raw colour + // through was the second: it is better, but the five real accounts are all + // mid-tone by construction and still read as faint stripes on a dark theme. + // + // A FLOOR rather than a repaint. A colour already past it is returned + // untouched, so a user who deliberately picked something vivid keeps + // exactly what they picked, and only the muted ones move. Hue is never + // touched at all, because hue is the entire information the bar carries: + // shifting it would make a bar stop matching its account's chip and its + // swatch in the dropdown. + // + // The numbers were chosen by rendering all five accounts as 3px bars on + // both a dark and a light card background and looking. Higher pushed the + // green toward a neon that no longer matched its own chip; lower left it + // where it started. + constexpr float kMinSaturation = 0.65f; + constexpr float kMinLightness = 0.50f; + + float h = 0, s = 0, l = 0, a = 0; + accountColour.getHslF(&h, &s, &l, &a); + return QColor::fromHslF(h, qMax(s, kMinSaturation), + qMax(l, kMinLightness), a); } QSize CardDelegate::sizeHint(const QStyleOptionViewItem &option, diff --git a/src/carddelegate.h b/src/carddelegate.h index 7012eaa..74dee8e 100644 --- a/src/carddelegate.h +++ b/src/carddelegate.h @@ -52,20 +52,27 @@ public: static QRect expanderRectFor(const QStyleOptionViewItem &option, const QModelIndex &index); - /// The colour the accent bar is painted in: the account's own, undiluted. + /// The colour the accent bar is painted in: the account's hue, lifted to a + /// floor of saturation and lightness. /// - /// Blending it toward the palette's Base was tried first, at the 0.35 - /// weight threadLineColour() uses, and produced an INVISIBLE bar on a dark - /// theme: against a Base of (0.169, 0.169, 0.169) it landed at (0.18, 0.22, - /// 0.26), which is the background. The weight is a fraction OF THE ACCOUNT - /// COLOUR, so a low one keeps the background rather than the hue. + /// Two earlier versions were wrong in opposite directions. Blending toward + /// the palette's Base at threadLineColour()'s 0.35 weight produced an + /// INVISIBLE bar on a dark theme, landing at (0.18, 0.22, 0.26) against a + /// Base of (0.169, 0.169, 0.169): the weight is a fraction OF THE ACCOUNT + /// COLOUR, so a low one keeps the background rather than the hue. Passing + /// the raw colour through fixed that and was still too quiet, because an + /// account colour is chosen as a CHIP's fill with legible text on top and + /// is therefore mid-tone by construction. /// - /// An account colour is already chosen to be a chip's fill with legible - /// text on top, so it is muted to begin with; three pixels of a muted - /// colour is nothing. Nothing is drawn on this bar, so it needs none of the - /// contrast that choice was made for. The SPINE is where the muting belongs - /// and is blended in paint(): it runs the full height of every reply in an - /// expansion and has to be followable without competing with the senders. + /// A floor, not a repaint: a colour already past it is returned untouched, + /// so a deliberately vivid choice is preserved. Hue is never altered, since + /// hue is the whole information the bar carries and a shifted one would + /// stop matching the account's chip and its swatch in the dropdown. + /// + /// The SPINE takes its colour from this and mutes it again in paint(): it + /// runs the full height of every reply in an expansion and has to be + /// followable without competing with the senders, which is a different + /// problem from a 3px edge marker. /// /// Falls back to threadLineColour() for a thread with no account tag. static QColor accentLineColour(const QColor &accountColour); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4ecc04d..13063e9 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(carddelegate) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) add_qtmaildir_test(syncmonitor) diff --git a/tests/test_carddelegate.cpp b/tests/test_carddelegate.cpp new file mode 100644 index 0000000..d5e55b5 --- /dev/null +++ b/tests/test_carddelegate.cpp @@ -0,0 +1,135 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * 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 "carddelegate.h" + +#include "threadlistmodel.h" + +#include <QTest> + +class TestCardDelegate : public QObject +{ + Q_OBJECT + +private slots: + void theAccentLiftsAMutedAccountColour(); + void theAccentKeepsEachAccountTellableApart(); + void anAccountWithNoColourFallsBackToTheNeutralLine(); +}; + +namespace { + +/// The user's five configured account colours, as a realistic sample. They are +/// all mid-tone because they were chosen as CHIP fills, which is the whole +/// reason the bar has to lift them. +const QList<QColor> &sampleAccounts() +{ + static const QList<QColor> colours{ + QColor("#2f6fa8"), QColor("#3d7a4a"), QColor("#a83f2f"), + QColor("#8a5cb8"), QColor("#b8862f"), + }; + return colours; +} + +float saturationOf(const QColor &c) +{ + float h = 0, s = 0, l = 0, a = 0; + c.getHslF(&h, &s, &l, &a); + return s; +} + +float lightnessOf(const QColor &c) +{ + float h = 0, s = 0, l = 0, a = 0; + c.getHslF(&h, &s, &l, &a); + return l; +} + +float hueOf(const QColor &c) +{ + float h = 0, s = 0, l = 0, a = 0; + c.getHslF(&h, &s, &l, &a); + return h; +} + +} // namespace + +void TestCardDelegate::theAccentLiftsAMutedAccountColour() +{ + // An account colour is chosen to be a chip's fill with legible text on top, + // so it is mid-tone by construction. Three pixels of a mid-tone colour + // beside a card's own background barely register, which is what the user + // reported: "the colours could be a little more vivid". + // + // The green is the weakest of the five (S 0.33, L 0.36) and is the one that + // has to move most. + const QColor muted("#3d7a4a"); + const QColor accent = CardDelegate::accentLineColour(muted); + + QVERIFY2(saturationOf(accent) > saturationOf(muted), + "the accent is no more saturated than the chip colour it comes " + "from, so a muted account stays muted as a 3px bar"); + QVERIFY2(lightnessOf(accent) > lightnessOf(muted), + "the accent is no lighter than the chip colour, so it cannot " + "carry on a dark theme"); + + // The floor, stated as the numbers that were chosen by rendering all five + // against both themes. Higher pushed the green toward a neon that no + // longer matched its own chip. + QVERIFY(saturationOf(accent) >= 0.65f - 0.01f); + QVERIFY(lightnessOf(accent) >= 0.50f - 0.01f); + + // A colour already past the floor is left alone: the lift is a floor, not + // a repaint, or a user who picked a vivid colour would have it changed. + const QColor alreadyVivid = QColor::fromHslF(0.6f, 0.9f, 0.6f); + const QColor untouched = CardDelegate::accentLineColour(alreadyVivid); + QCOMPARE(saturationOf(untouched), saturationOf(alreadyVivid)); + QCOMPARE(lightnessOf(untouched), lightnessOf(alreadyVivid)); +} + +void TestCardDelegate::theAccentKeepsEachAccountTellableApart() +{ + // The bar's whole job is saying WHICH account, so the lift must not + // converge two hues. Asserted across the real five rather than one pair: + // a floor applied to saturation and lightness leaves hue untouched, and + // this is what proves it stayed that way. + for (const QColor &configured : sampleAccounts()) { + const QColor accent = CardDelegate::accentLineColour(configured); + QVERIFY2(qAbs(hueOf(accent) - hueOf(configured)) < 0.01f, + qPrintable(QStringLiteral("account %1 changed hue to %2, so " + "it no longer matches its own chip") + .arg(configured.name(), accent.name()))); + } + + // And no two of them collapse onto each other. + QSet<QRgb> seen; + for (const QColor &configured : sampleAccounts()) + seen.insert(CardDelegate::accentLineColour(configured).rgb()); + QCOMPARE(seen.size(), sampleAccounts().size()); +} + +void TestCardDelegate::anAccountWithNoColourFallsBackToTheNeutralLine() +{ + // A thread with no account tag has no colour to lift, and must not end up + // with a saturated bar invented out of an invalid QColor. + QCOMPARE(CardDelegate::accentLineColour(QColor()), + ThreadListModel::threadLineColour()); +} + +QTEST_MAIN(TestCardDelegate) +#include "test_carddelegate.moc" |