From ecd363cd439442029229f45aa6db7d760cc41e1a Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 10 Aug 2026 08:37:22 +0200 Subject: feat(view): paint the whole card in one delegate Replaces SubjectDelegate. The tag chips come home from the view: the strip was painted there only because a delegate cannot paint outside its column and the strip spanned all five, and with one column there is nothing to span. RowStyleDelegate is inherited rather than dropped. Its job survives the redesign: Qt resolves ForegroundRole into the palette's Text roles and prefers those over HighlightedText, so the read/unread dimming would win on a selected row and land as grey on the highlight. What it loses is the rest of its body, which aligned cells against a text band and centred two marker columns; both described a grid that no longer exists. A reply's Re: prefix is stripped here. Every reply repeating the thread's subject is the visual signature of a table of records, which is the thing item 53 is about. The account chip becomes a bar down the card's left edge, and the reply spines inherit its colour, so an expanded thread is bounded by one accent from its root to its last reply without a second line in the gutter. Neither uses the raw account colour: that colour is chosen to be a chip's fill with legible text on top, and the same value as a thin line has to be followable down an expansion without competing with the senders, so it is blended toward the palette's Base by the weight threadLineColour() already uses. A reply resolves its THREAD's colour by walking to the root, since AccountColourRole is empty on a message row and a neutral spine under an accented root would break the continuous edge. The build is red at this commit; the view and window still name the old delegate. --- src/carddelegate.h | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 src/carddelegate.h (limited to 'src/carddelegate.h') diff --git a/src/carddelegate.h b/src/carddelegate.h new file mode 100644 index 0000000..317c78a --- /dev/null +++ b/src/carddelegate.h @@ -0,0 +1,68 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * 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. + */ + +#pragma once + +#include "tagchip.h" + +/// Paints a whole card: three lines, all of it, including the tag chips. +/// +/// It replaces both SubjectDelegate and ThreadListView::paintEvent. The view +/// used to paint the tag strip because a delegate cannot paint outside its +/// column and the strip spanned all five; with one column there is nothing to +/// span, so the strip comes home to the delegate and the view stops painting +/// entirely. That removes the two failure modes CLAUDE.md records for the +/// strip, a deleted row cut in half and every other row showing a bare stripe, +/// both of which existed because the view had to re-honour alternating +/// colours, the selection and BackgroundRole across cells it did not own. +/// +/// Inherits RowStyleDelegate for its one job, which still matters: Qt resolves +/// Qt::ForegroundRole into the palette's Text roles and then prefers those over +/// HighlightedText, so the read/unread dimming would otherwise win on a +/// selected row and land as grey on the highlight colour. +class CardDelegate : public RowStyleDelegate +{ + Q_OBJECT +public: + using RowStyleDelegate::RowStyleDelegate; + + void paint(QPainter *painter, const QStyleOptionViewItem &option, + const QModelIndex &index) const override; + QSize sizeHint(const QStyleOptionViewItem &option, + const QModelIndex &index) const override; + + /// The expander's rect for a row, so the VIEW can hit-test a click without + /// duplicating the layout. The delegate draws it and the view owns the + /// click, because a delegate gets no click of its own without an editor. + static QRect expanderRectFor(const QStyleOptionViewItem &option, + const QModelIndex &index); + + /// An account's colour as a thin LINE rather than as a chip's fill. + /// + /// Never use the raw account colour for the accent bar or the spine. That + /// colour is chosen to be a background with legible text drawn on top + /// (TagColors::textColourOn picks black or white against it). The same + /// colour as a few pixels of line on the pane's own background is a + /// different problem: it has to be followable down a long expansion + /// WITHOUT competing with the senders beside it, which is the constraint + /// threadLineColour() states and meets by blending 0.35 toward the + /// palette's text. This blends the account colour toward the palette's + /// Base by the same weight, keeping the hue that identifies the account + /// and dropping the saturation that would shout. + static QColor accentLineColour(const QColor &accountColour); +}; -- cgit v1.2.3 From 8767e8d33f5065ff57d7abd5bc916dbb13ada2d5 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 10 Aug 2026 08:58:48 +0200 Subject: fix(view): stop clipping the date, and make the accent bar visible Both found by rendering real cards to an image and looking at them, not by any assertion. The suite was green through both. The date lost the leading digit of its year on every UNREAD card. The layout reserves the date's width from the font it is handed, which is the view's regular font, while the delegate paints with the bold one the model supplies for unread: 154px reserved against 170px needed. CardLayout now measures the date bold whatever font it is given, so the reserved width cannot be narrower than what is drawn. A few pixels are wasted on a read card, which is the cheap side of the trade. The accent bar was painted correctly and was invisible. Blending the account colour 0.35 toward the palette's Base, as the plan specified, is a fraction OF THE ACCOUNT COLOUR, so on a dark theme it produced (0.18, 0.22, 0.26) against a Base of (0.169, 0.169, 0.169): the background. The blend is dropped entirely. An account colour is already chosen to be a chip's fill carrying legible text, so it is muted to begin with, and nothing is drawn on the bar that needs that contrast. The spine keeps a blend, at 0.55, because it runs the full height of every reply in an expansion and is a different problem from a 3px edge marker. The bar is still faint at 3px on a dark theme, since the account colours are chosen as chip fills. Whether kAccentWidth needs raising cannot be settled without the user's own accounts, screen and theme; that is Task 10's open question and it is left open. --- src/carddelegate.cpp | 44 +++++++++++++++++++++++++++++++------------- src/carddelegate.h | 26 +++++++++++++++----------- src/cardlayout.cpp | 11 ++++++++++- tests/test_cardlayout.cpp | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 25 deletions(-) (limited to 'src/carddelegate.h') diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp index bc03b5a..a3a846d 100644 --- a/src/carddelegate.cpp +++ b/src/carddelegate.cpp @@ -53,15 +53,21 @@ QColor CardDelegate::accentLineColour(const QColor &accountColour) if (!accountColour.isValid()) return ThreadListModel::threadLineColour(); - // The same 0.35 weight threadLineColour() uses, toward Base rather than - // toward Text, so the two kinds of line sit at the same visual strength. - const QColor base = QGuiApplication::palette().color(QPalette::Base); - constexpr qreal kWeight = 0.35; - const qreal inverse = 1.0 - kWeight; - return QColor::fromRgbF( - accountColour.redF() * kWeight + base.redF() * inverse, - accountColour.greenF() * kWeight + base.greenF() * inverse, - accountColour.blueF() * kWeight + base.blueF() * inverse); + // 0.35 toward Base was the first attempt and produced an INVISIBLE bar on + // a dark theme: rendered 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, 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. + // + // 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; } QSize CardDelegate::sizeHint(const QStyleOptionViewItem &option, @@ -110,10 +116,22 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, if (!card.accentRect.isEmpty()) painter->fillRect(card.accentRect, lineColour); - // Spines, under everything else, in the same accent so an expanded thread - // is bounded by one colour from its root to its last reply. - for (const QRect &spine : card.spines) - painter->fillRect(spine, lineColour); + // Spines, under everything else, in the account's hue so an expanded thread + // is bounded by one colour from its root to its last reply. Muted against + // the pane's own background, unlike the accent bar: this line runs the full + // height of every reply and at full strength it shouts. + if (!card.spines.isEmpty()) { + const QColor base = + QGuiApplication::palette().color(QPalette::Base); + constexpr qreal kSpineWeight = 0.55; + const qreal inverse = 1.0 - kSpineWeight; + const QColor spineColour = QColor::fromRgbF( + lineColour.redF() * kSpineWeight + base.redF() * inverse, + lineColour.greenF() * kSpineWeight + base.greenF() * inverse, + lineColour.blueF() * kSpineWeight + base.blueF() * inverse); + for (const QRect &spine : card.spines) + painter->fillRect(spine, spineColour); + } // Selection outranks the model's foreground, and the order matters: a read // card carries a dimmed colour blended against the UNSELECTED background, diff --git a/src/carddelegate.h b/src/carddelegate.h index 317c78a..7012eaa 100644 --- a/src/carddelegate.h +++ b/src/carddelegate.h @@ -52,17 +52,21 @@ public: static QRect expanderRectFor(const QStyleOptionViewItem &option, const QModelIndex &index); - /// An account's colour as a thin LINE rather than as a chip's fill. + /// The colour the accent bar is painted in: the account's own, undiluted. /// - /// Never use the raw account colour for the accent bar or the spine. That - /// colour is chosen to be a background with legible text drawn on top - /// (TagColors::textColourOn picks black or white against it). The same - /// colour as a few pixels of line on the pane's own background is a - /// different problem: it has to be followable down a long expansion - /// WITHOUT competing with the senders beside it, which is the constraint - /// threadLineColour() states and meets by blending 0.35 toward the - /// palette's text. This blends the account colour toward the palette's - /// Base by the same weight, keeping the hue that identifies the account - /// and dropping the saturation that would shout. + /// 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. + /// + /// 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. + /// + /// Falls back to threadLineColour() for a thread with no account tag. static QColor accentLineColour(const QColor &accountColour); }; diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp index 8d952b2..1a79e3b 100644 --- a/src/cardlayout.cpp +++ b/src/cardlayout.cpp @@ -87,7 +87,16 @@ CardLayout CardLayout::compute(const Input &input, const QRect &rect, // The date is measured first and the sender gets what is left, so a long // sender is elided rather than painting over the date. - const int dateWidth = metrics.horizontalAdvance( + // + // Measured BOLD whatever font this is handed. An unread card draws bold and + // the delegate computes its layout from the view's regular font, so a rect + // sized regular clips a bold date: 154px reserved against 170px needed, one + // digit of the year gone from every unread card. Reserving the wider of the + // two costs a few pixels on a read card and cannot disagree with what is + // painted. + QFont dateFont = font; + dateFont.setBold(true); + const int dateWidth = QFontMetrics(dateFont).horizontalAdvance( QStringLiteral("8888-88-88 88:88")); out.dateRect = QRect(right - dateWidth, lineOneTop, dateWidth, metrics.height()); diff --git a/tests/test_cardlayout.cpp b/tests/test_cardlayout.cpp index 1082f4c..c38f728 100644 --- a/tests/test_cardlayout.cpp +++ b/tests/test_cardlayout.cpp @@ -35,6 +35,7 @@ private slots: void dateIsFlushRight(); void threadCardCarriesAnAccentBar(); void replyCardCarriesNoAccentBar(); + void theDateFitsWhenTheCardIsBold(); }; namespace { @@ -231,5 +232,41 @@ void TestCardLayout::replyCardCarriesNoAccentBar() QCOMPARE(reply.spines.size(), 1); } +void TestCardLayout::theDateFitsWhenTheCardIsBold() +{ + // An UNREAD card draws BOLD, and bold is wider. The layout is computed from + // option.font, which is the view's regular font, while the text is painted + // with the font initStyleOption resolved from the model's Qt::FontRole. So + // a date measured regular and drawn bold overflows its rect: measured at + // 154px reserved against 170px needed, which clipped the leading digit of + // the year off every unread card. + // + // The fix is in the layout rather than in the delegate: it reserves the + // BOLD width whatever font it is handed, so the two can never disagree. + QFont regular; + regular.setBold(false); + QFont bold = regular; + bold.setBold(true); + + const QString sample = QStringLiteral("2025-08-10 06:26"); + const int boldWidth = QFontMetrics(bold).horizontalAdvance(sample); + + // Guard: bold must actually be wider here, or this asserts nothing. + QVERIFY2(boldWidth > QFontMetrics(regular).horizontalAdvance(sample), + "bold is not wider than regular in this environment, so this test " + "cannot detect the overflow it exists for"); + + const int h = CardLayout::heightFor(regular); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), regular); + + QVERIFY2(card.dateRect.width() >= boldWidth, + qPrintable(QStringLiteral("a layout computed from the REGULAR " + "font reserves %1px, and the date needs " + "%2px when the card draws bold") + .arg(card.dateRect.width()) + .arg(boldWidth))); +} + QTEST_MAIN(TestCardLayout) #include "test_cardlayout.moc" -- cgit v1.2.3